Re: [PATCH] usb: ohci-nxp: add support for stotg04 phy

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Jul 29, 2020 at 09:00:04PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 7/29/20 8:49 PM, Alexandre Belloni wrote:
> 
> > The STOTG04 phy is used as a drop-in replacement of the ISP1301 but some
> > bits doesn't have exactly the same meaning and this can lead to issues.
> > Detect the phy dynamically and avoid writing to reserved bits.
> > 
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > ---
> > 
> > Hi Trevor, this is totally untested but at least it builds ;)
> > 
> >  drivers/usb/host/ohci-nxp.c | 21 +++++++++++++++------
> >  1 file changed, 15 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/usb/host/ohci-nxp.c b/drivers/usb/host/ohci-nxp.c
> > index 85878e8ad331..36ab1501c28f 100644
> > --- a/drivers/usb/host/ohci-nxp.c
> > +++ b/drivers/usb/host/ohci-nxp.c
> > @@ -55,6 +55,15 @@ static struct clk *usb_host_clk;
> >  
> >  static void isp1301_configure_lpc32xx(void)
> >  {
> > +	u8 value, atx_is_stotg = 0;
> 
>    Why the flag is not *bool*?

That's not an issue so much as:

> 
> > +	s32 vendor, product;
> > +
> > +	vendor = i2c_smbus_read_word_data(isp1301_i2c_client, 0x00);
> > +	product = i2c_smbus_read_word_data(isp1301_i2c_client, 0x02);

Why are these signed 32bit numbers?  Shouldn't they be unsigned?

> > +
> > +	if (vendor == 0x0483 && product == 0xa0c4)

No endian flips anywhere?

thanks,

greg k-h