Re: [PATCH 1/2] usb: core: add support for HCD providers
From: Greg Kroah-Hartman <hidden>
Date: 2016-08-09 13:56:29
Also in:
linux-leds, lkml
On Tue, Jul 12, 2016 at 02:35:19PM +0200, Rafał Miłecki wrote:
quoted hunk ↗ jump to hunk
When working with Device Tree we may need to reference controllers (their nodes) and query for HCDs. This is useful for getting some runtime info about host controllers like e.g. assigned bus number. Signed-off-by: Rafał Miłecki <redacted> --- drivers/usb/core/Makefile | 1 + drivers/usb/core/provider.c | 79 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/usb/provider.h | 39 ++++++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 drivers/usb/core/provider.c create mode 100644 include/linux/usb/provider.hdiff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile index 9780877..20b91d1 100644 --- a/drivers/usb/core/Makefile +++ b/drivers/usb/core/Makefile@@ -9,5 +9,6 @@ usbcore-y += port.o of.o usbcore-$(CONFIG_PCI) += hcd-pci.o usbcore-$(CONFIG_ACPI) += usb-acpi.o +usbcore-$(CONFIG_OF) += provider.o obj-$(CONFIG_USB) += usbcore.odiff --git a/drivers/usb/core/provider.c b/drivers/usb/core/provider.c new file mode 100644 index 0000000..4b9165a --- /dev/null +++ b/drivers/usb/core/provider.c@@ -0,0 +1,79 @@ +#include <linux/slab.h> +#include <linux/usb/provider.h> + +static DEFINE_MUTEX(hcd_provider_mutex); +static LIST_HEAD(hcd_provider_list); + +struct hcd_provider { + struct device_node *np; + struct usb_hcd *(*of_xlate)(struct of_phandle_args *args, void *data); + void *data; + struct list_head list; +}; + +struct hcd_provider *of_hcd_provider_register(struct device_node *np, + struct usb_hcd *(*of_xlate)(struct of_phandle_args *args, void *data),
Typedef for the function pointer?
+ void *data)
+{
+ struct hcd_provider *hcd_provider;
+
+ if (!np)
+ return ERR_PTR(-EINVAL);How can that be true?
+
+ hcd_provider = kzalloc(sizeof(*hcd_provider), GFP_KERNEL);
+ if (!hcd_provider)
+ return ERR_PTR(-ENOMEM);
+
+ hcd_provider->np = np;
+ hcd_provider->of_xlate = of_xlate;
+ hcd_provider->data = data;
+
+ mutex_lock(&hcd_provider_mutex);
+ list_add_tail(&hcd_provider->list, &hcd_provider_list);
+ mutex_unlock(&hcd_provider_mutex);
+
+ return hcd_provider;
+}
+EXPORT_SYMBOL_GPL(of_hcd_provider_register);
+
+void of_hcd_provider_unregister(struct hcd_provider *hcd_provider)
+{
+ if (IS_ERR(hcd_provider))
+ return;
+
+ mutex_lock(&hcd_provider_mutex);
+ list_del(&hcd_provider->list);
+ mutex_unlock(&hcd_provider_mutex);
+
+ kfree(hcd_provider);
+}
+EXPORT_SYMBOL_GPL(of_hcd_provider_unregister);
+
+struct usb_hcd *of_hcd_xlate_simple(struct of_phandle_args *args, void *data)
+{
+ if (args->args_count != 0)
+ return ERR_PTR(-EINVAL);Huh?
+ return data;
What is this function for? Why even have it?
+}
+EXPORT_SYMBOL_GPL(of_hcd_xlate_simple);
+
+struct usb_hcd *of_hcd_get_from_provider(struct of_phandle_args *args)
+{
+ struct usb_hcd *hcd = ERR_PTR(-ENOENT);
+ struct hcd_provider *provider;
+
+ if (!args)
+ return ERR_PTR(-EINVAL);How is args not set?
quoted hunk ↗ jump to hunk
+ + mutex_lock(&hcd_provider_mutex); + list_for_each_entry(provider, &hcd_provider_list, list) { + if (provider->np == args->np) { + hcd = provider->of_xlate(args, provider->data); + break; + } + } + mutex_unlock(&hcd_provider_mutex); + + return hcd; +} +EXPORT_SYMBOL_GPL(of_hcd_get_from_provider);diff --git a/include/linux/usb/provider.h b/include/linux/usb/provider.h new file mode 100644 index 0000000..c66e006 --- /dev/null +++ b/include/linux/usb/provider.h@@ -0,0 +1,39 @@ +#ifndef __USB_CORE_PROVIDER_H +#define __USB_CORE_PROVIDER_H + +#include <linux/of.h> +#include <linux/usb.h> +#include <linux/usb/hcd.h> + +struct hcd_provider; + +#ifdef CONFIG_OF +struct hcd_provider *of_hcd_provider_register(struct device_node *np, + struct usb_hcd *(*of_xlate)(struct of_phandle_args *args, void *data), + void *data); +void of_hcd_provider_unregister(struct hcd_provider *hcd_provider); +struct usb_hcd *of_hcd_xlate_simple(struct of_phandle_args *args, void *data); +struct usb_hcd *of_hcd_get_from_provider(struct of_phandle_args *args); +#else +static inline +struct hcd_provider *of_hcd_provider_register(struct device_node *np, + struct usb_hcd *(*of_xlate)(struct of_phandle_args *args, void *data), + void *data) +{ + return ERR_PTR(-ENOSYS); +} +static inline void of_hcd_provider_unregister(struct hcd_provider *hcd_provider) +{ +} +static inline struct usb_hcd *of_hcd_xlate_simple(struct of_phandle_args *args, + void *data) +{ + return ERR_PTR(-ENOSYS); +} +static inline struct usb_hcd *of_hcd_get_from_provider(struct of_phandle_args *args) +{ + return NULL; +} +#endif
Why all of the "of" stuff? Why not make it generic for any firmware type (acpi, OF, etc.)? And I really don't like this, isn't there other ways to get this information if you really need it? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html