diff mbox series

[v2,02/11] dt-bindings: hwmon: lm75: use common hwmon schema

Message ID 20240229-mbly-i2c-v2-2-b32ed18c098c@bootlin.com (mailing list archive)
State New, archived
Headers show
Series Add Mobileye EyeQ5 support to the Nomadik I2C controller & use hrtimers for timeouts | expand

Commit Message

Théo Lebrun Feb. 29, 2024, 6:10 p.m. UTC
Reference common hwmon schema which has the generic "label" property,
parsed by Linux hwmon subsystem.

To: Jean Delvare <jdelvare@suse.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-hwmon@vger.kernel.org
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 Documentation/devicetree/bindings/hwmon/lm75.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Rob Herring (Arm) Feb. 29, 2024, 7:26 p.m. UTC | #1
On Thu, 29 Feb 2024 19:10:50 +0100, Théo Lebrun wrote:
> Reference common hwmon schema which has the generic "label" property,
> parsed by Linux hwmon subsystem.
> 
> To: Jean Delvare <jdelvare@suse.com>
> To: Guenter Roeck <linux@roeck-us.net>
> Cc: linux-hwmon@vger.kernel.org
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  Documentation/devicetree/bindings/hwmon/lm75.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwmon/lm75.yaml:
Error in referenced schema matching $id: http://devicetree.org/schemas/hwmon/hwmon-common.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwmon/lm75.example.dtb: sensor@48: False schema does not allow {'compatible': ['st,stlm75'], 'reg': [[72]], 'vs-supply': [[4294967295]], '$nodename': ['sensor@48']}
	from schema $id: http://devicetree.org/schemas/hwmon/lm75.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwmon/lm75.example.dtb: temperature-sensor@48: False schema does not allow {'compatible': ['ams,as6200'], 'reg': [[72]], 'vs-supply': [[4294967295]], 'interrupts': [[17, 3]], '$nodename': ['temperature-sensor@48']}
	from schema $id: http://devicetree.org/schemas/hwmon/lm75.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240229-mbly-i2c-v2-2-b32ed18c098c@bootlin.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Krzysztof Kozlowski March 1, 2024, 6:37 a.m. UTC | #2
On 29/02/2024 19:10, Théo Lebrun wrote:
> Reference common hwmon schema which has the generic "label" property,
> parsed by Linux hwmon subsystem.
> 

Please do not mix independent patchsets. You create unneeded
dependencies blocking this patch. This patch depends on hwmon work, so
it cannot go through different tree.

If you insist to combine independent patches, then at least clearly
express merging strategy or dependency in patch changelog --- .


> To: Jean Delvare <jdelvare@suse.com>
> To: Guenter Roeck <linux@roeck-us.net>
> Cc: linux-hwmon@vger.kernel.org
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Guenter Roeck March 1, 2024, 6:53 a.m. UTC | #3
On 2/29/24 22:37, Krzysztof Kozlowski wrote:
> On 29/02/2024 19:10, Théo Lebrun wrote:
>> Reference common hwmon schema which has the generic "label" property,
>> parsed by Linux hwmon subsystem.
>>
> 
> Please do not mix independent patchsets. You create unneeded
> dependencies blocking this patch. This patch depends on hwmon work, so
> it cannot go through different tree.
> 
> If you insist to combine independent patches, then at least clearly
> express merging strategy or dependency in patch changelog --- .
> 

For my part I have to say that I don't know what to do with it.
Rob's robot reported errors, so I won't apply it, and I don't
feel comfortable giving it an ack either because of those errors.

Guenter

> 
>> To: Jean Delvare <jdelvare@suse.com>
>> To: Guenter Roeck <linux@roeck-us.net>
>> Cc: linux-hwmon@vger.kernel.org
>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>> ---
> 
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Best regards,
> Krzysztof
>
Théo Lebrun March 1, 2024, 9:41 a.m. UTC | #4
Hello,

