[PATCH 1/4] ARM: shmobile: r8a7778: add usb phy power control function

[PATCH 1/4] ARM: shmobile: r8a7778: add usb phy power control function

[Kuninori Morimoto]

USB phy initialisation function is needed from not only
USB Host but also USB Function too.
This patch adds usb phy common control function.

Tested-by: Yusuke Goda <yusuke.goda.sx@renesas.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm/mach-shmobile/setup-r8a7778.c |   35 ++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-shmobile/setup-r8a7778.c b/arch/arm/mach-shmobile/setup-r8a7778.c
index 2a101eb..cd98079 100644
--- a/arch/arm/mach-shmobile/setup-r8a7778.c
+++ b/arch/arm/mach-shmobile/setup-r8a7778.c
@@ -109,29 +109,44 @@ void __init r8a7778_add_usb_phy_device(struct rcar_phy_platform_data *pdata)
 					  pdata, sizeof(*pdata));
 }
 
-/* USB */
-static struct usb_phy *phy;
-
-static int usb_power_on(struct platform_device *pdev)
+static int r8a7778_usb_phy_power(int enable)
 {
+	static struct usb_phy *phy = NULL;
+	int ret = 0;
+
+	if (!phy)
+		phy = usb_get_phy(USB_PHY_TYPE_USB2);
+
 	if (IS_ERR(phy))
 		return PTR_ERR(phy);
 
+	if (enable)
+		ret = usb_phy_init(phy);
+	else
+		usb_phy_shutdown(phy);
+
+	return ret;
+}
+
+/* USB */
+static int usb_power_on(struct platform_device *pdev)
+{
+	int ret = r8a7778_usb_phy_power(1);
+
+	if (ret)
+		return ret;
+
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
-	usb_phy_init(phy);
-
 	return 0;
 }
 
 static void usb_power_off(struct platform_device *pdev)
 {
-	if (IS_ERR(phy))
+	if (r8a7778_usb_phy_power(0))
 		return;
 
-	usb_phy_shutdown(phy);
-
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 }
@@ -359,8 +374,6 @@ void __init r8a7778_add_standard_devices(void)
 
 void __init r8a7778_init_late(void)
 {
-	phy = usb_get_phy(USB_PHY_TYPE_USB2);
-
 	platform_device_register_full(&ehci_info);
 	platform_device_register_full(&ohci_info);
 }
-- 
1.7.9.5

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add usb phy power control function

[Sergei Shtylyov]

Hello.

On 09-07-2013 9:52, Kuninori Morimoto wrote:

> USB phy initialisation function is needed from not only
> USB Host but also USB Function too.
> This patch adds usb phy common control function.

> Tested-by: Yusuke Goda <yusuke.goda.sx@renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>   arch/arm/mach-shmobile/setup-r8a7778.c |   35 ++++++++++++++++++++++----------
>   1 file changed, 24 insertions(+), 11 deletions(-)

> diff --git a/arch/arm/mach-shmobile/setup-r8a7778.c b/arch/arm/mach-shmobile/setup-r8a7778.c
> index 2a101eb..cd98079 100644
> --- a/arch/arm/mach-shmobile/setup-r8a7778.c
> +++ b/arch/arm/mach-shmobile/setup-r8a7778.c
> @@ -109,29 +109,44 @@ void __init r8a7778_add_usb_phy_device(struct rcar_phy_platform_data *pdata)
>   					  pdata, sizeof(*pdata));
>   }
>
> -/* USB */

    Why move this comment?

> -static struct usb_phy *phy;
> -
> -static int usb_power_on(struct platform_device *pdev)
> +static int r8a7778_usb_phy_power(int enable)

    Why not 'bool enable'?

WBR, Sergei

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add usb phy power control function

[Simon Horman]

