* [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators @ 2018-12-06 14:02 Maxime Ripard 2018-12-06 14:02 ` [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators Maxime Ripard ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Maxime Ripard @ 2018-12-06 14:02 UTC (permalink / raw) To: Chen-Yu Tsai, Maxime Ripard, Linus Walleij Cc: linux-gpio, Mylene Josserand, linux-arm-kernel, Thomas Petazzoni Hi, Here is a first attempt at getting the regulators properly accounted for the GPIO banks on the Allwinner SoCs. The main interogation I have currently is whether we should always try to get the regulator for the current branch, or if we should restrict it to the one available on the SoCs. Let me know what you think, Maxime Maxime Ripard (2): pinctrl: sunxi: Deal with per-bank regulators ARM: dts: sun7i: bananapi: Add GPIO banks regulators arch/arm/boot/dts/sun7i-a20-bananapi.dts | 5 ++- drivers/pinctrl/sunxi/pinctrl-sunxi.c | 63 +++++++++++++++++++++++++- drivers/pinctrl/sunxi/pinctrl-sunxi.h | 6 ++- 3 files changed, 74 insertions(+) base-commit: 651022382c7f8da46cb4872a545ee1da6d097d2a -- git-series 0.9.1 _______________________________________________ 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] 13+ messages in thread
* [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators 2018-12-06 14:02 [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators Maxime Ripard @ 2018-12-06 14:02 ` Maxime Ripard 2018-12-14 15:08 ` Linus Walleij ` (2 more replies) 2018-12-06 14:02 ` [PATCH 2/2] ARM: dts: sun7i: bananapi: Add GPIO banks regulators Maxime Ripard 2018-12-06 15:28 ` [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators Chen-Yu Tsai 2 siblings, 3 replies; 13+ messages in thread From: Maxime Ripard @ 2018-12-06 14:02 UTC (permalink / raw) To: Chen-Yu Tsai, Maxime Ripard, Linus Walleij Cc: linux-gpio, Mylene Josserand, linux-arm-kernel, Thomas Petazzoni The Allwinner SoCs have on most of their GPIO banks a regulator input. This issue was mainly ignored so far because either the regulator was a static regulator that would be providing power anyway, or the bank was used for a feature unsupported so far (CSI). For the odd cases, enabling it in the bootloader was the preferred option. However, now that we are starting to support those features, and that we can't really rely on the bootloader for this, we need to model those regulators as such in the DT. This is slightly more complicated than what it looks like, since some regulators will be tied to the PMIC, and in order to have access to the PMIC bus, you need to mux its pins, which will need the pinctrl driver, that needs the regulator driver to be registered. And this is how you get a circular dependency. In practice however, the hardware cannot fall into this case since it would result in a completely unusable bus. In order to avoid that circular dependency, we can thus get and enable the regulators at pin_request time. We'll then need to account for the references of all the pins of a particular branch to know when to put the reference, but it works pretty nicely once implemented. Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> --- drivers/pinctrl/sunxi/pinctrl-sunxi.c | 63 ++++++++++++++++++++++++++++- drivers/pinctrl/sunxi/pinctrl-sunxi.h | 6 +++- 2 files changed, 69 insertions(+) diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c index 34e17376ef99..5d9184d18c16 100644 --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c @@ -26,6 +26,7 @@ #include <linux/pinctrl/pinctrl.h> #include <linux/pinctrl/pinconf-generic.h> #include <linux/pinctrl/pinmux.h> +#include <linux/regulator/consumer.h> #include <linux/platform_device.h> #include <linux/slab.h> @@ -693,12 +694,74 @@ sunxi_pmx_gpio_set_direction(struct pinctrl_dev *pctldev, return 0; } +static int sunxi_pmx_request(struct pinctrl_dev *pctldev, unsigned offset) +{ + struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev); + unsigned short bank = offset / PINS_PER_BANK; + struct sunxi_pinctrl_regulator *s_reg = &pctl->regulators[bank]; + struct regulator *reg; + int ret; + + reg = s_reg->regulator; + if (!reg) { + char supply[16]; + + snprintf(supply, sizeof(supply), "vcc-p%c", 'a' + bank); + reg = regulator_get(pctl->dev, supply); + if (IS_ERR(reg)) { + dev_err(pctl->dev, "Couldn't get bank P%c regulator\n", + 'A' + bank); + return PTR_ERR(reg); + } + + s_reg->regulator = reg; + refcount_set(&s_reg->refcount, 1); + } else { + refcount_inc(&s_reg->refcount); + } + + ret = regulator_enable(reg); + if (ret) { + dev_err(pctl->dev, + "Couldn't enable bank P%c regulator\n", 'A' + bank); + goto out; + } + + return 0; + +out: + if (refcount_dec_and_test(&s_reg->refcount)) { + regulator_put(s_reg->regulator); + s_reg->regulator = NULL; + } + + return ret; +} + +static int sunxi_pmx_free(struct pinctrl_dev *pctldev, unsigned offset) +{ + struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev); + unsigned short bank = offset / PINS_PER_BANK; + struct sunxi_pinctrl_regulator *s_reg = &pctl->regulators[bank]; + + if (!refcount_dec_and_test(&s_reg->refcount)) + return 0; + + regulator_disable(s_reg->regulator); + regulator_put(s_reg->regulator); + s_reg->regulator = NULL; + + return 0; +} + static const struct pinmux_ops sunxi_pmx_ops = { .get_functions_count = sunxi_pmx_get_funcs_cnt, .get_function_name = sunxi_pmx_get_func_name, .get_function_groups = sunxi_pmx_get_func_groups, .set_mux = sunxi_pmx_set_mux, .gpio_set_direction = sunxi_pmx_gpio_set_direction, + .request = sunxi_pmx_request, + .free = sunxi_pmx_free, .strict = true, }; diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h index 4a892e7dde66..e340d2a24b44 100644 --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h @@ -126,11 +126,17 @@ struct sunxi_pinctrl_group { unsigned pin; }; +struct sunxi_pinctrl_regulator { + struct regulator *regulator; + refcount_t refcount; +}; + struct sunxi_pinctrl { void __iomem *membase; struct gpio_chip *chip; const struct sunxi_pinctrl_desc *desc; struct device *dev; + struct sunxi_pinctrl_regulator regulators[12]; struct irq_domain *domain; struct sunxi_pinctrl_function *functions; unsigned nfunctions; -- git-series 0.9.1 _______________________________________________ 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] 13+ messages in thread
* Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators 2018-12-06 14:02 ` [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators Maxime Ripard @ 2018-12-14 15:08 ` Linus Walleij 2018-12-14 15:11 ` Linus Walleij 2019-01-07 5:20 ` Chen-Yu Tsai 2 siblings, 0 replies; 13+ messages in thread From: Linus Walleij @ 2018-12-14 15:08 UTC (permalink / raw) To: Maxime Ripard Cc: open list:GPIO SUBSYSTEM, Chen-Yu Tsai, Mylène Josserand, Linux ARM, Thomas Petazzoni On Thu, Dec 6, 2018 at 3:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > The Allwinner SoCs have on most of their GPIO banks a regulator input. > > This issue was mainly ignored so far because either the regulator was a > static regulator that would be providing power anyway, or the bank was used > for a feature unsupported so far (CSI). For the odd cases, enabling it in > the bootloader was the preferred option. > > However, now that we are starting to support those features, and that we > can't really rely on the bootloader for this, we need to model those > regulators as such in the DT. > > This is slightly more complicated than what it looks like, since some > regulators will be tied to the PMIC, and in order to have access to the > PMIC bus, you need to mux its pins, which will need the pinctrl driver, > that needs the regulator driver to be registered. And this is how you get a > circular dependency. > > In practice however, the hardware cannot fall into this case since it would > result in a completely unusable bus. In order to avoid that circular > dependency, we can thus get and enable the regulators at pin_request time. > We'll then need to account for the references of all the pins of a > particular branch to know when to put the reference, but it works pretty > nicely once implemented. > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> Patch applied. 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] 13+ messages in thread
* Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators 2018-12-06 14:02 ` [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators Maxime Ripard 2018-12-14 15:08 ` Linus Walleij @ 2018-12-14 15:11 ` Linus Walleij 2018-12-17 13:16 ` Maxime Ripard 2019-01-07 5:20 ` Chen-Yu Tsai 2 siblings, 1 reply; 13+ messages in thread From: Linus Walleij @ 2018-12-14 15:11 UTC (permalink / raw) To: Maxime Ripard Cc: open list:GPIO SUBSYSTEM, Chen-Yu Tsai, Mylène Josserand, Linux ARM, Thomas Petazzoni I have applied the patch, but noted that this might need device tree binding changes/amendments, are these already done? 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] 13+ messages in thread
* Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators 2018-12-14 15:11 ` Linus Walleij @ 2018-12-17 13:16 ` Maxime Ripard 0 siblings, 0 replies; 13+ messages in thread From: Maxime Ripard @ 2018-12-17 13:16 UTC (permalink / raw) To: Linus Walleij Cc: open list:GPIO SUBSYSTEM, Chen-Yu Tsai, Mylène Josserand, Linux ARM, Thomas Petazzoni [-- Attachment #1.1: Type: text/plain, Size: 355 bytes --] On Fri, Dec 14, 2018 at 04:11:16PM +0100, Linus Walleij wrote: > I have applied the patch, but noted that this might need device tree binding > changes/amendments, are these already done? I forgot to send it, I just sent a patch with the bindings. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ 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] 13+ messages in thread
* Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators 2018-12-06 14:02 ` [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators Maxime Ripard 2018-12-14 15:08 ` Linus Walleij 2018-12-14 15:11 ` Linus Walleij @ 2019-01-07 5:20 ` Chen-Yu Tsai 2019-01-09 4:36 ` Chen-Yu Tsai 2 siblings, 1 reply; 13+ messages in thread From: Chen-Yu Tsai @ 2019-01-07 5:20 UTC (permalink / raw) To: Maxime Ripard Cc: linux-gpio, Linus Walleij, Mylene Josserand, linux-arm-kernel, Thomas Petazzoni On Thu, Dec 6, 2018 at 10:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > The Allwinner SoCs have on most of their GPIO banks a regulator input. > > This issue was mainly ignored so far because either the regulator was a > static regulator that would be providing power anyway, or the bank was used > for a feature unsupported so far (CSI). For the odd cases, enabling it in > the bootloader was the preferred option. > > However, now that we are starting to support those features, and that we > can't really rely on the bootloader for this, we need to model those > regulators as such in the DT. > > This is slightly more complicated than what it looks like, since some > regulators will be tied to the PMIC, and in order to have access to the > PMIC bus, you need to mux its pins, which will need the pinctrl driver, > that needs the regulator driver to be registered. And this is how you get a > circular dependency. > > In practice however, the hardware cannot fall into this case since it would > result in a completely unusable bus. In order to avoid that circular > dependency, we can thus get and enable the regulators at pin_request time. > We'll then need to account for the references of all the pins of a > particular branch to know when to put the reference, but it works pretty > nicely once implemented. > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> I'm getting a warning on my Bananapi M1+: [ +0.004918] sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator [ +0.009931] sun7i-dwmac 1c50000.ethernet: PTP uses main clock [ +0.005764] sun7i-dwmac 1c50000.ethernet: no reset control found [ +0.006111] ------------[ cut here ]------------ [ +0.004640] WARNING: CPU: 1 PID: 1 at drivers/regulator/core.c:2054 _regulator_put.part.8+0xf8/0xfc [ +0.009065] Modules linked in: [ +0.003085] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.0.0-rc1 #5 [ +0.006179] Hardware name: Allwinner sun7i (A20) Family [ +0.005252] [<c010eaf0>] (unwind_backtrace) from [<c010b854>] (show_stack+0x10/0x14) [ +0.007755] [<c010b854>] (show_stack) from [<c089ac28>] (dump_stack+0x88/0x9c) [ +0.007233] [<c089ac28>] (dump_stack) from [<c0127450>] (__warn+0xd4/0xf0) [ +0.006881] [<c0127450>] (__warn) from [<c01274ac>] (warn_slowpath_null+0x40/0x48) [ +0.007576] [<c01274ac>] (warn_slowpath_null) from [<c03fd25c>] (_regulator_put.part.8+0xf8/0xfc) [ +0.008876] [<c03fd25c>] (_regulator_put.part.8) from [<c03fd288>] (regulator_put+0x28/0x38) [ +0.008446] [<c03fd288>] (regulator_put) from [<c03c5f2c>] (sunxi_pmx_free+0x38/0x48) [ +0.007837] [<c03c5f2c>] (sunxi_pmx_free) from [<c03c2444>] (pin_free+0x9c/0xfc) [ +0.007402] [<c03c2444>] (pin_free) from [<c03c2fb0>] (pinmux_disable_setting+0x118/0x184) [ +0.008271] [<c03c2fb0>] (pinmux_disable_setting) from [<c03c0060>] (pinctrl_free+0x13c/0x144) [ +0.008619] [<c03c0060>] (pinctrl_free) from [<c048c6a0>] (release_nodes+0x1bc/0x200) [ +0.007839] [<c048c6a0>] (release_nodes) from [<c0488964>] (really_probe+0x110/0x2cc) [ +0.007838] [<c0488964>] (really_probe) from [<c0488c84>] (driver_probe_device+0x60/0x16c) [ +0.008270] [<c0488c84>] (driver_probe_device) from [<c0488e6c>] (__driver_attach+0xdc/0xe0) [ +0.008444] [<c0488e6c>] (__driver_attach) from [<c0486d44>] (bus_for_each_dev+0x74/0xb4) [ +0.008184] [<c0486d44>] (bus_for_each_dev) from [<c0487edc>] (bus_add_driver+0x1bc/0x200) [ +0.008270] [<c0487edc>] (bus_add_driver) from [<c04897ac>] (driver_register+0x74/0x108) [ +0.008098] [<c04897ac>] (driver_register) from [<c010270c>] (do_one_initcall+0x7c/0x1a8) [ +0.008187] [<c010270c>] (do_one_initcall) from [<c0d00e14>] (kernel_init_freeable+0x13c/0x1d8) [ +0.008705] [<c0d00e14>] (kernel_init_freeable) from [<c08b08b8>] (kernel_init+0x8/0x110) [ +0.008184] [<c08b08b8>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c) [ +0.007570] Exception stack(0xef04ffb0 to 0xef04fff8) [ +0.005054] ffa0: 00000000 00000000 00000000 00000000 [ +0.008180] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ +0.008179] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ +0.006654] ---[ end trace 2bdb4a597b3c54ef ]--- Note that sun7i-dwmac probe is deferred here. The instance where the probe succeeds shows no warning. 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] 13+ messages in thread
* Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators 2019-01-07 5:20 ` Chen-Yu Tsai @ 2019-01-09 4:36 ` Chen-Yu Tsai 0 siblings, 0 replies; 13+ messages in thread From: Chen-Yu Tsai @ 2019-01-09 4:36 UTC (permalink / raw) To: Maxime Ripard Cc: linux-gpio, Linus Walleij, Mylene Josserand, linux-arm-kernel, Thomas Petazzoni On Mon, Jan 7, 2019 at 1:20 PM Chen-Yu Tsai <wens@csie.org> wrote: > > On Thu, Dec 6, 2018 at 10:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > The Allwinner SoCs have on most of their GPIO banks a regulator input. > > > > This issue was mainly ignored so far because either the regulator was a > > static regulator that would be providing power anyway, or the bank was used > > for a feature unsupported so far (CSI). For the odd cases, enabling it in > > the bootloader was the preferred option. > > > > However, now that we are starting to support those features, and that we > > can't really rely on the bootloader for this, we need to model those > > regulators as such in the DT. > > > > This is slightly more complicated than what it looks like, since some > > regulators will be tied to the PMIC, and in order to have access to the > > PMIC bus, you need to mux its pins, which will need the pinctrl driver, > > that needs the regulator driver to be registered. And this is how you get a > > circular dependency. > > > > In practice however, the hardware cannot fall into this case since it would > > result in a completely unusable bus. In order to avoid that circular > > dependency, we can thus get and enable the regulators at pin_request time. > > We'll then need to account for the references of all the pins of a > > particular branch to know when to put the reference, but it works pretty > > nicely once implemented. > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > I'm getting a warning on my Bananapi M1+: > > [ +0.004918] sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply > vcc-pa not found, using dummy regulator > [ +0.009931] sun7i-dwmac 1c50000.ethernet: PTP uses main clock > [ +0.005764] sun7i-dwmac 1c50000.ethernet: no reset control found > [ +0.006111] ------------[ cut here ]------------ > [ +0.004640] WARNING: CPU: 1 PID: 1 at drivers/regulator/core.c:2054 > _regulator_put.part.8+0xf8/0xfc > [ +0.009065] Modules linked in: > [ +0.003085] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.0.0-rc1 #5 > [ +0.006179] Hardware name: Allwinner sun7i (A20) Family > [ +0.005252] [<c010eaf0>] (unwind_backtrace) from [<c010b854>] > (show_stack+0x10/0x14) > [ +0.007755] [<c010b854>] (show_stack) from [<c089ac28>] (dump_stack+0x88/0x9c) > [ +0.007233] [<c089ac28>] (dump_stack) from [<c0127450>] (__warn+0xd4/0xf0) > [ +0.006881] [<c0127450>] (__warn) from [<c01274ac>] > (warn_slowpath_null+0x40/0x48) > [ +0.007576] [<c01274ac>] (warn_slowpath_null) from [<c03fd25c>] > (_regulator_put.part.8+0xf8/0xfc) > [ +0.008876] [<c03fd25c>] (_regulator_put.part.8) from [<c03fd288>] > (regulator_put+0x28/0x38) > [ +0.008446] [<c03fd288>] (regulator_put) from [<c03c5f2c>] > (sunxi_pmx_free+0x38/0x48) > [ +0.007837] [<c03c5f2c>] (sunxi_pmx_free) from [<c03c2444>] > (pin_free+0x9c/0xfc) > [ +0.007402] [<c03c2444>] (pin_free) from [<c03c2fb0>] > (pinmux_disable_setting+0x118/0x184) > [ +0.008271] [<c03c2fb0>] (pinmux_disable_setting) from [<c03c0060>] > (pinctrl_free+0x13c/0x144) > [ +0.008619] [<c03c0060>] (pinctrl_free) from [<c048c6a0>] > (release_nodes+0x1bc/0x200) > [ +0.007839] [<c048c6a0>] (release_nodes) from [<c0488964>] > (really_probe+0x110/0x2cc) > [ +0.007838] [<c0488964>] (really_probe) from [<c0488c84>] > (driver_probe_device+0x60/0x16c) > [ +0.008270] [<c0488c84>] (driver_probe_device) from [<c0488e6c>] > (__driver_attach+0xdc/0xe0) > [ +0.008444] [<c0488e6c>] (__driver_attach) from [<c0486d44>] > (bus_for_each_dev+0x74/0xb4) > [ +0.008184] [<c0486d44>] (bus_for_each_dev) from [<c0487edc>] > (bus_add_driver+0x1bc/0x200) > [ +0.008270] [<c0487edc>] (bus_add_driver) from [<c04897ac>] > (driver_register+0x74/0x108) > [ +0.008098] [<c04897ac>] (driver_register) from [<c010270c>] > (do_one_initcall+0x7c/0x1a8) > [ +0.008187] [<c010270c>] (do_one_initcall) from [<c0d00e14>] > (kernel_init_freeable+0x13c/0x1d8) > [ +0.008705] [<c0d00e14>] (kernel_init_freeable) from [<c08b08b8>] > (kernel_init+0x8/0x110) > [ +0.008184] [<c08b08b8>] (kernel_init) from [<c01010e8>] > (ret_from_fork+0x14/0x2c) > [ +0.007570] Exception stack(0xef04ffb0 to 0xef04fff8) > [ +0.005054] ffa0: 00000000 > 00000000 00000000 00000000 > [ +0.008180] ffc0: 00000000 00000000 00000000 00000000 00000000 > 00000000 00000000 00000000 > [ +0.008179] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 > [ +0.006654] ---[ end trace 2bdb4a597b3c54ef ]--- > > Note that sun7i-dwmac probe is deferred here. The instance where the > probe succeeds shows > no warning. Found the issue. There's a mismatch on the conditions when the regulator is enabled and disabled. I'll send a fix for it. 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] 13+ messages in thread
* [PATCH 2/2] ARM: dts: sun7i: bananapi: Add GPIO banks regulators 2018-12-06 14:02 [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators Maxime Ripard 2018-12-06 14:02 ` [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators Maxime Ripard @ 2018-12-06 14:02 ` Maxime Ripard 2018-12-14 15:10 ` Linus Walleij 2018-12-06 15:28 ` [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators Chen-Yu Tsai 2 siblings, 1 reply; 13+ messages in thread From: Maxime Ripard @ 2018-12-06 14:02 UTC (permalink / raw) To: Chen-Yu Tsai, Maxime Ripard, Linus Walleij Cc: linux-gpio, Mylene Josserand, linux-arm-kernel, Thomas Petazzoni The bananapi has all its bank regulators tied to the 3v3 static regulator. Make sure it's properly represented. Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> --- arch/arm/boot/dts/sun7i-a20-bananapi.dts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts index 70dfc4ac0bb5..cd8157c125eb 100644 --- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts +++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts @@ -201,6 +201,11 @@ }; &pio { + vcc-pa-supply = <®_vcc3v3>; + vcc-pc-supply = <®_vcc3v3>; + vcc-pe-supply = <®_vcc3v3>; + vcc-pf-supply = <®_vcc3v3>; + vcc-pg-supply = <®_vcc3v3>; gpio-line-names = /* PA */ "ERXD3", "ERXD2", "ERXD1", "ERXD0", "ETXD3", -- git-series 0.9.1 _______________________________________________ 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] 13+ messages in thread
* Re: [PATCH 2/2] ARM: dts: sun7i: bananapi: Add GPIO banks regulators 2018-12-06 14:02 ` [PATCH 2/2] ARM: dts: sun7i: bananapi: Add GPIO banks regulators Maxime Ripard @ 2018-12-14 15:10 ` Linus Walleij 0 siblings, 0 replies; 13+ messages in thread From: Linus Walleij @ 2018-12-14 15:10 UTC (permalink / raw) To: Maxime Ripard Cc: open list:GPIO SUBSYSTEM, Chen-Yu Tsai, Mylène Josserand, Linux ARM, Thomas Petazzoni On Thu, Dec 6, 2018 at 3:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > The bananapi has all its bank regulators tied to the 3v3 static regulator. > Make sure it's properly represented. > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> I suppose the DT bindings are already modified? Acked-by: Linus Walleij <linus.walleij@linaro.org> 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] 13+ messages in thread
* Re: [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators 2018-12-06 14:02 [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators Maxime Ripard 2018-12-06 14:02 ` [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators Maxime Ripard 2018-12-06 14:02 ` [PATCH 2/2] ARM: dts: sun7i: bananapi: Add GPIO banks regulators Maxime Ripard @ 2018-12-06 15:28 ` Chen-Yu Tsai 2018-12-06 15:47 ` Maxime Ripard 2 siblings, 1 reply; 13+ messages in thread From: Chen-Yu Tsai @ 2018-12-06 15:28 UTC (permalink / raw) To: Maxime Ripard Cc: linux-gpio, Linus Walleij, Mylène Josserand, linux-arm-kernel, Thomas Petazzoni On Thu, Dec 6, 2018 at 10:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi, > > Here is a first attempt at getting the regulators properly accounted for > the GPIO banks on the Allwinner SoCs. Cool. This is better than what I had in mind, which involved a deferred task to grab the regulators. > The main interogation I have currently is whether we should always try to > get the regulator for the current branch, or if we should restrict it to > the one available on the SoCs. Not sure what you mean here, but we should probably just list the actual names. For pre-A20 SoCs (A10/A10s/A13), they aren't even named VCC-Px. Instead they are named after the primary function of the pin bank, such as VCC-CARD, VCC-NAND, VCC-CSI0, VCC-CSI1. For pin banks that don't have per-bank power inputs, you should fall back to VCC-IO, or VCC-RTC in the case of the PL pins. So here's the rub: On A33 and later SoCs that are paired with a PMIC, VCC-PL or VCC-RTC is powered by the RTC regulator of the PMIC, which only gets registered when the PMIC regulator driver is probed, which needs the RSB controller, which needs the pin controller and the PL pins... ChenYu > > Let me know what you think, > Maxime > > Maxime Ripard (2): > pinctrl: sunxi: Deal with per-bank regulators > ARM: dts: sun7i: bananapi: Add GPIO banks regulators > > arch/arm/boot/dts/sun7i-a20-bananapi.dts | 5 ++- > drivers/pinctrl/sunxi/pinctrl-sunxi.c | 63 +++++++++++++++++++++++++- > drivers/pinctrl/sunxi/pinctrl-sunxi.h | 6 ++- > 3 files changed, 74 insertions(+) > > base-commit: 651022382c7f8da46cb4872a545ee1da6d097d2a > -- > git-series 0.9.1 _______________________________________________ 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] 13+ messages in thread
* Re: [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators 2018-12-06 15:28 ` [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators Chen-Yu Tsai @ 2018-12-06 15:47 ` Maxime Ripard 2018-12-06 16:01 ` Chen-Yu Tsai 0 siblings, 1 reply; 13+ messages in thread From: Maxime Ripard @ 2018-12-06 15:47 UTC (permalink / raw) To: Chen-Yu Tsai Cc: linux-gpio, Linus Walleij, Mylène Josserand, linux-arm-kernel, Thomas Petazzoni [-- Attachment #1.1: Type: text/plain, Size: 1539 bytes --] Hi, On Thu, Dec 06, 2018 at 11:28:21PM +0800, Chen-Yu Tsai wrote: > On Thu, Dec 6, 2018 at 10:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > The main interogation I have currently is whether we should always try to > > get the regulator for the current branch, or if we should restrict it to > > the one available on the SoCs. > > Not sure what you mean here, but we should probably just list the actual > names. The A20 for example doesn't have a VCC-PB regulator, so do we want to try to grab it if we request a PB* pin, or should we just know that somehow and not do it? > For pre-A20 SoCs (A10/A10s/A13), they aren't even named VCC-Px. Instead > they are named after the primary function of the pin bank, such as > VCC-CARD, VCC-NAND, VCC-CSI0, VCC-CSI1. I'd really prefer to stick to vcc-pX, that's pretty obvious even for those older SoCs, and we can maintain some consistency that way. > For pin banks that don't have per-bank power inputs, you should fall back > to VCC-IO, or VCC-RTC in the case of the PL pins. > > So here's the rub: On A33 and later SoCs that are paired with a PMIC, VCC-PL > or VCC-RTC is powered by the RTC regulator of the PMIC, which only gets > registered when the PMIC regulator driver is probed, which needs the RSB > controller, which needs the pin controller and the PL pins... I haven't seen any VCC-P* on the A33, do you have a reference? Thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ 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] 13+ messages in thread
* Re: [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators 2018-12-06 15:47 ` Maxime Ripard @ 2018-12-06 16:01 ` Chen-Yu Tsai 2018-12-11 17:01 ` Maxime Ripard 0 siblings, 1 reply; 13+ messages in thread From: Chen-Yu Tsai @ 2018-12-06 16:01 UTC (permalink / raw) To: Maxime Ripard Cc: linux-gpio, Linus Walleij, Mylène Josserand, linux-arm-kernel, Thomas Petazzoni On Thu, Dec 6, 2018 at 11:47 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi, > > On Thu, Dec 06, 2018 at 11:28:21PM +0800, Chen-Yu Tsai wrote: > > On Thu, Dec 6, 2018 at 10:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > The main interogation I have currently is whether we should always try to > > > get the regulator for the current branch, or if we should restrict it to > > > the one available on the SoCs. > > > > Not sure what you mean here, but we should probably just list the actual > > names. > > The A20 for example doesn't have a VCC-PB regulator, so do we want to > try to grab it if we request a PB* pin, or should we just know that > somehow and not do it? AFAIK, pin banks that don't have a separate supply are powered collectively by VCC-IO, as mentioned below in my previous reply. > > For pre-A20 SoCs (A10/A10s/A13), they aren't even named VCC-Px. Instead > > they are named after the primary function of the pin bank, such as > > VCC-CARD, VCC-NAND, VCC-CSI0, VCC-CSI1. > > I'd really prefer to stick to vcc-pX, that's pretty obvious even for > those older SoCs, and we can maintain some consistency that way. IRRC Mark said that supply names should match design names, not what is convenient. And then again there's the fallback for those that don't have separate rails. > > For pin banks that don't have per-bank power inputs, you should fall back > > to VCC-IO, or VCC-RTC in the case of the PL pins. > > > > So here's the rub: On A33 and later SoCs that are paired with a PMIC, VCC-PL > > or VCC-RTC is powered by the RTC regulator of the PMIC, which only gets > > registered when the PMIC regulator driver is probed, which needs the RSB > > controller, which needs the pin controller and the PL pins... > > I haven't seen any VCC-P* on the A33, do you have a reference? The A33 has a VCC-PD. This is not listed in the datasheet, but is shown in schematics. Other pins would be powered by VCC-IO, with the exception of PL, which would be power by VCC-RTC. The last bit is just a guess. We should probably ask Allwinner, or try to do some tests. 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] 13+ messages in thread
* Re: [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators 2018-12-06 16:01 ` Chen-Yu Tsai @ 2018-12-11 17:01 ` Maxime Ripard 0 siblings, 0 replies; 13+ messages in thread From: Maxime Ripard @ 2018-12-11 17:01 UTC (permalink / raw) To: Chen-Yu Tsai Cc: linux-gpio, Linus Walleij, Mylène Josserand, linux-arm-kernel, Thomas Petazzoni [-- Attachment #1.1: Type: text/plain, Size: 2875 bytes --] Hi! On Fri, Dec 07, 2018 at 12:01:18AM +0800, Chen-Yu Tsai wrote: > On Thu, Dec 6, 2018 at 11:47 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Thu, Dec 06, 2018 at 11:28:21PM +0800, Chen-Yu Tsai wrote: > > > On Thu, Dec 6, 2018 at 10:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > The main interogation I have currently is whether we should always try to > > > > get the regulator for the current branch, or if we should restrict it to > > > > the one available on the SoCs. > > > > > > Not sure what you mean here, but we should probably just list the actual > > > names. > > > > The A20 for example doesn't have a VCC-PB regulator, so do we want to > > try to grab it if we request a PB* pin, or should we just know that > > somehow and not do it? > > AFAIK, pin banks that don't have a separate supply are powered collectively > by VCC-IO, as mentioned below in my previous reply. Sigh, it was too easy :) Is VCC-IO supposed to be used for something else in the SoCs (and therefore could be ignored?) or not? Otherwise, we could have some hack where if the regulator is missing, or if it's marked always-on, we just skip it, and if it isn't we emit a warning. It should cover most cases, while keeping the code quite simple. > > > For pre-A20 SoCs (A10/A10s/A13), they aren't even named VCC-Px. Instead > > > they are named after the primary function of the pin bank, such as > > > VCC-CARD, VCC-NAND, VCC-CSI0, VCC-CSI1. > > > > I'd really prefer to stick to vcc-pX, that's pretty obvious even for > > those older SoCs, and we can maintain some consistency that way. > > IRRC Mark said that supply names should match design names, not what is > convenient. And then again there's the fallback for those that don't have > separate rails. That would require to complicate the logic a bit, to have something less obvious. I'd really like to avoid it if possible. > > > For pin banks that don't have per-bank power inputs, you should fall back > > > to VCC-IO, or VCC-RTC in the case of the PL pins. > > > > > > So here's the rub: On A33 and later SoCs that are paired with a PMIC, VCC-PL > > > or VCC-RTC is powered by the RTC regulator of the PMIC, which only gets > > > registered when the PMIC regulator driver is probed, which needs the RSB > > > controller, which needs the pin controller and the PL pins... > > > > I haven't seen any VCC-P* on the A33, do you have a reference? > > The A33 has a VCC-PD. This is not listed in the datasheet, but is shown in > schematics. Other pins would be powered by VCC-IO, with the exception of PL, > which would be power by VCC-RTC. The last bit is just a guess. We should > probably ask Allwinner, or try to do some tests. Ack. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ 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] 13+ messages in thread
end of thread, other threads:[~2019-01-09 4:36 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-06 14:02 [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators Maxime Ripard 2018-12-06 14:02 ` [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators Maxime Ripard 2018-12-14 15:08 ` Linus Walleij 2018-12-14 15:11 ` Linus Walleij 2018-12-17 13:16 ` Maxime Ripard 2019-01-07 5:20 ` Chen-Yu Tsai 2019-01-09 4:36 ` Chen-Yu Tsai 2018-12-06 14:02 ` [PATCH 2/2] ARM: dts: sun7i: bananapi: Add GPIO banks regulators Maxime Ripard 2018-12-14 15:10 ` Linus Walleij 2018-12-06 15:28 ` [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators Chen-Yu Tsai 2018-12-06 15:47 ` Maxime Ripard 2018-12-06 16:01 ` Chen-Yu Tsai 2018-12-11 17:01 ` Maxime Ripard
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).