On Fri Mar 1, 2024 at 7:53 AM CET, Guenter Roeck wrote:
> On 2/29/24 22:37, Krzysztof Kozlowski wrote:
> > On 29/02/2024 19:10, Théo Lebrun wrote:
> >> Reference common hwmon schema which has the generic "label" property,
> >> parsed by Linux hwmon subsystem.
> >>
> > 
> > Please do not mix independent patchsets. You create unneeded
> > dependencies blocking this patch. This patch depends on hwmon work, so
> > it cannot go through different tree.

I had to pick between this or dtbs_check failing on my DTS that uses a
label on temperature-sensor@48.

> > If you insist to combine independent patches, then at least clearly
> > express merging strategy or dependency in patch changelog --- .

I do not know how such indirect conflicts are usually resolved. Hwmon
can take it but MIPS might want to also take it to have valid DTS.

Any advice?

> For my part I have to say that I don't know what to do with it.
> Rob's robot reported errors, so I won't apply it, and I don't
> feel comfortable giving it an ack either because of those errors.

Can reproduce the error when patch "dt-bindings: hwmon: add common
properties" is not applied. Cannot reproduce when patch is applied.
Commit d590900b62f0 on hwmon-next. Cannot reproduce with hwmon-next as
parent.

Regards,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Krzysztof Kozlowski March 1, 2024, 10:13 a.m. UTC | #5
On 01/03/2024 10:41, Théo Lebrun wrote:
> Hello,
> 
> On Fri Mar 1, 2024 at 7:53 AM CET, Guenter Roeck wrote:
>> On 2/29/24 22:37, Krzysztof Kozlowski wrote:
>>> On 29/02/2024 19:10, Théo Lebrun wrote:
>>>> Reference common hwmon schema which has the generic "label" property,
>>>> parsed by Linux hwmon subsystem.
>>>>
>>>
>>> Please do not mix independent patchsets. You create unneeded
>>> dependencies blocking this patch. This patch depends on hwmon work, so
>>> it cannot go through different tree.
> 
> I had to pick between this or dtbs_check failing on my DTS that uses a
> label on temperature-sensor@48.

I don't see how is that relevant. You can organize your branches as you
wish, e.g. base one b4 branch on another and you will not have any warnings.

> 
>>> If you insist to combine independent patches, then at least clearly
>>> express merging strategy or dependency in patch changelog --- .
> 
> I do not know how such indirect conflicts are usually resolved. Hwmon
> can take it but MIPS might want to also take it to have valid DTS.
> 
> Any advice?

I don't see any conflict.


> 
>> For my part I have to say that I don't know what to do with it.
>> Rob's robot reported errors, so I won't apply it, and I don't
>> feel comfortable giving it an ack either because of those errors.
> 
> Can reproduce the error when patch "dt-bindings: hwmon: add common
> properties" is not applied. Cannot reproduce when patch is applied.
> Commit d590900b62f0 on hwmon-next. Cannot reproduce with hwmon-next as
> parent.

Yeah, but we see the error reported and it means something is missing.

Best regards,
Krzysztof
Théo Lebrun March 1, 2024, 10:44 a.m. UTC | #6
Hello,

On Fri Mar 1, 2024 at 11:13 AM CET, Krzysztof Kozlowski wrote:
> On 01/03/2024 10:41, Théo Lebrun wrote:
> > Hello,
> > 
> > On Fri Mar 1, 2024 at 7:53 AM CET, Guenter Roeck wrote:
> >> On 2/29/24 22:37, Krzysztof Kozlowski wrote:
> >>> On 29/02/2024 19:10, Théo Lebrun wrote:
> >>>> Reference common hwmon schema which has the generic "label" property,
> >>>> parsed by Linux hwmon subsystem.
> >>>>
> >>>
> >>> Please do not mix independent patchsets. You create unneeded
> >>> dependencies blocking this patch. This patch depends on hwmon work, so
> >>> it cannot go through different tree.
> > 
> > I had to pick between this or dtbs_check failing on my DTS that uses a
> > label on temperature-sensor@48.
>
> I don't see how is that relevant. You can organize your branches as you
> wish, e.g. base one b4 branch on another and you will not have any warnings.

