All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sunxi: usb: convert PHY GPIO functions to DM
@ 2022-06-08  0:06 Andre Przywara
  2022-06-08  2:07 ` Samuel Holland
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Andre Przywara @ 2022-06-08  0:06 UTC (permalink / raw)
  To: Jagan Teki, Simon Glass, Tom Rini
  Cc: u-boot, Milan P . Stanić, Samuel Holland, linux-sunxi

The Allwinner USB PHY driver is still using the legacy GPIO interface,
which is now implemented by the DM_GPIO compat functions.
Those seem to have some design flaws, as setting the direction, then
later setting the value will not work, if the DM_GPIO driver is
implementing set_flags.

Fix this by using the dm_ version of the direct GPIO interface, which
uses struct gpio_desc structs to handle requested GPIOs, and actually
keeps the flags we set earlier.

This fixes USB operation on boards which need to toggle the VBUS supply
via a GPIO, like the Teres-I laptop or the BananaPi M2 Berry board.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reported-by: Milan P. Stanić <mps@arvanta.net>
---
 drivers/phy/allwinner/phy-sun4i-usb.c | 59 +++++++++++++++------------
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
index 86c589a65fd..aef2cb8f6f8 100644
--- a/drivers/phy/allwinner/phy-sun4i-usb.c
+++ b/drivers/phy/allwinner/phy-sun4i-usb.c
@@ -125,9 +125,9 @@ struct sun4i_usb_phy_info {
 
 struct sun4i_usb_phy_plat {
 	void __iomem *pmu;
-	int gpio_vbus;
-	int gpio_vbus_det;
-	int gpio_id_det;
+	struct gpio_desc gpio_vbus;
+	struct gpio_desc gpio_vbus_det;
+	struct gpio_desc gpio_id_det;
 	struct clk clocks;
 	struct reset_ctl resets;
 	int id;
@@ -224,8 +224,8 @@ static int sun4i_usb_phy_power_on(struct phy *phy)
 		initial_usb_scan_delay = 0;
 	}
 
-	if (usb_phy->gpio_vbus >= 0)
-		gpio_set_value(usb_phy->gpio_vbus, SUNXI_GPIO_PULL_UP);
+	if (dm_gpio_is_valid(&usb_phy->gpio_vbus))
+		dm_gpio_set_value(&usb_phy->gpio_vbus, 1);
 
 	return 0;
 }
@@ -235,8 +235,8 @@ static int sun4i_usb_phy_power_off(struct phy *phy)
 	struct sun4i_usb_phy_data *data = dev_get_priv(phy->dev);
 	struct sun4i_usb_phy_plat *usb_phy = &data->usb_phy[phy->id];
 
-	if (usb_phy->gpio_vbus >= 0)
-		gpio_set_value(usb_phy->gpio_vbus, SUNXI_GPIO_PULL_DISABLE);
+	if (dm_gpio_is_valid(&usb_phy->gpio_vbus))
+		dm_gpio_set_value(&usb_phy->gpio_vbus, 0);
 
 	return 0;
 }
@@ -386,8 +386,8 @@ int sun4i_usb_phy_vbus_detect(struct phy *phy)
 	struct sun4i_usb_phy_plat *usb_phy = &data->usb_phy[phy->id];
 	int err = 1, retries = 3;
 
-	if (usb_phy->gpio_vbus_det >= 0) {
-		err = gpio_get_value(usb_phy->gpio_vbus_det);
+	if (dm_gpio_is_valid(&usb_phy->gpio_vbus_det)) {
+		err = dm_gpio_get_value(&usb_phy->gpio_vbus_det);
 		/*
 		 * Vbus may have been provided by the board and just turned off
 		 * some milliseconds ago on reset. What we're measuring then is
@@ -395,7 +395,7 @@ int sun4i_usb_phy_vbus_detect(struct phy *phy)
 		 */
 		while (err > 0 && retries--) {
 			mdelay(100);
-			err = gpio_get_value(usb_phy->gpio_vbus_det);
+			err = dm_gpio_get_value(&usb_phy->gpio_vbus_det);
 		}
 	} else if (data->vbus_power_supply) {
 		err = regulator_get_enable(data->vbus_power_supply);
@@ -409,10 +409,10 @@ int sun4i_usb_phy_id_detect(struct phy *phy)
 	struct sun4i_usb_phy_data *data = dev_get_priv(phy->dev);
 	struct sun4i_usb_phy_plat *usb_phy = &data->usb_phy[phy->id];
 
-	if (usb_phy->gpio_id_det < 0)
-		return usb_phy->gpio_id_det;
+	if (!dm_gpio_is_valid(&usb_phy->gpio_id_det))
+		return -1;
 
-	return gpio_get_value(usb_phy->gpio_id_det);
+	return dm_gpio_get_value(&usb_phy->gpio_id_det);
 }
 
 void sun4i_usb_phy_set_squelch_detect(struct phy *phy, bool enabled)
@@ -454,35 +454,42 @@ static int sun4i_usb_phy_probe(struct udevice *dev)
 		if (data->cfg->missing_phys & BIT(i))
 			continue;
 
-		phy->gpio_vbus = sunxi_name_to_gpio(info->gpio_vbus);
-		if (phy->gpio_vbus >= 0) {
-			ret = gpio_request(phy->gpio_vbus, "usb_vbus");
+		ret = dm_gpio_lookup_name(info->gpio_vbus, &phy->gpio_vbus);
+		if (ret == 0) {
+			ret = dm_gpio_request(&phy->gpio_vbus, "usb_vbus");
 			if (ret)
 				return ret;
-			ret = gpio_direction_output(phy->gpio_vbus, 0);
+			ret = dm_gpio_set_dir_flags(&phy->gpio_vbus,
+						    GPIOD_IS_OUT);
+			if (ret)
+				return ret;
+			ret = dm_gpio_set_value(&phy->gpio_vbus, 0);
 			if (ret)
 				return ret;
 		}
 
-		phy->gpio_vbus_det = sunxi_name_to_gpio(info->gpio_vbus_det);
-		if (phy->gpio_vbus_det >= 0) {
-			ret = gpio_request(phy->gpio_vbus_det, "usb_vbus_det");
+		ret = dm_gpio_lookup_name(info->gpio_vbus_det,
+					  &phy->gpio_vbus_det);
+		if (ret == 0) {
+			ret = dm_gpio_request(&phy->gpio_vbus_det,
+					      "usb_vbus_det");
 			if (ret)
 				return ret;
-			ret = gpio_direction_input(phy->gpio_vbus_det);
+			ret = dm_gpio_set_dir_flags(&phy->gpio_vbus_det,
+						    GPIOD_IS_IN);
 			if (ret)
 				return ret;
 		}
 
-		phy->gpio_id_det = sunxi_name_to_gpio(info->gpio_id_det);
-		if (phy->gpio_id_det >= 0) {
-			ret = gpio_request(phy->gpio_id_det, "usb_id_det");
+		ret = dm_gpio_lookup_name(info->gpio_id_det, &phy->gpio_id_det);
+		if (ret == 0) {
+			ret = dm_gpio_request(&phy->gpio_id_det, "usb_id_det");
 			if (ret)
 				return ret;
-			ret = gpio_direction_input(phy->gpio_id_det);
+			ret = dm_gpio_set_dir_flags(&phy->gpio_id_det,
+						GPIOD_IS_IN | GPIOD_PULL_UP);
 			if (ret)
 				return ret;
-			sunxi_gpio_set_pull(phy->gpio_id_det, SUNXI_GPIO_PULL_UP);
 		}
 
 		if (data->cfg->dedicated_clocks)
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] sunxi: usb: convert PHY GPIO functions to DM
  2022-06-08  0:06 [PATCH] sunxi: usb: convert PHY GPIO functions to DM Andre Przywara
@ 2022-06-08  2:07 ` Samuel Holland
  2022-06-08  9:46   ` Andre Przywara
  2022-06-08 10:30 ` Milan P. Stanić
  2022-06-27  0:12 ` Andre Przywara
  2 siblings, 1 reply; 7+ messages in thread
From: Samuel Holland @ 2022-06-08  2:07 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Jagan Teki, Simon Glass, Tom Rini, u-boot, Milan P . Stanić,
	linux-sunxi

On 6/7/22 7:06 PM, Andre Przywara wrote:
> The Allwinner USB PHY driver is still using the legacy GPIO interface,
> which is now implemented by the DM_GPIO compat functions.
> Those seem to have some design flaws, as setting the direction, then
> later setting the value will not work, if the DM_GPIO driver is
> implementing set_flags.
> 
> Fix this by using the dm_ version of the direct GPIO interface, which
> uses struct gpio_desc structs to handle requested GPIOs, and actually
> keeps the flags we set earlier.
> 
> This fixes USB operation on boards which need to toggle the VBUS supply
> via a GPIO, like the Teres-I laptop or the BananaPi M2 Berry board.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Reported-by: Milan P. Stanić <mps@arvanta.net>

Reviewed-by: Samuel Holland <samuel@sholland.org>

It looks like the other users of sunxi_name_to_gpio are unaffected because they
call only gpio_direction_output (sometimes repeatedly), never gpio_set_value.

Regards,
Samuel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] sunxi: usb: convert PHY GPIO functions to DM
  2022-06-08  2:07 ` Samuel Holland
