Re: [RFC PATCH] drivers: phy: add generic PHY framework
Hi, On Mon, Sep 17, 2012 at 3:03 PM, Marc Kleine-Budde m...@pengutronix.de wrote: On 09/14/2012 03:06 PM, ABRAHAM, KISHON VIJAY wrote: [...] diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c new file mode 100644 index 000..c55446a --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,437 @@ +/* + * phy-core.c -- Generic Phy framework. + * + * Copyright (C) 2012 Texas Instruments + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ + +#include linux/kernel.h +#include linux/export.h +#include linux/module.h +#include linux/err.h +#include linux/device.h +#include linux/slab.h +#include linux/of.h +#include linux/phy/phy.h + +static struct class *phy_class; +static LIST_HEAD(phy_list); +static DEFINE_MUTEX(phy_list_mutex); +static LIST_HEAD(phy_bind_list); + +static void devm_phy_release(struct device *dev, void *res) +{ + struct phy *phy = *(struct phy **)res; What about adding a struct phy_res, doing so,m you don't need these casts, and it's easier to add more pointers if needed. Wont we still need to do the cast since you get only a void pointer. Maybe I'm not getting you. As res is a void pointer, no need to hast to to a struct phy_res pointer, if you think that's unclean code, you can still cast it. But IMHO the code is far more readable. + + phy_put(phy); +} + +static int devm_phy_match(struct device *dev, void *res, void *match_data) +{ + return res == match_data; +} + +static struct phy *phy_lookup(struct device *dev, u8 index) +{ + struct phy_bind *phy_bind = NULL; + + list_for_each_entry(phy_bind, phy_bind_list, list) { + if (!(strcmp(phy_bind-dev_name, dev_name(dev))) + phy_bind-index == index) + return phy_bind-phy; + } + + return ERR_PTR(-ENODEV); +} + +static struct phy *of_phy_lookup(struct device *dev, struct device_node *node) +{ + int index = 0; + struct phy *phy; ^^ Please remove that stray space. Sure. + struct phy_bind *phy_map = NULL; + + list_for_each_entry(phy_map, phy_bind_list, list) + if (!(strcmp(phy_map-dev_name, dev_name(dev + index++; + + list_for_each_entry(phy, phy_list, head) { + if (node != phy-desc-of_node) + continue; + + phy_map = phy_bind(dev_name(dev), index, dev_name(phy-dev)); + if (!IS_ERR(phy_map)) { + phy_map-phy = phy; + phy_map-auto_bind = true; + } + + return phy; + } + + return ERR_PTR(-ENODEV); +} + +/** + * devm_phy_get - lookup and obtain a reference to a phy. + * @dev: device that requests this phy + * @index: the index of the phy + * + * Gets the phy using phy_get(), and associates a device with it using + * devres. On driver detach, release function is invoked on the devres data, + * then, devres data is freed. + */ +struct phy *devm_phy_get(struct device *dev, u8 index) +{ + struct phy **ptr, *phy; + + ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL); + if (!ptr) + return NULL; + + phy = phy_get(dev, index); + if (!IS_ERR(phy)) { + *ptr = phy; + devres_add(dev, ptr); + } else + devres_free(ptr); nitpick: when when if has { }, else should have, too. Sure. + + return phy; +} +EXPORT_SYMBOL_GPL(devm_phy_get); + +/** + * devm_phy_put - release the PHY + * @dev: device that wants to release this phy + * @phy: the phy returned by devm_phy_get() + * + * destroys the devres associated with this phy and invokes phy_put + * to release the phy. + */ +void devm_phy_put(struct device *dev, struct phy *phy) +{ + int r; + + r = devres_destroy(dev, devm_phy_release, devm_phy_match, phy); + dev_WARN_ONCE(dev, r, couldn't find PHY resource\n); +} +EXPORT_SYMBOL_GPL(devm_phy_put); + +/** + * devm_of_phy_get - lookup and obtain a reference to a phy by phandle + * @dev: device that requests this phy + * @phandle: name of the property holding the phy phandle value + * + * Returns the phy driver associated with the given phandle value,
Re: [RFC PATCH] drivers: phy: add generic PHY framework
On 09/26/2012 11:20 AM, ABRAHAM, KISHON VIJAY wrote: Hi, On Mon, Sep 17, 2012 at 3:03 PM, Marc Kleine-Budde m...@pengutronix.de wrote: On 09/14/2012 03:06 PM, ABRAHAM, KISHON VIJAY wrote: [...] diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c new file mode 100644 index 000..c55446a --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,437 @@ +/* + * phy-core.c -- Generic Phy framework. + * + * Copyright (C) 2012 Texas Instruments + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ + +#include linux/kernel.h +#include linux/export.h +#include linux/module.h +#include linux/err.h +#include linux/device.h +#include linux/slab.h +#include linux/of.h +#include linux/phy/phy.h + +static struct class *phy_class; +static LIST_HEAD(phy_list); +static DEFINE_MUTEX(phy_list_mutex); +static LIST_HEAD(phy_bind_list); + +static void devm_phy_release(struct device *dev, void *res) +{ + struct phy *phy = *(struct phy **)res; What about adding a struct phy_res, doing so,m you don't need these casts, and it's easier to add more pointers if needed. Wont we still need to do the cast since you get only a void pointer. Maybe I'm not getting you. As res is a void pointer, no need to hast to to a struct phy_res pointer, if you think that's unclean code, you can still cast it. But IMHO the code is far more readable. + + phy_put(phy); +} + +static int devm_phy_match(struct device *dev, void *res, void *match_data) +{ + return res == match_data; +} + +static struct phy *phy_lookup(struct device *dev, u8 index) +{ + struct phy_bind *phy_bind = NULL; + + list_for_each_entry(phy_bind, phy_bind_list, list) { + if (!(strcmp(phy_bind-dev_name, dev_name(dev))) + phy_bind-index == index) + return phy_bind-phy; + } + + return ERR_PTR(-ENODEV); +} + +static struct phy *of_phy_lookup(struct device *dev, struct device_node *node) +{ + int index = 0; + struct phy *phy; ^^ Please remove that stray space. Sure. + struct phy_bind *phy_map = NULL; + + list_for_each_entry(phy_map, phy_bind_list, list) + if (!(strcmp(phy_map-dev_name, dev_name(dev + index++; + + list_for_each_entry(phy, phy_list, head) { + if (node != phy-desc-of_node) + continue; + + phy_map = phy_bind(dev_name(dev), index, dev_name(phy-dev)); + if (!IS_ERR(phy_map)) { + phy_map-phy = phy; + phy_map-auto_bind = true; + } + + return phy; + } + + return ERR_PTR(-ENODEV); +} + +/** + * devm_phy_get - lookup and obtain a reference to a phy. + * @dev: device that requests this phy + * @index: the index of the phy + * + * Gets the phy using phy_get(), and associates a device with it using + * devres. On driver detach, release function is invoked on the devres data, + * then, devres data is freed. + */ +struct phy *devm_phy_get(struct device *dev, u8 index) +{ + struct phy **ptr, *phy; + + ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL); + if (!ptr) + return NULL; + + phy = phy_get(dev, index); + if (!IS_ERR(phy)) { + *ptr = phy; + devres_add(dev, ptr); + } else + devres_free(ptr); nitpick: when when if has { }, else should have, too. Sure. + + return phy; +} +EXPORT_SYMBOL_GPL(devm_phy_get); + +/** + * devm_phy_put - release the PHY + * @dev: device that wants to release this phy + * @phy: the phy returned by devm_phy_get() + * + * destroys the devres associated with this phy and invokes phy_put + * to release the phy. + */ +void devm_phy_put(struct device *dev, struct phy *phy) +{ + int r; + + r = devres_destroy(dev, devm_phy_release, devm_phy_match, phy); + dev_WARN_ONCE(dev, r, couldn't find PHY resource\n); +} +EXPORT_SYMBOL_GPL(devm_phy_put); + +/** + * devm_of_phy_get - lookup and obtain a reference to a phy by phandle + * @dev: device that requests this phy + * @phandle: name of the property holding the phy phandle value + * + *
[PATCH] drivers: phy: add generic PHY framework
The PHY framework provides a set of API's for the PHY drivers to create/destroy a PHY and API's for the PHY users to obtain a reference to the PHY with or without using phandle. To obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. PHY drivers should create the PHY by passing phy_descriptor that has describes the PHY (label, type etc..) and ops like init, exit, suspend, resume, poweron, shutdown. Cc: Felipe Balbi ba...@ti.com Cc: Marc Kleine-Budde m...@pengutronix.de Signed-off-by: Kishon Vijay Abraham I kis...@ti.com --- Did testing (phandle path) to some extent by hacking omap-usb2 to use this new framework. (completely modifying omap-usb2 to use this framework will take some time mostly because of the OTG stuff). Also perfomed non-dt testing by moving to a older kernel. Any kind of testing for this patch is welcome :-) MAINTAINERS |7 + drivers/Kconfig |2 + drivers/Makefile|2 + drivers/phy/Kconfig | 13 ++ drivers/phy/Makefile|5 + drivers/phy/phy-core.c | 445 +++ include/linux/phy/phy.h | 182 +++ 7 files changed, 656 insertions(+) create mode 100644 drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create mode 100644 drivers/phy/phy-core.c create mode 100644 include/linux/phy/phy.h diff --git a/MAINTAINERS b/MAINTAINERS index ea0519a..48f3cfb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3078,6 +3078,13 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git S: Maintained F: include/asm-generic +GENERIC PHY FRAMEWORK +M: Kishon Vijay Abraham I kis...@ti.com +L: linux-ker...@vger.kernel.org +S: Supported +F: drivers/phy/ +F: include/linux/phy/ + GENERIC UIO DRIVER FOR PCI DEVICES M: Michael S. Tsirkin m...@redhat.com L: k...@vger.kernel.org diff --git a/drivers/Kconfig b/drivers/Kconfig index 324e958..d289df5 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -154,4 +154,6 @@ source drivers/vme/Kconfig source drivers/pwm/Kconfig +source drivers/phy/Kconfig + endmenu diff --git a/drivers/Makefile b/drivers/Makefile index d64a0f7..e39252d 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -40,6 +40,8 @@ obj-y += char/ # gpu/ comes after char for AGP vs DRM startup obj-y += gpu/ +obj-y += phy/ + obj-$(CONFIG_CONNECTOR)+= connector/ # i810fb and intelfb depend on char/agp/ diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig new file mode 100644 index 000..34f7077 --- /dev/null +++ b/drivers/phy/Kconfig @@ -0,0 +1,13 @@ +# +# PHY +# + +menuconfig GENERIC_PHY + tristate Generic PHY Support + help + Generic PHY support. + + This framework is designed to provide a generic interface for PHY + devices present in the kernel. This layer will have the generic + API by which phy drivers can create PHY using the phy framework and + phy users can obtain reference to the PHY. diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile new file mode 100644 index 000..9e9560f --- /dev/null +++ b/drivers/phy/Makefile @@ -0,0 +1,5 @@ +# +# Makefile for the phy drivers. +# + +obj-$(CONFIG_GENERIC_PHY) += phy-core.o diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c new file mode 100644 index 000..bf47a88 --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,445 @@ +/* + * phy-core.c -- Generic Phy framework. + * + * Copyright (C) 2012 Texas Instruments + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ + +#include linux/kernel.h +#include linux/export.h +#include linux/module.h +#include linux/err.h +#include linux/device.h +#include linux/slab.h +#include linux/of.h +#include linux/phy/phy.h + +static struct class *phy_class; +static LIST_HEAD(phy_list); +static DEFINE_MUTEX(phy_list_mutex); +static LIST_HEAD(phy_bind_list); + +static void devm_phy_release(struct device *dev, void *res)
Re: [PATCH] drivers: phy: add generic PHY framework
On Wed, 2012-09-26 at 20:31 +0530, Kishon Vijay Abraham I wrote: The PHY framework provides a set of API's for the PHY drivers to create/destroy a PHY and API's Just some trivial notes. diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c [] @@ -0,0 +1,445 @@ [] +static void devm_phy_release(struct device *dev, void *res) +{ + struct phy *phy = *(struct phy **)res; That's a bit twisted isn't it? Perhaps struct phy *phy = res; [] +static int devm_phy_match(struct device *dev, void *res, void *match_data) +{ + return res == match_data; static bool devm_phy_match(etc...) [] +struct phy *devm_of_phy_get(struct device *dev, const char *phandle, u8 index) +{ + struct phy *phy = NULL, **ptr; + struct device_node *node; + + if (!dev-of_node) { + dev_dbg(dev, device does not have a device node entry\n); + return ERR_PTR(-EINVAL); + } + + node = of_parse_phandle(dev-of_node, phandle, index); + if (!node) { + dev_dbg(dev, failed to get %s phandle in %s node\n, phandle, + dev-of_node-full_name); + return ERR_PTR(-ENODEV); + } + + ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL); Is this the right size? Because ptr is **, perhaps sizeof(struct phy) is clearer. + if (!ptr) { + dev_dbg(dev, failed to allocate memory for devres\n); alloc failures generally don't need specific OOM messages as the general alloc OOM dumps stack. + return ERR_PTR(-ENOMEM); + } + + mutex_lock(phy_list_mutex); + + phy = of_phy_lookup(dev, node); + if (IS_ERR(phy) || !phy-dev.class || + !try_module_get(phy-dev.class-owner)) { I think it's tasteful coding style to align like: if (IS_ERR(phy) || !phy-dev.class || !try_module_get(phy-dev.class-owner)) { [] +struct phy *phy_create(struct device *dev, struct phy_descriptor *desc) +{ + int ret; + struct phy *phy; + struct phy_bind *phy_bind; + const char *devname = NULL; + + if (!dev || !desc) { + dev_err(dev, no descriptor/device provided for PHY\n); + ret = -EINVAL; + goto err0; + } + + if (!desc-ops) { + dev_err(dev, no PHY ops provided\n); + ret = -EINVAL; + goto err0; + } + + phy = kzalloc(sizeof(*phy), GFP_KERNEL); + if (!phy) { + dev_err(dev, no memory for PHY\n); No OOM message required... [] +static int __init phy_core_init(void) +{ + phy_class = class_create(THIS_MODULE, phy); + if (IS_ERR(phy_class)) { + pr_err(failed to create phy class -- %ld\n, + PTR_ERR(phy_class)); You might add #define pr_fmt(fmt) KBUILD_MODNAME : fmt before any include so that pr_levels are prefixed. cheers, Joe -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers: phy: add generic PHY framework
On Wed, Sep 26, 2012 at 08:31:15PM +0530, Kishon Vijay Abraham I wrote: The PHY framework provides a set of API's for the PHY drivers to create/destroy a PHY and API's for the PHY users to obtain a reference to the PHY with or without using phandle. To obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. PHY drivers should create the PHY by passing phy_descriptor that has describes the PHY (label, type etc..) and ops like init, exit, suspend, resume, poweron, shutdown. Do you have an example driver that uses this new framework? How does it look in sysfs? You need to add Documentation/ABI/ entries for the sysfs files you created as well. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers: phy: add generic PHY framework
On Wednesday, September 26, 2012 09:57:57 AM Joe Perches wrote: On Wed, 2012-09-26 at 20:31 +0530, Kishon Vijay Abraham I wrote: The PHY framework provides a set of API's for the PHY drivers to create/destroy a PHY and API's Just some trivial notes. diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c [] @@ -0,0 +1,445 @@ [] +static void devm_phy_release(struct device *dev, void *res) +{ + struct phy *phy = *(struct phy **)res; That's a bit twisted isn't it? Perhaps struct phy *phy = res; No, because you really need to dereference ptr, not simply cast. ... + + ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL); Is this the right size? Because ptr is **, perhaps sizeof(struct phy) is clearer. But incorrect. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers: phy: add generic PHY framework
On 09/26/2012 07:41 PM, Dmitry Torokhov wrote: On Wednesday, September 26, 2012 09:57:57 AM Joe Perches wrote: On Wed, 2012-09-26 at 20:31 +0530, Kishon Vijay Abraham I wrote: The PHY framework provides a set of API's for the PHY drivers to create/destroy a PHY and API's Just some trivial notes. diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c [] @@ -0,0 +1,445 @@ [] +static void devm_phy_release(struct device *dev, void *res) +{ + struct phy *phy = *(struct phy **)res; That's a bit twisted isn't it? Perhaps struct phy *phy = res; No, because you really need to dereference ptr, not simply cast. To avoid this confusion I suggest to create a struct phy_res. For now its only member will be a pointer to a struct phy. ... + + ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL); Is this the right size? Because ptr is **, perhaps sizeof(struct phy) is clearer. But incorrect. A proper struct will make the code more readable here, too. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH] drivers: phy: add generic PHY framework
On 09/26/2012 05:01 PM, Kishon Vijay Abraham I wrote: The PHY framework provides a set of API's for the PHY drivers to create/destroy a PHY and API's for the PHY users to obtain a reference to the PHY with or without using phandle. To obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. PHY drivers should create the PHY by passing phy_descriptor that has describes the PHY (label, type etc..) and ops like init, exit, suspend, resume, poweron, shutdown. Cc: Felipe Balbi ba...@ti.com Cc: Marc Kleine-Budde m...@pengutronix.de Signed-off-by: Kishon Vijay Abraham I kis...@ti.com I suggest to talk to someone who is familiar with the linux-bus abstraction. Adding devices, drivers, matching them and maintaining binding information should not be reinvented. See more comments inline. --- Did testing (phandle path) to some extent by hacking omap-usb2 to use this new framework. (completely modifying omap-usb2 to use this framework will take some time mostly because of the OTG stuff). Also perfomed non-dt testing by moving to a older kernel. Any kind of testing for this patch is welcome :-) MAINTAINERS |7 + drivers/Kconfig |2 + drivers/Makefile|2 + drivers/phy/Kconfig | 13 ++ drivers/phy/Makefile|5 + drivers/phy/phy-core.c | 445 +++ include/linux/phy/phy.h | 182 +++ 7 files changed, 656 insertions(+) create mode 100644 drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create mode 100644 drivers/phy/phy-core.c create mode 100644 include/linux/phy/phy.h diff --git a/MAINTAINERS b/MAINTAINERS index ea0519a..48f3cfb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3078,6 +3078,13 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git S: Maintained F: include/asm-generic +GENERIC PHY FRAMEWORK +M: Kishon Vijay Abraham I kis...@ti.com +L: linux-ker...@vger.kernel.org +S: Supported +F: drivers/phy/ +F: include/linux/phy/ + GENERIC UIO DRIVER FOR PCI DEVICES M: Michael S. Tsirkin m...@redhat.com L: k...@vger.kernel.org diff --git a/drivers/Kconfig b/drivers/Kconfig index 324e958..d289df5 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -154,4 +154,6 @@ source drivers/vme/Kconfig source drivers/pwm/Kconfig +source drivers/phy/Kconfig + endmenu diff --git a/drivers/Makefile b/drivers/Makefile index d64a0f7..e39252d 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -40,6 +40,8 @@ obj-y += char/ # gpu/ comes after char for AGP vs DRM startup obj-y+= gpu/ +obj-y+= phy/ + obj-$(CONFIG_CONNECTOR) += connector/ # i810fb and intelfb depend on char/agp/ diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig new file mode 100644 index 000..34f7077 --- /dev/null +++ b/drivers/phy/Kconfig @@ -0,0 +1,13 @@ +# +# PHY +# + +menuconfig GENERIC_PHY + tristate Generic PHY Support + help + Generic PHY support. + + This framework is designed to provide a generic interface for PHY + devices present in the kernel. This layer will have the generic + API by which phy drivers can create PHY using the phy framework and + phy users can obtain reference to the PHY. diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile new file mode 100644 index 000..9e9560f --- /dev/null +++ b/drivers/phy/Makefile @@ -0,0 +1,5 @@ +# +# Makefile for the phy drivers. +# + +obj-$(CONFIG_GENERIC_PHY)+= phy-core.o diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c new file mode 100644 index 000..bf47a88 --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,445 @@ +/* + * phy-core.c -- Generic Phy framework. + * + * Copyright (C) 2012 Texas Instruments + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ + +#include
Re: [RFC PATCH] drivers: phy: add generic PHY framework
On 09/14/2012 03:06 PM, ABRAHAM, KISHON VIJAY wrote: [...] diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c new file mode 100644 index 000..c55446a --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,437 @@ +/* + * phy-core.c -- Generic Phy framework. + * + * Copyright (C) 2012 Texas Instruments + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ + +#include linux/kernel.h +#include linux/export.h +#include linux/module.h +#include linux/err.h +#include linux/device.h +#include linux/slab.h +#include linux/of.h +#include linux/phy/phy.h + +static struct class *phy_class; +static LIST_HEAD(phy_list); +static DEFINE_MUTEX(phy_list_mutex); +static LIST_HEAD(phy_bind_list); + +static void devm_phy_release(struct device *dev, void *res) +{ + struct phy *phy = *(struct phy **)res; What about adding a struct phy_res, doing so,m you don't need these casts, and it's easier to add more pointers if needed. Wont we still need to do the cast since you get only a void pointer. Maybe I'm not getting you. As res is a void pointer, no need to hast to to a struct phy_res pointer, if you think that's unclean code, you can still cast it. But IMHO the code is far more readable. + + phy_put(phy); +} + +static int devm_phy_match(struct device *dev, void *res, void *match_data) +{ + return res == match_data; +} + +static struct phy *phy_lookup(struct device *dev, u8 index) +{ + struct phy_bind *phy_bind = NULL; + + list_for_each_entry(phy_bind, phy_bind_list, list) { + if (!(strcmp(phy_bind-dev_name, dev_name(dev))) + phy_bind-index == index) + return phy_bind-phy; + } + + return ERR_PTR(-ENODEV); +} + +static struct phy *of_phy_lookup(struct device *dev, struct device_node *node) +{ + int index = 0; + struct phy *phy; ^^ Please remove that stray space. Sure. + struct phy_bind *phy_map = NULL; + + list_for_each_entry(phy_map, phy_bind_list, list) + if (!(strcmp(phy_map-dev_name, dev_name(dev + index++; + + list_for_each_entry(phy, phy_list, head) { + if (node != phy-desc-of_node) + continue; + + phy_map = phy_bind(dev_name(dev), index, dev_name(phy-dev)); + if (!IS_ERR(phy_map)) { + phy_map-phy = phy; + phy_map-auto_bind = true; + } + + return phy; + } + + return ERR_PTR(-ENODEV); +} + +/** + * devm_phy_get - lookup and obtain a reference to a phy. + * @dev: device that requests this phy + * @index: the index of the phy + * + * Gets the phy using phy_get(), and associates a device with it using + * devres. On driver detach, release function is invoked on the devres data, + * then, devres data is freed. + */ +struct phy *devm_phy_get(struct device *dev, u8 index) +{ + struct phy **ptr, *phy; + + ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL); + if (!ptr) + return NULL; + + phy = phy_get(dev, index); + if (!IS_ERR(phy)) { + *ptr = phy; + devres_add(dev, ptr); + } else + devres_free(ptr); nitpick: when when if has { }, else should have, too. Sure. + + return phy; +} +EXPORT_SYMBOL_GPL(devm_phy_get); + +/** + * devm_phy_put - release the PHY + * @dev: device that wants to release this phy + * @phy: the phy returned by devm_phy_get() + * + * destroys the devres associated with this phy and invokes phy_put + * to release the phy. + */ +void devm_phy_put(struct device *dev, struct phy *phy) +{ + int r; + + r = devres_destroy(dev, devm_phy_release, devm_phy_match, phy); + dev_WARN_ONCE(dev, r, couldn't find PHY resource\n); +} +EXPORT_SYMBOL_GPL(devm_phy_put); + +/** + * devm_of_phy_get - lookup and obtain a reference to a phy by phandle + * @dev: device that requests this phy + * @phandle: name of the property holding the phy phandle value + * + * Returns the phy driver associated with the given phandle value, + * after getting a refcount to it or -ENODEV if there is no such phy. + * While at that,
Re: [RFC PATCH] drivers: phy: add generic PHY framework
On Mon, Sep 17, 2012 at 11:19:53AM +0530, ABRAHAM, KISHON VIJAY wrote: Hi, On Mon, Sep 17, 2012 at 6:50 AM, Chen Peter-B29397 b29...@freescale.com wrote: The PHY framework provides a set of API's for the PHY drivers to create/remove a PHY and the PHY users to obtain a reference to the PHY using or without using phandle. If the PHY users has to obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. What's an example of the same phy user binds to multiple phys? Single controller using multiple phys.. to be more specific here: any usb3 controller needs a USB2 PHY and USB3 PHY. -- balbi signature.asc Description: Digital signature
RE: [RFC PATCH] drivers: phy: add generic PHY framework
The PHY framework provides a set of API's for the PHY drivers to create/remove a PHY and the PHY users to obtain a reference to the PHY using or without using phandle. If the PHY users has to obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. What's an example of the same phy user binds to multiple phys? I only remembered that Felipe said there are two phy users for one single phy at omap5 that is both usb3 and sata uses the same phy. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] drivers: phy: add generic PHY framework
Hi, On Mon, Sep 17, 2012 at 6:50 AM, Chen Peter-B29397 b29...@freescale.com wrote: The PHY framework provides a set of API's for the PHY drivers to create/remove a PHY and the PHY users to obtain a reference to the PHY using or without using phandle. If the PHY users has to obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. What's an example of the same phy user binds to multiple phys? Single controller using multiple phys.. I only remembered that Felipe said there are two phy users for one single phy at omap5 that is both usb3 and sata uses the same phy. *index* is used when a single controller uses multiple phys. For example it could be used for dwc3 (usb3 controller) where it uses usb2 phy and usb3 phy. Thanks Kishon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH] drivers: phy: add generic PHY framework
The PHY framework provides a set of API's for the PHY drivers to create/remove a PHY and the PHY users to obtain a reference to the PHY using or without using phandle. If the PHY users has to obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. PHY drivers should create the PHY by passing phy_descriptor that has information about the PHY and ops like init, exit, suspend, resume, poweron, shutdown. Nyet-signed-off-by: Kishon Vijay Abraham I kis...@ti.com --- This framework is actually intended to be used by all the PHY drivers in the kernel. Though it's going to take a while for that, I intend to migrate existing USB/OTG phy drivers to use this framework as we align on the design of this framework. Once I migrate these phy drivers, I'll be able to test this framework (I haven't tested this framework so far). I sent this patch early so as to get review comments and align on the design. Thanks :-) drivers/Kconfig |2 + drivers/Makefile|2 + drivers/phy/Kconfig | 13 ++ drivers/phy/Makefile|5 + drivers/phy/phy-core.c | 437 +++ include/linux/phy/phy.h | 181 6 files changed, 640 insertions(+) create mode 100644 drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create mode 100644 drivers/phy/phy-core.c create mode 100644 include/linux/phy/phy.h diff --git a/drivers/Kconfig b/drivers/Kconfig index ece958d..8488818 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -152,4 +152,6 @@ source drivers/vme/Kconfig source drivers/pwm/Kconfig +source drivers/phy/Kconfig + endmenu diff --git a/drivers/Makefile b/drivers/Makefile index 5b42184..63d6bbe 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -38,6 +38,8 @@ obj-y += char/ # gpu/ comes after char for AGP vs DRM startup obj-y += gpu/ +obj-y += phy/ + obj-$(CONFIG_CONNECTOR)+= connector/ # i810fb and intelfb depend on char/agp/ diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig new file mode 100644 index 000..34f7077 --- /dev/null +++ b/drivers/phy/Kconfig @@ -0,0 +1,13 @@ +# +# PHY +# + +menuconfig GENERIC_PHY + tristate Generic PHY Support + help + Generic PHY support. + + This framework is designed to provide a generic interface for PHY + devices present in the kernel. This layer will have the generic + API by which phy drivers can create PHY using the phy framework and + phy users can obtain reference to the PHY. diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile new file mode 100644 index 000..9e9560f --- /dev/null +++ b/drivers/phy/Makefile @@ -0,0 +1,5 @@ +# +# Makefile for the phy drivers. +# + +obj-$(CONFIG_GENERIC_PHY) += phy-core.o diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c new file mode 100644 index 000..c55446a --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,437 @@ +/* + * phy-core.c -- Generic Phy framework. + * + * Copyright (C) 2012 Texas Instruments + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ + +#include linux/kernel.h +#include linux/export.h +#include linux/module.h +#include linux/err.h +#include linux/device.h +#include linux/slab.h +#include linux/of.h +#include linux/phy/phy.h + +static struct class *phy_class; +static LIST_HEAD(phy_list); +static DEFINE_MUTEX(phy_list_mutex); +static LIST_HEAD(phy_bind_list); + +static void devm_phy_release(struct device *dev, void *res) +{ + struct phy *phy = *(struct phy **)res; + + phy_put(phy); +} + +static int devm_phy_match(struct device *dev, void *res, void *match_data) +{ + return res == match_data; +} + +static struct phy *phy_lookup(struct device *dev, u8 index) +{ + struct phy_bind *phy_bind = NULL; + + list_for_each_entry(phy_bind, phy_bind_list, list) { + if (!(strcmp(phy_bind-dev_name, dev_name(dev))) + phy_bind-index == index) +
Re: [RFC PATCH] drivers: phy: add generic PHY framework
On 09/14/2012 01:58 PM, Kishon Vijay Abraham I wrote: The PHY framework provides a set of API's for the PHY drivers to create/remove a PHY and the PHY users to obtain a reference to the PHY using or without using phandle. If the PHY users has to obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. PHY drivers should create the PHY by passing phy_descriptor that has information about the PHY and ops like init, exit, suspend, resume, poweron, shutdown. Some comments inside. While looking over the code, I was thinking why not abstract the phy with a bus in the linux kernel. The ethernet phys are on the mdio_bus, see /sys/bus/mdio_bus. This saves you hand crafting devices, drivers and bindings, Marc Nyet-signed-off-by: Kishon Vijay Abraham I kis...@ti.com --- This framework is actually intended to be used by all the PHY drivers in the kernel. Though it's going to take a while for that, I intend to migrate existing USB/OTG phy drivers to use this framework as we align on the design of this framework. Once I migrate these phy drivers, I'll be able to test this framework (I haven't tested this framework so far). I sent this patch early so as to get review comments and align on the design. Thanks :-) drivers/Kconfig |2 + drivers/Makefile|2 + drivers/phy/Kconfig | 13 ++ drivers/phy/Makefile|5 + drivers/phy/phy-core.c | 437 +++ include/linux/phy/phy.h | 181 6 files changed, 640 insertions(+) create mode 100644 drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create mode 100644 drivers/phy/phy-core.c create mode 100644 include/linux/phy/phy.h diff --git a/drivers/Kconfig b/drivers/Kconfig index ece958d..8488818 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -152,4 +152,6 @@ source drivers/vme/Kconfig source drivers/pwm/Kconfig +source drivers/phy/Kconfig + endmenu diff --git a/drivers/Makefile b/drivers/Makefile index 5b42184..63d6bbe 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -38,6 +38,8 @@ obj-y += char/ # gpu/ comes after char for AGP vs DRM startup obj-y+= gpu/ +obj-y+= phy/ + obj-$(CONFIG_CONNECTOR) += connector/ # i810fb and intelfb depend on char/agp/ diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig new file mode 100644 index 000..34f7077 --- /dev/null +++ b/drivers/phy/Kconfig @@ -0,0 +1,13 @@ +# +# PHY +# + +menuconfig GENERIC_PHY + tristate Generic PHY Support + help + Generic PHY support. + + This framework is designed to provide a generic interface for PHY + devices present in the kernel. This layer will have the generic + API by which phy drivers can create PHY using the phy framework and + phy users can obtain reference to the PHY. diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile new file mode 100644 index 000..9e9560f --- /dev/null +++ b/drivers/phy/Makefile @@ -0,0 +1,5 @@ +# +# Makefile for the phy drivers. +# + +obj-$(CONFIG_GENERIC_PHY)+= phy-core.o diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c new file mode 100644 index 000..c55446a --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,437 @@ +/* + * phy-core.c -- Generic Phy framework. + * + * Copyright (C) 2012 Texas Instruments + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ + +#include linux/kernel.h +#include linux/export.h +#include linux/module.h +#include linux/err.h +#include linux/device.h +#include linux/slab.h +#include linux/of.h +#include linux/phy/phy.h + +static struct class *phy_class; +static LIST_HEAD(phy_list); +static DEFINE_MUTEX(phy_list_mutex); +static LIST_HEAD(phy_bind_list); + +static void devm_phy_release(struct device *dev, void *res) +{ + struct phy *phy = *(struct phy **)res; What
Re: [RFC PATCH] drivers: phy: add generic PHY framework
Hi, On Fri, Sep 14, 2012 at 5:58 PM, Marc Kleine-Budde m...@pengutronix.de wrote: On 09/14/2012 01:58 PM, Kishon Vijay Abraham I wrote: The PHY framework provides a set of API's for the PHY drivers to create/remove a PHY and the PHY users to obtain a reference to the PHY using or without using phandle. If the PHY users has to obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. PHY drivers should create the PHY by passing phy_descriptor that has information about the PHY and ops like init, exit, suspend, resume, poweron, shutdown. Some comments inside. While looking over the code, I was thinking why not abstract the phy with a bus in the linux kernel. The ethernet phys are on the mdio_bus, see /sys/bus/mdio_bus. This saves you hand crafting devices, drivers and bindings, well... have to think about it. Marc Nyet-signed-off-by: Kishon Vijay Abraham I kis...@ti.com --- This framework is actually intended to be used by all the PHY drivers in the kernel. Though it's going to take a while for that, I intend to migrate existing USB/OTG phy drivers to use this framework as we align on the design of this framework. Once I migrate these phy drivers, I'll be able to test this framework (I haven't tested this framework so far). I sent this patch early so as to get review comments and align on the design. Thanks :-) drivers/Kconfig |2 + drivers/Makefile|2 + drivers/phy/Kconfig | 13 ++ drivers/phy/Makefile|5 + drivers/phy/phy-core.c | 437 +++ include/linux/phy/phy.h | 181 6 files changed, 640 insertions(+) create mode 100644 drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create mode 100644 drivers/phy/phy-core.c create mode 100644 include/linux/phy/phy.h diff --git a/drivers/Kconfig b/drivers/Kconfig index ece958d..8488818 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -152,4 +152,6 @@ source drivers/vme/Kconfig source drivers/pwm/Kconfig +source drivers/phy/Kconfig + endmenu diff --git a/drivers/Makefile b/drivers/Makefile index 5b42184..63d6bbe 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -38,6 +38,8 @@ obj-y += char/ # gpu/ comes after char for AGP vs DRM startup obj-y+= gpu/ +obj-y+= phy/ + obj-$(CONFIG_CONNECTOR) += connector/ # i810fb and intelfb depend on char/agp/ diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig new file mode 100644 index 000..34f7077 --- /dev/null +++ b/drivers/phy/Kconfig @@ -0,0 +1,13 @@ +# +# PHY +# + +menuconfig GENERIC_PHY + tristate Generic PHY Support + help + Generic PHY support. + + This framework is designed to provide a generic interface for PHY + devices present in the kernel. This layer will have the generic + API by which phy drivers can create PHY using the phy framework and + phy users can obtain reference to the PHY. diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile new file mode 100644 index 000..9e9560f --- /dev/null +++ b/drivers/phy/Makefile @@ -0,0 +1,5 @@ +# +# Makefile for the phy drivers. +# + +obj-$(CONFIG_GENERIC_PHY)+= phy-core.o diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c new file mode 100644 index 000..c55446a --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,437 @@ +/* + * phy-core.c -- Generic Phy framework. + * + * Copyright (C) 2012 Texas Instruments + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ + +#include linux/kernel.h +#include linux/export.h +#include linux/module.h +#include linux/err.h +#include linux/device.h +#include linux/slab.h +#include linux/of.h +#include linux/phy/phy.h + +static struct class *phy_class; +static LIST_HEAD(phy_list); +static DEFINE_MUTEX(phy_list_mutex); +static LIST_HEAD(phy_bind_list); + +static
Re: [RFC PATCH] drivers: phy: add generic PHY framework
On Fri, Sep 14, 2012 at 02:28:19PM +0200, Marc Kleine-Budde wrote: On 09/14/2012 01:58 PM, Kishon Vijay Abraham I wrote: The PHY framework provides a set of API's for the PHY drivers to create/remove a PHY and the PHY users to obtain a reference to the PHY using or without using phandle. If the PHY users has to obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. PHY drivers should create the PHY by passing phy_descriptor that has information about the PHY and ops like init, exit, suspend, resume, poweron, shutdown. Some comments inside. While looking over the code, I was thinking why not abstract the phy with a bus in the linux kernel. The ethernet phys are on the mdio_bus, see /sys/bus/mdio_bus. This saves you hand crafting devices, drivers and bindings, I don't think that's a good idea, actually. You can have USB PHYs which are memory mapped, or connected through i2c, or connected through any other bus. If the PHYs layer itself is a bus, it means we will need to register a device on two different buses, which doesn't sound very nice IMHO. -- balbi signature.asc Description: Digital signature