| Message ID | 20191211145054.24835-1-m.szyprowski@samsung.com |
|---|---|
| Headers | show |
| Series | USB3503: correct GPIOs polarity and update the driver | expand |
On Wed, 2019-12-11 at 15:52 +0100, Marek Szyprowski wrote: > From: Linus Walleij <linus.walleij@linaro.org> > > This converts the USB3503 to pick GPIO descriptors from the > device tree instead of iteratively picking out GPIO number > references and then referencing these from the global GPIO > numberspace. > > The USB3503 is only used from device tree among the in-tree > platforms. If board files would still desire to use it they can > provide machine descriptor tables. > > Make sure to preserve semantics such as the reset delay > introduced by Stefan. > > Cc: Chunfeng Yun <chunfeng.yun@mediatek.com> > Cc: Marek Szyprowski <m.szyprowski@samsung.com> > Cc: Stefan Agner <stefan@agner.ch> > Cc: Krzysztof Kozlowski <krzk@kernel.org> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > [mszyprow: invert the logic behind reset GPIO line] > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/usb/misc/usb3503.c | 94 ++++++++++----------------- > include/linux/platform_data/usb3503.h | 3 - > 2 files changed, 35 insertions(+), 62 deletions(-) > > diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c > index 72f39a9751b5..116bd789e568 100644 > --- a/drivers/usb/misc/usb3503.c > +++ b/drivers/usb/misc/usb3503.c > @@ -7,11 +7,10 @@ > > #include <linux/clk.h> > #include <linux/i2c.h> > -#include <linux/gpio.h> > +#include <linux/gpio/consumer.h> > #include <linux/delay.h> > #include <linux/slab.h> > #include <linux/module.h> > -#include <linux/of_gpio.h> > #include <linux/platform_device.h> > #include <linux/platform_data/usb3503.h> > #include <linux/regmap.h> > @@ -47,19 +46,19 @@ struct usb3503 { > struct device *dev; > struct clk *clk; > u8 port_off_mask; > - int gpio_intn; > - int gpio_reset; > - int gpio_connect; > + struct gpio_desc *intn; > + struct gpio_desc *reset; > + struct gpio_desc *connect; > bool secondary_ref_clk; > }; > > static int usb3503_reset(struct usb3503 *hub, int state) > { > - if (!state && gpio_is_valid(hub->gpio_connect)) > - gpio_set_value_cansleep(hub->gpio_connect, 0); > + if (!state && hub->connect) > + gpiod_set_value_cansleep(hub->connect, 0); > > - if (gpio_is_valid(hub->gpio_reset)) > - gpio_set_value_cansleep(hub->gpio_reset, state); > + if (hub->reset) > + gpiod_set_value_cansleep(hub->reset, !state); What about preparing another patch for @state before this path? > > /* Wait T_HUBINIT == 4ms for hub logic to stabilize */ > if (state) > @@ -115,8 +114,8 @@ static int usb3503_connect(struct usb3503 *hub) > } > } > > - if (gpio_is_valid(hub->gpio_connect)) > - gpio_set_value_cansleep(hub->gpio_connect, 1); > + if (hub->connect) > + gpiod_set_value_cansleep(hub->connect, 1); > > hub->mode = USB3503_MODE_HUB; > dev_info(dev, "switched to HUB mode\n"); > @@ -163,13 +162,11 @@ static int usb3503_probe(struct usb3503 *hub) > int err; > u32 mode = USB3503_MODE_HUB; > const u32 *property; > + enum gpiod_flags flags; > int len; > > if (pdata) { > hub->port_off_mask = pdata->port_off_mask; > - hub->gpio_intn = pdata->gpio_intn; > - hub->gpio_connect = pdata->gpio_connect; > - hub->gpio_reset = pdata->gpio_reset; > hub->mode = pdata->initial_mode; > } else if (np) { > u32 rate = 0; > @@ -230,59 +227,38 @@ static int usb3503_probe(struct usb3503 *hub) > } > } > > - hub->gpio_intn = of_get_named_gpio(np, "intn-gpios", 0); > - if (hub->gpio_intn == -EPROBE_DEFER) > - return -EPROBE_DEFER; > - hub->gpio_connect = of_get_named_gpio(np, "connect-gpios", 0); > - if (hub->gpio_connect == -EPROBE_DEFER) > - return -EPROBE_DEFER; > - hub->gpio_reset = of_get_named_gpio(np, "reset-gpios", 0); > - if (hub->gpio_reset == -EPROBE_DEFER) > - return -EPROBE_DEFER; > of_property_read_u32(np, "initial-mode", &mode); > hub->mode = mode; > } > > - if (hub->port_off_mask && !hub->regmap) > - dev_err(dev, "Ports disabled with no control interface\n"); > - > - if (gpio_is_valid(hub->gpio_intn)) { > - int val = hub->secondary_ref_clk ? GPIOF_OUT_INIT_LOW : > - GPIOF_OUT_INIT_HIGH; > - err = devm_gpio_request_one(dev, hub->gpio_intn, val, > - "usb3503 intn"); > - if (err) { > - dev_err(dev, > - "unable to request GPIO %d as interrupt pin (%d)\n", > - hub->gpio_intn, err); > - return err; > - } > - } > - > - if (gpio_is_valid(hub->gpio_connect)) { > - err = devm_gpio_request_one(dev, hub->gpio_connect, > - GPIOF_OUT_INIT_LOW, "usb3503 connect"); > - if (err) { > - dev_err(dev, > - "unable to request GPIO %d as connect pin (%d)\n", > - hub->gpio_connect, err); > - return err; > - } > - } > - > - if (gpio_is_valid(hub->gpio_reset)) { > - err = devm_gpio_request_one(dev, hub->gpio_reset, > - GPIOF_OUT_INIT_LOW, "usb3503 reset"); > + if (hub->secondary_ref_clk) > + flags = GPIOD_OUT_LOW; > + else > + flags = GPIOD_OUT_HIGH; > + hub->intn = devm_gpiod_get_optional(dev, "intn", flags); > + if (IS_ERR(hub->intn)) > + return PTR_ERR(hub->intn); > + if (hub->intn) > + gpiod_set_consumer_name(hub->intn, "usb3503 intn"); > + > + hub->connect = devm_gpiod_get_optional(dev, "connect", GPIOD_OUT_LOW); > + if (IS_ERR(hub->connect)) > + return PTR_ERR(hub->connect); > + if (hub->connect) > + gpiod_set_consumer_name(hub->connect, "usb3503 connect"); > + > + hub->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(hub->reset)) > + return PTR_ERR(hub->reset); > + if (hub->reset) { > /* Datasheet defines a hardware reset to be at least 100us */ > usleep_range(100, 10000); > - if (err) { > - dev_err(dev, > - "unable to request GPIO %d as reset pin (%d)\n", > - hub->gpio_reset, err); > - return err; > - } > + gpiod_set_consumer_name(hub->reset, "usb3503 reset"); > } > > + if (hub->port_off_mask && !hub->regmap) > + dev_err(dev, "Ports disabled with no control interface\n"); > + > usb3503_switch_mode(hub, hub->mode); > > dev_info(dev, "%s: probed in %s mode\n", __func__, > diff --git a/include/linux/platform_data/usb3503.h b/include/linux/platform_data/usb3503.h > index e049d51c1353..d01ef97ddf36 100644 > --- a/include/linux/platform_data/usb3503.h > +++ b/include/linux/platform_data/usb3503.h > @@ -17,9 +17,6 @@ enum usb3503_mode { > struct usb3503_platform_data { > enum usb3503_mode initial_mode; > u8 port_off_mask; > - int gpio_intn; > - int gpio_connect; > - int gpio_reset; > }; > > #endif
On Wed, Dec 11, 2019 at 03:52:17PM +0100, Marek Szyprowski wrote: > Current USB3503 driver ignores GPIO polarity and always operates as if the > GPIO lines were flagged as ACTIVE_HIGH. Fix the polarity for the existing > USB3503 chip applications to match the chip specification and common > convention for naming the pins. The only pin, which has to be ACTIVE_LOW > is the reset pin. The remaining are ACTIVE_HIGH. This change allows later > to fix the USB3503 driver to properly use generic GPIO bindings and read > polarity from DT. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Applied, thanks Maxime