On Tue, Jul 09, 2013 at 03:52:48PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 09-07-2013 9:52, Kuninori Morimoto wrote:
> 
> >USB phy initialisation function is needed from not only
> >USB Host but also USB Function too.
> >This patch adds usb phy common control function.
> 
> >Tested-by: Yusuke Goda <yusuke.goda.sx@renesas.com>
> >Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >---
> >  arch/arm/mach-shmobile/setup-r8a7778.c |   35 ++++++++++++++++++++++----------
> >  1 file changed, 24 insertions(+), 11 deletions(-)
> 
> >diff --git a/arch/arm/mach-shmobile/setup-r8a7778.c b/arch/arm/mach-shmobile/setup-r8a7778.c
> >index 2a101eb..cd98079 100644
> >--- a/arch/arm/mach-shmobile/setup-r8a7778.c
> >+++ b/arch/arm/mach-shmobile/setup-r8a7778.c
> >@@ -109,29 +109,44 @@ void __init r8a7778_add_usb_phy_device(struct rcar_phy_platform_data *pdata)
> >  					  pdata, sizeof(*pdata));
> >  }
> >
> >-/* USB */
> 
>    Why move this comment?
> 
> >-static struct usb_phy *phy;
> >-
> >-static int usb_power_on(struct platform_device *pdev)
> >+static int r8a7778_usb_phy_power(int enable)
> 
>    Why not 'bool enable'?

Morimoto-san, these requests seem reasonable to me.
Could you update your patch and repost with the rest of the series?

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add usb phy power control function

[Kuninori Morimoto]

Hi Simon

> >    Why not 'bool enable'?
> 
> Morimoto-san, these requests seem reasonable to me.
> Could you update your patch and repost with the rest of the series?

Sure.
I will send v2 patch soon.


Best regards
---
Kuninori Morimoto

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add usb phy power control function

[Kuninori Morimoto]

Hi

> > diff --git a/arch/arm/mach-shmobile/setup-r8a7778.c b/arch/arm/mach-shmobile/setup-r8a7778.c
> > index 2a101eb..cd98079 100644
> > --- a/arch/arm/mach-shmobile/setup-r8a7778.c
> > +++ b/arch/arm/mach-shmobile/setup-r8a7778.c
> > @@ -109,29 +109,44 @@ void __init r8a7778_add_usb_phy_device(struct rcar_phy_platform_data *pdata)
> >   					  pdata, sizeof(*pdata));
> >   }
> >
> > -/* USB */
> 
>     Why move this comment?

Current setup-r8a7778.c already has /* USB PHY */ above here

> > -static struct usb_phy *phy;
> > -
> > -static int usb_power_on(struct platform_device *pdev)
> > +static int r8a7778_usb_phy_power(int enable)
> 
>     Why not 'bool enable'?

will fix in v2


Best regards
---
Kuninori Morimoto

[PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Sergei Shtylyov]

Define the generic R8A777x part of the Ether device node. 

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
=================================--- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
+++ renesas/arch/arm/boot/dts/r8a7778.dtsi
@@ -98,4 +98,15 @@
 		reg = <0xfffc000 0x118>;
 		#gpio-range-cells = <3>;
 	};
+
+	ether: ethernet@fde00000 {
+		device_type = "network";
+		compatible = "renesas,ether-r8a7779";
+		reg = <0xfde00000 0x400>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 105 0x4>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 };

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Laurent Pinchart]

Hi Sergei,

Thank you for the patch.

On Sunday 01 September 2013 03:15:48 Sergei Shtylyov wrote:
> Define the generic R8A777x part of the Ether device node.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
>  arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
> =================================> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
> @@ -98,4 +98,15 @@
>  		reg = <0xfffc000 0x118>;
>  		#gpio-range-cells = <3>;
>  	};
> +
> +	ether: ethernet@fde00000 {
> +		device_type = "network";

Documentation/devicetree/bindings/net/sh_eth.txt from "[PATCH 2/2] sh_eth: add 
device tree support" doesn't document this property, should you update it ?

> +		compatible = "renesas,ether-r8a7779";
> +		reg = <0xfde00000 0x400>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 105 0x4>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		status = "disabled";
> +	};
>  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Sergei Shtylyov]

