2017-03-23 16:27:42

by Neil Armstrong

[permalink] [raw]
Subject: [RFT PATCH 0/6] pinctrl: meson: Fix gpio-ranged for GPIO Hog

Whem trying to add a gpio hog to enable the USB Hub on the Odroid-C2, I
encountered a strange bug where when calling gpiochip_add_data() the gpiolib
code was trying to add the Hog but failed because the gpio ranges were missing.

In the meson-pinctrl driver, the gpio ranges are added manually /after/ the
call to gpiochip_add_data().
The arch/arm meson8 and meson8b patches has not been tested, this is why this
patchset is an RFT.

So this patchset uses the DT gpio-ranges attribute instead and solves the issue.

The final patch is the actual GPIO Hog for the Odroid-C2 board, which is an ugly
hack but is necessary to have USB Ports working on the board until the generic
power sequence framework is merged.

Neil Armstrong (6):
ARM64: dts: meson-gxbb: Add gpio-ranges properties
ARM64: dts: meson-gxl: Add gpio-ranges properties
ARM: dts: meson8: Add gpio-ranges properties
ARM: dts: meson8b: Add gpio-ranges properties
pinctrl: meson: use gpio-ranges from DT
ARM64: dts: meson-gxbb: Add USB Hub GPIO hog

arch/arm/boot/dts/meson8.dtsi | 2 ++
arch/arm/boot/dts/meson8b.dtsi | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 15 +++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 2 ++
drivers/pinctrl/meson/pinctrl-meson.c | 14 +-------------
6 files changed, 24 insertions(+), 13 deletions(-)

--
1.9.1


2017-03-23 16:27:44

by Neil Armstrong

[permalink] [raw]
Subject: [RFT PATCH 1/6] ARM64: dts: meson-gxbb: Add gpio-ranges properties

Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 04b3324..84c590b 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -129,6 +129,7 @@
reg-names = "mux", "pull", "gpio";
gpio-controller;
#gpio-cells = <2>;
+ gpio-ranges = <&pinctrl_aobus 0 0 14>;
};

uart_ao_a_pins: uart_ao_a {
@@ -245,6 +246,7 @@
reg-names = "mux", "pull", "pull-enable", "gpio";
gpio-controller;
#gpio-cells = <2>;
+ gpio-ranges = <&pinctrl_periphs 0 14 120>;
};

emmc_pins: emmc {
--
1.9.1

2017-03-23 16:27:48

by Neil Armstrong

[permalink] [raw]
Subject: [RFT PATCH 6/6] ARM64: dts: meson-gxbb: Add USB Hub GPIO hog

The ODroid-C2 on-board USB Hub needs to to have it's reset signal set to
high level in order to be enumerated by the USB Host Controller.

But this management must be part of the currently in-development Generic
Power Sequence patch that will allow a USB Controller driver to start and stop
a power sequence associated to the USB Bus.

In the meantime, a simple USB Hog will work to enable the USB Hub.

Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index c59403a..6288538 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -177,6 +177,21 @@
pinctrl-names = "default";
};

+&gpio_ao {
+ /*
+ * WARNING: The USB Hub on the Odroid-C2 needs a reset signal
+ * to be turned high in order to be detected by the USB Controller
+ * This signal should be handled by a USB specific power sequence
+ * in order to reset the Hub when USB bus is powered down.
+ */
+ usb-hub {
+ gpio-hog;
+ gpios = <GPIOAO_4 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "usb-hub-reset";
+ };
+};
+
&usb0_phy {
status = "okay";
phy-supply = <&usb_otg_pwr>;
--
1.9.1

2017-03-23 16:28:03

by Neil Armstrong

[permalink] [raw]
Subject: [RFT PATCH 5/6] pinctrl: meson: use gpio-ranges from DT

When trying to add a gpio-hog, we enter a weird loop where the gpio-ranges
is needed when gpiochip_add_data() is called but in the current implementation
the ranges are added from the driver afterwards.

A simple solution is to rely on the DR gpio-ranges attribute and remove the
call to gpiochip_add_pin_range().

Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/pinctrl/meson/pinctrl-meson.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
index cf1686e..66ed70c 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.c
+++ b/drivers/pinctrl/meson/pinctrl-meson.c
@@ -555,22 +555,10 @@ static int meson_gpiolib_register(struct meson_pinctrl *pc)
if (ret) {
dev_err(pc->dev, "can't add gpio chip %s\n",
pc->data->name);
- goto fail;
- }
-
- ret = gpiochip_add_pin_range(&pc->chip, dev_name(pc->dev),
- 0, pc->data->pin_base,
- pc->chip.ngpio);
- if (ret) {
- dev_err(pc->dev, "can't add pin range\n");
- goto fail;
+ return ret;
}

return 0;
-fail:
- gpiochip_remove(&pc->chip);
-
- return ret;
}

