| Message ID | 1372970334-21485-6-git-send-email-g.liakhovetski@gmx.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Hi Guennadi, On Fri, Jul 5, 2013 at 5:38 AM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > This adds DT templates for all MMCIF and SDHI controllers on r8a73a4. > They are added with status="disabled". To use them platform-specific > DTs have to enable the required ones. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> > --- > arch/arm/boot/dts/r8a73a4.dtsi | 45 ++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 45 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/boot/dts/r8a73a4.dtsi b/arch/arm/boot/dts/r8a73a4.dtsi > index 4e1ddf0..064f045 100644 > --- a/arch/arm/boot/dts/r8a73a4.dtsi > +++ b/arch/arm/boot/dts/r8a73a4.dtsi > @@ -166,4 +166,49 @@ > interrupt-parent = <&gic>; > interrupts = <0 173 0x4>; > }; > + > + mmcif0: mmcif@ee200000 { > + compatible = "renesas,sh-mmcif"; > + reg = <0 0xee200000 0 0x100>; > + interrupt-parent = <&gic>; > + interrupts = <0 169 0x4>; > + reg-io-width = <4>; > + status = "disabled"; > + }; > + > + mmcif1: mmcif@ee220000 { > + compatible = "renesas,sh-mmcif"; > + reg = <0 0xee220000 0 0x100>; > + interrupt-parent = <&gic>; > + interrupts = <0 170 0x4>; > + reg-io-width = <4>; > + status = "disabled"; > + }; I don't think the size of the MMCIF I/O memory resource is correct, please check with the data sheet and fix. > + sdhi0: sdhi@ee100000 { > + compatible = "renesas,r8a7740-sdhi"; > + reg = <0 0xee100000 0 0x100>; > + interrupt-parent = <&gic>; > + interrupts = <0 165 4>; > + cap-sd-highspeed; > + status = "disabled"; > + }; > + > + sdhi1: sdhi@ee120000 { > + compatible = "renesas,r8a7740-sdhi"; > + reg = <0 0xee120000 0 0x100>; > + interrupt-parent = <&gic>; > + interrupts = <0 166 4>; > + cap-sd-highspeed; > + status = "disabled"; > + }; > + > + sdhi2: sdhi@ee140000 { > + compatible = "renesas,r8a7740-sdhi"; > + reg = <0 0xee140000 0 0x100>; > + interrupt-parent = <&gic>; > + interrupts = <0 167 4>; > + cap-sd-highspeed; > + status = "disabled"; > + }; And now we're here again using "r8a7740" for SDHI on r8a73a4. I don't think you have full documentation for the SDHI block, do you? So please don't assume it is compatible with r8a7740 just because it happens to work for you. Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 5 Jul 2013, Magnus Damm wrote: > Hi Guennadi, > > On Fri, Jul 5, 2013 at 5:38 AM, Guennadi Liakhovetski > <g.liakhovetski@gmx.de> wrote: [snip] > > + sdhi0: sdhi@ee100000 { > > + compatible = "renesas,r8a7740-sdhi"; > > + reg = <0 0xee100000 0 0x100>; > > + interrupt-parent = <&gic>; > > + interrupts = <0 165 4>; > > + cap-sd-highspeed; > > + status = "disabled"; > > + }; > > + > > + sdhi1: sdhi@ee120000 { > > + compatible = "renesas,r8a7740-sdhi"; > > + reg = <0 0xee120000 0 0x100>; > > + interrupt-parent = <&gic>; > > + interrupts = <0 166 4>; > > + cap-sd-highspeed; > > + status = "disabled"; > > + }; > > + > > + sdhi2: sdhi@ee140000 { > > + compatible = "renesas,r8a7740-sdhi"; > > + reg = <0 0xee140000 0 0x100>; > > + interrupt-parent = <&gic>; > > + interrupts = <0 167 4>; > > + cap-sd-highspeed; > > + status = "disabled"; > > + }; > > And now we're here again using "r8a7740" for SDHI on r8a73a4. Would this be ok: compatible = "renesas,r8a73a4-sdhi", "renesas,r8a7740-sdhi"; Thanks Guennadi > I don't think you have full documentation for the SDHI block, do you? > So please don't assume it is compatible with r8a7740 just because it > happens to work for you. --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guennadi, On Fri, Jul 5, 2013 at 5:41 PM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > On Fri, 5 Jul 2013, Magnus Damm wrote: > >> Hi Guennadi, >> >> On Fri, Jul 5, 2013 at 5:38 AM, Guennadi Liakhovetski >> <g.liakhovetski@gmx.de> wrote: > > [snip] > >> > + sdhi0: sdhi@ee100000 { >> > + compatible = "renesas,r8a7740-sdhi"; >> > + reg = <0 0xee100000 0 0x100>; >> > + interrupt-parent = <&gic>; >> > + interrupts = <0 165 4>; >> > + cap-sd-highspeed; >> > + status = "disabled"; >> > + }; >> > + >> > + sdhi1: sdhi@ee120000 { >> > + compatible = "renesas,r8a7740-sdhi"; >> > + reg = <0 0xee120000 0 0x100>; >> > + interrupt-parent = <&gic>; >> > + interrupts = <0 166 4>; >> > + cap-sd-highspeed; >> > + status = "disabled"; >> > + }; >> > + >> > + sdhi2: sdhi@ee140000 { >> > + compatible = "renesas,r8a7740-sdhi"; >> > + reg = <0 0xee140000 0 0x100>; >> > + interrupt-parent = <&gic>; >> > + interrupts = <0 167 4>; >> > + cap-sd-highspeed; >> > + status = "disabled"; >> > + }; >> >> And now we're here again using "r8a7740" for SDHI on r8a73a4. > > Would this be ok: > > compatible = "renesas,r8a73a4-sdhi", "renesas,r8a7740-sdhi"; At least it is better. Thanks. I prefer "renesas,r8a73a4-sdhi" only, but the above is acceptable to me as long as we remember to add "renesas,r8a73a4-sdhi" to the SDHI driver that keeps old behaviour before updating "renesas,r8a7740-sdhi" with features that may break r8a73a4. Maybe we should simply add per-soc bindings to the driver to avoid future potential issues? Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Magnus On Mon, 8 Jul 2013, Magnus Damm wrote: > Hi Guennadi, > > On Fri, Jul 5, 2013 at 5:41 PM, Guennadi Liakhovetski > <g.liakhovetski@gmx.de> wrote: > > On Fri, 5 Jul 2013, Magnus Damm wrote: > > > >> Hi Guennadi, > >> > >> On Fri, Jul 5, 2013 at 5:38 AM, Guennadi Liakhovetski > >> <g.liakhovetski@gmx.de> wrote: > > > > [snip] > > > >> > + sdhi0: sdhi@ee100000 { > >> > + compatible = "renesas,r8a7740-sdhi"; > >> > + reg = <0 0xee100000 0 0x100>; > >> > + interrupt-parent = <&gic>; > >> > + interrupts = <0 165 4>; > >> > + cap-sd-highspeed; > >> > + status = "disabled"; > >> > + }; > >> > + > >> > + sdhi1: sdhi@ee120000 { > >> > + compatible = "renesas,r8a7740-sdhi"; > >> > + reg = <0 0xee120000 0 0x100>; > >> > + interrupt-parent = <&gic>; > >> > + interrupts = <0 166 4>; > >> > + cap-sd-highspeed; > >> > + status = "disabled"; > >> > + }; > >> > + > >> > + sdhi2: sdhi@ee140000 { > >> > + compatible = "renesas,r8a7740-sdhi"; > >> > + reg = <0 0xee140000 0 0x100>; > >> > + interrupt-parent = <&gic>; > >> > + interrupts = <0 167 4>; > >> > + cap-sd-highspeed; > >> > + status = "disabled"; > >> > + }; > >> > >> And now we're here again using "r8a7740" for SDHI on r8a73a4. > > > > Would this be ok: > > > > compatible = "renesas,r8a73a4-sdhi", "renesas,r8a7740-sdhi"; > > At least it is better. Thanks. > > I prefer "renesas,r8a73a4-sdhi" only, but the above is acceptable to > me as long as we remember to add "renesas,r8a73a4-sdhi" to the SDHI > driver that keeps old behaviour before updating "renesas,r8a7740-sdhi" > with features that may break r8a73a4. I don't think that would make sense. The whole purpose of adding the compatible string to DT was to avoid touching the driver _unless_ needed. I.e. unless we do know SDHI on the new SoC has a feature that we want to support and we know, that all DTs out there contain the required compatibility string, so, now we can just hook to it in the driver and all platforms will start using that feature magically. If you anyway want to add _every_ new SoC compatibility to _every_ related driver, I don't think using two compatibility strings would make sense. And no, I don't think "omap2," "omap3," "omap4" strings in OMAP drivers is a good example - as you know, those are SoC classes, not specific SoC versions. Using "generation 5," "generation 6" etc., or whatever you would prefer to call them, might be acceptable, but I'd really prefer to avoid adding every SoC to every driver. Another option would certainly be to bite the bullet and admit, that going the compatibility string way was a mistake and add a binding for the SDHI wait-idle feature and keep both in the driver. That way we add redundancy to the driver - at least until we decide to finally remove the compatibility string. But at least we can transfer all mainline platforms to use the binding parameter. > Maybe we should simply add per-soc bindings to the driver to avoid > future potential issues? A real binding - not a compatibility string? No, don't think I like this either. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guenandi, On Mon, Jul 8, 2013 at 4:06 PM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > Hi Magnus > > On Mon, 8 Jul 2013, Magnus Damm wrote: > >> Hi Guennadi, >> >> On Fri, Jul 5, 2013 at 5:41 PM, Guennadi Liakhovetski >> <g.liakhovetski@gmx.de> wrote: >> > On Fri, 5 Jul 2013, Magnus Damm wrote: >> > >> >> Hi Guennadi, >> >> >> >> On Fri, Jul 5, 2013 at 5:38 AM, Guennadi Liakhovetski >> >> <g.liakhovetski@gmx.de> wrote: >> > >> > [snip] >> > >> >> > + sdhi0: sdhi@ee100000 { >> >> > + compatible = "renesas,r8a7740-sdhi"; >> >> > + reg = <0 0xee100000 0 0x100>; >> >> > + interrupt-parent = <&gic>; >> >> > + interrupts = <0 165 4>; >> >> > + cap-sd-highspeed; >> >> > + status = "disabled"; >> >> > + }; >> >> > + >> >> > + sdhi1: sdhi@ee120000 { >> >> > + compatible = "renesas,r8a7740-sdhi"; >> >> > + reg = <0 0xee120000 0 0x100>; >> >> > + interrupt-parent = <&gic>; >> >> > + interrupts = <0 166 4>; >> >> > + cap-sd-highspeed; >> >> > + status = "disabled"; >> >> > + }; >> >> > + >> >> > + sdhi2: sdhi@ee140000 { >> >> > + compatible = "renesas,r8a7740-sdhi"; >> >> > + reg = <0 0xee140000 0 0x100>; >> >> > + interrupt-parent = <&gic>; >> >> > + interrupts = <0 167 4>; >> >> > + cap-sd-highspeed; >> >> > + status = "disabled"; >> >> > + }; >> >> >> >> And now we're here again using "r8a7740" for SDHI on r8a73a4. >> > >> > Would this be ok: >> > >> > compatible = "renesas,r8a73a4-sdhi", "renesas,r8a7740-sdhi"; >> >> At least it is better. Thanks. >> >> I prefer "renesas,r8a73a4-sdhi" only, but the above is acceptable to >> me as long as we remember to add "renesas,r8a73a4-sdhi" to the SDHI >> driver that keeps old behaviour before updating "renesas,r8a7740-sdhi" >> with features that may break r8a73a4. > > I don't think that would make sense. The whole purpose of adding the > compatible string to DT was to avoid touching the driver _unless_ needed. So instead of using DT to describe hardware we should use it to cut slack from a software implementation point of view? > I.e. unless we do know SDHI on the new SoC has a feature that we want to > support and we know, that all DTs out there contain the required > compatibility string, so, now we can just hook to it in the driver and all > platforms will start using that feature magically. If you anyway want to > add _every_ new SoC compatibility to _every_ related driver, I don't think > using two compatibility strings would make sense. I think you know what I think, and that is using a per-SoC compatibility string is stupid since it has nothing to do with SoC. But if we should go down that route, why don't we do it properly at least? I would like to know how you think the following case should be handled: 1. On r8a73a4 without documentation you introduce the following line in the DTS compatible = "renesas,r8a73a4-sdhi", "renesas,r8a7740-sdhi"; 2. After this you happen to receive documentation about r8a7740 SDHI. 3. You update the SDHI driver with r8a7740 specific new features. 4. r8a73a4 now breaks unless you remember to also add "r8a73a4-sdhi" This turns into a dependency mess IMO. > And no, I don't think "omap2," "omap3," "omap4" strings in OMAP drivers is > a good example - as you know, those are SoC classes, not specific SoC > versions. Using "generation 5," "generation 6" etc., or whatever you would > prefer to call them, might be acceptable, but I'd really prefer to avoid > adding every SoC to every driver. Yes, me too. So how the hell did the bindings turn into this state? > Another option would certainly be to bite the bullet and admit, that going > the compatibility string way was a mistake and add a binding for the SDHI > wait-idle feature and keep both in the driver. That way we add redundancy > to the driver - at least until we decide to finally remove the > compatibility string. But at least we can transfer all mainline platforms > to use the binding parameter. Now since we for some unknown reason started using SoC in the compatibility string I think we have to bite the bullet and keep on updating the SoCs with proper information. To avoid the mess listed in 1 + 2 +3 + 4 above. >> Maybe we should simply add per-soc bindings to the driver to avoid >> future potential issues? > > A real binding - not a compatibility string? No, don't think I like this > either. So how do you propose that we handle this then? We have already committed to per-SoC compatibility strings... / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Magnus On Mon, 8 Jul 2013, Magnus Damm wrote: > Hi Guenandi, > > On Mon, Jul 8, 2013 at 4:06 PM, Guennadi Liakhovetski > <g.liakhovetski@gmx.de> wrote: > > Hi Magnus > > > > On Mon, 8 Jul 2013, Magnus Damm wrote: > > > >> Hi Guennadi, > >> > >> On Fri, Jul 5, 2013 at 5:41 PM, Guennadi Liakhovetski > >> <g.liakhovetski@gmx.de> wrote: > >> > On Fri, 5 Jul 2013, Magnus Damm wrote: > >> > > >> >> Hi Guennadi, > >> >> > >> >> On Fri, Jul 5, 2013 at 5:38 AM, Guennadi Liakhovetski > >> >> <g.liakhovetski@gmx.de> wrote: > >> > > >> > [snip] > >> > > >> >> > + sdhi0: sdhi@ee100000 { > >> >> > + compatible = "renesas,r8a7740-sdhi"; > >> >> > + reg = <0 0xee100000 0 0x100>; > >> >> > + interrupt-parent = <&gic>; > >> >> > + interrupts = <0 165 4>; > >> >> > + cap-sd-highspeed; > >> >> > + status = "disabled"; > >> >> > + }; > >> >> > + > >> >> > + sdhi1: sdhi@ee120000 { > >> >> > + compatible = "renesas,r8a7740-sdhi"; > >> >> > + reg = <0 0xee120000 0 0x100>; > >> >> > + interrupt-parent = <&gic>; > >> >> > + interrupts = <0 166 4>; > >> >> > + cap-sd-highspeed; > >> >> > + status = "disabled"; > >> >> > + }; > >> >> > + > >> >> > + sdhi2: sdhi@ee140000 { > >> >> > + compatible = "renesas,r8a7740-sdhi"; > >> >> > + reg = <0 0xee140000 0 0x100>; > >> >> > + interrupt-parent = <&gic>; > >> >> > + interrupts = <0 167 4>; > >> >> > + cap-sd-highspeed; > >> >> > + status = "disabled"; > >> >> > + }; > >> >> > >> >> And now we're here again using "r8a7740" for SDHI on r8a73a4. > >> > > >> > Would this be ok: > >> > > >> > compatible = "renesas,r8a73a4-sdhi", "renesas,r8a7740-sdhi"; > >> > >> At least it is better. Thanks. > >> > >> I prefer "renesas,r8a73a4-sdhi" only, but the above is acceptable to > >> me as long as we remember to add "renesas,r8a73a4-sdhi" to the SDHI > >> driver that keeps old behaviour before updating "renesas,r8a7740-sdhi" > >> with features that may break r8a73a4. > > > > I don't think that would make sense. The whole purpose of adding the > > compatible string to DT was to avoid touching the driver _unless_ needed. > > So instead of using DT to describe hardware we should use it to cut > slack from a software implementation point of view? Sorry, don't understand how this is cutting slack. We do use that string, so, DT does describe the hardware. Only I don't understand why we shall unconditionally add these strings to the drivers, even when no action should be taken there? > > I.e. unless we do know SDHI on the new SoC has a feature that we want to > > support and we know, that all DTs out there contain the required > > compatibility string, so, now we can just hook to it in the driver and all > > platforms will start using that feature magically. If you anyway want to > > add _every_ new SoC compatibility to _every_ related driver, I don't think > > using two compatibility strings would make sense. > > I think you know what I think, and that is using a per-SoC > compatibility string is stupid since it has nothing to do with SoC. > But if we should go down that route, why don't we do it properly at > least? > > I would like to know how you think the following case should be handled: > > 1. On r8a73a4 without documentation you introduce the following line in the DTS > compatible = "renesas,r8a73a4-sdhi", "renesas,r8a7740-sdhi"; > > 2. After this you happen to receive documentation about r8a7740 SDHI. > > 3. You update the SDHI driver with r8a7740 specific new features. > > 4. r8a73a4 now breaks unless you remember to also add "r8a73a4-sdhi" Again - how is this different from your proposal? Let's say these SoCs have a common feature A (idle-wait) and then you discover a new feature B on r8a7740 only. With your proposal we have an entry for r8a73a4 in the driver, which does nothing. Now, after updating the driver to handle B on r8a7740 you also have to remember to extend your r8a73a4 handling to disable B there. So, it is just exactly the same, the only difference is, with my approach you first add that r8a73a4 compatibility entry when you discover the need, with your approach you already have it there in the driver as an empty entry, and you anyway have to update it to disable B, when it is discovered. Thanks Guennadi > This turns into a dependency mess IMO. > > > And no, I don't think "omap2," "omap3," "omap4" strings in OMAP drivers is > > a good example - as you know, those are SoC classes, not specific SoC > > versions. Using "generation 5," "generation 6" etc., or whatever you would > > prefer to call them, might be acceptable, but I'd really prefer to avoid > > adding every SoC to every driver. > > Yes, me too. So how the hell did the bindings turn into this state? > > > Another option would certainly be to bite the bullet and admit, that going > > the compatibility string way was a mistake and add a binding for the SDHI > > wait-idle feature and keep both in the driver. That way we add redundancy > > to the driver - at least until we decide to finally remove the > > compatibility string. But at least we can transfer all mainline platforms > > to use the binding parameter. > > Now since we for some unknown reason started using SoC in the > compatibility string I think we have to bite the bullet and keep on > updating the SoCs with proper information. To avoid the mess listed in > 1 + 2 +3 + 4 above. > > >> Maybe we should simply add per-soc bindings to the driver to avoid > >> future potential issues? > > > > A real binding - not a compatibility string? No, don't think I like this > > either. > > So how do you propose that we handle this then? We have already > committed to per-SoC compatibility strings... > > / magnus > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guennadi, On Mon, Jul 8, 2013 at 5:57 PM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > Hi Magnus > > On Mon, 8 Jul 2013, Magnus Damm wrote: > >> Hi Guenandi, >> >> On Mon, Jul 8, 2013 at 4:06 PM, Guennadi Liakhovetski >> <g.liakhovetski@gmx.de> wrote: >> > Hi Magnus >> > >> > On Mon, 8 Jul 2013, Magnus Damm wrote: >> > >> >> Hi Guennadi, >> >> >> >> On Fri, Jul 5, 2013 at 5:41 PM, Guennadi Liakhovetski >> >> <g.liakhovetski@gmx.de> wrote: >> >> > On Fri, 5 Jul 2013, Magnus Damm wrote: >> >> > >> >> >> Hi Guennadi, >> >> >> >> >> >> On Fri, Jul 5, 2013 at 5:38 AM, Guennadi Liakhovetski >> >> >> <g.liakhovetski@gmx.de> wrote: >> >> > >> >> > [snip] >> >> > >> >> >> > + sdhi0: sdhi@ee100000 { >> >> >> > + compatible = "renesas,r8a7740-sdhi"; >> >> >> > + reg = <0 0xee100000 0 0x100>; >> >> >> > + interrupt-parent = <&gic>; >> >> >> > + interrupts = <0 165 4>; >> >> >> > + cap-sd-highspeed; >> >> >> > + status = "disabled"; >> >> >> > + }; >> >> >> > + >> >> >> > + sdhi1: sdhi@ee120000 { >> >> >> > + compatible = "renesas,r8a7740-sdhi"; >> >> >> > + reg = <0 0xee120000 0 0x100>; >> >> >> > + interrupt-parent = <&gic>; >> >> >> > + interrupts = <0 166 4>; >> >> >> > + cap-sd-highspeed; >> >> >> > + status = "disabled"; >> >> >> > + }; >> >> >> > + >> >> >> > + sdhi2: sdhi@ee140000 { >> >> >> > + compatible = "renesas,r8a7740-sdhi"; >> >> >> > + reg = <0 0xee140000 0 0x100>; >> >> >> > + interrupt-parent = <&gic>; >> >> >> > + interrupts = <0 167 4>; >> >> >> > + cap-sd-highspeed; >> >> >> > + status = "disabled"; >> >> >> > + }; >> >> >> >> >> >> And now we're here again using "r8a7740" for SDHI on r8a73a4. >> >> > >> >> > Would this be ok: >> >> > >> >> > compatible = "renesas,r8a73a4-sdhi", "renesas,r8a7740-sdhi"; >> >> >> >> At least it is better. Thanks. >> >> >> >> I prefer "renesas,r8a73a4-sdhi" only, but the above is acceptable to >> >> me as long as we remember to add "renesas,r8a73a4-sdhi" to the SDHI >> >> driver that keeps old behaviour before updating "renesas,r8a7740-sdhi" >> >> with features that may break r8a73a4. >> > >> > I don't think that would make sense. The whole purpose of adding the >> > compatible string to DT was to avoid touching the driver _unless_ needed. >> >> So instead of using DT to describe hardware we should use it to cut >> slack from a software implementation point of view? > > Sorry, don't understand how this is cutting slack. We do use that string, > so, DT does describe the hardware. Only I don't understand why we shall > unconditionally add these strings to the drivers, even when no action > should be taken there? Well, at the time of writing the DTS, do you know if action is needed or not? So using r8a7740 looks to me like it's an assumption about how r8a7740 software support happen to be at some certain point in time. It is not guaranteed that r8a7740 compatibility would be kept in the future, basically because we don't know how r8a7740 support is going to evolve. So it's basically describing a certain software state IMO. I want it to describe hardware instead. I would be fine it the string was like this UART example: compatible = "vendor,16550", "vendor,8250"; In the above example with UART we know that 16550 is backwards compatible with 8250. So if the 16550 driver happens to be disabled in the kernel then it should be possible to use the 8250 driver. In this case the register layout and the documentation should point out that they are compatible. As a contrast, I believe the following are true for SDHI - We have no decent documentation - neither for r8a7740 or r8a73a4 - r8a73a4 is most likely not in a parent-child relation ship with r8a7740 So I prefer not to make any assumptions to avoid future headache. >> > I.e. unless we do know SDHI on the new SoC has a feature that we want to >> > support and we know, that all DTs out there contain the required >> > compatibility string, so, now we can just hook to it in the driver and all >> > platforms will start using that feature magically. If you anyway want to >> > add _every_ new SoC compatibility to _every_ related driver, I don't think >> > using two compatibility strings would make sense. >> >> I think you know what I think, and that is using a per-SoC >> compatibility string is stupid since it has nothing to do with SoC. >> But if we should go down that route, why don't we do it properly at >> least? >> >> I would like to know how you think the following case should be handled: >> >> 1. On r8a73a4 without documentation you introduce the following line in the DTS >> compatible = "renesas,r8a73a4-sdhi", "renesas,r8a7740-sdhi"; >> >> 2. After this you happen to receive documentation about r8a7740 SDHI. >> >> 3. You update the SDHI driver with r8a7740 specific new features. >> >> 4. r8a73a4 now breaks unless you remember to also add "r8a73a4-sdhi" > > Again - how is this different from your proposal? Let's say these SoCs > have a common feature A (idle-wait) and then you discover a new feature B > on r8a7740 only. With your proposal we have an entry for r8a73a4 in the > driver, which does nothing. Now, after updating the driver to handle B on > r8a7740 you also have to remember to extend your r8a73a4 handling to > disable B there. So, it is just exactly the same, the only difference is, > with my approach you first add that r8a73a4 compatibility entry when you > discover the need, with your approach you already have it there in the > driver as an empty entry, and you anyway have to update it to disable B, > when it is discovered. I'm not sure what you mean with"my proposal", however.. Above I don't understand why you have to extend r8a73a4 handling to disable B. From my point of view, when you update r8a7740 with feature B you do it with a feature flag that only applies to r8a7740. When updating r8a7740 you leave that particular feature flag not set on r8a73a4. So r8a73a4 does not have to be touched. / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/boot/dts/r8a73a4.dtsi b/arch/arm/boot/dts/r8a73a4.dtsi index 4e1ddf0..064f045 100644 --- a/arch/arm/boot/dts/r8a73a4.dtsi +++ b/arch/arm/boot/dts/r8a73a4.dtsi @@ -166,4 +166,49 @@ interrupt-parent = <&gic>; interrupts = <0 173 0x4>; }; + + mmcif0: mmcif@ee200000 { + compatible = "renesas,sh-mmcif"; + reg = <0 0xee200000 0 0x100>; + interrupt-parent = <&gic>; + interrupts = <0 169 0x4>; + reg-io-width = <4>; + status = "disabled"; + }; + + mmcif1: mmcif@ee220000 { + compatible = "renesas,sh-mmcif"; + reg = <0 0xee220000 0 0x100>; + interrupt-parent = <&gic>; + interrupts = <0 170 0x4>; + reg-io-width = <4>; + status = "disabled"; + }; + + sdhi0: sdhi@ee100000 { + compatible = "renesas,r8a7740-sdhi"; + reg = <0 0xee100000 0 0x100>; + interrupt-parent = <&gic>; + interrupts = <0 165 4>; + cap-sd-highspeed; + status = "disabled"; + }; + + sdhi1: sdhi@ee120000 { + compatible = "renesas,r8a7740-sdhi"; + reg = <0 0xee120000 0 0x100>; + interrupt-parent = <&gic>; + interrupts = <0 166 4>; + cap-sd-highspeed; + status = "disabled"; + }; + + sdhi2: sdhi@ee140000 { + compatible = "renesas,r8a7740-sdhi"; + reg = <0 0xee140000 0 0x100>; + interrupt-parent = <&gic>; + interrupts = <0 167 4>; + cap-sd-highspeed; + status = "disabled"; + }; };
This adds DT templates for all MMCIF and SDHI controllers on r8a73a4. They are added with status="disabled". To use them platform-specific DTs have to enable the required ones. Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> --- arch/arm/boot/dts/r8a73a4.dtsi | 45 ++++++++++++++++++++++++++++++++++++++++ 1 files changed, 45 insertions(+), 0 deletions(-)