On Wed, Oct 24, 2018 at 05:43:27PM +0000, Ajay Gupta wrote:
> Hi Andy
>
> > > > > Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller over
> > > > > I2C interface.
> > > > >
> > > > > This UCSI I2C driver uses I2C bus driver interface for
> > > > > communicating with Type-C controller.
> >
> > > > > + /*
> > > > > + * Flush CCGx RESPONSE queue by acking interrupts. Above ucsi
> > > > control
> > > > > + * register write will push response which must be cleared.
> > > > > + */
> > > > > + status = ccg_read(uc, CCGX_RAB_INTR_REG, &data, sizeof(data));
> > > > > + if (status < 0)
> > > > > + return status;
> >
> > (1)
> >
> > > > > + do {
> > > > > + status = ccg_write(uc, CCGX_RAB_INTR_REG, &data,
> > > > sizeof(data));
> > > > > + if (status < 0)
> > > > > + return status;
> >
> > (2)
> >
> > > > > +
> > > > > + usleep_range(10000, 11000);
> > > > > +
> > > > > + status = ccg_read(uc, CCGX_RAB_INTR_REG, &data,
> > > > sizeof(data));
> > > > > + if (status < 0)
> > > > > + return status;
> > > > > + } while ((data != 0x00) && count--);
> > > >
> > > > What's the significance of that count?
> > > It is like a retry count to clear interrupt status.
> > >
> > > > Shouldn't you return -ETIMEDOUT if count == 0?
> > > Yes. Good catch. Does the below fix looks ok?
> >
> > At least for me looks OK (I dunno why I missed that what Heikki found
> > recently).
> > Nevertheless, I have one more question about (1) and (2) above.
> >
> > Is it necessary to do one more read before do-while loop?
> Yes, we need to read to get interrupt status bits and then write
> them back to clear the status.
Ah, I see, but why you not reorganize it to put this into do-while loop?
do {
read
write
check for data
sleep
} while (--count);
Also note predecrement.
--
With Best Regards,
Andy Shevchenko
