Re: [PATCH 1/3] Use local version of readl & writel

[Jean-Hugues Deschenes <jean-hugues.deschenes-YGVykHU+fedBDgjK7y7TUQ@public.gmane.org>]

Shinya Kuribayashi wrote:
> Jean-Hugues Deschenes wrote:
>> Use local versions of readl & writel, so per-access manipulations may 
>> be performed
>
> Let's put Signed-off-by: here.
My apologies; I need to figure out where in the process that was droppted...
>
>> ---
>>  drivers/i2c/busses/i2c-designware.c |   88 
>> ++++++++++++++++++++----------------
>>  1 file changed, 49 insertions(+), 39 deletions(-)
>>
>> Index: linux-2.6_i2c/drivers/i2c/busses/i2c-designware.c
>> ===================================================================
>> --- linux-2.6_i2c.orig/drivers/i2c/busses/i2c-designware.c
>> +++ linux-2.6_i2c/drivers/i2c/busses/i2c-designware.c
>> @@ -219,6 +219,16 @@ struct dw_i2c_dev {
>>      unsigned int        rx_fifo_depth;
>>  };
>>
>> +static u32 i2c_dw_readl(struct dw_i2c_dev *dev, int addr)
>> +{
>> +    return readl(dev->base + addr);
>> +}
>
> nit: As being used with dev->base, "offset", "reg" or something
> like that would be nice, rather than "addr".
Good idea; It'll be in the updated patch...
>
>> +static void i2c_dw_writel(struct dw_i2c_dev *dev, u32 b, int addr)
>> +{
>> +    writel(b, dev->base + addr);
>> +}
>> +
>>  static u32
>>  i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
>>  {
>
> Since these are I/O accessros / wrappers, and don't provide any
> meaningful functions, how about simply dw_readl() / dw_writel(),
> similarly as other {read,write}[bwl] wrappers do.  I'd prefer a
> reasonably shorter name for this kind of accessors!
... That should help with the code's readeability as well. Will be in 
the next patch

jh