static struct regmap_config meson_regmap_config = {
--
1.9.1

2017-03-23 16:28:37

by Neil Armstrong

[permalink] [raw]
Subject: [RFT PATCH 4/6] ARM: dts: meson8b: Add gpio-ranges properties

Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm/boot/dts/meson8b.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index 41fd536..828aa49 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -198,6 +198,7 @@
reg-names = "mux", "pull", "pull-enable", "gpio";
gpio-controller;
#gpio-cells = <2>;
+ gpio-ranges = <&pinctrl_cbus 0 0 130>;
};
};

@@ -215,6 +216,7 @@
reg-names = "mux", "pull", "gpio";
gpio-controller;
#gpio-cells = <2>;
+ gpio-ranges = <&pinctrl_aobus 0 130 16>;
};

uart_ao_a_pins: uart_ao_a {
--
1.9.1

2017-03-23 16:28:52

by Neil Armstrong

[permalink] [raw]
Subject: [RFT PATCH 3/6] ARM: dts: meson8: Add gpio-ranges properties

Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm/boot/dts/meson8.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
index 45619f6..ebc763e 100644
--- a/arch/arm/boot/dts/meson8.dtsi
+++ b/arch/arm/boot/dts/meson8.dtsi
@@ -106,6 +106,7 @@
reg-names = "mux", "pull", "pull-enable", "gpio";
gpio-controller;
#gpio-cells = <2>;
+ gpio-ranges = <&pinctrl_cbus 0 0 120>;
};

spi_nor_pins: nor {
@@ -148,6 +149,7 @@
reg-names = "mux", "pull", "gpio";
gpio-controller;
#gpio-cells = <2>;
+ gpio-ranges = <&pinctrl_aobus 0 120 16>;
};

uart_ao_a_pins: uart_ao_a {
--
1.9.1

2017-03-23 16:29:14

by Neil Armstrong

[permalink] [raw]
Subject: [RFT PATCH 2/6] ARM64: dts: meson-gxl: Add gpio-ranges properties

Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index fe11b5f..64f4b6e 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -79,6 +79,7 @@
reg-names = "mux", "pull", "gpio";
gpio-controller;
#gpio-cells = <2>;
+ gpio-ranges = <&pinctrl_aobus 0 0 14>;
};

uart_ao_a_pins: uart_ao_a {
@@ -142,6 +143,7 @@
reg-names = "mux", "pull", "pull-enable", "gpio";
gpio-controller;
#gpio-cells = <2>;
+ gpio-ranges = <&pinctrl_periphs 0 14 101>;
};

