[PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Khiem Nguyen]

I2C bus for VDD MPU regulator is IIC3, not I2C3.

Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
---
v2:
  - re-based on top of tag renesas-devel-v3.16-rc3-20140701.
  - removed redundant spaces.

 arch/arm/boot/dts/r8a7790-lager.dts |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index 544202b..ffe0523 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -215,9 +215,9 @@
 		renesas,function = "i2c2";
 	};
 
-	i2c3_pins: i2c3 {
-		renesas,groups = "i2c3";
-		renesas,function = "i2c3";
+	iic3_pins: iic3 {
+		renesas,groups = "iic3";
+		renesas,function = "iic3";
 	};
 
 	usb0_pins: usb0 {
@@ -368,9 +368,9 @@
 	pinctrl-names = "default";
 };
 
-&i2c3 {
+&iic3 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&i2c3_pins>;
+	pinctrl-0 = <&iic3_pins>;
 	status = "okay";
 
 	vdd_dvfs: regulator@68 {
-- 
1.7.9.5

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Gaku Inami]

(2014/07/04 9:19), Khiem Nguyen wrote:
> I2C bus for VDD MPU regulator is IIC3, not I2C3.
>
> Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
> ---
> v2:
>   - re-based on top of tag renesas-devel-v3.16-rc3-20140701.
>   - removed redundant spaces.
>
>  arch/arm/boot/dts/r8a7790-lager.dts |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index 544202b..ffe0523 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -215,9 +215,9 @@
>  		renesas,function = "i2c2";
>  	};
>  
> -	i2c3_pins: i2c3 {
> -		renesas,groups = "i2c3";
> -		renesas,function = "i2c3";
> +	iic3_pins: iic3 {
> +		renesas,groups = "iic3";
> +		renesas,function = "iic3";
>  	};
>  
>  	usb0_pins: usb0 {
> @@ -368,9 +368,9 @@
>  	pinctrl-names = "default";
>  };
>  
> -&i2c3 {
> +&iic3 {
>  	pinctrl-names = "default";
> -	pinctrl-0 = <&i2c3_pins>;
> +	pinctrl-0 = <&iic3_pins>;
>  	status = "okay";
>  
>  	vdd_dvfs: regulator@68 {

Thank you for updating patch. 
I tested your patch. Test reult is good.

[Test result] (in both SMP and non-SMP)
  - kernel boot          : OK
  - regurator initialize : OK
  - cpufreq initialize   : OK
  - frequency scaling    : OK
  - changes governor     : OK

Tested-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>

This is a bug fix of my cpufreq patch. Thank you for your fix.

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Khiem Nguyen]

Hi Inami-san,

On 7/4/2014 2:40 PM, Gaku Inami wrote:
>> I2C bus for VDD MPU regulator is IIC3, not I2C3.
[...]
>> Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
[...]
> Thank you for updating patch. 
> I tested your patch. Test reult is good.
Thanks for your testing.

[...]
> Tested-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
[...]
> This is a bug fix of my cpufreq patch. Thank you for your fix.
OK. 
Let's wait for final comment from Magnus-san.

Best regards,
KHIEM Nguyen

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Simon Horman]

On Fri, Jul 04, 2014 at 03:41:21PM +0900, Khiem Nguyen wrote:
> Hi Inami-san,
> 
> On 7/4/2014 2:40 PM, Gaku Inami wrote:
> >> I2C bus for VDD MPU regulator is IIC3, not I2C3.
> [...]
> >> Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
> [...]
> > Thank you for updating patch. 
> > I tested your patch. Test reult is good.
> Thanks for your testing.
> 
> [...]
> > Tested-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
> [...]
> > This is a bug fix of my cpufreq patch. Thank you for your fix.
> OK. 
> Let's wait for final comment from Magnus-san.

Thanks, I have decided to queue this up.

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Simon Horman]

On Sun, Jul 06, 2014 at 04:37:28PM +0200, Simon Horman wrote:
> On Fri, Jul 04, 2014 at 03:41:21PM +0900, Khiem Nguyen wrote:
> > Hi Inami-san,
> > 
> > On 7/4/2014 2:40 PM, Gaku Inami wrote:
> > >> I2C bus for VDD MPU regulator is IIC3, not I2C3.
> > [...]
> > >> Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
> > [...]
> > > Thank you for updating patch. 
> > > I tested your patch. Test reult is good.
> > Thanks for your testing.
> > 
> > [...]
> > > Tested-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
> > [...]
> > > This is a bug fix of my cpufreq patch. Thank you for your fix.
> > OK. 
> > Let's wait for final comment from Magnus-san.
> 
> Thanks, I have decided to queue this up.

On second thoughts I will wait for Magnus.

In particular I think you still need to address his feedback:

	Hi Khiem-san,

	Thanks for your patch. Can you please extend the commit message to
	include the reason behind this patch?

	From my side this looks like a software policy change. Unless I'm
	mistaken both I2C3 and IIC3 share the same pins on the SoC, and
	because of that it should be possible to access that particular I2C
	bus already without any modification.

	So please explain why you want to change this.

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Khiem Nguyen]

Dear Simon-san, Magnus-san,
cc Inami-san,

>> 	Thanks for your patch. Can you please extend the commit message to
>> 	include the reason behind this patch?

>> In particular I think you still need to address his feedback:

I'd like to explain it here. After all parties agree,
I will send new version of my patch.

The reason of this patch is simply to align with information
in datasheet of R-Car H2.
With this change, CPUFreq implementation for Lager board is aligned 
with implementation for Koelsch board.

Is it reasonable enough ?

Thanks.

Best regards,
KHIEM Nguyen

On 7/6/2014 11:58 PM, Simon Horman wrote:
> On Sun, Jul 06, 2014 at 04:37:28PM +0200, Simon Horman wrote:
>> On Fri, Jul 04, 2014 at 03:41:21PM +0900, Khiem Nguyen wrote:
>>> Hi Inami-san,
>>>
>>> On 7/4/2014 2:40 PM, Gaku Inami wrote:
>>>>> I2C bus for VDD MPU regulator is IIC3, not I2C3.
>>> [...]
>>>>> Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
>>> [...]
>>>> Thank you for updating patch. 
>>>> I tested your patch. Test reult is good.
>>> Thanks for your testing.
>>>
>>> [...]
>>>> Tested-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
>>> [...]
>>>> This is a bug fix of my cpufreq patch. Thank you for your fix.
>>> OK. 
>>> Let's wait for final comment from Magnus-san.
>>
>> Thanks, I have decided to queue this up.
> 
> On second thoughts I will wait for Magnus.
> 
> In particular I think you still need to address his feedback:
> 
> 	Hi Khiem-san,
> 
> 	Thanks for your patch. Can you please extend the commit message to
> 	include the reason behind this patch?
> 
> 	From my side this looks like a software policy change. Unless I'm
> 	mistaken both I2C3 and IIC3 share the same pins on the SoC, and
> 	because of that it should be possible to access that particular I2C
> 	bus already without any modification.
> 
> 	So please explain why you want to change this.
> 

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Magnus Damm]

