mbox series

[RESEND,v6,0/6] provide power off support for iMX6 with external PMIC

Message ID 20180305102524.5905-1-o.rempel@pengutronix.de
Headers show
Series provide power off support for iMX6 with external PMIC | expand

Message

Oleksij Rempel March 5, 2018, 10:25 a.m. UTC
This patch series is providing power off support for Freescale/NXP iMX6 based
boards with external power management integrated circuit (PMIC).
As a first step the PMIC is configured to turn off the system if the
standby pin is asserted. On second step we assert the standby pin.
For this reason we need to use pm_power_off_prepare.

Usage of stnadby pin for power off is described in official iMX6
documentation.

2018.03.05:
As this patch set touches multiple subsystems I think it would make
sense for Shawn Guo to take the all patch set.
The only part which didn't receive an ACK is regulator stuff. So I would
hope that Mark Brown can ACK it.

Kind regards,
Oleksij Rempel

2017.12.06:
Adding Linus. Probably there is no maintainer for this patch set.
No changes are made, tested on v4.15-rc1.

2017.10.27:
Last version of this patch set was send at 20 Jun 2017, this is a rebase against
kernel v4.14-rc6. Probably this set got lost. If I forgot to address some comments,
please point me.

changes:
v6:
 - rename imx6_pm_poweroff to imx6_pm_stby_poweroff
 - fix "MPIC_STBY_REQ" typo in the comment.

v5:
 - remove useless includes from pm-imx6.c patch
 - add Acked-by to "regulator: pfuze100: add fsl,pmic-stby-poweroff property"
   patch

v4:
 - update comment in "regulator: pfuze100: add fsl,pmic-stby-poweroff ..."
   patch
 - add Acked-by to "ARM: imx6q: provide documentation for new ..."
   patch

v3:
 - set pm_power_off_prepare = NULL on .remove.
 - documentation and spelling fixes.
 - use %pf instead of lookup_symbol_name.


Oleksij Rempel (6):
  ARM: imx6q: provide documentation for new fsl,pmic-stby-poweroff
    property
  ARM: imx6: register pm_power_off handler if "fsl,pmic-stby-poweroff"
    is set
  kernel/reboot.c: export pm_power_off_prepare
  regulator: pfuze100: add fsl,pmic-stby-poweroff property
  regulator: pfuze100-regulator: provide pm_power_off_prepare handler
  ARM: dts: imx6: RIoTboard provide standby on power off option

 .../devicetree/bindings/clock/imx6q-clock.txt      |  8 ++
 .../devicetree/bindings/regulator/pfuze100.txt     |  7 ++
 arch/arm/boot/dts/imx6dl-riotboard.dts             |  5 ++
 arch/arm/mach-imx/pm-imx6.c                        | 25 ++++++
 drivers/regulator/pfuze100-regulator.c             | 92 ++++++++++++++++++++++
 kernel/reboot.c                                    |  1 +
 6 files changed, 138 insertions(+)

Comments

Oleksij Rempel April 3, 2018, 5:59 a.m. UTC | #1
ping,

this week is a merge window. Any chance to get it in to 4.17?

