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.jarz...@free.fr>
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
signature.asc
Description: Digital signature