@ 2022-06-08  9:46   ` Andre Przywara
  2022-06-08 13:14     ` Samuel Holland
  0 siblings, 1 reply; 7+ messages in thread
From: Andre Przywara @ 2022-06-08  9:46 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Jagan Teki, Simon Glass, Tom Rini, u-boot, Milan P . Stanić,
	linux-sunxi

On Tue, 7 Jun 2022 21:07:59 -0500
Samuel Holland <samuel@sholland.org> wrote:

Hi,

> On 6/7/22 7:06 PM, Andre Przywara wrote:
> > The Allwinner USB PHY driver is still using the legacy GPIO interface,
> > which is now implemented by the DM_GPIO compat functions.
> > Those seem to have some design flaws, as setting the direction, then
> > later setting the value will not work, if the DM_GPIO driver is
> > implementing set_flags.
> > 
> > Fix this by using the dm_ version of the direct GPIO interface, which
> > uses struct gpio_desc structs to handle requested GPIOs, and actually
> > keeps the flags we set earlier.
> > 
> > This fixes USB operation on boards which need to toggle the VBUS supply
> > via a GPIO, like the Teres-I laptop or the BananaPi M2 Berry board.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > Reported-by: Milan P. Stanić <mps@arvanta.net>  
> 
> Reviewed-by: Samuel Holland <samuel@sholland.org>

