[U-Boot] [PATCH v2 2/5] regmap: Support reading from specific range

[Mario Six]

Hi Simon,

On Thu, May 3, 2018 at 4:33 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Mario,
>
> On 27 April 2018 at 06:52, Mario Six <mario.six at gdsys.cc> wrote:
>>
>> It is useful to be able to treat the different ranges of a regmap
>> separately to be able to use distinct offset for them, but this is
>> currently not implemented in the regmap API.
>>
>> To preserve backwards compatibility, add regmap_read_ext and
>> regmap_write_ext functions that take an additional parameter 'range_num'
>> that identifies the range to operate on.
>>
>> Signed-off-by: Mario Six <mario.six at gdsys.cc>
>> ---
>>
>> v1 -> v2:
>> New in v2
>>
>> ---
>>  drivers/core/regmap.c | 69 +++++++++++++++++++++++++++++++++++++++------------
>>  include/regmap.h      |  2 ++
>>  2 files changed, 55 insertions(+), 16 deletions(-)
>>
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
>
>> diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
>> index c2cdff3979..80718b3820 100644
>> --- a/drivers/core/regmap.c
>> +++ b/drivers/core/regmap.c
>> @@ -130,47 +130,84 @@ int regmap_uninit(struct regmap *map)
>>         return 0;
>>  }
>>
>> -int regmap_read(struct regmap *map, ulong offset, ulong *valp,
>> -               enum regmap_size_t size)
>> +int regmap_read_ext(struct regmap *map, uint range_num, ulong offset,
>> +                   ulong *valp, enum regmap_size_t size)
>
> How about regmap_read_range() ?
>

Yes, sounds more descriptive. Will change for v3.

>>  {
>> +       struct regmap_range *range;
>>         void *ptr;
>>
>> +       if (range_num >= map->range_count)
>> +               return 0;
>> +       range = &map->range[range_num];
>> +
>> +       ptr = map_physmem(range->start + offset, size, MAP_NOCACHE);
>> +
>> +       if (offset + size > range->size) {
>> +               debug("%s: offset/size combination invalid\n", __func__);
>> +               return -EINVAL;
>> +       }
>> +
>>         switch (size) {
>>         case REGMAP_SIZE_8:
>> -               ptr = map_physmem(map->base + offset, 1, MAP_NOCACHE);
>> -               *valp = readb(ptr);
>> +               *((u8 *)valp) = readb((u8 *)ptr);
>>                 break;
>>         case REGMAP_SIZE_16:
>> -               ptr = map_physmem(map->base + offset, 2, MAP_NOCACHE);
>> -               *valp = le16_to_cpu(readw(ptr));
>> +               *((u16 *)valp) = readw((u16 *)ptr);
>>                 break;
>>         case REGMAP_SIZE_32:
>> -               ptr = map_physmem(map->base + offset, 4, MAP_NOCACHE);
>> -               *valp = le32_to_cpu(readl(ptr));
>> +               *((u32 *)valp) = readl((u32 *)ptr);
>>                 break;
>> +       default:
>> +               debug("%s: size %d not supported\n", __func__, size);
>> +               return -ENOTSUPP;
>>         }
>> +
>>         return 0;
>>  }
>>
>> -int regmap_write(struct regmap *map, ulong offset, ulong val,
>> -                enum regmap_size_t size)
>> +int regmap_read(struct regmap *map, ulong offset, ulong *valp,
>> +               enum regmap_size_t size)
>>  {
>> +       return regmap_read_ext(map, 0, offset, valp, size);
>> +}
>> +
>> +int regmap_write_ext(struct regmap *map, uint range_num, ulong offset,
>> +                    ulong val, enum regmap_size_t size)
>> +{
>> +       struct regmap_range *range;
>>         void *ptr;
>>
>> +       if (range_num >= map->range_count)
>> +               return 0;
>> +       range = &map->range[range_num];
>> +
>> +       ptr = map_physmem(range->start + offset, size, MAP_NOCACHE);
>> +
>> +       if (offset + size > range->size) {
>> +               debug("%s: offset/size combination invalid\n", __func__);
>> +               return -EINVAL;
>> +       }
>> +
>>         switch (size) {
>>         case REGMAP_SIZE_8:
>> -               ptr = map_physmem(map->base + offset, 1, MAP_NOCACHE);
>> -               writel((u8)val, ptr);
>> +               writeb((u8)val, (u8 *)ptr);
>>                 break;
>>         case REGMAP_SIZE_16:
>> -               ptr = map_physmem(map->base + offset, 2, MAP_NOCACHE);
>> -               writel(cpu_to_le16((u16)val), ptr);
>> +               out_le16((u16 *)ptr, (u16)val);
>>                 break;
>>         case REGMAP_SIZE_32:
>> -               ptr = map_physmem(map->base + offset, 4, MAP_NOCACHE);
>> -               writel(cpu_to_le32((u32)val), ptr);
>> +               writel((u32)val, (u32 *)ptr);
>>                 break;
>> +       default:
>> +               debug("%s: size %d not supported\n", __func__, size);
>> +               return -ENOTSUPP;
>>         }
>>
>>         return 0;
>>  }
>> +
>> +int regmap_write(struct regmap *map, ulong offset, ulong val,
>> +                enum regmap_size_t size)
>> +{
>> +       return regmap_write_ext(map, 0, offset, val, size);
>> +}
>> diff --git a/include/regmap.h b/include/regmap.h
>> index a6e2fafd27..bb7e947ce2 100644
>> --- a/include/regmap.h
>> +++ b/include/regmap.h
>> @@ -44,7 +44,9 @@ struct regmap {
>>   * Interface to provide access to registers either through a direct memory
>>   * bus or through a peripheral bus like I2C, SPI.
>>   */
>> +int regmap_write_ext(struct regmap *map, uint range_num, ulong offset, ulong val, enum regmap_size_t size);
>>  int regmap_write(struct regmap *map, ulong offset, ulong val, enum regmap_size_t size);
>> +int regmap_read_ext(struct regmap *map, uint range_num, ulong offset, ulong *valp, enum regmap_size_t size);
>>  int regmap_read(struct regmap *map, ulong offset, ulong *valp, enum regmap_size_t size);
>
> Can you please add full function comments?
>

OK, will be documented in v3.

>>
>>  #define regmap_write32(map, ptr, member, val) \
>> --
>> 2.16.1
>>
>
> Regards,
> Simon

Best regards,
Mario