* [PATCH] dt-bindings: pinctrl: qcom,pmic-mpp: Fix schema for "qcom,paired"
@ 2023-04-18 15:06 Rob Herring
2023-04-19 19:56 ` Krzysztof Kozlowski
2023-06-13 6:50 ` Krzysztof Kozlowski
0 siblings, 2 replies; 8+ messages in thread
From: Rob Herring @ 2023-04-18 15:06 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
Krzysztof Kozlowski
Cc: linux-arm-msm, linux-gpio, devicetree, linux-kernel
The "qcom,paired" schema is all wrong. First, it's a list rather than an
object(dictionary). Second, it is missing a required type. The meta-schema
normally catches this, but schemas under "$defs" was not getting checked.
A fix for that is pending.
Signed-off-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
index 9412b9362328..4c3e9ff82105 100644
--- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
@@ -144,8 +144,9 @@ $defs:
enum: [0, 1, 2, 3, 4, 5, 6, 7]
qcom,paired:
- - description:
- Indicates that the pin should be operating in paired mode.
+ type: boolean
+ description:
+ Indicates that the pin should be operating in paired mode.
required:
- pins
--
2.39.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] dt-bindings: pinctrl: qcom,pmic-mpp: Fix schema for "qcom,paired"
2023-04-18 15:06 [PATCH] dt-bindings: pinctrl: qcom,pmic-mpp: Fix schema for "qcom,paired" Rob Herring
@ 2023-04-19 19:56 ` Krzysztof Kozlowski
2023-04-21 19:48 ` Rob Herring
2023-06-13 6:50 ` Krzysztof Kozlowski
1 sibling, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2023-04-19 19:56 UTC (permalink / raw)
To: Rob Herring, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Linus Walleij, Krzysztof Kozlowski
Cc: linux-arm-msm, linux-gpio, devicetree, linux-kernel
On 18/04/2023 17:06, Rob Herring wrote:
> The "qcom,paired" schema is all wrong. First, it's a list rather than an
> object(dictionary). Second, it is missing a required type. The meta-schema
> normally catches this, but schemas under "$defs" was not getting checked.
> A fix for that is pending.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
> index 9412b9362328..4c3e9ff82105 100644
> --- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
> @@ -144,8 +144,9 @@ $defs:
> enum: [0, 1, 2, 3, 4, 5, 6, 7]
>
> qcom,paired:
> - - description:
> - Indicates that the pin should be operating in paired mode.
> + type: boolean
> + description:
> + Indicates that the pin should be operating in paired mode.
Current Linux implementation uses it as a generic pinconf param
pinconf_generic_params which is parsed by:
pinconf_generic_parse_dt_config() -> parse_dt_cfg() ->
of_property_read_u32()
The pinctrl-spmi-mpp.c driver, using this schema, treat it as a bool,
but I still wonder how the code will parse bool with
of_property_read_u32(). Maybe it should be uint32 with value of 0 and 1?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: pinctrl: qcom,pmic-mpp: Fix schema for "qcom,paired"
2023-04-19 19:56 ` Krzysztof Kozlowski
@ 2023-04-21 19:48 ` Rob Herring
2023-06-09 21:34 ` Rob Herring
0 siblings, 1 reply; 8+ messages in thread
From: Rob Herring @ 2023-04-21 19:48 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
Krzysztof Kozlowski, linux-arm-msm, linux-gpio, devicetree,
linux-kernel
On Wed, Apr 19, 2023 at 2:56 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 18/04/2023 17:06, Rob Herring wrote:
> > The "qcom,paired" schema is all wrong. First, it's a list rather than an
> > object(dictionary). Second, it is missing a required type. The meta-schema
> > normally catches this, but schemas under "$defs" was not getting checked.
> > A fix for that is pending.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
> > index 9412b9362328..4c3e9ff82105 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
> > @@ -144,8 +144,9 @@ $defs:
> > enum: [0, 1, 2, 3, 4, 5, 6, 7]
> >
> > qcom,paired:
> > - - description:
> > - Indicates that the pin should be operating in paired mode.
> > + type: boolean
> > + description:
> > + Indicates that the pin should be operating in paired mode.
>
> Current Linux implementation uses it as a generic pinconf param
> pinconf_generic_params which is parsed by:
>
> pinconf_generic_parse_dt_config() -> parse_dt_cfg() ->
> of_property_read_u32()
>
>
> The pinctrl-spmi-mpp.c driver, using this schema, treat it as a bool,
> but I still wonder how the code will parse bool with
> of_property_read_u32(). Maybe it should be uint32 with value of 0 and 1?
That should be an error because the length is too short so it should
go with some default from the code.
Looks like there is no user, though no property could mean keep the
default/bootloader setting. Can you sort out which type is really
desired here and hopefully we can get rid of the other type. It's not
the first case of pinctrl properties with multiple types, but we don't
really need more.
Rob
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: pinctrl: qcom,pmic-mpp: Fix schema for "qcom,paired"
2023-04-21 19:48 ` Rob Herring
@ 2023-06-09 21:34 ` Rob Herring
2023-06-13 6:48 ` Krzysztof Kozlowski
0 siblings, 1 reply; 8+ messages in thread
From: Rob Herring @ 2023-06-09 21:34 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bjorn Andersson
Cc: Andy Gross, Konrad Dybcio, Linus Walleij, linux-arm-msm,
linux-gpio, devicetree, linux-kernel
On Fri, Apr 21, 2023 at 1:48 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Apr 19, 2023 at 2:56 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 18/04/2023 17:06, Rob Herring wrote:
> > > The "qcom,paired" schema is all wrong. First, it's a list rather than an
> > > object(dictionary). Second, it is missing a required type. The meta-schema
> > > normally catches this, but schemas under "$defs" was not getting checked.
> > > A fix for that is pending.
> > >
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > ---
> > > Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
> > > index 9412b9362328..4c3e9ff82105 100644
> > > --- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
> > > +++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
> > > @@ -144,8 +144,9 @@ $defs:
> > > enum: [0, 1, 2, 3, 4, 5, 6, 7]
> > >
> > > qcom,paired:
> > > - - description:
> > > - Indicates that the pin should be operating in paired mode.
> > > + type: boolean
> > > + description:
> > > + Indicates that the pin should be operating in paired mode.
> >
> > Current Linux implementation uses it as a generic pinconf param
> > pinconf_generic_params which is parsed by:
> >
> > pinconf_generic_parse_dt_config() -> parse_dt_cfg() ->
> > of_property_read_u32()
> >
> >
> > The pinctrl-spmi-mpp.c driver, using this schema, treat it as a bool,
> > but I still wonder how the code will parse bool with
> > of_property_read_u32(). Maybe it should be uint32 with value of 0 and 1?
>
> That should be an error because the length is too short so it should
> go with some default from the code.
>
> Looks like there is no user, though no property could mean keep the
> default/bootloader setting. Can you sort out which type is really
> desired here and hopefully we can get rid of the other type. It's not
> the first case of pinctrl properties with multiple types, but we don't
> really need more.
Still an issue here. Please sort out what functionality QCom wants here.
dtschema (main br) will now throw a warning on it.
Rob
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: pinctrl: qcom,pmic-mpp: Fix schema for "qcom,paired"
2023-06-09 21:34 ` Rob Herring
@ 2023-06-13 6:48 ` Krzysztof Kozlowski
0 siblings, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-13 6:48 UTC (permalink / raw)
To: Rob Herring, Bjorn Andersson
Cc: Andy Gross, Konrad Dybcio, Linus Walleij, linux-arm-msm,
linux-gpio, devicetree, linux-kernel
On 09/06/2023 23:34, Rob Herring wrote:
> On Fri, Apr 21, 2023 at 1:48 PM Rob Herring <robh@kernel.org> wrote:
>>
>> On Wed, Apr 19, 2023 at 2:56 PM Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>> On 18/04/2023 17:06, Rob Herring wrote:
>>>> The "qcom,paired" schema is all wrong. First, it's a list rather than an
>>>> object(dictionary). Second, it is missing a required type. The meta-schema
>>>> normally catches this, but schemas under "$defs" was not getting checked.
>>>> A fix for that is pending.
>>>>
>>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>>> ---
>>>> Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml | 5 +++--
>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
>>>> index 9412b9362328..4c3e9ff82105 100644
>>>> --- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
>>>> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-mpp.yaml
>>>> @@ -144,8 +144,9 @@ $defs:
>>>> enum: [0, 1, 2, 3, 4, 5, 6, 7]
>>>>
>>>> qcom,paired:
>>>> - - description:
>>>> - Indicates that the pin should be operating in paired mode.
>>>> + type: boolean
>>>> + description:
>>>> + Indicates that the pin should be operating in paired mode.
>>>
>>> Current Linux implementation uses it as a generic pinconf param
>>> pinconf_generic_params which is parsed by:
>>>
>>> pinconf_generic_parse_dt_config() -> parse_dt_cfg() ->
>>> of_property_read_u32()
>>>
>>>
>>> The pinctrl-spmi-mpp.c driver, using this schema, treat it as a bool,
>>> but I still wonder how the code will parse bool with
>>> of_property_read_u32(). Maybe it should be uint32 with value of 0 and 1?
>>
>> That should be an error because the length is too short so it should
>> go with some default from the code.
>>
>> Looks like there is no user, though no property could mean keep the
>> default/bootloader setting. Can you sort out which type is really
>> desired here and hopefully we can get rid of the other type. It's not
>> the first case of pinctrl properties with multiple types, but we don't
>> really need more.
>
> Still an issue here. Please sort out what functionality QCom wants here.
>
> dtschema (main br) will now throw a warning on it.
I think we can go with your patch, after double checking my previous
concerns are not relevant here - driver reads it as bool just like other
bool properties.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: pinctrl: qcom,pmic-mpp: Fix schema for "qcom,paired"
2023-04-18 15:06 [PATCH] dt-bindings: pinctrl: qcom,pmic-mpp: Fix schema for "qcom,paired" Rob Herring
2023-04-19 19:56 ` Krzysztof Kozlowski
@ 2023-06-13 6:50 ` Krzysztof Kozlowski
2023-06-13 13:46 ` Rob Herring
1 sibling, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-13 6:50 UTC (permalink / raw)
To: Rob Herring, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Linus Walleij, Krzysztof Kozlowski
Cc: linux-arm-msm, linux-gpio, devicetree, linux-kernel
On 18/04/2023 17:06, Rob Herring wrote:
> The "qcom,paired" schema is all wrong. First, it's a list rather than an
> object(dictionary). Second, it is missing a required type. The meta-schema
> normally catches this, but schemas under "$defs" was not getting checked.
> A fix for that is pending.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
Linus,
Could you take it for current fixes? The code was wrong and dtschema is
warning now about this.
Fixes: f9a06b810951 ("dt-bindings: pinctrl: qcom,pmic-mpp: Convert qcom
pmic mpp bindings to YAML")
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] dt-bindings: pinctrl: qcom,pmic-mpp: Fix schema for "qcom,paired"
2023-06-13 6:50 ` Krzysztof Kozlowski
@ 2023-06-13 13:46 ` Rob Herring
2023-06-13 18:20 ` Linus Walleij
0 siblings, 1 reply; 8+ messages in thread
From: Rob Herring @ 2023-06-13 13:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
Krzysztof Kozlowski, linux-arm-msm, linux-gpio, devicetree,
linux-kernel
On Tue, Jun 13, 2023 at 12:50 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 18/04/2023 17:06, Rob Herring wrote:
> > The "qcom,paired" schema is all wrong. First, it's a list rather than an
> > object(dictionary). Second, it is missing a required type. The meta-schema
> > normally catches this, but schemas under "$defs" was not getting checked.
> > A fix for that is pending.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
>
> Linus,
> Could you take it for current fixes? The code was wrong and dtschema is
> warning now about this.
I have other things ready for 6.4, so I'll add this one.
Thanks,
Rob
>
> Fixes: f9a06b810951 ("dt-bindings: pinctrl: qcom,pmic-mpp: Convert qcom
> pmic mpp bindings to YAML")
>
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] dt-bindings: pinctrl: qcom,pmic-mpp: Fix schema for "qcom,paired"
2023-06-13 13:46 ` Rob Herring
@ 2023-06-13 18:20 ` Linus Walleij
0 siblings, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2023-06-13 18:20 UTC (permalink / raw)
To: Rob Herring
Cc: Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Krzysztof Kozlowski, linux-arm-msm, linux-gpio, devicetree,
linux-kernel
On Tue, Jun 13, 2023 at 3:46 PM Rob Herring <robh@kernel.org> wrote:
> On Tue, Jun 13, 2023 at 12:50 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 18/04/2023 17:06, Rob Herring wrote:
> > > The "qcom,paired" schema is all wrong. First, it's a list rather than an
> > > object(dictionary). Second, it is missing a required type. The meta-schema
> > > normally catches this, but schemas under "$defs" was not getting checked.
> > > A fix for that is pending.
> > >
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > ---
> >
> > Linus,
> > Could you take it for current fixes? The code was wrong and dtschema is
> > warning now about this.
>
> I have other things ready for 6.4, so I'll add this one.
Thanks for picking this one Rob!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-06-13 18:20 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-18 15:06 [PATCH] dt-bindings: pinctrl: qcom,pmic-mpp: Fix schema for "qcom,paired" Rob Herring
2023-04-19 19:56 ` Krzysztof Kozlowski
2023-04-21 19:48 ` Rob Herring
2023-06-09 21:34 ` Rob Herring
2023-06-13 6:48 ` Krzysztof Kozlowski
2023-06-13 6:50 ` Krzysztof Kozlowski
2023-06-13 13:46 ` Rob Herring
2023-06-13 18:20 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).