Re: [PATCH V4 3/4] ARM: bcm2835: add thermal node to device-tree of bcm283x

[Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>]

> On 09.09.2016, at 17:36, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
> 
> 
>> Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> hat am 9. September 2016 um 16:58
>> geschrieben:
>> 
>> 
>> 
>>>> On 09.09.2016, at 16:25, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
>>>> 
>>>> Am 09.09.2016 um 09:49 schrieb kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org:
>>>> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>>>> 
>>>> Add the node for the thermal sensor of the bcm2835-soc
>>>> to the device tree.
>>>> 
>>>> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>>>> Reviewed-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
>>>> ---
>>>> arch/arm/boot/dts/bcm283x.dtsi | 6 ++++++
>>>> 1 file changed, 6 insertions(+)
>>>> 
>>>> diff --git a/arch/arm/boot/dts/bcm283x.dtsi
>>>> b/arch/arm/boot/dts/bcm283x.dtsi
>>>> index b982522..e2e3a46 100644
>>>> --- a/arch/arm/boot/dts/bcm283x.dtsi
>>>> +++ b/arch/arm/boot/dts/bcm283x.dtsi
>>>> @@ -186,6 +186,12 @@
>>>>           interrupts = <2 14>; /* pwa1 */
>>>>       };
>>>> 
>>>> +        thermal: thermal@0x7e212000 {
>>>> +            compatible = "brcm,bcm2835-thermal";
>>>> +            reg = <0x7e212000 0x8>;
>>>> +            clocks = <&clocks BCM2835_CLOCK_TSENS>;
>>>> +        };
>>>> +
>>> 
>>> Since the driver handles 3 different SoC (2835, 2836, 2837). This node
>>> should be defined in the SoC specific dtsi files, because the BCM2836
>>> includes bcm283x.dtsi too.
>>> 
>>> Be aware the patch for bcm2837 must go to ARM64.
>> 
>> I can not really follow:
>> * the node is defined in the dtsi included by all 3 soc,
>>   and it is available on all so it sits where for example
>>   spi0 or uart0 is located
>> * as for arm64: this describes the registers that are
>>   identical for arm and arm64 and the bcm2837.dtsi
>>   is also including ../../../../arm/boot/dts/bcm283x.dtsi
>> 
>> So what is the problem?
> 
> The thermal driver specifies 3 different compatibles with partially different
> settings.
> If this patch is applied, all SoC (bcm2835, bcm2836, bcm2837) would use the
> bcm2835 settings.
> I can't believe this is intended.
> 
> Why does the binding contains 3 different compatibles if only one is used?

Now I understand - did not think of that: good catch!

To minimize the patch: what about setting it to 2837 by default (in 283x.dtsi)
and only change compatible in 2835.dtsi and 2836.dtsi - that is 3 files
changed instead of 4 or 5...

Or any other ideas how to detect which Soc we run on during runtime?
I.e: query the root compatible string in the device-tree?

Just before I create a new patch set to fix that.

Martin
--
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