Re: [PATCH v7 5/6] Backward compatible to previous Mediatek's bias-pull usage

Re: [PATCH v7 5/6] Backward compatible to previous Mediatek's bias-pull usage

[Linus Walleij]

On Tue, Dec 31, 2019 at 2:41 PM Light Hsieh <light.hsieh@mediatek.com> wrote:

> Refine mtk_pinconf_set()/mtk_pinconf_get() for backward compatibility to
> previous MediaTek's bias-pull usage.
> In PINCTRL_MTK that use pinctrl-mtk-common.c, bias-pull setting for pins
> with 2 pull resistors can be specified as value for bias-pull-up and
> bias-pull-down. For example:
>     bias-pull-up = <MTK_PUPD_SET_R1R0_00>;
>     bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
>     bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
>     bias-pull-up = <MTK_PUPD_SET_R1R0_11>;
>     bias-pull-down = <MTK_PUPD_SET_R1R0_00>;
>     bias-pull-down = <MTK_PUPD_SET_R1R0_01>;
>     bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
>     bias-pull-down = <MTK_PUPD_SET_R1R0_11>;
>
> On the other hand, PINCTRL_MTK_PARIS use customized properties
> "mediatek,pull-up-adv" and "mediatek,pull-down-adv" to specify bias-pull
> setting for pins with 2 pull resistors.
> This introduce in-compatibility in device tree and increase porting
> effort to MediaTek's customer that had already used PINCTRL_MTK version.
> Besides, if customers are not aware of this change and still write devicetree
> for PINCTRL_MTK version, they may encounter runtime failure with pinctrl and
> spent time to debug.
>
> This patch adds backward compatible to previous MediaTek's bias-pull usage
> so that Mediatek's customer need not use a new devicetree property name.
> The rationale is that: changing driver implementation had better leave
> interface unchanged.

Are these devicetree bindings upstream, or are these bindings never
submitted for inclusion in the official device tree bindings?

I don't really want to encourage out-of-tree non-canonical device
tree experiments.

On the other hand I want running code.

I suppose if there is a solid use case for backwards compatibility
that also affect independent developers (such as people just
hacking around with these devices) then we could add it.

Yours,
Linus Walleij

Re: [PATCH v7 5/6] Backward compatible to previous Mediatek's bias-pull usage

[Light Hsieh]

On Tue, 2020-01-07 at 11:40 +0100, Linus Walleij wrote:
> On Tue, Dec 31, 2019 at 2:41 PM Light Hsieh <light.hsieh@mediatek.com> wrote:
> 
> > Refine mtk_pinconf_set()/mtk_pinconf_get() for backward compatibility to
> > previous MediaTek's bias-pull usage.
> > In PINCTRL_MTK that use pinctrl-mtk-common.c, bias-pull setting for pins
> > with 2 pull resistors can be specified as value for bias-pull-up and
> > bias-pull-down. For example:
> >     bias-pull-up = <MTK_PUPD_SET_R1R0_00>;
> >     bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> >     bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
> >     bias-pull-up = <MTK_PUPD_SET_R1R0_11>;
> >     bias-pull-down = <MTK_PUPD_SET_R1R0_00>;
> >     bias-pull-down = <MTK_PUPD_SET_R1R0_01>;
> >     bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> >     bias-pull-down = <MTK_PUPD_SET_R1R0_11>;
> >
> > On the other hand, PINCTRL_MTK_PARIS use customized properties
> > "mediatek,pull-up-adv" and "mediatek,pull-down-adv" to specify bias-pull
> > setting for pins with 2 pull resistors.
> > This introduce in-compatibility in device tree and increase porting
> > effort to MediaTek's customer that had already used PINCTRL_MTK version.
> > Besides, if customers are not aware of this change and still write devicetree
> > for PINCTRL_MTK version, they may encounter runtime failure with pinctrl and
> > spent time to debug.
> >
> > This patch adds backward compatible to previous MediaTek's bias-pull usage
> > so that Mediatek's customer need not use a new devicetree property name.
> > The rationale is that: changing driver implementation had better leave
> > interface unchanged.
> 
> Are these devicetree bindings upstream, or are these bindings never
> submitted for inclusion in the official device tree bindings?
> 
> I don't really want to encourage out-of-tree non-canonical device
> tree experiments.
> 
> On the other hand I want running code.
> 
> I suppose if there is a solid use case for backwards compatibility
> that also affect independent developers (such as people just
> hacking around with these devices) then we could add it.
> 
> Yours,
> Linus Walleij

Use of "bias-pull-down = <MTK_PUPD_SET_R1R0_XX>" had already been
described in
Documentation/devicetree/binding/pinctrl/pinctrl-mt65xx.txt.
Previous MediaTek chips using pinctrl-mtk-common.c had already use such
binding. This is so-called backward compatibility.

Besides, according to
Documentation/devicetree/binding/pinctrl/pincfg-node.yaml, bias-pull-up
and bias-pull-down can have an optional argument to select pull strength
when supported by hardware.
So, I think the propose of using "mediatek,pull-up-adv" and
"mediatek,pull-down-adv" is not necessary. However, I fail to stop
upstream of using "mediatek,pull-up-adv" and "mediatek,pull-down-adv" in
pinctrl-mtk-common-v2.c.

Light Hsieh

Re: [PATCH v7 5/6] Backward compatible to previous Mediatek's bias-pull usage

[Linus Walleij]

On Tue, Jan 7, 2020 at 1:16 PM Light Hsieh <light.hsieh@mediatek.com> wrote:

> Use of "bias-pull-down = <MTK_PUPD_SET_R1R0_XX>" had already been
> described in
> Documentation/devicetree/binding/pinctrl/pinctrl-mt65xx.txt.
> Previous MediaTek chips using pinctrl-mtk-common.c had already use such
> binding. This is so-called backward compatibility.
>
> Besides, according to
> Documentation/devicetree/binding/pinctrl/pincfg-node.yaml, bias-pull-up
> and bias-pull-down can have an optional argument to select pull strength
> when supported by hardware.

That's right, I was wrong about this. This should be fine to use.

> So, I think the propose of using "mediatek,pull-up-adv" and
> "mediatek,pull-down-adv" is not necessary. However, I fail to stop
> upstream of using "mediatek,pull-up-adv" and "mediatek,pull-down-adv" in
> pinctrl-mtk-common-v2.c.

Let's see what Sean says, I am fine with the patch set in general.

Yours,
Linus Walleij