That is what I do, I however do not want mips-next to have errors when
running dtbs_check. Having dtbs_check return errors is not an issue?

> >>> If you insist to combine independent patches, then at least clearly
> >>> express merging strategy or dependency in patch changelog --- .
> > 
> > I do not know how such indirect conflicts are usually resolved. Hwmon
> > can take it but MIPS might want to also take it to have valid DTS.
> > 
> > Any advice?
>
> I don't see any conflict.

I shouldn't have called that a conflict, more like a dependency.

> >> For my part I have to say that I don't know what to do with it.
> >> Rob's robot reported errors, so I won't apply it, and I don't
> >> feel comfortable giving it an ack either because of those errors.
> > 
> > Can reproduce the error when patch "dt-bindings: hwmon: add common
> > properties" is not applied. Cannot reproduce when patch is applied.
> > Commit d590900b62f0 on hwmon-next. Cannot reproduce with hwmon-next as
> > parent.
>
> Yeah, but we see the error reported and it means something is missing.

Yes, this series depends on "dt-bindings: hwmon: add common properties"
which the bot doesn't know it needs to apply.

Should I submit this patch independently and have my DTS be broken wrt
dtbs_check?

Have a nice day,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Krzysztof Kozlowski March 1, 2024, 11:35 a.m. UTC | #7
On 01/03/2024 11:44, Théo Lebrun wrote:
> Hello,
> 
> On Fri Mar 1, 2024 at 11:13 AM CET, Krzysztof Kozlowski wrote:
>> On 01/03/2024 10:41, Théo Lebrun wrote:
>>> Hello,
>>>
>>> On Fri Mar 1, 2024 at 7:53 AM CET, Guenter Roeck wrote:
>>>> On 2/29/24 22:37, Krzysztof Kozlowski wrote:
>>>>> On 29/02/2024 19:10, Théo Lebrun wrote:
>>>>>> Reference common hwmon schema which has the generic "label" property,
>>>>>> parsed by Linux hwmon subsystem.
>>>>>>
>>>>>
>>>>> Please do not mix independent patchsets. You create unneeded
>>>>> dependencies blocking this patch. This patch depends on hwmon work, so
>>>>> it cannot go through different tree.
>>>
>>> I had to pick between this or dtbs_check failing on my DTS that uses a
>>> label on temperature-sensor@48.
>>
>> I don't see how is that relevant. You can organize your branches as you
>> wish, e.g. base one b4 branch on another and you will not have any warnings.
> 
> That is what I do, I however do not want mips-next to have errors when
> running dtbs_check. Having dtbs_check return errors is not an issue?

You should ask your maintainer, but I don't understand how this is
achievable anyway. Subsystem bindings *should not* go via MIPS-next, so
how are you going to solve this?

And why MIPS shall be different than all other ARM/RISC-V SoCs?

Best regards,
Krzysztof
Théo Lebrun March 1, 2024, 2:09 p.m. UTC | #8
Hello,

On Fri Mar 1, 2024 at 12:35 PM CET, Krzysztof Kozlowski wrote:
> On 01/03/2024 11:44, Théo Lebrun wrote:
> > On Fri Mar 1, 2024 at 11:13 AM CET, Krzysztof Kozlowski wrote:
> >> On 01/03/2024 10:41, Théo Lebrun wrote:
> >>> On Fri Mar 1, 2024 at 7:53 AM CET, Guenter Roeck wrote:
> >>>> On 2/29/24 22:37, Krzysztof Kozlowski wrote:
> >>>>> On 29/02/2024 19:10, Théo Lebrun wrote:
> >>>>>> Reference common hwmon schema which has the generic "label" property,
> >>>>>> parsed by Linux hwmon subsystem.
> >>>>>
> >>>>> Please do not mix independent patchsets. You create unneeded
> >>>>> dependencies blocking this patch. This patch depends on hwmon work, so
> >>>>> it cannot go through different tree.
> >>>
> >>> I had to pick between this or dtbs_check failing on my DTS that uses a
> >>> label on temperature-sensor@48.
> >>
> >> I don't see how is that relevant. You can organize your branches as you
> >> wish, e.g. base one b4 branch on another and you will not have any warnings.
> > 
> > That is what I do, I however do not want mips-next to have errors when
> > running dtbs_check. Having dtbs_check return errors is not an issue?
>
> You should ask your maintainer, but I don't understand how this is
> achievable anyway. Subsystem bindings *should not* go via MIPS-next, so
> how are you going to solve this?

