diff mbox

[6/6] ARM: shmobile: r8a7790: add MMCIF and SDHI DT templates

Message ID 1372970334-21485-7-git-send-email-g.liakhovetski@gmx.de (mailing list archive)
State Superseded
Headers show

Commit Message

Guennadi Liakhovetski July 4, 2013, 8:38 p.m. UTC
This adds DT templates for all MMCIF and SDHI controllers on r8a7790.
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/r8a7790.dtsi |   56 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 56 insertions(+), 0 deletions(-)

Comments

Magnus Damm July 5, 2013, 5:55 a.m. UTC | #1
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 r8a7790.
> 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/r8a7790.dtsi |   56 ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 56 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index 339d9b1..18d818f 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -54,4 +54,60 @@
>                 interrupt-parent = <&gic>;
>                 interrupts = <0 0 4>, <0 1 4>, <0 2 4>, <0 3 4>;
>         };
> +
> +       /* No MMC_CAP_UHS_DDR50 (dual data rate) capability on r8a7790! */
> +       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";
> +       };

Can please you tell me which kernel configuration i need to enable to
make the comment above affect the MMCIF driver so DDR50 support is
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";
> +       };

The I/O mem resource size of MMCIF0 and MMCIF1 seems wrong here too.

> +       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";
> +       };
> +
> +       /* What are SDHI2 C2 and SDHI3 C2 controllers? */

42?

I think you can drop this comment.

> +       sdhi2: sdhi@ee140000 {
> +               compatible = "renesas,r8a7740-sdhi";
> +               reg = <0 0xee140000 0 0x100>;
> +               interrupt-parent = <&gic>;
> +               interrupts = <0 167 4>;
> +               cap-sd-highspeed;
> +               status = "disabled";
> +       };
> +
> +       sdhi3: sdhi@ee160000 {
> +               compatible = "renesas,r8a7740-sdhi";
> +               reg = <0 0xee160000 0 0x100>;
> +               interrupt-parent = <&gic>;
> +               interrupts = <0 168 4>;
> +               cap-sd-highspeed;
> +               status = "disabled";
> +       };

Same thing as r8a73a4 here wrt to r8a7740.

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
Guennadi Liakhovetski July 5, 2013, 9:01 a.m. UTC | #2
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:
> > This adds DT templates for all MMCIF and SDHI controllers on r8a7790.
> > 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/r8a7790.dtsi |   56 ++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 56 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> > index 339d9b1..18d818f 100644
> > --- a/arch/arm/boot/dts/r8a7790.dtsi
> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
> > @@ -54,4 +54,60 @@
> >                 interrupt-parent = <&gic>;
> >                 interrupts = <0 0 4>, <0 1 4>, <0 2 4>, <0 3 4>;
> >         };
> > +
> > +       /* No MMC_CAP_UHS_DDR50 (dual data rate) capability on r8a7790! */
> > +       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";
> > +       };
> 
> Can please you tell me which kernel configuration i need to enable to
> make the comment above affect the MMCIF driver so DDR50 support is
> disabled?

The above comment means, that r8a7790 platforms shouldn't set 
MMC_CAP_UHS_DDR50 capability for MMCIF interfaces. In the platform data 
this would be done by oring it to struct sh_mmcif_plat_data::caps. 
Currently there is no way to set this capability in DT, so, that comment 
doesn't have any effect here. But this possibility will most likely be 
added to DT at some point. So, this comment is here as a reminder. But we 
can remove it too, if you prefer.

Thanks
Guennadi

> 
> > +       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";
> > +       };
> 
> The I/O mem resource size of MMCIF0 and MMCIF1 seems wrong here too.
> 
> > +       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";
> > +       };
> > +
> > +       /* What are SDHI2 C2 and SDHI3 C2 controllers? */
> 
> 42?
> 
> I think you can drop this comment.
> 
> > +       sdhi2: sdhi@ee140000 {
> > +               compatible = "renesas,r8a7740-sdhi";
> > +               reg = <0 0xee140000 0 0x100>;
> > +               interrupt-parent = <&gic>;
> > +               interrupts = <0 167 4>;
> > +               cap-sd-highspeed;
> > +               status = "disabled";
> > +       };
> > +
> > +       sdhi3: sdhi@ee160000 {
> > +               compatible = "renesas,r8a7740-sdhi";
> > +               reg = <0 0xee160000 0 0x100>;
> > +               interrupt-parent = <&gic>;
> > +               interrupts = <0 168 4>;
> > +               cap-sd-highspeed;
> > +               status = "disabled";
> > +       };
> 
> Same thing as r8a73a4 here wrt to r8a7740.
> 
> Thanks,
> 
> / 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
Magnus Damm July 5, 2013, 9:19 a.m. UTC | #3
On Fri, Jul 5, 2013 at 6:01 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:
>> > This adds DT templates for all MMCIF and SDHI controllers on r8a7790.
>> > 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/r8a7790.dtsi |   56 ++++++++++++++++++++++++++++++++++++++++
>> >  1 files changed, 56 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
>> > index 339d9b1..18d818f 100644
>> > --- a/arch/arm/boot/dts/r8a7790.dtsi
>> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
>> > @@ -54,4 +54,60 @@
>> >                 interrupt-parent = <&gic>;
>> >                 interrupts = <0 0 4>, <0 1 4>, <0 2 4>, <0 3 4>;
>> >         };
>> > +
>> > +       /* No MMC_CAP_UHS_DDR50 (dual data rate) capability on r8a7790! */
>> > +       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";
>> > +       };
>>
>> Can please you tell me which kernel configuration i need to enable to
>> make the comment above affect the MMCIF driver so DDR50 support is
>> disabled?
>
> The above comment means, that r8a7790 platforms shouldn't set
> MMC_CAP_UHS_DDR50 capability for MMCIF interfaces. In the platform data
> this would be done by oring it to struct sh_mmcif_plat_data::caps.
> Currently there is no way to set this capability in DT, so, that comment
> doesn't have any effect here. But this possibility will most likely be
> added to DT at some point. So, this comment is here as a reminder. But we
> can remove it too, if you prefer.