emmc_pins: emmc {
--
1.9.1

2017-03-23 20:17:17

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [RFT PATCH 5/6] pinctrl: meson: use gpio-ranges from DT

Hi Neil,

On Thu, Mar 23, 2017 at 5:27 PM, Neil Armstrong <[email protected]> wrote:
> When trying to add a gpio-hog, we enter a weird loop where the gpio-ranges
> is needed when gpiochip_add_data() is called but in the current implementation
> the ranges are added from the driver afterwards.
>
> A simple solution is to rely on the DR gpio-ranges attribute and remove the
> call to gpiochip_add_pin_range().
did you mean devicetree or DT instead of "DR"?


Regards,
Martin

2017-03-24 16:52:34

by Neil Armstrong

[permalink] [raw]
Subject: Re: [RFT PATCH 5/6] pinctrl: meson: use gpio-ranges from DT

On 03/23/2017 09:09 PM, Martin Blumenstingl wrote:
> Hi Neil,
>
> On Thu, Mar 23, 2017 at 5:27 PM, Neil Armstrong <[email protected]> wrote:
>> When trying to add a gpio-hog, we enter a weird loop where the gpio-ranges
>> is needed when gpiochip_add_data() is called but in the current implementation
>> the ranges are added from the driver afterwards.
>>
>> A simple solution is to rely on the DR gpio-ranges attribute and remove the
>> call to gpiochip_add_pin_range().
> did you mean devicetree or DT instead of "DR"?
>
>
> Regards,
> Martin
>

Indeed, sorry for the typo.

Neil

2017-03-24 20:11:47

by Kevin Hilman

[permalink] [raw]
Subject: Re: [RFT PATCH 0/6] pinctrl: meson: Fix gpio-ranged for GPIO Hog

Neil Armstrong <[email protected]> writes:

> Whem trying to add a gpio hog to enable the USB Hub on the Odroid-C2, I
> encountered a strange bug where when calling gpiochip_add_data() the gpiolib
> code was trying to add the Hog but failed because the gpio ranges were missing.
>
> In the meson-pinctrl driver, the gpio ranges are added manually /after/ the
> call to gpiochip_add_data().
> The arch/arm meson8 and meson8b patches has not been tested, this is why this
> patchset is an RFT.
>
> So this patchset uses the DT gpio-ranges attribute instead and solves the issue.
>
> The final patch is the actual GPIO Hog for the Odroid-C2 board, which is an ugly
> hack but is necessary to have USB Ports working on the board until the generic
> power sequence framework is merged.
>
> Neil Armstrong (6):
> ARM64: dts: meson-gxbb: Add gpio-ranges properties
> ARM64: dts: meson-gxl: Add gpio-ranges properties
> ARM: dts: meson8: Add gpio-ranges properties
> ARM: dts: meson8b: Add gpio-ranges properties
> pinctrl: meson: use gpio-ranges from DT
> ARM64: dts: meson-gxbb: Add USB Hub GPIO hog

Not sure what kind of extra testing is needed on meson8*, but I tested
on meson8b-odroidc1 by replacing the heartbeat LED GPIO with a GPIO
hog[1] and see the LED turn on and stay on, so it seems good to me.

Tested-by: Kevin Hilman <[email protected]>

Kevin

[1]
diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts b/arch/arm/boot/dts/meson8b-odroidc1.dts
index e50f1a1fdbc7..d750d6b4c9f1 100644
--- a/arch/arm/boot/dts/meson8b-odroidc1.dts
+++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
@@ -60,6 +60,7 @@
reg = <0x40000000 0x40000000>;
};

+/*
leds {
compatible = "gpio-leds";
blue {
@@ -69,10 +70,18 @@
default-state = "off";
};
};
+*/
};

&uart_AO {
status = "okay";
pinctrl-0 = <&uart_ao_a_pins>;
pinctrl-names = "default";
+
+ usb-hub {
+ gpio-hog;
+ gpios = <GPIOAO_13 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "usb-hub-reset";
+ };
};

2017-03-28 09:28:24

by Linus Walleij

[permalink] [raw]
Subject: Re: [RFT PATCH 1/6] ARM64: dts: meson-gxbb: Add gpio-ranges properties

On Thu, Mar 23, 2017 at 5:27 PM, Neil Armstrong <[email protected]> wrote:

> Signed-off-by: Neil Armstrong <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2017-03-28 09:28:48

by Linus Walleij

[permalink] [raw]
Subject: Re: [RFT PATCH 2/6] ARM64: dts: meson-gxl: Add gpio-ranges properties

On Thu, Mar 23, 2017 at 5:27 PM, Neil Armstrong <[email protected]> wrote:

> Signed-off-by: Neil Armstrong <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2017-03-28 09:29:44

by Linus Walleij

[permalink] [raw]
Subject: Re: [RFT PATCH 3/6] ARM: dts: meson8: Add gpio-ranges properties

On Thu, Mar 23, 2017 at 5:27 PM, Neil Armstrong <[email protected]> wrote:

> Signed-off-by: Neil Armstrong <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2017-03-28 09:30:43

by Linus Walleij

[permalink] [raw]
Subject: Re: [RFT PATCH 4/6] ARM: dts: meson8b: Add gpio-ranges properties

On Thu, Mar 23, 2017 at 5:27 PM, Neil Armstrong <[email protected]> wrote:

> Signed-off-by: Neil Armstrong <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2017-03-28 09:33:25

by Linus Walleij

[permalink] [raw]
Subject: Re: [RFT PATCH 6/6] ARM64: dts: meson-gxbb: Add USB Hub GPIO hog

On Thu, Mar 23, 2017 at 5:27 PM, Neil Armstrong <[email protected]> wrote:

