diff mbox series

[6/6] dt-bindings: hwmon: jedec,jc42: add nxp,se97b

Message ID 20210920182114.339419-6-krzysztof.kozlowski@canonical.com (mailing list archive)
State Accepted
Headers show
Series [1/6] dt-bindings: hwmon: lm70: move to trivial devices | expand

Commit Message

Krzysztof Kozlowski Sept. 20, 2021, 6:21 p.m. UTC
Document bindings for NXP SE97B, a DDR memory module temperature sensor
with integrated SPD and EEPROM via Atmel's AT24 interface.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Rob Herring Sept. 23, 2021, 9:16 p.m. UTC | #1
On Mon, Sep 20, 2021 at 08:21:14PM +0200, Krzysztof Kozlowski wrote:
> Document bindings for NXP SE97B, a DDR memory module temperature sensor
> with integrated SPD and EEPROM via Atmel's AT24 interface.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
>  Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> index a7bb4e3a1c46..0e49b3901161 100644
> --- a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> @@ -10,6 +10,14 @@ maintainers:
>    - Jean Delvare <jdelvare@suse.com>
>    - Guenter Roeck <linux@roeck-us.net>
>  
> +select:
> +  properties:
> +    compatible:
> +      const: jedec,jc-42.4-temp
> +
> +  required:
> +    - compatible
> +

Is this supposed to be in the last patch? And why is it needed?

>  properties:
>    compatible:
>      oneOf:
> @@ -31,6 +39,7 @@ properties:
>                - microchip,mcp98244
>                - microchip,mcp9843
>                - nxp,se97
> +              - nxp,se97b
>                - nxp,se98
>                - onnn,cat6095
>                - onnn,cat34ts02
> -- 
> 2.30.2
> 
>
Krzysztof Kozlowski Sept. 24, 2021, 6:57 a.m. UTC | #2
On 23/09/2021 23:16, Rob Herring wrote:
> On Mon, Sep 20, 2021 at 08:21:14PM +0200, Krzysztof Kozlowski wrote:
>> Document bindings for NXP SE97B, a DDR memory module temperature sensor
>> with integrated SPD and EEPROM via Atmel's AT24 interface.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>> ---
>>  Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
>> index a7bb4e3a1c46..0e49b3901161 100644
>> --- a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
>> @@ -10,6 +10,14 @@ maintainers:
>>    - Jean Delvare <jdelvare@suse.com>
>>    - Guenter Roeck <linux@roeck-us.net>
>>  
>> +select:
>> +  properties:
>> +    compatible:
>> +      const: jedec,jc-42.4-temp
>> +
>> +  required:
>> +    - compatible
>> +
> 
> Is this supposed to be in the last patch? And why is it needed?

Yes, this is here on purpose because of nxp,se97b which is sensor with
at24-compatible EEPROM.

