Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230418150606.1528107-1-robh@kernel.org> <9543f619-88fa-8e54-6e9a-4334750e51b4@linaro.org>
In-Reply-To: <9543f619-88fa-8e54-6e9a-4334750e51b4@linaro.org>
From:   Rob Herring <robh@kernel.org>
Date:   Fri, 21 Apr 2023 14:48:11 -0500
Message-ID: <CAL_Jsq+ZVAZc1nYJVLPQt=KM1qOZrZCrRC4q_o8XQjDdo_NuKA@mail.gmail.com>
Subject: Re: [PATCH] dt-bindings: pinctrl: qcom,pmic-mpp: Fix schema for "qcom,paired"
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-arm-msm@vger.kernel.org, linux-gpio@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Apr 19, 2023 at 2:56=E2=80=AFPM 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 a=
n
> > object(dictionary). Second, it is missing a required type. The meta-sch=
ema
> > normally catches this, but schemas under "$defs" was not getting checke=
d.
> > 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.ya=
ml 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