Re: [PATCH] usb: typec: ucsi_ccg: use dev_info for timeout error

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Sep 21, 2020 at 08:50:04PM +0000, Ajay Gupta wrote:
> Hi Greg
> 
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Monday, September 21, 2020 11:55 AM
> > To: Ajay Gupta <ajaykuee@gmail.com>
> > Cc: heikki.krogerus@linux.intel.com; linux-usb@vger.kernel.org; Ajay Gupta
> > <ajayg@nvidia.com>
> > Subject: Re: [PATCH] usb: typec: ucsi_ccg: use dev_info for timeout error
> > 
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Mon, Sep 21, 2020 at 10:42:16AM -0700, Ajay Gupta wrote:
> > > From: Ajay Gupta <ajayg@nvidia.com>
> > >
> > > Timeout error may be seen due to missing USB Type-C UCSI controller on
> > > some NVIDIA GPU card. Currently we don't have a correct way to
> > > identify these cards.
> > 
> > No unique id anywhere???  That feels like someone really messed up :(
> Yes, there is none.

Can you identify them by the failure of an i2c transfer?  Feels like
that would work :)

> > > Tools like Plymouth (splashscreen) doesn't like dev_err so changing
> > > timeout status log to dev_info.
> > >
> > > Bug information:
> > > https://bugzilla.kernel.org/show_bug.cgi?id=206653
> > >
> > > Signed-off-by: Ajay Gupta <ajayg@nvidia.com>
> > > ---
> > >  drivers/usb/typec/ucsi/ucsi_ccg.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
> > > b/drivers/usb/typec/ucsi/ucsi_ccg.c
> > > index bff96d64dddf..d69432df866e 100644
> > > --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
> > > +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> > > @@ -252,7 +252,7 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8
> > *data, u32 len)
> > >               put_unaligned_le16(rab, buf);
> > >               status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> > >               if (status < 0) {
> > > -                     dev_err(uc->dev, "i2c_transfer failed %d\n", status);
> > > +                     dev_info(uc->dev, "i2c_transfer failed %d\n",
> > > + status);
> > 
> > It's an error, leave it an error.
> > 
> > If it's not an error, don't log it.
> Looks like this is the best option we have.

It is an error, so we log it, that's fine.  We are correctly reporting
the fact that the hardware is broken and not working properly, don't
paper over that please.

thanks,

greg k-h