Thanks!

> 
> It looks like the other users of sunxi_name_to_gpio are unaffected because they
> call only gpio_direction_output (sometimes repeatedly), never gpio_set_value.

Yeah, that's right, I was concerned about SATAPWR and MACPWR too, but they
work, for the very reason you mentioned.
But we should eventually eliminate their usage, together with
CONFIG_USB?_VBUS_PIN, as they are only used in U-Boot proper, where we can
rely on proper DT information now.

Cheers,
Andre

P.S.: I just picked the lowest hanging fruit, and removed
CONFIG_MMC?_CD_PIN, since we can ignore card detect in the same way the
BROM does.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] sunxi: usb: convert PHY GPIO functions to DM
  2022-06-08  0:06 [PATCH] sunxi: usb: convert PHY GPIO functions to DM Andre Przywara
  2022-06-08  2:07 ` Samuel Holland
@ 2022-06-08 10:30 ` Milan P. Stanić
  2022-06-27  0:12 ` Andre Przywara
  2 siblings, 0 replies; 7+ messages in thread
From: Milan P. Stanić @ 2022-06-08 10:30 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Jagan Teki, Simon Glass, Tom Rini, u-boot, Samuel Holland, linux-sunxi

Hi,

it works fine. Thank you.

On Wed, 2022-06-08 at 01:06, Andre Przywara wrote:
> The Allwinner USB PHY driver is still using the legacy GPIO interface,
> which is now implemented by the DM_GPIO compat functions.
> Those seem to have some design flaws, as setting the direction, then
> later setting the value will not work, if the DM_GPIO driver is
> implementing set_flags.
> 
> Fix this by using the dm_ version of the direct GPIO interface, which
> uses struct gpio_desc structs to handle requested GPIOs, and actually
> keeps the flags we set earlier.
> 
> This fixes USB operation on boards which need to toggle the VBUS supply
> via a GPIO, like the Teres-I laptop or the BananaPi M2 Berry board.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Reported-by: Milan P. Stanić <mps@arvanta.net>
Tested-by: Milan P. Stanić <mps@arvanta.net>
> ---
>  drivers/phy/allwinner/phy-sun4i-usb.c | 59 +++++++++++++++------------
>  1 file changed, 33 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> index 86c589a65fd..aef2cb8f6f8 100644
> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> @@ -125,9 +125,9 @@ struct sun4i_usb_phy_info {
>  
>  struct sun4i_usb_phy_plat {
>  	void __iomem *pmu;
> -	int gpio_vbus;
> -	int gpio_vbus_det;
> -	int gpio_id_det;
> +	struct gpio_desc gpio_vbus;
> +	struct gpio_desc gpio_vbus_det;
> +	struct gpio_desc gpio_id_det;
>  	struct clk clocks;
>  	struct reset_ctl resets;
>  	int id;
> @@ -224,8 +224,8 @@ static int sun4i_usb_phy_power_on(struct phy *phy)
>  		initial_usb_scan_delay = 0;
>  	}
>  
> -	if (usb_phy->gpio_vbus >= 0)
> -		gpio_set_value(usb_phy->gpio_vbus, SUNXI_GPIO_PULL_UP);
> +	if (dm_gpio_is_valid(&usb_phy->gpio_vbus))
> +		dm_gpio_set_value(&usb_phy->gpio_vbus, 1);
>  
>  	return 0;
>  }
> @@ -235,8 +235,8 @@ static int sun4i_usb_phy_power_off(struct phy *phy)
>  	struct sun4i_usb_phy_data *data = dev_get_priv(phy->dev);
>  	struct sun4i_usb_phy_plat *usb_phy = &data->usb_phy[phy->id];
>  
> -	if (usb_phy->gpio_vbus >= 0)
> -		gpio_set_value(usb_phy->gpio_vbus, SUNXI_GPIO_PULL_DISABLE);
> +	if (dm_gpio_is_valid(&usb_phy->gpio_vbus))
> +		dm_gpio_set_value(&usb_phy->gpio_vbus, 0);
>  
>  	return 0;
>  }
> @@ -386,8 +386,8 @@ int sun4i_usb_phy_vbus_detect(struct phy *phy)
>  	struct sun4i_usb_phy_plat *usb_phy = &data->usb_phy[phy->id];
>  	int err = 1, retries = 3;
>  
> -	if (usb_phy->gpio_vbus_det >= 0) {
> -		err = gpio_get_value(usb_phy->gpio_vbus_det);
> +	if (dm_gpio_is_valid(&usb_phy->gpio_vbus_det)) {
> +		err = dm_gpio_get_value(&usb_phy->gpio_vbus_det);
>  		/*
>  		 * Vbus may have been provided by the board and just turned off
>  		 * some milliseconds ago on reset. What we're measuring then is
> @@ -395,7 +395,7 @@ int sun4i_usb_phy_vbus_detect(struct phy *phy)
>  		 */
>  		while (err > 0 && retries--) {
>  			mdelay(100);
> -			err = gpio_get_value(usb_phy->gpio_vbus_det);
> +			err = dm_gpio_get_value(&usb_phy->gpio_vbus_det);
>  		}
>  	} else if (data->vbus_power_supply) {
>  		err = regulator_get_enable(data->vbus_power_supply);
> @@ -409,10 +409,10 @@ int sun4i_usb_phy_id_detect(struct phy *phy)
>  	struct sun4i_usb_phy_data *data = dev_get_priv(phy->dev);
>  	struct sun4i_usb_phy_plat *usb_phy = &data->usb_phy[phy->id];
>  
> -	if (usb_phy->gpio_id_det < 0)
> -		return usb_phy->gpio_id_det;
> +	if (!dm_gpio_is_valid(&usb_phy->gpio_id_det))
> +		return -1;
>  
> -	return gpio_get_value(usb_phy->gpio_id_det);
> +	return dm_gpio_get_value(&usb_phy->gpio_id_det);
>  }
>  
>  void sun4i_usb_phy_set_squelch_detect(struct phy *phy, bool enabled)
> @@ -454,35 +454,42 @@ static int sun4i_usb_phy_probe(struct udevice *dev)
>  		if (data->cfg->missing_phys & BIT(i))
>  			continue;
>  
> -		phy->gpio_vbus = sunxi_name_to_gpio(info->gpio_vbus);
> -		if (phy->gpio_vbus >= 0) {
> -			ret = gpio_request(phy->gpio_vbus, "usb_vbus");
> +		ret = dm_gpio_lookup_name(info->gpio_vbus, &phy->gpio_vbus);
> +		if (ret == 0) {
> +			ret = dm_gpio_request(&phy->gpio_vbus, "usb_vbus");
>  			if (ret)
>  				return ret;
> -			ret = gpio_direction_output(phy->gpio_vbus, 0);
> +			ret = dm_gpio_set_dir_flags(&phy->gpio_vbus,
> +						    GPIOD_IS_OUT);
> +			if (ret)
> +				return ret;
> +			ret = dm_gpio_set_value(&phy->gpio_vbus, 0);
>  			if (ret)
>  				return ret;
>  		}
>  
> -		phy->gpio_vbus_det = sunxi_name_to_gpio(info->gpio_vbus_det);
> -		if (phy->gpio_vbus_det >= 0) {
> -			ret = gpio_request(phy->gpio_vbus_det, "usb_vbus_det");
> +		ret = dm_gpio_lookup_name(info->gpio_vbus_det,
> +					  &phy->gpio_vbus_det);
> +		if (ret == 0) {
> +			ret = dm_gpio_request(&phy->gpio_vbus_det,
> +					      "usb_vbus_det");
>  			if (ret)
>  				return ret;
> -			ret = gpio_direction_input(phy->gpio_vbus_det);
> +			ret = dm_gpio_set_dir_flags(&phy->gpio_vbus_det,
> +						    GPIOD_IS_IN);
>  			if (ret)
>  				return ret;
>  		}
>  
> -		phy->gpio_id_det = sunxi_name_to_gpio(info->gpio_id_det);
> -		if (phy->gpio_id_det >= 0) {
> -			ret = gpio_request(phy->gpio_id_det, "usb_id_det");
> +		ret = dm_gpio_lookup_name(info->gpio_id_det, &phy->gpio_id_det);
> +		if (ret == 0) {
> +			ret = dm_gpio_request(&phy->gpio_id_det, "usb_id_det");
>  			if (ret)
>  				return ret;
> -			ret = gpio_direction_input(phy->gpio_id_det);
> +			ret = dm_gpio_set_dir_flags(&phy->gpio_id_det,
> +						GPIOD_IS_IN | GPIOD_PULL_UP);
>  			if (ret)
>  				return ret;
> -			sunxi_gpio_set_pull(phy->gpio_id_det, SUNXI_GPIO_PULL_UP);
>  		}
>  
>  		if (data->cfg->dedicated_clocks)
> -- 
> 2.35.3
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] sunxi: usb: convert PHY GPIO functions to DM
  2022-06-08  9:46   ` Andre Przywara