If I understand you correctly then in case of DT this capability isn't
available at this time.

Obviously we need to add support for these features also in the case
of DT. We would not want to have the DT case to be left
half-implemented. If we would leave it half implemented then we could
never move from C to DT for board support.

What is your proposed plan forward to implement this DT capability? Or
are patches queued up already?

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
Guennadi Liakhovetski July 5, 2013, 10 a.m. UTC | #4
On Fri, 5 Jul 2013, Magnus Damm wrote:

> On Fri, Jul 5, 2013 at 6:01 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:
> >> > This adds DT templates for all MMCIF and SDHI controllers on r8a7790.
> >> > 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/r8a7790.dtsi |   56 ++++++++++++++++++++++++++++++++++++++++
> >> >  1 files changed, 56 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> >> > index 339d9b1..18d818f 100644
> >> > --- a/arch/arm/boot/dts/r8a7790.dtsi
> >> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
> >> > @@ -54,4 +54,60 @@
> >> >                 interrupt-parent = <&gic>;
> >> >                 interrupts = <0 0 4>, <0 1 4>, <0 2 4>, <0 3 4>;
> >> >         };
> >> > +
> >> > +       /* No MMC_CAP_UHS_DDR50 (dual data rate) capability on r8a7790! */
> >> > +       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";
> >> > +       };
> >>
> >> Can please you tell me which kernel configuration i need to enable to
> >> make the comment above affect the MMCIF driver so DDR50 support is
> >> disabled?
> >
> > The above comment means, that r8a7790 platforms shouldn't set
> > MMC_CAP_UHS_DDR50 capability for MMCIF interfaces. In the platform data
> > this would be done by oring it to struct sh_mmcif_plat_data::caps.
> > Currently there is no way to set this capability in DT, so, that comment
> > doesn't have any effect here. But this possibility will most likely be
> > added to DT at some point. So, this comment is here as a reminder. But we
> > can remove it too, if you prefer.
> 
> If I understand you correctly then in case of DT this capability isn't
> available at this time.

Yes, you understand correctly.

> Obviously we need to add support for these features also in the case
> of DT. We would not want to have the DT case to be left
> half-implemented. If we would leave it half implemented then we could
> never move from C to DT for board support.

Sure, step by step in the order of priorities.

> What is your proposed plan forward to implement this DT capability? Or
> are patches queued up already?

I do quite a bit of development in the MMC area, but I don't have access 
to full MMC/SD standards, so, I'm not sure what that specific feature 
does. I heard opinions about those similar features being redundant and 
deducible from other interface parameters like clock rate etc. So, I can 
certainly propose a patch that simply sets that (and other) capability, 
when a certain property is found, but I think it'd be better if someone 
with a complete information would do that. I could throw in an RFC and see 
reaction and ask for advise, of course. But without information such an 
attempt would probably require a few rounds and thus would take some time.

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
diff mbox

Patch

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 339d9b1..18d818f 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -54,4 +54,60 @@ 
 		interrupt-parent = <&gic>;
 		interrupts = <0 0 4>, <0 1 4>, <0 2 4>,	<0 3 4>;
 	};
+
+	/* No MMC_CAP_UHS_DDR50 (dual data rate) capability on r8a7790! */
+	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";
+	};
+
+	/* What are SDHI2 C2 and SDHI3 C2 controllers? */
+	sdhi2: sdhi@ee140000 {
+		compatible = "renesas,r8a7740-sdhi";
+		reg = <0 0xee140000 0 0x100>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 167 4>;
+		cap-sd-highspeed;
+		status = "disabled";
+	};
+
+	sdhi3: sdhi@ee160000 {
+		compatible = "renesas,r8a7740-sdhi";
+		reg = <0 0xee160000 0 0x100>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 168 4>;
+		cap-sd-highspeed;
+		status = "disabled";
+	};
 };