[PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[Magnus Damm]

pinctrl: sh-pfc: Extend Multiplatform Support

[PATCH 01/03] pinctrl: sh-pfc: r8a7740: Allow Multiplatform Build
[PATCH 02/03] pinctrl: sh-pfc: sh73a0: Allow Multiplatform Build
[PATCH 03/03] pinctrl: sh-pfc: r8a73a4: Allow Multiplatform Build

These patches enable Multiplatform support for r8a7740, sh73a0 and r8a73a4.

The legacy case is still kept around, so the mach-header is still needed
to allow static GPIO-IRQ assignment. In case of DT the GPIO IRQs are already
provided via DT properties, so we can simply #ifdef out the static IRQ tables
in case of Multiplatform build. Not pretty but it works.

sh7372 is intentionally excluded from this series since it will not be
moved over to Multiplatform. Instead software support for sh7372 will be
phased out in the a few kernel releases.

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 Written against renesas-devel-v3.15-rc3-20140502

 drivers/pinctrl/sh-pfc/pfc-r8a73a4.c |    7 +++++++
 drivers/pinctrl/sh-pfc/pfc-r8a7740.c |    7 ++++++-
 drivers/pinctrl/sh-pfc/pfc-sh73a0.c  |    6 ++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

Re: [PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[Geert Uytterhoeven]

Hi Magnus,

On Thu, May 8, 2014 at 1:17 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> pinctrl: sh-pfc: Extend Multiplatform Support
>
> [PATCH 01/03] pinctrl: sh-pfc: r8a7740: Allow Multiplatform Build
> [PATCH 02/03] pinctrl: sh-pfc: sh73a0: Allow Multiplatform Build
> [PATCH 03/03] pinctrl: sh-pfc: r8a73a4: Allow Multiplatform Build
>
> These patches enable Multiplatform support for r8a7740, sh73a0 and r8a73a4.

Thanks  for your patches!

> The legacy case is still kept around, so the mach-header is still needed
> to allow static GPIO-IRQ assignment. In case of DT the GPIO IRQs are already
> provided via DT properties, so we can simply #ifdef out the static IRQ tables
> in case of Multiplatform build. Not pretty but it works.

I think it's worthwhile to add the above paragraph to the individual patches,
as the cover letter is not committed.

Speaking about the DT properties: this is handled through the "intc*" and
"irqc*" pinmux groups on r8a7740 resp. r8a73a4, right?

r8a7740-armadillo800eva-reference.dts does have a reference to pinmux
group intc_irq10, but arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts
doesn't have any. Perhaps the feature is unused on ape6evm?

How is it handled on sh73a0? I didn't find any irq pinmux group definitions
in drivers/pinctrl/sh-pfc/pfc-sh73a0.c. Am I missing something?

Thanks again!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[Linus Walleij]

On Thu, May 8, 2014 at 1:17 AM, Magnus Damm <magnus.damm@gmail.com> wrote:

> pinctrl: sh-pfc: Extend Multiplatform Support
>
> [PATCH 01/03] pinctrl: sh-pfc: r8a7740: Allow Multiplatform Build
> [PATCH 02/03] pinctrl: sh-pfc: sh73a0: Allow Multiplatform Build
> [PATCH 03/03] pinctrl: sh-pfc: r8a73a4: Allow Multiplatform Build
>
> These patches enable Multiplatform support for r8a7740, sh73a0 and r8a73a4.

Hm can I have Laurent's quick look at this?

Yours,
Linus Walleij

Re: [PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[Laurent Pinchart]

Hi Magnus,

Thank you for the patches.

On Thursday 08 May 2014 08:17:48 Magnus Damm wrote:
> pinctrl: sh-pfc: Extend Multiplatform Support
> 
> [PATCH 01/03] pinctrl: sh-pfc: r8a7740: Allow Multiplatform Build
> [PATCH 02/03] pinctrl: sh-pfc: sh73a0: Allow Multiplatform Build
> [PATCH 03/03] pinctrl: sh-pfc: r8a73a4: Allow Multiplatform Build
> 
> These patches enable Multiplatform support for r8a7740, sh73a0 and r8a73a4.
> 
> The legacy case is still kept around, so the mach-header is still needed
> to allow static GPIO-IRQ assignment. In case of DT the GPIO IRQs are already
> provided via DT properties, so we can simply #ifdef out the static IRQ
> tables in case of Multiplatform build. Not pretty but it works.

Well, it builds, but does it really work ? If I'm not mistaken this patch set 
essentially disables gpio_to_irq support for the above three SoCs as the 
gpio_pin_to_irq() function (drivers/pinctrl/sh-pfc/gpio.c) will immediately 
return -ENOSYS due to pfc->info->gpio_irq_size being 0. Worse, the 
sh_pfc_register_gpiochip() function will fail with an error due to

        if (pfc->num_irqs && pfc->num_irqs != pfc->info->gpio_irq_size) {
                dev_err(pfc->dev, "invalid number of IRQ resources\n");
                return -EINVAL;
        }

with num_irqs being non-zero and gpio_irq_size being 0.

Am I missing something, or is this series completely untested ? :-)

I'll send another (also untested, as I have no Armadillo board anymore ;-)) 
patch set shortly with a proposed fix.

> sh7372 is intentionally excluded from this series since it will not be
> moved over to Multiplatform. Instead software support for sh7372 will be
> phased out in the a few kernel releases.
> 
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  Written against renesas-devel-v3.15-rc3-20140502
> 
>  drivers/pinctrl/sh-pfc/pfc-r8a73a4.c |    7 +++++++
>  drivers/pinctrl/sh-pfc/pfc-r8a7740.c |    7 ++++++-
>  drivers/pinctrl/sh-pfc/pfc-sh73a0.c  |    6 ++++++
>  3 files changed, 19 insertions(+), 1 deletion(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[Simon Horman]

On Tue, May 13, 2014 at 01:24:09PM +0200, Laurent Pinchart wrote:
> Hi Magnus,
> 
> Thank you for the patches.
> 
> On Thursday 08 May 2014 08:17:48 Magnus Damm wrote:
> > pinctrl: sh-pfc: Extend Multiplatform Support
> > 
> > [PATCH 01/03] pinctrl: sh-pfc: r8a7740: Allow Multiplatform Build
> > [PATCH 02/03] pinctrl: sh-pfc: sh73a0: Allow Multiplatform Build
> > [PATCH 03/03] pinctrl: sh-pfc: r8a73a4: Allow Multiplatform Build
> > 
> > These patches enable Multiplatform support for r8a7740, sh73a0 and r8a73a4.
> > 
> > The legacy case is still kept around, so the mach-header is still needed
> > to allow static GPIO-IRQ assignment. In case of DT the GPIO IRQs are already
> > provided via DT properties, so we can simply #ifdef out the static IRQ
> > tables in case of Multiplatform build. Not pretty but it works.
> 
> Well, it builds, but does it really work ? If I'm not mistaken this patch set 
> essentially disables gpio_to_irq support for the above three SoCs as the 
> gpio_pin_to_irq() function (drivers/pinctrl/sh-pfc/gpio.c) will immediately 
> return -ENOSYS due to pfc->info->gpio_irq_size being 0. Worse, the 
> sh_pfc_register_gpiochip() function will fail with an error due to
> 
>         if (pfc->num_irqs && pfc->num_irqs != pfc->info->gpio_irq_size) {
>                 dev_err(pfc->dev, "invalid number of IRQ resources\n");
>                 return -EINVAL;
>         }
> 
> with num_irqs being non-zero and gpio_irq_size being 0.
> 
> Am I missing something, or is this series completely untested ? :-)
> 
> I'll send another (also untested, as I have no Armadillo board anymore ;-)) 
> patch set shortly with a proposed fix.

I'm happy to provide testing, if you let me know what to look for.
I'm also happy to provide remote access to a board if you think
this would be useful in future (its a bit of work for me to set up).

Re: [PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[Laurent Pinchart]

Hi Simon,

On Wednesday 14 May 2014 06:42:48 Simon Horman wrote:
> On Tue, May 13, 2014 at 01:24:09PM +0200, Laurent Pinchart wrote:
> > Hi Magnus,
> > 
> > Thank you for the patches.
> > 
> > On Thursday 08 May 2014 08:17:48 Magnus Damm wrote:
> > > pinctrl: sh-pfc: Extend Multiplatform Support
> > > 
> > > [PATCH 01/03] pinctrl: sh-pfc: r8a7740: Allow Multiplatform Build
> > > [PATCH 02/03] pinctrl: sh-pfc: sh73a0: Allow Multiplatform Build
> > > [PATCH 03/03] pinctrl: sh-pfc: r8a73a4: Allow Multiplatform Build
> > > 
> > > These patches enable Multiplatform support for r8a7740, sh73a0 and
> > > r8a73a4.
> > > 
> > > The legacy case is still kept around, so the mach-header is still needed
> > > to allow static GPIO-IRQ assignment. In case of DT the GPIO IRQs are
> > > already provided via DT properties, so we can simply #ifdef out the
> > > static IRQ tables in case of Multiplatform build. Not pretty but it
> > > works.
> > 
> > Well, it builds, but does it really work ? If I'm not mistaken this patch
> > set essentially disables gpio_to_irq support for the above three SoCs as
> > the gpio_pin_to_irq() function (drivers/pinctrl/sh-pfc/gpio.c) will
> > immediately return -ENOSYS due to pfc->info->gpio_irq_size being 0.
> > Worse, the sh_pfc_register_gpiochip() function will fail with an error
> > due to
> >
> >         if (pfc->num_irqs && pfc->num_irqs != pfc->info->gpio_irq_size) {
> >         
> >                 dev_err(pfc->dev, "invalid number of IRQ resources\n");
> >                 return -EINVAL;
> >         
> >         }
> > 
> > with num_irqs being non-zero and gpio_irq_size being 0.
> > 
> > Am I missing something, or is this series completely untested ? :-)
> > 
> > I'll send another (also untested, as I have no Armadillo board anymore
> > ;-)) patch set shortly with a proposed fix.
> 
> I'm happy to provide testing, if you let me know what to look for.

