Skip to contentSkip to conversation

mfd: kconfig exposing unbuildable driver

PATCH In review Harvey Harrison 7 participants 13 messages Apr 22, 2008 – Apr 23, 2008 Multifunction devices (MFD)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mfd: kconfig exposing unbuildable driver

Limit this driver to ARCH_PXA, noticed by breakage on allyesconfig builds on 32-bit x86.

---
Linus, please apply and Russell can fix it up however he likes later in the week. Many people are hitting this.

 drivers/mfd/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/mfd/Kconfig +1 -0
index 2566479..bd29c61 100644
@@ -32,6 +32,7 @@ config HTC_EGPIO
 
 config HTC_PASIC3
 	tristate "HTC PASIC3 LED/DS1WM chip support"
+	depends on ARCH_PXA
 	help
 	  This core driver provides register access for the LED/DS1WM
 	  chips labeled "AIC2" and "AIC3", found on HTC Blueangel and
Signature · 1 line
1.5.5.144.g3e42
#1 lore raw

That was my initial approach as well, which got shot down by Andrew Morton and others as being unacceptable.

Quoted 28 lines · Harvey Harrison

Limit this driver to ARCH_PXA, noticed by breakage on allyesconfig builds on 32-bit x86.

---
Linus, please apply and Russell can fix it up however he likes later in the week. Many people are hitting this.

 drivers/mfd/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/mfd/Kconfig +1 -0
index 2566479..bd29c61 100644
@@ -32,6 +32,7 @@ config HTC_EGPIO
 
 config HTC_PASIC3
 	tristate "HTC PASIC3 LED/DS1WM chip support"
+	depends on ARCH_PXA
 	help
 	  This core driver provides register access for the LED/DS1WM
 	  chips labeled "AIC2" and "AIC3", found on HTC Blueangel and
Signature · 1 line
1.5.5.144.g3e42
Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
#2 lore raw
Quoted 2 lines · Russell King

That was my initial approach as well, which got shot down by Andrew Morton and others as being unacceptable.

where? why?

Seems like we need to push back on that part.

Quoted 26 lines · Harvey Harrison
Quoted 25 lines

Limit this driver to ARCH_PXA, noticed by breakage on allyesconfig builds on 32-bit x86.

---
Linus, please apply and Russell can fix it up however he likes later in the week. Many people are hitting this.

 drivers/mfd/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/mfd/Kconfig +1 -0
index 2566479..bd29c61 100644
@@ -32,6 +32,7 @@ config HTC_EGPIO
 
 config HTC_PASIC3
 	tristate "HTC PASIC3 LED/DS1WM chip support"
+	depends on ARCH_PXA
 	help
 	  This core driver provides register access for the LED/DS1WM
 	  chips labeled "AIC2" and "AIC3", found on HTC Blueangel and
Signature · 1 line
1.5.5.144.g3e42
Signature · 1 line
~Randy
#3 lore raw
Quoted 6 lines · Randy.Dunlap
Quoted 2 lines · Russell King

That was my initial approach as well, which got shot down by Andrew Morton and others as being unacceptable.

where?

In private mail.

Quoted 1 line

why?

Well, first I need to gain the permission of Andrew to post his private message. I'm not being subborn here - I _do_ _not_ reproduce private messages in public without prior permission.

Quoted 1 line

Seems like we need to push back on that part.

Talk to Andrew then.

Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
#4 lore raw
Quoted 8 lines · Russell King
Quoted 6 lines · Randy.Dunlap
Quoted 2 lines · Russell King

That was my initial approach as well, which got shot down by Andrew Morton and others as being unacceptable.

where?

Quoted 7 lines

In private mail.

Quoted 1 line

why?

Well, first I need to gain the permission of Andrew to post his private message. I'm not being subborn here - I _do_ _not_ reproduce private messages in public without prior permission.

Sure, understood.

Quoted 3 lines
Quoted 1 line

Seems like we need to push back on that part.

Talk to Andrew then.

He is cc-ed (although traveling much this week IIRC).

Signature · 1 line
~Randy
#5 lore raw
Quoted 17 lines · Randy.Dunlap
Quoted 8 lines · Russell King
Quoted 6 lines · Randy.Dunlap
Quoted 2 lines · Russell King

That was my initial approach as well, which got shot down by Andrew Morton and others as being unacceptable.

where?

Quoted 7 lines

In private mail.

Quoted 1 line

why?

Well, first I need to gain the permission of Andrew to post his private message. I'm not being subborn here - I _do_ _not_ reproduce private messages in public without prior permission.

Quoted 7 lines

Sure, understood.

Quoted 3 lines
Quoted 1 line

Seems like we need to push back on that part.

Talk to Andrew then.

He is cc-ed (although traveling much this week IIRC).