I thought it'd go in hwmon-next and be picked up by mips-next as well.
It's clear now that the right approach is to send the lm75.yaml patch
alone.

I'll wait some more before sending a new revision that drops this
lm75.yaml patch.

Have a nice day,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Krzysztof Kozlowski March 1, 2024, 2:13 p.m. UTC | #9
On 01/03/2024 15:09, Théo Lebrun wrote:
>>>> I don't see how is that relevant. You can organize your branches as you
>>>> wish, e.g. base one b4 branch on another and you will not have any warnings.
>>>
>>> That is what I do, I however do not want mips-next to have errors when
>>> running dtbs_check. Having dtbs_check return errors is not an issue?
>>
>> You should ask your maintainer, but I don't understand how this is
>> achievable anyway. Subsystem bindings *should not* go via MIPS-next, so
>> how are you going to solve this?
> 
> I thought it'd go in hwmon-next and be picked up by mips-next as well.
> It's clear now that the right approach is to send the lm75.yaml patch
> alone.

Then mips-next-dts branch would be based on subsystem branch with driver
changes? That violates the policy of not creating dependencies between
DTS and drivers.

What matters is final or even future release, not intermediate state.

Best regards,
Krzysztof
Rob Herring (Arm) March 1, 2024, 3:35 p.m. UTC | #10
On Fri, Mar 01, 2024 at 11:44:37AM +0100, Théo Lebrun wrote:
> Hello,
> 
> On Fri Mar 1, 2024 at 11:13 AM CET, Krzysztof Kozlowski wrote:
> > On 01/03/2024 10:41, Théo Lebrun wrote:
> > > Hello,
> > > 
> > > On Fri Mar 1, 2024 at 7:53 AM CET, Guenter Roeck wrote:
> > >> On 2/29/24 22:37, Krzysztof Kozlowski wrote:
> > >>> On 29/02/2024 19:10, Théo Lebrun wrote:
> > >>>> Reference common hwmon schema which has the generic "label" property,
> > >>>> parsed by Linux hwmon subsystem.
> > >>>>
> > >>>
> > >>> Please do not mix independent patchsets. You create unneeded
> > >>> dependencies blocking this patch. This patch depends on hwmon work, so
> > >>> it cannot go through different tree.
> > > 
> > > I had to pick between this or dtbs_check failing on my DTS that uses a
> > > label on temperature-sensor@48.
> >
> > I don't see how is that relevant. You can organize your branches as you
> > wish, e.g. base one b4 branch on another and you will not have any warnings.
> 
> That is what I do, I however do not want mips-next to have errors when
> running dtbs_check. Having dtbs_check return errors is not an issue?

That's a good goal, but difficult to achieve as you can see. Given 
dtbs_check in general has tons of warnings already, we currently don't 
worry about more warnings in specific branches. We just look at mainline 
and linux-next. And for that it's still so many, I'm just looking at 
general trends. It runs daily here[1].

As we get more platforms trying to stay at zero warnings, then we'll 
need to revisit this. I imagine that will mean all schemas have to go in 
1 branch with acks from subsystem maintainers. That's the opposite of 
what we do now. And then .dts branches will have to merge in the schema 
branch as needed. There are some checks (make dt_compatible_check) to 
for drivers vs. the schemas, so we'd have the same issue with 
intermittent warnings. But the drivers should be more decoupled from the 
schemas than the dts files.

Rob