On 05.03.2018 11:25, Oleksij Rempel wrote:
> This patch series is providing power off support for Freescale/NXP iMX6 based
> boards with external power management integrated circuit (PMIC).
> As a first step the PMIC is configured to turn off the system if the
> standby pin is asserted. On second step we assert the standby pin.
> For this reason we need to use pm_power_off_prepare.
> 
> Usage of stnadby pin for power off is described in official iMX6
> documentation.
> 
> 2018.03.05:
> As this patch set touches multiple subsystems I think it would make
> sense for Shawn Guo to take the all patch set.
> The only part which didn't receive an ACK is regulator stuff. So I would
> hope that Mark Brown can ACK it.
> 
> Kind regards,
> Oleksij Rempel
> 
> 2017.12.06:
> Adding Linus. Probably there is no maintainer for this patch set.
> No changes are made, tested on v4.15-rc1.
> 
> 2017.10.27:
> Last version of this patch set was send at 20 Jun 2017, this is a rebase against
> kernel v4.14-rc6. Probably this set got lost. If I forgot to address some comments,
> please point me.
> 
> changes:
> v6:
>  - rename imx6_pm_poweroff to imx6_pm_stby_poweroff
>  - fix "MPIC_STBY_REQ" typo in the comment.
> 
> v5:
>  - remove useless includes from pm-imx6.c patch
>  - add Acked-by to "regulator: pfuze100: add fsl,pmic-stby-poweroff property"
>    patch
> 
> v4:
>  - update comment in "regulator: pfuze100: add fsl,pmic-stby-poweroff ..."
>    patch
>  - add Acked-by to "ARM: imx6q: provide documentation for new ..."
>    patch
> 
> v3:
>  - set pm_power_off_prepare = NULL on .remove.
>  - documentation and spelling fixes.
>  - use %pf instead of lookup_symbol_name.
> 
> 
> Oleksij Rempel (6):
>   ARM: imx6q: provide documentation for new fsl,pmic-stby-poweroff
>     property
>   ARM: imx6: register pm_power_off handler if "fsl,pmic-stby-poweroff"
>     is set
>   kernel/reboot.c: export pm_power_off_prepare
>   regulator: pfuze100: add fsl,pmic-stby-poweroff property
>   regulator: pfuze100-regulator: provide pm_power_off_prepare handler
>   ARM: dts: imx6: RIoTboard provide standby on power off option
> 
>  .../devicetree/bindings/clock/imx6q-clock.txt      |  8 ++
>  .../devicetree/bindings/regulator/pfuze100.txt     |  7 ++
>  arch/arm/boot/dts/imx6dl-riotboard.dts             |  5 ++
>  arch/arm/mach-imx/pm-imx6.c                        | 25 ++++++
>  drivers/regulator/pfuze100-regulator.c             | 92 ++++++++++++++++++++++
>  kernel/reboot.c                                    |  1 +
>  6 files changed, 138 insertions(+)
>
Shawn Guo April 9, 2018, 11:37 a.m. UTC | #2
On Mon, Mar 05, 2018 at 11:25:17AM +0100, Oleksij Rempel wrote:
> This patch series is providing power off support for Freescale/NXP iMX6 based
> boards with external power management integrated circuit (PMIC).
> As a first step the PMIC is configured to turn off the system if the
> standby pin is asserted. On second step we assert the standby pin.
> For this reason we need to use pm_power_off_prepare.
> 
> Usage of stnadby pin for power off is described in official iMX6
> documentation.
> 
> 2018.03.05:
> As this patch set touches multiple subsystems I think it would make
> sense for Shawn Guo to take the all patch set.
> The only part which didn't receive an ACK is regulator stuff. So I would
> hope that Mark Brown can ACK it.

Besides regulator changes, I did not see an ACK on kernel/reboot.c
either.

Shawn
Mark Brown April 9, 2018, 11:40 a.m. UTC | #3
On Mon, Apr 09, 2018 at 07:37:45PM +0800, Shawn Guo wrote:
> On Mon, Mar 05, 2018 at 11:25:17AM +0100, Oleksij Rempel wrote:

> > As this patch set touches multiple subsystems I think it would make
> > sense for Shawn Guo to take the all patch set.
> > The only part which didn't receive an ACK is regulator stuff. So I would
> > hope that Mark Brown can ACK it.

> Besides regulator changes, I did not see an ACK on kernel/reboot.c
> either.

Right, that's the big blocker here.
Oleksij Rempel April 9, 2018, 11:43 a.m. UTC | #4
On 09.04.2018 13:40, Mark Brown wrote:
> On Mon, Apr 09, 2018 at 07:37:45PM +0800, Shawn Guo wrote:
>> On Mon, Mar 05, 2018 at 11:25:17AM +0100, Oleksij Rempel wrote:
> 
>>> As this patch set touches multiple subsystems I think it would make
>>> sense for Shawn Guo to take the all patch set.
>>> The only part which didn't receive an ACK is regulator stuff. So I would
>>> hope that Mark Brown can ACK it.
> 
>> Besides regulator changes, I did not see an ACK on kernel/reboot.c
>> either.
> 
> Right, that's the big blocker here.
> 

Ok, Who should ACK it?
Oleksij Rempel May 4, 2018, 6:50 p.m. UTC | #5
Hallo Andrew,
I need your ACK or NACK for this patch.

This function is used to configure external PMIC to interpret
signal which will be triggered by pm_power_off as power off.
Since same signal can be used for stand by, I linked PMIC configuration
with pm_power_off_prepare to avoid possible conflicts.

On Mon, Mar 05, 2018 at 11:25:20AM +0100, Oleksij Rempel wrote:
> Export pm_power_off_prepare. It is needed to implement power off on
> Freescale/NXP iMX6 based boards with external power management
> integrated circuit (PMIC).
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  kernel/reboot.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/reboot.c b/kernel/reboot.c
> index e4ced883d8de..350be6baa60d 100644
> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -49,6 +49,7 @@ int reboot_force;
>   */
>  
>  void (*pm_power_off_prepare)(void);
> +EXPORT_SYMBOL(pm_power_off_prepare);
>  
>  /**
>   *	emergency_restart - reboot the system
> -- 
> 2.16.1
> 
>
Andrew Morton May 4, 2018, 8:49 p.m. UTC | #6
On Fri, 4 May 2018 20:50:52 +0200 Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Hallo Andrew,
> I need your ACK or NACK for this patch.
> 
> This function is used to configure external PMIC to interpret
> signal which will be triggered by pm_power_off as power off.
> Since same signal can be used for stand by, I linked PMIC configuration
> with pm_power_off_prepare to avoid possible conflicts.
> 
> ...
>
> > --- a/kernel/reboot.c
> > +++ b/kernel/reboot.c
> > @@ -49,6 +49,7 @@ int reboot_force;
> >   */
> >  
> >  void (*pm_power_off_prepare)(void);
> > +EXPORT_SYMBOL(pm_power_off_prepare);
> >  

