diff mbox series

[v3,5/6] usb: Iterator for ports

Message ID 20210331105908.67066-6-heikki.krogerus@linux.intel.com
State Superseded
Headers show
Series None | expand

Commit Message

Heikki Krogerus March 31, 2021, 10:59 a.m. UTC 
Introducing usb_for_each_port(). It works the same way as
usb_for_each_dev(), but instead of going through every USB
device in the system, it walks through the USB ports in the
system.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 drivers/usb/core/usb.c | 46 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb.h    |  9 +++++++++
 2 files changed, 55 insertions(+)

Comments

kernel test robot March 31, 2021, 2:27 p.m. UTC | #1 
Hi Heikki,

I love your patch! Yet something to improve:

[auto build test ERROR on peter.chen-usb/for-usb-next]
[also build test ERROR on linus/master v5.12-rc5 next-20210331]
[cannot apply to usb/usb-testing balbi-usb/testing/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Heikki-Krogerus/usb-Linking-ports-to-their-Type-C-connectors/20210331-190638
base:   https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git for-usb-next
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/067cb7efe925811fd52b7b9550578f88a20d2226
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Heikki-Krogerus/usb-Linking-ports-to-their-Type-C-connectors/20210331-190638
        git checkout 067cb7efe925811fd52b7b9550578f88a20d2226
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/usb/core/usb.c:439:5: error: redefinition of 'usb_for_each_port'
     439 | int usb_for_each_port(void *data, int (*fn)(struct device *, void *))
         |     ^~~~~~~~~~~~~~~~~
   In file included from drivers/usb/core/usb.c:35:
   include/linux/usb.h:884:19: note: previous definition of 'usb_for_each_port' was here
     884 | static inline int usb_for_each_port(void *data, int (*fn)(struct device *, void *))
         |                   ^~~~~~~~~~~~~~~~~

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for SND_ATMEL_SOC_PDC
   Depends on SOUND && !UML && SND && SND_SOC && SND_ATMEL_SOC && HAS_DMA
   Selected by
   - SND_ATMEL_SOC_SSC && SOUND && !UML && SND && SND_SOC && SND_ATMEL_SOC
   - SND_ATMEL_SOC_SSC_PDC && SOUND && !UML && SND && SND_SOC && SND_ATMEL_SOC && ATMEL_SSC


vim +/usb_for_each_port +439 drivers/usb/core/usb.c

   429	
   430	/**
   431	 * usb_for_each_port - interate over all USB ports in the system
   432	 * @data: data pointer that will be handed to the callback function
   433	 * @fn: callback function to be called for each USB port
   434	 *
   435	 * Iterate over all USB ports and call @fn for each, passing it @data. If it
   436	 * returns anything other than 0, we break the iteration prematurely and return
   437	 * that value.
   438	 */
 > 439	int usb_for_each_port(void *data, int (*fn)(struct device *, void *))
   440	{
   441		struct each_hub_arg arg = {data, fn};
   442	
   443		return usb_for_each_dev(&arg, __each_hub);
   444	}
   445	EXPORT_SYMBOL_GPL(usb_for_each_port);
   446	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Guenter Roeck March 31, 2021, 4:41 p.m. UTC | #2 
On 3/31/21 3:59 AM, Heikki Krogerus wrote:
> Introducing usb_for_each_port(). It works the same way as
> usb_for_each_dev(), but instead of going through every USB
> device in the system, it walks through the USB ports in the
> system.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
> ---
>  drivers/usb/core/usb.c | 46 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/usb.h    |  9 +++++++++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 2ce3667ec6fae..62368c4ed37af 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -398,6 +398,52 @@ int usb_for_each_dev(void *data, int (*fn)(struct usb_device *, void *))
>  }
>  EXPORT_SYMBOL_GPL(usb_for_each_dev);
>  
> +struct each_hub_arg {
> +	void *data;
> +	int (*fn)(struct device *, void *);
> +};
> +
> +static int __each_hub(struct usb_device *hdev, void *data)
> +{
> +	struct each_hub_arg *arg = (struct each_hub_arg *)data;
> +	struct usb_hub *hub;
> +	int ret = 0;
> +	int i;
> +
> +	hub = usb_hub_to_struct_hub(hdev);
> +	if (!hub)
> +		return 0;
> +
> +	mutex_lock(&usb_port_peer_mutex);
> +
> +	for (i = 0; i < hdev->maxchild; i++) {
> +		ret = arg->fn(&hub->ports[i]->dev, arg->data);
> +		if (ret)
> +			break;
> +	}
> +
> +	mutex_unlock(&usb_port_peer_mutex);
> +
> +	return ret;
> +}
> +
> +/**
> + * usb_for_each_port - interate over all USB ports in the system
> + * @data: data pointer that will be handed to the callback function
> + * @fn: callback function to be called for each USB port
> + *
> + * Iterate over all USB ports and call @fn for each, passing it @data. If it
> + * returns anything other than 0, we break the iteration prematurely and return
> + * that value.
> + */
> +int usb_for_each_port(void *data, int (*fn)(struct device *, void *))
> +{
> +	struct each_hub_arg arg = {data, fn};
> +
> +	return usb_for_each_dev(&arg, __each_hub);
> +}
> +EXPORT_SYMBOL_GPL(usb_for_each_port);
> +
>  /**
>   * usb_release_dev - free a usb device structure when all users of it are finished.
>   * @dev: device that's been disconnected
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index ddd2f5b2a2827..ebcd03d835d04 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -882,6 +882,15 @@ extern struct usb_host_interface *usb_find_alt_setting(
>  		unsigned int iface_num,
>  		unsigned int alt_num);
>  
> +#ifdef CONFIG_USB

#if IS_ENABLED(CONFIG_USB)

> +int usb_for_each_port(void *data, int (*fn)(struct device *, void *));
> +#else
> +static inline int usb_for_each_port(void *data, int (*fn)(struct device *, void *))
> +{
> +	return 0;
> +}
> +#endif
> +
>  /* port claiming functions */
>  int usb_hub_claim_port(struct usb_device *hdev, unsigned port1,
>  		struct usb_dev_state *owner);
>
Heikki Krogerus April 1, 2021, 6:37 a.m. UTC | #3 
On Wed, Mar 31, 2021 at 09:41:22AM -0700, Guenter Roeck wrote:
> > diff --git a/include/linux/usb.h b/include/linux/usb.h