In which case, since it's likely I won't get a reply in the next hour (which'll delay my response by 24 hours) let me paraphrase what Andrew said.

Andrew believes that it is beneficial to have other architectures, particularly x86, build other architectures drivers.

Meanwhile, pHilipp Zabel believes this hardware not to be ARM specific.

So, the majority concensus in the three way discussion was that it should remain visible, and the (unnecessary) include and its dependents be removed.

Having now had some time (read: half an hour after dinner after getting back home) to investigate, I can point to the patch in several places:

http://ftp.arm.linux.org.uk/pub/armlinux/kernel/git-cur/arm:devel.mbox

and find the patch with subject line:

[ARM] 5010/1: htc-pasic3: remove unused defines and includes

or grab 5010/1 from my patch system. If you want a patch to plaster over it, merging that would be a far better solution.

Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
#6 lore raw

--- Original Message --- (from rmk:)

Quoted 26 lines · Randy.Dunlap
Quoted 17 lines · Russell King
Quoted 8 lines · Randy.Dunlap
Quoted 6 lines · Russell King
Quoted 3 lines

That was my initial approach as well, which got shot down by Andrew Morton and others as being unacceptable.

where?

Quoted 7 lines

In private mail.

Quoted 1 line

why?

Well, first I need to gain the permission of Andrew to post his private message. I'm not being subborn here - I _do_ _not_ reproduce private messages in public without prior permission.

Quoted 7 lines

Sure, understood.

Quoted 3 lines
Quoted 1 line

Seems like we need to push back on that part.

Talk to Andrew then.

He is cc-ed (although traveling much this week IIRC).

Quoted 7 lines

In which case, since it's likely I won't get a reply in the next hour
(which'll delay my response by 24 hours) let me paraphrase what Andrew said.

Andrew believes that it is beneficial to have other architectures, particularly x86, build other architectures drivers.

Sure.

Quoted 2 lines

Meanwhile, pHilipp Zabel believes this hardware not to be ARM specific.

That's the big Important missing info. So the source file is incorrect, not the Kconfig.l

Quoted 10 lines

So, the majority concensus in the three way discussion was that it
should remain visible, and the (unnecessary) include and its dependents be removed.

Having now had some time (read: half an hour after dinner after getting
back home) to investigate, I can point to the patch in several places:

Quoted 8 lines

http://ftp.arm.linux.org.uk/pub/armlinux/kernel/git-cur/arm:devel.mbox

and find the patch with subject line:

[ARM] 5010/1: htc-pasic3: remove unused defines and includes

or grab 5010/1 from my patch system. If you want a patch to plaster over it, merging that would be a far better solution.

OK, sounds fair. Thanks.

~Randy

#7 lore raw
In reply to Russell King
Quoted 25 lines · On Tue, 22 Apr 2008 22:01:53 +0100 Russell King <[email protected]>…
Quoted 23 lines · Randy.Dunlap
Quoted 8 lines · Russell King
Quoted 6 lines · Randy.Dunlap
Quoted 2 lines · Russell King

That was my initial approach as well, which got shot down by Andrew Morton and others as being unacceptable.

where?

Quoted 7 lines

In private mail.

Quoted 1 line

why?

Well, first I need to gain the permission of Andrew to post his private message. I'm not being subborn here - I _do_ _not_ reproduce private messages in public without prior permission.

Sure, understood.

Quoted 3 lines
Quoted 1 line

Seems like we need to push back on that part.

Talk to Andrew then.

Quoted 8 lines
Quoted 1 line

He is cc-ed (although traveling much this week IIRC).

In which case, since it's likely I won't get a reply in the next hour (which'll delay my response by 24 hours) let me paraphrase what Andrew said.

Andrew believes that it is beneficial to have other architectures, particularly x86, build other architectures drivers.

Well. I pointed out that there are arguments either way, and I do tend to think that the let-x86-compile-it-too approach is perhaps the better one, but that's a 51%/49% opinion. Others disagree and lots and lots of code has gone the other way. Do whatever you think best ;)

#8 lore raw
In reply to Russell King
Quoted 3 lines · Russell King

That was my initial approach as well, which got shot down by Andrew Morton and others as being unacceptable.

Hmm. While I in general support the notion of trying to compile drivers on as wide a variety as hardware as possible, if that HTC_PASIC3 thing really is a PXA-only piece of hardware, I don't really see the point of not making the config file accurately represent that.

That said, as far as I can tell, the compile failure is because of this line:

	#include <asm/arch/pxa-regs.h>

and I cannot for the life of me see _why_ it tries to include that header file. It seems to compile fine on x96-64 if you just remove that include, and while I still think it should depend on ARCH_PXA just because it makes no sense _not_ to, I do wonder why that include is there in the first place.

Hmm?

Linus

#9 lore raw
Quoted 14 lines · Linus Torvalds
Quoted 2 lines · Russell King

That was my initial approach as well, which got shot down by Andrew Morton and others as being unacceptable.

Hmm. While I in general support the notion of trying to compile drivers on as wide a variety as hardware as possible, if that HTC_PASIC3 thing really is a PXA-only piece of hardware, I don't really see the point of not making the config file accurately represent that.

That said, as far as I can tell, the compile failure is because of this line:

	#include <asm/arch/pxa-regs.h>
Quoted 7 lines

and I cannot for the life of me see _why_ it tries to include that header file. It seems to compile fine on x96-64 if you just remove that include, and while I still think it should depend on ARCH_PXA just because it makes no sense _not_ to, I do wonder why that include is there in the first place.

Hmm?

I think we've settled this in one of my replies to Randy.

However, I think there's a bigger issue here.

[I'm not including all the details here about the timing - since you know the timing of my emails to you, the only thing left out is that Andrew told me about this problem on Sunday Morning (UK time).]

My kernel work pattern is Saturday to Monday inclusive and Thursday. I believe yours is to avoid Saturday through into most of Monday. This unfortunately means that we currently have almost no overlap, and very few consecutive days where issues from the previous day can be resolved.

In light of that, given my request on Monday not to pull my tree, should I have deleted the for-linus head at the same time to prevent the pull occuring if you (as has become apparant) didn't read the message?

I rather guessed that you hadn't pulled my tree by then, but I wasn't 100% sure - sometimes you've pulled my tree but not pushed yours out for a few days.

It's really the "not knowing" which has caused this issue - had I known positively that you hadn't pulled my tree I could have prepared a replacement tree with all the (now numerous) cockups fixed. Had I known you had pulled my tree, I would've been able to queue up the various fixes and had that branch ready to go by Monday evening.

To be really frank, I'm beginning to wonder whether using git is such a good idea - at least if I was sending you a stream of patches then all I'd needed to have done was forward you the relevant patches as fixes to the previous set - and I wouldn't care whether you'd applied the previous set at that point.

But with git, it has to be known what you're doing at your end before I can make a decision about how to fix issues at my end.

Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
#10 lore raw
Quoted 9 lines · Russell King

To be really frank, I'm beginning to wonder whether using git is such a good idea - at least if I was sending you a stream of patches then all I'd needed to have done was forward you the relevant patches as fixes to the previous set - and I wouldn't care whether you'd applied the previous set at that point.

But with git, it has to be known what you're doing at your end before I can make a decision about how to fix issues at my end.

I really don't see what you are talking about, and nobody else has that problem.

With patches, once you send them, they are sent, and you can't fix it.

With git, once you send my the "please pull", it's sent, and you can't fix it.

And with either, you can update the queue later. I don't understand AT ALL why you think they are different.

In fact, if anything, git trees are a lot more flexible. With git, what you can do is fix up the tree at any time, and send me an email saying "ok, if you already pulled it's too late, but if you didn't, the tree is now fixed". That you can't do with patches, because once you've sent them out they are out of your control.

But with both git and patches you can *always* just append on top of the previous set. Just send me a new set of patches (with git, that obviously means just sending me a new pull-request with updated information).

This is what everybody else does, it's not even unusual.

Linus

#11 lore raw
In reply to Linus Torvalds
Quoted 18 lines · Linus Torvalds
Quoted 4 lines · Russell King

That was my initial approach as well, which got shot down by Andrew Morton and others as being unacceptable.

Hmm. While I in general support the notion of trying to compile drivers on as wide a variety as hardware as possible, if that HTC_PASIC3 thing really is a PXA-only piece of hardware, I don't really see the point of not making the config file accurately represent that.

That said, as far as I can tell, the compile failure is because of this line:

        #include <asm/arch/pxa-regs.h>
Quoted 7 lines

and I cannot for the life of me see _why_ it tries to include that header file. It seems to compile fine on x96-64 if you just remove that include, and while I still think it should depend on ARCH_PXA just because it makes no sense _not_ to, I do wonder why that include is there in the first place.

Hmm?

I'm embarrased to say, this is a left-over from a clean-up that happened a long time ago. I probably forgot to double-check the includes of this file before submitting to Russell. And this superfluous include was never pointed out to me because I only compiled that driver for ARM/PXA.
It seems having the driver enabled for x86 builds had its benefits (a bug was found :)). I guess now it could just as well be made to depend on ARCH_PXA, because although it does not really depend on pxa, it is not of much use on any other architecture.

regards
Philipp

#12 lore raw
In reply to Russell King
Quoted 2 lines · Russell King

That was my initial approach as well, which got shot down by Andrew
Morton and others as being unacceptable.

'Shot down' is a bit harsh. I just argued that technically the driver does not depend on ARM (and neither does the ASIC, most probably). On the other hand, AFAIK it is proprietary HTC silicon only found in a handful of their devices, all of which are PXA2xx based.

Quoted 36 lines · Harvey Harrison
Quoted 24 lines

Limit this driver to ARCH_PXA, noticed by breakage on allyesconfig builds on 32-bit x86.

---
Linus, please apply and Russell can fix it up however he likes later in the week. Many people are hitting this.

 drivers/mfd/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/mfd/Kconfig +1 -1
index 2566479..bd29c61 100644
@@ -32,6 +32,7 @@ config HTC_EGPIO
 
 config HTC_PASIC3
      tristate "HTC PASIC3 LED/DS1WM chip support"
+     depends on ARCH_PXA
      help
        This core driver provides register access for the LED/DS1WM
        chips labeled "AIC2" and "AIC3", found on HTC Blueangel and
--
Quoted 4 lines

1.5.5.144.g3e42

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:

Quoted 6 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

regards
Philipp