Freescale mpc8315 IRQ0 setup

Freescale mpc8315 IRQ0 setup

[Juergen Schindele]

[-- Attachment #1: Type: text/plain, Size: 1928 bytes --]

Dear mailing list,
i found out on our platform with freescale mpc8315 SOC that in
linux kernel code the setup of IRQ0 which we use is not correct.
One should be able to use falling EDGE interrupt capabilities like on
IRQ1-IRQ7. These setups are fixed in "arch/powerpc/sysdev/ipic.c"
The internal interrupt number of IRQ0 is not like IRQ1-IRQ7 in one block
but on number 48. To verify details please consult MPC8315ERM.pdf
developpers manual.

To correct these "EDGE" capabilities of IRQ0 i suggest the following 
patch:
please consider integrating it to your patches.

--- arch/powerpc/sysdev/ipic.c  (Revision correct)
+++ arch/powerpc/sysdev/ipic.c  (Arbeitskopie)
@@ -316,6 +316,7 @@
                .prio_mask = 7,
        },
        [48] = {
+               .ack    = IPIC_SEPNR,
                .mask   = IPIC_SEMSR,
                .prio   = IPIC_SMPRR_A,
                .force  = IPIC_SEFCR,

Thank you for your attention
-- 
i. A.
Jürgen Schindele
Softwareentwicklung

PSI Nentec GmbH
Greschbachstraße 12
76229 Karlsruhe
Deutschland
Telefon: +49 721 94249-51
Telefax: +49 721 94249-10
schindele@nentec.de
www.nentec.de

Geschäftsführung: Klaus Becker, Wolfgang Fischer
Sitz der Gesellschaft: Karlsruhe
Handelsregister: Amtsgericht Mannheim HRB 107658

Diese E-Mail enthält vertrauliche oder rechtlich geschützte 
Informationen. Wenn Sie nicht der vorgesehene Empfänger sind, 
informieren Sie bitte sofort den Absender und löschen Sie diese E-Mail. 
Das unbefugte Kopieren dieser E-Mail oder die unbefugte Weitergabe 
der enthaltenen Informationen ist nicht gestattet.

The information contained in this message is confidential or protected 
by law. If you are not the intended recipient, please contact the sender 
and delete this message. Any unauthorised copying of this message or 
unauthorised distribution of the information contained herein is 
prohibited. 

[-- Attachment #2: Type: text/html, Size: 8417 bytes --]

Re: Freescale mpc8315 IRQ0 setup

[Scott Wood]

On Mon, Apr 10, 2017 at 04:53:18PM +0200, Juergen Schindele wrote:
> Dear mailing list,
> i found out on our platform with freescale mpc8315 SOC that in
> linux kernel code the setup of IRQ0 which we use is not correct.
> One should be able to use falling EDGE interrupt capabilities like on
> IRQ1-IRQ7. These setups are fixed in "arch/powerpc/sysdev/ipic.c"
> The internal interrupt number of IRQ0 is not like IRQ1-IRQ7 in one block
> but on number 48. To verify details please consult MPC8315ERM.pdf
> developpers manual.
> 
> To correct these "EDGE" capabilities of IRQ0 i suggest the following 
> patch:
> please consider integrating it to your patches.
> 
> 
> Thank you for your attention
> 
> --- arch/powerpc/sysdev/ipic.c  (Revision correct)
> +++ arch/powerpc/sysdev/ipic.c  (Arbeitskopie)
> @@ -316,6 +316,7 @@
>                 .prio_mask = 7,
>         },
>         [48] = {
> +               .ack    = IPIC_SEPNR,
>                 .mask   = IPIC_SEMSR,
>                 .prio   = IPIC_SMPRR_A,
>                 .force  = IPIC_SEFCR,

Could you send this with a signoff and a commit message intended for
merging?

-Scott

Re: Freescale mpc8315 IRQ0 setup

[Juergen Schindele]

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

Dear Scott,
sorry for the delay but i am not very familiar with the formating.
I passed the patch trough checkpatch.pl and there was no more error.
pease find patch in attached file.
Thanks
-- 
i. A.
Jürgen Schindele
Softwareentwicklung

PSI Nentec GmbH
Greschbachstraße 12
76229 Karlsruhe
Deutschland
Telefon: +49 721 94249-51
Telefax: +49 721 94249-10
schindele@nentec.de
www.nentec.de

Geschäftsführung: Klaus Becker, Wolfgang Fischer
Sitz der Gesellschaft: Karlsruhe
Handelsregister: Amtsgericht Mannheim HRB 107658

Diese E-Mail enthält vertrauliche oder rechtlich geschützte Informationen. 
Wenn Sie nicht der vorgesehene Empfänger sind, informieren Sie bitte sofort 
den Absender und löschen Sie diese E-Mail. Das unbefugte Kopieren dieser E-
Mail oder die unbefugte Weitergabe der enthaltenen Informationen ist nicht 
gestattet.

The information contained in this message is confidential or protected by law. 
If you are not the intended recipient, please contact the sender and delete 
this message. Any unauthorised copying of this message or unauthorised 
distribution of the information contained herein is prohibited. 

[-- Attachment #2: ipic.patch --]
[-- Type: text/x-patch, Size: 955 bytes --]

[PATCH] configure "EDGE" capabilities for IRQ0 (like IRQ1-7)
Signed-off-by: Jurgen Schindele <schindele@nentec.de>
--- linux-a/arch/powerpc/sysdev/ipic.c        2017-04-04 15:28:11.201308780 +0200
+++ linux-b/arch/powerpc/sysdev/ipic.c        2016-12-11 20:17:54.000000000 +0100
@@ -315,7 +315,6 @@ static struct ipic_info ipic_info[] = {
                .prio_mask = 7,
        },
        [48] = {
-               .ack    = IPIC_SEPNR,
                .mask   = IPIC_SEMSR,
                .prio   = IPIC_SMPRR_A,
                .force  = IPIC_SEFCR,
@@ -618,7 +617,7 @@ static int ipic_set_irq_type(struct irq_
        /* ipic supports only edge mode on external interrupts */
        if ((flow_type & IRQ_TYPE_EDGE_FALLING) && !ipic_info[src].ack) {
                printk(KERN_ERR "ipic: edge sense not supported on internal "
-                               "interrupts %d\n", src);
+				"interrupts\n");
                return -EINVAL;

        }

Re: Freescale mpc8315 IRQ0 setup

[Scott Wood]

On Tue, 2017-05-02 at 14:43 +0200, Juergen Schindele wrote:
> Dear Scott,
> sorry for the delay but i am not very familiar with the formating.
> I passed the patch trough checkpatch.pl and there was no more error.
> pease find patch in attached file.
> Thanks

Documentation/process/submitting-patches.rst explains the way to format and
submit kernel patches.

Also, why the unrelated change to a print statement in ipic_set_irq_type()?

-Scott

Re: Freescale mpc8315 IRQ0 setup

[Juergen Schindele]

[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]

Am Dienstag, 2. Mai 2017, 22:29:34 schrieb Scott Wood:
> On Tue, 2017-05-02 at 14:43 +0200, Juergen Schindele wrote:
> > Dear Scott,
> > sorry for the delay but i am not very familiar with the formating.
> > I passed the patch trough checkpatch.pl and there was no more error.
> > pease find patch in attached file.
> > Thanks
> 
> Documentation/process/submitting-patches.rst explains the way to format and
> submit kernel patches.
> 
> Also, why the unrelated change to a print statement in ipic_set_irq_type()?
> 
> -Scott
The second diff is not completely unrelated because when i was investigating 
the problem i saw only a message "edge sense not supported" but you dont
know on which interrupt he is complaining about. So i added this to find out 
who the suspect is.

Corrected patch
-- 
i. A.
Jürgen Schindele
Softwareentwicklung

PSI Nentec GmbH
Greschbachstraße 12
76229 Karlsruhe
Deutschland
Telefon: +49 721 94249-51
Telefax: +49 721 94249-10
schindele@nentec.de
www.nentec.de

Geschäftsführung: Klaus Becker, Wolfgang Fischer
Sitz der Gesellschaft: Karlsruhe
Handelsregister: Amtsgericht Mannheim HRB 107658

Diese E-Mail enthält vertrauliche oder rechtlich geschützte Informationen. 
Wenn Sie nicht der vorgesehene Empfänger sind, informieren Sie bitte sofort 
den Absender und löschen Sie diese E-Mail. Das unbefugte Kopieren dieser E-
Mail oder die unbefugte Weitergabe der enthaltenen Informationen ist nicht 
gestattet.

The information contained in this message is confidential or protected by law. 
If you are not the intended recipient, please contact the sender and delete 
this message. Any unauthorised copying of this message or unauthorised 
distribution of the information contained herein is prohibited. 

[-- Attachment #2: ipic.patch --]
[-- Type: text/x-patch, Size: 773 bytes --]

[PATCH] configure "EDGE" capabilities for IRQ0 (like IRQ1-7)
Signed-off-by: Jurgen Schindele <schindele@nentec.de>

--- a/arch/powerpc/sysdev/ipic.c	2016-12-11 20:17:54.000000000 +0100
+++ b/arch/powerpc/sysdev/ipic.c	2017-04-04 15:28:11.201308780 +0200
@@ -315,6 +315,7 @@ static struct ipic_info ipic_info[] = {
 		.prio_mask = 7,
 	},
 	[48] = {
+		.ack	= IPIC_SEPNR,
 		.mask	= IPIC_SEMSR,
 		.prio	= IPIC_SMPRR_A,
 		.force	= IPIC_SEFCR,
@@ -617,7 +618,7 @@ static int ipic_set_irq_type(struct irq_
 	/* ipic supports only edge mode on external interrupts */
 	if ((flow_type & IRQ_TYPE_EDGE_FALLING) && !ipic_info[src].ack) {
 		printk(KERN_ERR "ipic: edge sense not supported on internal "
-				"interrupts\n");
+				"interrupts %d\n", src);
 		return -EINVAL;

 	}

Re: Freescale mpc8315 IRQ0 setup

[Scott Wood]

On Thu, 2017-05-04 at 17:06 +0200, Juergen Schindele wrote:
> Am Dienstag, 2. Mai 2017, 22:29:34 schrieb Scott Wood:
> > On Tue, 2017-05-02 at 14:43 +0200, Juergen Schindele wrote:
> > > Dear Scott,
> > > sorry for the delay but i am not very familiar with the formating.
> > > I passed the patch trough checkpatch.pl and there was no more error.
> > > pease find patch in attached file.
> > > Thanks
> > 
> > Documentation/process/submitting-patches.rst explains the way to format
> > and
> > submit kernel patches.
> > 
> > Also, why the unrelated change to a print statement in
> > ipic_set_irq_type()?
> > 
> > -Scott
> 
> The second diff is not completely unrelated because when i was
> investigating 
> the problem i saw only a message "edge sense not supported" but you dont
> know on which interrupt he is complaining about. So i added this to find
> out 
> who the suspect is.

That's fine but it's still fixing a different problem than "irq0 setup" and
should be a separate patch.

> Corrected patch

Again, please read Documentation/process/submitting-patches.rst.  Patches
should be inline, not attached.  The subject line should be something like
"powerpc/ipic: Configure "EDGE" capabilities for IRQ0 (like IRQ1-7)" and there
should be more description in the body of the changelog.

-Scott

[PATCH] Freescale mpc8315 interrupt chip

[Juergen Schindele]

Next try to submit two patches for Freescale mpc8315.

first one
-----------------------------------------------------------------------=
------------
powerpc/ipic: Configure "EDGE" capabilities for IRQ0 too (like IRQ1-7)
Signed-off-by: Jurgen Schindele <schindele@nentec.de>

The external IRQ0 has the same capabilities as the other IRQ1-7 and is
handeled by the same register IPIC_SEPNR. When this register for "ack"
is not setup in "ipic_info" you can not configure this IRQ for
IRQ_TYPE_EDGE_FALLING. This is probably due to the non-continued number=

of IRQ0 in the Freescale hwirq number mapping.

--- linux-4.11/arch/powerpc/sysdev/ipic.c.orig  2017-05-11 13:40:43.874=
801534=20
+0200
+++ linux-4.11/arch/powerpc/sysdev/ipic.c       2017-05-11 14:05:06.336=
610289=20
+0200
@@ -315,6 +315,7 @@ static struct ipic_info ipic_info[] =3D {
                .prio_mask =3D 7,
        },
        [48] =3D {
+               .ack    =3D IPIC_SEPNR,
                .mask   =3D IPIC_SEMSR,
                .prio   =3D IPIC_SMPRR_A,
                .force  =3D IPIC_SEFCR,

second one
-----------------------------------------------------------------------=
------------------
powerpc/ipic: Precise ERR printk when "ipic_set_irq_type" fails
Signed-off-by: Jurgen Schindele <schindele@nentec.de>

When you setup an interrupt of a certain type on this interrupt-control=
er
which is not supported on this hardware you will get an error log but y=
ou
dont know which interrupt was the cause.
With this patch you are able to localize the source of failure.

--- linux-4.11/arch/powerpc/sysdev/ipic.c.orig  2017-05-11 14:08:07.098=
824986=20
+0200
+++ linux-4.11/arch/powerpc/sysdev/ipic.c       2017-05-11 14:10:51.010=
934755=20
+0200
@@ -611,14 +611,14 @@ static int ipic_set_irq_type(struct irq_
        /* ipic supports only low assertion and high-to-low change sens=
es
         */
        if (!(flow_type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING))=
) {
-               printk(KERN_ERR "ipic: sense type 0x%x not supported\n"=
,
-                       flow_type);
+               printk(KERN_ERR "ipic: sense type 0x%x not supported on=
 "
+                       "interrupt %d\n", flow_type, src);
                return -EINVAL;
        }
        /* ipic supports only edge mode on external interrupts */
        if ((flow_type & IRQ_TYPE_EDGE_FALLING) && !ipic_info[src].ack)=
 {
                printk(KERN_ERR "ipic: edge sense not supported on inte=
rnal "
-                               "interrupts\n");
+                               "interrupt %d\n", src);
                return -EINVAL;

        }

I hope i respected all patch requirements ;-)
Am Donnerstag, 4. Mai 2017, 15:55:28 schrieb Scott Wood:
> On Thu, 2017-05-04 at 17:06 +0200, Juergen Schindele wrote:
> > Am Dienstag, 2. Mai 2017, 22:29:34 schrieb Scott Wood:
> > > On Tue, 2017-05-02 at 14:43 +0200, Juergen Schindele wrote:
> > > > Dear Scott,
> > > > sorry for the delay but i am not very familiar with the formati=
ng.
> > > > I passed the patch trough checkpatch.pl and there was no more e=
rror.
> > > > pease find patch in attached file.
> > > > Thanks
> > >=20
> > > Documentation/process/submitting-patches.rst explains the way to =
format
> > > and
> > > submit kernel patches.
> > >=20
> > > Also, why the unrelated change to a print statement in
> > > ipic_set_irq_type()?
> > >=20
> > > -Scott
> >=20
> > The second diff is not completely unrelated because when i was
> > investigating=20
> > the problem i saw only a message "edge sense not supported" but you=
 dont
> > know on which interrupt he is complaining about. So i added this to=
 find
> > out=20
> > who the suspect is.
>=20
> That's fine but it's still fixing a different problem than "irq0 setu=
p" and
> should be a separate patch.
>=20
> > Corrected patch
>=20
> Again, please read Documentation/process/submitting-patches.rst.  Pat=
ches
> should be inline, not attached.  The subject line should be something=
 like
> "powerpc/ipic: Configure "EDGE" capabilities for IRQ0 (like IRQ1-7)" =
and
> there should be more description in the body of the changelog.
>=20
> -Scott
Thanks for your hints
--=20
i. A.
J=FCrgen Schindele
Softwareentwicklung

PSI Nentec GmbH
Greschbachstra=DFe 12
76229 Karlsruhe
Deutschland
Telefon: +49 721 94249-51
Telefax: +49 721 94249-10
schindele@nentec.de
www.nentec.de

Gesch=E4ftsf=FChrung: Klaus Becker, Wolfgang Fischer
Sitz der Gesellschaft: Karlsruhe
Handelsregister: Amtsgericht Mannheim HRB 107658

Diese E-Mail enth=E4lt vertrauliche oder rechtlich gesch=FCtzte Informa=
tionen.=20
Wenn Sie nicht der vorgesehene Empf=E4nger sind, informieren Sie bitte =
sofort=20
den Absender und l=F6schen Sie diese E-Mail. Das unbefugte Kopieren die=
ser E-
Mail oder die unbefugte Weitergabe der enthaltenen Informationen ist ni=
cht=20
gestattet.

The information contained in this message is confidential or protected =
by law.=20
If you are not the intended recipient, please contact the sender and de=
lete=20
this message. Any unauthorised copying of this message or unauthorised=20=

distribution of the information contained herein is prohibited. 

Re: [PATCH] Freescale mpc8315 interrupt chip

[Oliver O'Halloran]

On Thu, May 11, 2017 at 10:34 PM, Juergen Schindele <schindele@nentec.de> wrote:
> Next try to submit two patches for Freescale mpc8315.
>
> first one
> -----------------------------------------------------------------------------------
> powerpc/ipic: Configure "EDGE" capabilities for IRQ0 too (like IRQ1-7)
> Signed-off-by: Jurgen Schindele <schindele@nentec.de>
>
> The external IRQ0 has the same capabilities as the other IRQ1-7 and is
> handeled by the same register IPIC_SEPNR. When this register for "ack"
> is not setup in "ipic_info" you can not configure this IRQ for
> IRQ_TYPE_EDGE_FALLING. This is probably due to the non-continued number
> of IRQ0 in the Freescale hwirq number mapping.
>
> --- linux-4.11/arch/powerpc/sysdev/ipic.c.orig  2017-05-11 13:40:43.874801534
> +0200
> +++ linux-4.11/arch/powerpc/sysdev/ipic.c       2017-05-11 14:05:06.336610289
> +0200
> @@ -315,6 +315,7 @@ static struct ipic_info ipic_info[] = {
>                 .prio_mask = 7,
>         },
>         [48] = {
> +               .ack    = IPIC_SEPNR,
>                 .mask   = IPIC_SEMSR,
>                 .prio   = IPIC_SMPRR_A,
>                 .force  = IPIC_SEFCR,
>
> second one
> -----------------------------------------------------------------------------------------

> powerpc/ipic: Precise ERR printk when "ipic_set_irq_type" fails
> Signed-off-by: Jurgen Schindele <schindele@nentec.de>

The Signed-off-by: goes after the commit message. So rather than being here:
| >
| > When you setup an interrupt of a certain type on this interrupt-controler
| > which is not supported on this hardware you will get an error log but you
| > dont know which interrupt was the cause.
| > With this patch you are able to localize the source of failure.
|
\---> you want it down here

>
> --- linux-4.11/arch/powerpc/sysdev/ipic.c.orig  2017-05-11 14:08:07.098824986
> +0200
> +++ linux-4.11/arch/powerpc/sysdev/ipic.c       2017-05-11 14:10:51.010934755
> +0200
> @@ -611,14 +611,14 @@ static int ipic_set_irq_type(struct irq_
>         /* ipic supports only low assertion and high-to-low change senses
>          */
>         if (!(flow_type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING))) {
> -               printk(KERN_ERR "ipic: sense type 0x%x not supported\n",
> -                       flow_type);
> +               printk(KERN_ERR "ipic: sense type 0x%x not supported on "
> +                       "interrupt %d\n", flow_type, src);
>                 return -EINVAL;
>         }
>         /* ipic supports only edge mode on external interrupts */
>         if ((flow_type & IRQ_TYPE_EDGE_FALLING) && !ipic_info[src].ack) {
>                 printk(KERN_ERR "ipic: edge sense not supported on internal "
> -                               "interrupts\n");
> +                               "interrupt %d\n", src);

While you're here might as well change these from the old style
"printk(KERN_ERR" to the new style "pr_err()"

>                 return -EINVAL;
>
>         }
>

> I hope i respected all patch requirements ;-)

It's an interesting take on the rules, but it needs a bit of work.
We're pedantic about patch formatting, etc because we have tools for
handling patches that require strict formatting in order to function
(e.g https://patchwork.ozlabs.org/project/linuxppc-dev/list/). The
easiest way to ensure you get the formatting right is to use git to
generate the patch files for you, so:

First of all grab a kernel git tree (warning: it's big):
$ git clone https://github.com/torvalds/linux.git

Once it's done, make your changes inside of the git tree. You can run
"git diff" to get a diff between what has been commited in the tree
and what's in the filesystem. To start turning your changes into
patches use:
$ git add -p

NB: you can see what's been staged so fair with:
 $ git diff --staged

When you're happy with the staged changes commit them with:
$ git commit

This will give you a text editor where you can enter the commit
message. You can skip the signed-off-by for now. You'll probably get
yelled at by git for not having a name or an email. You should do what
it says to set those up so your name appears correctly in the patch.

Once you've done all that you should see two commits in:
$ git log

To finish generate the actual patch files with:
$ git format-patch -s HEAD~2

You can (in theory) use any email client to send patches. In practice
most clients are bad at handling whitespace and will do things like
turn tabs into spaces which breaks the diffs. The easiest way get it
right is to use git's email sending plugin. send-email isn't part of
the standard git package so you'll probably need to install and
configure it to use your local SMTP server. It's well worth the hassle
though.
$ git send-email --to=linuxppc-dev@lists.ozlabs.org <patch files go here>

Hope all that helps. There are much better git tutorials out there
than some idiot of the linuxppc-dev mailing list, but that should be
enough to get you started.
Oliver