On Thu, 10 Jul 2014, Peter Griffin wrote:
> This driver adds support for the USB HCD present in STi
> SoC's from STMicroelectronics. It has been tested on the
> stih416-b2020 board.
>
> Signed-off-by: Peter Griffin <peter.griffin@xxxxxxxxxx>
> ---
> drivers/usb/host/Kconfig | 17 ++
> drivers/usb/host/Makefile | 1 +
> drivers/usb/host/st-hcd.c | 471 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 489 insertions(+)
> create mode 100644 drivers/usb/host/st-hcd.c
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 61b7817..dc0358e 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -753,6 +753,23 @@ config USB_HCD_SSB
>
> If unsure, say N.
>
> +config USB_HCD_ST
> + tristate "STMicroelectronics STi family HCD support"
> + depends on ARCH_STI
> + select USB_OHCI_HCD_PLATFORM if USB_OHCI_HCD
> + select USB_EHCI_HCD_PLATFORM if USB_EHCI_HCD
> + select MFD_SYSCON
Are you still using Syscon? If not, this and the header file can be
removed.
> + select GENERIC_PHY
> + help
> + Enable support for the EHCI and OCHI host controller on ST
> + consumer electronics SoCs.
> +
> + It converts the ST driver into two platform device drivers
> + for EHCI and OHCI and provides bus configuration, clock and power
> + management for the combined hardware block.
> +
> + If unsure, say N.
> +
> config USB_HCD_TEST_MODE
> bool "HCD test mode support"
> ---help---
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index af89a90..af0b81d 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -74,3 +74,4 @@ obj-$(CONFIG_USB_HCD_SSB) += ssb-hcd.o
> obj-$(CONFIG_USB_FUSBH200_HCD) += fusbh200-hcd.o
> obj-$(CONFIG_USB_FOTG210_HCD) += fotg210-hcd.o
> obj-$(CONFIG_USB_MAX3421_HCD) += max3421-hcd.o
> +obj-$(CONFIG_USB_HCD_ST) += st-hcd.o
> diff --git a/drivers/usb/host/st-hcd.c b/drivers/usb/host/st-hcd.c
> new file mode 100644
> index 0000000..8a9636c
> --- /dev/null
> +++ b/drivers/usb/host/st-hcd.c
> @@ -0,0 +1,471 @@
> +/*
> + * STMicroelectronics HCD (Host Controller Driver) for USB 2.0 and 1.1.
> + *
> + * Copyright (c) 2013 STMicroelectronics (R&D) Ltd.
> + * Authors: Stephen Gallimore <stephen.gallimore@xxxxxx>
> + * Peter Griffin <peter.griffin@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/of.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pm_clock.h>
> +#include <linux/delay.h>
> +#include <linux/usb.h>
> +#include <linux/usb/hcd.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/reset.h>
> +#include <linux/regmap.h>
Is this used?
> +#include <linux/phy/phy.h>
> +#include <linux/mfd/syscon.h>
Is this used?
> +#include <linux/usb/ohci_pdriver.h>
> +#include <linux/usb/ehci_pdriver.h>
> +
> +#include "ohci.h"
> +
> +#define EHCI_CAPS_SIZE 0x10
> +#define AHB2STBUS_INSREG01 (EHCI_CAPS_SIZE + 0x84)
> +
> +struct st_hcd_dev {
> + int port_nr;
> + struct platform_device *ehci_device;
> + struct platform_device *ohci_device;
> + struct clk *ic_clk; /* Interconnect clock to the controller block */
> + struct clk *ohci_clk; /* 48MHz clock for OHCI */
> + struct reset_control *pwr;
> + struct reset_control *rst;
> + struct phy *phy;
> +};
As there are comments, consider using kerneldoc instead.
> +static inline void st_ehci_configure_bus(void __iomem *regs)
> +{
> + /* Set EHCI packet buffer IN/OUT threshold to 128 bytes */
> + u32 threshold = 128 | (128 << 16);
> +
> + writel(threshold, regs + AHB2STBUS_INSREG01);
> +}
> +
> +static int st_hcd_enable_clocks(struct device *dev,
> + struct st_hcd_dev *hcd_dev)
> +{
> + int err;
Separate code (and comments) from declaration.
> + /*
> + * The interconnect input clock have either fixed
s/have either/has either a/
> + * rate or the rate is defined on boot, so we are only concerned about
> + * enabling any gates for this clock.
> + */
> + err = clk_prepare_enable(hcd_dev->ic_clk);
> + if (err) {
> + dev_err(dev, "can't enable ic clock\n");
> + return err;
> + }
Nit: '\n'
> + /*
> + * The 48MHz OHCI clock is usually provided by a programmable
> + * frequency synthesizer, which is often not programmed on boot/chip
> + * reset, so we set its rate here to ensure it is correct.
> + *
> + * However not all SoC's have a dedicated ohci clock so it isn't fatal
s/ohci/OHCI
> + * for this not to exist.
--^
> + */
> + if (!IS_ERR(hcd_dev->ohci_clk)) {
This is ugly. If it's not found NULL it, then check for:
if (hcd_dev->ohci_clk) {
Or, even better, do:
if (hcd_dev->ohci_clk)
return 0;
Then de-indent this:
> + err = clk_set_rate(hcd_dev->ohci_clk, 48000000);
> + if (err) {
> + dev_err(dev, "can't set ohci_clk rate\n");
> + goto error;
> + }
> +
> + err = clk_prepare_enable(hcd_dev->ohci_clk);
> + if (err) {
> + dev_err(dev, "can't enable ohci_clk\n");
> + goto error;
> + }
> + }
> +
> + return 0;
> +error:
> + clk_disable_unprepare(hcd_dev->ic_clk);
> + return err;
> +}
> +
> +
Remove the superfluous '\n'.
> +static void st_hcd_disable_clocks(struct st_hcd_dev *hcd_dev)
> +{
> + /* not all SoCs provide a dedicated ohci_clk */
Really small nit:
All but two of the comments in this file start with uppercase.
> + if (!IS_ERR(hcd_dev->ohci_clk))
You don't need to worry about this here. The clk framework already
does this check for you, so you can omit it.
> + clk_disable_unprepare(hcd_dev->ohci_clk);
> +
> + clk_disable_unprepare(hcd_dev->ic_clk);
> +}
> +
> +static void st_hcd_assert_resets(struct device *dev,
> + struct st_hcd_dev *hcd_dev)
> +{
> + int err;
> +
> + err = reset_control_assert(hcd_dev->pwr);
> + if (err)
> + dev_err(dev, "unable to put into powerdown\n");
> +
> + err = reset_control_assert(hcd_dev->rst);
> + if (err)
> + dev_err(dev, "unable to put into softreset\n");
> +}
> +
> +static int st_hcd_deassert_resets(struct device *dev,
> + struct st_hcd_dev *hcd_dev)
> +{
> + int err;
> +
> + err = reset_control_deassert(hcd_dev->pwr);
> + if (err) {
> + dev_err(dev, "unable to bring out of powerdown\n");
> + return err;
> + }
> +
> + err = reset_control_deassert(hcd_dev->rst);
> + if (err) {
> + dev_err(dev, "unable to bring out of softreset\n");
> + goto err_assert_power;
> + }
> +
> + return 0;
> +
> +err_assert_power:
> + reset_control_assert(hcd_dev->pwr);
I would put this in reset_control_deassert()'s error path and rid the
goto.
> + return err;
> +}
> +
> +static const struct usb_ehci_pdata ehci_pdata = {
> +};
> +
> +static const struct usb_ohci_pdata ohci_pdata = {
> +};
> +
> +static int st_hcd_remove(struct platform_device *pdev)
.remove() usually goes below (or at least near) .probe().
> +{
> + struct st_hcd_dev *hcd_dev = platform_get_drvdata(pdev);
> +
> + platform_device_unregister(hcd_dev->ehci_device);
> + platform_device_unregister(hcd_dev->ohci_device);
> +
> + phy_power_off(hcd_dev->phy);
> +
> + phy_exit(hcd_dev->phy);
> +
> + st_hcd_assert_resets(&pdev->dev, hcd_dev);
> +
> + st_hcd_disable_clocks(hcd_dev);
> +
> + return 0;
> +}
> +
> +static struct platform_device *st_hcd_device_create(const char *name, int id,
> + struct platform_device *parent)
> +{
> + struct platform_device *pdev;
> + const char *platform_name;
> + struct resource *res;
> + struct resource hcd_res[2];
> + int ret;
> +
> + res = platform_get_resource_byname(parent, IORESOURCE_MEM, name);
> + if (!res)
> + return ERR_PTR(-ENODEV);
> +
> + hcd_res[0] = *res;
> +
> + res = platform_get_resource_byname(parent, IORESOURCE_IRQ, name);
> + if (!res)
> + return ERR_PTR(-ENODEV);
> +
> + hcd_res[1] = *res;
> +
> + platform_name = kasprintf(GFP_KERNEL, "%s-platform", name);
> + if (!platform_name)
> + return ERR_PTR(-ENOMEM);
> +
> + pdev = platform_device_alloc(platform_name, id);
> +
> + kfree(platform_name);
> +
> + if (!pdev)
> + return ERR_PTR(-ENOMEM);
> +
> + pdev->dev.parent = &parent->dev;
> + pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> + pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> +
> + ret = platform_device_add_resources(pdev, hcd_res, ARRAY_SIZE(hcd_res));
> + if (ret)
> + goto error;
> +
> + if (!strcmp(name, "ohci"))
> + ret = platform_device_add_data(pdev, &ohci_pdata,
> + sizeof(ohci_pdata));
> + else
> + ret = platform_device_add_data(pdev, &ehci_pdata,
> + sizeof(ehci_pdata));
> +
> + if (ret)
> + goto error;
> +
> + ret = platform_device_add(pdev);
> + if (ret)
> + goto error;
> +
> + return pdev;
> +
> +error:
> + platform_device_put(pdev);
> + return ERR_PTR(ret);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int st_hcd_resume(struct device *dev)
> +{
> + struct st_hcd_dev *hcd_dev = dev_get_drvdata(dev);
> + struct usb_hcd *ehci_hcd = platform_get_drvdata(hcd_dev->ehci_device);
> + int err;
> +
> + pinctrl_pm_select_default_state(dev);
> +
> + err = st_hcd_enable_clocks(dev, hcd_dev);
> + if (err)
> + return err;
> +
> + err = phy_init(hcd_dev->phy);
> + if (err) {
> + dev_err(dev, "phy initialization failed\n");
> + goto err_disable_clocks;
> + }
> +
> + err = phy_power_on(hcd_dev->phy);
> + if (err && (err != -ENOTSUPP)) {
> + dev_err(dev, "phy power on failed\n");
> + goto err_phy_exit;
> + }
> +
> + err = st_hcd_deassert_resets(dev, hcd_dev);
> + if (err)
> + goto err_phy_off;
> +
> + st_ehci_configure_bus(ehci_hcd->regs);
> +
> + return 0;
> +
> +err_phy_off:
> + phy_power_off(hcd_dev->phy);
> +err_phy_exit:
> + phy_exit(hcd_dev->phy);
> +err_disable_clocks:
> + st_hcd_disable_clocks(hcd_dev);
> +
> + return err;
> +}
> +
> +static int st_hcd_suspend(struct device *dev)
> +{
> + struct st_hcd_dev *hcd_dev = dev_get_drvdata(dev);
> + int err;
> +
> + err = reset_control_assert(hcd_dev->pwr);
> + if (err) {
> + dev_err(dev, "unable to put into powerdown\n");
> + return err;
> + }
> +
> + err = reset_control_assert(hcd_dev->rst);
> + if (err) {
> + dev_err(dev, "unable to put into softreset\n");
> + return err;
> + }
> +
> + err = phy_power_off(hcd_dev->phy);
> + if (err && (err != -ENOTSUPP)) {
> + dev_err(dev, "phy power off failed\n");
> + return err;
> + }
> +
> + phy_exit(hcd_dev->phy);
> +
> + st_hcd_disable_clocks(hcd_dev);
> +
> + pinctrl_pm_select_sleep_state(dev);
> +
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(st_hcd_pm, st_hcd_suspend, st_hcd_resume);
> +
> +static struct of_device_id st_hcd_match[] = {
const?
> + { .compatible = "st,usb-300x" },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, st_hcd_match);
Why is this all the way up here? As you're not using
of_match_device() you can place this down by the platform driver
struct.
> +static int st_hcd_probe_clocks(struct device *dev,
> + struct st_hcd_dev *hcd_dev)
> +{
> + hcd_dev->ic_clk = devm_clk_get(dev, "ic");
> + if (IS_ERR(hcd_dev->ic_clk)) {
> + dev_err(dev, "ic clk not found\n");
> + return PTR_ERR(hcd_dev->ic_clk);
> + }
> +
> + /* some SoCs don't have a dedicated ohci clk */
Really small nit:
All but two of the comments in this file start with uppercase.
> + hcd_dev->ohci_clk = devm_clk_get(dev, "ohci");
> + if (IS_ERR(hcd_dev->ohci_clk))
> + dev_info(dev, "48MHz ohci clk not found\n");
s/ohci/OHCI
Also just be aware that some maintainers like s/clk/clock in error
messages.
> + return st_hcd_enable_clocks(dev, hcd_dev);
> +}
> +
> +
--^
> +static int st_hcd_probe_resets(struct device *dev,
> + struct st_hcd_dev *hcd_dev)
> +{
> + hcd_dev->pwr = devm_reset_control_get(dev, "power");
> + if (IS_ERR(hcd_dev->pwr)) {
> + dev_err(dev, "power reset control not found\n");
> + return PTR_ERR(hcd_dev->pwr);
> + }
> +
> + hcd_dev->rst = devm_reset_control_get(dev, "softreset");
> + if (IS_ERR(hcd_dev->rst)) {
> + dev_err(dev, "soft reset control not found\n");
> + return PTR_ERR(hcd_dev->rst);
> + }
> +
> + return st_hcd_deassert_resets(dev, hcd_dev);
> +}
> +
> +static int st_hcd_probe_ehci_setup(struct platform_device *pdev)
> +{
> + struct resource *res;
> + void __iomem *ehci_regs;
> +
> + /*
> + * We need to do some integration specific setup in the EHCI
> + * controller, which the EHCI platform driver does not provide any
> + * hooks to allow us to do during it's initialisation.
Nit: "Its" can not be expressed as possessive.
> + */
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ehci");
> + if (!res)
> + return -ENODEV;
> +
> + ehci_regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
Use devm_ioremap_resource()
> + if (!ehci_regs)
> + return -ENOMEM;
> +
> + st_ehci_configure_bus(ehci_regs);
> + devm_iounmap(&pdev->dev, ehci_regs);
> +
> + return 0;
> +}
> +
> +static int st_hcd_probe(struct platform_device *pdev)
> +{
> + struct st_hcd_dev *hcd_dev;
> + int id;
> + int err;
> +
> + if (!pdev->dev.of_node)
> + return -ENODEV;
No need for this if you depend on OF.
> + id = of_alias_get_id(pdev->dev.of_node, "usb");
> +
Remove the '\n' between get and check.
> + if (id < 0) {
> + dev_err(&pdev->dev, "No ID specified via DT alias\n");
> + return -ENODEV;
> + }
> +
> + hcd_dev = devm_kzalloc(&pdev->dev, sizeof(*hcd_dev), GFP_KERNEL);
> + if (!hcd_dev)
> + return -ENOMEM;
> +
> + hcd_dev->port_nr = id;
> +
> + err = st_hcd_probe_clocks(&pdev->dev, hcd_dev);
> + if (err)
> + return err;
> +
> + err = st_hcd_probe_resets(&pdev->dev, hcd_dev);
> + if (err)
> + goto err_disable_clocks;
> +
> + err = st_hcd_probe_ehci_setup(pdev);
> + if (err)
> + goto err_assert_resets;
> +
> + hcd_dev->phy = devm_phy_get(&pdev->dev, "usb2-phy");
> + if (IS_ERR(hcd_dev->phy)) {
> + dev_err(&pdev->dev, "no PHY configured\n");
> + err = PTR_ERR(hcd_dev->phy);
> + goto err_assert_resets;
> + }
> +
> + err = phy_init(hcd_dev->phy);
> + if (err) {
> + dev_err(&pdev->dev, "phy initialization failed\n");
> + goto err_assert_resets;
> + }
> +
> + err = phy_power_on(hcd_dev->phy);
> + if (err && (err != -ENOTSUPP)) {
> + dev_err(&pdev->dev, "phy power on failed\n");
> + goto err_phy_exit;
> + }
> +
> + hcd_dev->ehci_device = st_hcd_device_create("ehci", id, pdev);
> + if (IS_ERR(hcd_dev->ehci_device)) {
> + err = PTR_ERR(hcd_dev->ehci_device);
> + goto err_phy_off;
> + }
> +
> + hcd_dev->ohci_device = st_hcd_device_create("ohci", id, pdev);
> + if (IS_ERR(hcd_dev->ohci_device)) {
> + platform_device_del(hcd_dev->ehci_device);
Why do you break convention here? Place this down in amongst the
gotos.
> + err = PTR_ERR(hcd_dev->ohci_device);
> + goto err_phy_off;
> + }
> +
> + platform_set_drvdata(pdev, hcd_dev);
> +
> + return 0;
> +
> +err_phy_off:
> + phy_power_off(hcd_dev->phy);
> +err_phy_exit:
> + phy_exit(hcd_dev->phy);
> +err_assert_resets:
> + st_hcd_assert_resets(&pdev->dev, hcd_dev);
> +err_disable_clocks:
> + st_hcd_disable_clocks(hcd_dev);
> +
> + return err;
> +}
> +
> +static struct platform_driver st_hcd_driver = {
> + .probe = st_hcd_probe,
> + .remove = st_hcd_remove,
> + .driver = {
> + .name = "st-hcd",
> + .pm = &st_hcd_pm,
> + .of_match_table = st_hcd_match,
> + },
> +};
> +
> +module_platform_driver(st_hcd_driver);
> +
> +MODULE_DESCRIPTION("STMicroelectronics On-Chip USB Host Controller");
> +MODULE_AUTHOR("Stephen Gallimore <stephen.gallimore@xxxxxx>");
> +MODULE_LICENSE("GPL v2");
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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