On 09/02/2013 05:14 PM, Laurent Pinchart wrote:

>> Define the generic R8A777x part of the Ether device node.

>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

>> ---
>>   arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
>>   1 file changed, 11 insertions(+)

>> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
>> =================================>> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
>> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
>> @@ -98,4 +98,15 @@
>>   		reg = <0xfffc000 0x118>;
>>   		#gpio-range-cells = <3>;
>>   	};
>> +
>> +	ether: ethernet@fde00000 {
>> +		device_type = "network";

> Documentation/devicetree/bindings/net/sh_eth.txt from "[PATCH 2/2] sh_eth: add
> device tree support" doesn't document this property, should you update it ?

    It's a standard property but completely optional for non-OF systems. I've 
not seen anybody document it but I should grep for it...

WBR, Sergei

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Laurent Pinchart]

On Monday 02 September 2013 18:05:10 Sergei Shtylyov wrote:
> On 09/02/2013 05:14 PM, Laurent Pinchart wrote:
> >> Define the generic R8A777x part of the Ether device node.
> >> 
> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >> 
> >> ---
> >> 
> >>   arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
> >>   1 file changed, 11 insertions(+)
> >> 
> >> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
> >> =================================> >> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
> >> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
> >> @@ -98,4 +98,15 @@
> >> 
> >>   		reg = <0xfffc000 0x118>;
> >>   		#gpio-range-cells = <3>;
> >>   	
> >>   	};
> >> 
> >> +
> >> +	ether: ethernet@fde00000 {
> >> +		device_type = "network";
> > 
> > Documentation/devicetree/bindings/net/sh_eth.txt from "[PATCH 2/2] sh_eth:
> > add device tree support" doesn't document this property, should you
> > update it ?
>     It's a standard property but completely optional for non-OF systems.
> I've not seen anybody document it but I should grep for it...

Documentation/devicetree/booting-without-of.txt mentions it, as well as a 
couple of other DT bindings document, but there's no clear documentation of 
what device_type should be for network device, and whether the property is 
mandatory or optional. Clarifying the documentation would be great.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Pawel Moll]

On Mon, 2013-09-02 at 15:18 +0100, Laurent Pinchart wrote:
> Documentation/devicetree/booting-without-of.txt mentions it, as well as a 
> couple of other DT bindings document, but there's no clear documentation of 
> what device_type should be for network device, and whether the property is 
> mandatory or optional. Clarifying the documentation would be great.

It's documented in the ePAPR spec:

"The device_type property was used in IEEE 1275 to describe the device’s
FCode programming model. Because ePAPR does not have FCode, new use of
the property is deprecated, and it should be included only on cpu and
memory nodes for compatibility with IEEE 1275–derived device trees."

The bottom line is - don't use it, unless you've good good reasons?

Paweł

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Laurent Pinchart]

Hi Paweł,

On Monday 02 September 2013 16:32:34 Pawel Moll wrote:
> On Mon, 2013-09-02 at 15:18 +0100, Laurent Pinchart wrote:
> > Documentation/devicetree/booting-without-of.txt mentions it, as well as a
> > couple of other DT bindings document, but there's no clear documentation
> > of what device_type should be for network device, and whether the property
> > is mandatory or optional. Clarifying the documentation would be great.
> 
> It's documented in the ePAPR spec:
> 
> "The device_type property was used in IEEE 1275 to describe the device’s
> FCode programming model. Because ePAPR does not have FCode, new use of
> the property is deprecated, and it should be included only on cpu and
> memory nodes for compatibility with IEEE 1275–derived device trees."

Thank you for the clarification.

> The bottom line is - don't use it, unless you've good good reasons?

I'll let Sergei share his opinion on whether we've got good reasons :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Sergei Shtylyov]

Hello.

On 03-09-2013 16:11, Laurent Pinchart wrote:

