| Message ID | 1363609781-4045-6-git-send-email-vbyravarasu@nvidia.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hello. On 18-03-2013 16:29, Venu Byravarasu wrote: > As GPIO information is avail through DT, used it to get Tegra ULPI > reset GPIO number. Added a new member to tegra_usb_phy structure to > store this number. > Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com> > --- > drivers/usb/phy/tegra_usb_phy.c | 25 +++++++++++-------------- > include/linux/usb/tegra_usb_phy.h | 1 + > 2 files changed, 12 insertions(+), 14 deletions(-) > diff --git a/drivers/usb/phy/tegra_usb_phy.c b/drivers/usb/phy/tegra_usb_phy.c > index b5b2037..29c5ac4 100644 > --- a/drivers/usb/phy/tegra_usb_phy.c > +++ b/drivers/usb/phy/tegra_usb_phy.c [...] > @@ -622,18 +619,18 @@ static int tegra_phy_init(struct usb_phy *x) [...] > - gpio_request(ulpi_config->reset_gpio, "ulpi_phy_reset_b"); > - gpio_direction_output(ulpi_config->reset_gpio, 0); > + gpio_request(phy->reset_gpio, "ulpi_phy_reset_b"); > + gpio_direction_output(phy->reset_gpio, 0); Why not use goio_request_one() instead of these two? Thought maybe it's a material of another patch... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: linux-usb-owner@vger.kernel.org [mailto:linux-usb- > owner@vger.kernel.org] On Behalf Of Sergei Shtylyov > Sent: Monday, March 18, 2013 6:32 PM > To: Venu Byravarasu > Cc: gregkh@linuxfoundation.org; stern@rowland.harvard.edu; > balbi@ti.com; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; > swarren@wwwdotorg.org; linux-tegra@vger.kernel.org; devicetree- > discuss@lists.ozlabs.org > Subject: Re: [PATCH 5/7] usb: phy: tegra: get ULPI reset GPIO info using DT. > > Hello. > > On 18-03-2013 16:29, Venu Byravarasu wrote: > > > As GPIO information is avail through DT, used it to get Tegra ULPI > > reset GPIO number. Added a new member to tegra_usb_phy structure to > > store this number. > > > Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com> > > --- > > drivers/usb/phy/tegra_usb_phy.c | 25 +++++++++++-------------- > > include/linux/usb/tegra_usb_phy.h | 1 + > > 2 files changed, 12 insertions(+), 14 deletions(-) > > > diff --git a/drivers/usb/phy/tegra_usb_phy.c > b/drivers/usb/phy/tegra_usb_phy.c > > index b5b2037..29c5ac4 100644 > > --- a/drivers/usb/phy/tegra_usb_phy.c > > +++ b/drivers/usb/phy/tegra_usb_phy.c > [...] > > @@ -622,18 +619,18 @@ static int tegra_phy_init(struct usb_phy *x) > [...] > > - gpio_request(ulpi_config->reset_gpio, "ulpi_phy_reset_b"); > > - gpio_direction_output(ulpi_config->reset_gpio, 0); > > + gpio_request(phy->reset_gpio, "ulpi_phy_reset_b"); > > + gpio_direction_output(phy->reset_gpio, 0); > > Why not use goio_request_one() instead of these two? Thought maybe it's > a > material of another patch... Sure, can take this up as part of next patch. Thanks for the review. > > WBR, Sergei > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/18/2013 06:29 AM, Venu Byravarasu wrote: > As GPIO information is avail through DT, used it to get Tegra ULPI > reset GPIO number. Added a new member to tegra_usb_phy structure to > store this number. > diff --git a/drivers/usb/phy/tegra_usb_phy.c b/drivers/usb/phy/tegra_usb_phy.c > - gpio_direction_output(config->reset_gpio, 0); > + gpio_direction_output(phy->reset_gpio, 0); > msleep(5); > - gpio_direction_output(config->reset_gpio, 1); > + gpio_direction_output(phy->reset_gpio, 1); That implies that the PHY reset signal is active-low. This should be represented in the GPIO flags in the device tree. In other words, instead of e.g.: nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */ you want: nvidia,phy-reset-gpio = <&gpio 169 1>; /* gpio PV1 */ Flag 1 means active-low. See Documentation/devicetree/bindings/gpio/nvidia,tegra20-gpio.txt. This is a bug in the current device tree content, although it has no effect since no code currently uses the GPIO flags from DT. I suggest creating a separate patch to fix this, and inserting it between patch 1 and 2 of the series. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/phy/tegra_usb_phy.c b/drivers/usb/phy/tegra_usb_phy.c index b5b2037..29c5ac4 100644 --- a/drivers/usb/phy/tegra_usb_phy.c +++ b/drivers/usb/phy/tegra_usb_phy.c @@ -541,11 +541,10 @@ static int ulpi_phy_power_on(struct tegra_usb_phy *phy) int ret; unsigned long val; void __iomem *base = phy->regs; - struct tegra_ulpi_config *config = phy->config; - gpio_direction_output(config->reset_gpio, 0); + gpio_direction_output(phy->reset_gpio, 0); msleep(5); - gpio_direction_output(config->reset_gpio, 1); + gpio_direction_output(phy->reset_gpio, 1); clk_prepare_enable(phy->clk); msleep(1); @@ -603,10 +602,8 @@ static int ulpi_phy_power_on(struct tegra_usb_phy *phy) static int ulpi_phy_power_off(struct tegra_usb_phy *phy) { - struct tegra_ulpi_config *config = phy->config; - clk_disable(phy->clk); - return gpio_direction_output(config->reset_gpio, 0); + return gpio_direction_output(phy->reset_gpio, 0); } static int tegra_phy_init(struct usb_phy *x) @@ -622,18 +619,18 @@ static int tegra_phy_init(struct usb_phy *x) pr_err("%s: can't get ulpi clock\n", __func__); return PTR_ERR(phy->clk); } - if (!gpio_is_valid(ulpi_config->reset_gpio)) - ulpi_config->reset_gpio = - of_get_named_gpio(phy->dev->of_node, - "nvidia,phy-reset-gpio", 0); - if (!gpio_is_valid(ulpi_config->reset_gpio)) { + + phy->reset_gpio = + of_get_named_gpio(phy->dev->of_node, + "nvidia,phy-reset-gpio", 0); + if (!gpio_is_valid(phy->reset_gpio)) { pr_err("%s: invalid reset gpio: %d\n", __func__, - ulpi_config->reset_gpio); + phy->reset_gpio); err = -EINVAL; goto err1; } - gpio_request(ulpi_config->reset_gpio, "ulpi_phy_reset_b"); - gpio_direction_output(ulpi_config->reset_gpio, 0); + gpio_request(phy->reset_gpio, "ulpi_phy_reset_b"); + gpio_direction_output(phy->reset_gpio, 0); phy->ulpi = otg_ulpi_create(&ulpi_viewport_access_ops, 0); phy->ulpi->io_priv = phy->regs + ULPI_VIEWPORT; } else { diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h index a7af923..6cfb8f1 100644 --- a/include/linux/usb/tegra_usb_phy.h +++ b/include/linux/usb/tegra_usb_phy.h @@ -62,6 +62,7 @@ struct tegra_usb_phy { struct device *dev; bool is_legacy_phy; bool is_ulpi_phy; + int reset_gpio; }; struct tegra_usb_phy *tegra_usb_phy_open(struct device *dev, int instance,
As GPIO information is avail through DT, used it to get Tegra ULPI reset GPIO number. Added a new member to tegra_usb_phy structure to store this number. Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com> --- drivers/usb/phy/tegra_usb_phy.c | 25 +++++++++++-------------- include/linux/usb/tegra_usb_phy.h | 1 + 2 files changed, 12 insertions(+), 14 deletions(-)