[PATCH linux dev-5.15 1/2] ARM: dts: Aspeed: Rainier: Add GPIO hog for USB power control

[PATCH linux dev-5.15 1/2] ARM: dts: Aspeed: Rainier: Add GPIO hog for USB power control

[Eddie James]

Set the USB power control to always on.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
index 866f32cdccea..b2af8ad09d33 100644
--- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
@@ -255,7 +255,7 @@ &gpio0 {
 	/*L0-L7*/	"","","","","","","","",
 	/*M0-M7*/	"","","","","","","","",
 	/*N0-N7*/	"","","","","","","","",
-	/*O0-O7*/	"","","","usb-power","","","","",
+	/*O0-O7*/	"","","","","","","","",
 	/*P0-P7*/	"","","","","pcieslot-power","","","",
 	/*Q0-Q7*/	"cfam-reset","","regulator-standby-faulted","","","","","",
 	/*R0-R7*/	"bmc-tpm-reset","power-chassis-control","power-chassis-good","","","","","",
@@ -275,6 +275,13 @@ i2c3_mux_oe_n {
 		output-high;
 		line-name = "I2C3_MUX_OE_N";
 	};
+
+	usb_power_en_n {
+		gpio-hog;
+		gpios = <ASPEED_GPIO(O, 3) GPIO_ACTIVE_LOW>;
+		output-high;
+		line-name = "USB_POWER_EN_N";
+	};
 };
 
 &emmc_controller {
-- 
2.27.0

[PATCH linux dev-5.15 2/2] ARM: dts: Aspeed: Everest: Add GPIO hog for USB power control

[Eddie James]

Set the USB power control to always on. Also enable the first EHCI
controller.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
index f42e2d776ba8..d19c0139e327 100644
--- a/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
@@ -292,6 +292,13 @@ &gpio0 {
 	/*X0-X7*/	"","","","","","","","",
 	/*Y0-Y7*/	"","","","","","","","",
 	/*Z0-Z7*/   "","","","","","","","";
+
+	usb_power_en_n {
+		gpio-hog;
+		gpios = <ASPEED_GPIO(O, 3) GPIO_ACTIVE_LOW>;
+		output-high;
+		line-name = "USB_POWER_EN_N";
+	};
 };
 
 &i2c0 {
@@ -2375,6 +2382,10 @@ i2c15mux2chn3: i2c@3 {
 	};
 };
 
+&ehci0 {
+	status = "okay";
+};
+
 &ehci1 {
 	status = "okay";
 };
-- 
2.27.0

Re: [PATCH linux dev-5.15 1/2] ARM: dts: Aspeed: Rainier: Add GPIO hog for USB power control

[Joel Stanley]

On Mon, 6 Dec 2021 at 21:32, Eddie James <eajames@linux.ibm.com> wrote:
>
> Set the USB power control to always on.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
> index 866f32cdccea..b2af8ad09d33 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
> @@ -255,7 +255,7 @@ &gpio0 {
>         /*L0-L7*/       "","","","","","","","",
>         /*M0-M7*/       "","","","","","","","",
>         /*N0-N7*/       "","","","","","","","",
> -       /*O0-O7*/       "","","","usb-power","","","","",

Do we want to remove the pin from the gpio-names? Can we leave it
there as documentation, or does it conflict?

> +       /*O0-O7*/       "","","","","","","","",
>         /*P0-P7*/       "","","","","pcieslot-power","","","",
>         /*Q0-Q7*/       "cfam-reset","","regulator-standby-faulted","","","","","",
>         /*R0-R7*/       "bmc-tpm-reset","power-chassis-control","power-chassis-good","","","","","",
> @@ -275,6 +275,13 @@ i2c3_mux_oe_n {
>                 output-high;
>                 line-name = "I2C3_MUX_OE_N";
>         };
> +
> +       usb_power_en_n {
> +               gpio-hog;
> +               gpios = <ASPEED_GPIO(O, 3) GPIO_ACTIVE_LOW>;
> +               output-high;
> +               line-name = "USB_POWER_EN_N";

This contradicts what you called it above.

> +       };
>  };
>
>  &emmc_controller {
> --
> 2.27.0
>

Re: [PATCH linux dev-5.15 1/2] ARM: dts: Aspeed: Rainier: Add GPIO hog for USB power control

[Eddie James]

On 12/6/21 16:54, Joel Stanley wrote:
> On Mon, 6 Dec 2021 at 21:32, Eddie James <eajames@linux.ibm.com> wrote:
>> Set the USB power control to always on.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>>   arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>> index 866f32cdccea..b2af8ad09d33 100644
>> --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>> +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>> @@ -255,7 +255,7 @@ &gpio0 {
>>          /*L0-L7*/       "","","","","","","","",
>>          /*M0-M7*/       "","","","","","","","",
>>          /*N0-N7*/       "","","","","","","","",
>> -       /*O0-O7*/       "","","","usb-power","","","","",
> Do we want to remove the pin from the gpio-names? Can we leave it
> there as documentation, or does it conflict?


I don't know; The pin isn't documented so I thought it better to remove 
it from the line names. Then below, I named it according to the board 
spec, which somewhat fits with the i2c mux hog. I'm open to whatever we 
think is best here.


Thanks!

Eddie


>
>> +       /*O0-O7*/       "","","","","","","","",
>>          /*P0-P7*/       "","","","","pcieslot-power","","","",
>>          /*Q0-Q7*/       "cfam-reset","","regulator-standby-faulted","","","","","",
>>          /*R0-R7*/       "bmc-tpm-reset","power-chassis-control","power-chassis-good","","","","","",
>> @@ -275,6 +275,13 @@ i2c3_mux_oe_n {
>>                  output-high;
>>                  line-name = "I2C3_MUX_OE_N";
>>          };
>> +
>> +       usb_power_en_n {
>> +               gpio-hog;
>> +               gpios = <ASPEED_GPIO(O, 3) GPIO_ACTIVE_LOW>;
>> +               output-high;
>> +               line-name = "USB_POWER_EN_N";
> This contradicts what you called it above.
>
>> +       };
>>   };
>>
>>   &emmc_controller {
>> --
>> 2.27.0
>>

Re: [PATCH linux dev-5.15 1/2] ARM: dts: Aspeed: Rainier: Add GPIO hog for USB power control

[Andrew Geissler]

> On Dec 7, 2021, at 2:28 PM, Eddie James <eajames@linux.ibm.com> wrote:
> 
> 
> On 12/6/21 16:54, Joel Stanley wrote:
>> On Mon, 6 Dec 2021 at 21:32, Eddie James <eajames@linux.ibm.com> wrote:
>>> Set the USB power control to always on.
>>> 
>>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>>> ---
>>>  arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>>> index 866f32cdccea..b2af8ad09d33 100644
>>> --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>>> +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>>> @@ -255,7 +255,7 @@ &gpio0 {
>>>         /*L0-L7*/       "","","","","","","","",
>>>         /*M0-M7*/       "","","","","","","","",
>>>         /*N0-N7*/       "","","","","","","","",
>>> -       /*O0-O7*/       "","","","usb-power","","","","",
>> Do we want to remove the pin from the gpio-names? Can we leave it
>> there as documentation, or does it conflict?
> 
> 
> I don't know; The pin isn't documented so I thought it better to remove it from the line names. Then below, I named it according to the board spec, which somewhat fits with the i2c mux hog. I'm open to whatever we think is best here.

The goal is to ensure GPIO’s which could have similar function between different boards have a common name so that userspace applications could just use that one name and work on multiple boards. Using a board name for the GPIO breaks that design point. But…if the GPIO is only ever going to be used on a single system, or userspace already has a way to abstract the name, we’ve been getting less stringent on this rule. At first blush, the “usb-power” name seemed nice and generic and it seems like a function that could be applicable across multiple systems. https://github.com/openbmc/docs/blob/master/designs/device-tree-gpio-naming.md is the doc about this. 

> 
> 
> Thanks!
> 
> Eddie
> 
> 
>> 
>>> +       /*O0-O7*/       "","","","","","","","",
>>>         /*P0-P7*/       "","","","","pcieslot-power","","","",
>>>         /*Q0-Q7*/       "cfam-reset","","regulator-standby-faulted","","","","","",
>>>         /*R0-R7*/       "bmc-tpm-reset","power-chassis-control","power-chassis-good","","","","","",
>>> @@ -275,6 +275,13 @@ i2c3_mux_oe_n {
>>>                 output-high;
>>>                 line-name = "I2C3_MUX_OE_N";
>>>         };
>>> +
>>> +       usb_power_en_n {
>>> +               gpio-hog;
>>> +               gpios = <ASPEED_GPIO(O, 3) GPIO_ACTIVE_LOW>;
>>> +               output-high;
>>> +               line-name = "USB_POWER_EN_N";
>> This contradicts what you called it above.
>> 
>>> +       };
>>>  };
>>> 
>>>  &emmc_controller {
>>> --
>>> 2.27.0