Hi Khiem-san, Wolfram, everyone,

[CC Wolfram]

On Fri, Jul 4, 2014 at 2:19 AM, Khiem Nguyen
<khiem.nguyen.xt@renesas.com> wrote:
> I2C bus for VDD MPU regulator is IIC3, not I2C3.
>
> Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
> ---
> v2:
>   - re-based on top of tag renesas-devel-v3.16-rc3-20140701.
>   - removed redundant spaces.

This patch is changing I2C master controller device for the I2C bus
where the power chip is located. I've heard that all of our different
I2C controllers come with a range of features and issues, so I think
we should check with the I2C maintainer Wolfram Sang about this
matter.

Wolfram, can you give us your opinion about this patch? Does it
improve our I2C software support on Lager?

Khiem-san, in the future please CC Wolfram about issues related to I2C.

Thanks,

/ magnus

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Khiem Nguyen]

On 7/10/2014 5:23 PM, Magnus Damm wrote:
> Hi Khiem-san, Wolfram, everyone,
> 
> [CC Wolfram]
> 
> On Fri, Jul 4, 2014 at 2:19 AM, Khiem Nguyen
> <khiem.nguyen.xt@renesas.com> wrote:
>> I2C bus for VDD MPU regulator is IIC3, not I2C3.
>>
>> Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
>> ---
>> v2:
>>   - re-based on top of tag renesas-devel-v3.16-rc3-20140701.
>>   - removed redundant spaces.
> 
> This patch is changing I2C master controller device for the I2C bus
> where the power chip is located. I've heard that all of our different
> I2C controllers come with a range of features and issues, so I think
> we should check with the I2C maintainer Wolfram Sang about this
> matter.
> 
> Wolfram, can you give us your opinion about this patch? Does it
> improve our I2C software support on Lager?
IICDVFS is used for controlling power chip, according to H2 datasheet.
In H2/Lager board, name of IICDVFS is IIC3.
This patch is to correct I2C bus in H2/Lager board (revise to IIC3 from I2C3).

In M2/Koelsch board, name of IICDVFS is I2C6. 
> 
> Khiem-san, in the future please CC Wolfram about issues related to I2C.
I got it. Thanks. 
> 
> / magnus
> 

-- 
Best regards,
KHIEM Nguyen

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 780 bytes --]


> Wolfram, can you give us your opinion about this patch? Does it
> improve our I2C software support on Lager?

Yes, it does. Khiem-san is right, IIC3 is dedicated to DVFS, so it
should be activated. From that point of view:

Reviewed-by: Wolfram Sang <wsa@sang-engineering.com>

However, I sadly missed commit e489c2a9bc82713167d9f721ca764f4b0d37e543
("ARM: shmobile: lager: enable i2c devices") which enables i2c cores for
all busses. I do like the iic cores better because of SMBUS_QUICK
support and DMA capability (although not yet supported). i2c cores have
slave support as an advantage, yet they can be switched in when that is
actually needed (still needs to be implemented, too). So, it might make
sense to switch all busses to iic, I'd say.

All the best,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Magnus Damm]

Hi Wolfram, Khiem-san, everyone,

On Thu, Jul 10, 2014 at 11:40 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> Wolfram, can you give us your opinion about this patch? Does it
>> improve our I2C software support on Lager?
>
> Yes, it does. Khiem-san is right, IIC3 is dedicated to DVFS, so it
> should be activated. From that point of view:
>
> Reviewed-by: Wolfram Sang <wsa@sang-engineering.com>

Good - thanks for confirming!

> However, I sadly missed commit e489c2a9bc82713167d9f721ca764f4b0d37e543
> ("ARM: shmobile: lager: enable i2c devices") which enables i2c cores for
> all busses. I do like the iic cores better because of SMBUS_QUICK
> support and DMA capability (although not yet supported). i2c cores have
> slave support as an advantage, yet they can be switched in when that is
> actually needed (still needs to be implemented, too). So, it might make
> sense to switch all busses to iic, I'd say.

Yes, I agree about your observation. If you have time, can you cook up
a patch to improve the situation?

Best,

/ magnus

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Khiem Nguyen]

On 7/10/2014 6:46 PM, Magnus Damm wrote:
> Hi Wolfram, Khiem-san, everyone,
> 
> On Thu, Jul 10, 2014 at 11:40 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>>
>>> Wolfram, can you give us your opinion about this patch? Does it
>>> improve our I2C software support on Lager?
>>
>> Yes, it does. Khiem-san is right, IIC3 is dedicated to DVFS, so it
>> should be activated. From that point of view:
>>
>> Reviewed-by: Wolfram Sang <wsa@sang-engineering.com>
> 
> Good - thanks for confirming!
@Wolfram-san: Thanks for your review.

