ARM: shmobile: Koelsch SDHI DT support
Subsystems (2)
cover ARM: shmobile: Koelsch SDHI DT support
ARM: shmobile: Koelsch SDHI DT support
[PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI [PATCH 02/02] ARM: shmobile: Add SDHI devices for Koelsch DTS
Add SDHI devices to the Koelsch DT board support. The same run time dependencies as SDHI for Lager applies here too, and there is a PFC fix for SD1_CLK that is needed for r8a7791 as well.
Can be merged independently without waiting for v3.15 patches.
---
Written against renesas-devel-v3.14-rc1-20140207
Requires the following patch for correct run time operation:
[PATCH 02/03] pinctrl: sh-pfc: r8a7791: SD1_CLK fix
arch/arm/boot/dts/r8a7791-koelsch.dts | 131 +++++++++++++++++++++++++++++++++ arch/arm/boot/dts/r8a7791.dtsi | 27 ++++++ 2 files changed, 158 insertions(+)
PATCH v2 1/2 ARM: shmobile: Add SDHI devices to r8a7791 DTSI
From: Magnus Damm <[email protected]>
Add SDHI0, SDHI1 and SDHI2 to the r8a7791 DTSI.
---
Changes since V1:
- add r8a7790 suffix as fall back in case r8a7791 is missing from the driver
arch/arm/boot/dts/r8a7791.dtsi | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
arch/arm/boot/dts/r8a7791.dtsi +27 -0
@@ -186,6 +186,33 @@ #gpio-range-cells = <3>; }; + sdhi0: sd@ee100000 { + compatible = "renesas,sdhi-r8a7791", "renesas,sdhi-r8a7790"; + reg = <0 0xee100000 0 0x200>; + interrupt-parent = <&gic>; + interrupts = <0 165 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&mstp3_clks R8A7791_CLK_SDHI0>; + status = "disabled"; + }; + + sdhi1: sd@ee140000 { + compatible = "renesas,sdhi-r8a7791", "renesas,sdhi-r8a7790"; + reg = <0 0xee140000 0 0x100>; + interrupt-parent = <&gic>; + interrupts = <0 167 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&mstp3_clks R8A7791_CLK_SDHI1>; + status = "disabled"; + }; + + sdhi2: sd@ee160000 { + compatible = "renesas,sdhi-r8a7791", "renesas,sdhi-r8a7790"; + reg = <0 0xee160000 0 0x100>; + interrupt-parent = <&gic>; + interrupts = <0 168 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&mstp3_clks R8A7791_CLK_SDHI2>; + status = "disabled"; + }; + scifa0: serial@e6c40000 { compatible = "renesas,scifa-r8a7791", "renesas,scifa"; reg = <0 0xe6c40000 0 64>;
Re: [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI
Hi Magnus,
Quoted 2 lines · Magnus Damm
Changes since V1:
- add r8a7790 suffix as fall back in case r8a7791 is missing from the driver
I'm afraid that's not such a good idea: what if r8a7791 SDHI turns out to have a slight incompatibility with r8a7790 SDHI later?
What about adding "renesas,sdhi-rcar-gen2" as a fallback instead, for both r8a7790
and r8a7791?
This would reduce the size of the sh_mobile_sdhi_of_match[] table in drivers/mmc/host/sh_mobile_sdhi.c, as no SoC-specific entries are needed at the moment.
A similar thing can be done for "renesas,sdhi-rcar-gen1" (as long as those DTSes are not in mainline).
arch/arm/boot/dts/r8a7791.dtsi 2014-02-12 16:29:40.000000000 +0900 line 190
Quoted 11 lines
arch/arm/boot/dts/r8a7791.dtsi | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)arch/arm/boot/dts/r8a7791.dtsi 2014-02-12 16:29:40.000000000 +0900 renamed from arch/arm/boot/dts/r8a7791.dtsi +2
-0@@ -186,6 +186,33 @@ #gpio-range-cells = <3>; }; + sdhi0: sd@ee100000 { + compatible = "renesas,sdhi-r8a7791", "renesas,sdhi-r8a7790";
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
Re: [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI
In reply to Magnus DammHi Geert,
Quoted 8 lines · Geert Uytterhoeven
Hi Magnus,
Quoted 2 lines · Magnus Damm
Changes since V1:
- add r8a7790 suffix as fall back in case r8a7791 is missing from the driverI'm afraid that's not such a good idea: what if r8a7791 SDHI turns out to have a slight incompatibility with r8a7790 SDHI later?
It's a fallback so later we will simply use r8a7791 instead. r8a7790 is currently used with the v3.14-rc version of the SDHI driver. Future ones always include r8a7791. How does that introduce any issues?
To be clear, I'm not introducing a new ABI here, just using something that already exists. Of course, I can simply not use the fallback which is what I did in V1 of the patch. But then I can't use the v3.14-rc1 driver with r8a7791.
Morimoto-san has just posted a patch to add r8a7791 support to the SDHI driver, so at some point in the future the r8a7790 association won't be used anymore. Maybe v3.15-rc1, maybe later. This of course depends on patch pick up pace.
Quoted 3 lines
What about adding "renesas,sdhi-rcar-gen2" as a fallback instead, for both r8a7790
and r8a7791?
Not sure if that makes much sense really, wouldn't that require driver changes? If so we may as well modify the driver to add r8a7791 directly. =)
Also, I don't like to use "gen2" as compatible string. This because all gen2 SoCs have not been released yet. For platform devices this is not so serious due to lack of ABI, so there I am willing to accept "gen2" or "r8a77xx" or something since we can change that any time. But for DT bindings...
Ideally I'd prefer not to use the SoC string at all, but hey...
Quoted 3 lines
This would reduce the size of the sh_mobile_sdhi_of_match[] table in
drivers/mmc/host/sh_mobile_sdhi.c, as no SoC-specific entries are needed at the moment.
I understand the space saving benefit, but due to documentation issues we don't really know the difference between various SoCs, so assuming all the SoCs in the same generation is most likely not such a good idea.
Quoted 2 lines
A similar thing can be done for "renesas,sdhi-rcar-gen1" (as long as those DTSes are not in mainline).
We've had discussions internally about using the VERSION register in the SDHI hardware. This would be needed in some cases because withing a certain SoC the SDHI hardware varies with instance. So for r8a7790 SDHI0 and SDHI1 are different from SDHI2 and SDHI3. Now we get by because they have different I/O space size.
Cheers,
/ magnus
Re: [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI
In reply to Magnus DammQuoted 10 lines · Magnus Damm
Quoted 6 lines · Magnus Damm
Quoted 2 lines
Changes since V1:
- add r8a7790 suffix as fall back in case r8a7791 is missing from the driverI'm afraid that's not such a good idea: what if r8a7791 SDHI turns out to have a slight incompatibility with r8a7790 SDHI later?
It's a fallback so later we will simply use r8a7791 instead. r8a7790 is currently used with the v3.14-rc version of the SDHI driver. Future ones always include r8a7791. How does that introduce any issues?
Bummer, yes, it prefers the first compatible entry.
Will refrain from sending more emails until my coffee has been digested ;-)
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
Re: [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI
In reply to Magnus DammQuoted 8 lines · Geert Uytterhoeven
Quoted 7 lines · Magnus Damm
Quoted 6 lines · Magnus Damm
Quoted 2 lines
Changes since V1:
- add r8a7790 suffix as fall back in case r8a7791 is missing from the driverI'm afraid that's not such a good idea: what if r8a7791 SDHI turns out to have a slight incompatibility with r8a7790 SDHI later?
Quoted 7 lines
Quoted 3 lines
It's a fallback so later we will simply use r8a7791 instead. r8a7790 is currently used with the v3.14-rc version of the SDHI driver. Future ones always include r8a7791. How does that introduce any issues?
Bummer, yes, it prefers the first compatible entry.
Will refrain from sending more emails until my coffee has been digested ;-)
Heheh. But I think you raise a valid point:
Using the wrong SoC in the compatible string is pretty darn ugly.
What is the best practise here?
/ magnus
Re: [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI
Quoted 14 lines · Magnus Damm
Quoted 14 lines
Quoted 11 lines
Quoted 7 lines
Review
Quoted 4 lines
- add r8a7790 suffix as fall back in case r8a7791 is missing from the driver
(quoted diff) +2
-0+ sdhi0: sd@ee100000 { + compatible = "renesas,sdhi-r8a7791", "renesas,sdhi-r8a7790";I'm afraid that's not such a good idea: what if r8a7791 SDHI turns out to have a slight incompatibility with r8a7790 SDHI later?
It's a fallback so later we will simply use r8a7791 instead. r8a7790 is currently used with the v3.14-rc version of the SDHI driver. Future ones always include r8a7791. How does that introduce any issues?
Bummer, yes, it prefers the first compatible entry.
Quoted 7 lines
Quoted 1 line
Will refrain from sending more emails until my coffee has been digested ;-)
Heheh. But I think you raise a valid point:
Using the wrong SoC in the compatible string is pretty darn ugly.
What is the best practise here?
(adding devicetree)
It should be a reference to the hardware block.
Before the advent of SoCs, a hardware block was an IC, with a part number (e.g. de21040), and sometimes a name (e.g. tulip). Further evolutions of the hardware block got (usually, but not always) different part numbers.
With SoCs, the part numbers of the hardware blocks were lost. Only the SoC still carries a part number. The hardware blocks (now called "IP cores") still have names (e.g. RSPI), but they're more abstract, and it's difficult to know what exact version of the hardware block they're referring to[*].
Using "<manufacturer>,<name>-<soc>" is future-proof, but cumbersome. Having a more generic fallback name to group SoCs with the same IP core is convenient.
So we have to "invent" generic fallback names with versioning ourselves?
- SoC family name? But there's no guarantee a new SoC from the same family will be 100% compatible.
- "-v1", "-v2" suffixes? The new SoC may have something in between v1 and v2
- ???
[*] With OpenRISC it's easier, as we have the hardware source files, but
it's still cumbersome to find good version naming.
The OpenCores-mandated "<name>-rtlsvn<version>" is not such a good match for todays distributed development, with IP cores being imported into git repositories and modified there.
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