On Sat, Dec 6, 2014 at 7:05 PM, Robert Jarzmik <robert.jarzmik@xxxxxxx> 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.
I can confirm it is buggy. It makes the property 'reset-gpios' to be
mandatory now and causes regression.
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@xxxxxxx>
> Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> Since v1: added Linus's ack
> ---
> drivers/usb/phy/phy-generic.c | 61 ++++++++++++++-----------------------------
> drivers/usb/phy/phy-generic.h | 4 +--
> 2 files changed, 21 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
> index 7594e50..71e061d 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);
Previously we had the active_low or active_high flag taken into
account. Now we don't.
>
> if (!asserted)
> usleep_range(10000, 20000);
> @@ -145,35 +137,38 @@ 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");
We should do 'devm_gpiod_get(dev, "reset");' instead.
This commit is now in linux-next as e9f2cefb0cdc2aea.
Should we revert it as it has so many issues?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