arch/arm/boot/dts/at91-nattis-2-natte-2.dts:
169         temp@18 {
170                 compatible = "nxp,se97b", "jedec,jc-42.4-temp";

171                 reg = <0x18>;
172                 smbus-timeout-disable;
173         };
174
175         eeprom@50 {
176                 compatible = "nxp,se97b", "atmel,24c02";
177                 reg = <0x50>;
178                 pagesize = <16>;

Without the select, dtbs_check was complaining about the second node:

eeprom@50: compatible: 'oneOf' conditional failed, one must be fixed:
	['nxp,se97b', 'atmel,24c02'] is too long
	Additional items are not allowed ('atmel,24c02' was unexpected)
	'jedec,jc-42.4-temp' was expected
	From schema:
/home/dev/linux/linux/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml

eeprom@50: 'pagesize' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema:
/home/dev/linux/linux/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml


>>  properties:
>>    compatible:
>>      oneOf:
>> @@ -31,6 +39,7 @@ properties:
>>                - microchip,mcp98244
>>                - microchip,mcp9843
>>                - nxp,se97
>> +              - nxp,se97b
>>                - nxp,se98
>>                - onnn,cat6095
>>                - onnn,cat34ts02
>> -- 
>> 2.30.2
>>
>>


Best regards,
Krzysztof
Guenter Roeck Sept. 24, 2021, 11:51 a.m. UTC | #3
On Fri, Sep 24, 2021 at 08:57:44AM +0200, Krzysztof Kozlowski wrote:
> On 23/09/2021 23:16, Rob Herring wrote:
> > On Mon, Sep 20, 2021 at 08:21:14PM +0200, Krzysztof Kozlowski wrote:
> >> Document bindings for NXP SE97B, a DDR memory module temperature sensor
> >> with integrated SPD and EEPROM via Atmel's AT24 interface.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> >> ---
> >>  Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> >> index a7bb4e3a1c46..0e49b3901161 100644
> >> --- a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> >> +++ b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> >> @@ -10,6 +10,14 @@ maintainers:
> >>    - Jean Delvare <jdelvare@suse.com>
> >>    - Guenter Roeck <linux@roeck-us.net>
> >>  
> >> +select:
> >> +  properties:
> >> +    compatible:
> >> +      const: jedec,jc-42.4-temp
> >> +
> >> +  required:
> >> +    - compatible
> >> +
> > 
> > Is this supposed to be in the last patch? And why is it needed?
> 
> Yes, this is here on purpose because of nxp,se97b which is sensor with
> at24-compatible EEPROM.
> 
> arch/arm/boot/dts/at91-nattis-2-natte-2.dts:
> 169         temp@18 {
> 170                 compatible = "nxp,se97b", "jedec,jc-42.4-temp";
> 
> 171                 reg = <0x18>;
> 172                 smbus-timeout-disable;
> 173         };
> 174
> 175         eeprom@50 {
> 176                 compatible = "nxp,se97b", "atmel,24c02";

How would that be handled anyway ? Yes, the chip includes both a temperature
sensor and an eeprom, but this node should most definitely not instantiate as
temperature sensor.

Guenter

> 177                 reg = <0x50>;
> 178                 pagesize = <16>;
> 
> Without the select, dtbs_check was complaining about the second node:
> 
> eeprom@50: compatible: 'oneOf' conditional failed, one must be fixed:
> 	['nxp,se97b', 'atmel,24c02'] is too long
> 	Additional items are not allowed ('atmel,24c02' was unexpected)
> 	'jedec,jc-42.4-temp' was expected
> 	From schema:
> /home/dev/linux/linux/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> 
> eeprom@50: 'pagesize' does not match any of the regexes: 'pinctrl-[0-9]+'
> 	From schema:
> /home/dev/linux/linux/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> 
> 
> >>  properties:
> >>    compatible:
> >>      oneOf:
> >> @@ -31,6 +39,7 @@ properties:
> >>                - microchip,mcp98244
> >>                - microchip,mcp9843
> >>                - nxp,se97
> >> +              - nxp,se97b
> >>                - nxp,se98
> >>                - onnn,cat6095
> >>                - onnn,cat34ts02
> >> -- 
> >> 2.30.2
> >>
> >>
> 
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Oct. 8, 2021, 8 a.m. UTC | #4
On 24/09/2021 13:51, Guenter Roeck wrote:
> On Fri, Sep 24, 2021 at 08:57:44AM +0200, Krzysztof Kozlowski wrote:
>> On 23/09/2021 23:16, Rob Herring wrote:
>>> On Mon, Sep 20, 2021 at 08:21:14PM +0200, Krzysztof Kozlowski wrote:
>>>> Document bindings for NXP SE97B, a DDR memory module temperature sensor
>>>> with integrated SPD and EEPROM via Atmel's AT24 interface.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml | 9 +++++++++
>>>>  1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
>>>> index a7bb4e3a1c46..0e49b3901161 100644
>>>> --- a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
>>>> +++ b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
>>>> @@ -10,6 +10,14 @@ maintainers:
>>>>    - Jean Delvare <jdelvare@suse.com>
>>>>    - Guenter Roeck <linux@roeck-us.net>
>>>>  
>>>> +select:
>>>> +  properties:
>>>> +    compatible:
>>>> +      const: jedec,jc-42.4-temp
>>>> +
>>>> +  required:
>>>> +    - compatible
>>>> +
>>>
>>> Is this supposed to be in the last patch? And why is it needed?
>>
>> Yes, this is here on purpose because of nxp,se97b which is sensor with
>> at24-compatible EEPROM.
>>
>> arch/arm/boot/dts/at91-nattis-2-natte-2.dts:
>> 169         temp@18 {
>> 170                 compatible = "nxp,se97b", "jedec,jc-42.4-temp";
>>
>> 171                 reg = <0x18>;
>> 172                 smbus-timeout-disable;
>> 173         };
>> 174
>> 175         eeprom@50 {
>> 176                 compatible = "nxp,se97b", "atmel,24c02";
> 
> How would that be handled anyway ? Yes, the chip includes both a temperature
> sensor and an eeprom, but this node should most definitely not instantiate as
> temperature sensor.
> 

I am not sure if I understand the problem you are mentioning. You have
two nods in DT, two different compatible sets and two difference
devices. One eeprom and other one a temperature sensor.


Best regards,
Krzysztof
Guenter Roeck Oct. 8, 2021, 3:15 p.m. UTC | #5
On 10/8/21 1:00 AM, Krzysztof Kozlowski wrote:
> On 24/09/2021 13:51, Guenter Roeck wrote:
>> On Fri, Sep 24, 2021 at 08:57:44AM +0200, Krzysztof Kozlowski wrote:
>>> On 23/09/2021 23:16, Rob Herring wrote:
>>>> On Mon, Sep 20, 2021 at 08:21:14PM +0200, Krzysztof Kozlowski wrote:
>>>>> Document bindings for NXP SE97B, a DDR memory module temperature sensor
>>>>> with integrated SPD and EEPROM via Atmel's AT24 interface.
>>>>>
>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>>>>> ---
>>>>>   Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml | 9 +++++++++
>>>>>   1 file changed, 9 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
>>>>> index a7bb4e3a1c46..0e49b3901161 100644
>>>>> --- a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
>>>>> +++ b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
>>>>> @@ -10,6 +10,14 @@ maintainers:
>>>>>     - Jean Delvare <jdelvare@suse.com>
>>>>>     - Guenter Roeck <linux@roeck-us.net>
>>>>>   
>>>>> +select:
>>>>> +  properties:
>>>>> +    compatible:
>>>>> +      const: jedec,jc-42.4-temp
>>>>> +
>>>>> +  required:
>>>>> +    - compatible
>>>>> +
>>>>
>>>> Is this supposed to be in the last patch? And why is it needed?
>>>
>>> Yes, this is here on purpose because of nxp,se97b which is sensor with
>>> at24-compatible EEPROM.
>>>
>>> arch/arm/boot/dts/at91-nattis-2-natte-2.dts:
>>> 169         temp@18 {
>>> 170                 compatible = "nxp,se97b", "jedec,jc-42.4-temp";
>>>
>>> 171                 reg = <0x18>;
>>> 172                 smbus-timeout-disable;
>>> 173         };
>>> 174
>>> 175         eeprom@50 {
>>> 176                 compatible = "nxp,se97b", "atmel,24c02";
>>
>> How would that be handled anyway ? Yes, the chip includes both a temperature
>> sensor and an eeprom, but this node should most definitely not instantiate as
>> temperature sensor.
>>
> 
> I am not sure if I understand the problem you are mentioning. You have
> two nods in DT, two different compatible sets and two difference
> devices. One eeprom and other one a temperature sensor.
> 

I didn't realize that the driver is supposed to bind to "jedec,jc-42.4-temp"
and that "nxp,se97b" is really informational.

Sorry for the confusion.

Guenter
Rob Herring Oct. 8, 2021, 6:57 p.m. UTC | #6
On Fri, Sep 24, 2021 at 1:57 AM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 23/09/2021 23:16, Rob Herring wrote:
> > On Mon, Sep 20, 2021 at 08:21:14PM +0200, Krzysztof Kozlowski wrote:
> >> Document bindings for NXP SE97B, a DDR memory module temperature sensor
> >> with integrated SPD and EEPROM via Atmel's AT24 interface.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> >> ---
> >>  Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> >> index a7bb4e3a1c46..0e49b3901161 100644
> >> --- a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> >> +++ b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> >> @@ -10,6 +10,14 @@ maintainers:
> >>    - Jean Delvare <jdelvare@suse.com>
> >>    - Guenter Roeck <linux@roeck-us.net>
> >>
> >> +select:
> >> +  properties:
> >> +    compatible:
> >> +      const: jedec,jc-42.4-temp
> >> +
> >> +  required:
> >> +    - compatible
> >> +
> >
> > Is this supposed to be in the last patch? And why is it needed?
>
> Yes, this is here on purpose because of nxp,se97b which is sensor with
> at24-compatible EEPROM.
>
> arch/arm/boot/dts/at91-nattis-2-natte-2.dts:
> 169         temp@18 {
> 170                 compatible = "nxp,se97b", "jedec,jc-42.4-temp";
>
> 171                 reg = <0x18>;
> 172                 smbus-timeout-disable;
> 173         };
> 174
> 175         eeprom@50 {
> 176                 compatible = "nxp,se97b", "atmel,24c02";
> 177                 reg = <0x50>;
> 178                 pagesize = <16>;
>
> Without the select, dtbs_check was complaining about the second node:
>
> eeprom@50: compatible: 'oneOf' conditional failed, one must be fixed:
>         ['nxp,se97b', 'atmel,24c02'] is too long
>         Additional items are not allowed ('atmel,24c02' was unexpected)
>         'jedec,jc-42.4-temp' was expected
>         From schema:
> /home/dev/linux/linux/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
>
> eeprom@50: 'pagesize' does not match any of the regexes: 'pinctrl-[0-9]+'
>         From schema:
> /home/dev/linux/linux/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml

If a dt only lists one of the vendor specific compatibles, it is going
to pass as this schema won't be applied with this change. We won't get
an undocumented compatible either because it is documented. I don't
have a better suggestion other than listing everything but
'nxp,se97b'. I don't think it is really worth do that, so:

Reviewed-by: Rob Herring <robh@kernel.org>
Guenter Roeck Oct. 8, 2021, 9:06 p.m. UTC | #7
On Mon, Sep 20, 2021 at 08:21:14PM +0200, Krzysztof Kozlowski wrote:
> Document bindings for NXP SE97B, a DDR memory module temperature sensor
> with integrated SPD and EEPROM via Atmel's AT24 interface.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Applied.

Thanks,
Guenter

> ---
>  Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> index a7bb4e3a1c46..0e49b3901161 100644
> --- a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
> @@ -10,6 +10,14 @@ maintainers:
>    - Jean Delvare <jdelvare@suse.com>
>    - Guenter Roeck <linux@roeck-us.net>
>  
> +select:
> +  properties:
> +    compatible:
> +      const: jedec,jc-42.4-temp
> +
> +  required:
> +    - compatible
> +
>  properties:
>    compatible:
>      oneOf:
> @@ -31,6 +39,7 @@ properties:
>                - microchip,mcp98244
>                - microchip,mcp9843
>                - nxp,se97
> +              - nxp,se97b
>                - nxp,se98
>                - onnn,cat6095
>                - onnn,cat34ts02
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
index a7bb4e3a1c46..0e49b3901161 100644
--- a/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
+++ b/Documentation/devicetree/bindings/hwmon/jedec,jc42.yaml
@@ -10,6 +10,14 @@  maintainers:
   - Jean Delvare <jdelvare@suse.com>
   - Guenter Roeck <linux@roeck-us.net>
 
+select:
+  properties:
+    compatible:
+      const: jedec,jc-42.4-temp
+
+  required:
+    - compatible
+
 properties:
   compatible:
     oneOf:
@@ -31,6 +39,7 @@  properties:
               - microchip,mcp98244
               - microchip,mcp9843
               - nxp,se97
+              - nxp,se97b
               - nxp,se98
               - onnn,cat6095
               - onnn,cat34ts02