On 11/16/2012 12:31 PM, Felipe Balbi wrote:
> Hi,
>
> On Wed, Nov 14, 2012 at 05:19:06PM +0100, Michael Grzeschik wrote:
>> This patch makes it possible to set the connection of the usbphy to the
>> soc. It is derived from the oftree bindings for the ethernetphy and adds
>> similar helperfunctions.
>>
>> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
>> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
>> ---
>> drivers/of/Kconfig | 4 ++++
>> drivers/of/Makefile | 1 +
>> drivers/of/of_usbphy.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/of_usbphy.h | 15 ++++++++++++++
>> include/linux/usb/phy.h | 8 ++++++++
>> 5 files changed, 77 insertions(+)
>> create mode 100644 drivers/of/of_usbphy.c
>> create mode 100644 include/linux/of_usbphy.h
>>
>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>> index dfba3e6..28f99fb 100644
>> --- a/drivers/of/Kconfig
>> +++ b/drivers/of/Kconfig
>> @@ -67,6 +67,10 @@ config OF_MDIO
>> help
>> OpenFirmware MDIO bus (Ethernet PHY) accessors
>>
>> +config OF_USBPHY
>> + depends on USB
>> + def_bool y
>> +
>> config OF_PCI
>> def_tristate PCI
>> depends on PCI
>> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
>> index e027f44..fdcaf51 100644
>> --- a/drivers/of/Makefile
>> +++ b/drivers/of/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_OF_IRQ) += irq.o
>> obj-$(CONFIG_OF_DEVICE) += device.o platform.o
>> obj-$(CONFIG_OF_I2C) += of_i2c.o
>> obj-$(CONFIG_OF_NET) += of_net.o
>> +obj-$(CONFIG_OF_USBPHY) += of_usbphy.o
>> obj-$(CONFIG_OF_SELFTEST) += selftest.o
>> obj-$(CONFIG_OF_MDIO) += of_mdio.o
>> obj-$(CONFIG_OF_PCI) += of_pci.o
>> diff --git a/drivers/of/of_usbphy.c b/drivers/of/of_usbphy.c
>> new file mode 100644
>> index 0000000..2e71f7b
>> --- /dev/null
>> +++ b/drivers/of/of_usbphy.c
>> @@ -0,0 +1,49 @@
>> +/*
>> + * OF helpers for network devices.
>> + *
>> + * This file is released under the GPLv2
>> + *
>> + * Initially copied out of drivers/of/of_net.c
>> + */
>> +#include <linux/etherdevice.h>
>> +#include <linux/kernel.h>
>> +#include <linux/of_usbphy.h>
>> +#include <linux/usb/phy.h>
>> +#include <linux/export.h>
>> +
>> +/**
>> + * It maps 'enum usb_phy_interface' found in include/linux/usb/phy.h
>> + * into the device tree binding of 'phy-mode', so that USB
>> + * device driver can get phy interface from device tree.
>> + */
>
> provide proper kernel-doc
>
>> +static const char *usbphy_modes[] = {
>> + [USBPHY_INTERFACE_MODE_NA] = "",
>> + [USBPHY_INTERFACE_MODE_UTMI] = "utmi",
>> + [USBPHY_INTERFACE_MODE_UTMIW] = "utmiw",
>> + [USBPHY_INTERFACE_MODE_ULPI] = "ulpi",
>> + [USBPHY_INTERFACE_MODE_SERIAL] = "fsls",
>> +};
>
> In fact, these would be better off as constants:
>
> #define USBPHY_INTERFACE_MODE_UTMI 1
> #define USBPHY_INTERFACE_MODE_UTMIW 2
> ...
Why are defines better than an enum? BTW: this code is a copy of the
ethernet phy of-helper code.
>
>> +/**
>> + * of_get_phy_mode - Get phy mode for given device_node
>> + * @np: Pointer to the given device_node
>> + *
>> + * The function gets phy interface string from property 'phy-mode',
>> + * and return its index in phy_modes table, or errno in error case.
>> + */
>
> why do you pass a string instead of passing an Integer ? This makes no
> sense to me.
This code returns an integer or rather an enum....see header file below.
>> +const int of_get_usbphy_mode(struct device_node *np)
>> +{
>> + const char *pm;
>> + int err, i;
>> +
>> + err = of_property_read_string(np, "phy-mode", &pm);
>> + if (err < 0)
>> + return err;
>> +
>> + for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++)
>> + if (!strcasecmp(pm, usbphy_modes[i]))
>> + return i;
>> +
>> + return -ENODEV;
>> +}
>> +EXPORT_SYMBOL_GPL(of_get_usbphy_mode);
>> diff --git a/include/linux/of_usbphy.h b/include/linux/of_usbphy.h
>> new file mode 100644
>> index 0000000..9a4132d
>> --- /dev/null
>> +++ b/include/linux/of_usbphy.h
>> @@ -0,0 +1,15 @@
>> +/*
>> + * OF helpers for usb devices.
>> + *
>> + * This file is released under the GPLv2
>> + */
>> +
>> +#ifndef __LINUX_OF_USBPHY_H
>> +#define __LINUX_OF_USBPHY_H
>> +
>> +#ifdef CONFIG_OF_USBPHY
>> +#include <linux/of.h>
>> +extern const int of_get_usbphy_mode(struct device_node *np);
>> +#endif
>> +
>> +#endif /* __LINUX_OF_USBPHY_H */
>> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
>> index a29ae1e..150eb69 100644
>> --- a/include/linux/usb/phy.h
>> +++ b/include/linux/usb/phy.h
>> @@ -12,6 +12,14 @@
>> #include <linux/notifier.h>
>> #include <linux/usb.h>
>>
>> +enum usb_phy_interface {
>> + USBPHY_INTERFACE_MODE_NA,
>> + USBPHY_INTERFACE_MODE_UTMI,
>> + USBPHY_INTERFACE_MODE_UTMIW,
>> + USBPHY_INTERFACE_MODE_ULPI,
>> + USBPHY_INTERFACE_MODE_SERIAL,
>> +};
>
> no documentation at all here... What does UTMIW mean ? Why do you mean
> the NA mode ?
>
Good point, W means wide.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
Attachment:
signature.asc
Description: OpenPGP digital signature
