From: Magnus Damm <magnus.damm@gmail.com> To: Geert Uytterhoeven <geert@linux-m68k.org> Cc: Simon Horman <horms@verge.net.au>, SH-Linux <linux-sh@vger.kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Jason Cooper <jason@lakedaemon.net> Subject: Re: [PATCH 02/02] ARM: shmobile: marzen-reference: Remove IRLM workaround Date: Thu, 04 Dec 2014 10:20:58 +0000 [thread overview] Message-ID: <CANqRtoRan=sNiN5Eo-CZhnTSFHBte8SyanGXnYsTC68K2_z=EQ@mail.gmail.com> (raw) In-Reply-To: <CAMuHMdVtHN6amVXP_uqEfrOyQG9S4fjeJPAe6Kvr94u_Sb6fPQ@mail.gmail.com> Hi Geert, On Thu, Dec 4, 2014 at 6:31 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Magnus, > > On Thu, Dec 4, 2014 at 10:24 AM, Magnus Damm <magnus.damm@gmail.com> wrote: >> On Thu, Dec 4, 2014 at 6:19 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >>> On Thu, Dec 4, 2014 at 8:33 AM, Magnus Damm <magnus.damm@gmail.com> wrote: >>>>>> --- 0002/arch/arm/boot/dts/r8a7779.dtsi >>>>>> +++ work/arch/arm/boot/dts/r8a7779.dtsi 2014-12-03 20:27:49.000000000 +0900 >>>>>> @@ -139,7 +139,7 @@ >>>>>> interrupt-controller; >>>>>> }; >>>>>> >>>>>> - irqpin0: irqpin@fe780010 { >>>>>> + irqpin0: irqpin@fe780000 { >>>>>> compatible = "renesas,intc-irqpin-r8a7779", "renesas,intc-irqpin"; >>>>>> #interrupt-cells = <2>; >>>>>> status = "disabled"; >>>>>> @@ -148,7 +148,8 @@ >>>>>> <0xfe780010 4>, >>>>>> <0xfe780024 4>, >>>>>> <0xfe780044 4>, >>>>>> - <0xfe780064 4>; >>>>>> + <0xfe780064 4>, >>>>>> + <0xfe780000 4>; >>>>> >>>>> Is there any order implied by the above list? >>>>> Naïvely I would expect it to be sorted numerically. >>>> >>>> Yes, the driver assumes the register banks to be passed in a certain >>>> order. In the case of r8a7779 we add one more register bank at the end >>>> for IRLM setup. Register detail (base address, access size, order and >>>> bitfield width) varies with SoC version. So the IRLM register will be >>>> at different addresses depending on SoC, but the driver wants it at >>>> the end of the list. >>> >>> As these are all individual registers, and there are that many, I think >>> it's worth adding a reg-names property to identify the registers. >>> Of course the driver still has to support the old anonymous order >>> for backwards compatibility. >> >> If we should rework things, then I propose going the other way around. >> =) Basically only passing a single base address with a certain SoC >> specific compat string, and based on that letting the driver >> internally figure out which register is at what offset and the access >> size and bitfield size. > > That's gonna mean a complete new compatible value. > Seems like we shouldn't have added "renesas,intc-irqpin-r8a7779", > as the SoC-type was encoded in the reg properties... Yeah, having that SoC specific compat string in the DTS does not exactly help us that this point. We could however use a slightly different SoC compat string or something else that is unused if we wanted to change things around. >> Either way we have a limited number of SoCs and they are all old. > > So your current patch looks like the best option for now > (can you promise future R-Car SoCs won't have an intc-irqpin hardware > block ;-)? Hehe, almost. =) I think I can promise that R-Car hardware won't use that hardware - the IRQC hardware block replaced INTC-IRQPIN in R-Car Gen2 and it will most likely not make a comeback. However, other SoC product lines may show up with old interrupt controllers. But if so we can rework things at that point and make use of a fresh compatible string for a clean start. Cheers, / magnus
WARNING: multiple messages have this Message-ID (diff)
From: Magnus Damm <magnus.damm@gmail.com> To: Geert Uytterhoeven <geert@linux-m68k.org> Cc: Simon Horman <horms@verge.net.au>, SH-Linux <linux-sh@vger.kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Jason Cooper <jason@lakedaemon.net> Subject: Re: [PATCH 02/02] ARM: shmobile: marzen-reference: Remove IRLM workaround Date: Thu, 4 Dec 2014 19:20:58 +0900 [thread overview] Message-ID: <CANqRtoRan=sNiN5Eo-CZhnTSFHBte8SyanGXnYsTC68K2_z=EQ@mail.gmail.com> (raw) In-Reply-To: <CAMuHMdVtHN6amVXP_uqEfrOyQG9S4fjeJPAe6Kvr94u_Sb6fPQ@mail.gmail.com> Hi Geert, On Thu, Dec 4, 2014 at 6:31 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Magnus, > > On Thu, Dec 4, 2014 at 10:24 AM, Magnus Damm <magnus.damm@gmail.com> wrote: >> On Thu, Dec 4, 2014 at 6:19 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >>> On Thu, Dec 4, 2014 at 8:33 AM, Magnus Damm <magnus.damm@gmail.com> wrote: >>>>>> --- 0002/arch/arm/boot/dts/r8a7779.dtsi >>>>>> +++ work/arch/arm/boot/dts/r8a7779.dtsi 2014-12-03 20:27:49.000000000 +0900 >>>>>> @@ -139,7 +139,7 @@ >>>>>> interrupt-controller; >>>>>> }; >>>>>> >>>>>> - irqpin0: irqpin@fe780010 { >>>>>> + irqpin0: irqpin@fe780000 { >>>>>> compatible = "renesas,intc-irqpin-r8a7779", "renesas,intc-irqpin"; >>>>>> #interrupt-cells = <2>; >>>>>> status = "disabled"; >>>>>> @@ -148,7 +148,8 @@ >>>>>> <0xfe780010 4>, >>>>>> <0xfe780024 4>, >>>>>> <0xfe780044 4>, >>>>>> - <0xfe780064 4>; >>>>>> + <0xfe780064 4>, >>>>>> + <0xfe780000 4>; >>>>> >>>>> Is there any order implied by the above list? >>>>> Naïvely I would expect it to be sorted numerically. >>>> >>>> Yes, the driver assumes the register banks to be passed in a certain >>>> order. In the case of r8a7779 we add one more register bank at the end >>>> for IRLM setup. Register detail (base address, access size, order and >>>> bitfield width) varies with SoC version. So the IRLM register will be >>>> at different addresses depending on SoC, but the driver wants it at >>>> the end of the list. >>> >>> As these are all individual registers, and there are that many, I think >>> it's worth adding a reg-names property to identify the registers. >>> Of course the driver still has to support the old anonymous order >>> for backwards compatibility. >> >> If we should rework things, then I propose going the other way around. >> =) Basically only passing a single base address with a certain SoC >> specific compat string, and based on that letting the driver >> internally figure out which register is at what offset and the access >> size and bitfield size. > > That's gonna mean a complete new compatible value. > Seems like we shouldn't have added "renesas,intc-irqpin-r8a7779", > as the SoC-type was encoded in the reg properties... Yeah, having that SoC specific compat string in the DTS does not exactly help us that this point. We could however use a slightly different SoC compat string or something else that is unused if we wanted to change things around. >> Either way we have a limited number of SoCs and they are all old. > > So your current patch looks like the best option for now > (can you promise future R-Car SoCs won't have an intc-irqpin hardware > block ;-)? Hehe, almost. =) I think I can promise that R-Car hardware won't use that hardware - the IRQC hardware block replaced INTC-IRQPIN in R-Car Gen2 and it will most likely not make a comeback. However, other SoC product lines may show up with old interrupt controllers. But if so we can rework things at that point and make use of a fresh compatible string for a clean start. Cheers, / magnus
next prev parent reply other threads:[~2014-12-04 10:20 UTC|newest] Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-12-03 12:17 [PATCH 00/02] r8a7779 renesas-intc-irqpin IRLM configuration patches Magnus Damm 2014-12-03 12:17 ` Magnus Damm 2014-12-03 12:18 ` [PATCH 01/02] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support Magnus Damm 2014-12-03 12:18 ` Magnus Damm 2014-12-04 7:18 ` Simon Horman 2014-12-04 7:18 ` Simon Horman 2014-12-04 7:29 ` Magnus Damm 2014-12-04 7:29 ` Magnus Damm 2014-12-04 7:51 ` Simon Horman 2014-12-04 7:51 ` Simon Horman 2015-01-26 10:43 ` [tip:irq/core] " tip-bot for Magnus Damm 2014-12-03 12:18 ` [PATCH 02/02] ARM: shmobile: marzen-reference: Remove IRLM workaround Magnus Damm 2014-12-03 12:18 ` Magnus Damm 2014-12-04 7:21 ` Simon Horman 2014-12-04 7:21 ` Simon Horman 2014-12-04 7:33 ` Magnus Damm 2014-12-04 7:33 ` Magnus Damm 2014-12-04 7:52 ` Simon Horman 2014-12-04 7:52 ` Simon Horman 2014-12-04 9:19 ` Geert Uytterhoeven 2014-12-04 9:19 ` Geert Uytterhoeven 2014-12-04 9:24 ` Magnus Damm 2014-12-04 9:24 ` Magnus Damm 2014-12-04 9:31 ` Geert Uytterhoeven 2014-12-04 9:31 ` Geert Uytterhoeven 2014-12-04 10:20 ` Magnus Damm [this message] 2014-12-04 10:20 ` Magnus Damm
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CANqRtoRan=sNiN5Eo-CZhnTSFHBte8SyanGXnYsTC68K2_z=EQ@mail.gmail.com' \ --to=magnus.damm@gmail.com \ --cc=geert@linux-m68k.org \ --cc=horms@verge.net.au \ --cc=jason@lakedaemon.net \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-sh@vger.kernel.org \ --cc=tglx@linutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.