[PATCH] kernel/irq/devres.c: move "kernel/irq/devres.c" to "drivers/base/devres_irq.c"

[PATCH] kernel/irq/devres.c: move "kernel/irq/devres.c" to "drivers/base/devres_irq.c"

[Chen Gang]

"kernel/irq/devres.c" is a driver extension tool for irq (with devres)
which is independent on 'GENERIC_HARDIRQS', so it is not suitable to
still be in "kernel/irq/" which depends on 'GENERIC_HARDIRQS'.

It is a basic tool for drivers, so can move it to "drivers/base/" to be
independent on 'GENERIC_HARDIRQS'.

It is about irq features, so if can not find other more suitable place,
can still let their declaration in "include/linux/interrupts.h".

The related error (with randconfig which disable 'GENERIC_HARDIRQS')

  drivers/built-in.o: In function `dw_dma_probe':
  (.text+0x3747a): undefined reference to `devm_request_threaded_irq'


Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
 drivers/base/Makefile     |    2 +-
 drivers/base/devres_irq.c |   94 +++++++++++++++++++++++++++++++++++++++++++++
 kernel/irq/Makefile       |    2 +-
 kernel/irq/devres.c       |   94 ---------------------------------------------
 4 files changed, 96 insertions(+), 96 deletions(-)
 create mode 100644 drivers/base/devres_irq.c
 delete mode 100644 kernel/irq/devres.c

diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 94e8a80..cb706f9 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -2,7 +2,7 @@
 
 obj-y			:= core.o bus.o dd.o syscore.o \
 			   driver.o class.o platform.o \
-			   cpu.o firmware.o init.o map.o devres.o \
+			   cpu.o firmware.o init.o map.o devres.o devres_irq.o \
 			   attribute_container.o transport_class.o \
 			   topology.o
 obj-$(CONFIG_DEVTMPFS)	+= devtmpfs.o
diff --git a/drivers/base/devres_irq.c b/drivers/base/devres_irq.c
new file mode 100644
index 0000000..bd8e788
--- /dev/null
+++ b/drivers/base/devres_irq.c
@@ -0,0 +1,94 @@
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/gfp.h>
+
+/*
+ * Device resource management aware IRQ request/free implementation.
+ */
+struct irq_devres {
+	unsigned int irq;
+	void *dev_id;
+};
+
+static void devm_irq_release(struct device *dev, void *res)
+{
+	struct irq_devres *this = res;
+
+	free_irq(this->irq, this->dev_id);
+}
+
+static int devm_irq_match(struct device *dev, void *res, void *data)
+{
+	struct irq_devres *this = res, *match = data;
+
+	return this->irq == match->irq && this->dev_id == match->dev_id;
+}
+
+/**
+ *	devm_request_threaded_irq - allocate an interrupt line for a managed device
+ *	@dev: device to request interrupt for
+ *	@irq: Interrupt line to allocate
+ *	@handler: Function to be called when the IRQ occurs
+ *	@thread_fn: function to be called in a threaded interrupt context. NULL
+ *		    for devices which handle everything in @handler
+ *	@irqflags: Interrupt type flags
+ *	@devname: An ascii name for the claiming device
+ *	@dev_id: A cookie passed back to the handler function
+ *
+ *	Except for the extra @dev argument, this function takes the
+ *	same arguments and performs the same function as
+ *	request_irq().  IRQs requested with this function will be
+ *	automatically freed on driver detach.
+ *
+ *	If an IRQ allocated with this function needs to be freed
+ *	separately, devm_free_irq() must be used.
+ */
+int devm_request_threaded_irq(struct device *dev, unsigned int irq,
+			      irq_handler_t handler, irq_handler_t thread_fn,
+			      unsigned long irqflags, const char *devname,
+			      void *dev_id)
+{
+	struct irq_devres *dr;
+	int rc;
+
+	dr = devres_alloc(devm_irq_release, sizeof(struct irq_devres),
+			  GFP_KERNEL);
+	if (!dr)
+		return -ENOMEM;
+
+	rc = request_threaded_irq(irq, handler, thread_fn, irqflags, devname,
+				  dev_id);
+	if (rc) {
+		devres_free(dr);
+		return rc;
+	}
+
+	dr->irq = irq;
+	dr->dev_id = dev_id;
+	devres_add(dev, dr);
+
+	return 0;
+}
+EXPORT_SYMBOL(devm_request_threaded_irq);
+
+/**
+ *	devm_free_irq - free an interrupt
+ *	@dev: device to free interrupt for
+ *	@irq: Interrupt line to free
+ *	@dev_id: Device identity to free
+ *
+ *	Except for the extra @dev argument, this function takes the
+ *	same arguments and performs the same function as free_irq().
+ *	This function instead of free_irq() should be used to manually
+ *	free IRQs allocated with devm_request_irq().
+ */
+void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id)
+{
+	struct irq_devres match_data = { irq, dev_id };
+
+	WARN_ON(devres_destroy(dev, devm_irq_release, devm_irq_match,
+			       &match_data));
+	free_irq(irq, dev_id);
+}
+EXPORT_SYMBOL(devm_free_irq);
diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
index fff1738..d98eec9 100644
--- a/kernel/irq/Makefile
+++ b/kernel/irq/Makefile
@@ -1,5 +1,5 @@
 
