[PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators

[PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators

[Maxime Ripard]

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

[PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators

[Maxime Ripard]

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

[PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators

[Maxime Ripard]

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

[PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators

[Maxime Ripard]

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

Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators

[Linus Walleij]

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

Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators

[Linus Walleij]

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

Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators

[Linus Walleij]

I have applied the patch, but noted that this might need device tree binding
changes/amendments, are these already done?

Yours,
Linus Walleij

Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators

[Linus Walleij]

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

Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators

[Maxime Ripard]

[-- 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

Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators

[Chen-Yu Tsai]

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

Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators

[Chen-Yu Tsai]

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

Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators

[Chen-Yu Tsai]

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

Re: [PATCH 1/2] pinctrl: sunxi: Deal with per-bank regulators

[Chen-Yu Tsai]

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

[PATCH 2/2] ARM: dts: sun7i: bananapi: Add GPIO banks regulators

[Maxime Ripard]

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 = <&reg_vcc3v3>;
+	vcc-pc-supply = <&reg_vcc3v3>;
+	vcc-pe-supply = <&reg_vcc3v3>;
+	vcc-pf-supply = <&reg_vcc3v3>;
+	vcc-pg-supply = <&reg_vcc3v3>;
 	gpio-line-names =
 		/* PA */
 		"ERXD3", "ERXD2", "ERXD1", "ERXD0", "ETXD3",
-- 
git-series 0.9.1

[PATCH 2/2] ARM: dts: sun7i: bananapi: Add GPIO banks regulators

[Maxime Ripard]

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 = <&reg_vcc3v3>;
+	vcc-pc-supply = <&reg_vcc3v3>;
+	vcc-pe-supply = <&reg_vcc3v3>;
+	vcc-pf-supply = <&reg_vcc3v3>;
+	vcc-pg-supply = <&reg_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

Re: [PATCH 2/2] ARM: dts: sun7i: bananapi: Add GPIO banks regulators

[Linus Walleij]

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

Re: [PATCH 2/2] ARM: dts: sun7i: bananapi: Add GPIO banks regulators

[Linus Walleij]

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

Re: [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators

[Chen-Yu Tsai]

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

Re: [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators

[Chen-Yu Tsai]

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

Re: [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators

[Maxime Ripard]

[-- 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

Re: [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators

[Chen-Yu Tsai]

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

Re: [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators

[Chen-Yu Tsai]

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

Re: [PATCH 0/2] pinctrl: sunxi: Account for per-bank GPIO regulators

[Maxime Ripard]

[-- 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