>>> Documentation/devicetree/booting-without-of.txt mentions it, as well as a
>>> couple of other DT bindings document, but there's no clear documentation
>>> of what device_type should be for network device, and whether the property
>>> is mandatory or optional. Clarifying the documentation would be great.

>> It's documented in the ePAPR spec:

>> "The device_type property was used in IEEE 1275 to describe the device’s
>> FCode programming model. Because ePAPR does not have FCode, new use of
>> the property is deprecated, and it should be included only on cpu and
>> memory nodes for compatibility with IEEE 1275–derived device trees."

> Thank you for the clarification.

>> The bottom line is - don't use it, unless you've good good reasons?

> I'll let Sergei share his opinion on whether we've got good reasons :-)

    No, we don't, I'll remove the prop.

WBR, Sergei

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Magnus Damm]

On Sun, Sep 1, 2013 at 8:15 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Define the generic R8A777x part of the Ether device node.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
>  arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
> =================================> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
> @@ -98,4 +98,15 @@
>                 reg = <0xfffc000 0x118>;
>                 #gpio-range-cells = <3>;
>         };
> +
> +       ether: ethernet@fde00000 {
> +               device_type = "network";
> +               compatible = "renesas,ether-r8a7779";

Hi Sergei,

Thanks for your patch. What's the reason behind the r8a7778 SoC using
a compatible string for r8a7779 like "renesas,ether-r8a7779"?

It seems that you assume that the r8a7778 ethernet controller is 100%
compatible with r8a7779. Is that really true? For earlier versions the
sh_eth hardware documentation was anything but accurate, so it seems
to me that it must be more safe that r8a7778 would be using
"renesas,ether-r8a7778". What do you think?

Thanks,

/ magnus

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Sergei Shtylyov]

Hello.

On 09/03/2013 07:17 PM, Magnus Damm wrote:

>> Define the generic R8A777x part of the Ether device node.

>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

>> ---
>>   arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
>> =================================>> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
>> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
>> @@ -98,4 +98,15 @@
>>                  reg = <0xfffc000 0x118>;
>>                  #gpio-range-cells = <3>;
>>          };
>> +
>> +       ether: ethernet@fde00000 {
>> +               device_type = "network";
>> +               compatible = "renesas,ether-r8a7779";

> Hi Sergei,

> Thanks for your patch. What's the reason behind the r8a7778 SoC using
> a compatible string for r8a7779 like "renesas,ether-r8a7779"?

    R8A7779 support has appeared first in Linux and as R8A7778 Ether is 
identical to R8A7779 and no wildcards are allowed in the device tree, I 
decided to use this "compatible" prop.

> It seems that you assume that the r8a7778 ethernet controller is 100%
> compatible with r8a7779. Is that really true? For earlier versions the
> sh_eth hardware documentation was anything but accurate, so it seems
> to me that it must be more safe that r8a7778 would be using
> "renesas,ether-r8a7778". What do you think?

    I think R8A7778 and R8A7779 EtherMACs are identical. I've cross checked 
the documentation at the start of the development and the registers appeared 
to be the same.

> Thanks,

> / magnus

WBR, Sergei

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Magnus Damm]

Hi Sergei,

[CC Laurent]

