Re: [PATCH v1] usb: phy: generic: migrate to gpio_desc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sun, Nov 09, 2014 at 01:23:07PM +0100, Robert Jarzmik wrote:
> Change internal gpio handling from integer gpios into gpio
> descriptors. This change only addresses the internal API and
> device-tree/ACPI, while the legacy platform data remains integer space
> based.
> 
> This change is only build compile tested, and very prone to error. I
> leave this comment for now in the commit message so that this patch gets
> some testing as I'm pretty sure it's buggy.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@xxxxxxx>

To my eyes, patch looks fine, but let's get a review from Linus W.

Linus, can you have a look below ? Is this being used the way you
intended ? BTW, we need support DT and pdata platforms here.

> ---
>  drivers/usb/phy/phy-generic.c | 58 ++++++++++++-------------------------------
>  drivers/usb/phy/phy-generic.h |  4 +--
>  2 files changed, 18 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
> index 7594e50..eba097a 100644
> --- a/drivers/usb/phy/phy-generic.c
> +++ b/drivers/usb/phy/phy-generic.c
> @@ -61,16 +61,8 @@ static int nop_set_suspend(struct usb_phy *x, int suspend)
>  
>  static void nop_reset_set(struct usb_phy_generic *nop, int asserted)
>  {
> -	int value;
> -
> -	if (!gpio_is_valid(nop->gpio_reset))
> -		return;
> -
> -	value = asserted;
> -	if (nop->reset_active_low)
> -		value = !value;
> -
> -	gpio_set_value_cansleep(nop->gpio_reset, value);
> +	if (nop->gpiod_reset)
> +		gpiod_set_value(nop->gpiod_reset, asserted);
>  
>  	if (!asserted)
>  		usleep_range(10000, 20000);
> @@ -145,35 +137,35 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop,
>  		struct usb_phy_generic_platform_data *pdata)
>  {
>  	enum usb_phy_type type = USB_PHY_TYPE_USB2;
> -	int err;
> +	int err = 0;
>  
>  	u32 clk_rate = 0;
>  	bool needs_vcc = false;
>  
> -	nop->reset_active_low = true;	/* default behaviour */
> -
>  	if (dev->of_node) {
>  		struct device_node *node = dev->of_node;
> -		enum of_gpio_flags flags = 0;
>  
>  		if (of_property_read_u32(node, "clock-frequency", &clk_rate))
>  			clk_rate = 0;
>  
>  		needs_vcc = of_property_read_bool(node, "vcc-supply");
> -		nop->gpio_reset = of_get_named_gpio_flags(node, "reset-gpios",
> -								0, &flags);
> -		if (nop->gpio_reset == -EPROBE_DEFER)
> -			return -EPROBE_DEFER;
> -
> -		nop->reset_active_low = flags & OF_GPIO_ACTIVE_LOW;
> -
> +		nop->gpiod_reset = devm_gpiod_get(dev, "reset-gpios");
> +		err = PTR_ERR(nop->gpiod_reset);
>  	} else if (pdata) {
>  		type = pdata->type;
>  		clk_rate = pdata->clk_rate;
>  		needs_vcc = pdata->needs_vcc;
> -		nop->gpio_reset = pdata->gpio_reset;
> -	} else {
> -		nop->gpio_reset = -1;
> +		err = devm_gpio_request_one(dev, pdata->gpio_reset, 0,
> +					    dev_name(dev));
> +		if (!err)
> +			nop->gpiod_reset = gpio_to_desc(pdata->gpio_reset);
> +	}
> +
> +	if (err == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
> +	if (err) {
> +		dev_err(dev, "Error requesting RESET GPIO\n");
> +		return err;
>  	}
>  
>  	nop->phy.otg = devm_kzalloc(dev, sizeof(*nop->phy.otg),
> @@ -203,24 +195,6 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop,
>  			return -EPROBE_DEFER;
>  	}
>  
> -	if (gpio_is_valid(nop->gpio_reset)) {
> -		unsigned long gpio_flags;
> -
> -		/* Assert RESET */
> -		if (nop->reset_active_low)
> -			gpio_flags = GPIOF_OUT_INIT_LOW;
> -		else
> -			gpio_flags = GPIOF_OUT_INIT_HIGH;
> -
> -		err = devm_gpio_request_one(dev, nop->gpio_reset,
> -						gpio_flags, dev_name(dev));
> -		if (err) {
> -			dev_err(dev, "Error requesting RESET GPIO %d\n",
> -					nop->gpio_reset);
> -			return err;
> -		}
> -	}
> -
>  	nop->dev		= dev;
>  	nop->phy.dev		= nop->dev;
>  	nop->phy.label		= "nop-xceiv";
> diff --git a/drivers/usb/phy/phy-generic.h b/drivers/usb/phy/phy-generic.h
> index d8feacc..09924fd 100644
> --- a/drivers/usb/phy/phy-generic.h
> +++ b/drivers/usb/phy/phy-generic.h
> @@ -2,14 +2,14 @@
>  #define _PHY_GENERIC_H_
>  
>  #include <linux/usb/usb_phy_generic.h>
> +#include <linux/gpio/consumer.h>
>  
>  struct usb_phy_generic {
>  	struct usb_phy phy;
>  	struct device *dev;
>  	struct clk *clk;
>  	struct regulator *vcc;
> -	int gpio_reset;
> -	bool reset_active_low;
> +	struct gpio_desc *gpiod_reset;
>  };
>  
>  int usb_gen_phy_init(struct usb_phy *phy);
> -- 
> 2.1.0
> 

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux