| Message ID | 53B5F327.5040301@renesas.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5179ffd099446e65a1e094b9cb17cc7edab45583 |
| Headers | show |
(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. -- 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 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 -- 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, 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. -- 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 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. -- 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
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. > -- 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 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 -- 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 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 >
> 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
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 -- 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 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 >
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 -- 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 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. -- 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
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.
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 {
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(-)