Received-SPF: pass (google.com: domain of linux-kernel+bounces-166694-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Thu, 2 May 2024 19:08:21 +0300 (EEST)
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
cc: Parker Newman <pnewman@connecttech.com>, 
    LKML <linux-kernel@vger.kernel.org>, 
    linux-serial <linux-serial@vger.kernel.org>, 
    Greg Kroah-Hartman <gregkh@linuxfoundation.org>, 
    Jiri Slaby <jirislaby@kernel.org>
Subject: Re: [PATCH v1 11/13] serial: 8250_exar: Use BIT() in
 exar_ee_read()
In-Reply-To: <20240502144626.2716994-12-andriy.shevchenko@linux.intel.com>
Message-ID: <702a9145-5bc1-c765-a1fa-278702741637@linux.intel.com>
References: <20240502144626.2716994-1-andriy.shevchenko@linux.intel.com> <20240502144626.2716994-12-andriy.shevchenko@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII

On Thu, 2 May 2024, Andy Shevchenko wrote:

> Use BIT() in exar_ee_read() like other functions do.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/tty/serial/8250/8250_exar.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> index 306bc6d7c141..bf3730f4231d 100644
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -340,13 +340,13 @@ static u16 exar_ee_read(struct exar8250 *priv, u8 ee_addr)
>  	exar_ee_write_bit(priv, 0);
>  
>  	// Send address to read from
> -	for (i = 1 << (UART_EXAR_REGB_EE_ADDR_SIZE - 1); i; i >>= 1)
> -		exar_ee_write_bit(priv, (ee_addr & i));
> +	for (i = UART_EXAR_REGB_EE_ADDR_SIZE - 1; i >= 0; i--)
> +		exar_ee_write_bit(priv, ee_addr & BIT(i));
>  
>  	// Read data 1 bit at a time
>  	for (i = 0; i <= UART_EXAR_REGB_EE_DATA_SIZE; i++) {
> -		data <<= 1;
> -		data |= exar_ee_read_bit(priv);
> +		if (exar_ee_read_bit(priv))
> +			data |= BIT(i);

Does this end up reversing the order of bits? In the original, data was 
left shifted which moved the existing bits and added the lsb but the 
replacement adds highest bit on each iteration?

-- 
 i.