Re: [PATCH v2 03/18] dt-bindings: pinctrl: mediatek,mt8365-pinctrl: add drive strength property

[AngeloGioacchino Del Regno]

Il 10/03/23 09:32, Krzysztof Kozlowski ha scritto:
> On 07/03/2023 14:17, Alexandre Mergnat wrote:
> > This SoC is able to drive the following output current:
> > - 2 mA
> > - 4 mA
> > - 6 mA
> > - 8 mA
> > - 10 mA
> > - 12 mA
> > - 14 mA
> > - 16 mA
> >
> > Then drive-strength property is set with enum to reflect its HW capability.
> >
> > Signed-off-by: Alexandre Mergnat <amergnat@xxxxxxxxxxxx>
> > ---
> >   Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml
> > index 4b96884a1afc..101871ec6693 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt8365-pinctrl.yaml
> > @@ -79,6 +79,9 @@ patternProperties:
> >   
> >             bias-pull-down: true
> >   
> > +          drive-strength:
> > +            enum: [2, 4, 6, 8, 10, 12, 14, 16]
>
> Isn't this conflicting with mediatek,drive-strength-adv? Your commit msg
> suggests you add a missing property, but I would say nothing was missing
> here.
>
> You need review from (pinctrl) Mediatek maintainers how the bindings for
> all Mediateks are organized.

Hello Krzysztof,

mediatek,drive-strength-adv *shall not exist*, that was an unnecessary property
that leaked upstream from downstream kernels and there's no reason to use it.

Upstream, we have drive-strength-microamp and mediatek,rsel-resistance-in-si-unit.

Since mediatek,mt8365-pinctrl.yaml got picked with that property already, I have
nothing to complain about this specific commit... drive-strength does not conflict
with the mediatek,drive-strength-adv property, as the "adv" is for microamp
adjustments.

You can pick it, it's fine.

Anyway, Alexandre: can you please perform a cleanup to the MT8365 pinctrl binding?
The cleanup means you're setting mediatek,drive-strength-adv as deprecated and
adding the right properties (...and possibly changing the devicetrees to use it).

For more information, you can look at commit history for the (unfortunately, named
incorrectly) MT8195 pinctrl documentation: bindings/pinctrl/pinctrl-mt8195.yaml
where we performed the same cleanup that I'm asking you to do, except we didn't
have to set any property as deprecated because there was *no devicetree upstream*
that was actually using that property (hence not an ABI breakage).

Cheers!
Angelo