OK by me, but it's more of a Rafael thing.
Oleksij Rempel May 5, 2018, 7:01 a.m. UTC | #7
Moving Rafael from Cc to To.

Hi Rafael,

can you please help here?
I need your ACK or NACK for this patch.

On Fri, May 04, 2018 at 01:49:56PM -0700, Andrew Morton wrote:
> On Fri, 4 May 2018 20:50:52 +0200 Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > Hallo Andrew,
> > I need your ACK or NACK for this patch.
> > 
> > This function is used to configure external PMIC to interpret
> > signal which will be triggered by pm_power_off as power off.
> > Since same signal can be used for stand by, I linked PMIC configuration
> > with pm_power_off_prepare to avoid possible conflicts.
> > 
> > ...
> >
> > > --- a/kernel/reboot.c
> > > +++ b/kernel/reboot.c
> > > @@ -49,6 +49,7 @@ int reboot_force;
> > >   */
> > >  
> > >  void (*pm_power_off_prepare)(void);
> > > +EXPORT_SYMBOL(pm_power_off_prepare);
> > >  
> 
> OK by me, but it's more of a Rafael thing.
>
Wysocki, Rafael J May 8, 2018, 9:39 p.m. UTC | #8
On 5/5/2018 9:01 AM, Oleksij Rempel wrote:
> Moving Rafael from Cc to To.
>
> Hi Rafael,
>
> can you please help here?
> I need your ACK or NACK for this patch.

Sorry for the delay, I'll have a closer look at it in the next couple of 
days.

Thanks,
Rafael


> On Fri, May 04, 2018 at 01:49:56PM -0700, Andrew Morton wrote:
>> On Fri, 4 May 2018 20:50:52 +0200 Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>>
>>> Hallo Andrew,
>>> I need your ACK or NACK for this patch.
>>>
>>> This function is used to configure external PMIC to interpret
>>> signal which will be triggered by pm_power_off as power off.
>>> Since same signal can be used for stand by, I linked PMIC configuration
>>> with pm_power_off_prepare to avoid possible conflicts.
>>>
>>> ...
>>>
>>>> --- a/kernel/reboot.c
>>>> +++ b/kernel/reboot.c
>>>> @@ -49,6 +49,7 @@ int reboot_force;
>>>>    */
>>>>   
>>>>   void (*pm_power_off_prepare)(void);
>>>> +EXPORT_SYMBOL(pm_power_off_prepare);
>>>>   
>> OK by me, but it's more of a Rafael thing.
>>
Rafael J. Wysocki May 12, 2018, 11:13 a.m. UTC | #9
On Friday, May 4, 2018 8:50:52 PM CEST Oleksij Rempel wrote:
> Hallo Andrew,
> I need your ACK or NACK for this patch.
> 
> This function is used to configure external PMIC to interpret
> signal which will be triggered by pm_power_off as power off.
> Since same signal can be used for stand by, I linked PMIC configuration
> with pm_power_off_prepare to avoid possible conflicts.
> 
> On Mon, Mar 05, 2018 at 11:25:20AM +0100, Oleksij Rempel wrote:
> > Export pm_power_off_prepare. It is needed to implement power off on
> > Freescale/NXP iMX6 based boards with external power management
> > integrated circuit (PMIC).
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  kernel/reboot.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/kernel/reboot.c b/kernel/reboot.c
> > index e4ced883d8de..350be6baa60d 100644
> > --- a/kernel/reboot.c
> > +++ b/kernel/reboot.c
> > @@ -49,6 +49,7 @@ int reboot_force;
> >   */
> >  
> >  void (*pm_power_off_prepare)(void);
> > +EXPORT_SYMBOL(pm_power_off_prepare);

Why not EXPORT_SYMBOL_GPL() ?