Making sure that the GPIO keys still work on Armadillo with a multiplatform 
kernel should be enough.

> I'm also happy to provide remote access to a board if you think this would
> be useful in future (its a bit of work for me to set up).

It might be useful in the future, but given that the test is pretty simple 
here, it would probably be easier if you could perform it yourself.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[Simon Horman]

On Tue, May 13, 2014 at 11:50:31PM +0200, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Wednesday 14 May 2014 06:42:48 Simon Horman wrote:
> > On Tue, May 13, 2014 at 01:24:09PM +0200, Laurent Pinchart wrote:
> > > Hi Magnus,
> > > 
> > > Thank you for the patches.
> > > 
> > > On Thursday 08 May 2014 08:17:48 Magnus Damm wrote:
> > > > pinctrl: sh-pfc: Extend Multiplatform Support
> > > > 
> > > > [PATCH 01/03] pinctrl: sh-pfc: r8a7740: Allow Multiplatform Build
> > > > [PATCH 02/03] pinctrl: sh-pfc: sh73a0: Allow Multiplatform Build
> > > > [PATCH 03/03] pinctrl: sh-pfc: r8a73a4: Allow Multiplatform Build
> > > > 
> > > > These patches enable Multiplatform support for r8a7740, sh73a0 and
> > > > r8a73a4.
> > > > 
> > > > The legacy case is still kept around, so the mach-header is still needed
> > > > to allow static GPIO-IRQ assignment. In case of DT the GPIO IRQs are
> > > > already provided via DT properties, so we can simply #ifdef out the
> > > > static IRQ tables in case of Multiplatform build. Not pretty but it
> > > > works.
> > > 
> > > Well, it builds, but does it really work ? If I'm not mistaken this patch
> > > set essentially disables gpio_to_irq support for the above three SoCs as
> > > the gpio_pin_to_irq() function (drivers/pinctrl/sh-pfc/gpio.c) will
> > > immediately return -ENOSYS due to pfc->info->gpio_irq_size being 0.
> > > Worse, the sh_pfc_register_gpiochip() function will fail with an error
> > > due to
> > >
> > >         if (pfc->num_irqs && pfc->num_irqs != pfc->info->gpio_irq_size) {
> > >         
> > >                 dev_err(pfc->dev, "invalid number of IRQ resources\n");
> > >                 return -EINVAL;
> > >         
> > >         }
> > > 
> > > with num_irqs being non-zero and gpio_irq_size being 0.
> > > 
> > > Am I missing something, or is this series completely untested ? :-)
> > > 
> > > I'll send another (also untested, as I have no Armadillo board anymore
> > > ;-)) patch set shortly with a proposed fix.
> > 
> > I'm happy to provide testing, if you let me know what to look for.
> 
> Making sure that the GPIO keys still work on Armadillo with a multiplatform 
> kernel should be enough.
> 
> > I'm also happy to provide remote access to a board if you think this would
> > be useful in future (its a bit of work for me to set up).
> 
> It might be useful in the future, but given that the test is pretty simple 
> here, it would probably be easier if you could perform it yourself.

