Hi,
On Thursday 28 November 2013 09:29 PM, Heikki Krogerus wrote:
> ULPI PHY is an USB2 PHY that is accessed from the USB
> controller. ULPI PHYs allow discovery based on vendor and
> product ids which allows binding the PHY to a driver.
>
> For USB controllers that are enumerated from buses such as
> PCI, that do not provide any information about the PHY, ULPI
> abstraction layer allows runtime detection. This makes it
> possible to take advantage of vendor specific functions of
> the PHYs with product specific drivers without the need for
> platform or device specific quirks.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
> drivers/phy/Kconfig | 2 +
> drivers/phy/Makefile | 1 +
> drivers/phy/ulpi/Kconfig | 11 ++
> drivers/phy/ulpi/Makefile | 1 +
> drivers/phy/ulpi/ulpi.c | 273 ++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/phy/ulpi.h | 105 ++++++++++++++++++
> 6 files changed, 393 insertions(+)
> create mode 100644 drivers/phy/ulpi/Kconfig
> create mode 100644 drivers/phy/ulpi/Makefile
> create mode 100644 drivers/phy/ulpi/ulpi.c
> create mode 100644 include/linux/phy/ulpi.h
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index a344f3d..6c03824 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -51,4 +51,6 @@ config PHY_EXYNOS_DP_VIDEO
> help
> Support for Display Port PHY found on Samsung EXYNOS SoCs.
>
> +source "drivers/phy/ulpi/Kconfig"
> +
> endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index d0caae9..d6af605 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
> obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
> obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
> +obj-$(CONFIG_ULPI_PHY) += ulpi/
> diff --git a/drivers/phy/ulpi/Kconfig b/drivers/phy/ulpi/Kconfig
> new file mode 100644
> index 0000000..3211aaa
> --- /dev/null
> +++ b/drivers/phy/ulpi/Kconfig
> @@ -0,0 +1,11 @@
> +
> +config ULPI_PHY
> + tristate "USB ULPI PHY interface support"
> + depends on USB || USB_GADGET
> + select GENERIC_PHY
> + help
> + Say yes if you have ULPI PHY attached to your USB controller. If no
> + vendor modules are selected, the driver will act as NOP PHY driver.
> +
> + If unsure, say Y.
> +
> diff --git a/drivers/phy/ulpi/Makefile b/drivers/phy/ulpi/Makefile
> new file mode 100644
> index 0000000..7ed0895
> --- /dev/null
> +++ b/drivers/phy/ulpi/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_ULPI_PHY) += ulpi.o
> diff --git a/drivers/phy/ulpi/ulpi.c b/drivers/phy/ulpi/ulpi.c
> new file mode 100644
> index 0000000..7aa2f5d
> --- /dev/null
> +++ b/drivers/phy/ulpi/ulpi.c
> @@ -0,0 +1,273 @@
> +/**
> + * ulpi.c - USB ULPI PHY abstraction module
> + *
> + * Copyright (C) 2013 Intel Corporation
> + *
> + * Author: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/phy/ulpi.h>
> +#include <linux/phy/phy.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +/* -------------------------------------------------------------------------- */
> +
> +static struct phy_consumer phy_consumer[] = {
> + { .port = ULPI_PORT_NAME, },
We had to introduce the entire phy_consumer stuff to support legacy pdata. Not
sure if it is a good idea to use it here.
> +};
> +
> +static struct phy_init_data phy_data = {
> + .num_consumers = 1,
> + .consumers = phy_consumer,
> +};
> +
> +static int ulpi_power_on(struct phy *phy)
> +{
> + struct ulpi_dev *ulpi = phy_get_drvdata(phy);
> + struct ulpi_driver *drv = to_ulpi_driver(ulpi->dev.driver);
> +
> + if (drv && drv->phy_ops && drv->phy_ops->power_on)
> + return drv->phy_ops->power_on(phy);
> +
> + return 0;
> +}
> +
> +static int ulpi_power_off(struct phy *phy)
> +{
> + struct ulpi_dev *ulpi = phy_get_drvdata(phy);
> + struct ulpi_driver *drv = to_ulpi_driver(ulpi->dev.driver);
> +
> + if (drv && drv->phy_ops && drv->phy_ops->power_off)
> + return drv->phy_ops->power_off(phy);
> +
> + return 0;
> +}
> +
> +static struct phy_ops phy_ops = {
> + .owner = THIS_MODULE,
> + .power_on = ulpi_power_on,
> + .power_off = ulpi_power_off,
> +};
> +
> +/* -------------------------------------------------------------------------- */
> +
> +static int ulpi_match(struct device *dev, struct device_driver *driver)
> +{
> + struct ulpi_driver *drv = to_ulpi_driver(driver);
> + struct ulpi_dev *ulpi = to_ulpi_dev(dev);
> + const struct ulpi_device_id *id;
> +
> + for (id = drv->id_table; id->vendor; id++)
> + if (id->vendor == ulpi->id.vendor &&
> + id->product == ulpi->id.product)
> + return 1;
> +
> + return 0;
> +}
> +
> +static int ulpi_probe(struct device *dev)
> +{
> + struct ulpi_driver *drv = to_ulpi_driver(dev->driver);
> +
> + return drv->probe(to_ulpi_dev(dev));
> +}
> +
> +static int ulpi_remove(struct device *dev)
> +{
> + struct ulpi_driver *drv = to_ulpi_driver(dev->driver);
> +
> + if (drv->remove)
> + drv->remove(to_ulpi_dev(dev));
> +
> + return 0;
> +}
> +
> +static struct bus_type ulpi_bus = {
> + .name = "ulpi",
> + .match = ulpi_match,
> + .probe = ulpi_probe,
> + .remove = ulpi_remove,
> +};
> +
> +/**
> + * ulpi_register_driver - register driver with the bus
> + * @drv: the driver
> + */
> +int ulpi_register_driver(struct ulpi_driver *drv)
> +{
> + if (!drv->probe)
> + return -EINVAL;
> +
> + drv->driver.bus = &ulpi_bus;
> +
> + return driver_register(&drv->driver);
> +}
> +EXPORT_SYMBOL_GPL(ulpi_register_driver);
> +
> +void ulpi_unregister_driver(struct ulpi_driver *drv)
> +{
> + driver_unregister(&drv->driver);
> +}
> +EXPORT_SYMBOL_GPL(ulpi_unregister_driver);
> +
> +/* -------------------------------------------------------------------------- */
> +
> +/**
> + * ulpi_alloc_interface - allocate new ULPI interface
> + *
> + * Allows USB controller to create struct ulpi_interface without
> + * registering the interface immediately.
> + */
> +struct ulpi_interface *ulpi_alloc_interface(void)
> +{
> + struct ulpi_dev *ulpi;
> +
> + ulpi = kzalloc(sizeof(*ulpi), GFP_KERNEL);
> + if (!ulpi)
> + return NULL;
> +
> + return &ulpi->interface;
> +}
> +EXPORT_SYMBOL_GPL(ulpi_alloc_interface);
Should this be exported?
> +
> +#define interface_to_ulpi(c) container_of(c, struct ulpi_dev, interface)
> +
> +/**
> + * ulpi_register - register new ULPI device
> + * @dev: USB controller's device interface
> + * @interface: Controller which the device is connected
> + *
> + * Companion function to ulpi_alloc_interface. Creates device for the
> + * ULPI PHY attached to the interface, binds it to a driver and
> + * creates phy instance for it.
> + */
> +int ulpi_register(struct device *dev, struct ulpi_interface *interface)
> +{
> + struct ulpi_dev *ulpi = interface_to_ulpi(interface);
> + int ret;
> +
> + /* Test the interface */
> + ret = ulpi_write(ulpi, ULPI_SCRATCH, 0xaa);
> + if (ret < 0)
> + goto err;
> +
> + ret = ulpi_read(ulpi, ULPI_SCRATCH);
> + if (ret < 0)
> + goto err;
> +
> + if (ret != 0xaa) {
> + ret = -ENODEV;
> + goto err;
> + }
> +
> + ulpi->id.vendor = ulpi_read(ulpi, ULPI_VENDOR_ID_LOW);
> + ulpi->id.vendor |= ulpi_read(ulpi, ULPI_VENDOR_ID_HIGH) << 8;
> +
> + ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
> + ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
> +
> + ulpi->dev.parent = dev;
> + ulpi->dev.bus = &ulpi_bus;
> + dev_set_name(&ulpi->dev, "%s.ulpi", dev_name(dev));
> +
> + ret = device_register(&ulpi->dev);
> + if (ret)
> + goto err;
> +
> + /**
> + * Create phy for the device.
> + * The controller is always the consumer
> + */
> + phy_consumer[0].dev_name = dev_name(dev);
> +
> + ulpi->phy = phy_create(&ulpi->dev, &phy_ops, &phy_data);
Instead of creating ulpi device and phy device here, phy device (phy_create)
should be created when the driver is probed (in this case when NXP ISP170X is
probed).
> + if (IS_ERR(ulpi->phy)) {
> + ret = PTR_ERR(ulpi->phy);
> + device_unregister(&ulpi->dev);
> + goto err;
> + }
> +
> + phy_set_drvdata(ulpi->phy, ulpi);
> +
> + dev_dbg(&ulpi->dev, "registered vendor %04x, product %04x\n",
> + ulpi->id.vendor, ulpi->id.product);
> +
> + return 0;
> +err:
> + kfree(ulpi);
It's difficult to follow if you have allocation in one function and free it in
the error path of some other function. This can be improved.
Thanks
Kishon
--
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