@ 2022-06-08 13:14     ` Samuel Holland
  2022-06-08 13:37       ` Andre Przywara
  0 siblings, 1 reply; 7+ messages in thread
From: Samuel Holland @ 2022-06-08 13:14 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Jagan Teki, Simon Glass, Tom Rini, u-boot, Milan P . Stanić,
	linux-sunxi

On 6/8/22 4:46 AM, Andre Przywara wrote:
> On Tue, 7 Jun 2022 21:07:59 -0500
> Samuel Holland <samuel@sholland.org> wrote:
> 
> Hi,
> 
>> On 6/7/22 7:06 PM, Andre Przywara wrote:
>>> The Allwinner USB PHY driver is still using the legacy GPIO interface,
>>> which is now implemented by the DM_GPIO compat functions.
>>> Those seem to have some design flaws, as setting the direction, then
>>> later setting the value will not work, if the DM_GPIO driver is
>>> implementing set_flags.
>>>
>>> Fix this by using the dm_ version of the direct GPIO interface, which
>>> uses struct gpio_desc structs to handle requested GPIOs, and actually
>>> keeps the flags we set earlier.
>>>
>>> This fixes USB operation on boards which need to toggle the VBUS supply
>>> via a GPIO, like the Teres-I laptop or the BananaPi M2 Berry board.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> Reported-by: Milan P. Stanić <mps@arvanta.net>  
>>
>> Reviewed-by: Samuel Holland <samuel@sholland.org>
> 
> Thanks!
> 
>>
>> It looks like the other users of sunxi_name_to_gpio are unaffected because they
>> call only gpio_direction_output (sometimes repeatedly), never gpio_set_value.
> 
> Yeah, that's right, I was concerned about SATAPWR and MACPWR too, but they
> work, for the very reason you mentioned.
> But we should eventually eliminate their usage, together with
> CONFIG_USB?_VBUS_PIN, as they are only used in U-Boot proper, where we can
> rely on proper DT information now.