@Magnus-san, Simon-san: Then, is it OK to merge this patch ?
> 
>> However, I sadly missed commit e489c2a9bc82713167d9f721ca764f4b0d37e543
>> ("ARM: shmobile: lager: enable i2c devices") which enables i2c cores for
>> all busses. I do like the iic cores better because of SMBUS_QUICK
>> support and DMA capability (although not yet supported). i2c cores have
>> slave support as an advantage, yet they can be switched in when that is
>> actually needed (still needs to be implemented, too). So, it might make
>> sense to switch all busses to iic, I'd say.
> 
> Yes, I agree about your observation. If you have time, can you cook up
> a patch to improve the situation?
> 
> Best,
> 
> / magnus
> 

-- 
Best regards,
KHIEM Nguyen

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Magnus Damm]

Hi Khiem-san,

On Thu, Jul 10, 2014 at 12:01 PM, Khiem Nguyen
<khiem.nguyen.xt@renesas.com> wrote:
> On 7/10/2014 6:46 PM, Magnus Damm wrote:
>> Hi Wolfram, Khiem-san, everyone,
>>
>> On Thu, Jul 10, 2014 at 11:40 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>>>
>>>> Wolfram, can you give us your opinion about this patch? Does it
>>>> improve our I2C software support on Lager?
>>>
>>> Yes, it does. Khiem-san is right, IIC3 is dedicated to DVFS, so it
>>> should be activated. From that point of view:
>>>
>>> Reviewed-by: Wolfram Sang <wsa@sang-engineering.com>
>>
>> Good - thanks for confirming!
> @Wolfram-san: Thanks for your review.
>
> @Magnus-san, Simon-san: Then, is it OK to merge this patch ?

Yes, from my side all is fine. I believe Simon will merge it when he
has some time to spare.

Cheers,

/ magnus

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Simon Horman]

On Thu, Jul 10, 2014 at 12:03:12PM +0200, Magnus Damm wrote:
> Hi Khiem-san,
> 
> On Thu, Jul 10, 2014 at 12:01 PM, Khiem Nguyen
> <khiem.nguyen.xt@renesas.com> wrote:
> > On 7/10/2014 6:46 PM, Magnus Damm wrote:
> >> Hi Wolfram, Khiem-san, everyone,
> >>
> >> On Thu, Jul 10, 2014 at 11:40 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >>>
> >>>> Wolfram, can you give us your opinion about this patch? Does it
> >>>> improve our I2C software support on Lager?
> >>>
> >>> Yes, it does. Khiem-san is right, IIC3 is dedicated to DVFS, so it
> >>> should be activated. From that point of view:
> >>>
> >>> Reviewed-by: Wolfram Sang <wsa@sang-engineering.com>
> >>
> >> Good - thanks for confirming!
> > @Wolfram-san: Thanks for your review.
> >
> > @Magnus-san, Simon-san: Then, is it OK to merge this patch ?
> 
> Yes, from my side all is fine. I believe Simon will merge it when he
> has some time to spare.

Thanks, I have queued this up with Wolfram's Reviewed-by tag.

Re: [PATCH v2] ARM: shmobile: Lager: Correct I2C bus for VDD MPU regulator

[Khiem Nguyen]

Dear Simon-san,

On 7/11/2014 5:45 PM, Simon Horman wrote:

>>>>>> Wolfram, can you give us your opinion about this patch? Does it
>>>>>> improve our I2C software support on Lager?

>>>>> Yes, it does. Khiem-san is right, IIC3 is dedicated to DVFS, so it
>>>>> should be activated. From that point of view:

>>>>> Reviewed-by: Wolfram Sang <wsa@sang-engineering.com>

> 
> Thanks, I have queued this up with Wolfram's Reviewed-by tag.
>

Thanks. I got it.

-- 
Best regards,
KHIEM Nguyen