mfd: ucb1x00: add irq field to the platform data
Subsystems (3)
PATCH 1/3 mfd: ucb1x00: add irq field to the platform data
To allow boards to specify the irq that is used by UCB1x00 chip, add irq field to the platform data structure.
--- include/linux/mfd/ucb1x00.h | 1 + 1 file changed, 1 insertion(+)
include/linux/mfd/ucb1x00.h +1 -0
@@ -118,6 +118,7 @@ struct ucb1x00_plat_data {unsigned irq_base;int gpio_base;unsigned can_wakeup; +int irq; };struct ucb1x00 {
Signature · 1 line
2.1.4
PATCH 2/3 ARM: sa1100: populate codec platform data with irq
For the sa11x0 boards that provide ucb1x00 codec platform data, provide the irq data.
--- arch/arm/mach-sa1100/assabet.c | 1 + arch/arm/mach-sa1100/collie.c | 1 + arch/arm/mach-sa1100/simpad.c | 1 + 3 files changed, 3 insertions(+)
arch/arm/mach-sa1100/assabet.c +1 -0
@@ -309,6 +309,7 @@ static struct ucb1x00_plat_data assabet_ucb1x00_data = { .reset = assabet_ucb1x00_reset, .gpio_base = -1, .can_wakeup = 1, + .irq = IRQ_GPIO23, };static struct mcp_plat_data assabet_mcp_data = {
arch/arm/mach-sa1100/collie.c +1 -0
@@ -97,6 +97,7 @@ static struct scoop_pcmcia_config collie_pcmcia_config = {static struct ucb1x00_plat_data collie_ucb1x00_data = { .gpio_base =COLLIE_TC35143_GPIO_BASE , + .irq = COLLIE_IRQ_GPIO_UCB1x00_IRQ, };static struct mcp_plat_data collie_mcp_data = {
arch/arm/mach-sa1100/simpad.c +1 -0
@@ -184,6 +184,7 @@ static struct resource simpad_flash_resources [] = {static struct ucb1x00_plat_data simpad_ucb1x00_data = { .gpio_base =SIMPAD_UCB1X00_GPIO_BASE , + .irq =IRQ_GPIO_UCB1300_IRQ , };static struct mcp_plat_data simpad_mcp_data = {
Signature · 1 line
2.1.4
PATCH 2/3 ARM: sa1100: populate codec platform data with irq
Quoted 2 lines · Dmitry Eremin-Solenikov
For the sa11x0 boards that provide ucb1x00 codec platform data, provide the irq data.
This isn't necessary. The ucb1x00 can find its IRQ on its own.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
PATCH 2/3 ARM: sa1100: populate codec platform data with irq
2015-04-02 19:00 GMT+03:00 Russell King - ARM Linux <[email protected]>:
Quoted 5 lines · Dmitry Eremin-Solenikov
Quoted 2 lines
For the sa11x0 boards that provide ucb1x00 codec platform data, provide the irq data.
This isn't necessary. The ucb1x00 can find its IRQ on its own.
Yes, it can. But if it's a static configuration, why can't it be configured statically?
Signature · 2 lines
With best wishes Dmitry
PATCH 2/3 ARM: sa1100: populate codec platform data with irq
Quoted 9 lines · Dmitry Eremin-Solenikov
2015-04-02 19:00 GMT+03:00 Russell King - ARM Linux <[email protected]>:
Quoted 5 lines · Dmitry Eremin-Solenikov
Quoted 2 lines
For the sa11x0 boards that provide ucb1x00 codec platform data, provide the irq data.
This isn't necessary. The ucb1x00 can find its IRQ on its own.
Yes, it can. But if it's a static configuration, why can't it be configured statically?
Why change something that's worked for ages?
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
PATCH 3/3 mfd: ucb1x00: make use of provided irq
In reply to Dmitry Eremin-SolenikovIf the platform data provides IRQ information, use that instead always probing the IRQ.
--- drivers/mfd/ucb1x00-core.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/mfd/ucb1x00-core.c +6 -3
@@ -465,9 +465,12 @@ static int ucb1x00_probe(struct mcp *mcp)if (ret) goto err_dev_add; -ucb1x00_enable (ucb); - ucb->irq =ucb1x00_detect_irq (ucb); -ucb1x00_disable (ucb); + ucb->irq = pdata ? pdata->irq :NO_IRQ ; +if (ucb->irq ==NO_IRQ ) { +ucb1x00_enable (ucb); + ucb->irq =ucb1x00_detect_irq (ucb); +ucb1x00_disable (ucb); + }if (ucb->irq ==NO_IRQ ) {dev_err (&ucb->dev,"IRQ probe failed\n" ); ret = -ENODEV ;
Signature · 1 line
2.1.4
PATCH 1/3 mfd: ucb1x00: add irq field to the platform data
In reply to Dmitry Eremin-SolenikovJust like has already been pointed out on the list once today, we need much better commit descriptions than this.
The code has been perfectly happy to auto-detect the IRQ for over ten years. Why do we suddenly need to change it now? What's the justification for this change.
Or is this just a case of "because we can" and you happen to have a different opinion on how stuff should be done from how it's been successfully done for the last ten years?
Quoted 28 lines · Dmitry Eremin-Solenikov
To allow boards to specify the irq that is used by UCB1x00 chip, add irq field to the platform data structure.
Signed-off-by Dmitry Eremin-Solenikov <[email protected]>--- include/linux/mfd/ucb1x00.h | 1 + 1 file changed, 1 insertion(+)include/linux/mfd/ucb1x00.h +1
-0@@ -118,6 +118,7 @@ struct ucb1x00_plat_data {unsigned irq_base;int gpio_base;unsigned can_wakeup; +int irq; };struct ucb1x00 {Signature · 7 lines
2.1.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
PATCH 1/3 mfd: ucb1x00: add irq field to the platform data
Hello,
2015-04-02 19:02 GMT+03:00 Russell King - ARM Linux <[email protected]>:
Quoted 6 lines
Just like has already been pointed out on the list once today, we need much better commit descriptions than this.
The code has been perfectly happy to auto-detect the IRQ for over ten years. Why do we suddenly need to change it now? What's the justification for this change.
Because using static configuration would allow me to drop a nice piece of code. Because it would allow later to use dts to describe ucb1x00 configuration. Because I will no longer have to think whether ucb1x00I h driver barfing on IRQ is a problem of a chip, of an IRQ or just me having touched wrong GPIO line at wrong time with a scope probe. Yes, I didn't put all these causes to the commit description. Do you really want it?
Quoted 4 lines
Or is this just a case of "because we can" and you happen to have a different opinion on how stuff should be done from how it's been successfully done for the last ten years?
Not only "because we can", but "because we should". In my humble opinion.
It looks like the whole story of me touching sa1100 is like fighting with 'it was done so for ages' windmills. Yes, I have different opinions sometimes. I would like for sa1100 to converge to other platforms. PXA, especially. Why? Because I like several small devices sitting on my table. PXA is now slowly moving towards dts, cleaner drivers and cleaner interfaces. We should have done this ages ago. Nobody had time and interest. SA-11x0 had even older drivers. Older implementations. Older ideas. Yes, I have a different opinion at this place. I'd like for sa1100 to live in a contemporary world. I'd like to have my devices work with the rest of Linux subsystems. Should I just fork sa1100 subarch, make it work with devices I have at hand and submit it as a 'new' subarch?
Or we can have an evolution-like process that will make sa1100 live in style with the rest of the Linux kernel.
Quoted 34 lines · Dmitry Eremin-Solenikov
Quoted 24 lines
To allow boards to specify the irq that is used by UCB1x00 chip, add irq field to the platform data structure.
Signed-off-by Dmitry Eremin-Solenikov <[email protected]>--- include/linux/mfd/ucb1x00.h | 1 + 1 file changed, 1 insertion(+)include/linux/mfd/ucb1x00.h +1
-1@@ -118,6 +118,7 @@ struct ucb1x00_plat_data {unsigned irq_base;int gpio_base;unsigned can_wakeup; +int irq; };struct ucb1x00 { --2.1.4
Quoted 4 lines
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
Signature · 2 lines
With best wishes Dmitry
PATCH 1/3 mfd: ucb1x00: add irq field to the platform data
Quoted 6 lines · Dmitry Eremin-Solenikov
Because using static configuration would allow me to drop a nice piece of code. Because it would allow later to use dts to describe ucb1x00 configuration. Because I will no longer have to think whether ucb1x00I h driver barfing on IRQ is a problem of a chip, of an IRQ or just me having touched wrong GPIO line at wrong time with a scope probe. Yes, I didn't put all these causes to the commit description. Do you really want it?
What I want is a decent commit description, one which isn't airy fairy and looks like it's just mindless churn. That's all.
It's what every other kernel developer wants to know. This is precisely what the ARM ecosystem has been soo bad at for decades.
It matters. It's one of the things what pisses Linus off. What Linus sees from ARM people is lots and lots of apparently pointless churn which he doesn't understand - and part of that problem is that ARM people do _not_ justify in their commit messages why a change is necessary.
So please, improve the commit messages and justify the commits.
It really doesn't help that all that was posted was three patches and no covering message which could either explain what the three patches were trying to achieve. Like most people do. (And some people take that to an extreme, posting a cover message for just one patch.)
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
PATCH 1/3 mfd: ucb1x00: add irq field to the platform data
In reply to Dmitry Eremin-SolenikovQuoted 5 lines · Dmitry Eremin-Solenikov
Not only "because we can", but "because we should". In my humble opinion.
It looks like the whole story of me touching sa1100 is like fighting with 'it was done so for ages' windmills. Yes, I have different opinions sometimes.
Right, and what you're forgetting is that others may have different opinions. Like me - and as I'm the sa1100 and ucb1x00 maintainer, if you have a different opinion, you need to convince me that your solution is the right thing to do.
Had I thought that passing an IRQ through platform data would be a good thing, then I would've done it from the outset.
On principle, I don't like the idea of passing driver resources through platform data, I find it abhorrent. However, there isn't really any good solution here other than platform data or auto-detecting it, and auto-detecting it is, in my opinion, the lesser of the evils.
Quoted 3 lines
I would like for sa1100 to converge to other platforms. PXA, especially. Why? Because I like several small devices sitting on my table. PXA is now slowly moving towards dts, cleaner drivers and cleaner interfaces.
Right, so when the UCB1x00 gets a node in DT, the driver can parse the interrupt from DT, without needing it in platform data - and if it finds that it's in DT, then there's no need to auto-probe it.
I don't see the point of adding it to platform data only to later have to change it /again/ when SA11x0 moves to DT.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.