Re: [PATCH v2 0/2] arm64: dts: r8a779[56]: update register size for thermal

[Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>]

Hi Simon,

On Mon, Jan 8, 2018 at 7:59 AM, Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org> wrote:
> On Fri, Jan 05, 2018 at 04:54:45PM +0100, Niklas Söderlund wrote:
>> This series updates the register size for the thermal hardware. In later
>> versions of the datasheet one additional register is documented. This
>> register is needed to be able to determine if calibration data should be
>> read from register or if static values from the driver should be used.
>>
>> There is already a posted RFC patch which make use of this additional
>> register (THSCP) if the register size described in DT covers it.
>> Currently no hardware where the calibration value is fused have been
>> found so that patch is not tested and therefore kept back until such a
>> system is found. In the mean time there is no harm in getting the
>> correct register size described in DT now that it's documented in the
>> datasheet (Rev 0.80).
>>
>> * Changes since v1
>> - Increase size from just covering the new register in TSC1 to
>>   increasing the size of all TSC's to 0x100 which is the smallest
>>   granularity of the address decoder circuitry. Suggested by Geert.
>>
>> Niklas Söderlund (2):
>>   arm64: dts: r8a7795: update register size for thermal
>>   arm64: dts: r8a7796: update register size for thermal
>
> Applied, but this seems to go in the opposite direction of:
>
> 846106d9d973 ("ARM: dts: r8a7793: Reduce size of thermal registers")
> 1af62038f499 ("ARM: dts: r8a7791: Reduce size of thermal registers")
> d0191d85a237 ("ARM: dts: r8a7790: Reduce size of thermal registers")
>
> Should those patches be dropped and the register areas grown to 0x100
> instead?

There are two "policies" to specify register block lengths:
  1. Cover all registers documented in the datasheet, and nothing more (at the
   low/high end),
  2. Use a power-of-two that covers all registers, which is most probably
    what matches the actual address decoder in the hardware.

Of course, we all know datasheets are volatile, and registers may appear or
disappear any time ;-)

In reality, ioremap() granularity is PAGE_SIZE, which is also a power-of-two.

So personally, I'm more leaning towards policy #2.

The 3 commits you refer to use policy #1, but covered a register not
documented in the datasheet.

Note that some drivers (SD?) use the register block lengths to check if a
feature is available or not. Ideally they should use the compatible value
instead.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html