Agreed.

As it involves pressing keys I'll defer it until I get to my office a
little later today.

Re: [PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[Simon Horman]

On Wed, May 14, 2014 at 08:45:09AM +0900, Simon Horman wrote:
> On Tue, May 13, 2014 at 11:50:31PM +0200, Laurent Pinchart wrote:
> > Hi Simon,
> > 
> > On Wednesday 14 May 2014 06:42:48 Simon Horman wrote:
> > > On Tue, May 13, 2014 at 01:24:09PM +0200, Laurent Pinchart wrote:
> > > > Hi Magnus,
> > > > 
> > > > Thank you for the patches.
> > > > 
> > > > On Thursday 08 May 2014 08:17:48 Magnus Damm wrote:
> > > > > pinctrl: sh-pfc: Extend Multiplatform Support
> > > > > 
> > > > > [PATCH 01/03] pinctrl: sh-pfc: r8a7740: Allow Multiplatform Build
> > > > > [PATCH 02/03] pinctrl: sh-pfc: sh73a0: Allow Multiplatform Build
> > > > > [PATCH 03/03] pinctrl: sh-pfc: r8a73a4: Allow Multiplatform Build
> > > > > 
> > > > > These patches enable Multiplatform support for r8a7740, sh73a0 and
> > > > > r8a73a4.
> > > > > 
> > > > > The legacy case is still kept around, so the mach-header is still needed
> > > > > to allow static GPIO-IRQ assignment. In case of DT the GPIO IRQs are
> > > > > already provided via DT properties, so we can simply #ifdef out the
> > > > > static IRQ tables in case of Multiplatform build. Not pretty but it
> > > > > works.
> > > > 
> > > > Well, it builds, but does it really work ? If I'm not mistaken this patch
> > > > set essentially disables gpio_to_irq support for the above three SoCs as
> > > > the gpio_pin_to_irq() function (drivers/pinctrl/sh-pfc/gpio.c) will
> > > > immediately return -ENOSYS due to pfc->info->gpio_irq_size being 0.
> > > > Worse, the sh_pfc_register_gpiochip() function will fail with an error
> > > > due to
> > > >
> > > >         if (pfc->num_irqs && pfc->num_irqs != pfc->info->gpio_irq_size) {
> > > >         
> > > >                 dev_err(pfc->dev, "invalid number of IRQ resources\n");
> > > >                 return -EINVAL;
> > > >         
> > > >         }
> > > > 
> > > > with num_irqs being non-zero and gpio_irq_size being 0.
> > > > 
> > > > Am I missing something, or is this series completely untested ? :-)
> > > > 
> > > > I'll send another (also untested, as I have no Armadillo board anymore
> > > > ;-)) patch set shortly with a proposed fix.
> > > 
> > > I'm happy to provide testing, if you let me know what to look for.
> > 
> > Making sure that the GPIO keys still work on Armadillo with a multiplatform 
> > kernel should be enough.
> > 
> > > I'm also happy to provide remote access to a board if you think this would
> > > be useful in future (its a bit of work for me to set up).
> > 
> > It might be useful in the future, but given that the test is pretty simple 
> > here, it would probably be easier if you could perform it yourself.
> 
> Agreed.
> 
> As it involves pressing keys I'll defer it until I get to my office a
> little later today.