On Wed, Sep 4, 2013 at 3:27 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
>
> On 09/03/2013 07:17 PM, Magnus Damm wrote:
>
>>> Define the generic R8A777x part of the Ether device node.
>
>
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
>
>>> ---
>>>   arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
>>>   1 file changed, 11 insertions(+)
>>>
>>> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
>>> =================================>>> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
>>> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
>>> @@ -98,4 +98,15 @@
>>>                  reg = <0xfffc000 0x118>;
>>>                  #gpio-range-cells = <3>;
>>>          };
>>> +
>>> +       ether: ethernet@fde00000 {
>>> +               device_type = "network";
>>> +               compatible = "renesas,ether-r8a7779";
>
>
>> Hi Sergei,
>
>
>> Thanks for your patch. What's the reason behind the r8a7778 SoC using
>> a compatible string for r8a7779 like "renesas,ether-r8a7779"?
>
>
>    R8A7779 support has appeared first in Linux and as R8A7778 Ether is
> identical to R8A7779 and no wildcards are allowed in the device tree, I
> decided to use this "compatible" prop.

Thanks for your reply, I see.

>> It seems that you assume that the r8a7778 ethernet controller is 100%
>> compatible with r8a7779. Is that really true? For earlier versions the
>> sh_eth hardware documentation was anything but accurate, so it seems
>> to me that it must be more safe that r8a7778 would be using
>> "renesas,ether-r8a7778". What do you think?
>
>
>    I think R8A7778 and R8A7779 EtherMACs are identical. I've cross checked
> the documentation at the start of the development and the registers appeared
> to be the same.

But even if the current version of the documentation happens to be
similar in it still doesn't guarantee that the IP is the same. And
using the "correct" SoC compatible value doesn't really hurt in any
way, does it?

My feeling is that using the identical SoC as compatible value must be
the best option - unless we know for sure they are identical that is.

So unless we're 100% certain about IP compatibility I'm trying to
enforce that we either use a strict matching for exactly the same SoC
version or IP block. Using a different maybe-compatible SoC string
seems to be begging for future trouble IMO.

Laurent, any opinion?

Cheers,

/ manus

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Laurent Pinchart]

On Wednesday 04 September 2013 14:35:14 Magnus Damm wrote:
> On Wed, Sep 4, 2013 at 3:27 AM, Sergei Shtylyov wrote:
> > On 09/03/2013 07:17 PM, Magnus Damm wrote:
> >>> Define the generic R8A777x part of the Ether device node.
> >>> 
> >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >>> 
> >>> ---
> >>> 
> >>>   arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
> >>>   1 file changed, 11 insertions(+)
> >>> 
> >>> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
> >>> =================================> >>> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
> >>> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
> >>> @@ -98,4 +98,15 @@
> >>>                  reg = <0xfffc000 0x118>;
> >>>                  #gpio-range-cells = <3>;
> >>>          };
> >>> +
> >>> +       ether: ethernet@fde00000 {
> >>> +               device_type = "network";
> >>> +               compatible = "renesas,ether-r8a7779";
> >> 
> >> Hi Sergei,
> >> 
> >> Thanks for your patch. What's the reason behind the r8a7778 SoC using
> >> a compatible string for r8a7779 like "renesas,ether-r8a7779"?
> >
> > R8A7779 support has appeared first in Linux and as R8A7778 Ether is
> > identical to R8A7779 and no wildcards are allowed in the device tree, I
> > decided to use this "compatible" prop.
> 
> Thanks for your reply, I see.
> 
> >> It seems that you assume that the r8a7778 ethernet controller is 100%
> >> compatible with r8a7779. Is that really true? For earlier versions the
> >> sh_eth hardware documentation was anything but accurate, so it seems
> >> to me that it must be more safe that r8a7778 would be using
> >> "renesas,ether-r8a7778". What do you think?
> >
> > I think R8A7778 and R8A7779 EtherMACs are identical. I've cross checked
> > the documentation at the start of the development and the registers
> > appeared to be the same.
> 
> But even if the current version of the documentation happens to be
> similar in it still doesn't guarantee that the IP is the same. And
> using the "correct" SoC compatible value doesn't really hurt in any
> way, does it?
> 
> My feeling is that using the identical SoC as compatible value must be
> the best option - unless we know for sure they are identical that is.
> 
> So unless we're 100% certain about IP compatibility I'm trying to
> enforce that we either use a strict matching for exactly the same SoC
> version or IP block. Using a different maybe-compatible SoC string
> seems to be begging for future trouble IMO.
> 
> Laurent, any opinion?

I agree. If we had access to IP version information we could use that to 
construct the compatible name. As we don't, the SoC name is the best option, 
and to ensure future compatibility we should use the exact SoC version.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Simon Horman]

