| Message ID | 1389198608-24339-2-git-send-email-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello, 2014/1/8 Hans de Goede <hdegoede@redhat.com>: > Add support for ohci-platform instantiation from devicetree, including > optionally getting clks and a phy from devicetree, and enabling / disabling > those on power_on / off. > > This should allow using ohci-platform from devicetree in various cases. > Specifically after this commit it can be used for the ohci controller found > on Allwinner sunxi SoCs. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > .../devicetree/bindings/usb/platform-ohci.txt | 25 ++++ > drivers/usb/host/ohci-platform.c | 148 ++++++++++++++++++--- > 2 files changed, 153 insertions(+), 20 deletions(-) > create mode 100644 Documentation/devicetree/bindings/usb/platform-ohci.txt > > diff --git a/Documentation/devicetree/bindings/usb/platform-ohci.txt b/Documentation/devicetree/bindings/usb/platform-ohci.txt > new file mode 100644 > index 0000000..44bfa57 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/platform-ohci.txt ohci-mmio might be a better name than platform maybe? [snip] > +static int ohci_platform_power_on(struct platform_device *dev) > +{ > + struct usb_hcd *hcd = platform_get_drvdata(dev); > + struct ohci_platform_priv *priv = > + (struct ohci_platform_priv *)hcd_to_ohci(hcd)->priv; > + int clk, ret; > + > + for (clk = 0; priv->clks[clk] && clk < OHCI_MAX_CLKS; clk++) { > + ret = clk_prepare_enable(priv->clks[clk]); > + if (ret) > + goto err_disable_clks; > + } Do we really need to cap this to OHCI_MAX_CLKS, the next driver which has 4 clocks will have to bump this, so maybe a linked-list would be best? > + > + if (priv->phy) { > + ret = phy_init(priv->phy); > + if (ret) > + goto err_disable_clks; > + > + ret = phy_power_on(priv->phy); > + if (ret) > + goto err_exit_phy; > + } Although I do value the idea of having DT probing for ohci-platform, ehci-platform et al, I am not sure if this really belongs in the generic OHCI platform driver, or if we should rather have small glue drivers which register the ohci-platform driver and which provides all the platform-specific power_on, power_off, suspend callbacks to the ohci platform driver? Such glue driver would be the one which gets probed based off a specific compatible string a At first glance it does look like this covers 80% of the existing OHCI drivers out there, so this is good. We might also want to support clock pointers provided via platform_data so we can remove ohci-at91, ohci-ep93xx, ohci-spear and friends in the future. ohci-ppc-of could also probably be removed once we add quirks and endian properties parsing to ohci-platform? > + > + return 0; > + > +err_exit_phy: > + phy_exit(priv->phy); > +err_disable_clks: > + while (--clk >= 0) > + clk_disable_unprepare(priv->clks[clk]); > + > + return ret; > +} > + > +static void ohci_platform_power_off(struct platform_device *dev) > +{ > + struct usb_hcd *hcd = platform_get_drvdata(dev); > + struct ohci_platform_priv *priv = > + (struct ohci_platform_priv *)hcd_to_ohci(hcd)->priv; > + int clk; > + > + if (priv->phy) { > + phy_power_off(priv->phy); > + phy_exit(priv->phy); > + } > + > + for (clk = OHCI_MAX_CLKS - 1; clk >= 0; clk--) > + if (priv->clks[clk]) > + clk_disable_unprepare(priv->clks[clk]); > +} > + > static struct hc_driver __read_mostly ohci_platform_hc_driver; > > static const struct ohci_driver_overrides platform_overrides __initconst = { > - .product_desc = "Generic Platform OHCI controller", > - .reset = ohci_platform_reset, > + .product_desc = "Generic Platform OHCI controller", > + .reset = ohci_platform_reset, > + .extra_priv_size = sizeof(struct ohci_platform_priv), > +}; > + > +static struct usb_ohci_pdata ohci_platform_defaults = { > + .power_on = ohci_platform_power_on, > + .power_suspend = ohci_platform_power_off, > + .power_off = ohci_platform_power_off, > }; > > static int ohci_platform_probe(struct platform_device *dev) > @@ -60,12 +128,24 @@ static int ohci_platform_probe(struct platform_device *dev) > struct usb_hcd *hcd; > struct resource *res_mem; > struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev); > - int irq; > - int err = -ENOMEM; > + int clk, irq, err; > + char name[8]; > > + /* > + * Use reasonable defaults so platforms don't have to provide these > + * with DT probing on ARM. > + */ > if (!pdata) { > - WARN_ON(1); > - return -ENODEV; > + pdata = dev->dev.platform_data = &ohci_platform_defaults; > + /* > + * Right now device-tree probed devices don't get dma_mask set. > + * Since shared usb code relies on it, set it here for now. > + * Once we have dma capability bindings this can go away. > + */ > + err = dma_coerce_mask_and_coherent(&dev->dev, > + DMA_BIT_MASK(32)); > + if (err) > + return err; > } > > if (usb_disabled()) > @@ -83,17 +163,40 @@ static int ohci_platform_probe(struct platform_device *dev) > return -ENXIO; > } > > + hcd = usb_create_hcd(&ohci_platform_hc_driver, &dev->dev, > + dev_name(&dev->dev)); > + if (!hcd) > + return -ENOMEM; > + > + if (pdata == &ohci_platform_defaults) { > + struct ohci_platform_priv *priv = (struct ohci_platform_priv *) > + hcd_to_ohci(hcd)->priv; > + > + priv->phy = devm_phy_get(&dev->dev, "phy0"); Does this work with a more than one OHCI controller which could possibly have a dedicated PHY? > + if (IS_ERR(priv->phy)) { > + err = PTR_ERR(priv->phy); > + if (err == -EPROBE_DEFER) > + goto err_put_hcd; > + priv->phy = NULL; > + }
Hi, On 01/08/2014 09:36 PM, Florian Fainelli wrote: > Hello, > > 2014/1/8 Hans de Goede <hdegoede@redhat.com>: >> Add support for ohci-platform instantiation from devicetree, including >> optionally getting clks and a phy from devicetree, and enabling / disabling >> those on power_on / off. >> >> This should allow using ohci-platform from devicetree in various cases. >> Specifically after this commit it can be used for the ohci controller found >> on Allwinner sunxi SoCs. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> .../devicetree/bindings/usb/platform-ohci.txt | 25 ++++ >> drivers/usb/host/ohci-platform.c | 148 ++++++++++++++++++--- >> 2 files changed, 153 insertions(+), 20 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/usb/platform-ohci.txt >> >> diff --git a/Documentation/devicetree/bindings/usb/platform-ohci.txt b/Documentation/devicetree/bindings/usb/platform-ohci.txt >> new file mode 100644 >> index 0000000..44bfa57 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/usb/platform-ohci.txt > > ohci-mmio might be a better name than platform maybe? We already have platform-uhci too, and this matches the name of the .c file so that people can actually correlate the 2. > > [snip] > >> +static int ohci_platform_power_on(struct platform_device *dev) >> +{ >> + struct usb_hcd *hcd = platform_get_drvdata(dev); >> + struct ohci_platform_priv *priv = >> + (struct ohci_platform_priv *)hcd_to_ohci(hcd)->priv; >> + int clk, ret; >> + >> + for (clk = 0; priv->clks[clk] && clk < OHCI_MAX_CLKS; clk++) { >> + ret = clk_prepare_enable(priv->clks[clk]); >> + if (ret) >> + goto err_disable_clks; >> + } > > Do we really need to cap this to OHCI_MAX_CLKS, the next driver which > has 4 clocks will have to bump this, so maybe a linked-list would be > best? I can make OHCI_MAX_CLKS 4 or 8 if that makes you happy, other then that I greatly prefer to keep this KISS. Devices with 3 clks are already quite rare, so its not like we're going to need to bump this every other week a linked list where a simple array suffices is just overkill and unnecessarily complicates things. > >> + >> + if (priv->phy) { >> + ret = phy_init(priv->phy); >> + if (ret) >> + goto err_disable_clks; >> + >> + ret = phy_power_on(priv->phy); >> + if (ret) >> + goto err_exit_phy; >> + } > > Although I do value the idea of having DT probing for ohci-platform, > ehci-platform et al, I am not sure if this really belongs in the > generic OHCI platform driver, or if we should rather have small glue > drivers which register the ohci-platform driver and which provides all > the platform-specific power_on, power_off, suspend callbacks to the > ohci platform driver? Such glue driver would be the one which gets > probed based off a specific compatible string a This can already be done by having a platform driver with its own compatible string providing platform data with the hooks. The whole idea here is to avoid having to write such a glue driver for every other soc, so cover the common simple case, and use glue drivers for more complicated scenarios. > At first glance it does look like this covers 80% of the existing OHCI > drivers out there, so this is good. We might also want to support > clock pointers provided via platform_data so we can remove ohci-at91, > ohci-ep93xx, ohci-spear and friends in the future. Lets first get the generic devicetree support merged as is, if after that people want to extend it, so that (more) existing ohci drivers can be removed, they are very much welcome to do so. Preferably people with access to the hardware, so that they can actually test the changes. > ohci-ppc-of could also probably be removed once we add quirks and > endian properties parsing to ohci-platform? Add-on patches extending my initial work are welcome. Again probably best done by people who actually have access to the relevant hw. Regards, Hans
diff --git a/Documentation/devicetree/bindings/usb/platform-ohci.txt b/Documentation/devicetree/bindings/usb/platform-ohci.txt new file mode 100644 index 0000000..44bfa57 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/platform-ohci.txt @@ -0,0 +1,25 @@ +Generic Platform OHCI controller + +Required properties: +- compatible : "allwinner,sun4i-ohci" +- reg : ohci controller register range (address and length) +- interrupts : ohci controller interrupt + +Optional properties: +- clocks : a list of phandle + clock specifier pairs, one for each entry + in clock-names. +- clock-names : "clk0", "clk1", ... +- phys : phy +- phy-names : "phy0" + +Example: + + ohci0: ohci@0x01c14400 { + compatible = "allwinner,sun4i-ohci"; + reg = <0x01c14400 0x100>; + interrupts = <64>; + clocks = <&usb_clk 6>, <&ahb_gates 2>; + clock-names = "clk0", "clk1"; + phys = <&usbphy 1>; + phy-names = "phy0"; + }; diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c index f351ff5..7cab768 100644 --- a/drivers/usb/host/ohci-platform.c +++ b/drivers/usb/host/ohci-platform.c @@ -3,6 +3,7 @@ * * Copyright 2007 Michael Buesch <m@bues.ch> * Copyright 2011-2012 Hauke Mehrtens <hauke@hauke-m.de> + * Copyright 2014 Hans de Goede <hdegoede@redhat.com> * * Derived from the OCHI-SSB driver * Derived from the OHCI-PCI driver @@ -14,11 +15,14 @@ * Licensed under the GNU/GPL. See COPYING for details. */ +#include <linux/clk.h> +#include <linux/dma-mapping.h> #include <linux/hrtimer.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/err.h> +#include <linux/phy/phy.h> #include <linux/platform_device.h> #include <linux/usb/ohci_pdriver.h> #include <linux/usb.h> @@ -28,6 +32,12 @@ #define DRIVER_DESC "OHCI generic platform driver" +#define OHCI_MAX_CLKS 3 +struct ohci_platform_priv { + struct clk *clks[OHCI_MAX_CLKS]; + struct phy *phy; +}; + static const char hcd_name[] = "ohci-platform"; static int ohci_platform_reset(struct usb_hcd *hcd) @@ -48,11 +58,69 @@ static int ohci_platform_reset(struct usb_hcd *hcd) return ohci_setup(hcd); } +static int ohci_platform_power_on(struct platform_device *dev) +{ + struct usb_hcd *hcd = platform_get_drvdata(dev); + struct ohci_platform_priv *priv = + (struct ohci_platform_priv *)hcd_to_ohci(hcd)->priv; + int clk, ret; + + for (clk = 0; priv->clks[clk] && clk < OHCI_MAX_CLKS; clk++) { + ret = clk_prepare_enable(priv->clks[clk]); + if (ret) + goto err_disable_clks; + } + + if (priv->phy) { + ret = phy_init(priv->phy); + if (ret) + goto err_disable_clks; + + ret = phy_power_on(priv->phy); + if (ret) + goto err_exit_phy; + } + + return 0; + +err_exit_phy: + phy_exit(priv->phy); +err_disable_clks: + while (--clk >= 0) + clk_disable_unprepare(priv->clks[clk]); + + return ret; +} + +static void ohci_platform_power_off(struct platform_device *dev) +{ + struct usb_hcd *hcd = platform_get_drvdata(dev); + struct ohci_platform_priv *priv = + (struct ohci_platform_priv *)hcd_to_ohci(hcd)->priv; + int clk; + + if (priv->phy) { + phy_power_off(priv->phy); + phy_exit(priv->phy); + } + + for (clk = OHCI_MAX_CLKS - 1; clk >= 0; clk--) + if (priv->clks[clk]) + clk_disable_unprepare(priv->clks[clk]); +} + static struct hc_driver __read_mostly ohci_platform_hc_driver; static const struct ohci_driver_overrides platform_overrides __initconst = { - .product_desc = "Generic Platform OHCI controller", - .reset = ohci_platform_reset, + .product_desc = "Generic Platform OHCI controller", + .reset = ohci_platform_reset, + .extra_priv_size = sizeof(struct ohci_platform_priv), +}; + +static struct usb_ohci_pdata ohci_platform_defaults = { + .power_on = ohci_platform_power_on, + .power_suspend = ohci_platform_power_off, + .power_off = ohci_platform_power_off, }; static int ohci_platform_probe(struct platform_device *dev) @@ -60,12 +128,24 @@ static int ohci_platform_probe(struct platform_device *dev) struct usb_hcd *hcd; struct resource *res_mem; struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev); - int irq; - int err = -ENOMEM; + int clk, irq, err; + char name[8]; + /* + * Use reasonable defaults so platforms don't have to provide these + * with DT probing on ARM. + */ if (!pdata) { - WARN_ON(1); - return -ENODEV; + pdata = dev->dev.platform_data = &ohci_platform_defaults; + /* + * Right now device-tree probed devices don't get dma_mask set. + * Since shared usb code relies on it, set it here for now. + * Once we have dma capability bindings this can go away. + */ + err = dma_coerce_mask_and_coherent(&dev->dev, + DMA_BIT_MASK(32)); + if (err) + return err; } if (usb_disabled()) @@ -83,17 +163,40 @@ static int ohci_platform_probe(struct platform_device *dev) return -ENXIO; } + hcd = usb_create_hcd(&ohci_platform_hc_driver, &dev->dev, + dev_name(&dev->dev)); + if (!hcd) + return -ENOMEM; + + if (pdata == &ohci_platform_defaults) { + struct ohci_platform_priv *priv = (struct ohci_platform_priv *) + hcd_to_ohci(hcd)->priv; + + priv->phy = devm_phy_get(&dev->dev, "phy0"); + if (IS_ERR(priv->phy)) { + err = PTR_ERR(priv->phy); + if (err == -EPROBE_DEFER) + goto err_put_hcd; + priv->phy = NULL; + } + + for (clk = 0; clk < OHCI_MAX_CLKS; clk++) { + snprintf(name, sizeof(name), "clk%d", clk); + priv->clks[clk] = devm_clk_get(&dev->dev, name); + if (IS_ERR(priv->clks[clk])) { + err = PTR_ERR(priv->clks[clk]); + if (err == -EPROBE_DEFER) + goto err_put_hcd; + priv->clks[clk] = NULL; + } + } + } + + platform_set_drvdata(dev, hcd); if (pdata->power_on) { err = pdata->power_on(dev); if (err < 0) - return err; - } - - hcd = usb_create_hcd(&ohci_platform_hc_driver, &dev->dev, - dev_name(&dev->dev)); - if (!hcd) { - err = -ENOMEM; - goto err_power; + goto err_put_hcd; } hcd->rsrc_start = res_mem->start; @@ -102,21 +205,19 @@ static int ohci_platform_probe(struct platform_device *dev) hcd->regs = devm_ioremap_resource(&dev->dev, res_mem); if (IS_ERR(hcd->regs)) { err = PTR_ERR(hcd->regs); - goto err_put_hcd; + goto err_power; } err = usb_add_hcd(hcd, irq, IRQF_SHARED); if (err) - goto err_put_hcd; - - platform_set_drvdata(dev, hcd); + goto err_power; return err; -err_put_hcd: - usb_put_hcd(hcd); err_power: if (pdata->power_off) pdata->power_off(dev); +err_put_hcd: + usb_put_hcd(hcd); return err; } @@ -178,6 +279,12 @@ static int ohci_platform_resume(struct device *dev) #define ohci_platform_resume NULL #endif /* CONFIG_PM */ +static const struct of_device_id ohci_platform_ids[] = { + { .compatible = "allwinner,sun4i-ohci", }, + { } +}; +MODULE_DEVICE_TABLE(of, ohci_platform_ids); + static const struct platform_device_id ohci_platform_table[] = { { "ohci-platform", 0 }, { } @@ -198,6 +305,7 @@ static struct platform_driver ohci_platform_driver = { .owner = THIS_MODULE, .name = "ohci-platform", .pm = &ohci_platform_pm_ops, + .of_match_table = ohci_platform_ids, } };
Add support for ohci-platform instantiation from devicetree, including optionally getting clks and a phy from devicetree, and enabling / disabling those on power_on / off. This should allow using ohci-platform from devicetree in various cases. Specifically after this commit it can be used for the ohci controller found on Allwinner sunxi SoCs. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- .../devicetree/bindings/usb/platform-ohci.txt | 25 ++++ drivers/usb/host/ohci-platform.c | 148 ++++++++++++++++++--- 2 files changed, 153 insertions(+), 20 deletions(-) create mode 100644 Documentation/devicetree/bindings/usb/platform-ohci.txt