* [PATCH v10 0/2] Mediatek pinctrl patch on mt8195 @ 2021-07-10 8:17 Zhiyong Tao 2021-07-10 8:17 ` [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define Zhiyong Tao 2021-07-10 8:17 ` [PATCH v10 2/2] pinctrl: mediatek: add rsel setting on MT8195 Zhiyong Tao 0 siblings, 2 replies; 19+ messages in thread From: Zhiyong Tao @ 2021-07-10 8:17 UTC (permalink / raw) To: robh+dt, linus.walleij, mark.rutland, matthias.bgg, sean.wang Cc: srv_heupstream, zhiyong.tao, hui.liu, eddie.huang, light.hsieh, biao.huang, hongzhou.yang, sean.wang, seiya.wang, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, linux-gpio This series includes 2 patches: 1.add rsel define 2.add pinctrl rsel setting on MT8195. Changes in patch v10: 1)fix PARENTHESIS_ALIGNMENT of mtk_pinconf_bias_set_rsel 2)fix LONG_LINE warning in 615 in pinctrl-paris.c. Changes in patch v9: 1)fix "mtk_pinconf_bias_set_rsel" build warning. Changes in patch v8: 1)add rsel define patch 2)avoid CamelCase 3)add pinctrl rsel setting patch which is another resistance selection solution for I2C on MT8195. Changes in patch v7: 1)add version in patch and fix spelling mistakes. Changes in patch v6: 1)add "pintcrl: mediatek" as prefix. Changes in patch v5: 1)document and driver patch are apploed. 2)change '-EOPNOTSUPP' to '-ENOTSUPP' Changes in patch v4: 1)fix pinctrl-mt8195.yaml warning error. 2)remove pinctrl device node patch which is based on "mt8195.dtsi". Changes in patch v3: 1)change '^pins' to '-pins$'. 2)change 'state_0_node_a' to 'gpio_pin' which is defined in dts. 3)change 'state_0_node_b' to 'i2c0_pin' which is defined in dts. 4)reorder this series patches. change pinctrl file and binding document together in one patch. There are no changes in v1 & v2. Zhiyong Tao (2): dt-bindings: pinctrl: mt8195: add rsel define pinctrl: mediatek: add rsel setting on MT8195 drivers/pinctrl/mediatek/pinctrl-mt8195.c | 96 +++++++++++++ .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 134 +++++++++++++++--- .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 10 +- drivers/pinctrl/mediatek/pinctrl-paris.c | 24 +++- drivers/pinctrl/mediatek/pinctrl-paris.h | 2 +- include/dt-bindings/pinctrl/mt65xx.h | 9 ++ 6 files changed, 248 insertions(+), 27 deletions(-) -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-07-10 8:17 [PATCH v10 0/2] Mediatek pinctrl patch on mt8195 Zhiyong Tao @ 2021-07-10 8:17 ` Zhiyong Tao 2021-07-13 7:17 ` Chen-Yu Tsai 2021-07-10 8:17 ` [PATCH v10 2/2] pinctrl: mediatek: add rsel setting on MT8195 Zhiyong Tao 1 sibling, 1 reply; 19+ messages in thread From: Zhiyong Tao @ 2021-07-10 8:17 UTC (permalink / raw) To: robh+dt, linus.walleij, mark.rutland, matthias.bgg, sean.wang Cc: srv_heupstream, zhiyong.tao, hui.liu, eddie.huang, light.hsieh, biao.huang, hongzhou.yang, sean.wang, seiya.wang, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, linux-gpio This patch adds rsel define for mt8195. Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> --- include/dt-bindings/pinctrl/mt65xx.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/dt-bindings/pinctrl/mt65xx.h b/include/dt-bindings/pinctrl/mt65xx.h index 7e16e58fe1f7..f5934abcd1bd 100644 --- a/include/dt-bindings/pinctrl/mt65xx.h +++ b/include/dt-bindings/pinctrl/mt65xx.h @@ -16,6 +16,15 @@ #define MTK_PUPD_SET_R1R0_10 102 #define MTK_PUPD_SET_R1R0_11 103 +#define MTK_PULL_SET_RSEL_000 200 +#define MTK_PULL_SET_RSEL_001 201 +#define MTK_PULL_SET_RSEL_010 202 +#define MTK_PULL_SET_RSEL_011 203 +#define MTK_PULL_SET_RSEL_100 204 +#define MTK_PULL_SET_RSEL_101 205 +#define MTK_PULL_SET_RSEL_110 206 +#define MTK_PULL_SET_RSEL_111 207 + #define MTK_DRIVE_2mA 2 #define MTK_DRIVE_4mA 4 #define MTK_DRIVE_6mA 6 -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-07-10 8:17 ` [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define Zhiyong Tao @ 2021-07-13 7:17 ` Chen-Yu Tsai 2021-07-22 7:54 ` zhiyong tao 0 siblings, 1 reply; 19+ messages in thread From: Chen-Yu Tsai @ 2021-07-13 7:17 UTC (permalink / raw) To: Zhiyong Tao Cc: Rob Herring, Linus Walleij, mark.rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, light.hsieh, biao.huang, hongzhou.yang, sean.wang, seiya.wang, devicetree, LKML, linux-arm-kernel, linux-mediatek, linux-gpio Hi, On Sat, Jul 10, 2021 at 4:17 PM Zhiyong Tao <zhiyong.tao@mediatek.com> wrote: > > This patch adds rsel define for mt8195. > > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> > --- > include/dt-bindings/pinctrl/mt65xx.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/include/dt-bindings/pinctrl/mt65xx.h b/include/dt-bindings/pinctrl/mt65xx.h > index 7e16e58fe1f7..f5934abcd1bd 100644 > --- a/include/dt-bindings/pinctrl/mt65xx.h > +++ b/include/dt-bindings/pinctrl/mt65xx.h > @@ -16,6 +16,15 @@ > #define MTK_PUPD_SET_R1R0_10 102 > #define MTK_PUPD_SET_R1R0_11 103 > > +#define MTK_PULL_SET_RSEL_000 200 > +#define MTK_PULL_SET_RSEL_001 201 > +#define MTK_PULL_SET_RSEL_010 202 > +#define MTK_PULL_SET_RSEL_011 203 > +#define MTK_PULL_SET_RSEL_100 204 > +#define MTK_PULL_SET_RSEL_101 205 > +#define MTK_PULL_SET_RSEL_110 206 > +#define MTK_PULL_SET_RSEL_111 207 > + Instead of all the obscure macros and the new custom "rsel" property, which BTW is not in the bindings, can't we just list the actual bias resistance of each setting? We could also migrate away from R1R0. Then we can specify the setting with the standard bias-pull-up/down properties [1]. Also, please ask internally if Mediatek could relicense all the header files that Mediatek has contributed under include/dt-bindings/pinctrl/ [2] to GPL-2.0 and BSD dual license. These files are part of the DT bindings and we really want them to be dual licensed as well, and not just the YAML files. Regards ChenYu [1] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml#L37 [2] Note that a few files were contributed by other people > #define MTK_DRIVE_2mA 2 > #define MTK_DRIVE_4mA 4 > #define MTK_DRIVE_6mA 6 > -- > 2.18.0 > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-07-13 7:17 ` Chen-Yu Tsai @ 2021-07-22 7:54 ` zhiyong tao 2021-07-26 8:02 ` Chen-Yu Tsai 0 siblings, 1 reply; 19+ messages in thread From: zhiyong tao @ 2021-07-22 7:54 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Rob Herring, Linus Walleij, mark.rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, light.hsieh, biao.huang, hongzhou.yang, sean.wang, seiya.wang, devicetree, LKML, linux-arm-kernel, linux-mediatek, linux-gpio On Tue, 2021-07-13 at 15:17 +0800, Chen-Yu Tsai wrote: > Hi, > > On Sat, Jul 10, 2021 at 4:17 PM Zhiyong Tao <zhiyong.tao@mediatek.com> wrote: > > > > This patch adds rsel define for mt8195. > > > > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> > > --- > > include/dt-bindings/pinctrl/mt65xx.h | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/include/dt-bindings/pinctrl/mt65xx.h b/include/dt-bindings/pinctrl/mt65xx.h > > index 7e16e58fe1f7..f5934abcd1bd 100644 > > --- a/include/dt-bindings/pinctrl/mt65xx.h > > +++ b/include/dt-bindings/pinctrl/mt65xx.h > > @@ -16,6 +16,15 @@ > > #define MTK_PUPD_SET_R1R0_10 102 > > #define MTK_PUPD_SET_R1R0_11 103 > > > > +#define MTK_PULL_SET_RSEL_000 200 > > +#define MTK_PULL_SET_RSEL_001 201 > > +#define MTK_PULL_SET_RSEL_010 202 > > +#define MTK_PULL_SET_RSEL_011 203 > > +#define MTK_PULL_SET_RSEL_100 204 > > +#define MTK_PULL_SET_RSEL_101 205 > > +#define MTK_PULL_SET_RSEL_110 206 > > +#define MTK_PULL_SET_RSEL_111 207 > > + > > Instead of all the obscure macros and the new custom "rsel" property, > which BTW is not in the bindings, can't we just list the actual bias > resistance of each setting? We could also migrate away from R1R0. > ==>Hi Chenyu, The rsel actual bias resistance of each setting: MTK_PULL_SET_RSEL_000:75K in PU, 75k in PD; MTK_PULL_SET_RSEL_001:10k in PU, 5k in PD; MTK_PULL_SET_RSEL_010:5k in PU, 75k in PD; MTK_PULL_SET_RSEL_011:4k in PU, 5K in PD; MTK_PULL_SET_RSEL_100:3k in PU, 75k in PD; MTK_PULL_SET_RSEL_101:2k in PU, 5K in PD; MTK_PULL_SET_RSEL_110:1.5k in PU, 75k in PD; MTK_PULL_SET_RSEL_111:1k in PU, 5k in PD. The rsel actual bias resistance is different between PU and PD. > Then we can specify the setting with the standard bias-pull-up/down > properties [1]. > > Also, please ask internally if Mediatek could relicense all the header > files that Mediatek has contributed under include/dt-bindings/pinctrl/ [2] > to GPL-2.0 and BSD dual license. These files are part of the DT bindings > and we really want them to be dual licensed as well, and not just the > YAML files. > ==> We will confirm it internally and reply it later. Thanks. > > Regards > ChenYu > > > [1] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml#L37 > [2] Note that a few files were contributed by other people > > > #define MTK_DRIVE_2mA 2 > > #define MTK_DRIVE_4mA 4 > > #define MTK_DRIVE_6mA 6 > > -- > > 2.18.0 > > _______________________________________________ > > Linux-mediatek mailing list > > Linux-mediatek@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-mediatek _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-07-22 7:54 ` zhiyong tao @ 2021-07-26 8:02 ` Chen-Yu Tsai 2021-07-29 8:23 ` zhiyong.tao 0 siblings, 1 reply; 19+ messages in thread From: Chen-Yu Tsai @ 2021-07-26 8:02 UTC (permalink / raw) To: zhiyong tao Cc: Rob Herring, Linus Walleij, mark.rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, light.hsieh, biao.huang, hongzhou.yang, sean.wang, seiya.wang, devicetree, LKML, linux-arm-kernel, linux-mediatek, linux-gpio On Thu, Jul 22, 2021 at 3:54 PM zhiyong tao <zhiyong.tao@mediatek.com> wrote: > > On Tue, 2021-07-13 at 15:17 +0800, Chen-Yu Tsai wrote: > > Hi, > > > > On Sat, Jul 10, 2021 at 4:17 PM Zhiyong Tao <zhiyong.tao@mediatek.com> wrote: > > > > > > This patch adds rsel define for mt8195. > > > > > > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> > > > --- > > > include/dt-bindings/pinctrl/mt65xx.h | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > diff --git a/include/dt-bindings/pinctrl/mt65xx.h b/include/dt-bindings/pinctrl/mt65xx.h > > > index 7e16e58fe1f7..f5934abcd1bd 100644 > > > --- a/include/dt-bindings/pinctrl/mt65xx.h > > > +++ b/include/dt-bindings/pinctrl/mt65xx.h > > > @@ -16,6 +16,15 @@ > > > #define MTK_PUPD_SET_R1R0_10 102 > > > #define MTK_PUPD_SET_R1R0_11 103 > > > > > > +#define MTK_PULL_SET_RSEL_000 200 > > > +#define MTK_PULL_SET_RSEL_001 201 > > > +#define MTK_PULL_SET_RSEL_010 202 > > > +#define MTK_PULL_SET_RSEL_011 203 > > > +#define MTK_PULL_SET_RSEL_100 204 > > > +#define MTK_PULL_SET_RSEL_101 205 > > > +#define MTK_PULL_SET_RSEL_110 206 > > > +#define MTK_PULL_SET_RSEL_111 207 > > > + > > > > Instead of all the obscure macros and the new custom "rsel" property, > > which BTW is not in the bindings, can't we just list the actual bias > > resistance of each setting? We could also migrate away from R1R0. > > > ==>Hi Chenyu, > The rsel actual bias resistance of each setting: > > MTK_PULL_SET_RSEL_000:75K in PU, 75k in PD; > MTK_PULL_SET_RSEL_001:10k in PU, 5k in PD; > MTK_PULL_SET_RSEL_010:5k in PU, 75k in PD; > MTK_PULL_SET_RSEL_011:4k in PU, 5K in PD; > MTK_PULL_SET_RSEL_100:3k in PU, 75k in PD; > MTK_PULL_SET_RSEL_101:2k in PU, 5K in PD; > MTK_PULL_SET_RSEL_110:1.5k in PU, 75k in PD; > MTK_PULL_SET_RSEL_111:1k in PU, 5k in PD. > > The rsel actual bias resistance is different between PU and PD. Thanks. Somehow I missed this when looking through the datasheet. This encoding is interesting. Since it doesn't make sense to have both pull-up and pull-down, even though the hardware seems capable of doing so, I suppose the intent is to support 75k or 5k for pull-down, and (75k, 10k, 5k, 4k, 3k, 2k, 1.5k, 1k) for pull-up? We could add these values to the binding so we could check for misuse. The range of values seems to also cover those supported by the alternative R0/R1 settings. The values for kprow[01] and kpcol[01] seem to be different though. We should get rid of the MTK_PUPD_SET_R1R0_* macros at the same time. They seem to be some magic values used with bias-pull-*, which is not how the properties should be used. At the same time, they overlap with mediatek,pull-* properties. It would be great if we could standardize on the generic pinconf properties, and also use real values that fit the requirements of the properties, i.e. using real resistance values. I'm not sure if it would make sense to enumerate which pins support which configurations though. Thanks ChenYu > > Then we can specify the setting with the standard bias-pull-up/down > > properties [1]. > > > > Also, please ask internally if Mediatek could relicense all the header > > files that Mediatek has contributed under include/dt-bindings/pinctrl/ [2] > > to GPL-2.0 and BSD dual license. These files are part of the DT bindings > > and we really want them to be dual licensed as well, and not just the > > YAML files. > > > > ==> We will confirm it internally and reply it later. > > Thanks. > > > > Regards > > ChenYu > > > > > > [1] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml#L37 > > [2] Note that a few files were contributed by other people > > > > > #define MTK_DRIVE_2mA 2 > > > #define MTK_DRIVE_4mA 4 > > > #define MTK_DRIVE_6mA 6 > > > -- > > > 2.18.0 > > > _______________________________________________ > > > Linux-mediatek mailing list > > > Linux-mediatek@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-mediatek > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-07-26 8:02 ` Chen-Yu Tsai @ 2021-07-29 8:23 ` zhiyong.tao 2021-07-29 9:43 ` Chen-Yu Tsai 0 siblings, 1 reply; 19+ messages in thread From: zhiyong.tao @ 2021-07-29 8:23 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Rob Herring, Linus Walleij, mark.rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, light.hsieh, biao.huang, hongzhou.yang, sean.wang, seiya.wang, devicetree, LKML, linux-arm-kernel, linux-mediatek, linux-gpio On Mon, 2021-07-26 at 16:02 +0800, Chen-Yu Tsai wrote: > On Thu, Jul 22, 2021 at 3:54 PM zhiyong tao <zhiyong.tao@mediatek.com > > wrote: > > > > On Tue, 2021-07-13 at 15:17 +0800, Chen-Yu Tsai wrote: > > > Hi, > > > > > > On Sat, Jul 10, 2021 at 4:17 PM Zhiyong Tao < > > > zhiyong.tao@mediatek.com> wrote: > > > > > > > > This patch adds rsel define for mt8195. > > > > > > > > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> > > > > --- > > > > include/dt-bindings/pinctrl/mt65xx.h | 9 +++++++++ > > > > 1 file changed, 9 insertions(+) > > > > > > > > diff --git a/include/dt-bindings/pinctrl/mt65xx.h b/include/dt- > > > > bindings/pinctrl/mt65xx.h > > > > index 7e16e58fe1f7..f5934abcd1bd 100644 > > > > --- a/include/dt-bindings/pinctrl/mt65xx.h > > > > +++ b/include/dt-bindings/pinctrl/mt65xx.h > > > > @@ -16,6 +16,15 @@ > > > > #define MTK_PUPD_SET_R1R0_10 102 > > > > #define MTK_PUPD_SET_R1R0_11 103 > > > > > > > > +#define MTK_PULL_SET_RSEL_000 200 > > > > +#define MTK_PULL_SET_RSEL_001 201 > > > > +#define MTK_PULL_SET_RSEL_010 202 > > > > +#define MTK_PULL_SET_RSEL_011 203 > > > > +#define MTK_PULL_SET_RSEL_100 204 > > > > +#define MTK_PULL_SET_RSEL_101 205 > > > > +#define MTK_PULL_SET_RSEL_110 206 > > > > +#define MTK_PULL_SET_RSEL_111 207 > > > > + > > > > > > Instead of all the obscure macros and the new custom "rsel" > > > property, > > > which BTW is not in the bindings, can't we just list the actual > > > bias > > > resistance of each setting? We could also migrate away from R1R0. > > > > > > > ==>Hi Chenyu, > > The rsel actual bias resistance of each setting: > > > > MTK_PULL_SET_RSEL_000:75K in PU, 75k in PD; > > MTK_PULL_SET_RSEL_001:10k in PU, 5k in PD; > > MTK_PULL_SET_RSEL_010:5k in PU, 75k in PD; > > MTK_PULL_SET_RSEL_011:4k in PU, 5K in PD; > > MTK_PULL_SET_RSEL_100:3k in PU, 75k in PD; > > MTK_PULL_SET_RSEL_101:2k in PU, 5K in PD; > > MTK_PULL_SET_RSEL_110:1.5k in PU, 75k in PD; > > MTK_PULL_SET_RSEL_111:1k in PU, 5k in PD. > > > > The rsel actual bias resistance is different between PU and PD. > > Thanks. Somehow I missed this when looking through the datasheet. > This > encoding is interesting. Since it doesn't make sense to have both > pull-up and pull-down, even though the hardware seems capable of > doing > so, I suppose the intent is to support 75k or 5k for pull-down, and > (75k, 10k, 5k, 4k, 3k, 2k, 1.5k, 1k) for pull-up? > > We could add these values to the binding so we could check for > misuse. > > The range of values seems to also cover those supported by the > alternative R0/R1 settings. The values for kprow[01] and kpcol[01] > seem to be different though. > > We should get rid of the MTK_PUPD_SET_R1R0_* macros at the same time. > They seem to be some magic values used with bias-pull-*, which is not > how the properties should be used. At the same time, they overlap > with > mediatek,pull-* properties. > > It would be great if we could standardize on the generic pinconf > properties, and also use real values that fit the requirements of the > properties, i.e. using real resistance values. I'm not sure if it > would make sense to enumerate which pins support which configurations > though. > > > Thanks > ChenYu > > The rsel actual bias resistance of each setting is different in different IC. we think that the define "MTK_PULL_SET_RSEL_000" is more common for all different IC. Thanks. > > > Then we can specify the setting with the standard bias-pull- > > > up/down > > > properties [1]. > > > > > > Also, please ask internally if Mediatek could relicense all the > > > header > > > files that Mediatek has contributed under include/dt- > > > bindings/pinctrl/ [2] > > > to GPL-2.0 and BSD dual license. These files are part of the DT > > > bindings > > > and we really want them to be dual licensed as well, and not just > > > the > > > YAML files. > > > > > > > ==> We will confirm it internally and reply it later. > > > > Thanks. > > > > > > Regards > > > ChenYu > > > > > > > > > [1] > > > https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml#L37 > > > [2] Note that a few files were contributed by other people > > > > > > > #define MTK_DRIVE_2mA 2 > > > > #define MTK_DRIVE_4mA 4 > > > > #define MTK_DRIVE_6mA 6 > > > > -- > > > > 2.18.0 > > > > _______________________________________________ > > > > Linux-mediatek mailing list > > > > Linux-mediatek@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-mediatek _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-07-29 8:23 ` zhiyong.tao @ 2021-07-29 9:43 ` Chen-Yu Tsai 2021-08-04 23:01 ` Linus Walleij 0 siblings, 1 reply; 19+ messages in thread From: Chen-Yu Tsai @ 2021-07-29 9:43 UTC (permalink / raw) To: zhiyong.tao Cc: Rob Herring, Linus Walleij, mark.rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, light.hsieh, biao.huang, hongzhou.yang, Sean Wang, Seiya Wang, Devicetree List, LKML, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, moderated list:ARM/Mediatek SoC support, linux-gpio On Thu, Jul 29, 2021 at 4:23 PM zhiyong.tao <zhiyong.tao@mediatek.com> wrote: > > On Mon, 2021-07-26 at 16:02 +0800, Chen-Yu Tsai wrote: > > On Thu, Jul 22, 2021 at 3:54 PM zhiyong tao <zhiyong.tao@mediatek.com > > > wrote: > > > > > > On Tue, 2021-07-13 at 15:17 +0800, Chen-Yu Tsai wrote: > > > > Hi, > > > > > > > > On Sat, Jul 10, 2021 at 4:17 PM Zhiyong Tao < > > > > zhiyong.tao@mediatek.com> wrote: > > > > > > > > > > This patch adds rsel define for mt8195. > > > > > > > > > > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> > > > > > --- > > > > > include/dt-bindings/pinctrl/mt65xx.h | 9 +++++++++ > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > diff --git a/include/dt-bindings/pinctrl/mt65xx.h b/include/dt- > > > > > bindings/pinctrl/mt65xx.h > > > > > index 7e16e58fe1f7..f5934abcd1bd 100644 > > > > > --- a/include/dt-bindings/pinctrl/mt65xx.h > > > > > +++ b/include/dt-bindings/pinctrl/mt65xx.h > > > > > @@ -16,6 +16,15 @@ > > > > > #define MTK_PUPD_SET_R1R0_10 102 > > > > > #define MTK_PUPD_SET_R1R0_11 103 > > > > > > > > > > +#define MTK_PULL_SET_RSEL_000 200 > > > > > +#define MTK_PULL_SET_RSEL_001 201 > > > > > +#define MTK_PULL_SET_RSEL_010 202 > > > > > +#define MTK_PULL_SET_RSEL_011 203 > > > > > +#define MTK_PULL_SET_RSEL_100 204 > > > > > +#define MTK_PULL_SET_RSEL_101 205 > > > > > +#define MTK_PULL_SET_RSEL_110 206 > > > > > +#define MTK_PULL_SET_RSEL_111 207 > > > > > + > > > > > > > > Instead of all the obscure macros and the new custom "rsel" > > > > property, > > > > which BTW is not in the bindings, can't we just list the actual > > > > bias > > > > resistance of each setting? We could also migrate away from R1R0. > > > > > > > > > > ==>Hi Chenyu, > > > The rsel actual bias resistance of each setting: > > > > > > MTK_PULL_SET_RSEL_000:75K in PU, 75k in PD; > > > MTK_PULL_SET_RSEL_001:10k in PU, 5k in PD; > > > MTK_PULL_SET_RSEL_010:5k in PU, 75k in PD; > > > MTK_PULL_SET_RSEL_011:4k in PU, 5K in PD; > > > MTK_PULL_SET_RSEL_100:3k in PU, 75k in PD; > > > MTK_PULL_SET_RSEL_101:2k in PU, 5K in PD; > > > MTK_PULL_SET_RSEL_110:1.5k in PU, 75k in PD; > > > MTK_PULL_SET_RSEL_111:1k in PU, 5k in PD. > > > > > > The rsel actual bias resistance is different between PU and PD. > > > > Thanks. Somehow I missed this when looking through the datasheet. > > This > > encoding is interesting. Since it doesn't make sense to have both > > pull-up and pull-down, even though the hardware seems capable of > > doing > > so, I suppose the intent is to support 75k or 5k for pull-down, and > > (75k, 10k, 5k, 4k, 3k, 2k, 1.5k, 1k) for pull-up? > > > > We could add these values to the binding so we could check for > > misuse. > > > > The range of values seems to also cover those supported by the > > alternative R0/R1 settings. The values for kprow[01] and kpcol[01] > > seem to be different though. > > > > We should get rid of the MTK_PUPD_SET_R1R0_* macros at the same time. > > They seem to be some magic values used with bias-pull-*, which is not > > how the properties should be used. At the same time, they overlap > > with > > mediatek,pull-* properties. > > > > It would be great if we could standardize on the generic pinconf > > properties, and also use real values that fit the requirements of the > > properties, i.e. using real resistance values. I'm not sure if it > > would make sense to enumerate which pins support which configurations > > though. > > > > > > Thanks > > ChenYu > > > > > The rsel actual bias resistance of each setting is different in > different IC. we think that the define "MTK_PULL_SET_RSEL_000" is more > common for all different IC. I see. I personally prefer having things clearly described. I can understand this might be an extra burden to support different chips with different parameters, though this should be fairly straightforward with lookup tables tied to the compatible strings. Let's see if Rob and Linus have anything to add. ChenYu > Thanks. > > > > > Then we can specify the setting with the standard bias-pull- > > > > up/down > > > > properties [1]. > > > > > > > > Also, please ask internally if Mediatek could relicense all the > > > > header > > > > files that Mediatek has contributed under include/dt- > > > > bindings/pinctrl/ [2] > > > > to GPL-2.0 and BSD dual license. These files are part of the DT > > > > bindings > > > > and we really want them to be dual licensed as well, and not just > > > > the > > > > YAML files. > > > > > > > > > > ==> We will confirm it internally and reply it later. > > > > > > Thanks. > > > > > > > > Regards > > > > ChenYu > > > > > > > > > > > > [1] > > > > https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml#L37 > > > > [2] Note that a few files were contributed by other people > > > > > > > > > #define MTK_DRIVE_2mA 2 > > > > > #define MTK_DRIVE_4mA 4 > > > > > #define MTK_DRIVE_6mA 6 > > > > > -- > > > > > 2.18.0 > > > > > _______________________________________________ > > > > > Linux-mediatek mailing list > > > > > Linux-mediatek@lists.infradead.org > > > > > http://lists.infradead.org/mailman/listinfo/linux-mediatek _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-07-29 9:43 ` Chen-Yu Tsai @ 2021-08-04 23:01 ` Linus Walleij 2021-08-16 6:10 ` Chen-Yu Tsai 0 siblings, 1 reply; 19+ messages in thread From: Linus Walleij @ 2021-08-04 23:01 UTC (permalink / raw) To: Chen-Yu Tsai Cc: zhiyong.tao, Rob Herring, Mark Rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, Light Hsieh, Biao Huang, Hongzhou Yang, Sean Wang, Seiya Wang, Devicetree List, LKML, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, moderated list:ARM/Mediatek SoC support, open list:GPIO SUBSYSTEM On Thu, Jul 29, 2021 at 11:43 AM Chen-Yu Tsai <wenst@chromium.org> wrote: > On Thu, Jul 29, 2021 at 4:23 PM zhiyong.tao <zhiyong.tao@mediatek.com> wrote: > > The rsel actual bias resistance of each setting is different in > > different IC. we think that the define "MTK_PULL_SET_RSEL_000" is more > > common for all different IC. > > I see. I personally prefer having things clearly described. I can > understand this might be an extra burden to support different chips > with different parameters, though this should be fairly straightforward > with lookup tables tied to the compatible strings. > > Let's see if Rob and Linus have anything to add. Not much. We have "soft pushed" for this to be described as generic as possible, using SI units (ohms). But we also allow vendor-specific numbers in this attribute. Especially when reverse engineering SoCs that the contributor don't really have specs on (example M1 Mac). The intent with the SI units is especially for people like you folks working with Chromium to be able to use different SoCs and not feel lost to a forest of different ways of doing things and associated mistakes because vendors have hopelessly idiomatic pin configs. Yours, Linus Walleij _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-08-04 23:01 ` Linus Walleij @ 2021-08-16 6:10 ` Chen-Yu Tsai 2021-08-16 10:48 ` zhiyong.tao 0 siblings, 1 reply; 19+ messages in thread From: Chen-Yu Tsai @ 2021-08-16 6:10 UTC (permalink / raw) To: Linus Walleij Cc: zhiyong.tao, Rob Herring, Mark Rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, Light Hsieh, Biao Huang, Hongzhou Yang, Sean Wang, Seiya Wang, Devicetree List, LKML, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, moderated list:ARM/Mediatek SoC support, open list:GPIO SUBSYSTEM On Thu, Aug 5, 2021 at 7:02 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Thu, Jul 29, 2021 at 11:43 AM Chen-Yu Tsai <wenst@chromium.org> wrote: > > On Thu, Jul 29, 2021 at 4:23 PM zhiyong.tao <zhiyong.tao@mediatek.com> wrote: > > > > The rsel actual bias resistance of each setting is different in > > > different IC. we think that the define "MTK_PULL_SET_RSEL_000" is more > > > common for all different IC. > > > > I see. I personally prefer having things clearly described. I can > > understand this might be an extra burden to support different chips > > with different parameters, though this should be fairly straightforward > > with lookup tables tied to the compatible strings. > > > > Let's see if Rob and Linus have anything to add. > > Not much. We have "soft pushed" for this to be described as generic > as possible, using SI units (ohms). But we also allow vendor-specific > numbers in this attribute. Especially when reverse engineering SoCs > that the contributor don't really have specs on (example M1 Mac). > > The intent with the SI units is especially for people like you folks working > with Chromium to be able to use different SoCs and not feel lost > to a forest of different ways of doing things and associated > mistakes because vendors have hopelessly idiomatic pin configs. I'll take that as "use SI units whenever possible and reasonable". Thanks. ChenYu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-08-16 6:10 ` Chen-Yu Tsai @ 2021-08-16 10:48 ` zhiyong.tao 2021-08-16 15:37 ` Chen-Yu Tsai 0 siblings, 1 reply; 19+ messages in thread From: zhiyong.tao @ 2021-08-16 10:48 UTC (permalink / raw) To: Chen-Yu Tsai, Linus Walleij Cc: Rob Herring, Mark Rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, Light Hsieh, Biao Huang, Hongzhou Yang, Sean Wang, Seiya Wang, Devicetree List, LKML, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, moderated list:ARM/Mediatek SoC support, open list:GPIO SUBSYSTEM On Mon, 2021-08-16 at 14:10 +0800, Chen-Yu Tsai wrote: > On Thu, Aug 5, 2021 at 7:02 AM Linus Walleij < > linus.walleij@linaro.org> wrote: > > > > On Thu, Jul 29, 2021 at 11:43 AM Chen-Yu Tsai <wenst@chromium.org> > > wrote: > > > On Thu, Jul 29, 2021 at 4:23 PM zhiyong.tao < > > > zhiyong.tao@mediatek.com> wrote: > > > > The rsel actual bias resistance of each setting is different in > > > > different IC. we think that the define "MTK_PULL_SET_RSEL_000" > > > > is more > > > > common for all different IC. > > > > > > I see. I personally prefer having things clearly described. I can > > > understand this might be an extra burden to support different > > > chips > > > with different parameters, though this should be fairly > > > straightforward > > > with lookup tables tied to the compatible strings. > > > > > > Let's see if Rob and Linus have anything to add. > > > > Not much. We have "soft pushed" for this to be described as generic > > as possible, using SI units (ohms). But we also allow vendor- > > specific > > numbers in this attribute. Especially when reverse engineering SoCs > > that the contributor don't really have specs on (example M1 Mac). > > > > The intent with the SI units is especially for people like you > > folks working > > with Chromium to be able to use different SoCs and not feel lost > > to a forest of different ways of doing things and associated > > mistakes because vendors have hopelessly idiomatic pin configs. > > I'll take that as "use SI units whenever possible and reasonable". ==> so It doesn't need to change the define, is it right? we will keep the common define. Thanks. > > Thanks. > > ChenYu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-08-16 10:48 ` zhiyong.tao @ 2021-08-16 15:37 ` Chen-Yu Tsai 2021-08-16 23:00 ` Linus Walleij 0 siblings, 1 reply; 19+ messages in thread From: Chen-Yu Tsai @ 2021-08-16 15:37 UTC (permalink / raw) To: zhiyong.tao Cc: Linus Walleij, Rob Herring, Mark Rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, Light Hsieh, Biao Huang, Hongzhou Yang, Sean Wang, Seiya Wang, Devicetree List, LKML, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, moderated list:ARM/Mediatek SoC support, open list:GPIO SUBSYSTEM On Mon, Aug 16, 2021 at 6:48 PM zhiyong.tao <zhiyong.tao@mediatek.com> wrote: > > On Mon, 2021-08-16 at 14:10 +0800, Chen-Yu Tsai wrote: > > On Thu, Aug 5, 2021 at 7:02 AM Linus Walleij < > > linus.walleij@linaro.org> wrote: > > > > > > On Thu, Jul 29, 2021 at 11:43 AM Chen-Yu Tsai <wenst@chromium.org> > > > wrote: > > > > On Thu, Jul 29, 2021 at 4:23 PM zhiyong.tao < > > > > zhiyong.tao@mediatek.com> wrote: > > > > > The rsel actual bias resistance of each setting is different in > > > > > different IC. we think that the define "MTK_PULL_SET_RSEL_000" > > > > > is more > > > > > common for all different IC. > > > > > > > > I see. I personally prefer having things clearly described. I can > > > > understand this might be an extra burden to support different > > > > chips > > > > with different parameters, though this should be fairly > > > > straightforward > > > > with lookup tables tied to the compatible strings. > > > > > > > > Let's see if Rob and Linus have anything to add. > > > > > > Not much. We have "soft pushed" for this to be described as generic > > > as possible, using SI units (ohms). But we also allow vendor- > > > specific > > > numbers in this attribute. Especially when reverse engineering SoCs > > > that the contributor don't really have specs on (example M1 Mac). > > > > > > The intent with the SI units is especially for people like you > > > folks working > > > with Chromium to be able to use different SoCs and not feel lost > > > to a forest of different ways of doing things and associated > > > mistakes because vendors have hopelessly idiomatic pin configs. > > > > I'll take that as "use SI units whenever possible and reasonable". > > ==> so It doesn't need to change the define, is it right? > we will keep the common define. Actually I think it would be possible and reasonable to use SI units in this case, since you are the vendor and have the resistor values to implement the support. Having different sets of values for different chips is nothing out of the ordinary. We already have to account for different number of pins and different pin functions. That is what compatible strings are for. Now if you have _many_ different sets of values within the same chip, that could make things more difficult. However the clearness of having the real values visible in the device tree would likely benefit both software and hardware engineers outside of Mediatek. That is what we should be aiming for. Regards ChenYu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-08-16 15:37 ` Chen-Yu Tsai @ 2021-08-16 23:00 ` Linus Walleij 2021-08-17 2:21 ` zhiyong.tao 0 siblings, 1 reply; 19+ messages in thread From: Linus Walleij @ 2021-08-16 23:00 UTC (permalink / raw) To: Chen-Yu Tsai Cc: zhiyong.tao, Rob Herring, Mark Rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, Light Hsieh, Biao Huang, Hongzhou Yang, Sean Wang, Seiya Wang, Devicetree List, LKML, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, moderated list:ARM/Mediatek SoC support, open list:GPIO SUBSYSTEM On Mon, Aug 16, 2021 at 5:38 PM Chen-Yu Tsai <wenst@chromium.org> wrote: > On Mon, Aug 16, 2021 at 6:48 PM zhiyong.tao <zhiyong.tao@mediatek.com> wrote: > > > I'll take that as "use SI units whenever possible and reasonable". > > > > ==> so It doesn't need to change the define, is it right? > > we will keep the common define. > > Actually I think it would be possible and reasonable to use SI units > in this case, since you are the vendor and have the resistor values > to implement the support. Having different sets of values for different > chips is nothing out of the ordinary. We already have to account for > different number of pins and different pin functions. That is what > compatible strings are for. I fully agree with Chen-Yu's analysis here. Zhiyong can you make an attempt to use SI units (Ohms) and see what it will look like? I think it will look better for users and it will be less risk to make mistakes. Yours, Linus Walleij _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-08-16 23:00 ` Linus Walleij @ 2021-08-17 2:21 ` zhiyong.tao 2021-08-17 5:44 ` Chen-Yu Tsai 0 siblings, 1 reply; 19+ messages in thread From: zhiyong.tao @ 2021-08-17 2:21 UTC (permalink / raw) To: Linus Walleij, Chen-Yu Tsai Cc: zhiyong.tao, Rob Herring, Mark Rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, Light Hsieh, Biao Huang, Hongzhou Yang, Sean Wang, Seiya Wang, Devicetree List, LKML, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, moderated list:ARM/Mediatek SoC support, open list:GPIO SUBSYSTEM On Tue, 2021-08-17 at 01:00 +0200, Linus Walleij wrote: > On Mon, Aug 16, 2021 at 5:38 PM Chen-Yu Tsai <wenst@chromium.org> > wrote: > > On Mon, Aug 16, 2021 at 6:48 PM zhiyong.tao < > > zhiyong.tao@mediatek.com> wrote: > > > > I'll take that as "use SI units whenever possible and > > > > reasonable". > > > > > > ==> so It doesn't need to change the define, is it right? > > > we will keep the common define. > > > > Actually I think it would be possible and reasonable to use SI > > units > > in this case, since you are the vendor and have the resistor values > > to implement the support. Having different sets of values for > > different > > chips is nothing out of the ordinary. We already have to account > > for > > different number of pins and different pin functions. That is what > > compatible strings are for. > > I fully agree with Chen-Yu's analysis here. > > Zhiyong can you make an attempt to use SI units (Ohms) and see > what it will look like? I think it will look better for users and it > will > be less risk to make mistakes. > > Yours, > Linus Walleij Hi Linus & chen-yu, The rsel actual bias resistance of each setting is different in different IC. For example, in mt8195, the rsel actual bias resistance setting like as: MTK_PULL_SET_RSEL_000:75K in PU, 75k in PD; MTK_PULL_SET _RSEL_001:10k in PU, 5k in PD; MTK_PULL_SET_RSEL_010:5k in PU, 75k in PD; MTK_PULL_SET_RSEL_011:4k in PU, 5K in PD; MTK_PULL_SET_RSEL_100:3k in PU, 75k in PD; MTK_PULL_SET_RSEL_101:2k in PU, 5K in PD; MTK_PULL_SET_RSE L_110:1.5k in PU, 75k in PD; MTK_PULL_SET_RSEL_111:1k in PU, 5k in PD. but in mt8192, the rsel actual bias resistance setting like as: MTK_PULL_SET_RSEL_000:75K in PU, 75k in PD; MTK_PULL_SET_RSEL_001:3k in PU, 5k in PD; MTK_PULL_SET_RSEL_010:10k in PU, 75k in PD; MTK_PULL_SET_RSEL_011:1k in PU, 5K in PD; Can you help me to provide a suggestion common define for the all different IC? It seems that we should add a new define, if we upstream a new IC pinctrl driver in the future. Thanks. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-08-17 2:21 ` zhiyong.tao @ 2021-08-17 5:44 ` Chen-Yu Tsai 2021-08-17 7:51 ` zhiyong.tao 0 siblings, 1 reply; 19+ messages in thread From: Chen-Yu Tsai @ 2021-08-17 5:44 UTC (permalink / raw) To: zhiyong.tao Cc: Linus Walleij, Rob Herring, Mark Rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, Light Hsieh, Biao Huang, Hongzhou Yang, Sean Wang, Seiya Wang, Devicetree List, LKML, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, moderated list:ARM/Mediatek SoC support, open list:GPIO SUBSYSTEM On Tue, Aug 17, 2021 at 10:21 AM zhiyong.tao <zhiyong.tao@mediatek.com> wrote: > > On Tue, 2021-08-17 at 01:00 +0200, Linus Walleij wrote: > > On Mon, Aug 16, 2021 at 5:38 PM Chen-Yu Tsai <wenst@chromium.org> > > wrote: > > > On Mon, Aug 16, 2021 at 6:48 PM zhiyong.tao < > > > zhiyong.tao@mediatek.com> wrote: > > > > > I'll take that as "use SI units whenever possible and > > > > > reasonable". > > > > > > > > ==> so It doesn't need to change the define, is it right? > > > > we will keep the common define. > > > > > > Actually I think it would be possible and reasonable to use SI > > > units > > > in this case, since you are the vendor and have the resistor values > > > to implement the support. Having different sets of values for > > > different > > > chips is nothing out of the ordinary. We already have to account > > > for > > > different number of pins and different pin functions. That is what > > > compatible strings are for. > > > > I fully agree with Chen-Yu's analysis here. > > > > Zhiyong can you make an attempt to use SI units (Ohms) and see > > what it will look like? I think it will look better for users and it > > will > > be less risk to make mistakes. > > > > Yours, > > Linus Walleij > > Hi Linus & chen-yu, > > The rsel actual bias resistance of each setting is different in > different IC. For example, in mt8195, the rsel actual bias resistance > setting like as: > MTK_PULL_SET_RSEL_000:75K in PU, 75k in PD; > MTK_PULL_SET > _RSEL_001:10k in PU, 5k in PD; > MTK_PULL_SET_RSEL_010:5k in PU, 75k in > PD; > MTK_PULL_SET_RSEL_011:4k in PU, 5K in PD; > MTK_PULL_SET_RSEL_100:3k in > PU, 75k in PD; > MTK_PULL_SET_RSEL_101:2k in PU, 5K in PD; > MTK_PULL_SET_RSE > L_110:1.5k in PU, 75k in PD; > MTK_PULL_SET_RSEL_111:1k in PU, 5k in PD. > > but in mt8192, the rsel actual bias resistance setting like as: > MTK_PULL_SET_RSEL_000:75K in PU, 75k in PD; > MTK_PULL_SET_RSEL_001:3k in PU, 5k in PD; > MTK_PULL_SET_RSEL_010:10k in PU, 75k in PD; > MTK_PULL_SET_RSEL_011:1k in PU, 5K in PD; > > Can you help me to provide a suggestion common define for the all > different IC? > It seems that we should add a new define, if we upstream a new IC > pinctrl driver in the future. I assume you mean the macros used in the device tree? The point of using SI units is to get rid of the macros. Instead of: bias-pull-up = <MTK_PULL_SET_RSEL_000>; and bias-pull-down = <MTK_PULL_SET_RSEL_011>; We want: bias-pull-up = <75000>; and bias-pull-down = <5000>; And the pinctrl driver then converts the real values in the device tree into register values using some lookup table. The DT schema could then enumerate all the valid resistor values, and get proper validity checking. Now if you really wanted to keep some symbols for mapping hardware register values to resistor values, you could have #define MT8192_PULL_UP_RSEL_001 75000 #define MT8192_PULL_DOWN_RSEL_001 5000 or have them all named MTK_PULL_{UP,DOWN}_RSEL_NNN, but split into different header files, one per SoC. Personally I think having the macros is a bad idea if proper values are available. It just adds another layer of indirection, and another area where errors can creep in. Regards ChenYu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-08-17 5:44 ` Chen-Yu Tsai @ 2021-08-17 7:51 ` zhiyong.tao 2021-08-17 20:09 ` Linus Walleij 0 siblings, 1 reply; 19+ messages in thread From: zhiyong.tao @ 2021-08-17 7:51 UTC (permalink / raw) To: Chen-Yu Tsai Cc: zhiyong.tao, Linus Walleij, Rob Herring, Mark Rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, Light Hsieh, Biao Huang, Hongzhou Yang, Sean Wang, Seiya Wang, Devicetree List, LKML, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, moderated list:ARM/Mediatek SoC support, open list:GPIO SUBSYSTEM On Tue, 2021-08-17 at 13:44 +0800, Chen-Yu Tsai wrote: > On Tue, Aug 17, 2021 at 10:21 AM zhiyong.tao < > zhiyong.tao@mediatek.com> wrote: > > > > On Tue, 2021-08-17 at 01:00 +0200, Linus Walleij wrote: > > > On Mon, Aug 16, 2021 at 5:38 PM Chen-Yu Tsai <wenst@chromium.org> > > > wrote: > > > > On Mon, Aug 16, 2021 at 6:48 PM zhiyong.tao < > > > > zhiyong.tao@mediatek.com> wrote: > > > > > > I'll take that as "use SI units whenever possible and > > > > > > reasonable". > > > > > > > > > > ==> so It doesn't need to change the define, is it right? > > > > > we will keep the common define. > > > > > > > > Actually I think it would be possible and reasonable to use SI > > > > units > > > > in this case, since you are the vendor and have the resistor > > > > values > > > > to implement the support. Having different sets of values for > > > > different > > > > chips is nothing out of the ordinary. We already have to > > > > account > > > > for > > > > different number of pins and different pin functions. That is > > > > what > > > > compatible strings are for. > > > > > > I fully agree with Chen-Yu's analysis here. > > > > > > Zhiyong can you make an attempt to use SI units (Ohms) and see > > > what it will look like? I think it will look better for users and > > > it > > > will > > > be less risk to make mistakes. > > > > > > Yours, > > > Linus Walleij > > > > Hi Linus & chen-yu, > > > > The rsel actual bias resistance of each setting is different in > > different IC. For example, in mt8195, the rsel actual bias > > resistance > > setting like as: > > MTK_PULL_SET_RSEL_000:75K in PU, 75k in PD; > > MTK_PULL_SET > > _RSEL_001:10k in PU, 5k in PD; > > MTK_PULL_SET_RSEL_010:5k in PU, 75k in > > PD; > > MTK_PULL_SET_RSEL_011:4k in PU, 5K in PD; > > MTK_PULL_SET_RSEL_100:3k in > > PU, 75k in PD; > > MTK_PULL_SET_RSEL_101:2k in PU, 5K in PD; > > MTK_PULL_SET_RSE > > L_110:1.5k in PU, 75k in PD; > > MTK_PULL_SET_RSEL_111:1k in PU, 5k in PD. > > > > but in mt8192, the rsel actual bias resistance setting like as: > > MTK_PULL_SET_RSEL_000:75K in PU, 75k in PD; > > MTK_PULL_SET_RSEL_001:3k in PU, 5k in PD; > > MTK_PULL_SET_RSEL_010:10k in PU, 75k in PD; > > MTK_PULL_SET_RSEL_011:1k in PU, 5K in PD; > > > > Can you help me to provide a suggestion common define for the all > > different IC? > > It seems that we should add a new define, if we upstream a new IC > > pinctrl driver in the future. > > I assume you mean the macros used in the device tree? > > The point of using SI units is to get rid of the macros. Instead of: > > bias-pull-up = <MTK_PULL_SET_RSEL_000>; > > and > > bias-pull-down = <MTK_PULL_SET_RSEL_011>; > > We want: > > bias-pull-up = <75000>; > > and > > bias-pull-down = <5000>; > > And the pinctrl driver then converts the real values in the device > tree > into register values using some lookup table. > > The DT schema could then enumerate all the valid resistor values, > and get proper validity checking. > > Now if you really wanted to keep some symbols for mapping hardware > register values to resistor values, you could have > > #define MT8192_PULL_UP_RSEL_001 75000 > #define MT8192_PULL_DOWN_RSEL_001 5000 > > or have them all named MTK_PULL_{UP,DOWN}_RSEL_NNN, but split into > different header files, one per SoC. > > Personally I think having the macros is a bad idea if proper values > are available. It just adds another layer of indirection, and another > area where errors can creep in. > > > Regards > ChenYu Hi Chenyu, In one chip, If GPIO is different, the MTXXXX_PULL_UP_RSEL_001 may means different actual bias resistance setting. For example, KPROW IO Paramters Descriptions Min Typ Max UNIT Rpd Input pull-down resistance 40 75 190 Kohm Rpu Input pull-up resistance 40 75 190 Kohm Rpd Input pull-down resistance 0.8 1.6 2 Kohm Rpu Input pull-up resistance 0.8 1.6 2 Kohm KPCOL IO Paramters Descriptions Min Typ Max UNIT Rpd Input pull-down resistance 40 75 190 Kohm Rpu Input pull-up resistance 40 75 190 Kohm Rpd Input pull-down resistance 200 260 400 Kohm Rpu Input pull-up resistance 200 260 400 Kohm MSDC1 IO Paramters Descriptions Min Typ Max UNIT Rpd Input pull-down resistance 5 7.5 10 Kohm Rpu Input pull-up resistance 5 7.5 10 Kohm Rpd Input pull-down resistance 10 50 100 Kohm Rpu Input pull-up resistance 10 50 100 Kohm we think that we can't define like this. Thanks. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-08-17 7:51 ` zhiyong.tao @ 2021-08-17 20:09 ` Linus Walleij 2021-08-18 6:25 ` Chen-Yu Tsai 0 siblings, 1 reply; 19+ messages in thread From: Linus Walleij @ 2021-08-17 20:09 UTC (permalink / raw) To: zhiyong.tao Cc: Chen-Yu Tsai, Rob Herring, Mark Rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, Light Hsieh, Biao Huang, Hongzhou Yang, Sean Wang, Seiya Wang, Devicetree List, LKML, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, moderated list:ARM/Mediatek SoC support, open list:GPIO SUBSYSTEM On Tue, Aug 17, 2021 at 9:51 AM zhiyong.tao <zhiyong.tao@mediatek.com> wrote: > In one chip, If GPIO is different, the MTXXXX_PULL_UP_RSEL_001 may > means different actual bias resistance setting. > > For example, > > KPROW IO > Paramters Descriptions Min Typ Max > UNIT > Rpd Input pull-down resistance 40 75 190 Kohm > Rpu Input pull-up resistance 40 75 190 Kohm > Rpd Input pull-down resistance 0.8 1.6 2 Kohm > Rpu Input pull-up resistance 0.8 1.6 2 Kohm This is exactly why we should try to use SI units in the device tree. I assume that the software can eventually configure which resistance it gets? The electronics people will say make sure it is pulled down by around 80 kOhm, they can put that on the device tree and your code can say, "hm 40 < 80 < 190 this is OK" and let the value pass. We do not define these exact semantics, it is up to the driver code to decide what to do with the Ohm value 80000 in this case, but it makes perfect sent for me to let it pass and fail if someone for example requests 20 kOhm, or at least print a helpful warning: dev_warn(dev, "the requested resistance %d is out of range, supported range %d to %d kOhm\n", val, low, high); This is what makes the SI units really helpful for people writing device trees: solve real integration tasks and make it easy to do the right thing. Yours, Linus Walleij _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-08-17 20:09 ` Linus Walleij @ 2021-08-18 6:25 ` Chen-Yu Tsai 2021-08-19 1:49 ` zhiyong.tao 0 siblings, 1 reply; 19+ messages in thread From: Chen-Yu Tsai @ 2021-08-18 6:25 UTC (permalink / raw) To: zhiyong.tao Cc: Linus Walleij, Rob Herring, Mark Rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, Light Hsieh, Biao Huang, Hongzhou Yang, Sean Wang, Seiya Wang, Devicetree List, LKML, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, moderated list:ARM/Mediatek SoC support, open list:GPIO SUBSYSTEM On Wed, Aug 18, 2021 at 4:09 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Tue, Aug 17, 2021 at 9:51 AM zhiyong.tao <zhiyong.tao@mediatek.com> wrote: > > > In one chip, If GPIO is different, the MTXXXX_PULL_UP_RSEL_001 may > > means different actual bias resistance setting. > > > > For example, > > > > KPROW IO > > Paramters Descriptions Min Typ Max > > UNIT > > Rpd Input pull-down resistance 40 75 190 Kohm > > Rpu Input pull-up resistance 40 75 190 Kohm > > Rpd Input pull-down resistance 0.8 1.6 2 Kohm > > Rpu Input pull-up resistance 0.8 1.6 2 Kohm > > This is exactly why we should try to use SI units in the device tree. > I assume that the software can eventually configure which resistance > it gets? > > The electronics people will say make sure it is pulled down by around > 80 kOhm, they can put that on the device tree and your code can > say, "hm 40 < 80 < 190 this is OK" and let the value pass. > > We do not define these exact semantics, it is up to the driver code > to decide what to do with the Ohm value 80000 in this case, but > it makes perfect sent for me to let it pass and fail if someone > for example requests 20 kOhm, or at least print a helpful warning: > > dev_warn(dev, "the requested resistance %d is out of range, supported > range %d to %d kOhm\n", > val, low, high); > > This is what makes the SI units really helpful for people writing device > trees: solve real integration tasks and make it easy to do the right thing. I think this makes a lot of sense. The driver could select the closest setting. And from what Zhiyong mentioned offline, the resistor values aren't exact as specified in the datasheet. I suppose this is expected with any electronics. So the hardware integration will say to pull up or down by some value, and the driver will do its best to fulfill that request. That precludes DT schema checking for the values used, but I think that is a good compromise. Zhiyong also mentioned that some of their downstream integrators might not be able to deal with actual values, and would prefer symbols tied to specific RSEL values. I think that would be doable together using some _magic_ values, but I would prefer not to if it were avoidable. Regards ChenYu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define 2021-08-18 6:25 ` Chen-Yu Tsai @ 2021-08-19 1:49 ` zhiyong.tao 0 siblings, 0 replies; 19+ messages in thread From: zhiyong.tao @ 2021-08-19 1:49 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Linus Walleij, Rob Herring, Mark Rutland, Matthias Brugger, Sean Wang, srv_heupstream, hui.liu, Eddie Huang, Light Hsieh, Biao Huang, Hongzhou Yang, Sean Wang, Seiya Wang, Devicetree List, LKML, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, moderated list:ARM/Mediatek SoC support, open list:GPIO SUBSYSTEM On Wed, 2021-08-18 at 14:25 +0800, Chen-Yu Tsai wrote: > On Wed, Aug 18, 2021 at 4:09 AM Linus Walleij < > linus.walleij@linaro.org> wrote: > > > > On Tue, Aug 17, 2021 at 9:51 AM zhiyong.tao < > > zhiyong.tao@mediatek.com> wrote: > > > > > In one chip, If GPIO is different, the MTXXXX_PULL_UP_RSEL_001 > > > may > > > means different actual bias resistance setting. > > > > > > For example, > > > > > > KPROW IO > > > Paramters Descriptions Min Typ M > > > ax > > > UNIT > > > Rpd Input pull-down > > > resistance 40 75 190 Kohm > > > Rpu Input pull-up > > > resistance 40 75 190 Kohm > > > Rpd Input pull-down > > > resistance 0.8 1.6 2 Kohm > > > Rpu Input pull-up > > > resistance 0.8 1.6 2 Kohm > > > > This is exactly why we should try to use SI units in the device > > tree. > > I assume that the software can eventually configure which > > resistance > > it gets? > > > > The electronics people will say make sure it is pulled down by > > around > > 80 kOhm, they can put that on the device tree and your code can > > say, "hm 40 < 80 < 190 this is OK" and let the value pass. > > > > We do not define these exact semantics, it is up to the driver code > > to decide what to do with the Ohm value 80000 in this case, but > > it makes perfect sent for me to let it pass and fail if someone > > for example requests 20 kOhm, or at least print a helpful warning: > > > > dev_warn(dev, "the requested resistance %d is out of range, > > supported > > range %d to %d kOhm\n", > > val, low, high); > > > > This is what makes the SI units really helpful for people writing > > device > > trees: solve real integration tasks and make it easy to do the > > right thing. > > I think this makes a lot of sense. The driver could select the > closest > setting. And from what Zhiyong mentioned offline, the resistor values > aren't exact as specified in the datasheet. I suppose this is > expected > with any electronics. So the hardware integration will say to pull up > or down by some value, and the driver will do its best to fulfill > that > request. That precludes DT schema checking for the values used, but I > think that is a good compromise. > > Zhiyong also mentioned that some of their downstream integrators > might > not be able to deal with actual values, and would prefer symbols tied > to specific RSEL values. I think that would be doable together using > some _magic_ values, but I would prefer not to if it were avoidable. > > > Regards > ChenYu Hi chenyu & Linus, Thanks for your suggestion. we will try to update a new version to use SI units in the device tree in the rsel feature patch. Thanks _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v10 2/2] pinctrl: mediatek: add rsel setting on MT8195 2021-07-10 8:17 [PATCH v10 0/2] Mediatek pinctrl patch on mt8195 Zhiyong Tao 2021-07-10 8:17 ` [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define Zhiyong Tao @ 2021-07-10 8:17 ` Zhiyong Tao 1 sibling, 0 replies; 19+ messages in thread From: Zhiyong Tao @ 2021-07-10 8:17 UTC (permalink / raw) To: robh+dt, linus.walleij, mark.rutland, matthias.bgg, sean.wang Cc: srv_heupstream, zhiyong.tao, hui.liu, eddie.huang, light.hsieh, biao.huang, hongzhou.yang, sean.wang, seiya.wang, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, linux-gpio This patch provides rsel setting on MT8195 Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> --- drivers/pinctrl/mediatek/pinctrl-mt8195.c | 96 +++++++++++++ .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 134 +++++++++++++++--- .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 10 +- drivers/pinctrl/mediatek/pinctrl-paris.c | 24 +++- drivers/pinctrl/mediatek/pinctrl-paris.h | 2 +- 5 files changed, 239 insertions(+), 27 deletions(-) diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8195.c b/drivers/pinctrl/mediatek/pinctrl-mt8195.c index a7500e18bb1d..ccfdbac94544 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mt8195.c +++ b/drivers/pinctrl/mediatek/pinctrl-mt8195.c @@ -779,6 +779,100 @@ static const struct mtk_pin_field_calc mt8195_pin_drv_adv_range[] = { PIN_FIELD_BASE(45, 45, 1, 0x040, 0x10, 9, 3), }; +static const struct mtk_pin_field_calc mt8195_pin_rsel_range[] = { + PIN_FIELD_BASE(8, 8, 4, 0x0c0, 0x10, 15, 3), + PIN_FIELD_BASE(9, 9, 4, 0x0c0, 0x10, 0, 3), + PIN_FIELD_BASE(10, 10, 4, 0x0c0, 0x10, 18, 3), + PIN_FIELD_BASE(11, 11, 4, 0x0c0, 0x10, 3, 3), + PIN_FIELD_BASE(12, 12, 4, 0x0c0, 0x10, 21, 3), + PIN_FIELD_BASE(13, 13, 4, 0x0c0, 0x10, 6, 3), + PIN_FIELD_BASE(14, 14, 4, 0x0c0, 0x10, 24, 3), + PIN_FIELD_BASE(15, 15, 4, 0x0c0, 0x10, 9, 3), + PIN_FIELD_BASE(16, 16, 4, 0x0c0, 0x10, 27, 3), + PIN_FIELD_BASE(17, 17, 4, 0x0c0, 0x10, 12, 3), + PIN_FIELD_BASE(29, 29, 2, 0x080, 0x10, 0, 3), + PIN_FIELD_BASE(30, 30, 2, 0x080, 0x10, 3, 3), + PIN_FIELD_BASE(34, 34, 1, 0x0e0, 0x10, 0, 3), + PIN_FIELD_BASE(35, 35, 1, 0x0e0, 0x10, 3, 3), + PIN_FIELD_BASE(44, 44, 1, 0x0e0, 0x10, 6, 3), + PIN_FIELD_BASE(45, 45, 1, 0x0e0, 0x10, 9, 3), +}; + +static const unsigned int mt8195_pull_type[] = { + MTK_PULL_PUPD_R1R0_TYPE /* 0 */, MTK_PULL_PUPD_R1R0_TYPE /* 1 */, + MTK_PULL_PUPD_R1R0_TYPE /* 2 */, MTK_PULL_PUPD_R1R0_TYPE /* 3 */, + MTK_PULL_PUPD_R1R0_TYPE /* 4 */, MTK_PULL_PUPD_R1R0_TYPE /* 5 */, + MTK_PULL_PU_PD_TYPE /* 6 */, MTK_PULL_PU_PD_TYPE /* 7 */, + MTK_PULL_PU_PD_RSEL_TYPE /* 8 */, MTK_PULL_PU_PD_RSEL_TYPE /* 9 */, + MTK_PULL_PU_PD_RSEL_TYPE /* 10 */, MTK_PULL_PU_PD_RSEL_TYPE /* 11 */, + MTK_PULL_PU_PD_RSEL_TYPE /* 12 */, MTK_PULL_PU_PD_RSEL_TYPE /* 13 */, + MTK_PULL_PU_PD_RSEL_TYPE /* 14 */, MTK_PULL_PU_PD_RSEL_TYPE /* 15 */, + MTK_PULL_PU_PD_RSEL_TYPE /* 16 */, MTK_PULL_PU_PD_RSEL_TYPE /* 17 */, + MTK_PULL_PU_PD_TYPE /* 18 */, MTK_PULL_PU_PD_TYPE /* 19 */, + MTK_PULL_PU_PD_TYPE /* 20 */, MTK_PULL_PU_PD_TYPE /* 21 */, + MTK_PULL_PU_PD_TYPE /* 22 */, MTK_PULL_PU_PD_TYPE /* 23 */, + MTK_PULL_PU_PD_TYPE /* 24 */, MTK_PULL_PU_PD_TYPE /* 25 */, + MTK_PULL_PU_PD_TYPE /* 26 */, MTK_PULL_PU_PD_TYPE /* 27 */, + MTK_PULL_PU_PD_TYPE /* 28 */, MTK_PULL_PU_PD_RSEL_TYPE /* 29 */, + MTK_PULL_PU_PD_RSEL_TYPE /* 30 */, MTK_PULL_PU_PD_TYPE /* 31 */, + MTK_PULL_PU_PD_TYPE /* 32 */, MTK_PULL_PU_PD_TYPE /* 33 */, + MTK_PULL_PU_PD_RSEL_TYPE /* 34 */, MTK_PULL_PU_PD_RSEL_TYPE /* 35 */, + MTK_PULL_PU_PD_TYPE /* 36 */, MTK_PULL_PU_PD_TYPE /* 37 */, + MTK_PULL_PU_PD_TYPE /* 38 */, MTK_PULL_PU_PD_TYPE /* 39 */, + MTK_PULL_PU_PD_TYPE /* 40 */, MTK_PULL_PU_PD_TYPE /* 41 */, + MTK_PULL_PU_PD_TYPE /* 42 */, MTK_PULL_PU_PD_TYPE /* 43 */, + MTK_PULL_PU_PD_RSEL_TYPE /* 44 */, MTK_PULL_PU_PD_RSEL_TYPE /* 45 */, + MTK_PULL_PU_PD_TYPE /* 46 */, MTK_PULL_PU_PD_TYPE /* 47 */, + MTK_PULL_PU_PD_TYPE /* 48 */, MTK_PULL_PU_PD_TYPE /* 49 */, + MTK_PULL_PU_PD_TYPE /* 50 */, MTK_PULL_PU_PD_TYPE /* 51 */, + MTK_PULL_PU_PD_TYPE /* 52 */, MTK_PULL_PU_PD_TYPE /* 53 */, + MTK_PULL_PU_PD_TYPE /* 54 */, MTK_PULL_PU_PD_TYPE /* 55 */, + MTK_PULL_PU_PD_TYPE /* 56 */, MTK_PULL_PU_PD_TYPE /* 57 */, + MTK_PULL_PU_PD_TYPE /* 58 */, MTK_PULL_PU_PD_TYPE /* 59 */, + MTK_PULL_PU_PD_TYPE /* 60 */, MTK_PULL_PU_PD_TYPE /* 61 */, + MTK_PULL_PU_PD_TYPE /* 62 */, MTK_PULL_PU_PD_TYPE /* 63 */, + MTK_PULL_PU_PD_TYPE /* 64 */, MTK_PULL_PU_PD_TYPE /* 65 */, + MTK_PULL_PU_PD_TYPE /* 66 */, MTK_PULL_PU_PD_TYPE /* 67 */, + MTK_PULL_PU_PD_TYPE /* 68 */, MTK_PULL_PU_PD_TYPE /* 69 */, + MTK_PULL_PU_PD_TYPE /* 70 */, MTK_PULL_PU_PD_TYPE /* 71 */, + MTK_PULL_PU_PD_TYPE /* 72 */, MTK_PULL_PU_PD_TYPE /* 73 */, + MTK_PULL_PU_PD_TYPE /* 74 */, MTK_PULL_PU_PD_TYPE /* 75 */, + MTK_PULL_PU_PD_TYPE /* 76 */, MTK_PULL_PUPD_R1R0_TYPE /* 77 */, + MTK_PULL_PUPD_R1R0_TYPE /* 78 */, MTK_PULL_PUPD_R1R0_TYPE /* 79 */, + MTK_PULL_PUPD_R1R0_TYPE /* 80 */, MTK_PULL_PUPD_R1R0_TYPE /* 81 */, + MTK_PULL_PUPD_R1R0_TYPE /* 82 */, MTK_PULL_PUPD_R1R0_TYPE /* 83 */, + MTK_PULL_PUPD_R1R0_TYPE /* 84 */, MTK_PULL_PUPD_R1R0_TYPE /* 85 */, + MTK_PULL_PUPD_R1R0_TYPE /* 86 */, MTK_PULL_PUPD_R1R0_TYPE /* 87 */, + MTK_PULL_PUPD_R1R0_TYPE /* 88 */, MTK_PULL_PUPD_R1R0_TYPE /* 89 */, + MTK_PULL_PUPD_R1R0_TYPE /* 90 */, MTK_PULL_PUPD_R1R0_TYPE /* 91 */, + MTK_PULL_PUPD_R1R0_TYPE /* 92 */, MTK_PULL_PUPD_R1R0_TYPE /* 93 */, + MTK_PULL_PUPD_R1R0_TYPE /* 94 */, MTK_PULL_PUPD_R1R0_TYPE /* 95 */, + MTK_PULL_PUPD_R1R0_TYPE /* 96 */, MTK_PULL_PU_PD_TYPE /* 97 */, + MTK_PULL_PU_PD_TYPE /* 98 */, MTK_PULL_PU_PD_TYPE /* 99 */, + MTK_PULL_PU_PD_TYPE /* 100 */, MTK_PULL_PU_PD_TYPE /* 101 */, + MTK_PULL_PU_PD_TYPE /* 102 */, MTK_PULL_PU_PD_TYPE /* 103 */, + MTK_PULL_PUPD_R1R0_TYPE /* 104 */, MTK_PULL_PUPD_R1R0_TYPE /* 105 */, + MTK_PULL_PUPD_R1R0_TYPE /* 106 */, MTK_PULL_PUPD_R1R0_TYPE /* 107 */, + MTK_PULL_PU_PD_TYPE /* 108 */, MTK_PULL_PU_PD_TYPE /* 109 */, + MTK_PULL_PUPD_R1R0_TYPE /* 110 */, MTK_PULL_PUPD_R1R0_TYPE /* 111 */, + MTK_PULL_PUPD_R1R0_TYPE /* 112 */, MTK_PULL_PUPD_R1R0_TYPE /* 113 */, + MTK_PULL_PUPD_R1R0_TYPE /* 114 */, MTK_PULL_PUPD_R1R0_TYPE /* 115 */, + MTK_PULL_PUPD_R1R0_TYPE /* 116 */, MTK_PULL_PUPD_R1R0_TYPE /* 117 */, + MTK_PULL_PUPD_R1R0_TYPE /* 118 */, MTK_PULL_PUPD_R1R0_TYPE /* 119 */, + MTK_PULL_PUPD_R1R0_TYPE /* 120 */, MTK_PULL_PUPD_R1R0_TYPE /* 121 */, + MTK_PULL_PUPD_R1R0_TYPE /* 122 */, MTK_PULL_PUPD_R1R0_TYPE /* 123 */, + MTK_PULL_PUPD_R1R0_TYPE /* 124 */, MTK_PULL_PUPD_R1R0_TYPE /* 125 */, + MTK_PULL_PUPD_R1R0_TYPE /* 126 */, MTK_PULL_PUPD_R1R0_TYPE /* 127 */, + MTK_PULL_PU_PD_TYPE /* 128 */, MTK_PULL_PU_PD_TYPE /* 129 */, + MTK_PULL_PU_PD_TYPE /* 130 */, MTK_PULL_PU_PD_TYPE /* 131 */, + MTK_PULL_PU_PD_TYPE /* 132 */, MTK_PULL_PU_PD_TYPE /* 133 */, + MTK_PULL_PU_PD_TYPE /* 134 */, MTK_PULL_PU_PD_TYPE /* 135 */, + MTK_PULL_PU_PD_TYPE /* 136 */, MTK_PULL_PU_PD_TYPE /* 137 */, + MTK_PULL_PU_PD_TYPE /* 138 */, MTK_PULL_PU_PD_TYPE /* 139 */, + MTK_PULL_PU_PD_TYPE /* 140 */, MTK_PULL_PU_PD_TYPE /* 141 */, + MTK_PULL_PU_PD_TYPE /* 142 */, MTK_PULL_PU_PD_TYPE /* 143 */, +}; + static const struct mtk_pin_reg_calc mt8195_reg_cals[PINCTRL_PIN_REG_MAX] = { [PINCTRL_PIN_REG_MODE] = MTK_RANGE(mt8195_pin_mode_range), [PINCTRL_PIN_REG_DIR] = MTK_RANGE(mt8195_pin_dir_range), @@ -793,6 +887,7 @@ static const struct mtk_pin_reg_calc mt8195_reg_cals[PINCTRL_PIN_REG_MAX] = { [PINCTRL_PIN_REG_R0] = MTK_RANGE(mt8195_pin_r0_range), [PINCTRL_PIN_REG_R1] = MTK_RANGE(mt8195_pin_r1_range), [PINCTRL_PIN_REG_DRV_ADV] = MTK_RANGE(mt8195_pin_drv_adv_range), + [PINCTRL_PIN_REG_RSEL] = MTK_RANGE(mt8195_pin_rsel_range), }; static const char * const mt8195_pinctrl_register_base_names[] = { @@ -817,6 +912,7 @@ static const struct mtk_pin_soc mt8195_data = { .gpio_m = 0, .base_names = mt8195_pinctrl_register_base_names, .nbase_names = ARRAY_SIZE(mt8195_pinctrl_register_base_names), + .pull_type = mt8195_pull_type, .bias_set_combo = mtk_pinconf_bias_set_combo, .bias_get_combo = mtk_pinconf_bias_get_combo, .drive_set = mtk_pinconf_drive_set_rev1, diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c index 5b3b048725cc..d51c1b79300c 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c @@ -641,6 +641,9 @@ static int mtk_pinconf_bias_set_pupd_r1_r0(struct mtk_pinctrl *hw, } else if (arg == MTK_PUPD_SET_R1R0_11) { r0 = 1; r1 = 1; + } else if (arg == MTK_ENABLE) { + r0 = 1; + r1 = 0; } else { err = -EINVAL; goto out; @@ -661,6 +664,28 @@ static int mtk_pinconf_bias_set_pupd_r1_r0(struct mtk_pinctrl *hw, return err; } +static int mtk_pinconf_bias_set_rsel(struct mtk_pinctrl *hw, + const struct mtk_pin_desc *desc, + u32 pullup, u32 arg) +{ + int err; + + if (arg < MTK_PULL_SET_RSEL_000 || arg > MTK_PULL_SET_RSEL_111) { + err = -EINVAL; + goto out; + } + + arg -= MTK_PULL_SET_RSEL_000; + err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_RSEL, arg); + if (err) + goto out; + + err = mtk_pinconf_bias_set_pu_pd(hw, desc, pullup, MTK_ENABLE); + +out: + return err; +} + static int mtk_pinconf_bias_get_pu_pd(struct mtk_pinctrl *hw, const struct mtk_pin_desc *desc, u32 *pullup, u32 *enable) @@ -742,44 +767,117 @@ static int mtk_pinconf_bias_get_pupd_r1_r0(struct mtk_pinctrl *hw, return err; } -int mtk_pinconf_bias_set_combo(struct mtk_pinctrl *hw, - const struct mtk_pin_desc *desc, - u32 pullup, u32 arg) +static int mtk_pinconf_bias_get_rsel(struct mtk_pinctrl *hw, + const struct mtk_pin_desc *desc, + u32 *pullup, u32 *enable) { - int err; + int pu, pd, rsel, err; - err = mtk_pinconf_bias_set_pu_pd(hw, desc, pullup, arg); - if (!err) + err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_RSEL, &rsel); + if (err) goto out; - err = mtk_pinconf_bias_set_pullsel_pullen(hw, desc, pullup, arg); - if (!err) + err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_PU, &pu); + if (err) goto out; - err = mtk_pinconf_bias_set_pupd_r1_r0(hw, desc, pullup, arg); + err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_PD, &pd); + + if (pu == 0 && pd == 0) { + *pullup = 0; + *enable = MTK_DISABLE; + } else if (pu == 1 && pd == 0) { + *pullup = 1; + *enable = rsel + MTK_PULL_SET_RSEL_000; + } else if (pu == 0 && pd == 1) { + *pullup = 0; + *enable = rsel + MTK_PULL_SET_RSEL_000; + } else { + err = -EINVAL; + goto out; + } out: return err; } + +int mtk_pinconf_bias_set_combo(struct mtk_pinctrl *hw, + const struct mtk_pin_desc *desc, + u32 pullup, u32 arg) +{ + int err = -EOPNOTSUPP; + bool try_all_type; + + try_all_type = hw->soc->pull_type ? false : true; + + if (try_all_type || + (hw->soc->pull_type[desc->number] & MTK_PULL_RSEL_TYPE)) { + err = mtk_pinconf_bias_set_rsel(hw, desc, pullup, arg); + if (!err) + return err; + } + + if (try_all_type || + (hw->soc->pull_type[desc->number] & MTK_PULL_PU_PD_TYPE)) { + err = mtk_pinconf_bias_set_pu_pd(hw, desc, pullup, arg); + if (!err) + return err; + } + + if (try_all_type || + (hw->soc->pull_type[desc->number] & MTK_PULL_PULLSEL_TYPE)) { + err = mtk_pinconf_bias_set_pullsel_pullen(hw, desc, + pullup, arg); + if (!err) + return err; + } + + if (try_all_type || + (hw->soc->pull_type[desc->number] & MTK_PULL_PUPD_R1R0_TYPE)) { + err = mtk_pinconf_bias_set_pupd_r1_r0(hw, desc, pullup, arg); + if (err) + dev_err(hw->dev, "Invalid pull argument\n"); + } + + return err; +} EXPORT_SYMBOL_GPL(mtk_pinconf_bias_set_combo); int mtk_pinconf_bias_get_combo(struct mtk_pinctrl *hw, const struct mtk_pin_desc *desc, u32 *pullup, u32 *enable) { - int err; + int err = -EOPNOTSUPP; + bool try_all_type; - err = mtk_pinconf_bias_get_pu_pd(hw, desc, pullup, enable); - if (!err) - goto out; + try_all_type = hw->soc->pull_type ? false : true; - err = mtk_pinconf_bias_get_pullsel_pullen(hw, desc, pullup, enable); - if (!err) - goto out; + if (try_all_type || + (hw->soc->pull_type[desc->number] & MTK_PULL_RSEL_TYPE)) { + err = mtk_pinconf_bias_get_rsel(hw, desc, pullup, enable); + if (!err) + return err; + } - err = mtk_pinconf_bias_get_pupd_r1_r0(hw, desc, pullup, enable); + if (try_all_type || + (hw->soc->pull_type[desc->number] & MTK_PULL_PU_PD_TYPE)) { + err = mtk_pinconf_bias_get_pu_pd(hw, desc, pullup, enable); + if (!err) + return err; + } + + if (try_all_type || + (hw->soc->pull_type[desc->number] & MTK_PULL_PULLSEL_TYPE)) { + err = mtk_pinconf_bias_get_pullsel_pullen(hw, desc, + pullup, enable); + if (!err) + return err; + } + + if (try_all_type || + (hw->soc->pull_type[desc->number] & MTK_PULL_PUPD_R1R0_TYPE)) + err = mtk_pinconf_bias_get_pupd_r1_r0(hw, desc, pullup, enable); -out: return err; } EXPORT_SYMBOL_GPL(mtk_pinconf_bias_get_combo); diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h index a6f1bdb2083b..491ae4b3a8cc 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h @@ -17,6 +17,13 @@ #define MTK_ENABLE 1 #define MTK_PULLDOWN 0 #define MTK_PULLUP 1 +#define MTK_PULL_PU_PD_TYPE BIT(0) +#define MTK_PULL_PULLSEL_TYPE BIT(1) +#define MTK_PULL_PUPD_R1R0_TYPE BIT(2) +#define MTK_PULL_RSEL_TYPE BIT(3) +#define MTK_PULL_PU_PD_RSEL_TYPE (MTK_PULL_PU_PD_TYPE \ + | MTK_PULL_RSEL_TYPE) + #define EINT_NA U16_MAX #define NO_EINT_SUPPORT EINT_NA @@ -67,6 +74,7 @@ enum { PINCTRL_PIN_REG_DRV_E0, PINCTRL_PIN_REG_DRV_E1, PINCTRL_PIN_REG_DRV_ADV, + PINCTRL_PIN_REG_RSEL, PINCTRL_PIN_REG_MAX, }; @@ -206,6 +214,7 @@ struct mtk_pin_soc { bool ies_present; const char * const *base_names; unsigned int nbase_names; + const unsigned int *pull_type; /* Specific pinconfig operations */ int (*bias_disable_set)(struct mtk_pinctrl *hw, @@ -237,7 +246,6 @@ struct mtk_pin_soc { const struct mtk_pin_desc *desc, u32 arg); int (*adv_drive_get)(struct mtk_pinctrl *hw, const struct mtk_pin_desc *desc, u32 *val); - /* Specific driver data */ void *driver_data; }; diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c index 85db2e4377f0..8990cfe47d72 100644 --- a/drivers/pinctrl/mediatek/pinctrl-paris.c +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c @@ -577,9 +577,9 @@ static int mtk_hw_get_value_wrap(struct mtk_pinctrl *hw, unsigned int gpio, int mtk_hw_get_value_wrap(hw, gpio, PINCTRL_PIN_REG_DRV) ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, - unsigned int gpio, char *buf, unsigned int bufLen) + unsigned int gpio, char *buf, unsigned int buf_len) { - int pinmux, pullup, pullen, len = 0, r1 = -1, r0 = -1; + int pinmux, pullup, pullen, len = 0, r1 = -1, r0 = -1, rsel = -1; const struct mtk_pin_desc *desc; if (gpio >= hw->soc->npins) @@ -591,6 +591,8 @@ ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, pinmux -= hw->soc->nfuncs; mtk_pinconf_bias_get_combo(hw, desc, &pullup, &pullen); + + /* Case for: R1R0 */ if (pullen == MTK_PUPD_SET_R1R0_00) { pullen = 0; r1 = 0; @@ -607,10 +609,16 @@ ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, pullen = 1; r1 = 1; r0 = 1; - } else if (pullen != MTK_DISABLE && pullen != MTK_ENABLE) { - pullen = 0; } - len += scnprintf(buf + len, bufLen - len, + + /* Case for: RSEL */ + if (pullen >= MTK_PULL_SET_RSEL_000 && + pullen <= MTK_PULL_SET_RSEL_111) { + rsel = pullen - MTK_PULL_SET_RSEL_000; + pullen = 1; + } + + len += scnprintf(buf + len, buf_len - len, "%03d: %1d%1d%1d%1d%02d%1d%1d%1d%1d", gpio, pinmux, @@ -624,10 +632,12 @@ ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, pullup); if (r1 != -1) { - len += scnprintf(buf + len, bufLen - len, " (%1d %1d)\n", + len += scnprintf(buf + len, buf_len - len, " (%1d %1d)\n", r1, r0); + } else if (rsel != -1) { + len += scnprintf(buf + len, buf_len - len, " (%1d)\n", rsel); } else { - len += scnprintf(buf + len, bufLen - len, "\n"); + len += scnprintf(buf + len, buf_len - len, "\n"); } return len; diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.h b/drivers/pinctrl/mediatek/pinctrl-paris.h index afb7650fd25b..681267c0e1a4 100644 --- a/drivers/pinctrl/mediatek/pinctrl-paris.h +++ b/drivers/pinctrl/mediatek/pinctrl-paris.h @@ -61,7 +61,7 @@ int mtk_paris_pinctrl_probe(struct platform_device *pdev, const struct mtk_pin_soc *soc); ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, - unsigned int gpio, char *buf, unsigned int bufLen); + unsigned int gpio, char *buf, unsigned int buf_len); extern const struct dev_pm_ops mtk_paris_pinctrl_pm_ops; -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-08-19 1:52 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-10 8:17 [PATCH v10 0/2] Mediatek pinctrl patch on mt8195 Zhiyong Tao 2021-07-10 8:17 ` [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define Zhiyong Tao 2021-07-13 7:17 ` Chen-Yu Tsai 2021-07-22 7:54 ` zhiyong tao 2021-07-26 8:02 ` Chen-Yu Tsai 2021-07-29 8:23 ` zhiyong.tao 2021-07-29 9:43 ` Chen-Yu Tsai 2021-08-04 23:01 ` Linus Walleij 2021-08-16 6:10 ` Chen-Yu Tsai 2021-08-16 10:48 ` zhiyong.tao 2021-08-16 15:37 ` Chen-Yu Tsai 2021-08-16 23:00 ` Linus Walleij 2021-08-17 2:21 ` zhiyong.tao 2021-08-17 5:44 ` Chen-Yu Tsai 2021-08-17 7:51 ` zhiyong.tao 2021-08-17 20:09 ` Linus Walleij 2021-08-18 6:25 ` Chen-Yu Tsai 2021-08-19 1:49 ` zhiyong.tao 2021-07-10 8:17 ` [PATCH v10 2/2] pinctrl: mediatek: add rsel setting on MT8195 Zhiyong Tao
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 NNTP newsgroup(s).