| 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: Fri, 31 Oct 2014 18:03:41 +0530
From: Vivek Gautam <gautam.vivek@...sung.com>
To: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc: Kishon Vijay Abraham I <kishon@...com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Linux USB Mailing List <linux-usb@...r.kernel.org>,
andrew.kim@...el.com
Subject: Re: [PATCHv4 2/6] phy: improved lookup method
Hi Heikki,
On Fri, Oct 17, 2014 at 8:09 PM, Heikki Krogerus
<heikki.krogerus@...ux.intel.com> wrote:
> Removes the need for the phys to be aware of their users
> even when not using DT. The method is copied from clkdev.c.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> Tested-by: Vivek Gautam <gautam.vivek@...sung.com>
> ---
> Documentation/phy.txt | 66 ++++++++---------------
> drivers/phy/phy-core.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++-
> include/linux/phy/phy.h | 27 ++++++++++
> 3 files changed, 183 insertions(+), 45 deletions(-)
>
> diff --git a/Documentation/phy.txt b/Documentation/phy.txt
> index c6594af..8add515 100644
> --- a/Documentation/phy.txt
> +++ b/Documentation/phy.txt
> @@ -54,18 +54,14 @@ The PHY driver should create the PHY in order for other peripheral controllers
> to make use of it. The PHY framework provides 2 APIs to create the PHY.
>
> struct phy *phy_create(struct device *dev, struct device_node *node,
> - const struct phy_ops *ops,
> - struct phy_init_data *init_data);
> + const struct phy_ops *ops);
> struct phy *devm_phy_create(struct device *dev, struct device_node *node,
> - const struct phy_ops *ops,
> - struct phy_init_data *init_data);
> + const struct phy_ops *ops);
>
> The PHY drivers can use one of the above 2 APIs to create the PHY by passing
> -the device pointer, phy ops and init_data.
> +the device pointer and phy ops.
> phy_ops is a set of function pointers for performing PHY operations such as
> -init, exit, power_on and power_off. *init_data* is mandatory to get a reference
> -to the PHY in the case of non-dt boot. See section *Board File Initialization*
> -on how init_data should be used.
> +init, exit, power_on and power_off.
>
> Inorder to dereference the private data (in phy_ops), the phy provider driver
> can use phy_set_drvdata() after creating the PHY and use phy_get_drvdata() in
> @@ -137,42 +133,24 @@ There are exported APIs like phy_pm_runtime_get, phy_pm_runtime_get_sync,
> phy_pm_runtime_put, phy_pm_runtime_put_sync, phy_pm_runtime_allow and
> phy_pm_runtime_forbid for performing PM operations.
>
> -8. Board File Initialization
> -
> -Certain board file initialization is necessary in order to get a reference
> -to the PHY in the case of non-dt boot.
> -Say we have a single device that implements 3 PHYs that of USB, SATA and PCIe,
> -then in the board file the following initialization should be done.
> -
> -struct phy_consumer consumers[] = {
> - PHY_CONSUMER("dwc3.0", "usb"),
> - PHY_CONSUMER("pcie.0", "pcie"),
> - PHY_CONSUMER("sata.0", "sata"),
> -};
> -PHY_CONSUMER takes 2 parameters, first is the device name of the controller
> -(PHY consumer) and second is the port name.
> -
> -struct phy_init_data init_data = {
> - .consumers = consumers,
> - .num_consumers = ARRAY_SIZE(consumers),
> -};
> -
> -static const struct platform_device pipe3_phy_dev = {
> - .name = "pipe3-phy",
> - .id = -1,
> - .dev = {
> - .platform_data = {
> - .init_data = &init_data,
> - },
> - },
> -};
> -
> -then, while doing phy_create, the PHY driver should pass this init_data
> - phy_create(dev, ops, pdata->init_data);
> -
> -and the controller driver (phy consumer) should pass the port name along with
> -the device to get a reference to the PHY
> - phy_get(dev, "pcie");
> +8. PHY Mappings
> +
> +In order to get reference to a PHY without help from DeviceTree, the framework
> +offers lookups which can be compared to clkdev that allow clk structures to be
> +bound to devices. A lookup can be made statically by directly registering
> +phy_lookup structure which contains the name of the PHY device, the name of the
> +device which the PHY will be bind to and Connection ID string. Alternatively a
> +lookup can be made during runtime when a handle to the struct phy already
> +exists.
> +
> +The framework offers the following APIs for registering and unregistering the
> +lookups.
> +
> +void phy_register_lookup(struct phy_lookup *pl);
> +int phy_create_lookup(struct phy *phy, const char *con_id, const char *dev_id);
> +
> +void phy_unregister_lookup(struct phy_lookup *pl);
> +void phy_remove_lookup(struct phy *phy, const char *con_id, const char *dev_id);
>
> 9. DeviceTree Binding
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index ff5eec5..c8d0f66 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -26,6 +26,7 @@
> static struct class *phy_class;
> static DEFINE_MUTEX(phy_provider_mutex);
> static LIST_HEAD(phy_provider_list);
> +static LIST_HEAD(phys);
> static DEFINE_IDA(phy_ida);
>
> static void devm_phy_release(struct device *dev, void *res)
> @@ -84,6 +85,138 @@ static struct phy *phy_lookup(struct device *device, const char *port)
> return ERR_PTR(-ENODEV);
> }
>
> +/**
> + * phy_register_lookup() - register PHY/device association
> + * @pl: association to register
> + */
> +void phy_register_lookup(struct phy_lookup *pl)
> +{
> + mutex_lock(&phy_provider_mutex);
> + list_add_tail(&pl->node, &phys);
> + mutex_unlock(&phy_provider_mutex);
> +}
> +
> +/**
> + * phy_unregister_lookup() - remove PHY/device association
> + * @pl: association to be removed
> + */
> +void phy_unregister_lookup(struct phy_lookup *pl)
> +{
> + mutex_lock(&phy_provider_mutex);
> + list_del(&pl->node);
> + mutex_unlock(&phy_provider_mutex);
> +}
> +
> +/**
> + * phy_create_lookup() - allocate and register PHY/device association
> + * @phy: the phy of the association
> + * @con_id: connection ID string on device
> + * @dev_id: the device of the association
> + *
> + * Creates and registers phy_lookup entry.
> + */
> +int phy_create_lookup(struct phy *phy, const char *con_id, const char *dev_id)
> +{
> + struct phy_lookup *pl;
> +
> + if (!phy || (!dev_id && !con_id))
> + return -EINVAL;
> +
> + pl = kzalloc(sizeof(*pl), GFP_KERNEL);
> + if (!pl)
> + return -ENOMEM;
> +
> + pl->phy_name = dev_name(phy->dev.parent);
> + pl->dev_id = dev_id;
> + pl->con_id = con_id;
> +
> + phy_register_lookup(pl);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(phy_create_lookup);
> +
> +/**
> + * phy_remove_lookup() - find and remove PHY/device association
> + * @phy: the phy of the association
> + * @con_id: connection ID string on device
> + * @dev_id: the device of the association
> + *
> + * Finds and unregisters phy_lookup entry that was created with
> + * phy_create_lookup().
> + */
> +void phy_remove_lookup(struct phy *phy, const char *con_id, const char *dev_id)
> +{
> + struct phy_lookup *pl;
> +
> + if (!phy || (!dev_id && !con_id))
> + return;
> +
> + list_for_each_entry(pl, &phys, node)
> + if (!strcmp(pl->phy_name, dev_name(phy->dev.parent)) &&
> + !strcmp(pl->dev_id, dev_id) &&
> + !strcmp(pl->con_id, con_id)) {
> + phy_unregister_lookup(pl);
> + kfree(pl);
> + return;
> + }
> +}
> +EXPORT_SYMBOL_GPL(phy_remove_lookup);
> +
> +static struct phy *phy_find(struct device *dev, const char *con_id)
> +{
> + const char *dev_id = dev ? dev_name(dev) : NULL;
> + int match, best_found = 0, best_possible = 0;
> + struct phy *phy = ERR_PTR(-ENODEV);
> + struct phy_lookup *p, *pl = NULL;
> +
> + if (dev_id)
> + best_possible += 2;
> + if (con_id)
> + best_possible += 1;
> +
> + list_for_each_entry(p, &phys, node) {
> + match = 0;
> + if (p->dev_id) {
> + if (!dev_id || strcmp(p->dev_id, dev_id))
> + continue;
> + match += 2;
> + }
> + if (p->con_id) {
> + if (!con_id || strcmp(p->con_id, con_id))
> + continue;
> + match += 1;
> + }
> +
> + if (match > best_found) {
> + pl = p;
> + if (match != best_possible)
> + best_found = match;
> + else
> + break;
> + }
> + }
> +
> + if (pl) {
> + struct class_dev_iter iter;
> + struct device *phy_dev;
> +
> + class_dev_iter_init(&iter, phy_class, NULL, NULL);
> + while ((phy_dev = class_dev_iter_next(&iter))) {
We have the case with phy-exynos5-usbdrd driver, which registers two
phys usb2-phy and usb3-phy
both being used by xhci (after dwc3 creates a lookup table).
The phy_dev is coming same for both the PHYs, and that's the reason
when i try to get
"usb2-phy" and "usb3-phy" i end up getting only usb2-phy.
This is happening with V4 of this patch; V3 seems fine. The only
differnece i see is
we are creating the phy_lookup_table using phy_dev->parent.
Is there something that i am missing ?
> + if (!strcmp(dev_name(phy_dev->parent), pl->phy_name)) {
> + phy = to_phy(phy_dev);
> + break;
> + }
> + }
> + class_dev_iter_exit(&iter);
> + }
> +
> + /* fall-back to the old lookup method for now */
> + if (IS_ERR(phy))
> + phy = phy_lookup(dev, con_id);
> + return phy;
> +}
> +
> static struct phy_provider *of_phy_provider_lookup(struct device_node *node)
> {
> struct phy_provider *phy_provider;
> @@ -463,7 +596,7 @@ struct phy *phy_get(struct device *dev, const char *string)
> string);
> phy = _of_phy_get(dev->of_node, index);
> } else {
> - phy = phy_lookup(dev, string);
> + phy = phy_find(dev, string);
> }
> if (IS_ERR(phy))
> return phy;
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 9fda683..2696b95 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -110,6 +110,20 @@ struct phy_init_data {
> .port = _port, \
> }
>
> +struct phy_lookup {
> + struct list_head node;
> + const char *phy_name;
> + const char *dev_id;
> + const char *con_id;
> +};
> +
> +#define PHY_LOOKUP(a, b, c) \
> + { \
> + .phy_name = a, \
> + .dev_id = b, \
> + .con_id = c, \
> + }
> +
> #define to_phy(a) (container_of((a), struct phy, dev))
>
> #define of_phy_provider_register(dev, xlate) \
> @@ -174,6 +188,10 @@ struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
> void of_phy_provider_unregister(struct phy_provider *phy_provider);
> void devm_of_phy_provider_unregister(struct device *dev,
> struct phy_provider *phy_provider);
> +void phy_register_lookup(struct phy_lookup *pl);
> +void phy_unregister_lookup(struct phy_lookup *pl);
> +int phy_create_lookup(struct phy *phy, const char *con_id, const char *dev_id);
> +void phy_remove_lookup(struct phy *phy, const char *con_id, const char *dev_id);
> #else
> static inline int phy_pm_runtime_get(struct phy *phy)
> {
> @@ -345,6 +363,15 @@ static inline void devm_of_phy_provider_unregister(struct device *dev,
> struct phy_provider *phy_provider)
> {
> }
> +static inline void phy_register_lookup(struct phy_lookup *pl) { }
> +static inline void phy_unregister_lookup(struct phy_lookup *pl) { }
> +static inline int
> +phy_create_lookup(struct phy *phy, const char *con_id, const char *dev_id)
> +{
> + return 0;
> +}
> +static inline void phy_remove_lookup(struct phy *phy, const char *con_id,
> + const char *dev_id) { }
> #endif
>
> #endif /* __DRIVERS_PHY_H */
> --
> 2.1.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
--
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