* Re: [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI
2014-02-12 7:41 ` [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI Magnus Damm
@ 2014-02-12 8:19 ` Geert Uytterhoeven
2014-02-12 8:37 ` Magnus Damm
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2014-02-12 8:19 UTC (permalink / raw)
To: linux-sh
Hi Magnus,
On Wed, Feb 12, 2014 at 8:41 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> 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 | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> --- 0001/arch/arm/boot/dts/r8a7791.dtsi
> +++ work/arch/arm/boot/dts/r8a7791.dtsi 2014-02-12 16:29:40.000000000 +0900
> @@ -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 -- 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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI
2014-02-12 7:41 ` [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI Magnus Damm
2014-02-12 8:19 ` Geert Uytterhoeven
@ 2014-02-12 8:37 ` Magnus Damm
2014-02-12 8:48 ` Geert Uytterhoeven
2014-02-12 8:51 ` Magnus Damm
3 siblings, 0 replies; 7+ messages in thread
From: Magnus Damm @ 2014-02-12 8:37 UTC (permalink / raw)
To: linux-sh
Hi Geert,
On Wed, Feb 12, 2014 at 5:19 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Wed, Feb 12, 2014 at 8:41 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> 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?
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.
> 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...
> 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.
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI
2014-02-12 7:41 ` [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI Magnus Damm
2014-02-12 8:19 ` Geert Uytterhoeven
2014-02-12 8:37 ` Magnus Damm
@ 2014-02-12 8:48 ` Geert Uytterhoeven
2014-02-12 8:51 ` Magnus Damm
3 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2014-02-12 8:48 UTC (permalink / raw)
To: linux-sh
On Wed, Feb 12, 2014 at 9:37 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> On Wed, Feb 12, 2014 at 8:41 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>> 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?
>
> 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 -- 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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI
2014-02-12 7:41 ` [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI Magnus Damm
` (2 preceding siblings ...)
2014-02-12 8:48 ` Geert Uytterhoeven
@ 2014-02-12 8:51 ` Magnus Damm
2014-02-12 9:27 ` Geert Uytterhoeven
3 siblings, 1 reply; 7+ messages in thread
From: Magnus Damm @ 2014-02-12 8:51 UTC (permalink / raw)
To: linux-sh
On Wed, Feb 12, 2014 at 5:48 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Wed, Feb 12, 2014 at 9:37 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>> On Wed, Feb 12, 2014 at 8:41 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>>> 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?
>>
>> 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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 01/02] ARM: shmobile: Add SDHI devices to r8a7791 DTSI
2014-02-12 8:51 ` Magnus Damm
@ 2014-02-12 9:27 ` Geert Uytterhoeven
0 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2014-02-12 9:27 UTC (permalink / raw)
To: Magnus Damm
Cc: Linux-sh list, Simon Horman, Kuninori Morimoto,
devicetree@vger.kernel.org
On Wed, Feb 12, 2014 at 9:51 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>>>> - add r8a7790 suffix as fall back in case r8a7791 is missing from the driver
>>>>> + 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.
>>
>> 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 -- 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
^ permalink raw reply [flat|nested] 7+ messages in thread