On Tue, Oct 23, 2018 at 06:56:59PM +0000, Ajay Gupta wrote:
> > On Wed, Oct 03, 2018 at 11:27:28AM -0700, Ajay Gupta wrote:
> > > 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?
>
> do {
> status = ccg_write(uc, CCGX_RAB_INTR_REG, &data, sizeof(data));
> if (status < 0)
> return status;
>
> usleep_range(10000, 11000);
>
> status = ccg_read(uc, CCGX_RAB_INTR_REG, &data, sizeof(data));
> if (status < 0)
> return status;
>
> if (!data)
> return 0;
> } while (data && count--);
>
> return -ETIMEDOUT;
--
With Best Regards,
Andy Shevchenko
