* Fwd: [Socketcan-users] sja1000_platform interrupt sharing
[not found] <6C5A3B79888A804CAB605257CD26D37703955B43@vwagwox00032.vw.vwg>
@ 2012-08-09 11:21 ` Marc Kleine-Budde
2012-08-09 11:39 ` Marc Kleine-Budde
0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2012-08-09 11:21 UTC (permalink / raw)
To: Sven.Schmitt; +Cc: linux-can@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]
Hello Sven,
please use our new mailinglist linux-can@vger.kernel.org (Cc'ed), and please don't send HTML emails.
-------- Original Message --------
Subject: [Socketcan-users] sja1000_platform interrupt sharing
Date: Thu, 9 Aug 2012 12:43:38 +0200
From: Schmitt, Sven (EVM/8) <Sven.Schmitt@volkswagen.de>
To: <socketcan-users@lists.berlios.de>
Hi,
I've found something confusing when I wanted to use more than one
sja1000 attached to a platform bus with interrupt sharing. The flag for
the sja1000 resources I wanted to set first was the
IORESOURCE_IRQ_SHAREABLE* flag because of the other IORESOURCE_IRQ_XXX
flags. But this flag is ignored by the sja1000_platform driver because
it has another position. If I use the generic IRQF_SHARED** flag
everything works fine. Does it make sense to patch the driver in a way
like that:
--- linux-3.5/drivers/net/can/sja1000/sja1000_platform.c.orig
2012-08-09 10:38:57.405381025 +0200
+++ linux-3.5/drivers/net/can/sja1000/sja1000_platform.c
2012-08-09 10:57:22.693364557 +0200
@@ -109,6 +109,8 @@
priv = netdev_priv(dev);
dev->irq = res_irq->start;
+ if(res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
+ priv->irq_flags |= IRQF_SHARED;
priv->irq_flags = res_irq->flags & (IRQF_TRIGGER_MASK |
IRQF_SHARED);
priv->reg_base = addr;
/* The CAN clock frequency is half the oscillator clock
frequency */
*
include/linux/ioport.h:
#define IORESOURCE_IRQ_SHAREABLE
<http://lxr.free-electrons.com/ident?i=IORESOURCE_IRQ_SHAREABLE>
(1<<4)
**
include/linux/interrupt.h:
#define IRQF_SHARED <http://lxr.free-electrons.com/ident?i=IRQF_SHARED>
0x00000080
Best regards,
Sven Schmitt
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Fwd: [Socketcan-users] sja1000_platform interrupt sharing
2012-08-09 11:21 ` Fwd: [Socketcan-users] sja1000_platform interrupt sharing Marc Kleine-Budde
@ 2012-08-09 11:39 ` Marc Kleine-Budde
0 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2012-08-09 11:39 UTC (permalink / raw)
To: Sven.Schmitt; +Cc: linux-can@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 2099 bytes --]
On 08/09/2012 01:21 PM, Marc Kleine-Budde wrote:
> I've found something confusing when I wanted to use more than one
It's not confusing, it's a bug. Good catch.
> sja1000 attached to a platform bus with interrupt sharing. The flag for
> the sja1000 resources I wanted to set first was the
> IORESOURCE_IRQ_SHAREABLE* flag because of the other IORESOURCE_IRQ_XXX
> flags. But this flag is ignored by the sja1000_platform driver because
> it has another position. If I use the generic IRQF_SHARED** flag
> everything works fine. Does it make sense to patch the driver in a way
> like that:
>
> --- linux-3.5/drivers/net/can/sja1000/sja1000_platform.c.orig
> 2012-08-09 10:38:57.405381025 +0200
> +++ linux-3.5/drivers/net/can/sja1000/sja1000_platform.c
> 2012-08-09 10:57:22.693364557 +0200
> @@ -109,6 +109,8 @@
> priv = netdev_priv(dev);
>
> dev->irq = res_irq->start;
> + if(res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
> + priv->irq_flags |= IRQF_SHARED;
Please adjust to kernel coding style, space between if and opening bracket.
> priv->irq_flags = res_irq->flags & (IRQF_TRIGGER_MASK |
> IRQF_SHARED);
Please remove the IRQF_SHARED here, as IRQF_SHARED is wrong here and
means something different.
> priv->reg_base = addr;
> /* The CAN clock frequency is half the oscillator clock
> frequency */
If you have a look at the history of the file, it shows that this flag
was added Yegor Yefremov <yegor_sub1@visionsystems.de> in commit
abde89d can: sja1000: allow shared interrupt definition
Please resend your patch, with the problems fixed, use git send-email
for this, add Yegor on Cc and Sign your work [1]. If this is too
complicated for you, I'll fix it myself.
Marc
[1]
http://lxr.free-electrons.com/source/Documentation/SubmittingPatches#L298
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* AW: Fwd: [Socketcan-users] sja1000_platform interrupt sharing
@ 2012-08-09 12:46 Schmitt, Sven (EVM/8)
2012-08-09 12:56 ` Yegor Yefremov
0 siblings, 1 reply; 6+ messages in thread
From: Schmitt, Sven (EVM/8) @ 2012-08-09 12:46 UTC (permalink / raw)
Cc: yegor_sub1, linux-can
Second try:
Signed-off-by: Sven Schmitt <sven.schmitt@volkswagen.de>
--- linux-3.5/drivers/net/can/sja1000/sja1000_platform.c.orig
+++ linux-3.5/drivers/net/can/sja1000/sja1000_platform.c
@@ -109,7 +109,9 @@ static int sp_probe(struct platform_devi
priv = netdev_priv(dev);
dev->irq = res_irq->start;
- priv->irq_flags = res_irq->flags & (IRQF_TRIGGER_MASK |
IRQF_SHARED);
+ priv->irq_flags &= IRQF_TRIGGER_MASK;
+ if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
+ priv->irq_flags |= IRQF_SHARED;
priv->reg_base = addr;
/* The CAN clock frequency is half the oscillator clock
frequency */
priv->can.clock.freq = pdata->osc_freq / 2;
note: patch is compile tested only.
Sven
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fwd: [Socketcan-users] sja1000_platform interrupt sharing
2012-08-09 12:46 AW: " Schmitt, Sven (EVM/8)
@ 2012-08-09 12:56 ` Yegor Yefremov
2012-08-09 13:01 ` Marc Kleine-Budde
0 siblings, 1 reply; 6+ messages in thread
From: Yegor Yefremov @ 2012-08-09 12:56 UTC (permalink / raw)
To: Schmitt, Sven (EVM/8); +Cc: linux-can
Am 09.08.2012 14:46, schrieb Schmitt, Sven (EVM/8):
> Second try:
>
> Signed-off-by: Sven Schmitt <sven.schmitt@volkswagen.de>
>
> --- linux-3.5/drivers/net/can/sja1000/sja1000_platform.c.orig
> +++ linux-3.5/drivers/net/can/sja1000/sja1000_platform.c
> @@ -109,7 +109,9 @@ static int sp_probe(struct platform_devi
> priv = netdev_priv(dev);
>
> dev->irq = res_irq->start;
> - priv->irq_flags = res_irq->flags & (IRQF_TRIGGER_MASK |
> IRQF_SHARED);
> + priv->irq_flags &= IRQF_TRIGGER_MASK;
> + if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
> + priv->irq_flags |= IRQF_SHARED;
> priv->reg_base = addr;
> /* The CAN clock frequency is half the oscillator clock
> frequency */
> priv->can.clock.freq = pdata->osc_freq / 2;
>
>
> note: patch is compile tested only.
>
> Sven
O.K. Nice catch. I just picked the wrong header file include/linux/interrupt.h, it should be include/linux/ioport.h. Now I'll have to change my board file as well.
You can add my
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
See how I manage patches with git: https://libbits.wordpress.com/2011/12/27/managing-patch-series-with-git/
Yegor
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fwd: [Socketcan-users] sja1000_platform interrupt sharing
2012-08-09 12:56 ` Yegor Yefremov
@ 2012-08-09 13:01 ` Marc Kleine-Budde
2012-08-09 13:13 ` Yegor Yefremov
0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2012-08-09 13:01 UTC (permalink / raw)
To: yegor_sub1; +Cc: Schmitt, Sven (EVM/8), linux-can
[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]
On 08/09/2012 02:56 PM, Yegor Yefremov wrote:
> Am 09.08.2012 14:46, schrieb Schmitt, Sven (EVM/8):
>> Second try:
>>
>> Signed-off-by: Sven Schmitt <sven.schmitt@volkswagen.de>
>>
>> --- linux-3.5/drivers/net/can/sja1000/sja1000_platform.c.orig
>> +++ linux-3.5/drivers/net/can/sja1000/sja1000_platform.c
>> @@ -109,7 +109,9 @@ static int sp_probe(struct platform_devi
>> priv = netdev_priv(dev);
>>
>> dev->irq = res_irq->start;
>> - priv->irq_flags = res_irq->flags & (IRQF_TRIGGER_MASK |
>> IRQF_SHARED);
>> + priv->irq_flags &= IRQF_TRIGGER_MASK;
>> + if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
>> + priv->irq_flags |= IRQF_SHARED;
>> priv->reg_base = addr;
>> /* The CAN clock frequency is half the oscillator clock
>> frequency */
>> priv->can.clock.freq = pdata->osc_freq / 2;
>>
>>
>> note: patch is compile tested only.
>>
>> Sven
>
> O.K. Nice catch. I just picked the wrong header file
> include/linux/interrupt.h, it should be include/linux/ioport.h. Now
> I'll have to change my board file as well.
A quick serach showed no mainline board using shared irq, have I missed one?
> You can add my
>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
You mean Acked-by?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fwd: [Socketcan-users] sja1000_platform interrupt sharing
2012-08-09 13:01 ` Marc Kleine-Budde
@ 2012-08-09 13:13 ` Yegor Yefremov
2012-08-09 14:00 ` Marc Kleine-Budde
0 siblings, 1 reply; 6+ messages in thread
From: Yegor Yefremov @ 2012-08-09 13:13 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: Schmitt, Sven (EVM/8), linux-can
Am 09.08.2012 15:01, schrieb Marc Kleine-Budde:
> On 08/09/2012 02:56 PM, Yegor Yefremov wrote:
>> Am 09.08.2012 14:46, schrieb Schmitt, Sven (EVM/8):
>>> Second try:
>>>
>>> Signed-off-by: Sven Schmitt <sven.schmitt@volkswagen.de>
>>>
>>> --- linux-3.5/drivers/net/can/sja1000/sja1000_platform.c.orig
>>> +++ linux-3.5/drivers/net/can/sja1000/sja1000_platform.c
>>> @@ -109,7 +109,9 @@ static int sp_probe(struct platform_devi
>>> priv = netdev_priv(dev);
>>>
>>> dev->irq = res_irq->start;
>>> - priv->irq_flags = res_irq->flags & (IRQF_TRIGGER_MASK |
>>> IRQF_SHARED);
>>> + priv->irq_flags &= IRQF_TRIGGER_MASK;
>>> + if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
>>> + priv->irq_flags |= IRQF_SHARED;
>>> priv->reg_base = addr;
>>> /* The CAN clock frequency is half the oscillator clock
>>> frequency */
>>> priv->can.clock.freq = pdata->osc_freq / 2;
>>>
>>>
>>> note: patch is compile tested only.
>>>
>>> Sven
>> O.K. Nice catch. I just picked the wrong header file
>> include/linux/interrupt.h, it should be include/linux/ioport.h. Now
>> I'll have to change my board file as well.
> A quick serach showed no mainline board using shared irq, have I missed one?
You've missed one board in our own repos. We had to much of a custom code to try to mainline this board file.
>> You can add my
>>
>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> You mean Acked-by?
Yes.
Acked-by: Yegor Yefremov <yegorslists@googlemail.com>
Yegor
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fwd: [Socketcan-users] sja1000_platform interrupt sharing
2012-08-09 13:13 ` Yegor Yefremov
@ 2012-08-09 14:00 ` Marc Kleine-Budde
0 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2012-08-09 14:00 UTC (permalink / raw)
To: yegor_sub1; +Cc: Schmitt, Sven (EVM/8), linux-can
[-- Attachment #1: Type: text/plain, Size: 923 bytes --]
On 08/09/2012 03:13 PM, Yegor Yefremov wrote:
[...]
>>> O.K. Nice catch. I just picked the wrong header file
>>> include/linux/interrupt.h, it should be include/linux/ioport.h. Now
>>> I'll have to change my board file as well.
>> A quick serach showed no mainline board using shared irq, have I missed one?
>
> You've missed one board in our own repos. We had to much of a custom
> code to try to mainline this board file.
No mainline no breakage :)
>>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>> You mean Acked-by?
>
> Yes.
>
> Acked-by: Yegor Yefremov <yegorslists@googlemail.com>
Thanks, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-08-09 14:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <6C5A3B79888A804CAB605257CD26D37703955B43@vwagwox00032.vw.vwg>
2012-08-09 11:21 ` Fwd: [Socketcan-users] sja1000_platform interrupt sharing Marc Kleine-Budde
2012-08-09 11:39 ` Marc Kleine-Budde
2012-08-09 12:46 AW: " Schmitt, Sven (EVM/8)
2012-08-09 12:56 ` Yegor Yefremov
2012-08-09 13:01 ` Marc Kleine-Budde
2012-08-09 13:13 ` Yegor Yefremov
2012-08-09 14:00 ` Marc Kleine-Budde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).