> > index ddd2f5b2a2827..ebcd03d835d04 100644

> > --- a/include/linux/usb.h

> > +++ b/include/linux/usb.h

> > @@ -882,6 +882,15 @@ extern struct usb_host_interface *usb_find_alt_setting(

> >  		unsigned int iface_num,

> >  		unsigned int alt_num);

> >  

> > +#ifdef CONFIG_USB

> 

> #if IS_ENABLED(CONFIG_USB)


Thanks Guenter.

> > +int usb_for_each_port(void *data, int (*fn)(struct device *, void *));

> > +#else

> > +static inline int usb_for_each_port(void *data, int (*fn)(struct device *, void *))

> > +{

> > +	return 0;

> > +}

> > +#endif

> > +

> >  /* port claiming functions */

> >  int usb_hub_claim_port(struct usb_device *hdev, unsigned port1,

> >  		struct usb_dev_state *owner);

> > 


-- 
heikki
Guenter Roeck April 5, 2021, 7:25 p.m. UTC | #4 
On 3/31/21 11:37 PM, Heikki Krogerus wrote:
> On Wed, Mar 31, 2021 at 09:41:22AM -0700, Guenter Roeck wrote:

>>> diff --git a/include/linux/usb.h b/include/linux/usb.h

>>> index ddd2f5b2a2827..ebcd03d835d04 100644

>>> --- a/include/linux/usb.h

