Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-04 Thread Lothar Waßmann
Hi,

On Wed, 4 Jul 2018 01:42:54 + Robin Gong wrote:
> On 二, 2018-07-03 at 08:10 -0300, Fabio Estevam wrote:
> > Hi Anson,
> > 
> > On Tue, Jul 3, 2018 at 4:44 AM, Anson Huang 
> > wrote:
> > 
> > > 
> > > It is NOT easy to identify which switch is critical or NOT, and
> > > different platforms
> > > have different board design, it will introduce many platform
> > > specified code, so I think
> > > just revert the pfuze100 switch enable/disable patch should be OK
> > > for now.
> > I have sent the pfuze100 regulator patch revert and it is linux-next
> > now. Should probably reach 4.18-rc4.
> > 
> > > 
> > > After a couple of release cycles, add the pfuze100 switch
> > > enable/disable patch
> > > back to support this feature, I believe users should switch to new
> > > dtb with "regulator-always-on"
> > > existing already.
> > That will still break old dtb compatibility.
> > 
> > You cannot force users to use "regulator-always-on" and the old dtbs
> > need to always work.
> > 
> > So whatever new feature you need to introduce it needs to be done in
> > such a way that the existing dtb's will continue working.
> But actually existing dtb is not right since the critical power rail
> missing 'regulator-always-on'. It's a fix patch for dts, not related
> with following dtb/kernel break rules, just a simple dts patch. Why
> should we make promise for the wrong dtbs?
>
Because they are living in the outside world on real devices.


Lothar Waßmann


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-04 Thread Lothar Waßmann
Hi,

On Wed, 4 Jul 2018 01:42:54 + Robin Gong wrote:
> On 二, 2018-07-03 at 08:10 -0300, Fabio Estevam wrote:
> > Hi Anson,
> > 
> > On Tue, Jul 3, 2018 at 4:44 AM, Anson Huang 
> > wrote:
> > 
> > > 
> > > It is NOT easy to identify which switch is critical or NOT, and
> > > different platforms
> > > have different board design, it will introduce many platform
> > > specified code, so I think
> > > just revert the pfuze100 switch enable/disable patch should be OK
> > > for now.
> > I have sent the pfuze100 regulator patch revert and it is linux-next
> > now. Should probably reach 4.18-rc4.
> > 
> > > 
> > > After a couple of release cycles, add the pfuze100 switch
> > > enable/disable patch
> > > back to support this feature, I believe users should switch to new
> > > dtb with "regulator-always-on"
> > > existing already.
> > That will still break old dtb compatibility.
> > 
> > You cannot force users to use "regulator-always-on" and the old dtbs
> > need to always work.
> > 
> > So whatever new feature you need to introduce it needs to be done in
> > such a way that the existing dtb's will continue working.
> But actually existing dtb is not right since the critical power rail
> missing 'regulator-always-on'. It's a fix patch for dts, not related
> with following dtb/kernel break rules, just a simple dts patch. Why
> should we make promise for the wrong dtbs?
>
Because they are living in the outside world on real devices.


Lothar Waßmann


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-03 Thread Robin Gong
On 二, 2018-07-03 at 08:10 -0300, Fabio Estevam wrote:
> Hi Anson,
> 
> On Tue, Jul 3, 2018 at 4:44 AM, Anson Huang 
> wrote:
> 
> > 
> > It is NOT easy to identify which switch is critical or NOT, and
> > different platforms
> > have different board design, it will introduce many platform
> > specified code, so I think
> > just revert the pfuze100 switch enable/disable patch should be OK
> > for now.
> I have sent the pfuze100 regulator patch revert and it is linux-next
> now. Should probably reach 4.18-rc4.
> 
> > 
> > After a couple of release cycles, add the pfuze100 switch
> > enable/disable patch
> > back to support this feature, I believe users should switch to new
> > dtb with "regulator-always-on"
> > existing already.
> That will still break old dtb compatibility.
> 
> You cannot force users to use "regulator-always-on" and the old dtbs
> need to always work.
> 
> So whatever new feature you need to introduce it needs to be done in
> such a way that the existing dtb's will continue working.
But actually existing dtb is not right since the critical power rail
missing 'regulator-always-on'. It's a fix patch for dts, not related
with following dtb/kernel break rules, just a simple dts patch. Why
should we make promise for the wrong dtbs?

Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-03 Thread Robin Gong
On 二, 2018-07-03 at 08:10 -0300, Fabio Estevam wrote:
> Hi Anson,
> 
> On Tue, Jul 3, 2018 at 4:44 AM, Anson Huang 
> wrote:
> 
> > 
> > It is NOT easy to identify which switch is critical or NOT, and
> > different platforms
> > have different board design, it will introduce many platform
> > specified code, so I think
> > just revert the pfuze100 switch enable/disable patch should be OK
> > for now.
> I have sent the pfuze100 regulator patch revert and it is linux-next
> now. Should probably reach 4.18-rc4.
> 
> > 
> > After a couple of release cycles, add the pfuze100 switch
> > enable/disable patch
> > back to support this feature, I believe users should switch to new
> > dtb with "regulator-always-on"
> > existing already.
> That will still break old dtb compatibility.
> 
> You cannot force users to use "regulator-always-on" and the old dtbs
> need to always work.
> 
> So whatever new feature you need to introduce it needs to be done in
> such a way that the existing dtb's will continue working.
But actually existing dtb is not right since the critical power rail
missing 'regulator-always-on'. It's a fix patch for dts, not related
with following dtb/kernel break rules, just a simple dts patch. Why
should we make promise for the wrong dtbs?

Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-03 Thread Fabio Estevam
Hi Anson,

