Received-SPF: pass (google.com: domain of linux-kernel+bounces-166774-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 2 May 2024 20:20:01 +0300
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Ilpo =?iso-8859-1?Q?J=E4rvinen?= <ilpo.jarvinen@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()
Message-ID: <ZjPLQeTEKvt7B3mj@smile.fi.intel.com>
References: <20240502144626.2716994-1-andriy.shevchenko@linux.intel.com>
 <20240502144626.2716994-12-andriy.shevchenko@linux.intel.com>
 <702a9145-5bc1-c765-a1fa-278702741637@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <702a9145-5bc1-c765-a1fa-278702741637@linux.intel.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

On Thu, May 02, 2024 at 07:08:21PM +0300, Ilpo J?rvinen wrote:
> On Thu, 2 May 2024, Andy Shevchenko wrote:

..

> >  	// 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?

Oh, seems a good catch!

I was also wondering, but missed this somehow. Seems the EEPROM is in BE mode,
so two loops has to be aligned.

-- 
With Best Regards,
Andy Shevchenko