>>> +++ b/include/linux/usb.h

>>> @@ -882,6 +882,15 @@ extern struct usb_host_interface *usb_find_alt_setting(

>>>  		unsigned int iface_num,

>>>  		unsigned int alt_num);

>>>  

>>> +#ifdef CONFIG_USB

>>

>> #if IS_ENABLED(CONFIG_USB)

> 

Note that IS_REACHABLE(), as you ended up using, has a slightly different
semantics.

With IS_ENABLED(), one still has to ensure, via config file dependencies,
that the code using the API is reachable. That would be something like
	depends on USB || USB=n
This dependency ensures that the code using the API code is not built
into the kernel if USB is built as module. In most cases this reflects
the intended use case.

IS_REACHABLE(), on the other side, will disable the API if USB is built
as module and the calling code is built into the kernel. This can have
unexpected results and should be used with caution.

Thanks,
Guenter

> Thanks Guenter.

> 

>>> +int usb_for_each_port(void *data, int (*fn)(struct device *, void *));

>>> +#else

>>> +static inline int usb_for_each_port(void *data, int (*fn)(struct device *, void *))

>>> +{

>>> +	return 0;

>>> +}

>>> +#endif

>>> +

>>>  /* port claiming functions */

>>>  int usb_hub_claim_port(struct usb_device *hdev, unsigned port1,

>>>  		struct usb_dev_state *owner);

>>>

>
diff mbox series

Patch

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 2ce3667ec6fae..62368c4ed37af 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -398,6 +398,52 @@  int usb_for_each_dev(void *data, int (*fn)(struct usb_device *, void *))
 }
 EXPORT_SYMBOL_GPL(usb_for_each_dev);
 
+struct each_hub_arg {
+	void *data;
+	int (*fn)(struct device *, void *);
+};
+
+static int __each_hub(struct usb_device *hdev, void *data)
+{
+	struct each_hub_arg *arg = (struct each_hub_arg *)data;
+	struct usb_hub *hub;
+	int ret = 0;
+	int i;
+
+	hub = usb_hub_to_struct_hub(hdev);
+	if (!hub)
+		return 0;
+
+	mutex_lock(&usb_port_peer_mutex);
+
+	for (i = 0; i < hdev->maxchild; i++) {
+		ret = arg->fn(&hub->ports[i]->dev, arg->data);
+		if (ret)
+			break;
+	}
+
+	mutex_unlock(&usb_port_peer_mutex);
+
+	return ret;
+}
+
+/**
+ * usb_for_each_port - interate over all USB ports in the system
+ * @data: data pointer that will be handed to the callback function
+ * @fn: callback function to be called for each USB port
+ *
+ * Iterate over all USB ports and call @fn for each, passing it @data. If it
+ * returns anything other than 0, we break the iteration prematurely and return
+ * that value.
+ */
+int usb_for_each_port(void *data, int (*fn)(struct device *, void *))
+{
+	struct each_hub_arg arg = {data, fn};
+
+	return usb_for_each_dev(&arg, __each_hub);
+}
+EXPORT_SYMBOL_GPL(usb_for_each_port);
+
 /**
  * usb_release_dev - free a usb device structure when all users of it are finished.
  * @dev: device that's been disconnected
diff --git a/include/linux/usb.h b/include/linux/usb.h
index ddd2f5b2a2827..ebcd03d835d04 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -882,6 +882,15 @@  extern struct usb_host_interface *usb_find_alt_setting(
 		unsigned int iface_num,
 		unsigned int alt_num);
 
+#ifdef CONFIG_USB
+int usb_for_each_port(void *data, int (*fn)(struct device *, void *));
+#else
+static inline int usb_for_each_port(void *data, int (*fn)(struct device *, void *))
+{
+	return 0;
+}
+#endif
+
 /* port claiming functions */
 int usb_hub_claim_port(struct usb_device *hdev, unsigned port1,
 		struct usb_dev_state *owner);