-obj-y := irqdesc.o handle.o manage.o spurious.o resend.o chip.o dummychip.o devres.o
+obj-y := irqdesc.o handle.o manage.o spurious.o resend.o chip.o dummychip.o
 obj-$(CONFIG_GENERIC_IRQ_CHIP) += generic-chip.o
 obj-$(CONFIG_GENERIC_IRQ_PROBE) += autoprobe.o
 obj-$(CONFIG_IRQ_DOMAIN) += irqdomain.o
diff --git a/kernel/irq/devres.c b/kernel/irq/devres.c
deleted file mode 100644
index bd8e788..0000000
--- a/kernel/irq/devres.c
+++ /dev/null
@@ -1,94 +0,0 @@
-#include <linux/module.h>
-#include <linux/interrupt.h>
-#include <linux/device.h>
-#include <linux/gfp.h>
-
-/*
- * Device resource management aware IRQ request/free implementation.
- */
-struct irq_devres {
-	unsigned int irq;
-	void *dev_id;
-};
-
-static void devm_irq_release(struct device *dev, void *res)
-{
-	struct irq_devres *this = res;
-
-	free_irq(this->irq, this->dev_id);
-}
-
-static int devm_irq_match(struct device *dev, void *res, void *data)
-{
-	struct irq_devres *this = res, *match = data;
-
-	return this->irq == match->irq && this->dev_id == match->dev_id;
-}
-
-/**
- *	devm_request_threaded_irq - allocate an interrupt line for a managed device
- *	@dev: device to request interrupt for
- *	@irq: Interrupt line to allocate
- *	@handler: Function to be called when the IRQ occurs
- *	@thread_fn: function to be called in a threaded interrupt context. NULL
- *		    for devices which handle everything in @handler
- *	@irqflags: Interrupt type flags
- *	@devname: An ascii name for the claiming device
- *	@dev_id: A cookie passed back to the handler function
- *
- *	Except for the extra @dev argument, this function takes the
- *	same arguments and performs the same function as
- *	request_irq().  IRQs requested with this function will be
- *	automatically freed on driver detach.
- *
- *	If an IRQ allocated with this function needs to be freed
- *	separately, devm_free_irq() must be used.
- */
-int devm_request_threaded_irq(struct device *dev, unsigned int irq,
-			      irq_handler_t handler, irq_handler_t thread_fn,
-			      unsigned long irqflags, const char *devname,
-			      void *dev_id)
-{
-	struct irq_devres *dr;
-	int rc;
-
-	dr = devres_alloc(devm_irq_release, sizeof(struct irq_devres),
-			  GFP_KERNEL);
-	if (!dr)
-		return -ENOMEM;
-
-	rc = request_threaded_irq(irq, handler, thread_fn, irqflags, devname,
-				  dev_id);
-	if (rc) {
-		devres_free(dr);
-		return rc;
-	}
-
-	dr->irq = irq;
-	dr->dev_id = dev_id;
-	devres_add(dev, dr);
-
-	return 0;
-}
-EXPORT_SYMBOL(devm_request_threaded_irq);
-
-/**
- *	devm_free_irq - free an interrupt
- *	@dev: device to free interrupt for
- *	@irq: Interrupt line to free
- *	@dev_id: Device identity to free
- *
- *	Except for the extra @dev argument, this function takes the
- *	same arguments and performs the same function as free_irq().
- *	This function instead of free_irq() should be used to manually
- *	free IRQs allocated with devm_request_irq().
- */
-void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id)
-{
-	struct irq_devres match_data = { irq, dev_id };
-
-	WARN_ON(devres_destroy(dev, devm_irq_release, devm_irq_match,
-			       &match_data));
-	free_irq(irq, dev_id);
-}
-EXPORT_SYMBOL(devm_free_irq);
-- 
1.7.7.6

