lkml.org 
[lkml]   [2017]   [Aug]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [RESEND PATCH v4 2/2] i2c: Add Spreadtrum I2C controller driver
Hi Wolfram,

On 28 August 2017 at 23:13, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> >> + /*
>> >> + * If we did not get one ACK from slave when writing data, we should
>> >> + * dump all registers to check I2C status.
>> >
>> > Why? I would say no. NACK from a slave can always happen, e.g. when an
>> > EEPROM is busy erasing a page.
>>
>> For our I2C controller databook, if the master did not get one ACK
>> from slave when writing data to salve, we should send one STOP signal
>> to abort this data transfer or generate one repeated START signal to
>> start one new data transfer cycle. Considering our I2C usage
>
> Yes, so far so good.
>
>> scenarios, we should dump registers to analyze I2C status and notify
>> to user to re-start new data transfer.
>
> I disagree here. You notify the users by returning -EIO. The upper layer
> (e.g. the i2c client driver) will handle it, like the EEPROM driver
> might retry after a while. This all is expected behaviour, so no need to
> print the registers to the logfile.
>
> If you really, really want to keep it, make it debug output. But I think
> the sentence "we should dump all registers" needs to be rephrased.

Make sense. I will remove the registers printing here.

>
>> As I explained before, in our Spreadtrum platform, our regulator
>> driver will depend on I2C driver and the regulator driver uses
>> subsys_initcall() level to initialize. Moreover some other drivers
>> like GPU, they will depend on regulator to set voltage and they also
>> need initialization much earlier.
>>
>> Since it is arch_initcall() level, Andy suggested I should get rid of
>> tristate (use bool) and drop module.h here and all leftovers like
>> MODULE_*() calls including module_exit().
>
> I see. So the driver is really so essential for proper bootup that it is
> not even allowed to be unloaded. I might make an exception here and

Yes.

> allow arch_initcall() then. But I do wonder: did you try deferred
> probing all over the place?

Many modules (like GPU) need set voltage earlier by regulator which
depends on I2C ( or we also need regulate voltage for big-cores ASAP),
if we defer to set voltage for GPU or other modules, maybe will cause
some system problems. Thanks for your comments.

--
Baolin.wang
Best Regards

\
 
 \ /
  Last update: 2017-08-29 03:58    [W:0.103 / U:1.316 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site