| Message ID | b4b626c85ef3f75a0de936c818b2fff389e92c6d.1727855465.git.michal.simek@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | dt-bindings: clock: si5351: Make compatible string required property | expand |
On 02/10/2024 09:51, Michal Simek wrote: > Compatible property is likely also required property. > > Signed-off-by: Michal Simek <michal.simek@amd.com> > --- That's a convention but not necessary, a no-op. Best regards, Krzysztof
On 10/2/24 10:19, Krzysztof Kozlowski wrote: > On 02/10/2024 09:51, Michal Simek wrote: >> Compatible property is likely also required property. >> >> Signed-off-by: Michal Simek <michal.simek@amd.com> >> --- > > That's a convention but not necessary, a no-op. But how do you identify device then? Or are you saying that device description is valid even if there is no compatible string? M
On 10/2/24 10:24, Michal Simek wrote: > > > On 10/2/24 10:19, Krzysztof Kozlowski wrote: >> On 02/10/2024 09:51, Michal Simek wrote: >>> Compatible property is likely also required property. >>> >>> Signed-off-by: Michal Simek <michal.simek@amd.com> >>> --- >> >> That's a convention but not necessary, a no-op. > > But how do you identify device then? > Or are you saying that device description is valid even if there is no > compatible string? One more thing commit 524dfbc4e9fc ("dt-bindings: clock: si5351: convert to yaml") is showing that compatible property was required in txt file. -Required properties: -- compatible: shall be one of the following: - "silabs,si5351a" - Si5351a, QFN20 package - "silabs,si5351a-msop" - Si5351a, MSOP10 package I can update commit message to describe it too. M
On 02/10/2024 10:24, Michal Simek wrote: > > > On 10/2/24 10:19, Krzysztof Kozlowski wrote: >> On 02/10/2024 09:51, Michal Simek wrote: >>> Compatible property is likely also required property. >>> >>> Signed-off-by: Michal Simek <michal.simek@amd.com> >>> --- >> >> That's a convention but not necessary, a no-op. > > But how do you identify device then? > Or are you saying that device description is valid even if there is no > compatible string? No. I meant your change means nothing to the schema. It's a noop. Compatible is implied. Best regards, Krzysztof
On 02/10/2024 12:31, Michal Simek wrote: > > > On 10/2/24 10:24, Michal Simek wrote: >> >> >> On 10/2/24 10:19, Krzysztof Kozlowski wrote: >>> On 02/10/2024 09:51, Michal Simek wrote: >>>> Compatible property is likely also required property. >>>> >>>> Signed-off-by: Michal Simek <michal.simek@amd.com> >>>> --- >>> >>> That's a convention but not necessary, a no-op. >> >> But how do you identify device then? >> Or are you saying that device description is valid even if there is no >> compatible string? > > One more thing > commit 524dfbc4e9fc ("dt-bindings: clock: si5351: convert to yaml") is showing > that compatible property was required in txt file. > > -Required properties: > -- compatible: shall be one of the following: > - "silabs,si5351a" - Si5351a, QFN20 package > - "silabs,si5351a-msop" - Si5351a, MSOP10 package > > I can update commit message to describe it too. Devices do not work without compatible, so this is obvious... and like said - it is already required, so the change is redundant. Does not harm, though. Best regards, Krzysztof
On Wed, Oct 02, 2024 at 02:17:22PM +0200, Krzysztof Kozlowski wrote: > On 02/10/2024 12:31, Michal Simek wrote: > > > > > > On 10/2/24 10:24, Michal Simek wrote: > >> > >> > >> On 10/2/24 10:19, Krzysztof Kozlowski wrote: > >>> On 02/10/2024 09:51, Michal Simek wrote: > >>>> Compatible property is likely also required property. > >>>> > >>>> Signed-off-by: Michal Simek <michal.simek@amd.com> > >>>> --- > >>> > >>> That's a convention but not necessary, a no-op. > >> > >> But how do you identify device then? > >> Or are you saying that device description is valid even if there is no > >> compatible string? > > > > One more thing > > commit 524dfbc4e9fc ("dt-bindings: clock: si5351: convert to yaml") is showing > > that compatible property was required in txt file. > > > > -Required properties: > > -- compatible: shall be one of the following: > > - "silabs,si5351a" - Si5351a, QFN20 package > > - "silabs,si5351a-msop" - Si5351a, MSOP10 package > > > > I can update commit message to describe it too. > > Devices do not work without compatible, so this is obvious... and like > said - it is already required, so the change is redundant. Does not > harm, though. To put it another way, by the time the schema is applied, we already know that compatible is present because that is *how* the schema gets applied in the first place. Rob
On 10/2/24 23:41, Rob Herring wrote: > On Wed, Oct 02, 2024 at 02:17:22PM +0200, Krzysztof Kozlowski wrote: >> On 02/10/2024 12:31, Michal Simek wrote: >>> >>> >>> On 10/2/24 10:24, Michal Simek wrote: >>>> >>>> >>>> On 10/2/24 10:19, Krzysztof Kozlowski wrote: >>>>> On 02/10/2024 09:51, Michal Simek wrote: >>>>>> Compatible property is likely also required property. >>>>>> >>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com> >>>>>> --- >>>>> >>>>> That's a convention but not necessary, a no-op. >>>> >>>> But how do you identify device then? >>>> Or are you saying that device description is valid even if there is no >>>> compatible string? >>> >>> One more thing >>> commit 524dfbc4e9fc ("dt-bindings: clock: si5351: convert to yaml") is showing >>> that compatible property was required in txt file. >>> >>> -Required properties: >>> -- compatible: shall be one of the following: >>> - "silabs,si5351a" - Si5351a, QFN20 package >>> - "silabs,si5351a-msop" - Si5351a, MSOP10 package >>> >>> I can update commit message to describe it too. >> >> Devices do not work without compatible, so this is obvious... and like >> said - it is already required, so the change is redundant. Does not >> harm, though. > > To put it another way, by the time the schema is applied, we already > know that compatible is present because that is *how* the schema gets > applied in the first place. I get that argument but then based on this we should remove all records about compatible string as required property. Thanks, Michal
On 03/10/2024 07:57, Michal Simek wrote: > > > On 10/2/24 23:41, Rob Herring wrote: >> On Wed, Oct 02, 2024 at 02:17:22PM +0200, Krzysztof Kozlowski wrote: >>> On 02/10/2024 12:31, Michal Simek wrote: >>>> >>>> >>>> On 10/2/24 10:24, Michal Simek wrote: >>>>> >>>>> >>>>> On 10/2/24 10:19, Krzysztof Kozlowski wrote: >>>>>> On 02/10/2024 09:51, Michal Simek wrote: >>>>>>> Compatible property is likely also required property. >>>>>>> >>>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com> >>>>>>> --- >>>>>> >>>>>> That's a convention but not necessary, a no-op. >>>>> >>>>> But how do you identify device then? >>>>> Or are you saying that device description is valid even if there is no >>>>> compatible string? >>>> >>>> One more thing >>>> commit 524dfbc4e9fc ("dt-bindings: clock: si5351: convert to yaml") is showing >>>> that compatible property was required in txt file. >>>> >>>> -Required properties: >>>> -- compatible: shall be one of the following: >>>> - "silabs,si5351a" - Si5351a, QFN20 package >>>> - "silabs,si5351a-msop" - Si5351a, MSOP10 package >>>> >>>> I can update commit message to describe it too. >>> >>> Devices do not work without compatible, so this is obvious... and like >>> said - it is already required, so the change is redundant. Does not >>> harm, though. >> >> To put it another way, by the time the schema is applied, we already >> know that compatible is present because that is *how* the schema gets >> applied in the first place. > > I get that argument but then based on this we should remove all records about > compatible string as required property. We could... but we have a style of keeping it. What is the harm in having it in 99% of bindings and missing in a few? Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.yaml b/Documentation/devicetree/bindings/clock/silabs,si5351.yaml index d3e0ec29993b..3442400715d0 100644 --- a/Documentation/devicetree/bindings/clock/silabs,si5351.yaml +++ b/Documentation/devicetree/bindings/clock/silabs,si5351.yaml @@ -187,6 +187,7 @@ allOf: maxItems: 1 required: + - compatible - reg - "#address-cells" - "#size-cells"
Compatible property is likely also required property. Signed-off-by: Michal Simek <michal.simek@amd.com> --- Documentation/devicetree/bindings/clock/silabs,si5351.yaml | 1 + 1 file changed, 1 insertion(+)