I have a series for converting the USB PHY driver to devicetree already, but it
has been blocked on the devicetree update renaming "usb0_*_det-gpio" to
"usb0_*_det-gpios". In fact it is still blocked on that, because the sun4i
devicetree patch was only partially applied.

I will rebase the series on top of your patch and send it soon.

Regards,
Samuel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] sunxi: usb: convert PHY GPIO functions to DM
  2022-06-08 13:14     ` Samuel Holland
@ 2022-06-08 13:37       ` Andre Przywara
  0 siblings, 0 replies; 7+ messages in thread
From: Andre Przywara @ 2022-06-08 13:37 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Jagan Teki, Simon Glass, Tom Rini, u-boot, Milan P . Stanić,
	linux-sunxi

On Wed, 8 Jun 2022 08:14:27 -0500
Samuel Holland <samuel@sholland.org> wrote:

Hi,

> On 6/8/22 4:46 AM, Andre Przywara wrote:
> > On Tue, 7 Jun 2022 21:07:59 -0500
> > Samuel Holland <samuel@sholland.org> wrote:
> > 
> > Hi,
> >   
> >> On 6/7/22 7:06 PM, Andre Przywara wrote:  
> >>> The Allwinner USB PHY driver is still using the legacy GPIO interface,
> >>> which is now implemented by the DM_GPIO compat functions.
> >>> Those seem to have some design flaws, as setting the direction, then
> >>> later setting the value will not work, if the DM_GPIO driver is
> >>> implementing set_flags.
> >>>
> >>> Fix this by using the dm_ version of the direct GPIO interface, which
> >>> uses struct gpio_desc structs to handle requested GPIOs, and actually
> >>> keeps the flags we set earlier.
> >>>
> >>> This fixes USB operation on boards which need to toggle the VBUS supply
> >>> via a GPIO, like the Teres-I laptop or the BananaPi M2 Berry board.
> >>>
> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >>> Reported-by: Milan P. Stanić <mps@arvanta.net>    
> >>
> >> Reviewed-by: Samuel Holland <samuel@sholland.org>  
> > 
> > Thanks!
> >   
> >>
> >> It looks like the other users of sunxi_name_to_gpio are unaffected because they
> >> call only gpio_direction_output (sometimes repeatedly), never gpio_set_value.  
> > 
> > Yeah, that's right, I was concerned about SATAPWR and MACPWR too, but they
> > work, for the very reason you mentioned.
> > But we should eventually eliminate their usage, together with
> > CONFIG_USB?_VBUS_PIN, as they are only used in U-Boot proper, where we can
> > rely on proper DT information now.  
> 
> I have a series for converting the USB PHY driver to devicetree already, but it