> The ODroid-C2 on-board USB Hub needs to to have it's reset signal set to
> high level in order to be enumerated by the USB Host Controller.
>
> But this management must be part of the currently in-development Generic
> Power Sequence patch that will allow a USB Controller driver to start and stop
> a power sequence associated to the USB Bus.
>
> In the meantime, a simple USB Hog will work to enable the USB Hub.
>
> Signed-off-by: Neil Armstrong <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2017-03-28 09:38:31

by Neil Armstrong

[permalink] [raw]
Subject: Re: [RFT PATCH 5/6] pinctrl: meson: use gpio-ranges from DT

On 03/28/2017 11:30 AM, Linus Walleij wrote:
> On Thu, Mar 23, 2017 at 5:27 PM, Neil Armstrong <[email protected]> wrote:
>
> '> When trying to add a gpio-hog, we enter a weird loop where the gpio-ranges
>> is needed when gpiochip_add_data() is called but in the current implementation
>> the ranges are added from the driver afterwards.
>>
>> A simple solution is to rely on the DR gpio-ranges attribute and remove the
>> call to gpiochip_add_pin_range().
>>
>> Signed-off-by: Neil Armstrong <[email protected]>
>
> This is fine once the ranges have been applied to the device trees I guess.
>
> Tell me when you want me to merge this.
>
> Yours,
> Linus Walleij
>

Hi Linus,

Kevin should merge the DT patches since they have now Tested and Reviewed by's, so you can merge it now if you can.
Since it will live in you pinctrl tree, it will only impact linux-next until Kevin merges them.

Thanks,
Neil

2017-03-28 09:41:23

by Linus Walleij

[permalink] [raw]
Subject: Re: [RFT PATCH 5/6] pinctrl: meson: use gpio-ranges from DT

On Thu, Mar 23, 2017 at 5:27 PM, Neil Armstrong <[email protected]> wrote:

'> When trying to add a gpio-hog, we enter a weird loop where the gpio-ranges
> is needed when gpiochip_add_data() is called but in the current implementation
> the ranges are added from the driver afterwards.
>
> A simple solution is to rely on the DR gpio-ranges attribute and remove the
> call to gpiochip_add_pin_range().
>
> Signed-off-by: Neil Armstrong <[email protected]>

This is fine once the ranges have been applied to the device trees I guess.

Tell me when you want me to merge this.

Yours,
Linus Walleij

2017-03-28 09:41:31

by Linus Walleij

[permalink] [raw]
Subject: Re: [RFT PATCH 5/6] pinctrl: meson: use gpio-ranges from DT

On Thu, Mar 23, 2017 at 5:27 PM, Neil Armstrong <[email protected]> wrote:

> When trying to add a gpio-hog, we enter a weird loop where the gpio-ranges
> is needed when gpiochip_add_data() is called but in the current implementation
> the ranges are added from the driver afterwards.
>
> A simple solution is to rely on the DR gpio-ranges attribute and remove the
> call to gpiochip_add_pin_range().
>
> Signed-off-by: Neil Armstrong <[email protected]>

Patch applied.

Yours,
Linus Walleij

2017-03-28 14:57:56

by Kevin Hilman

[permalink] [raw]
Subject: Re: [RFT PATCH 5/6] pinctrl: meson: use gpio-ranges from DT

Neil Armstrong <[email protected]> writes:

> On 03/28/2017 11:30 AM, Linus Walleij wrote:
>> On Thu, Mar 23, 2017 at 5:27 PM, Neil Armstrong <[email protected]> wrote:
>>
>> '> When trying to add a gpio-hog, we enter a weird loop where the gpio-ranges
>>> is needed when gpiochip_add_data() is called but in the current implementation
>>> the ranges are added from the driver afterwards.
>>>
>>> A simple solution is to rely on the DR gpio-ranges attribute and remove the
>>> call to gpiochip_add_pin_range().
>>>
>>> Signed-off-by: Neil Armstrong <[email protected]>
>>
>> This is fine once the ranges have been applied to the device trees I guess.
>>
>> Tell me when you want me to merge this.
>>
>> Yours,
>> Linus Walleij
>>
>
> Hi Linus,
>
> Kevin should merge the DT patches since they have now Tested and Reviewed by's, so you can merge it now if you can.
> Since it will live in you pinctrl tree, it will only impact linux-next until Kevin merges them.

I've applied the DT patches now (branch: v4.12/dt64)

Kevin