Re: [PATCH] kernel/irq/devres.c: move "kernel/irq/devres.c" to "drivers/base/devres_irq.c"

[Chen Gang]

Currently, devm* are implemented in various places.

IO region: in kernel/resources.c  (it may be related with resource)
IOMAP:     in lib/devres.c        (it seem not be related with general lib)
Gen pool:  in lib/genalloc.c      (it maybe be related with lib)
IRQ:       in kernel/irq/devres.c (it doesn't belong to generic hard irq)
Others:    in drivers/*

For my opinion:
  move them all to "drivers/base/".
  or move some basic devm* to "lib/", and still let another devm* which related with the specific features no touch (excluding IRQ, it need still move to "kernel/").


Welcome any other members suggestions or completions.

Thanks.


On 07/23/2013 03:36 PM, Chen Gang wrote:
> "kernel/irq/devres.c" is a driver extension tool for irq (with devres)
> which is independent on 'GENERIC_HARDIRQS', so it is not suitable to
> still be in "kernel/irq/" which depends on 'GENERIC_HARDIRQS'.
> 
> It is a basic tool for drivers, so can move it to "drivers/base/" to be
> independent on 'GENERIC_HARDIRQS'.
> 
> It is about irq features, so if can not find other more suitable place,
> can still let their declaration in "include/linux/interrupts.h".
> 
> The related error (with randconfig which disable 'GENERIC_HARDIRQS')
> 
>   drivers/built-in.o: In function `dw_dma_probe':
>   (.text+0x3747a): undefined reference to `devm_request_threaded_irq'
> 
> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  drivers/base/Makefile     |    2 +-
>  drivers/base/devres_irq.c |   94 +++++++++++++++++++++++++++++++++++++++++++++
>  kernel/irq/Makefile       |    2 +-
>  kernel/irq/devres.c       |   94 ---------------------------------------------
>  4 files changed, 96 insertions(+), 96 deletions(-)
>  create mode 100644 drivers/base/devres_irq.c
>  delete mode 100644 kernel/irq/devres.c
> 
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 94e8a80..cb706f9 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -2,7 +2,7 @@
>  
>  obj-y			:= core.o bus.o dd.o syscore.o \
>  			   driver.o class.o platform.o \
> -			   cpu.o firmware.o init.o map.o devres.o \
> +			   cpu.o firmware.o init.o map.o devres.o devres_irq.o \
>  			   attribute_container.o transport_class.o \
>  			   topology.o
>  obj-$(CONFIG_DEVTMPFS)	+= devtmpfs.o
> diff --git a/drivers/base/devres_irq.c b/drivers/base/devres_irq.c
> new file mode 100644
> index 0000000..bd8e788
> --- /dev/null
> +++ b/drivers/base/devres_irq.c
> @@ -0,0 +1,94 @@
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/device.h>
> +#include <linux/gfp.h>
> +
> +/*
> + * Device resource management aware IRQ request/free implementation.
> + */
> +struct irq_devres {
> +	unsigned int irq;
> +	void *dev_id;
> +};
> +
> +static void devm_irq_release(struct device *dev, void *res)
> +{
> +	struct irq_devres *this = res;
> +
> +	free_irq(this->irq, this->dev_id);
> +}
> +
> +static int devm_irq_match(struct device *dev, void *res, void *data)
> +{
> +	struct irq_devres *this = res, *match = data;
> +
> +	return this->irq == match->irq && this->dev_id == match->dev_id;
> +}
> +
> +/**
> + *	devm_request_threaded_irq - allocate an interrupt line for a managed device
> + *	@dev: device to request interrupt for
> + *	@irq: Interrupt line to allocate
> + *	@handler: Function to be called when the IRQ occurs
> + *	@thread_fn: function to be called in a threaded interrupt context. NULL
> + *		    for devices which handle everything in @handler
> + *	@irqflags: Interrupt type flags
> + *	@devname: An ascii name for the claiming device
> + *	@dev_id: A cookie passed back to the handler function
> + *
> + *	Except for the extra @dev argument, this function takes the
> + *	same arguments and performs the same function as
> + *	request_irq().  IRQs requested with this function will be
> + *	automatically freed on driver detach.
> + *
> + *	If an IRQ allocated with this function needs to be freed
> + *	separately, devm_free_irq() must be used.
> + */
> +int devm_request_threaded_irq(struct device *dev, unsigned int irq,
> +			      irq_handler_t handler, irq_handler_t thread_fn,
> +			      unsigned long irqflags, const char *devname,
> +			      void *dev_id)
> +{
> +	struct irq_devres *dr;
> +	int rc;
> +
> +	dr = devres_alloc(devm_irq_release, sizeof(struct irq_devres),
> +			  GFP_KERNEL);
> +	if (!dr)
> +		return -ENOMEM;
> +
> +	rc = request_threaded_irq(irq, handler, thread_fn, irqflags, devname,
> +				  dev_id);
> +	if (rc) {
> +		devres_free(dr);
> +		return rc;
> +	}
> +
> +	dr->irq = irq;
> +	dr->dev_id = dev_id;
> +	devres_add(dev, dr);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(devm_request_threaded_irq);
> +
> +/**
> + *	devm_free_irq - free an interrupt
> + *	@dev: device to free interrupt for
> + *	@irq: Interrupt line to free
> + *	@dev_id: Device identity to free
> + *
> + *	Except for the extra @dev argument, this function takes the
> + *	same arguments and performs the same function as free_irq().
> + *	This function instead of free_irq() should be used to manually
> + *	free IRQs allocated with devm_request_irq().
> + */
> +void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id)
> +{
> +	struct irq_devres match_data = { irq, dev_id };
> +
> +	WARN_ON(devres_destroy(dev, devm_irq_release, devm_irq_match,
> +			       &match_data));
> +	free_irq(irq, dev_id);
> +}
> +EXPORT_SYMBOL(devm_free_irq);
> diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
> index fff1738..d98eec9 100644
> --- a/kernel/irq/Makefile
> +++ b/kernel/irq/Makefile
> @@ -1,5 +1,5 @@
>  
> -obj-y := irqdesc.o handle.o manage.o spurious.o resend.o chip.o dummychip.o devres.o
> +obj-y := irqdesc.o handle.o manage.o spurious.o resend.o chip.o dummychip.o
>  obj-$(CONFIG_GENERIC_IRQ_CHIP) += generic-chip.o
>  obj-$(CONFIG_GENERIC_IRQ_PROBE) += autoprobe.o
>  obj-$(CONFIG_IRQ_DOMAIN) += irqdomain.o
> diff --git a/kernel/irq/devres.c b/kernel/irq/devres.c
> deleted file mode 100644
> index bd8e788..0000000
> --- a/kernel/irq/devres.c
> +++ /dev/null
> @@ -1,94 +0,0 @@
> -#include <linux/module.h>
> -#include <linux/interrupt.h>
> -#include <linux/device.h>
> -#include <linux/gfp.h>
> -
> -/*
> - * Device resource management aware IRQ request/free implementation.
> - */
> -struct irq_devres {
> -	unsigned int irq;
> -	void *dev_id;
> -};
> -
> -static void devm_irq_release(struct device *dev, void *res)
> -{
> -	struct irq_devres *this = res;
> -
> -	free_irq(this->irq, this->dev_id);
> -}
> -
> -static int devm_irq_match(struct device *dev, void *res, void *data)
> -{
> -	struct irq_devres *this = res, *match = data;
> -
> -	return this->irq == match->irq && this->dev_id == match->dev_id;
> -}
> -
> -/**
> - *	devm_request_threaded_irq - allocate an interrupt line for a managed device
> - *	@dev: device to request interrupt for
> - *	@irq: Interrupt line to allocate
> - *	@handler: Function to be called when the IRQ occurs
> - *	@thread_fn: function to be called in a threaded interrupt context. NULL
> - *		    for devices which handle everything in @handler
> - *	@irqflags: Interrupt type flags
> - *	@devname: An ascii name for the claiming device
> - *	@dev_id: A cookie passed back to the handler function
> - *
> - *	Except for the extra @dev argument, this function takes the
> - *	same arguments and performs the same function as
> - *	request_irq().  IRQs requested with this function will be
> - *	automatically freed on driver detach.
> - *
> - *	If an IRQ allocated with this function needs to be freed
> - *	separately, devm_free_irq() must be used.
> - */
> -int devm_request_threaded_irq(struct device *dev, unsigned int irq,
> -			      irq_handler_t handler, irq_handler_t thread_fn,
> -			      unsigned long irqflags, const char *devname,
> -			      void *dev_id)
> -{
> -	struct irq_devres *dr;
> -	int rc;
> -
> -	dr = devres_alloc(devm_irq_release, sizeof(struct irq_devres),
> -			  GFP_KERNEL);
> -	if (!dr)
> -		return -ENOMEM;
> -
> -	rc = request_threaded_irq(irq, handler, thread_fn, irqflags, devname,
> -				  dev_id);
> -	if (rc) {
> -		devres_free(dr);
> -		return rc;
> -	}
> -
> -	dr->irq = irq;
> -	dr->dev_id = dev_id;
> -	devres_add(dev, dr);
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(devm_request_threaded_irq);
> -
> -/**
> - *	devm_free_irq - free an interrupt
> - *	@dev: device to free interrupt for
> - *	@irq: Interrupt line to free
> - *	@dev_id: Device identity to free
> - *
> - *	Except for the extra @dev argument, this function takes the
> - *	same arguments and performs the same function as free_irq().
> - *	This function instead of free_irq() should be used to manually
> - *	free IRQs allocated with devm_request_irq().
> - */
> -void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id)
> -{
> -	struct irq_devres match_data = { irq, dev_id };
> -
> -	WARN_ON(devres_destroy(dev, devm_irq_release, devm_irq_match,
> -			       &match_data));
> -	free_irq(irq, dev_id);
> -}
> -EXPORT_SYMBOL(devm_free_irq);
> 


-- 
Chen Gang

Re: [PATCH] kernel/irq/devres.c: move "kernel/irq/devres.c" to "drivers/base/devres_irq.c"

[Greg KH]

On Tue, Jul 23, 2013 at 03:36:04PM +0800, Chen Gang wrote:
> "kernel/irq/devres.c" is a driver extension tool for irq (with devres)
> which is independent on 'GENERIC_HARDIRQS', so it is not suitable to
> still be in "kernel/irq/" which depends on 'GENERIC_HARDIRQS'.
> 
> It is a basic tool for drivers, so can move it to "drivers/base/" to be
> independent on 'GENERIC_HARDIRQS'.
> 
> It is about irq features, so if can not find other more suitable place,
> can still let their declaration in "include/linux/interrupts.h".
> 
> The related error (with randconfig which disable 'GENERIC_HARDIRQS')
> 
>   drivers/built-in.o: In function `dw_dma_probe':
>   (.text+0x3747a): undefined reference to `devm_request_threaded_irq'

Don't fix problems when you are moving files around, that makes it
_very_ hard to review.

Remember, one thing per patch please.

> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  drivers/base/Makefile     |    2 +-
>  drivers/base/devres_irq.c |   94 +++++++++++++++++++++++++++++++++++++++++++++
>  kernel/irq/Makefile       |    2 +-
>  kernel/irq/devres.c       |   94 ---------------------------------------------
>  4 files changed, 96 insertions(+), 96 deletions(-)
>  create mode 100644 drivers/base/devres_irq.c
>  delete mode 100644 kernel/irq/devres.c

Please use git when renaming files so that the move is shown in the git
patch.  As it is, I would have to verify this by hand, and I don't want
to ever have to do that.

Also, I have no problem with the file being where it is.  This is for
irqs, which are handled by the interrupt maintainers, no need to put it
in the driver core, just because it happens to deal with "resources".
We arrange things for ease of maintainability, not always logically :)