On Wed, Sep 04, 2013 at 11:39:59AM +0200, Laurent Pinchart wrote:
> On Wednesday 04 September 2013 14:35:14 Magnus Damm wrote:
> > On Wed, Sep 4, 2013 at 3:27 AM, Sergei Shtylyov wrote:
> > > On 09/03/2013 07:17 PM, Magnus Damm wrote:
> > >>> Define the generic R8A777x part of the Ether device node.
> > >>> 
> > >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> > >>> 
> > >>> ---
> > >>> 
> > >>>   arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
> > >>>   1 file changed, 11 insertions(+)
> > >>> 
> > >>> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
> > >>> =================================> > >>> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
> > >>> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
> > >>> @@ -98,4 +98,15 @@
> > >>>                  reg = <0xfffc000 0x118>;
> > >>>                  #gpio-range-cells = <3>;
> > >>>          };
> > >>> +
> > >>> +       ether: ethernet@fde00000 {
> > >>> +               device_type = "network";
> > >>> +               compatible = "renesas,ether-r8a7779";
> > >> 
> > >> Hi Sergei,
> > >> 
> > >> Thanks for your patch. What's the reason behind the r8a7778 SoC using
> > >> a compatible string for r8a7779 like "renesas,ether-r8a7779"?
> > >
> > > R8A7779 support has appeared first in Linux and as R8A7778 Ether is
> > > identical to R8A7779 and no wildcards are allowed in the device tree, I
> > > decided to use this "compatible" prop.
> > 
> > Thanks for your reply, I see.
> > 
> > >> It seems that you assume that the r8a7778 ethernet controller is 100%
> > >> compatible with r8a7779. Is that really true? For earlier versions the
> > >> sh_eth hardware documentation was anything but accurate, so it seems
> > >> to me that it must be more safe that r8a7778 would be using
> > >> "renesas,ether-r8a7778". What do you think?
> > >
> > > I think R8A7778 and R8A7779 EtherMACs are identical. I've cross checked
> > > the documentation at the start of the development and the registers
> > > appeared to be the same.
> > 
> > But even if the current version of the documentation happens to be
> > similar in it still doesn't guarantee that the IP is the same. And
> > using the "correct" SoC compatible value doesn't really hurt in any
> > way, does it?
> > 
> > My feeling is that using the identical SoC as compatible value must be
> > the best option - unless we know for sure they are identical that is.
> > 
> > So unless we're 100% certain about IP compatibility I'm trying to
> > enforce that we either use a strict matching for exactly the same SoC
> > version or IP block. Using a different maybe-compatible SoC string
> > seems to be begging for future trouble IMO.
> > 
> > Laurent, any opinion?
> 
> I agree. If we had access to IP version information we could use that to 
> construct the compatible name. As we don't, the SoC name is the best option, 
> and to ensure future compatibility we should use the exact SoC version.

Using the SoC name is in keeping with recent discussions that
we have had about compat string names for shmobile. So I agree
with both Magnus and Laurent here (no surprise!).

[PATCH 1/4] ARM: shmobile: r8a7778: add I2C clock for DT

[Kuninori Morimoto]

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm/mach-shmobile/clock-r8a7778.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-shmobile/clock-r8a7778.c b/arch/arm/mach-shmobile/clock-r8a7778.c
index 3590cd5..e043715 100644
--- a/arch/arm/mach-shmobile/clock-r8a7778.c
+++ b/arch/arm/mach-shmobile/clock-r8a7778.c
@@ -184,9 +184,13 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("ohci-platform", &mstp_clks[MSTP100]), /* USB OHCI port0/1 */
 	CLKDEV_DEV_ID("renesas_usbhs", &mstp_clks[MSTP100]), /* USB FUNC */
 	CLKDEV_DEV_ID("i2c-rcar.0", &mstp_clks[MSTP030]), /* I2C0 */
+	CLKDEV_DEV_ID("ffc70000.i2c", &mstp_clks[MSTP030]), /* I2C0 */
 	CLKDEV_DEV_ID("i2c-rcar.1", &mstp_clks[MSTP029]), /* I2C1 */
+	CLKDEV_DEV_ID("ffc71000.i2c", &mstp_clks[MSTP029]), /* I2C1 */
 	CLKDEV_DEV_ID("i2c-rcar.2", &mstp_clks[MSTP028]), /* I2C2 */
+	CLKDEV_DEV_ID("ffc72000.i2c", &mstp_clks[MSTP028]), /* I2C2 */
 	CLKDEV_DEV_ID("i2c-rcar.3", &mstp_clks[MSTP027]), /* I2C3 */
+	CLKDEV_DEV_ID("ffc73000.i2c", &mstp_clks[MSTP027]), /* I2C3 */
 	CLKDEV_DEV_ID("sh-sci.0", &mstp_clks[MSTP026]), /* SCIF0 */
 	CLKDEV_DEV_ID("sh-sci.1", &mstp_clks[MSTP025]), /* SCIF1 */
 	CLKDEV_DEV_ID("sh-sci.2", &mstp_clks[MSTP024]), /* SCIF2 */
-- 
1.7.9.5

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add I2C clock for DT

[Simon Horman]

On Thu, Oct 03, 2013 at 11:43:54PM -0700, Kuninori Morimoto wrote:
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  arch/arm/mach-shmobile/clock-r8a7778.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/mach-shmobile/clock-r8a7778.c b/arch/arm/mach-shmobile/clock-r8a7778.c
> index 3590cd5..e043715 100644
> --- a/arch/arm/mach-shmobile/clock-r8a7778.c
> +++ b/arch/arm/mach-shmobile/clock-r8a7778.c
> @@ -184,9 +184,13 @@ static struct clk_lookup lookups[] = {
>  	CLKDEV_DEV_ID("ohci-platform", &mstp_clks[MSTP100]), /* USB OHCI port0/1 */
>  	CLKDEV_DEV_ID("renesas_usbhs", &mstp_clks[MSTP100]), /* USB FUNC */
>  	CLKDEV_DEV_ID("i2c-rcar.0", &mstp_clks[MSTP030]), /* I2C0 */
> +	CLKDEV_DEV_ID("ffc70000.i2c", &mstp_clks[MSTP030]), /* I2C0 */
>  	CLKDEV_DEV_ID("i2c-rcar.1", &mstp_clks[MSTP029]), /* I2C1 */
> +	CLKDEV_DEV_ID("ffc71000.i2c", &mstp_clks[MSTP029]), /* I2C1 */
>  	CLKDEV_DEV_ID("i2c-rcar.2", &mstp_clks[MSTP028]), /* I2C2 */
> +	CLKDEV_DEV_ID("ffc72000.i2c", &mstp_clks[MSTP028]), /* I2C2 */
>  	CLKDEV_DEV_ID("i2c-rcar.3", &mstp_clks[MSTP027]), /* I2C3 */
> +	CLKDEV_DEV_ID("ffc73000.i2c", &mstp_clks[MSTP027]), /* I2C3 */
>  	CLKDEV_DEV_ID("sh-sci.0", &mstp_clks[MSTP026]), /* SCIF0 */
>  	CLKDEV_DEV_ID("sh-sci.1", &mstp_clks[MSTP025]), /* SCIF1 */
>  	CLKDEV_DEV_ID("sh-sci.2", &mstp_clks[MSTP024]), /* SCIF2 */

Thanks, I have queued this up but I am unsure if it will
end up in v3.13 or v3.14.

[PATCH 1/4] ARM: shmobile: r8a7778: add audio clock in new style

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current audio clock doesn't have dependency to device/driver,
but, it is not good design for DT support.
To avoid branch merge conflict issue,
it uses this load map, and this patch is 1) part.
 1) add new style clock in platform
 2) add new style clock method in driver
 3) remove old tyle clock from platform

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm/mach-shmobile/clock-r8a7778.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-shmobile/clock-r8a7778.c b/arch/arm/mach-shmobile/clock-r8a7778.c
index 9783945..2009a9b 100644
--- a/arch/arm/mach-shmobile/clock-r8a7778.c
+++ b/arch/arm/mach-shmobile/clock-r8a7778.c
@@ -221,6 +221,10 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("fffc6000.spi", &mstp_clks[MSTP007]), /* HSPI2 */
 	CLKDEV_DEV_ID("rcar_sound", &mstp_clks[MSTP008]), /* SRU */
 