So I remembered correctly ...

> has been blocked on the devicetree update renaming "usb0_*_det-gpio" to
> "usb0_*_det-gpios". In fact it is still blocked on that, because the sun4i
> devicetree patch was only partially applied.

Yeah, I saw that, sorry for the mixup. That patch didn't make it to
patchwork, so I applied it manually on one tree, but fixed it up wrongly on
the other machine. It is in my (mental) fixes queue, together with that
H616 FEL issue (once I find the actual culprit), and that NAND environment
fix.
Once that's in, I will try to update the -next branch with stuff for the
next merge window (in July), so you can then work on that branch.

> I will rebase the series on top of your patch and send it soon.

Cool, many thanks. And no rush, I just wanted to check to avoid double
work.
I think I figured nice and easy solutions for SATAPWR and MACPWR, will
test them later tonight.

Cheers,
Andre

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] sunxi: usb: convert PHY GPIO functions to DM
  2022-06-08  0:06 [PATCH] sunxi: usb: convert PHY GPIO functions to DM Andre Przywara
  2022-06-08  2:07 ` Samuel Holland
  2022-06-08 10:30 ` Milan P. Stanić
@ 2022-06-27  0:12 ` Andre Przywara
  2 siblings, 0 replies; 7+ messages in thread
From: Andre Przywara @ 2022-06-27  0:12 UTC (permalink / raw)
  To: Jagan Teki, Simon Glass, Tom Rini
  Cc: u-boot, Milan P . Stanić, Samuel Holland, linux-sunxi

On Wed,  8 Jun 2022 01:06:04 +0100
Andre Przywara <andre.przywara@arm.com> wrote:

> The Allwinner USB PHY driver is still using the legacy GPIO interface,
> which is now implemented by the DM_GPIO compat functions.
> Those seem to have some design flaws, as setting the direction, then
> later setting the value will not work, if the DM_GPIO driver is
> implementing set_flags.
> 
> Fix this by using the dm_ version of the direct GPIO interface, which
> uses struct gpio_desc structs to handle requested GPIOs, and actually
> keeps the flags we set earlier.
> 
> This fixes USB operation on boards which need to toggle the VBUS supply
> via a GPIO, like the Teres-I laptop or the BananaPi M2 Berry board.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Reported-by: Milan P. Stanić <mps@arvanta.net>

Applied to sunxi/master, for v2022.07.

Cheers,
Andre

> ---
>  drivers/phy/allwinner/phy-sun4i-usb.c | 59 +++++++++++++++------------
>  1 file changed, 33 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> index 86c589a65fd..aef2cb8f6f8 100644
> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> @@ -125,9 +125,9 @@ struct sun4i_usb_phy_info {
>  
>  struct sun4i_usb_phy_plat {
>  	void __iomem *pmu;
> -	int gpio_vbus;
> -	int gpio_vbus_det;
> -	int gpio_id_det;
> +	struct gpio_desc gpio_vbus;
> +	struct gpio_desc gpio_vbus_det;
> +	struct gpio_desc gpio_id_det;
>  	struct clk clocks;
>  	struct reset_ctl resets;
>  	int id;
> @@ -224,8 +224,8 @@ static int sun4i_usb_phy_power_on(struct phy *phy)
>  		initial_usb_scan_delay = 0;
>  	}
>  
> -	if (usb_phy->gpio_vbus >= 0)
> -		gpio_set_value(usb_phy->gpio_vbus, SUNXI_GPIO_PULL_UP);
> +	if (dm_gpio_is_valid(&usb_phy->gpio_vbus))
> +		dm_gpio_set_value(&usb_phy->gpio_vbus, 1);
>  
>  	return 0;
>  }
> @@ -235,8 +235,8 @@ static int sun4i_usb_phy_power_off(struct phy *phy)
>  	struct sun4i_usb_phy_data *data = dev_get_priv(phy->dev);
>  	struct sun4i_usb_phy_plat *usb_phy = &data->usb_phy[phy->id];
>  
> -	if (usb_phy->gpio_vbus >= 0)
> -		gpio_set_value(usb_phy->gpio_vbus, SUNXI_GPIO_PULL_DISABLE);
> +	if (dm_gpio_is_valid(&usb_phy->gpio_vbus))
> +		dm_gpio_set_value(&usb_phy->gpio_vbus, 0);
>  
>  	return 0;
>  }
> @@ -386,8 +386,8 @@ int sun4i_usb_phy_vbus_detect(struct phy *phy)
>  	struct sun4i_usb_phy_plat *usb_phy = &data->usb_phy[phy->id];
>  	int err = 1, retries = 3;
>  
> -	if (usb_phy->gpio_vbus_det >= 0) {
> -		err = gpio_get_value(usb_phy->gpio_vbus_det);
> +	if (dm_gpio_is_valid(&usb_phy->gpio_vbus_det)) {
> +		err = dm_gpio_get_value(&usb_phy->gpio_vbus_det);
>  		/*
>  		 * Vbus may have been provided by the board and just turned off
>  		 * some milliseconds ago on reset. What we're measuring then is
> @@ -395,7 +395,7 @@ int sun4i_usb_phy_vbus_detect(struct phy *phy)
>  		 */
>  		while (err > 0 && retries--) {
>  			mdelay(100);
> -			err = gpio_get_value(usb_phy->gpio_vbus_det);
> +			err = dm_gpio_get_value(&usb_phy->gpio_vbus_det);
>  		}
>  	} else if (data->vbus_power_supply) {
>  		err = regulator_get_enable(data->vbus_power_supply);
> @@ -409,10 +409,10 @@ int sun4i_usb_phy_id_detect(struct phy *phy)
>  	struct sun4i_usb_phy_data *data = dev_get_priv(phy->dev);
>  	struct sun4i_usb_phy_plat *usb_phy = &data->usb_phy[phy->id];
>  
> -	if (usb_phy->gpio_id_det < 0)
> -		return usb_phy->gpio_id_det;
> +	if (!dm_gpio_is_valid(&usb_phy->gpio_id_det))
> +		return -1;
>  
> -	return gpio_get_value(usb_phy->gpio_id_det);
> +	return dm_gpio_get_value(&usb_phy->gpio_id_det);
>  }
>  
>  void sun4i_usb_phy_set_squelch_detect(struct phy *phy, bool enabled)
> @@ -454,35 +454,42 @@ static int sun4i_usb_phy_probe(struct udevice *dev)
>  		if (data->cfg->missing_phys & BIT(i))
>  			continue;
>  
> -		phy->gpio_vbus = sunxi_name_to_gpio(info->gpio_vbus);
> -		if (phy->gpio_vbus >= 0) {
> -			ret = gpio_request(phy->gpio_vbus, "usb_vbus");
> +		ret = dm_gpio_lookup_name(info->gpio_vbus, &phy->gpio_vbus);
> +		if (ret == 0) {
> +			ret = dm_gpio_request(&phy->gpio_vbus, "usb_vbus");
>  			if (ret)
>  				return ret;
> -			ret = gpio_direction_output(phy->gpio_vbus, 0);
> +			ret = dm_gpio_set_dir_flags(&phy->gpio_vbus,
> +						    GPIOD_IS_OUT);
> +			if (ret)
> +				return ret;
> +			ret = dm_gpio_set_value(&phy->gpio_vbus, 0);
>  			if (ret)
>  				return ret;
>  		}
>  
> -		phy->gpio_vbus_det = sunxi_name_to_gpio(info->gpio_vbus_det);
> -		if (phy->gpio_vbus_det >= 0) {
> -			ret = gpio_request(phy->gpio_vbus_det, "usb_vbus_det");
> +		ret = dm_gpio_lookup_name(info->gpio_vbus_det,
> +					  &phy->gpio_vbus_det);
> +		if (ret == 0) {
> +			ret = dm_gpio_request(&phy->gpio_vbus_det,
> +					      "usb_vbus_det");
>  			if (ret)
>  				return ret;
> -			ret = gpio_direction_input(phy->gpio_vbus_det);
> +			ret = dm_gpio_set_dir_flags(&phy->gpio_vbus_det,
> +						    GPIOD_IS_IN);
>  			if (ret)
>  				return ret;
>  		}
>  
> -		phy->gpio_id_det = sunxi_name_to_gpio(info->gpio_id_det);
> -		if (phy->gpio_id_det >= 0) {
> -			ret = gpio_request(phy->gpio_id_det, "usb_id_det");
> +		ret = dm_gpio_lookup_name(info->gpio_id_det, &phy->gpio_id_det);
> +		if (ret == 0) {
> +			ret = dm_gpio_request(&phy->gpio_id_det, "usb_id_det");
>  			if (ret)
>  				return ret;
> -			ret = gpio_direction_input(phy->gpio_id_det);
> +			ret = dm_gpio_set_dir_flags(&phy->gpio_id_det,
> +						GPIOD_IS_IN | GPIOD_PULL_UP);
>  			if (ret)
>  				return ret;
> -			sunxi_gpio_set_pull(phy->gpio_id_det, SUNXI_GPIO_PULL_UP);
>  		}
>  
>  		if (data->cfg->dedicated_clocks)


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-06-27  0:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-08  0:06 [PATCH] sunxi: usb: convert PHY GPIO functions to DM Andre Przywara
2022-06-08  2:07 ` Samuel Holland
2022-06-08  9:46   ` Andre Przywara
2022-06-08 13:14     ` Samuel Holland
2022-06-08 13:37       ` Andre Przywara
2022-06-08 10:30 ` Milan P. Stanić
2022-06-27  0:12 ` Andre Przywara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.