thanks,

greg k-h

Re: [PATCH] kernel/irq/devres.c: move "kernel/irq/devres.c" to "drivers/base/devres_irq.c"

[Chen Gang]

On 07/23/2013 11:31 PM, Greg KH wrote:
> On Tue, Jul 23, 2013 at 03:36:04PM +0800, Chen Gang wrote:
>> "kernel/irq/devres.c" is a driver extension tool for irq (with devres)
>> which is independent on 'GENERIC_HARDIRQS', so it is not suitable to
>> still be in "kernel/irq/" which depends on 'GENERIC_HARDIRQS'.
>>
>> It is a basic tool for drivers, so can move it to "drivers/base/" to be
>> independent on 'GENERIC_HARDIRQS'.
>>
>> It is about irq features, so if can not find other more suitable place,
>> can still let their declaration in "include/linux/interrupts.h".
>>
>> The related error (with randconfig which disable 'GENERIC_HARDIRQS')
>>
>>   drivers/built-in.o: In function `dw_dma_probe':
>>   (.text+0x3747a): undefined reference to `devm_request_threaded_irq'
> 
> Don't fix problems when you are moving files around, that makes it
> _very_ hard to review.
> 

OK, thanks, I should notice next time (originally, I really did not know
about it).

Hmm... but for our case, if move the related file, it also can solve the
related issue, it is only one action.

