pinctrl: sh-pfc: Extend Multiplatform Support

PATCH In review Magnus Damm 5 participants 12 messages May 7, 2014 – May 14, 2014 SuperH

lore

Share & tools

lore

Magnus Damm

author

2014-05-07 23:17 UTC #0 lore raw

cover pinctrl: sh-pfc: Extend Multiplatform Support

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 <[email protected]>

---

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(-)

Geert Uytterhoeven

2014-05-09 08:46 UTC #1 lore raw

Hi Magnus,

Quoted 7 lines · 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.

Thanks for your patches!

Quoted 4 lines

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 -- [email protected]

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

Linus Walleij

2014-05-13 09:28 UTC #2 lore raw

In reply to Magnus Damm

Quoted 7 lines · 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.

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

Yours,
Linus Walleij

Laurent Pinchart

2014-05-13 11:24 UTC #3 lore raw

In reply to Magnus Damm

Hi Magnus,

Thank you for the patches.

Quoted 6 lines · 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

Quoted 6 lines

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.

Quoted 13 lines

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 <[email protected]>
---
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(-)

Signature · 3 lines

Regards,

Laurent Pinchart

Simon Horman

2014-05-13 21:42 UTC #4 lore raw

In reply to Magnus Damm

Quoted 29 lines · Laurent Pinchart

Hi Magnus,
Thank you for the patches.
Quoted 6 lines · 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
Quoted 6 lines
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;
        }

Quoted 6 lines

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).

Laurent Pinchart

2014-05-13 21:50 UTC #5 lore raw

In reply to Magnus Damm

Hi Simon,

Quoted 35 lines · On Wednesday 14 May 2014 06:42:48 Simon Horman wrote:

Quoted 34 lines · Laurent Pinchart
Hi Magnus,
Thank you for the patches.
Quoted 6 lines · 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
Quoted 8 lines
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;
        
        }

Quoted 8 lines

Quoted 6 lines
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.

Quoted 2 lines

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.

Signature · 3 lines

Regards,

Laurent Pinchart

Simon Horman

2014-05-13 23:45 UTC #6 lore raw

In reply to Magnus Damm

Quoted 47 lines · Laurent Pinchart

Hi Simon,
Quoted 35 lines · On Wednesday 14 May 2014 06:42:48 Simon Horman wrote:
Quoted 34 lines · Laurent Pinchart
Hi Magnus,
Thank you for the patches.
Quoted 6 lines · 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
Quoted 8 lines
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;
        
        }
Quoted 8 lines
Quoted 6 lines
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.

Quoted 8 lines

Making sure that the GPIO keys still work on Armadillo with a multiplatform kernel should be enough.
Quoted 2 lines
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.

Simon Horman

2014-05-14 03:08 UTC #7 lore raw

In reply to Magnus Damm

Quoted 54 lines · Simon Horman

Quoted 53 lines · Laurent Pinchart
Hi Simon,
Quoted 35 lines · On Wednesday 14 May 2014 06:42:48 Simon Horman wrote:
Quoted 34 lines · Laurent Pinchart
Hi Magnus,
Thank you for the patches.
Quoted 6 lines · 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
Quoted 8 lines
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;
        
        }
Quoted 8 lines
Quoted 6 lines
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.
Quoted 2 lines
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).

Quoted 7 lines

Quoted 2 lines
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?

Laurent Pinchart

2014-05-14 03:23 UTC #8 lore raw

In reply to Magnus Damm

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

[snip]

Quoted 4 lines

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 :-)

Signature · 3 lines

Regards,

Laurent Pinchart

Simon Horman

2014-05-14 03:33 UTC #9 lore raw

In reply to Magnus Damm

Quoted 10 lines · Laurent Pinchart

On Wednesday 14 May 2014 12:08:32 Simon Horman wrote:
[snip]
Quoted 4 lines
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! :)

Geert Uytterhoeven

2014-05-14 09:00 UTC #10 lore raw

In reply to Magnus Damm

Hi Laurent, Simon,

Quoted 9 lines · Laurent Pinchart

On Wednesday 14 May 2014 12:08:32 Simon Horman wrote:
[snip]
Quoted 4 lines
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 -- [email protected]

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

Simon Horman

2014-05-14 23:05 UTC #11 lore raw

In reply to Magnus Damm

Quoted 9 lines · Geert Uytterhoeven

Hi Laurent, Simon,
Quoted 5 lines · Laurent Pinchart
On Wednesday 14 May 2014 12:08:32 Simon Horman wrote:
[snip]
Quoted 2 lines
I got a little ahead of myself there.

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
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