I got a little ahead of myself there.

I'm not sure how to compile (and boot) a multiplaform kernel for Armadillo.
Do I need some patches which aren't in the renesas devel branch?

Re: [PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[Laurent Pinchart]

On Wednesday 14 May 2014 12:08:32 Simon Horman wrote:

[snip]

> I got a little ahead of myself there.
> 
> I'm not sure how to compile (and boot) a multiplaform kernel for Armadillo.
> Do I need some patches which aren't in the renesas devel branch?

You most probably do, and I expect those patches to be in Magnus' tree :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[Simon Horman]

On Wed, May 14, 2014 at 05:23:49AM +0200, Laurent Pinchart wrote:
> On Wednesday 14 May 2014 12:08:32 Simon Horman wrote:
> 
> [snip]
> 
> > I got a little ahead of myself there.
> > 
> > I'm not sure how to compile (and boot) a multiplaform kernel for Armadillo.
> > Do I need some patches which aren't in the renesas devel branch?
> 
> You most probably do, and I expect those patches to be in Magnus' tree :-)

Thanks, I'll follow up with Magnus.

BTW, get some sleep soon! :)

Re: [PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[Geert Uytterhoeven]

Hi Laurent, Simon,

On Wed, May 14, 2014 at 5:23 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Wednesday 14 May 2014 12:08:32 Simon Horman wrote:
> [snip]
>
>> I got a little ahead of myself there.
>>
>> I'm not sure how to compile (and boot) a multiplaform kernel for Armadillo.
>> Do I need some patches which aren't in the renesas devel branch?
>
> You most probably do, and I expect those patches to be in Magnus' tree :-)

Yes. You need at least Ulrich's CCF patches.
And some extras (e.g. to hook it up into multi-platform) that AFAIK have
never been posted to the list.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 00/03] pinctrl: sh-pfc: Extend Multiplatform Support

[Simon Horman]

On Wed, May 14, 2014 at 11:00:22AM +0200, Geert Uytterhoeven wrote:
> Hi Laurent, Simon,
> 
> On Wed, May 14, 2014 at 5:23 AM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Wednesday 14 May 2014 12:08:32 Simon Horman wrote:
> > [snip]
> >
> >> I got a little ahead of myself there.
> >>
> >> I'm not sure how to compile (and boot) a multiplaform kernel for Armadillo.
> >> Do I need some patches which aren't in the renesas devel branch?
> >
> > You most probably do, and I expect those patches to be in Magnus' tree :-)
> 
> Yes. You need at least Ulrich's CCF patches.
> And some extras (e.g. to hook it up into multi-platform) that AFAIK have
> never been posted to the list.

Ok, I checked my git-over-telepathy skills this morning but I drew a blank.

Would it be possible for someone with all the pieces (assuming
there is someone) to either:

* Send a series or make a branch with them all present or;
* Test this series