On Tue, Jul 3, 2018 at 4:44 AM, Anson Huang  wrote:

> It is NOT easy to identify which switch is critical or NOT, and different 
> platforms
> have different board design, it will introduce many platform specified code, 
> so I think
> just revert the pfuze100 switch enable/disable patch should be OK for now.

I have sent the pfuze100 regulator patch revert and it is linux-next
now. Should probably reach 4.18-rc4.

> After a couple of release cycles, add the pfuze100 switch enable/disable patch
> back to support this feature, I believe users should switch to new dtb with 
> "regulator-always-on"
> existing already.

That will still break old dtb compatibility.

You cannot force users to use "regulator-always-on" and the old dtbs
need to always work.

So whatever new feature you need to introduce it needs to be done in
such a way that the existing dtb's will continue working.


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-03 Thread Fabio Estevam
Hi Anson,

On Tue, Jul 3, 2018 at 4:44 AM, Anson Huang  wrote:

> It is NOT easy to identify which switch is critical or NOT, and different 
> platforms
> have different board design, it will introduce many platform specified code, 
> so I think
> just revert the pfuze100 switch enable/disable patch should be OK for now.

I have sent the pfuze100 regulator patch revert and it is linux-next
now. Should probably reach 4.18-rc4.

> After a couple of release cycles, add the pfuze100 switch enable/disable patch
> back to support this feature, I believe users should switch to new dtb with 
> "regulator-always-on"
> existing already.

That will still break old dtb compatibility.

You cannot force users to use "regulator-always-on" and the old dtbs
need to always work.

So whatever new feature you need to introduce it needs to be done in
such a way that the existing dtb's will continue working.


RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-03 Thread Anson Huang
Hi, Shawn

Anson Huang
Best Regards!


> -Original Message-
> From: Shawn Guo [mailto:shawn...@kernel.org]
> Sent: Tuesday, July 3, 2018 1:39 PM
> To: Robin Gong 
> Cc: feste...@gmail.com; Anson Huang ;
> mark.rutl...@arm.com; devicet...@vger.kernel.org;
> linux-kernel@vger.kernel.org; robh...@kernel.org; dl-linux-imx
> ; ker...@pengutronix.de; Fabio Estevam
> ; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> On Mon, Jul 02, 2018 at 02:12:52AM +, Robin Gong wrote:
> > But in fact, the original dts is not correct without
> > 'regulator-always- on'since SW4 is the critical DDR power rail,
> > although, it's kept on in the previous kernel by no switches
> > enable/disable interfaces provided in pfuze driver. Adding new
> > property which can be done totally by the common 'regulator-always-on'
> > is not a good choice. Keep the dts patch adding 'regulator-always-on'
> > ahead of pfuze driver pach adding enable/disable interface is enough for 
> > such
> case I think.
> 
> We can not just break the installed DTBs like this.  If patching regulator 
> driver
> with a new property is really difficult, we could migrate the existing users 
> in a
> 'soft' way:
 
Patching regulator driver needs to add property for those regulators can be OFF,
it will make users confuse with original regulator framework knowledge, NOT a
good idea.

> 
>  - Add required regulator-always-on for regulator nodes in DTS.
 
I & Yibin already sent out patch to add " regulator-always-on " for regulator 
nodes in DTS,
so they can be applied first?


>  - Patch i.MX platform code to check the presence of regulator-always-on
>property for critical regulators, and give a big warning if it's
>missing.
It is NOT easy to identify which switch is critical or NOT, and different 
platforms
have different board design, it will introduce many platform specified code, so 
I think
just revert the pfuze100 switch enable/disable patch should be OK for now. 

>  - Wait for a couple of release cycles for users to migrate.
>  - Add regulator driver patch back and break users who keep ignoring
>the warning.
After a couple of release cycles, add the pfuze100 switch enable/disable patch
back to support this feature, I believe users should switch to new dtb with 
"regulator-always-on" 
existing already.

Anson.

> 
> Shawn


RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-03 Thread Anson Huang
Hi, Shawn

Anson Huang
Best Regards!


> -Original Message-
> From: Shawn Guo [mailto:shawn...@kernel.org]
> Sent: Tuesday, July 3, 2018 1:39 PM
> To: Robin Gong 
> Cc: feste...@gmail.com; Anson Huang ;
> mark.rutl...@arm.com; devicet...@vger.kernel.org;
> linux-kernel@vger.kernel.org; robh...@kernel.org; dl-linux-imx
> ; ker...@pengutronix.de; Fabio Estevam
> ; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> On Mon, Jul 02, 2018 at 02:12:52AM +, Robin Gong wrote:
> > But in fact, the original dts is not correct without
> > 'regulator-always- on'since SW4 is the critical DDR power rail,
> > although, it's kept on in the previous kernel by no switches
> > enable/disable interfaces provided in pfuze driver. Adding new
> > property which can be done totally by the common 'regulator-always-on'
> > is not a good choice. Keep the dts patch adding 'regulator-always-on'
> > ahead of pfuze driver pach adding enable/disable interface is enough for 
> > such
> case I think.
> 
> We can not just break the installed DTBs like this.  If patching regulator 
> driver
> with a new property is really difficult, we could migrate the existing users 
> in a
> 'soft' way:
 
Patching regulator driver needs to add property for those regulators can be OFF,
it will make users confuse with original regulator framework knowledge, NOT a
good idea.

> 
>  - Add required regulator-always-on for regulator nodes in DTS.
 
I & Yibin already sent out patch to add " regulator-always-on " for regulator 
nodes in DTS,
so they can be applied first?


>  - Patch i.MX platform code to check the presence of regulator-always-on
>property for critical regulators, and give a big warning if it's
>missing.
It is NOT easy to identify which switch is critical or NOT, and different 
platforms
have different board design, it will introduce many platform specified code, so 
I think
just revert the pfuze100 switch enable/disable patch should be OK for now. 

>  - Wait for a couple of release cycles for users to migrate.
>  - Add regulator driver patch back and break users who keep ignoring
>the warning.
After a couple of release cycles, add the pfuze100 switch enable/disable patch
back to support this feature, I believe users should switch to new dtb with 
"regulator-always-on" 
existing already.

Anson.

> 
> Shawn


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-02 Thread Shawn Guo
On Mon, Jul 02, 2018 at 02:12:52AM +, Robin Gong wrote:
> But in fact, the original dts is not correct without 'regulator-always-
> on'since SW4 is the critical DDR power rail, although, it's kept on in
> the previous kernel by no switches enable/disable interfaces provided
> in pfuze driver. Adding new property which can be done totally by the
> common 'regulator-always-on' is not a good choice. Keep the dts patch
> adding 'regulator-always-on' ahead of pfuze driver pach adding
> enable/disable interface is enough for such case I think. 

We can not just break the installed DTBs like this.  If patching
regulator driver with a new property is really difficult, we could
migrate the existing users in a 'soft' way:

 - Add required regulator-always-on for regulator nodes in DTS.
 - Patch i.MX platform code to check the presence of regulator-always-on
   property for critical regulators, and give a big warning if it's
   missing.
 - Wait for a couple of release cycles for users to migrate.
 - Add regulator driver patch back and break users who keep ignoring
   the warning.

Shawn


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-02 Thread Shawn Guo
On Mon, Jul 02, 2018 at 02:12:52AM +, Robin Gong wrote:
> But in fact, the original dts is not correct without 'regulator-always-
> on'since SW4 is the critical DDR power rail, although, it's kept on in
> the previous kernel by no switches enable/disable interfaces provided
> in pfuze driver. Adding new property which can be done totally by the
> common 'regulator-always-on' is not a good choice. Keep the dts patch
> adding 'regulator-always-on' ahead of pfuze driver pach adding
> enable/disable interface is enough for such case I think. 

We can not just break the installed DTBs like this.  If patching
regulator driver with a new property is really difficult, we could
migrate the existing users in a 'soft' way:

 - Add required regulator-always-on for regulator nodes in DTS.
 - Patch i.MX platform code to check the presence of regulator-always-on
   property for critical regulators, and give a big warning if it's
   missing.
 - Wait for a couple of release cycles for users to migrate.
 - Add regulator driver patch back and break users who keep ignoring
   the warning.

Shawn


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-02 Thread Fabio Estevam
Hi Robin,

On Sun, Jul 1, 2018 at 11:12 PM, Robin Gong  wrote:

> But in fact, the original dts is not correct without 'regulator-always-
> on'since SW4 is the critical DDR power rail, although, it's kept on in
> the previous kernel by no switches enable/disable interfaces provided
> in pfuze driver. Adding new property which can be done totally by the
> common 'regulator-always-on' is not a good choice. Keep the dts patch
> adding 'regulator-always-on' ahead of pfuze driver pach adding
> enable/disable interface is enough for such case I think.

We really want to avoid  breaking old dtb's running a new kernel.


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-02 Thread Fabio Estevam
Hi Robin,

On Sun, Jul 1, 2018 at 11:12 PM, Robin Gong  wrote:

> But in fact, the original dts is not correct without 'regulator-always-
> on'since SW4 is the critical DDR power rail, although, it's kept on in
> the previous kernel by no switches enable/disable interfaces provided
> in pfuze driver. Adding new property which can be done totally by the
> common 'regulator-always-on' is not a good choice. Keep the dts patch
> adding 'regulator-always-on' ahead of pfuze driver pach adding
> enable/disable interface is enough for such case I think.

We really want to avoid  breaking old dtb's running a new kernel.


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Robin Gong
On 日, 2018-07-01 at 22:17 -0300, Fabio Estevam wrote:
> On Sun, Jul 1, 2018 at 10:09 PM, Anson Huang 
> wrote:
> 
> > 
> > On some new i.MX platforms, PFuze switches are used for supplying
> > GPU/VPU
> > or other non-critical modules only, these switches need to be
> > turned off by
> > runtime PM to avoid very high power leakage, like on mScale850D.
> Ok, in this case I suggest adding a new property so that the switches
> can be turned off only when the new property is present.
> 
> When this new property is absent, then we keep the current behavior
> and avoid dtb breakage.
> 
> Since MX8M support is not in place yet, this is not urgent, so I will
> send a revert and then you can re-work the patch so that it does not
> affect the old dtbs.
> 
> Do you agree with such approach?
But in fact, the original dts is not correct without 'regulator-always-
on'since SW4 is the critical DDR power rail, although, it's kept on in
the previous kernel by no switches enable/disable interfaces provided
in pfuze driver. Adding new property which can be done totally by the
common 'regulator-always-on' is not a good choice. Keep the dts patch
adding 'regulator-always-on' ahead of pfuze driver pach adding
enable/disable interface is enough for such case I think. 

Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Robin Gong
On 日, 2018-07-01 at 22:17 -0300, Fabio Estevam wrote:
> On Sun, Jul 1, 2018 at 10:09 PM, Anson Huang 
> wrote:
> 
> > 
> > On some new i.MX platforms, PFuze switches are used for supplying
> > GPU/VPU
> > or other non-critical modules only, these switches need to be
> > turned off by
> > runtime PM to avoid very high power leakage, like on mScale850D.
> Ok, in this case I suggest adding a new property so that the switches
> can be turned off only when the new property is present.
> 
> When this new property is absent, then we keep the current behavior
> and avoid dtb breakage.
> 
> Since MX8M support is not in place yet, this is not urgent, so I will
> send a revert and then you can re-work the patch so that it does not
> affect the old dtbs.
> 
> Do you agree with such approach?
But in fact, the original dts is not correct without 'regulator-always-
on'since SW4 is the critical DDR power rail, although, it's kept on in
the previous kernel by no switches enable/disable interfaces provided
in pfuze driver. Adding new property which can be done totally by the
common 'regulator-always-on' is not a good choice. Keep the dts patch
adding 'regulator-always-on' ahead of pfuze driver pach adding
enable/disable interface is enough for such case I think. 

RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Anson Huang


Anson Huang
Best Regards!


> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Monday, July 2, 2018 9:17 AM
> To: Anson Huang 
> Cc: Shawn Guo ; Robin Gong ;
> Mark Rutland ; open list:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS ;
> linux-kernel ; Rob Herring
> ; dl-linux-imx ; Sascha Hauer
> ; Fabio Estevam ;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> 
> Subject: Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> On Sun, Jul 1, 2018 at 10:09 PM, Anson Huang 
> wrote:
> 
> > On some new i.MX platforms, PFuze switches are used for supplying
> > GPU/VPU or other non-critical modules only, these switches need to be
> > turned off by runtime PM to avoid very high power leakage, like on
> mScale850D.
> 
> Ok, in this case I suggest adding a new property so that the switches can be
> turned off only when the new property is present.
> 
> When this new property is absent, then we keep the current behavior and avoid
> dtb breakage.
> 
> Since MX8M support is not in place yet, this is not urgent, so I will send a 
> revert
> and then you can re-work the patch so that it does not affect the old dtbs.
> 
> Do you agree with such approach?

Sure, I agree for now. As I did NOT want to have any breakage. Thanks.
 
Anson.




RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Anson Huang


Anson Huang
Best Regards!


> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Monday, July 2, 2018 9:17 AM
> To: Anson Huang 
> Cc: Shawn Guo ; Robin Gong ;
> Mark Rutland ; open list:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS ;
> linux-kernel ; Rob Herring
> ; dl-linux-imx ; Sascha Hauer
> ; Fabio Estevam ;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> 
> Subject: Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> On Sun, Jul 1, 2018 at 10:09 PM, Anson Huang 
> wrote:
> 
> > On some new i.MX platforms, PFuze switches are used for supplying
> > GPU/VPU or other non-critical modules only, these switches need to be
> > turned off by runtime PM to avoid very high power leakage, like on
> mScale850D.
> 
> Ok, in this case I suggest adding a new property so that the switches can be
> turned off only when the new property is present.
> 
> When this new property is absent, then we keep the current behavior and avoid
> dtb breakage.
> 
> Since MX8M support is not in place yet, this is not urgent, so I will send a 
> revert
> and then you can re-work the patch so that it does not affect the old dtbs.
> 
> Do you agree with such approach?

Sure, I agree for now. As I did NOT want to have any breakage. Thanks.
 
Anson.




Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Fabio Estevam
On Sun, Jul 1, 2018 at 10:09 PM, Anson Huang  wrote:

> On some new i.MX platforms, PFuze switches are used for supplying GPU/VPU
> or other non-critical modules only, these switches need to be turned off by
> runtime PM to avoid very high power leakage, like on mScale850D.

Ok, in this case I suggest adding a new property so that the switches
can be turned off only when the new property is present.

When this new property is absent, then we keep the current behavior
and avoid dtb breakage.

Since MX8M support is not in place yet, this is not urgent, so I will
send a revert and then you can re-work the patch so that it does not
affect the old dtbs.

Do you agree with such approach?


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Fabio Estevam
On Sun, Jul 1, 2018 at 10:09 PM, Anson Huang  wrote:

> On some new i.MX platforms, PFuze switches are used for supplying GPU/VPU
> or other non-critical modules only, these switches need to be turned off by
> runtime PM to avoid very high power leakage, like on mScale850D.

Ok, in this case I suggest adding a new property so that the switches
can be turned off only when the new property is present.

When this new property is absent, then we keep the current behavior
and avoid dtb breakage.

Since MX8M support is not in place yet, this is not urgent, so I will
send a revert and then you can re-work the patch so that it does not
affect the old dtbs.

Do you agree with such approach?


RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Anson Huang


Anson Huang
Best Regards!


> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Monday, July 2, 2018 9:05 AM
> To: Anson Huang 
> Cc: Shawn Guo ; Robin Gong ;
> Mark Rutland ; open list:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS ;
> linux-kernel ; Rob Herring
> ; dl-linux-imx ; Sascha Hauer
> ; Fabio Estevam ;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> 
> Subject: Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> On Sun, Jul 1, 2018 at 10:03 PM, Anson Huang 
> wrote:
> 
> > So that mean such kind of kernel patch will never be into kernel? Even
> > if it is a necessary patch for fixing some other issues? I just wonder
> > how this case being handled.
> 
> What is the issue that 5fe156f1cab4f fixes? It is not clear by looking at the
> commit log.

On some new i.MX platforms, PFuze switches are used for supplying GPU/VPU
or other non-critical modules only, these switches need to be turned off by
runtime PM to avoid very high power leakage, like on mScale850D.

Anson



RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Anson Huang


Anson Huang
Best Regards!


> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Monday, July 2, 2018 9:05 AM
> To: Anson Huang 
> Cc: Shawn Guo ; Robin Gong ;
> Mark Rutland ; open list:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS ;
> linux-kernel ; Rob Herring
> ; dl-linux-imx ; Sascha Hauer
> ; Fabio Estevam ;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> 
> Subject: Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> On Sun, Jul 1, 2018 at 10:03 PM, Anson Huang 
> wrote:
> 
> > So that mean such kind of kernel patch will never be into kernel? Even
> > if it is a necessary patch for fixing some other issues? I just wonder
> > how this case being handled.
> 
> What is the issue that 5fe156f1cab4f fixes? It is not clear by looking at the
> commit log.

On some new i.MX platforms, PFuze switches are used for supplying GPU/VPU
or other non-critical modules only, these switches need to be turned off by
runtime PM to avoid very high power leakage, like on mScale850D.

Anson



Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Fabio Estevam
On Sun, Jul 1, 2018 at 10:03 PM, Anson Huang  wrote:

> So that mean such kind of kernel patch will never be into kernel? Even if it 
> is a
> necessary patch for fixing some other issues? I just wonder how this case 
> being
> handled.

What is the issue that 5fe156f1cab4f fixes? It is not clear by looking
at the commit log.


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Fabio Estevam
On Sun, Jul 1, 2018 at 10:03 PM, Anson Huang  wrote:

> So that mean such kind of kernel patch will never be into kernel? Even if it 
> is a
> necessary patch for fixing some other issues? I just wonder how this case 
> being
> handled.

What is the issue that 5fe156f1cab4f fixes? It is not clear by looking
at the commit log.


RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Anson Huang


Anson Huang
Best Regards!


> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Monday, July 2, 2018 9:00 AM
> To: Anson Huang 
> Cc: Shawn Guo ; Robin Gong ;
> Mark Rutland ; open list:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS ;
> linux-kernel ; Rob Herring
> ; dl-linux-imx ; Sascha Hauer
> ; Fabio Estevam ;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> 
> Subject: Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> Hi Anson,
> 
> On Sun, Jul 1, 2018 at 9:57 PM, Anson Huang  wrote:
> 
> > Just want to know how to handle such case? The kernel patch will never
> > be applied or is there any way to make kernel patch and dtb patch
> > applied together to avoid any breakage?
> 
> We always want to avoid breaking a working dtb when it is used with a newer
> kernel.
> 
> In this case we need to revert the kernel patch as it causes regression with 
> old
> dtbs.
 
So that mean such kind of kernel patch will never be into kernel? Even if it is 
a
necessary patch for fixing some other issues? I just wonder how this case being
handled.

Anson.


RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Anson Huang


Anson Huang
Best Regards!


> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Monday, July 2, 2018 9:00 AM
> To: Anson Huang 
> Cc: Shawn Guo ; Robin Gong ;
> Mark Rutland ; open list:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS ;
> linux-kernel ; Rob Herring
> ; dl-linux-imx ; Sascha Hauer
> ; Fabio Estevam ;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> 
> Subject: Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> Hi Anson,
> 
> On Sun, Jul 1, 2018 at 9:57 PM, Anson Huang  wrote:
> 
> > Just want to know how to handle such case? The kernel patch will never
> > be applied or is there any way to make kernel patch and dtb patch
> > applied together to avoid any breakage?
> 
> We always want to avoid breaking a working dtb when it is used with a newer
> kernel.
> 
> In this case we need to revert the kernel patch as it causes regression with 
> old
> dtbs.
 
So that mean such kind of kernel patch will never be into kernel? Even if it is 
a
necessary patch for fixing some other issues? I just wonder how this case being
handled.

Anson.


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Fabio Estevam
Hi Anson,

On Sun, Jul 1, 2018 at 9:57 PM, Anson Huang  wrote:

> Just want to know how to handle such case? The kernel patch will never
> be applied or is there any way to make kernel patch and dtb patch applied
> together to avoid any breakage?

We always want to avoid breaking a working dtb when it is used with a
newer kernel.

In this case we need to revert the kernel patch as it causes
regression with old dtbs.


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Fabio Estevam
Hi Anson,

On Sun, Jul 1, 2018 at 9:57 PM, Anson Huang  wrote:

> Just want to know how to handle such case? The kernel patch will never
> be applied or is there any way to make kernel patch and dtb patch applied
> together to avoid any breakage?

We always want to avoid breaking a working dtb when it is used with a
newer kernel.

In this case we need to revert the kernel patch as it causes
regression with old dtbs.


RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Anson Huang
Hi, Fabio

Anson Huang
Best Regards!


> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Monday, July 2, 2018 8:55 AM
> To: Anson Huang 
> Cc: Shawn Guo ; Robin Gong ;
> Mark Rutland ; open list:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS ;
> linux-kernel ; Rob Herring
> ; dl-linux-imx ; Sascha Hauer
> ; Fabio Estevam ;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> 
> Subject: Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> Hi Anson,
> 
> On Sun, Jul 1, 2018 at 9:53 PM, Anson Huang  wrote:
> 
> > Yes, I think we can revert it to avoid breakage. Didn't notice that
> > some i.MX platform do NOT have those critical switches always-on.
> 
> Ok, thanks for confirming.
> 
> I will send a revert patch then.
> 
> Thanks
 
Just want to know how to handle such case? The kernel patch will never
be applied or is there any way to make kernel patch and dtb patch applied
together to avoid any breakage?

Anson.



RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Anson Huang
Hi, Fabio

Anson Huang
Best Regards!


> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Monday, July 2, 2018 8:55 AM
> To: Anson Huang 
> Cc: Shawn Guo ; Robin Gong ;
> Mark Rutland ; open list:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS ;
> linux-kernel ; Rob Herring
> ; dl-linux-imx ; Sascha Hauer
> ; Fabio Estevam ;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> 
> Subject: Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> Hi Anson,
> 
> On Sun, Jul 1, 2018 at 9:53 PM, Anson Huang  wrote:
> 
> > Yes, I think we can revert it to avoid breakage. Didn't notice that
> > some i.MX platform do NOT have those critical switches always-on.
> 
> Ok, thanks for confirming.
> 
> I will send a revert patch then.
> 
> Thanks
 
Just want to know how to handle such case? The kernel patch will never
be applied or is there any way to make kernel patch and dtb patch applied
together to avoid any breakage?

Anson.



Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Fabio Estevam
Hi Anson,

On Sun, Jul 1, 2018 at 9:53 PM, Anson Huang  wrote:

> Yes, I think we can revert it to avoid breakage. Didn't notice that some
> i.MX platform do NOT have those critical switches always-on.

Ok, thanks for confirming.

I will send a revert patch then.

Thanks


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Fabio Estevam
Hi Anson,

On Sun, Jul 1, 2018 at 9:53 PM, Anson Huang  wrote:

> Yes, I think we can revert it to avoid breakage. Didn't notice that some
> i.MX platform do NOT have those critical switches always-on.

Ok, thanks for confirming.

I will send a revert patch then.

Thanks


RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Anson Huang


Anson Huang
Best Regards!


> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Monday, July 2, 2018 7:33 AM
> To: Shawn Guo ; Anson Huang
> 
> Cc: Robin Gong ; Mark Rutland
> ; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS ; linux-kernel
> ; Rob Herring ;
> dl-linux-imx ; Sascha Hauer ;
> Fabio Estevam ; moderated list:ARM/FREESCALE
> IMX / MXC ARM ARCHITECTURE 
> Subject: Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> On Sun, Jul 1, 2018 at 6:34 AM, Shawn Guo  wrote:
> > On Mon, Jun 25, 2018 at 08:34:11PM +0800, Robin Gong wrote:
> >> SW4 is one power rail for LPDDR2 on i.mx6sl-evk, so it should be kept
> >> always on. But it's disabled after switch disabled interface
> >> implemented in pfuze driver 'commit 5fe156f1cab4
> >> ("regulator: pfuze100: add enable/disable for switch")'.Thus, it
> >> breaks kernel bootup. Add 'regulator-always-on' for SW4.
> >>
> >> Signed-off-by: Robin Gong 
> >
> > Does that mean boards with existing DTB installed will stop working
> > with new kernel?  That's bad, and the kernel commit should probably be
> > reverted.
> 
> Yes, this is a good point.
> 
> Anson,
> 
> Should 5fe156f1cab4 ("regulator: pfuze100: add enable/disable for
> switch") be reverted to avoid such breakage?
 
Yes, I think we can revert it to avoid breakage. Didn't notice that some
i.MX platform do NOT have those critical switches always-on.

Anson.



RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Anson Huang


Anson Huang
Best Regards!


> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Monday, July 2, 2018 7:33 AM
> To: Shawn Guo ; Anson Huang
> 
> Cc: Robin Gong ; Mark Rutland
> ; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS ; linux-kernel
> ; Rob Herring ;
> dl-linux-imx ; Sascha Hauer ;
> Fabio Estevam ; moderated list:ARM/FREESCALE
> IMX / MXC ARM ARCHITECTURE 
> Subject: Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> On Sun, Jul 1, 2018 at 6:34 AM, Shawn Guo  wrote:
> > On Mon, Jun 25, 2018 at 08:34:11PM +0800, Robin Gong wrote:
> >> SW4 is one power rail for LPDDR2 on i.mx6sl-evk, so it should be kept
> >> always on. But it's disabled after switch disabled interface
> >> implemented in pfuze driver 'commit 5fe156f1cab4
> >> ("regulator: pfuze100: add enable/disable for switch")'.Thus, it
> >> breaks kernel bootup. Add 'regulator-always-on' for SW4.
> >>
> >> Signed-off-by: Robin Gong 
> >
> > Does that mean boards with existing DTB installed will stop working
> > with new kernel?  That's bad, and the kernel commit should probably be
> > reverted.
> 
> Yes, this is a good point.
> 
> Anson,
> 
> Should 5fe156f1cab4 ("regulator: pfuze100: add enable/disable for
> switch") be reverted to avoid such breakage?
 
Yes, I think we can revert it to avoid breakage. Didn't notice that some
i.MX platform do NOT have those critical switches always-on.

Anson.



Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Fabio Estevam
On Sun, Jul 1, 2018 at 6:34 AM, Shawn Guo  wrote:
> On Mon, Jun 25, 2018 at 08:34:11PM +0800, Robin Gong wrote:
>> SW4 is one power rail for LPDDR2 on i.mx6sl-evk, so it should
>> be kept always on. But it's disabled after switch disabled
>> interface implemented in pfuze driver
>> 'commit 5fe156f1cab4
>> ("regulator: pfuze100: add enable/disable for switch")'.Thus,
>> it breaks kernel bootup. Add 'regulator-always-on' for SW4.
>>
>> Signed-off-by: Robin Gong 
>
> Does that mean boards with existing DTB installed will stop working with
> new kernel?  That's bad, and the kernel commit should probably be
> reverted.

Yes, this is a good point.

Anson,

Should 5fe156f1cab4 ("regulator: pfuze100: add enable/disable for
switch") be reverted to avoid such breakage?


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Fabio Estevam
On Sun, Jul 1, 2018 at 6:34 AM, Shawn Guo  wrote:
> On Mon, Jun 25, 2018 at 08:34:11PM +0800, Robin Gong wrote:
>> SW4 is one power rail for LPDDR2 on i.mx6sl-evk, so it should
>> be kept always on. But it's disabled after switch disabled
>> interface implemented in pfuze driver
>> 'commit 5fe156f1cab4
>> ("regulator: pfuze100: add enable/disable for switch")'.Thus,
>> it breaks kernel bootup. Add 'regulator-always-on' for SW4.
>>
>> Signed-off-by: Robin Gong 
>
> Does that mean boards with existing DTB installed will stop working with
> new kernel?  That's bad, and the kernel commit should probably be
> reverted.

Yes, this is a good point.

Anson,

Should 5fe156f1cab4 ("regulator: pfuze100: add enable/disable for
switch") be reverted to avoid such breakage?


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Shawn Guo
On Mon, Jun 25, 2018 at 08:34:11PM +0800, Robin Gong wrote:
> SW4 is one power rail for LPDDR2 on i.mx6sl-evk, so it should
> be kept always on. But it's disabled after switch disabled
> interface implemented in pfuze driver
> 'commit 5fe156f1cab4
> ("regulator: pfuze100: add enable/disable for switch")'.Thus,
> it breaks kernel bootup. Add 'regulator-always-on' for SW4.
> 
> Signed-off-by: Robin Gong 

Does that mean boards with existing DTB installed will stop working with
new kernel?  That's bad, and the kernel commit should probably be
reverted.

Shawn

> ---
>  arch/arm/boot/dts/imx6sl-evk.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/imx6sl-evk.dts 
> b/arch/arm/boot/dts/imx6sl-evk.dts
> index 37e792f..798df66 100644
> --- a/arch/arm/boot/dts/imx6sl-evk.dts
> +++ b/arch/arm/boot/dts/imx6sl-evk.dts
> @@ -199,6 +199,7 @@
>   sw4_reg: sw4 {
>   regulator-min-microvolt = <80>;
>   regulator-max-microvolt = <330>;
> + regulator-always-on;
>   };
>  
>   swbst_reg: swbst {
> -- 
> 2.7.4
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


Re: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-07-01 Thread Shawn Guo
On Mon, Jun 25, 2018 at 08:34:11PM +0800, Robin Gong wrote:
> SW4 is one power rail for LPDDR2 on i.mx6sl-evk, so it should
> be kept always on. But it's disabled after switch disabled
> interface implemented in pfuze driver
> 'commit 5fe156f1cab4
> ("regulator: pfuze100: add enable/disable for switch")'.Thus,
> it breaks kernel bootup. Add 'regulator-always-on' for SW4.
> 
> Signed-off-by: Robin Gong 

Does that mean boards with existing DTB installed will stop working with
new kernel?  That's bad, and the kernel commit should probably be
reverted.

Shawn

> ---
>  arch/arm/boot/dts/imx6sl-evk.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/imx6sl-evk.dts 
> b/arch/arm/boot/dts/imx6sl-evk.dts
> index 37e792f..798df66 100644
> --- a/arch/arm/boot/dts/imx6sl-evk.dts
> +++ b/arch/arm/boot/dts/imx6sl-evk.dts
> @@ -199,6 +199,7 @@
>   sw4_reg: sw4 {
>   regulator-min-microvolt = <80>;
>   regulator-max-microvolt = <330>;
> + regulator-always-on;
>   };
>  
>   swbst_reg: swbst {
> -- 
> 2.7.4
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-06-24 Thread Anson Huang
Acked-by: Anson Huang 

I also saw such issue on i.MX6SLL evk board, and also sent out patch for 
i.MX6SLL.

Anson Huang
Best Regards!


> -Original Message-
> From: Robin Gong
> Sent: Monday, June 25, 2018 8:34 PM
> To: shawn...@kernel.org; ker...@pengutronix.de; Fabio Estevam
> ; robh...@kernel.org; mark.rutl...@arm.com;
> Anson Huang 
> Cc: linux-arm-ker...@lists.infradead.org; devicet...@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx 
> Subject: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> SW4 is one power rail for LPDDR2 on i.mx6sl-evk, so it should be kept always 
> on.
> But it's disabled after switch disabled interface implemented in pfuze driver
> 'commit 5fe156f1cab4
> ("regulator: pfuze100: add enable/disable for switch")'.Thus, it breaks kernel
> bootup. Add 'regulator-always-on' for SW4.
> 
> Signed-off-by: Robin Gong 
> ---
>  arch/arm/boot/dts/imx6sl-evk.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/imx6sl-evk.dts
> b/arch/arm/boot/dts/imx6sl-evk.dts
> index 37e792f..798df66 100644
> --- a/arch/arm/boot/dts/imx6sl-evk.dts
> +++ b/arch/arm/boot/dts/imx6sl-evk.dts
> @@ -199,6 +199,7 @@
>   sw4_reg: sw4 {
>   regulator-min-microvolt = <80>;
>   regulator-max-microvolt = <330>;
> + regulator-always-on;
>   };
> 
>   swbst_reg: swbst {
> --
> 2.7.4



RE: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on

2018-06-24 Thread Anson Huang
Acked-by: Anson Huang 

I also saw such issue on i.MX6SLL evk board, and also sent out patch for 
i.MX6SLL.

Anson Huang
Best Regards!


> -Original Message-
> From: Robin Gong
> Sent: Monday, June 25, 2018 8:34 PM
> To: shawn...@kernel.org; ker...@pengutronix.de; Fabio Estevam
> ; robh...@kernel.org; mark.rutl...@arm.com;
> Anson Huang 
> Cc: linux-arm-ker...@lists.infradead.org; devicet...@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx 
> Subject: [PATCH v1] ARM: dts: imx6sl-evk: keep sw4 always on
> 
> SW4 is one power rail for LPDDR2 on i.mx6sl-evk, so it should be kept always 
> on.
> But it's disabled after switch disabled interface implemented in pfuze driver
> 'commit 5fe156f1cab4
> ("regulator: pfuze100: add enable/disable for switch")'.Thus, it breaks kernel
> bootup. Add 'regulator-always-on' for SW4.
> 
> Signed-off-by: Robin Gong 
> ---
>  arch/arm/boot/dts/imx6sl-evk.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/imx6sl-evk.dts
> b/arch/arm/boot/dts/imx6sl-evk.dts
> index 37e792f..798df66 100644
> --- a/arch/arm/boot/dts/imx6sl-evk.dts
> +++ b/arch/arm/boot/dts/imx6sl-evk.dts
> @@ -199,6 +199,7 @@
>   sw4_reg: sw4 {
>   regulator-min-microvolt = <80>;
>   regulator-max-microvolt = <330>;
> + regulator-always-on;
>   };
> 
>   swbst_reg: swbst {
> --
> 2.7.4