| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 29 Jul 2013 17:51:15 +0200
From: Sylwester Nawrocki <s.nawrocki@...sung.com>
To: Kishon Vijay Abraham I <kishon@...com>
Cc: gregkh@...uxfoundation.org, kyungmin.park@...sung.com,
balbi@...com, jg1.han@...sung.com, kgene.kim@...sung.com,
stern@...land.harvard.edu, broonie@...nel.org,
tomasz.figa@...il.com, arnd@...db.de, devicetree@...r.kernel.org,
linux-fbdev@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
swarren@...dia.com, balajitk@...com, linux-doc@...r.kernel.org,
tony@...mide.com, Tomasz Figa <t.figa@...sung.com>,
linux-usb@...r.kernel.org, nsekhar@...com,
linux-kernel@...r.kernel.org, grant.likely@...aro.org,
linux@....linux.org.uk, akpm@...ux-foundation.org,
linux-omap@...r.kernel.org, george.cherian@...com,
linux-arm-kernel@...ts.infradead.org, linux-media@...r.kernel.org
Subject: Re: [RESEND PATCH v10 1/8] drivers: phy: add generic PHY framework
On 07/26/2013 02:49 PM, Kishon Vijay Abraham I wrote:
> The PHY framework provides a set of APIs for the PHY drivers to
> create/destroy a PHY and APIs for the PHY users to obtain a reference to the
> PHY with or without using phandle. For dt-boot, the PHY drivers should
> also register *PHY provider* with the framework.
>
> PHY drivers should create the PHY by passing id and ops like init, exit,
> power_on and power_off. This framework is also pm runtime enabled.
>
> The documentation for the generic PHY framework is added in
> Documentation/phy.txt and the documentation for dt binding can be found at
> Documentation/devicetree/bindings/phy/phy-bindings.txt
>
> Cc: Tomasz Figa <t.figa@...sung.com>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Signed-off-by: Kishon Vijay Abraham I <kishon@...com>
> Acked-by: Felipe Balbi <balbi@...com>
> Tested-by: Sylwester Nawrocki <s.nawrocki@...sung.com>
> ---
> .../devicetree/bindings/phy/phy-bindings.txt | 66 ++
> Documentation/phy.txt | 166 +++++
> MAINTAINERS | 8 +
> drivers/Kconfig | 2 +
> drivers/Makefile | 2 +
> drivers/phy/Kconfig | 18 +
> drivers/phy/Makefile | 5 +
> drivers/phy/phy-core.c | 714 ++++++++++++++++++++
> include/linux/phy/phy.h | 270 ++++++++
> 9 files changed, 1251 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
> create mode 100644 Documentation/phy.txt
> 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/Documentation/devicetree/bindings/phy/phy-bindings.txt b/Documentation/devicetree/bindings/phy/phy-bindings.txt
> new file mode 100644
> index 0000000..8ae844f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
> @@ -0,0 +1,66 @@
> +This document explains only the device tree data binding. For general
> +information about PHY subsystem refer to Documentation/phy.txt
[...]
> @@ -0,0 +1,166 @@
> + PHY SUBSYSTEM
> + Kishon Vijay Abraham I <kishon@...com>
> +
> +This document explains the Generic PHY Framework along with the APIs provided,
> +and how-to-use.
> +
> +1. Introduction
> +
> +*PHY* is the abbreviation for physical layer. It is used to connect a device
> +to the physical medium e.g., the USB controller has a PHY to provide functions
> +such as serialization, de-serialization, encoding, decoding and is responsible
> +for obtaining the required data transmission rate. Note that some USB
> +controllers have PHY functionality embedded into it and others use an external
> +PHY. Other peripherals that use PHY include Wireless LAN, Ethernet,
> +SATA etc.
> +
> +The intention of creating this framework is to bring the PHY drivers spread
> +all over the Linux kernel to drivers/phy to increase code re-use and for
> +better code maintainability.
> +
> +This framework will be of use only to devices that use external PHY (PHY
> +functionality is not embedded within the controller).
> +
> +2. Registering/Unregistering the PHY provider
> +
> +PHY provider refers to an entity that implements one or more PHY instances.
> +For the simple case where the PHY provider implements only a single instance of
> +the PHY, the framework provides its own implementation of of_xlate in
> +of_phy_simple_xlate. If the PHY provider implements multiple instances, it
> +should provide its own implementation of of_xlate. of_xlate is used only for
> +dt boot case.
> +
> +#define of_phy_provider_register(dev, xlate) \
> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
> +
> +#define devm_of_phy_provider_register(dev, xlate) \
> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
This needs to be:
__devm_of_phy_provider_register((dev), THIS_MODULE, (xlate))
as Kamil pointed out. We've tested it here with v9 and it makes
Bad Things happen. ;)
> +of_phy_provider_register and devm_of_phy_provider_register macros can be used to
> +register the phy_provider and it takes device and of_xlate as
> +arguments. For the dt boot case, all PHY providers should use one of the above
> +2 macros to register the PHY provider.
> +
> +void devm_of_phy_provider_unregister(struct device *dev,
> + struct phy_provider *phy_provider);
> +void of_phy_provider_unregister(struct phy_provider *phy_provider);
> +
> +devm_of_phy_provider_unregister and of_phy_provider_unregister can be used to
> +unregister the PHY.
> +
[...]
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> new file mode 100644
> index 0000000..f1d15e5
> --- /dev/null
> +++ b/drivers/phy/phy-core.c
> @@ -0,0 +1,714 @@
[...]
> +static struct phy *phy_lookup(struct device *device, const char *port)
> +{
> + unsigned int count;
> + struct phy *phy;
> + struct device *dev;
> + struct phy_consumer *consumers;
> + struct class_dev_iter iter;
Don't you need something like
if (phy->init_data == NULL)
return ERR_PTR(-EINVAL);
to ensure there is no attempt to dereference NULL platform data ?
> + class_dev_iter_init(&iter, phy_class, NULL, NULL);
> + while ((dev = class_dev_iter_next(&iter))) {
> + phy = to_phy(dev);
> + count = phy->init_data->num_consumers;
> + consumers = phy->init_data->consumers;
> + while (count--) {
> + if (!strcmp(consumers->dev_name, dev_name(device)) &&
> + !strcmp(consumers->port, port)) {
> + class_dev_iter_exit(&iter);
> + return phy;
> + }
> + consumers++;
> + }
> + }
> +
> + class_dev_iter_exit(&iter);
> + return ERR_PTR(-ENODEV);
> +}
> +
[...]
> +int phy_init(struct phy *phy)
> +{
> + int ret;
> +
> + ret = phy_pm_runtime_get_sync(phy);
> + if (ret < 0 && ret != -ENOTSUPP)
> + return ret;
> +
> + mutex_lock(&phy->mutex);
> + if (phy->init_count++ == 0 && phy->ops->init) {
> + ret = phy->ops->init(phy);
> + if (ret < 0) {
> + dev_err(&phy->dev, "phy init failed --> %d\n", ret);
> + goto out;
Is this 'goto' and similar ones below really needed ?
> + }
> + }
> +
> +out:
> + mutex_unlock(&phy->mutex);
> + phy_pm_runtime_put(phy);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_init);
> +
> +int phy_exit(struct phy *phy)
> +{
> + int ret;
> +
> + ret = phy_pm_runtime_get_sync(phy);
> + if (ret < 0 && ret != -ENOTSUPP)
> + return ret;
> +
> + mutex_lock(&phy->mutex);
> + if (--phy->init_count == 0 && phy->ops->exit) {
> + ret = phy->ops->exit(phy);
> + if (ret < 0) {
> + dev_err(&phy->dev, "phy exit failed --> %d\n", ret);
> + goto out;
> + }
> + }
> +
> +out:
> + mutex_unlock(&phy->mutex);
> + phy_pm_runtime_put(phy);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_exit);
> +
> +int phy_power_on(struct phy *phy)
> +{
> + int ret = -ENOTSUPP;
> +
> + ret = phy_pm_runtime_get_sync(phy);
> + if (ret < 0 && ret != -ENOTSUPP)
> + return ret;
> +
> + mutex_lock(&phy->mutex);
> + if (phy->power_count++ == 0 && phy->ops->power_on) {
> + ret = phy->ops->power_on(phy);
> + if (ret < 0) {
> + dev_err(&phy->dev, "phy poweron failed --> %d\n", ret);
> + goto out;
> + }
> + }
> +
> +out:
> + mutex_unlock(&phy->mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_power_on);
> +
> +int phy_power_off(struct phy *phy)
> +{
> + int ret = -ENOTSUPP;
> +
> + mutex_lock(&phy->mutex);
> + if (--phy->power_count == 0 && phy->ops->power_off) {
> + ret = phy->ops->power_off(phy);
> + if (ret < 0) {
> + dev_err(&phy->dev, "phy poweroff failed --> %d\n", ret);
> + goto out;
> + }
> + }
> +
> +out:
> + mutex_unlock(&phy->mutex);
> + phy_pm_runtime_put(phy);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_power_off);
--
Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists