| Message ID | 1372455427-20898-10-git-send-email-ttynkkynen@nvidia.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 06/28/2013 03:37 PM, Tuomas Tynkkynen wrote: > Use the new of_usb_get_dr_mode helper function for parsing dr_mode > from the device tree. Also replace the usage of the custom > tegra_usb_phy_mode enum with the standard enum. > diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c > + tegra_phy->mode = of_usb_get_dr_mode(np); > + if (tegra_phy->mode == USB_DR_MODE_UNKNOWN) { > + dev_err(&pdev->dev, "dr_mode is invalid\n"); > + return -EINVAL; > + } Unfortunately, of_usb_get_dr_mode() returns USB_DR_MODE_UNKNOWN if the property is missing, rather than defaulting to host mode as the original code here did. I would suggest solving this by: > diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c > index 675384d..6391de5 100644 > --- a/drivers/usb/usb-common.c > +++ b/drivers/usb/usb-common.c > @@ -103,7 +103,7 @@ enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np) > > err = of_property_read_string(np, "dr_mode", &dr_mode); > if (err < 0) > - return USB_DR_MODE_UNKNOWN; > + return USB_DR_MODE_HOST; > > for (i = 0; i < ARRAY_SIZE(usb_dr_modes); i++) > if (!strcmp(dr_mode, usb_dr_modes[i])) This can't be done by the caller, since of_usb_get_dr_mode() returns UNKNOWN in two cases, which the caller can't distinguish without manually checking whether the property exists first: a) Property is not present (which should default to HOST mode at least for the Tegra binding, as in the patch I show above). b) Property is present, but contains an invalid value, which probably should cause the driver to error-out. This is only a problem for dr_mode in the Tegra binding: dr_mode is an optional property, whereas the phy_type property as parsed by patch 8/9 is mandatory, so this issue doesn't come up. -- 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/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c index 13049ce..32d4394 100644 --- a/drivers/usb/phy/phy-tegra-usb.c +++ b/drivers/usb/phy/phy-tegra-usb.c @@ -377,7 +377,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy) UTMIP_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay); writel(val, base + UTMIP_PLL_CFG1); - if (phy->mode == TEGRA_USB_PHY_MODE_DEVICE) { + if (phy->mode == USB_DR_MODE_PERIPHERAL) { val = readl(base + USB_SUSP_CTRL); val &= ~(USB_WAKE_ON_CNNT_EN_DEV | USB_WAKE_ON_DISCON_EN_DEV); writel(val, base + USB_SUSP_CTRL); @@ -412,7 +412,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy) if (phy->is_legacy_phy) { val = readl(base + UTMIP_SPARE_CFG0); - if (phy->mode == TEGRA_USB_PHY_MODE_DEVICE) + if (phy->mode == USB_DR_MODE_PERIPHERAL) val &= ~FUSE_SETUP_SEL; else val |= FUSE_SETUP_SEL; @@ -453,7 +453,7 @@ static int utmi_phy_power_off(struct tegra_usb_phy *phy) utmi_phy_clk_disable(phy); - if (phy->mode == TEGRA_USB_PHY_MODE_DEVICE) { + if (phy->mode == USB_DR_MODE_PERIPHERAL) { val = readl(base + USB_SUSP_CTRL); val &= ~USB_WAKEUP_DEBOUNCE_COUNT(~0); val |= USB_WAKE_ON_CNNT_EN_DEV | USB_WAKEUP_DEBOUNCE_COUNT(5); @@ -906,15 +906,11 @@ static int tegra_usb_phy_probe(struct platform_device *pdev) return -EINVAL; } - err = of_property_match_string(np, "dr_mode", "otg"); - if (err < 0) { - err = of_property_match_string(np, "dr_mode", "peripheral"); - if (err < 0) - tegra_phy->mode = TEGRA_USB_PHY_MODE_HOST; - else - tegra_phy->mode = TEGRA_USB_PHY_MODE_DEVICE; - } else - tegra_phy->mode = TEGRA_USB_PHY_MODE_OTG; + tegra_phy->mode = of_usb_get_dr_mode(np); + if (tegra_phy->mode == USB_DR_MODE_UNKNOWN) { + dev_err(&pdev->dev, "dr_mode is invalid\n"); + return -EINVAL; + } /* On some boards, the VBUS regulator doesn't need to be controlled */ if (of_find_property(np, "vbus-supply", NULL)) { diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h index a095c98..d3db274 100644 --- a/include/linux/usb/tegra_usb_phy.h +++ b/include/linux/usb/tegra_usb_phy.h @@ -34,12 +34,6 @@ enum tegra_usb_phy_port_speed { TEGRA_USB_PHY_PORT_SPEED_HIGH, }; -enum tegra_usb_phy_mode { - TEGRA_USB_PHY_MODE_DEVICE, - TEGRA_USB_PHY_MODE_HOST, - TEGRA_USB_PHY_MODE_OTG, -}; - struct tegra_xtal_freq; struct tegra_usb_phy { @@ -51,7 +45,7 @@ struct tegra_usb_phy { struct clk *pll_u; struct clk *pad_clk; struct regulator *vbus; - enum tegra_usb_phy_mode mode; + enum usb_dr_mode mode; void *config; struct usb_phy *ulpi; struct usb_phy u_phy;
Use the new of_usb_get_dr_mode helper function for parsing dr_mode from the device tree. Also replace the usage of the custom tegra_usb_phy_mode enum with the standard enum. Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com> --- drivers/usb/phy/phy-tegra-usb.c | 20 ++++++++------------ include/linux/usb/tegra_usb_phy.h | 8 +------- 2 files changed, 9 insertions(+), 19 deletions(-)