> Remember, one thing per patch please.
> 

OK, thanks, I will try (that may let me make more patches, which is not
bad for myself ;-)).


>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>> ---
>>  drivers/base/Makefile     |    2 +-
>>  drivers/base/devres_irq.c |   94 +++++++++++++++++++++++++++++++++++++++++++++
>>  kernel/irq/Makefile       |    2 +-
>>  kernel/irq/devres.c       |   94 ---------------------------------------------
>>  4 files changed, 96 insertions(+), 96 deletions(-)
>>  create mode 100644 drivers/base/devres_irq.c
>>  delete mode 100644 kernel/irq/devres.c
> 
> Please use git when renaming files so that the move is shown in the git
> patch.  As it is, I would have to verify this by hand, and I don't want
> to ever have to do that.
> 

Oh, thanks, I need use git to perform it (I should try to familiar with
git).

> Also, I have no problem with the file being where it is.  This is for
> irqs, which are handled by the interrupt maintainers, no need to put it
> in the driver core, just because it happens to deal with "resources".
> We arrange things for ease of maintainability, not always logically :)
> 

Hmm... normally, 'maintainability' has no conflict with 'logically', if
we feel they are conflict, that means both of them need improvement.

For 'logically', is it suitable to move "resources" from "drivers/base"
to "lib/", since they are already not only for drivers wide, but also
for kernel wide ? (especially, some of "devm*" have already been in "lib/").