> >  
> >  /**
> >   *	emergency_restart - reboot the system
> 
>
Oleksij Rempel May 14, 2018, 4:33 a.m. UTC | #10
On 12.05.2018 13:13, Rafael J. Wysocki wrote:
> On Friday, May 4, 2018 8:50:52 PM CEST Oleksij Rempel wrote:
>> Hallo Andrew,
>> I need your ACK or NACK for this patch.
>>
>> This function is used to configure external PMIC to interpret
>> signal which will be triggered by pm_power_off as power off.
>> Since same signal can be used for stand by, I linked PMIC configuration
>> with pm_power_off_prepare to avoid possible conflicts.
>>
>> On Mon, Mar 05, 2018 at 11:25:20AM +0100, Oleksij Rempel wrote:
>>> Export pm_power_off_prepare. It is needed to implement power off on
>>> Freescale/NXP iMX6 based boards with external power management
>>> integrated circuit (PMIC).
>>>
>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>> ---
>>>  kernel/reboot.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/kernel/reboot.c b/kernel/reboot.c
>>> index e4ced883d8de..350be6baa60d 100644
>>> --- a/kernel/reboot.c
>>> +++ b/kernel/reboot.c
>>> @@ -49,6 +49,7 @@ int reboot_force;
>>>   */
>>>  
>>>  void (*pm_power_off_prepare)(void);
>>> +EXPORT_SYMBOL(pm_power_off_prepare);
> 
> Why not EXPORT_SYMBOL_GPL() ?

No special reason. Fixed.
Any other comments?
Oleksij Rempel May 14, 2018, 5:10 a.m. UTC | #11
On 14.05.2018 06:33, Oleksij Rempel wrote:
> 
> 
> On 12.05.2018 13:13, Rafael J. Wysocki wrote:
>> On Friday, May 4, 2018 8:50:52 PM CEST Oleksij Rempel wrote:
>>> Hallo Andrew,
>>> I need your ACK or NACK for this patch.
>>>
>>> This function is used to configure external PMIC to interpret
>>> signal which will be triggered by pm_power_off as power off.
>>> Since same signal can be used for stand by, I linked PMIC configuration
>>> with pm_power_off_prepare to avoid possible conflicts.
>>>
>>> On Mon, Mar 05, 2018 at 11:25:20AM +0100, Oleksij Rempel wrote:
>>>> Export pm_power_off_prepare. It is needed to implement power off on
>>>> Freescale/NXP iMX6 based boards with external power management
>>>> integrated circuit (PMIC).
>>>>
>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>> ---
>>>>  kernel/reboot.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/kernel/reboot.c b/kernel/reboot.c
>>>> index e4ced883d8de..350be6baa60d 100644
>>>> --- a/kernel/reboot.c
>>>> +++ b/kernel/reboot.c
>>>> @@ -49,6 +49,7 @@ int reboot_force;
>>>>   */
>>>>  
>>>>  void (*pm_power_off_prepare)(void);
>>>> +EXPORT_SYMBOL(pm_power_off_prepare);
>>
>> Why not EXPORT_SYMBOL_GPL() ?
> 
> No special reason. Fixed.
> Any other comments?
> 

Or with other words, will it be enough to get your Signed-of-by for this
patch?
Oleksij Rempel May 14, 2018, 10:12 a.m. UTC | #12
On 14.05.2018 07:10, Oleksij Rempel wrote:
> 
> 
> On 14.05.2018 06:33, Oleksij Rempel wrote:
>>
>>
>> On 12.05.2018 13:13, Rafael J. Wysocki wrote:
>>> On Friday, May 4, 2018 8:50:52 PM CEST Oleksij Rempel wrote:
>>>> Hallo Andrew,
>>>> I need your ACK or NACK for this patch.
>>>>
>>>> This function is used to configure external PMIC to interpret
>>>> signal which will be triggered by pm_power_off as power off.
>>>> Since same signal can be used for stand by, I linked PMIC configuration
>>>> with pm_power_off_prepare to avoid possible conflicts.
>>>>
>>>> On Mon, Mar 05, 2018 at 11:25:20AM +0100, Oleksij Rempel wrote:
>>>>> Export pm_power_off_prepare. It is needed to implement power off on
>>>>> Freescale/NXP iMX6 based boards with external power management
>>>>> integrated circuit (PMIC).
>>>>>
>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>>> ---
>>>>>  kernel/reboot.c | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/kernel/reboot.c b/kernel/reboot.c
>>>>> index e4ced883d8de..350be6baa60d 100644
>>>>> --- a/kernel/reboot.c
>>>>> +++ b/kernel/reboot.c
>>>>> @@ -49,6 +49,7 @@ int reboot_force;
>>>>>   */
>>>>>  
>>>>>  void (*pm_power_off_prepare)(void);
>>>>> +EXPORT_SYMBOL(pm_power_off_prepare);
>>>
>>> Why not EXPORT_SYMBOL_GPL() ?
>>
>> No special reason. Fixed.
>> Any other comments?
>>
> 
> Or with other words, will it be enough to get your Signed-of-by for this
> patch?

Hi again,

i was punished by my colleagues and actually by Russel for asking
"Signed-of-by". So, I correct my self, is it enough for a Reviewed-by?