+	CLKDEV_ICK_ID("clk_a", "rcar_sound", &audio_clk_a),
+	CLKDEV_ICK_ID("clk_b", "rcar_sound", &audio_clk_b),
+	CLKDEV_ICK_ID("clk_c", "rcar_sound", &audio_clk_c),
+	CLKDEV_ICK_ID("clk_i", "rcar_sound", &s1_clk),
 	CLKDEV_ICK_ID("ssi.0", "rcar_sound", &mstp_clks[MSTP012]),
 	CLKDEV_ICK_ID("ssi.1", "rcar_sound", &mstp_clks[MSTP011]),
 	CLKDEV_ICK_ID("ssi.2", "rcar_sound", &mstp_clks[MSTP010]),
-- 
1.7.9.5

[PATCH 1/4] ARM: shmobile: r8a7778: add SCI clock support for DT

[Simon Horman]

This will be used when initialising SCI devices using DT
until common clock framework support is added.

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/mach-shmobile/clock-r8a7778.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/mach-shmobile/clock-r8a7778.c b/arch/arm/mach-shmobile/clock-r8a7778.c
index 13f8f3a..a6dd601 100644
--- a/arch/arm/mach-shmobile/clock-r8a7778.c
+++ b/arch/arm/mach-shmobile/clock-r8a7778.c
@@ -202,11 +202,17 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("i2c-rcar.3", &mstp_clks[MSTP027]), /* I2C3 */
 	CLKDEV_DEV_ID("ffc73000.i2c", &mstp_clks[MSTP027]), /* I2C3 */
 	CLKDEV_DEV_ID("sh-sci.0", &mstp_clks[MSTP026]), /* SCIF0 */