[1] https://gitlab.com/robherring/linux-dt/-/jobs
Rob Herring (Arm) March 1, 2024, 3:38 p.m. UTC | #11
On Thu, Feb 29, 2024 at 10:53:07PM -0800, Guenter Roeck wrote:
> On 2/29/24 22:37, Krzysztof Kozlowski wrote:
> > On 29/02/2024 19:10, Théo Lebrun wrote:
> > > Reference common hwmon schema which has the generic "label" property,
> > > parsed by Linux hwmon subsystem.
> > > 
> > 
> > Please do not mix independent patchsets. You create unneeded
> > dependencies blocking this patch. This patch depends on hwmon work, so
> > it cannot go through different tree.
> > 
> > If you insist to combine independent patches, then at least clearly
> > express merging strategy or dependency in patch changelog --- .
> > 
> 
> For my part I have to say that I don't know what to do with it.
> Rob's robot reported errors, so I won't apply it, and I don't
> feel comfortable giving it an ack either because of those errors.

You can apply it. Those are just related to not finding 
hwmon-common.yaml which you have, and Théo confirmed it works on 
hwmon-next.

Rob
Théo Lebrun March 1, 2024, 3:52 p.m. UTC | #12
Hello,

On Fri Mar 1, 2024 at 4:35 PM CET, Rob Herring wrote:
> On Fri, Mar 01, 2024 at 11:44:37AM +0100, Théo Lebrun wrote:
> > Hello,
> > 
> > On Fri Mar 1, 2024 at 11:13 AM CET, Krzysztof Kozlowski wrote:
> > > On 01/03/2024 10:41, Théo Lebrun wrote:
> > > > Hello,
> > > > 
> > > > On Fri Mar 1, 2024 at 7:53 AM CET, Guenter Roeck wrote:
> > > >> On 2/29/24 22:37, Krzysztof Kozlowski wrote:
> > > >>> On 29/02/2024 19:10, Théo Lebrun wrote:
> > > >>>> Reference common hwmon schema which has the generic "label" property,
> > > >>>> parsed by Linux hwmon subsystem.
> > > >>>>
> > > >>>
> > > >>> Please do not mix independent patchsets. You create unneeded
> > > >>> dependencies blocking this patch. This patch depends on hwmon work, so
> > > >>> it cannot go through different tree.
> > > > 
> > > > I had to pick between this or dtbs_check failing on my DTS that uses a
> > > > label on temperature-sensor@48.
> > >
> > > I don't see how is that relevant. You can organize your branches as you
> > > wish, e.g. base one b4 branch on another and you will not have any warnings.
> > 
> > That is what I do, I however do not want mips-next to have errors when
> > running dtbs_check. Having dtbs_check return errors is not an issue?
>
> That's a good goal, but difficult to achieve as you can see. Given 
> dtbs_check in general has tons of warnings already, we currently don't 
> worry about more warnings in specific branches. We just look at mainline 
> and linux-next. And for that it's still so many, I'm just looking at 
> general trends. It runs daily here[1].

Here's my opportunity to ask a question I've had for a while: do you
have a way to filter out dtbs that are known to be bad actors (ie have
many many warnings)? Maybe a list of platforms you talk about below
that aim at zero warnings?

Another way to ask this: what would be a good default DT_SCHEMA_FILES
value? Not filtering leads to way too many errors.

Regards,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Guenter Roeck March 1, 2024, 7:21 p.m. UTC | #13
On Thu, Feb 29, 2024 at 07:10:50PM +0100, Théo Lebrun wrote:
> Reference common hwmon schema which has the generic "label" property,
> parsed by Linux hwmon subsystem.
> 
> To: Jean Delvare <jdelvare@suse.com>
> To: Guenter Roeck <linux@roeck-us.net>
> Cc: linux-hwmon@vger.kernel.org
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Applied to hwmon-next.

Thanks,
Guenter
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
index ed269e428a3d..29bd7460cc26 100644
--- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
+++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
@@ -57,6 +57,7 @@  required:
   - reg
 
 allOf:
+  - $ref: hwmon-common.yaml#
   - if:
       not:
         properties:
@@ -71,7 +72,7 @@  allOf:
       properties:
         interrupts: false
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |