[PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Peter Huewe]

This patch fixes a build failure[1] by converting the driver to use the
new irq_chip functions.
The old functions are deprecated and masked by GENERIC_HARDIRQS_NO__DO_IRQ

Cc: Stable <stable@kernel.org>
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
KernelVersion: linux-20101209

References:
[1] http://kisskb.ellerman.id.au/kisskb/buildresult/3607671/

 drivers/mfd/wm831x-irq.c |   42 +++++++++++++++++++++---------------------
 1 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/mfd/wm831x-irq.c b/drivers/mfd/wm831x-irq.c
index 294183b..018b9ec 100644
--- a/drivers/mfd/wm831x-irq.c
+++ b/drivers/mfd/wm831x-irq.c
@@ -345,16 +345,16 @@ static inline struct wm831x_irq_data *irq_to_wm831x_irq(struct wm831x *wm831x,
 	return &wm831x_irqs[irq - wm831x->irq_base];
 }
 
-static void wm831x_irq_lock(unsigned int irq)
+static void wm831x_irq_lock(struct irq_data *data)
 {
-	struct wm831x *wm831x = get_irq_chip_data(irq);
+	struct wm831x *wm831x = get_irq_chip_data(data->irq);
 
 	mutex_lock(&wm831x->irq_lock);
 }
 
-static void wm831x_irq_sync_unlock(unsigned int irq)
+static void wm831x_irq_sync_unlock(struct irq_data *data)
 {
-	struct wm831x *wm831x = get_irq_chip_data(irq);
+	struct wm831x *wm831x = get_irq_chip_data(data->irq);
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(wm831x->irq_masks_cur); i++) {
@@ -371,32 +371,32 @@ static void wm831x_irq_sync_unlock(unsigned int irq)
 	mutex_unlock(&wm831x->irq_lock);
 }
 
-static void wm831x_irq_unmask(unsigned int irq)
+static void wm831x_irq_unmask(struct irq_data *data)
 {
-	struct wm831x *wm831x = get_irq_chip_data(irq);
-	struct wm831x_irq_data *irq_data = irq_to_wm831x_irq(wm831x, irq);
+	struct wm831x *wm831x = get_irq_chip_data(data->irq);
+	struct wm831x_irq_data *irq_data = irq_to_wm831x_irq(wm831x, data->irq);
 
 	wm831x->irq_masks_cur[irq_data->reg - 1] &= ~irq_data->mask;
 }
 
-static void wm831x_irq_mask(unsigned int irq)
+static void wm831x_irq_mask(struct irq_data *data)
 {
-	struct wm831x *wm831x = get_irq_chip_data(irq);
-	struct wm831x_irq_data *irq_data = irq_to_wm831x_irq(wm831x, irq);
+	struct wm831x *wm831x = get_irq_chip_data(data->irq);
+	struct wm831x_irq_data *irq_data = irq_to_wm831x_irq(wm831x, data->irq);
 
 	wm831x->irq_masks_cur[irq_data->reg - 1] |= irq_data->mask;
 }
 
-static int wm831x_irq_set_type(unsigned int irq, unsigned int type)
+static int wm831x_irq_set_type(struct irq_data *data, unsigned int type)
 {
-	struct wm831x *wm831x = get_irq_chip_data(irq);
+	struct wm831x *wm831x = get_irq_chip_data(data->irq);
 	int val;
 
-	irq = irq - wm831x->irq_base;
+	data->irq = data->irq - wm831x->irq_base;
 
-	if (irq < WM831X_IRQ_GPIO_1 || irq > WM831X_IRQ_GPIO_11) {
+	if (data->irq < WM831X_IRQ_GPIO_1 || data->irq > WM831X_IRQ_GPIO_11) {
 		/* Ignore internal-only IRQs */
-		if (irq >= 0 && irq < WM831X_NUM_IRQS)
+		if (data->irq >= 0 && data->irq < WM831X_NUM_IRQS)
 			return 0;
 		else
 			return -EINVAL;
@@ -416,17 +416,17 @@ static int wm831x_irq_set_type(unsigned int irq, unsigned int type)
 		return -EINVAL;
 	}
 
-	return wm831x_set_bits(wm831x, WM831X_GPIO1_CONTROL + irq,
+	return wm831x_set_bits(wm831x, WM831X_GPIO1_CONTROL + data->irq,
 			       WM831X_GPN_INT_MODE | WM831X_GPN_POL, val);
 }
 
 static struct irq_chip wm831x_irq_chip = {
 	.name = "wm831x",
-	.bus_lock = wm831x_irq_lock,
-	.bus_sync_unlock = wm831x_irq_sync_unlock,
-	.mask = wm831x_irq_mask,
-	.unmask = wm831x_irq_unmask,
-	.set_type = wm831x_irq_set_type,
+	.irq_bus_lock = wm831x_irq_lock,
+	.irq_bus_sync_unlock = wm831x_irq_sync_unlock,
+	.irq_mask = wm831x_irq_mask,
+	.irq_unmask = wm831x_irq_unmask,
+	.irq_set_type = wm831x_irq_set_type,
 };
 
 /* The processing of the primary interrupt occurs in a thread so that
-- 
1.7.2.2

Re: [PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Thomas Gleixner]

On Fri, 10 Dec 2010, Mark Brown wrote:

> On 9 Dec 2010, at 23:34, Peter Huewe wrote:
> 
> > This patch fixes a build failure[1] by converting the driver to use the
> > new irq_chip functions.
> 
> The driver has already been converted; if this is required we should
> just backport the relevant change (mfd: Convert WM831x to new irq_
> interrupt methods) in order to avoid merge issues with current code
> rather than reimplementing. However, at this stage in 2.6.37 I don't
> think that's a good idea.

I think it is.

# git grep GENERIC_HARDIRQS_NO_DEPRECATED arch/
arch/sh/Kconfig:        select GENERIC_HARDIRQS_NO_DEPRECATED

Though the question remains, whether this driver is actually used with
sh platforms. If yes, then pushing the already existing change to
Linus is the right way to go. If no, adding a "depends on !SH" is the
simple fix to prevent this allmodconfig fallout. Another option is to
ignore it :) I leave that up to Paul.
 
> > The old functions are deprecated and masked by GENERIC_HARDIRQS_NO__DO_IRQ
> 
> 
> Are you sure it's that option and not GENERIC_HARDIRQS_NO_DEPRECATED? 

Right that changelog sucks. :)

> > Cc: Stable <stable@kernel.org>
> 
> Why are you CCing to stable? This API was only introduced in 2.6.37 which is still
> in -rc and attempting to backport to stable releases would cause those releases to
> fail to build.

Right, thats useless.

Thanks,

	tglx

Re: [PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Paul Mundt]

On Fri, Dec 10, 2010 at 02:39:55AM +0100, Thomas Gleixner wrote:
> On Fri, 10 Dec 2010, Mark Brown wrote:
> > On 9 Dec 2010, at 23:34, Peter Huewe wrote:
> > > This patch fixes a build failure[1] by converting the driver to use the
> > > new irq_chip functions.
> > 
> > The driver has already been converted; if this is required we should
> > just backport the relevant change (mfd: Convert WM831x to new irq_
> > interrupt methods) in order to avoid merge issues with current code
> > rather than reimplementing. However, at this stage in 2.6.37 I don't
> > think that's a good idea.
> 
> I think it is.
> 
> # git grep GENERIC_HARDIRQS_NO_DEPRECATED arch/
> arch/sh/Kconfig:        select GENERIC_HARDIRQS_NO_DEPRECATED
> 
> Though the question remains, whether this driver is actually used with
> sh platforms. If yes, then pushing the already existing change to
> Linus is the right way to go. If no, adding a "depends on !SH" is the
> simple fix to prevent this allmodconfig fallout. Another option is to
> ignore it :) I leave that up to Paul.
>  
There are no current SH boards that are using this MFD, but there's
certainly no technical reason for why there can't be. I'd rather avoid an
artificial !SH dependency, but adding in something like

	depends on !GENERIC_HARDIRQS_NO_DEPRECATED

for .37 would be fine. We do build randconfigs and so on quite regularly,
so it would obviously be nice not to have this break the build.

Re: [PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Mark Brown]

On Fri, Dec 10, 2010 at 02:07:55PM +0900, Paul Mundt wrote:
> On Fri, Dec 10, 2010 at 02:39:55AM +0100, Thomas Gleixner wrote:
> > On Fri, 10 Dec 2010, Mark Brown wrote:

> > > The driver has already been converted; if this is required we should
> > > just backport the relevant change (mfd: Convert WM831x to new irq_
> > > interrupt methods) in order to avoid merge issues with current code
> > > rather than reimplementing. However, at this stage in 2.6.37 I don't
> > > think that's a good idea.

> > I think it is.

> > # git grep GENERIC_HARDIRQS_NO_DEPRECATED arch/
> > arch/sh/Kconfig:        select GENERIC_HARDIRQS_NO_DEPRECATED

Oh dear.  Can anyone comment on why SH is selecting this?  My first
thought is that the savings from enabling it are going to be on the
small side so it wouldn't be a big issue to leave it off for 2.6.37
and possibly 2.6.38 also.

If we're going to start enabling this on platforms I'd also suggest that
we enable it on x86 in -next so that we get reasonable coverage from
build tests.  It needs to be enabled on a major architecture to catch
the change during development.

> > Though the question remains, whether this driver is actually used with
> > sh platforms. If yes, then pushing the already existing change to

It's not just this driver - I'd expect everything with an interrupt
controller in drivers/mfd to have an issue here, and I don't think
disabling them all on SH for this release is such a good approach.

> > Linus is the right way to go. If no, adding a "depends on !SH" is the
> > simple fix to prevent this allmodconfig fallout. Another option is to
> > ignore it :) I leave that up to Paul.

> There are no current SH boards that are using this MFD, but there's
> certainly no technical reason for why there can't be. I'd rather avoid an
> artificial !SH dependency, but adding in something like

> 	depends on !GENERIC_HARDIRQS_NO_DEPRECATED

> for .37 would be fine. We do build randconfigs and so on quite regularly,
> so it would obviously be nice not to have this break the build.

I'd rather not do that, certainly not for everything.

Re: [PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Paul Mundt]

On Fri, Dec 10, 2010 at 12:14:21PM +0000, Mark Brown wrote:
> On Fri, Dec 10, 2010 at 02:07:55PM +0900, Paul Mundt wrote:
> > On Fri, Dec 10, 2010 at 02:39:55AM +0100, Thomas Gleixner wrote:
> > > # git grep GENERIC_HARDIRQS_NO_DEPRECATED arch/
> > > arch/sh/Kconfig:        select GENERIC_HARDIRQS_NO_DEPRECATED
> 
> Oh dear.  Can anyone comment on why SH is selecting this?  My first
> thought is that the savings from enabling it are going to be on the
> small side so it wouldn't be a big issue to leave it off for 2.6.37
> and possibly 2.6.38 also.
> 
The "savings" are largely triggering these sorts of issues, where anyone
using deprecated functionality blows up the build and gets fixed up
incrementally, as well as stopping people from adding new users of the
deprecated API.

> If we're going to start enabling this on platforms I'd also suggest that
> we enable it on x86 in -next so that we get reasonable coverage from
> build tests.  It needs to be enabled on a major architecture to catch
> the change during development.
> 
Yes, well, now you've gotten reports on it being a build issue and your
first thought is to jut disable the option instead of marking the
deprecated API dependence as explicit in the driver? This has absolutely
nothing to do with development, it has to do with the fact the driver is
using an API that is flagged as deprecated, and going forward
architectures are going to begin to opt-in to having the deprecated parts
of the generic hardirq framework disabled outright. SH happened to be the
first to get there, but others will follow suit.

> > > Though the question remains, whether this driver is actually used with
> > > sh platforms. If yes, then pushing the already existing change to
> 
> It's not just this driver - I'd expect everything with an interrupt
> controller in drivers/mfd to have an issue here, and I don't think
> disabling them all on SH for this release is such a good approach.
> 
Again, none of this has anything to do with SH, so pretending like it an
SH "issue" is disingenuous at best. All of the offending drivers already
have a GENERIC_HARDIRQS dependency, adding a dependency that also depends
on the deprecated support for each of these doesn't exactly strike me as
being an undue burden. It is after all your driver and not my
architecture that depends on deprecated support.

> > There are no current SH boards that are using this MFD, but there's
> > certainly no technical reason for why there can't be. I'd rather avoid an
> > artificial !SH dependency, but adding in something like
> 
> > 	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
> 
> > for .37 would be fine. We do build randconfigs and so on quite regularly,
> > so it would obviously be nice not to have this break the build.
> 
> I'd rather not do that, certainly not for everything.

You depend on deprecated support, so what exactly is the issue? Are you
just not going to fix this until an architecture you are building for
happens to trigger this for you instead? You depend on a deprecated
subset of the generic hardirqs framework that has a matching Kconfig
symbol associated with it, I'm not sure how much more black and white you
want the dependency chain to be. Ideally these should have all been
flagged with a deprecated dependency when irq_chip got overhauled, but
some things do slip through.

In any event, I'm certainly not going to re-enable deprecated support to
satisfy a bunch of crap drivers with broken dependencies.

Re: [PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Paul Mundt]

On Sat, Dec 11, 2010 at 12:43:32AM +0900, Paul Mundt wrote:
> On Fri, Dec 10, 2010 at 12:14:21PM +0000, Mark Brown wrote:
> > It's not just this driver - I'd expect everything with an interrupt
> > controller in drivers/mfd to have an issue here, and I don't think
> > disabling them all on SH for this release is such a good approach.
> > 
> Again, none of this has anything to do with SH, so pretending like it an
> SH "issue" is disingenuous at best. All of the offending drivers already
> have a GENERIC_HARDIRQS dependency, adding a dependency that also depends
> on the deprecated support for each of these doesn't exactly strike me as
> being an undue burden. It is after all your driver and not my
> architecture that depends on deprecated support.
> 
So, I decided to gather some statistics..

Out of the current MFD drivers, the current users with dependency on
deprecated APIs and broken GENERIC_HARDIRQS dependencies are:

88pm860x-core.c
ab3550-core.c
ab8500-core.c
asic3.c
ezx-pcap.c
htc-egpio.c
htc-i2cpld.c
jz4740-adc.c
max8925-core.c
max8998-irq.c
stmpe.c
t7l66xb.c
tc35892.c
tc6393xb.c
tps6586x.c
twl4030-irq.c
twl6030-irq.c
wm831x-irq.c
wm8350-irq.c
wm8994-irq.c

Out of these, they fall in to two categories..

With no special platform dependencies, which are already broken for
platforms without GENERIC_HARDIRQS:

	htc-i2cpld.c

With platform dependencies that implicitly provide GENERIC_HARDIRQS:

	jz4740-adc.c, t7l66xb.c, and tc6393xb.c

That's a reasonable chunk of the MFD drivers that have the deprecated
dependency, but there are many that this isn't a problem for already, too.
(including all of the ones we use on SH). The rest are easily converted
over for .38.

The above 4 at least should have their dependencies tidied up. The rest
of them will break on an allmod/yesconfig for SH for .37, which the
following patch addresses. On the other hand, none of these are really
all that pressing, particularly as none of these are used by any of the
SH defconfigs.

I had considered adding a GENERIC_HARDIRQS_DEPRECATED, but that
unfortunately gets to be a bit tricky since almost no one sources
kernel/irq/Kconfig yet, and this isn't likely to be a problem for very
long anyways -- it's basically just the MFD drivers that play around with
the irq_chip anyways.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

---

 drivers/mfd/Kconfig |   22 +++++++++++++++++++---
 1 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3a1493b..2d783a3 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -27,6 +27,7 @@ config MFD_CORE
 config MFD_88PM860X
 	bool "Support Marvell 88PM8606/88PM8607"
 	depends on I2C=y && GENERIC_HARDIRQS
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	select MFD_CORE
 	help
 	  This supports for Marvell 88PM8606/88PM8607 Power Management IC.
@@ -55,6 +56,7 @@ config MFD_SM501_GPIO
 config MFD_ASIC3
 	bool "Support for Compaq ASIC3"
 	depends on GENERIC_HARDIRQS && GPIOLIB && ARM
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	select MFD_CORE
 	 ---help---
 	  This driver supports the ASIC3 multifunction chip found on many
@@ -84,6 +86,7 @@ config MFD_DM355EVM_MSP
 config HTC_EGPIO
 	bool "HTC EGPIO support"
 	depends on GENERIC_HARDIRQS && GPIOLIB && ARM
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	help
 	    This driver supports the CPLD egpio chip present on
 	    several HTC phones.  It provides basic support for input
@@ -101,6 +104,8 @@ config HTC_PASIC3
 config HTC_I2CPLD
 	bool "HTC I2C PLD chip support"
 	depends on I2C=y && GPIOLIB
+	depends on GENERIC_HARDIRQS
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	help
 	  If you say yes here you get support for the supposed CPLD
 	  found on omap850 HTC devices like the HTC Wizard and HTC Herald.
@@ -156,6 +161,7 @@ config MENELAUS
 config TWL4030_CORE
 	bool "Texas Instruments TWL4030/TWL5030/TWL6030/TPS659x0 Support"
 	depends on I2C=y && GENERIC_HARDIRQS
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	help
 	  Say yes here if you have TWL4030 / TWL6030 family chip on your board.
 	  This core driver provides register access and IRQ handling
@@ -198,6 +204,7 @@ config TWL6030_PWM
 config MFD_STMPE
 	bool "Support STMicroelectronics STMPE"
 	depends on I2C=y && GENERIC_HARDIRQS
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	select MFD_CORE
 	help
 	  Support for the STMPE family of I/O Expanders from
@@ -221,6 +228,7 @@ config MFD_STMPE
 config MFD_TC35892
 	bool "Support Toshiba TC35892"
 	depends on I2C=y && GENERIC_HARDIRQS
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	select MFD_CORE
 	help
 	  Support for the Toshiba TC35892 I/O Expander.
@@ -286,6 +294,7 @@ config PMIC_ADP5520
 config MFD_MAX8925
 	bool "Maxim Semiconductor MAX8925 PMIC Support"
 	depends on I2C=y && GENERIC_HARDIRQS
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	select MFD_CORE
 	help
 	  Say yes here to support for Maxim Semiconductor MAX8925. This is
@@ -296,6 +305,7 @@ config MFD_MAX8925
 config MFD_MAX8998
 	bool "Maxim Semiconductor MAX8998/National LP3974 PMIC Support"
 	depends on I2C=y && GENERIC_HARDIRQS
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	select MFD_CORE
 	help
 	  Say yes here to support for Maxim Semiconductor MAX8998 and
@@ -316,13 +326,13 @@ config MFD_WM8400
 
 config MFD_WM831X
 	bool
-	depends on GENERIC_HARDIRQS
+	depends on GENERIC_HARDIRQS && !GENERIC_HARDIRQS_NO_DEPRECATED
 
 config MFD_WM831X_I2C
 	bool "Support Wolfson Microelectronics WM831x/2x PMICs with I2C"
 	select MFD_CORE
 	select MFD_WM831X
-	depends on I2C=y && GENERIC_HARDIRQS
+	depends on I2C=y && GENERIC_HARDIRQS && !GENERIC_HARDIRQS_NO_DEPRECATED
 	help
 	  Support for the Wolfson Microelecronics WM831x and WM832x PMICs
 	  when controlled using I2C.  This driver provides common support
@@ -334,6 +344,7 @@ config MFD_WM831X_SPI
 	select MFD_CORE
 	select MFD_WM831X
 	depends on SPI_MASTER && GENERIC_HARDIRQS
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	help
 	  Support for the Wolfson Microelecronics WM831x and WM832x PMICs
 	  when controlled using SPI.  This driver provides common support
@@ -342,7 +353,7 @@ config MFD_WM831X_SPI
 
 config MFD_WM8350
 	bool
-	depends on GENERIC_HARDIRQS
+	depends on GENERIC_HARDIRQS && !GENERIC_HARDIRQS_NO_DEPRECATED
 
 config MFD_WM8350_CONFIG_MODE_0
 	bool
@@ -396,6 +407,7 @@ config MFD_WM8350_I2C
 	bool "Support Wolfson Microelectronics WM8350 with I2C"
 	select MFD_WM8350
 	depends on I2C=y && GENERIC_HARDIRQS
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	help
 	  The WM8350 is an integrated audio and power management
 	  subsystem with watchdog and RTC functionality for embedded
@@ -407,6 +419,7 @@ config MFD_WM8994
 	bool "Support Wolfson Microelectronics WM8994"
 	select MFD_CORE
 	depends on I2C=y && GENERIC_HARDIRQS
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	help
 	  The WM8994 is a highly integrated hi-fi CODEC designed for
 	  smartphone applicatiosn.  As well as audio functionality it
@@ -490,6 +503,7 @@ config AB3100_OTP
 config EZX_PCAP
 	bool "PCAP Support"
 	depends on GENERIC_HARDIRQS && SPI_MASTER
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	help
 	  This enables the PCAP ASIC present on EZX Phones. This is
 	  needed for MMC, TouchScreen, Sound, USB, etc..
@@ -527,6 +541,7 @@ config AB3550_CORE
         bool "ST-Ericsson AB3550 Mixed Signal Circuit core functions"
 	select MFD_CORE
 	depends on I2C=y && GENERIC_HARDIRQS && ABX500_CORE
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	help
 	  Select this to enable the AB3550 Mixed Signal IC core
 	  functionality. This connects to a AB3550 on the I2C bus
@@ -586,6 +601,7 @@ config MFD_JZ4740_ADC
 config MFD_TPS6586X
 	bool "TPS6586x Power Management chips"
 	depends on I2C=y && GPIOLIB && GENERIC_HARDIRQS
+	depends on !GENERIC_HARDIRQS_NO_DEPRECATED
 	select MFD_CORE
 	help
 	  If you say yes here you get support for the TPS6586X series of

Re: [PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Mark Brown]

On Sat, Dec 11, 2010 at 02:01:31AM +0900, Paul Mundt wrote:

> The above 4 at least should have their dependencies tidied up. The rest
> of them will break on an allmod/yesconfig for SH for .37, which the
> following patch addresses. On the other hand, none of these are really
> all that pressing, particularly as none of these are used by any of the
> SH defconfigs.

> I had considered adding a GENERIC_HARDIRQS_DEPRECATED, but that
> unfortunately gets to be a bit tricky since almost no one sources
> kernel/irq/Kconfig yet, and this isn't likely to be a problem for very
> long anyways -- it's basically just the MFD drivers that play around with
> the irq_chip anyways.

Please remove the changes for the Wolfson drivers, as I said in my
previous mails if you insist that this must be changed in 2.6.37 I would
much rather cherry-pick the existing changes over into 2.6.37 than have
them unusuable on SH for a release (and we'd need to fix -next to remove
the dependency anyway).  

If you're checking this stuff it'd be good to also take a look at
drivers/gpio - there's a bunch of irq_chips in there too.  I would also
urge you to also contact the maintainers for the relevant drivers while
doing these checks, I suspect quite a few people aren't aware of the new
APIs yet.

Re: [PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Mark Brown]

On Sat, Dec 11, 2010 at 12:43:32AM +0900, Paul Mundt wrote:
> On Fri, Dec 10, 2010 at 12:14:21PM +0000, Mark Brown wrote:

> > Oh dear.  Can anyone comment on why SH is selecting this?  My first
> > thought is that the savings from enabling it are going to be on the
> > small side so it wouldn't be a big issue to leave it off for 2.6.37
> > and possibly 2.6.38 also.

> The "savings" are largely triggering these sorts of issues, where anyone
> using deprecated functionality blows up the build and gets fixed up
> incrementally, as well as stopping people from adding new users of the
> deprecated API.

OK, so disabling this for 2.6.37 and reenabling in -next wouldn't cause
any substantial disrupton to SH?  As I say I would also argue in favour
of enabling on x86 if we're pushing the changeover aggressively (Thomas
did indicate that he wants to do this, I'd send a patch but I'm unlikely
to have the time to do sufficient build testts on that platform).

> > If we're going to start enabling this on platforms I'd also suggest that
> > we enable it on x86 in -next so that we get reasonable coverage from
> > build tests.  It needs to be enabled on a major architecture to catch
> > the change during development.

> Yes, well, now you've gotten reports on it being a build issue and your
> first thought is to jut disable the option instead of marking the
> deprecated API dependence as explicit in the driver? This has absolutely

No, not at all.  As I mentioned in my previous posting this particular
driver has already been converted to use the new API (I posted the
patches about a month ago), as have all the other drivers I'm
responsible for.  What I'm saying is that doing the conversion for this
and all the other affected drivers at this point in the 2.6.37 release
cycle seems rather invasive.

The changes should be straightforward enough, and I'm reasonably happy
those I've implemented have had enough testing to be backported, but
there's a reasonable number of other drivers that are affected and the
changes aren't quite trivial enough to make the balance of risk from
introducing them right now seem good to me.

> nothing to do with development, it has to do with the fact the driver is
> using an API that is flagged as deprecated, and going forward
> architectures are going to begin to opt-in to having the deprecated parts
> of the generic hardirq framework disabled outright. SH happened to be the
> first to get there, but others will follow suit.

The API was introduced and the old one flagged as deprecated during the
2.6.37 merge window so SH has been rather...  prompt in implementing and
enforcing the conversion.  2.6.37-rc1 was the first kernel that had the
new operations for drivers to use so implementations of this would have
had to go into Linus-destined trees after the merge window or done cross
tree merges with the genirq tree prior to then.  The normal expectation
with such an API change would be that conversions would be done once the
change had propagated through Linus' tree into the relevant development
tree for the driver and only appear in mainline in the following merge
window.

I agree we should do the conversion, and if you look in -next you'll see
that I've been doing active work on carrying out the conversion for
platforms and devices I've been working on (Lennert Buytenhek did some
patches covering arch/arm so that ground to a halt a bit), but it really
doesn't seem at all realistic to expect that all cross-platform drivers
will have been converted in 2.6.37.

> > It's not just this driver - I'd expect everything with an interrupt
> > controller in drivers/mfd to have an issue here, and I don't think
> > disabling them all on SH for this release is such a good approach.

> Again, none of this has anything to do with SH, so pretending like it an
> SH "issue" is disingenuous at best. All of the offending drivers already
> have a GENERIC_HARDIRQS dependency, adding a dependency that also depends
> on the deprecated support for each of these doesn't exactly strike me as
> being an undue burden. It is after all your driver and not my
> architecture that depends on deprecated support.

It's an SH issue in the sense that SH has turned this option on in
Linus' tree very quickly, causing integration issues.

> > > for .37 would be fine. We do build randconfigs and so on quite regularly,
> > > so it would obviously be nice not to have this break the build.

> > I'd rather not do that, certainly not for everything.

> You depend on deprecated support, so what exactly is the issue? Are you
> just not going to fix this until an architecture you are building for
> happens to trigger this for you instead? You depend on a deprecated

Not at all; like I say the conversion for this particular driver is
already done but in -next rather than in Linus' tree.

> subset of the generic hardirqs framework that has a matching Kconfig
> symbol associated with it, I'm not sure how much more black and white you
> want the dependency chain to be. Ideally these should have all been
> flagged with a deprecated dependency when irq_chip got overhauled, but
> some things do slip through.

I agree that when the Kconfig option was introduced it should also have
included updates to all the relevant IRQ controller drivers and/and or
platforms, though reading the changelog for the relevant commit it seems 
clear that the intention was to provide a testing facility rather than
have the option turned on immediately so it's easy to understand why
that happened.

> In any event, I'm certainly not going to re-enable deprecated support to
> satisfy a bunch of crap drivers with broken dependencies.

I think your characterisation of unconverted drivers as "crap" is
unreasonable.  As far as I can tell the first hint that any of the
affected maintainers had that SH (or any other platform) would be
making the conversion mandatory was Peter's patch and as I discussed
above the fact that the change was only introduced into mainline during
the merge window would make conversion by the driver maintainers for
2.6.37 surprising.  Had this been so urgent that it should be done
immediately I'd have expected to see active efforts being made to push
implementation the conversion, starting from before the last merge
window, but that didn't happen at all.

Please reconsider in view of all the above.  I've no personal problem
with introducing this in -next, though I would welcome more active
efforts from platforms enabling the option to test, but I have issues
with doing so in 2.6.37.  If you refuse to disable this in the SH config
I'd much rather cherry pick the relevant changes over to Linus' tree
than leave them broken on SH for a release, though I'd personally not be
too happy about doing the same for other things where the patches
haven't been kicking around in -next.

Re: [PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Paul Mundt]

On Fri, Dec 10, 2010 at 05:24:08PM +0000, Mark Brown wrote:
> On Sat, Dec 11, 2010 at 12:43:32AM +0900, Paul Mundt wrote:
> > The "savings" are largely triggering these sorts of issues, where anyone
> > using deprecated functionality blows up the build and gets fixed up
> > incrementally, as well as stopping people from adding new users of the
> > deprecated API.
> 
> OK, so disabling this for 2.6.37 and reenabling in -next wouldn't cause
> any substantial disrupton to SH?  As I say I would also argue in favour
> of enabling on x86 if we're pushing the changeover aggressively (Thomas
> did indicate that he wants to do this, I'd send a patch but I'm unlikely
> to have the time to do sufficient build testts on that platform).
> 
The conversion for x86 ought to be pretty straightforward, it's nowhere
near the minefield that most of the embedded architectures are with
regards to IRQ chip implementations. It's probably a bit late to run
around auditing all the drivers for .37 though, this is probably
something we should have done during -rc2 or so. It's certainly
reasonable for the .38 window, though.

> No, not at all.  As I mentioned in my previous posting this particular
> driver has already been converted to use the new API (I posted the
> patches about a month ago), as have all the other drivers I'm
> responsible for.  What I'm saying is that doing the conversion for this
> and all the other affected drivers at this point in the 2.6.37 release
> cycle seems rather invasive.
> 
Yes, agreed.

> The API was introduced and the old one flagged as deprecated during the
> 2.6.37 merge window so SH has been rather...  prompt in implementing and
> enforcing the conversion.  2.6.37-rc1 was the first kernel that had the
> new operations for drivers to use so implementations of this would have
> had to go into Linus-destined trees after the merge window or done cross
> tree merges with the genirq tree prior to then.  The normal expectation
> with such an API change would be that conversions would be done once the
> change had propagated through Linus' tree into the relevant development
> tree for the driver and only appear in mainline in the following merge
> window.
> 
Basically what happened is this came about as a result of the sparseirq
refactoring that was going on in the genirq tree. I was hacking on and
converting over in preparation for the merge window, so this was all done
well in advance of -rc1. In hindsight, yes, we probably could have done a
driver audit for the irq_chip users at the same time the rest of the
refactoring was going on, but I'm also willing to accept some early
adopter pain.

I have no intention of dropping the select from SH, but I'm not going to
insist that these drivers have the deprecated dependency if we're a) not
really using them and b) there's a reasonable expectation that they'll
basically be taken care of in .38 anyways.

I haven't been following the progress in -next, but now that you've
pointed it out I'll give it a look. It's less effort to just fix up the
remaining users than it is to audit API dependencies for all of them at
least.

Re: [PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Mark Brown]

On Sat, Dec 11, 2010 at 02:48:43AM +0900, Paul Mundt wrote:

> I have no intention of dropping the select from SH, but I'm not going to
> insist that these drivers have the deprecated dependency if we're a) not
> really using them and b) there's a reasonable expectation that they'll
> basically be taken care of in .38 anyways.

That's unfortunate, I'm a bit concerned about support for users picking
up the kernel and using it to build products.

Samuel, would you be OK with cherry picking the relevant commits to the
Wolfson drivers back into .37?  I'm especially concerned about WM8994
here - I'd really not like to see a kernel version go out where that
doesn't work.

> I haven't been following the progress in -next, but now that you've
> pointed it out I'll give it a look. It's less effort to just fix up the
> remaining users than it is to audit API dependencies for all of them at
> least.

Probably slightly more in that you can't just blindly add the dependency
when you see the problem but yeah.

Re: [PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Paul Mundt]

On Fri, Dec 10, 2010 at 06:24:55PM +0000, Mark Brown wrote:
> On Sat, Dec 11, 2010 at 02:48:43AM +0900, Paul Mundt wrote:
> 
> > I have no intention of dropping the select from SH, but I'm not going to
> > insist that these drivers have the deprecated dependency if we're a) not
> > really using them and b) there's a reasonable expectation that they'll
> > basically be taken care of in .38 anyways.
> 
> That's unfortunate, I'm a bit concerned about support for users picking
> up the kernel and using it to build products.
> 
As I pointed out initially, backtracking would only encourage people to
continue to add new code that uses deprecated interfaces. This happens
time and time again, and is a far greater concern.

> Samuel, would you be OK with cherry picking the relevant commits to the
> Wolfson drivers back into .37?  I'm especially concerned about WM8994
> here - I'd really not like to see a kernel version go out where that
> doesn't work.
> 
That hardly addresses the issue, and simply covers your own driver. The
issue at hand is whether it's worth flagging the deprecated API users
with an explicit dependency or not. You've dismissed the idea of getting
your dependencies right out of hand, but also don't wish to ship a broken
driver, so we need an alternative.

After a full tree audit it's the MFD drivers and a couple of GPIO
expanders that could theoretically be enabled and break the build. I
suppose I could switch to

	select GENERIC_HARDIRQS_NO_DEPRECATED if !MFD_SUPPORT && !GPIOLIB

for .37 since it's too late to convert the remaining drivers now.
Conditionalizing it will at least limit the number of platforms where the
old API is visible.

> > I haven't been following the progress in -next, but now that you've
> > pointed it out I'll give it a look. It's less effort to just fix up the
> > remaining users than it is to audit API dependencies for all of them at
> > least.
> 
> Probably slightly more in that you can't just blindly add the dependency
> when you see the problem but yeah.

At what point is a dependency being "blindly" added? The patch provided
was meant as a stop-gap for .37 so that the drivers using the deprecated
API have an explicit dependency on it. For .38 it's trivial to convert
them, and as I've done pretty much all of the conversion work that's
upstream right now I don't think you're in much position to say what has
and hasn't been thought out.

Re: [PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Samuel Ortiz]

Hi Paul,

On Sat, Dec 11, 2010 at 10:59:08AM +0900, Paul Mundt wrote:
> On Fri, Dec 10, 2010 at 06:24:55PM +0000, Mark Brown wrote:
> > On Sat, Dec 11, 2010 at 02:48:43AM +0900, Paul Mundt wrote:
> > 
> > > I have no intention of dropping the select from SH, but I'm not going to
> > > insist that these drivers have the deprecated dependency if we're a) not
> > > really using them and b) there's a reasonable expectation that they'll
> > > basically be taken care of in .38 anyways.
> > 
> > That's unfortunate, I'm a bit concerned about support for users picking
> > up the kernel and using it to build products.
> > 
> As I pointed out initially, backtracking would only encourage people to
> continue to add new code that uses deprecated interfaces. This happens
> time and time again, and is a far greater concern.
> 
> > Samuel, would you be OK with cherry picking the relevant commits to the
> > Wolfson drivers back into .37?  I'm especially concerned about WM8994
> > here - I'd really not like to see a kernel version go out where that
> > doesn't work.
> > 
> That hardly addresses the issue, and simply covers your own driver. The
> issue at hand is whether it's worth flagging the deprecated API users
> with an explicit dependency or not. You've dismissed the idea of getting
> your dependencies right out of hand, but also don't wish to ship a broken
> driver, so we need an alternative.
> 
> After a full tree audit it's the MFD drivers and a couple of GPIO
> expanders that could theoretically be enabled and break the build. I
> suppose I could switch to
> 
> 	select GENERIC_HARDIRQS_NO_DEPRECATED if !MFD_SUPPORT && !GPIOLIB
> 
> for .37 since it's too late to convert the remaining drivers now.
I see that you've pushed this one, thanks a lot.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH] mfd/wm831x-irq: Convert to new irq_chip functions and fix build failure

[Peter Hüwe]

Am Freitag 10 Dezember 2010, 02:09:00 schrieb Mark Brown:
> > Cc: Stable <stable@kernel.org>
> 
> Why are you CCing to stable? This API was only introduced in 2.6.37 which
> is still in -rc and attempting to backport to stable releases would cause
> those releases to fail to build.

The driver obviously fails since October 30 in linus tree [1], that's why I 
thought it might still have gone into .36
If not I'm sorry about the cc.

Seems like I got a little confused about that - sorry.
I'm doing this only in my leisure time and tracking the whole lkml and 
subprojects is quite a lot ;) 
I'm doing my usual checks with git, but sometimes things slip.

It seems the right thing for me to drop that patch at this time, since there 
is some actual development going on here. 
(I just don't like build failures in Linus' tree ;)

But I'd like to add that there are still many other drivers, especially in 
mtd, which still break - but mark already seems to have them on his list.


Peter

References:
[1] http://kisskb.ellerman.id.au/kisskb/buildresult/3423418/
[2] http://kisskb.ellerman.id.au/kisskb/buildresult/3607680/