For 'maintainability', is it suitable to let "kernel/irq" independent on
'GENERIC_HARDIRQS' or "mv kernel/irq/devres.c kernel/devres_irq.c" ?

:-)

Thanks.
-- 
Chen Gang

Re: [PATCH] kernel/irq/devres.c: move "kernel/irq/devres.c" to "drivers/base/devres_irq.c"

[Chen Gang]

In the other mail thread, the related maintainer for s390 said:

  "s390 will have GENERIC_HARDIRQS soon (very likely next merge window)"

For efficiency reason, I should stop trying this issue any more.

  I found this issue by building s390 with allmodconfig, but s390 will be OK for this issue soon.
  If no direct cause, it is not a good idea to spend members expensive time resources only for one discussing.


Thanks.

On 07/24/2013 09:59 AM, Chen Gang wrote:
> On 07/23/2013 11:31 PM, Greg KH wrote:
>> On Tue, Jul 23, 2013 at 03:36:04PM +0800, Chen Gang wrote:
>>> "kernel/irq/devres.c" is a driver extension tool for irq (with devres)
>>> which is independent on 'GENERIC_HARDIRQS', so it is not suitable to
>>> still be in "kernel/irq/" which depends on 'GENERIC_HARDIRQS'.
>>>
>>> It is a basic tool for drivers, so can move it to "drivers/base/" to be
>>> independent on 'GENERIC_HARDIRQS'.
>>>
>>> It is about irq features, so if can not find other more suitable place,
>>> can still let their declaration in "include/linux/interrupts.h".
>>>
>>> The related error (with randconfig which disable 'GENERIC_HARDIRQS')
>>>
>>>   drivers/built-in.o: In function `dw_dma_probe':
>>>   (.text+0x3747a): undefined reference to `devm_request_threaded_irq'
>>
>> Don't fix problems when you are moving files around, that makes it
>> _very_ hard to review.
>>
> 
> OK, thanks, I should notice next time (originally, I really did not know
> about it).
> 
> Hmm... but for our case, if move the related file, it also can solve the
> related issue, it is only one action.
> 
>> Remember, one thing per patch please.
>>
> 
> OK, thanks, I will try (that may let me make more patches, which is not
> bad for myself ;-)).
> 
> 
>>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>>> ---
>>>  drivers/base/Makefile     |    2 +-
>>>  drivers/base/devres_irq.c |   94 +++++++++++++++++++++++++++++++++++++++++++++
>>>  kernel/irq/Makefile       |    2 +-
>>>  kernel/irq/devres.c       |   94 ---------------------------------------------
>>>  4 files changed, 96 insertions(+), 96 deletions(-)
>>>  create mode 100644 drivers/base/devres_irq.c
>>>  delete mode 100644 kernel/irq/devres.c
>>
>> Please use git when renaming files so that the move is shown in the git
>> patch.  As it is, I would have to verify this by hand, and I don't want
>> to ever have to do that.
>>
> 
> Oh, thanks, I need use git to perform it (I should try to familiar with
> git).
> 
>> Also, I have no problem with the file being where it is.  This is for
>> irqs, which are handled by the interrupt maintainers, no need to put it
>> in the driver core, just because it happens to deal with "resources".
>> We arrange things for ease of maintainability, not always logically :)
>>
> 
> Hmm... normally, 'maintainability' has no conflict with 'logically', if
> we feel they are conflict, that means both of them need improvement.
> 
> For 'logically', is it suitable to move "resources" from "drivers/base"
> to "lib/", since they are already not only for drivers wide, but also
> for kernel wide ? (especially, some of "devm*" have already been in "lib/").
> 
> For 'maintainability', is it suitable to let "kernel/irq" independent on
> 'GENERIC_HARDIRQS' or "mv kernel/irq/devres.c kernel/devres_irq.c" ?
> 
> :-)
> 
> Thanks.
> 


-- 
Chen Gang