+	CLKDEV_DEV_ID("ffe40000.serial", &mstp_clks[MSTP026]), /* SCIF0 */
 	CLKDEV_DEV_ID("sh-sci.1", &mstp_clks[MSTP025]), /* SCIF1 */
+	CLKDEV_DEV_ID("ffe41000.serial", &mstp_clks[MSTP025]), /* SCIF1 */
 	CLKDEV_DEV_ID("sh-sci.2", &mstp_clks[MSTP024]), /* SCIF2 */
+	CLKDEV_DEV_ID("ffe42000.serial", &mstp_clks[MSTP024]), /* SCIF2 */
 	CLKDEV_DEV_ID("sh-sci.3", &mstp_clks[MSTP023]), /* SCIF3 */
+	CLKDEV_DEV_ID("ffe43000.serial", &mstp_clks[MSTP023]), /* SCIF3 */
 	CLKDEV_DEV_ID("sh-sci.4", &mstp_clks[MSTP022]), /* SCIF4 */
+	CLKDEV_DEV_ID("ffe44000.serial", &mstp_clks[MSTP022]), /* SCIF4 */
 	CLKDEV_DEV_ID("sh-sci.5", &mstp_clks[MSTP021]), /* SCIF6 */
+	CLKDEV_DEV_ID("ffe45000.serial", &mstp_clks[MSTP021]), /* SCIF5 */
 	CLKDEV_DEV_ID("sh-hspi.0", &mstp_clks[MSTP007]), /* HSPI0 */
 	CLKDEV_DEV_ID("fffc7000.spi", &mstp_clks[MSTP007]), /* HSPI0 */
 	CLKDEV_DEV_ID("sh-hspi.1", &mstp_clks[MSTP007]), /* HSPI1 */
-- 
2.0.0