2020-10-14 17:39:40

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 08/12] usb: hosts: Remove in_interrupt() from comments

From: Ahmed S. Darwish <[email protected]>

The usage of in_interrupt() in drivers is phased out for various reasons.

Various comments use !in_interrupt() to describe calling context for probe()
and remove() functions. That's wrong because the calling context has to be
preemptible task context, which is not what !in_interrupt() describes.

Cleanup the comments. While at it add the missing kernel doc argument
descriptors and make usb_hcd_msp_remove() static.

Signed-off-by: Ahmed S. Darwish <[email protected]>
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Kukjin Kim <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: [email protected]
Cc: [email protected]

---
drivers/usb/host/ehci-fsl.c | 9 ++++-----
drivers/usb/host/ehci-pmcmsp.c | 15 +++++++++------
drivers/usb/host/ohci-at91.c | 11 ++++++++---
drivers/usb/host/ohci-omap.c | 7 ++++---
drivers/usb/host/ohci-pxa27x.c | 11 ++++++-----
drivers/usb/host/ohci-s3c2410.c | 12 ++++++------
6 files changed, 37 insertions(+), 28 deletions(-)

--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -39,10 +39,10 @@ static struct hc_driver __read_mostly fs
/*
* fsl_ehci_drv_probe - initialize FSL-based HCDs
* @pdev: USB Host Controller being probed
- * Context: !in_interrupt()
*
- * Allocates basic resources for this USB host controller.
+ * Context: task context, might sleep
*
+ * Allocates basic resources for this USB host controller.
*/
static int fsl_ehci_drv_probe(struct platform_device *pdev)
{
@@ -684,12 +684,11 @@ static const struct ehci_driver_override
/**
* fsl_ehci_drv_remove - shutdown processing for FSL-based HCDs
* @pdev: USB Host Controller being removed
- * Context: !in_interrupt()
*
- * Reverses the effect of usb_hcd_fsl_probe().
+ * Context: task context, might sleep
*
+ * Reverses the effect of usb_hcd_fsl_probe().
*/
-
static int fsl_ehci_drv_remove(struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata = dev_get_platdata(&pdev->dev);
--- a/drivers/usb/host/ehci-pmcmsp.c
+++ b/drivers/usb/host/ehci-pmcmsp.c
@@ -147,12 +147,14 @@ static int usb_hcd_msp_map_regs(struct m

/**
* usb_hcd_msp_probe - initialize PMC MSP-based HCDs
- * Context: !in_interrupt()
+ * @driver: Pointer to hc driver instance
+ * @dev: USB controller to probe
+ *
+ * Context: task context, might sleep
*
* Allocates basic resources for this USB host controller, and
* then invokes the start() method for the HCD associated with it
* through the hotplug entry's driver_data.
- *
*/
int usb_hcd_msp_probe(const struct hc_driver *driver,
struct platform_device *dev)
@@ -223,8 +225,9 @@ int usb_hcd_msp_probe(const struct hc_dr

/**
* usb_hcd_msp_remove - shutdown processing for PMC MSP-based HCDs
- * @dev: USB Host Controller being removed
- * Context: !in_interrupt()
+ * @hcd: USB Host Controller being removed
+ *
+ * Context: task context, might sleep
*
* Reverses the effect of usb_hcd_msp_probe(), first invoking
* the HCD's stop() method. It is always called from a thread
@@ -233,7 +236,7 @@ int usb_hcd_msp_probe(const struct hc_dr
* may be called without controller electrically present
* may be called with controller, bus, and devices active
*/
-void usb_hcd_msp_remove(struct usb_hcd *hcd, struct platform_device *dev)
+static void usb_hcd_msp_remove(struct usb_hcd *hcd)
{
usb_remove_hcd(hcd);
iounmap(hcd->regs);
@@ -306,7 +309,7 @@ static int ehci_hcd_msp_drv_remove(struc
{
struct usb_hcd *hcd = platform_get_drvdata(pdev);

- usb_hcd_msp_remove(hcd, pdev);
+ usb_hcd_msp_remove(hcd);

/* free TWI GPIO USB_HOST_DEV pin */
gpio_free(MSP_PIN_USB0_HOST_DEV);
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -155,7 +155,10 @@ static struct regmap *at91_dt_syscon_sfr

/*
* usb_hcd_at91_probe - initialize AT91-based HCDs
- * Context: !in_interrupt()
+ * @driver: Pointer to hc driver instance
+ * @pdev: USB controller to probe
+ *
+ * Context: task context, might sleep
*
* Allocates basic resources for this USB host controller, and
* then invokes the start() method for the HCD associated with it
@@ -246,12 +249,14 @@ static int usb_hcd_at91_probe(const stru

/*
* usb_hcd_at91_remove - shutdown processing for AT91-based HCDs
- * Context: !in_interrupt()
+ * @hcd: USB controller to remove
+ * @pdev: Platform device required for cleanup
+ *
+ * Context: task context, might sleep
*
* Reverses the effect of usb_hcd_at91_probe(), first invoking
* the HCD's stop() method. It is always called from a thread
* context, "rmmod" or something similar.
- *
*/
static void usb_hcd_at91_remove(struct usb_hcd *hcd,
struct platform_device *pdev)
--- a/drivers/usb/host/ohci-omap.c
+++ b/drivers/usb/host/ohci-omap.c
@@ -285,7 +285,9 @@ static int ohci_omap_reset(struct usb_hc

/**
* ohci_hcd_omap_probe - initialize OMAP-based HCDs
- * Context: !in_interrupt()
+ * @pdev: USB controller to probe
+ *
+ * Context: task context, might sleep
*
* Allocates basic resources for this USB host controller, and
* then invokes the start() method for the HCD associated with it
@@ -399,8 +401,7 @@ static int ohci_hcd_omap_probe(struct pl

/**
* ohci_hcd_omap_remove - shutdown processing for OMAP-based HCDs
- * @dev: USB Host Controller being removed
- * Context: !in_interrupt()
+ * @pdev: USB Host Controller being removed
*
* Reverses the effect of ohci_hcd_omap_probe(), first invoking
* the HCD's stop() method. It is always called from a thread
--- a/drivers/usb/host/ohci-pxa27x.c
+++ b/drivers/usb/host/ohci-pxa27x.c
@@ -410,12 +410,13 @@ static int ohci_pxa_of_init(struct platf

/**
* ohci_hcd_pxa27x_probe - initialize pxa27x-based HCDs
- * Context: !in_interrupt()
+ * @pdev: USB Host controller to probe
+ *
+ * Context: task context, might sleep
*
* Allocates basic resources for this USB host controller, and
* then invokes the start() method for the HCD associated with it
* through the hotplug entry's driver_data.
- *
*/
static int ohci_hcd_pxa27x_probe(struct platform_device *pdev)
{
@@ -509,13 +510,13 @@ static int ohci_hcd_pxa27x_probe(struct

/**
* ohci_hcd_pxa27x_remove - shutdown processing for pxa27x-based HCDs
- * @dev: USB Host Controller being removed
- * Context: !in_interrupt()
+ * @pdev: USB Host Controller being removed
+ *
+ * Context: task context, might sleep
*
* Reverses the effect of ohci_hcd_pxa27x_probe(), first invoking
* the HCD's stop() method. It is always called from a thread
* context, normally "rmmod", "apmd", or something similar.
- *
*/
static int ohci_hcd_pxa27x_remove(struct platform_device *pdev)
{
--- a/drivers/usb/host/ohci-s3c2410.c
+++ b/drivers/usb/host/ohci-s3c2410.c
@@ -324,14 +324,13 @@ static void s3c2410_hcd_oc(struct s3c241
/*
* ohci_hcd_s3c2410_remove - shutdown processing for HCD
* @dev: USB Host Controller being removed
- * Context: !in_interrupt()
+ *
+ * Context: task context, might sleep
*
* Reverses the effect of ohci_hcd_3c2410_probe(), first invoking
* the HCD's stop() method. It is always called from a thread
* context, normally "rmmod", "apmd", or something similar.
- *
-*/
-
+ */
static int
ohci_hcd_s3c2410_remove(struct platform_device *dev)
{
@@ -345,12 +344,13 @@ ohci_hcd_s3c2410_remove(struct platform_

/*
* ohci_hcd_s3c2410_probe - initialize S3C2410-based HCDs
- * Context: !in_interrupt()
+ * @dev: USB Host Controller to be probed
+ *
+ * Context: task context, might sleep
*
* Allocates basic resources for this USB host controller, and
* then invokes the start() method for the HCD associated with it
* through the hotplug entry's driver_data.
- *
*/
static int ohci_hcd_s3c2410_probe(struct platform_device *dev)
{


2020-10-14 17:39:45

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [patch 08/12] usb: hosts: Remove in_interrupt() from comments

On Wed, Oct 14, 2020 at 04:52:23PM +0200, Thomas Gleixner wrote:
> From: Ahmed S. Darwish <[email protected]>
>
> The usage of in_interrupt() in drivers is phased out for various reasons.
>
> Various comments use !in_interrupt() to describe calling context for probe()
> and remove() functions. That's wrong because the calling context has to be
> preemptible task context, which is not what !in_interrupt() describes.
>
> Cleanup the comments. While at it add the missing kernel doc argument
> descriptors and make usb_hcd_msp_remove() static.
>
> Signed-off-by: Ahmed S. Darwish <[email protected]>
> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: Alan Stern <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: Kukjin Kim <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
>
> ---
> drivers/usb/host/ehci-fsl.c | 9 ++++-----
> drivers/usb/host/ehci-pmcmsp.c | 15 +++++++++------
> drivers/usb/host/ohci-at91.c | 11 ++++++++---
> drivers/usb/host/ohci-omap.c | 7 ++++---
> drivers/usb/host/ohci-pxa27x.c | 11 ++++++-----
> drivers/usb/host/ohci-s3c2410.c | 12 ++++++------

For the s3c2410:
Acked-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2020-10-14 17:41:22

by Alan Stern

[permalink] [raw]
Subject: Re: [patch 08/12] usb: hosts: Remove in_interrupt() from comments

On Wed, Oct 14, 2020 at 04:52:23PM +0200, Thomas Gleixner wrote:
> From: Ahmed S. Darwish <[email protected]>
>
> The usage of in_interrupt() in drivers is phased out for various reasons.
>
> Various comments use !in_interrupt() to describe calling context for probe()
> and remove() functions. That's wrong because the calling context has to be
> preemptible task context, which is not what !in_interrupt() describes.
>
> Cleanup the comments. While at it add the missing kernel doc argument
> descriptors and make usb_hcd_msp_remove() static.
>
> Signed-off-by: Ahmed S. Darwish <[email protected]>
> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: Alan Stern <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: Kukjin Kim <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
>
> ---

> --- a/drivers/usb/host/ehci-pmcmsp.c
> +++ b/drivers/usb/host/ehci-pmcmsp.c

> @@ -233,7 +236,7 @@ int usb_hcd_msp_probe(const struct hc_dr
> * may be called without controller electrically present
> * may be called with controller, bus, and devices active
> */
> -void usb_hcd_msp_remove(struct usb_hcd *hcd, struct platform_device *dev)
> +static void usb_hcd_msp_remove(struct usb_hcd *hcd)

Please don't intermix changes to comments with other more substantive
changes.

> --- a/drivers/usb/host/ohci-omap.c
> +++ b/drivers/usb/host/ohci-omap.c

> @@ -399,8 +401,7 @@ static int ohci_hcd_omap_probe(struct pl
>
> /**
> * ohci_hcd_omap_remove - shutdown processing for OMAP-based HCDs
> - * @dev: USB Host Controller being removed
> - * Context: !in_interrupt()
> + * @pdev: USB Host Controller being removed
> *
> * Reverses the effect of ohci_hcd_omap_probe(), first invoking
> * the HCD's stop() method. It is always called from a thread

You forgot to add the Context comment.

Alan Stern