Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1471524 > unrolled thread

Re: [PATCH v6 0/8] power: add power sequence library

Started byPeter Chen <hzpeterchen@gmail.com>
First post2016-08-29 05:20 +0200
Last post2016-09-19 09:50 +0200
Articles 10 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v6 0/8] power: add power sequence library Peter Chen <hzpeterchen@gmail.com> - 2016-08-29 05:20 +0200
    Re: [PATCH v6 0/8] power: add power sequence library Vaibhav Hiremath <vaibhav.hiremath@linaro.org> - 2016-08-31 10:20 +0200
      Re: [PATCH v6 0/8] power: add power sequence library Peter Chen <hzpeterchen@gmail.com> - 2016-08-31 12:00 +0200
        Re: [PATCH v6 0/8] power: add power sequence library Vaibhav Hiremath <vaibhav.hiremath@linaro.org> - 2016-08-31 19:00 +0200
          Re: [PATCH v6 0/8] power: add power sequence library Peter Chen <hzpeterchen@gmail.com> - 2016-09-02 03:30 +0200
            Re: [PATCH v6 0/8] power: add power sequence library Vaibhav Hiremath <vaibhav.hiremath@linaro.org> - 2016-09-06 12:20 +0200
              Re: [PATCH v6 0/8] power: add power sequence library Ulf Hansson <ulf.hansson@linaro.org> - 2016-09-09 10:50 +0200
                Re: [PATCH v6 0/8] power: add power sequence library Peter Chen <hzpeterchen@gmail.com> - 2016-09-19 09:50 +0200
                  Re: [PATCH v6 0/8] power: add power sequence library Vaibhav Hiremath <vaibhav.hiremath@linaro.org> - 2016-09-19 10:20 +0200
                Re: [PATCH v6 0/8] power: add power sequence library Vaibhav Hiremath <vaibhav.hiremath@linaro.org> - 2016-09-19 09:50 +0200

#1471524 — Re: [PATCH v6 0/8] power: add power sequence library

FromPeter Chen <hzpeterchen@gmail.com>
Date2016-08-29 05:20 +0200
SubjectRe: [PATCH v6 0/8] power: add power sequence library
Message-ID<sblh7-1WZ-13@gated-at.bofh.it>
On Wed, Aug 24, 2016 at 04:53:35PM +0800, Peter Chen wrote:
> On Tue, Aug 23, 2016 at 04:02:48PM +0530, Vaibhav Hiremath wrote:
> > 
> > 
> > On Monday 15 August 2016 02:43 PM, Peter Chen wrote:
> > >Hi all,
> > >
> > >This is a follow-up for my last power sequence framework patch set [1].
> > >According to Rob Herring and Ulf Hansson's comments[2], I use a generic
> > >power sequence library for parsing the power sequence elements on DT,
> > >and implement generic power sequence on library. The host driver
> > >can allocate power sequence instance, and calls pwrseq APIs accordingly.
> > >
> > >In future, if there are special power sequence requirements, the special
> > >power sequence library can be created.
> > >
> > >This patch set is tested on i.mx6 sabresx evk using a dts change, I use
> > >two hot-plug devices to simulate this use case, the related binding
> > >change is updated at patch [1/6], The udoo board changes were tested
> > >using my last power sequence patch set.[3]
> > >
> > >Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
> > >need to power on itself before it can be found by ULPI bus.
> > >
> > >[1] http://www.spinics.net/lists/linux-usb/msg142755.html
> > >[2] http://www.spinics.net/lists/linux-usb/msg143106.html
> > >[3] http://www.spinics.net/lists/linux-usb/msg142815.html
> > (Please ignore my response on V2)
> > 
> > Sorry being so late in the discussion...
> > 
> > If I am not missing anything, then I am afraid to say that the
> > generic library
> > implementation in this patch series is not going to solve many of
> > the custom
> > requirement of power on, off, etc...
> > I know you mentioned about adding another library when we come
> > across such platforms, but should we not keep provision (or easy
> > hooks/path)
> > to enable that ?
> > 
> > Let me bring in the use case I am dealing with,
> > 
> > 
> >                               Host
> >                                |
> >                                V
> >                            USB port
> > ------------------------------------------------------------
> >                                |
> >                                V
> >                       USB HUB device (May need custom on/off seq)
> >                                |
> >                                V
> >               =============================
> >              |                             |
> >              V                             V
> >          Device-1                       Device-2
> > (Needs special power               (Needs special power
> >  on/off sequence.                   on/off sequence.
> >  Also may need custom               Also, may need custom
> >  sequence for                       sequence for
> >  suspend/resume)                    suspend/resume)
> > 
> > 
> > Note: Both Devices are connected to HUB via HSIC and may differ
> >           in terms of functionality, features they support.
> > 
> > In the above case, both Device-1 and Device-2, need separate
> > power on/off sequence. So generic library currently we have in this
> > patch series is not going to satisfy the need here.
> > 
> > I looked at all 6 revisions of this patch-series, went through the
> > review comments, and looked at MMC power sequence code;
> > what I can say here is, we need something similar to
> > MMC power sequence here, where every device can have its own
> > power sequence (if needed).
> > 
> > I know Rob is not in favor of creating platform device for
> > this, and I understand his comment.
> > If not platform device, but atleast we need mechanism to
> > connect each device back to its of_node and its respective
> > driver/library fns. For example, the Devices may support different
> > boot modes, and platform driver needs to make sure that
> > the right sequence is followed for booting.
> > 
> > Peter, My apologies for taking you back again on this series.
> > I am OK, if you wish to address this in incremental addition,
> > but my point is, we know that the current generic way is not
> > enough for us, so I think we should try to fix it in initial phase only.
> > 
> 
> Rob, it seems generic power sequence can't cover all cases.
> Without information from DT, we can't know which power sequence
> for which device.
> 

Vaibhav, do you agree that I create pwrseq library list using postcore_initcall
for each library, and choose pwrseq library according to compatible
string first, if there is no compatible string for this library, just
use generic pwrseq library.

-- 

Best Regards,
Peter Chen

[toc] | [next] | [standalone]

#1473167

FromVaibhav Hiremath <vaibhav.hiremath@linaro.org>
Date2016-08-31 10:20 +0200
Message-ID<sc8Uy-8n3-25@gated-at.bofh.it>
In reply to#1471524

On Monday 29 August 2016 04:40 PM, Peter Chen wrote:
> On Wed, Aug 24, 2016 at 04:53:35PM +0800, Peter Chen wrote:
>> On Tue, Aug 23, 2016 at 04:02:48PM +0530, Vaibhav Hiremath wrote:
>>> On Monday 15 August 2016 02:43 PM, Peter Chen wrote:
>>>> Hi all,
>>>>
>>>> This is a follow-up for my last power sequence framework patch set [1].
>>>> According to Rob Herring and Ulf Hansson's comments[2], I use a generic
>>>> power sequence library for parsing the power sequence elements on DT,
>>>> and implement generic power sequence on library. The host driver
>>>> can allocate power sequence instance, and calls pwrseq APIs accordingly.
>>>>
>>>> In future, if there are special power sequence requirements, the special
>>>> power sequence library can be created.
>>>>
>>>> This patch set is tested on i.mx6 sabresx evk using a dts change, I use
>>>> two hot-plug devices to simulate this use case, the related binding
>>>> change is updated at patch [1/6], The udoo board changes were tested
>>>> using my last power sequence patch set.[3]
>>>>
>>>> Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
>>>> need to power on itself before it can be found by ULPI bus.
>>>>
>>>> [1]http://www.spinics.net/lists/linux-usb/msg142755.html
>>>> [2]http://www.spinics.net/lists/linux-usb/msg143106.html
>>>> [3]http://www.spinics.net/lists/linux-usb/msg142815.html
>>> (Please ignore my response on V2)
>>>
>>> Sorry being so late in the discussion...
>>>
>>> If I am not missing anything, then I am afraid to say that the
>>> generic library
>>> implementation in this patch series is not going to solve many of
>>> the custom
>>> requirement of power on, off, etc...
>>> I know you mentioned about adding another library when we come
>>> across such platforms, but should we not keep provision (or easy
>>> hooks/path)
>>> to enable that ?
>>>
>>> Let me bring in the use case I am dealing with,
>>>
>>>
>>>                                Host
>>>                                 |
>>>                                 V
>>>                             USB port
>>> ------------------------------------------------------------
>>>                                 |
>>>                                 V
>>>                        USB HUB device (May need custom on/off seq)
>>>                                 |
>>>                                 V
>>>                =============================
>>>               |                             |
>>>               V                             V
>>>           Device-1                       Device-2
>>> (Needs special power               (Needs special power
>>>   on/off sequence.                   on/off sequence.
>>>   Also may need custom               Also, may need custom
>>>   sequence for                       sequence for
>>>   suspend/resume)                    suspend/resume)
>>>
>>>
>>> Note: Both Devices are connected to HUB via HSIC and may differ
>>>            in terms of functionality, features they support.
>>>
>>> In the above case, both Device-1 and Device-2, need separate
>>> power on/off sequence. So generic library currently we have in this
>>> patch series is not going to satisfy the need here.
>>>
>>> I looked at all 6 revisions of this patch-series, went through the
>>> review comments, and looked at MMC power sequence code;
>>> what I can say here is, we need something similar to
>>> MMC power sequence here, where every device can have its own
>>> power sequence (if needed).
>>>
>>> I know Rob is not in favor of creating platform device for
>>> this, and I understand his comment.
>>> If not platform device, but atleast we need mechanism to
>>> connect each device back to its of_node and its respective
>>> driver/library fns. For example, the Devices may support different
>>> boot modes, and platform driver needs to make sure that
>>> the right sequence is followed for booting.
>>>
>>> Peter, My apologies for taking you back again on this series.
>>> I am OK, if you wish to address this in incremental addition,
>>> but my point is, we know that the current generic way is not
>>> enough for us, so I think we should try to fix it in initial phase only.
>>>
>> Rob, it seems generic power sequence can't cover all cases.
>> Without information from DT, we can't know which power sequence
>> for which device.
>>
> Vaibhav, do you agree that I create pwrseq library list using postcore_initcall
> for each library, and choose pwrseq library according to compatible
> string first, if there is no compatible string for this library, just
> use generic pwrseq library.
>

Lets hear from MMC folks. Ulf, do you agree on approach
for pwr seq ??


With above approach, I kind of agree on it, but I have couple
of comments,

  - How DTS looks like now ?? Can you put example here ?
  - Should we merge MMC power sequence in next series ?
    We also can take this as an incremental change, to avoid further
    delay :)
  - Lets also add suspend/resume callback to struct pwrseq


There are some comments I have on the patches,
I will respond directly on respective patches, it would be useful
for next series.


And Sorry for delayed response.

-- 
Thanks,
Vaibhav

[toc] | [prev] | [next] | [standalone]

#1473241

FromPeter Chen <hzpeterchen@gmail.com>
Date2016-08-31 12:00 +0200
Message-ID<scatj-KH-1@gated-at.bofh.it>
In reply to#1473167
On Wed, Aug 31, 2016 at 01:46:30PM +0530, Vaibhav Hiremath wrote:
> 
> 
> On Monday 29 August 2016 04:40 PM, Peter Chen wrote:
> >On Wed, Aug 24, 2016 at 04:53:35PM +0800, Peter Chen wrote:
> >>On Tue, Aug 23, 2016 at 04:02:48PM +0530, Vaibhav Hiremath wrote:
> >>>On Monday 15 August 2016 02:43 PM, Peter Chen wrote:
> >>>>Hi all,
> >>>>
> >>>>This is a follow-up for my last power sequence framework patch set [1].
> >>>>According to Rob Herring and Ulf Hansson's comments[2], I use a generic
> >>>>power sequence library for parsing the power sequence elements on DT,
> >>>>and implement generic power sequence on library. The host driver
> >>>>can allocate power sequence instance, and calls pwrseq APIs accordingly.
> >>>>
> >>>>In future, if there are special power sequence requirements, the special
> >>>>power sequence library can be created.
> >>>>
> >>>>This patch set is tested on i.mx6 sabresx evk using a dts change, I use
> >>>>two hot-plug devices to simulate this use case, the related binding
> >>>>change is updated at patch [1/6], The udoo board changes were tested
> >>>>using my last power sequence patch set.[3]
> >>>>
> >>>>Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
> >>>>need to power on itself before it can be found by ULPI bus.
> >>>>
> >>>>[1]http://www.spinics.net/lists/linux-usb/msg142755.html
> >>>>[2]http://www.spinics.net/lists/linux-usb/msg143106.html
> >>>>[3]http://www.spinics.net/lists/linux-usb/msg142815.html
> >>>(Please ignore my response on V2)
> >>>
> >>>Sorry being so late in the discussion...
> >>>
> >>>If I am not missing anything, then I am afraid to say that the
> >>>generic library
> >>>implementation in this patch series is not going to solve many of
> >>>the custom
> >>>requirement of power on, off, etc...
> >>>I know you mentioned about adding another library when we come
> >>>across such platforms, but should we not keep provision (or easy
> >>>hooks/path)
> >>>to enable that ?
> >>>
> >>>Let me bring in the use case I am dealing with,
> >>>
> >>>
> >>>                               Host
> >>>                                |
> >>>                                V
> >>>                            USB port
> >>>------------------------------------------------------------
> >>>                                |
> >>>                                V
> >>>                       USB HUB device (May need custom on/off seq)
> >>>                                |
> >>>                                V
> >>>               =============================
> >>>              |                             |
> >>>              V                             V
> >>>          Device-1                       Device-2
> >>>(Needs special power               (Needs special power
> >>>  on/off sequence.                   on/off sequence.
> >>>  Also may need custom               Also, may need custom
> >>>  sequence for                       sequence for
> >>>  suspend/resume)                    suspend/resume)
> >>>
> >>>
> >>>Note: Both Devices are connected to HUB via HSIC and may differ
> >>>           in terms of functionality, features they support.
> >>>
> >>>In the above case, both Device-1 and Device-2, need separate
> >>>power on/off sequence. So generic library currently we have in this
> >>>patch series is not going to satisfy the need here.
> >>>
> >>>I looked at all 6 revisions of this patch-series, went through the
> >>>review comments, and looked at MMC power sequence code;
> >>>what I can say here is, we need something similar to
> >>>MMC power sequence here, where every device can have its own
> >>>power sequence (if needed).
> >>>
> >>>I know Rob is not in favor of creating platform device for
> >>>this, and I understand his comment.
> >>>If not platform device, but atleast we need mechanism to
> >>>connect each device back to its of_node and its respective
> >>>driver/library fns. For example, the Devices may support different
> >>>boot modes, and platform driver needs to make sure that
> >>>the right sequence is followed for booting.
> >>>
> >>>Peter, My apologies for taking you back again on this series.
> >>>I am OK, if you wish to address this in incremental addition,
> >>>but my point is, we know that the current generic way is not
> >>>enough for us, so I think we should try to fix it in initial phase only.
> >>>
> >>Rob, it seems generic power sequence can't cover all cases.
> >>Without information from DT, we can't know which power sequence
> >>for which device.
> >>
> >Vaibhav, do you agree that I create pwrseq library list using postcore_initcall
> >for each library, and choose pwrseq library according to compatible
> >string first, if there is no compatible string for this library, just
> >use generic pwrseq library.
> >
> 
> Lets hear from MMC folks. Ulf, do you agree on approach
> for pwr seq ??
> 
> 
> With above approach, I kind of agree on it, but I have couple
> of comments,
> 
>  - How DTS looks like now ?? Can you put example here ?

The dts is the same with current version.

>  - Should we merge MMC power sequence in next series ?
>    We also can take this as an incremental change, to avoid further
>    delay :)

We had an agreement that keep mmc's pwrseq framework unchanging.
Unless Ulf and rob both agree to change.

>  - Lets also add suspend/resume callback to struct pwrseq
> 

Why suspend/resume can't do at related driver's suspend/resume API? 

> 
> There are some comments I have on the patches,
> I will respond directly on respective patches, it would be useful
> for next series.
> 

Thanks.

-- 

Best Regards,
Peter Chen

[toc] | [prev] | [next] | [standalone]

#1473706

FromVaibhav Hiremath <vaibhav.hiremath@linaro.org>
Date2016-08-31 19:00 +0200
Message-ID<sch1M-4Rl-21@gated-at.bofh.it>
In reply to#1473241

On Wednesday 31 August 2016 03:22 PM, Peter Chen wrote:
> On Wed, Aug 31, 2016 at 01:46:30PM +0530, Vaibhav Hiremath wrote:
>>
>> On Monday 29 August 2016 04:40 PM, Peter Chen wrote:
>>> On Wed, Aug 24, 2016 at 04:53:35PM +0800, Peter Chen wrote:
>>>> On Tue, Aug 23, 2016 at 04:02:48PM +0530, Vaibhav Hiremath wrote:
>>>>> On Monday 15 August 2016 02:43 PM, Peter Chen wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> This is a follow-up for my last power sequence framework patch set [1].
>>>>>> According to Rob Herring and Ulf Hansson's comments[2], I use a generic
>>>>>> power sequence library for parsing the power sequence elements on DT,
>>>>>> and implement generic power sequence on library. The host driver
>>>>>> can allocate power sequence instance, and calls pwrseq APIs accordingly.
>>>>>>
>>>>>> In future, if there are special power sequence requirements, the special
>>>>>> power sequence library can be created.
>>>>>>
>>>>>> This patch set is tested on i.mx6 sabresx evk using a dts change, I use
>>>>>> two hot-plug devices to simulate this use case, the related binding
>>>>>> change is updated at patch [1/6], The udoo board changes were tested
>>>>>> using my last power sequence patch set.[3]
>>>>>>
>>>>>> Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
>>>>>> need to power on itself before it can be found by ULPI bus.
>>>>>>
>>>>>> [1]http://www.spinics.net/lists/linux-usb/msg142755.html
>>>>>> [2]http://www.spinics.net/lists/linux-usb/msg143106.html
>>>>>> [3]http://www.spinics.net/lists/linux-usb/msg142815.html
>>>>> (Please ignore my response on V2)
>>>>>
>>>>> Sorry being so late in the discussion...
>>>>>
>>>>> If I am not missing anything, then I am afraid to say that the
>>>>> generic library
>>>>> implementation in this patch series is not going to solve many of
>>>>> the custom
>>>>> requirement of power on, off, etc...
>>>>> I know you mentioned about adding another library when we come
>>>>> across such platforms, but should we not keep provision (or easy
>>>>> hooks/path)
>>>>> to enable that ?
>>>>>
>>>>> Let me bring in the use case I am dealing with,
>>>>>
>>>>>
>>>>>                                Host
>>>>>                                 |
>>>>>                                 V
>>>>>                             USB port
>>>>> ------------------------------------------------------------
>>>>>                                 |
>>>>>                                 V
>>>>>                        USB HUB device (May need custom on/off seq)
>>>>>                                 |
>>>>>                                 V
>>>>>                =============================
>>>>>               |                             |
>>>>>               V                             V
>>>>>           Device-1                       Device-2
>>>>> (Needs special power               (Needs special power
>>>>>   on/off sequence.                   on/off sequence.
>>>>>   Also may need custom               Also, may need custom
>>>>>   sequence for                       sequence for
>>>>>   suspend/resume)                    suspend/resume)
>>>>>
>>>>>
>>>>> Note: Both Devices are connected to HUB via HSIC and may differ
>>>>>            in terms of functionality, features they support.
>>>>>
>>>>> In the above case, both Device-1 and Device-2, need separate
>>>>> power on/off sequence. So generic library currently we have in this
>>>>> patch series is not going to satisfy the need here.
>>>>>
>>>>> I looked at all 6 revisions of this patch-series, went through the
>>>>> review comments, and looked at MMC power sequence code;
>>>>> what I can say here is, we need something similar to
>>>>> MMC power sequence here, where every device can have its own
>>>>> power sequence (if needed).
>>>>>
>>>>> I know Rob is not in favor of creating platform device for
>>>>> this, and I understand his comment.
>>>>> If not platform device, but atleast we need mechanism to
>>>>> connect each device back to its of_node and its respective
>>>>> driver/library fns. For example, the Devices may support different
>>>>> boot modes, and platform driver needs to make sure that
>>>>> the right sequence is followed for booting.
>>>>>
>>>>> Peter, My apologies for taking you back again on this series.
>>>>> I am OK, if you wish to address this in incremental addition,
>>>>> but my point is, we know that the current generic way is not
>>>>> enough for us, so I think we should try to fix it in initial phase only.
>>>>>
>>>> Rob, it seems generic power sequence can't cover all cases.
>>>> Without information from DT, we can't know which power sequence
>>>> for which device.
>>>>
>>> Vaibhav, do you agree that I create pwrseq library list using postcore_initcall
>>> for each library, and choose pwrseq library according to compatible
>>> string first, if there is no compatible string for this library, just
>>> use generic pwrseq library.
>>>
>> Lets hear from MMC folks. Ulf, do you agree on approach
>> for pwr seq ??
>>
>>
>> With above approach, I kind of agree on it, but I have couple
>> of comments,
>>
>>   - How DTS looks like now ?? Can you put example here ?
> The dts is the same with current version.

How would consumer driver get the power sequence ?
You must point to right power sequence driver.
For example, in the above example, Device-1, should
get handle to pwrseq-1, and Device-2 to pwrseq-2.

>>   - Should we merge MMC power sequence in next series ?
>>     We also can take this as an incremental change, to avoid further
>>     delay :)
> We had an agreement that keep mmc's pwrseq framework unchanging.
> Unless Ulf and rob both agree to change.

Why 2 separate approach for same problem ?
And I see this as possible duplication of code/functionality :)

Rob ??? Ulf ???

>>   - Lets also add suspend/resume callback to struct pwrseq
>>
> Why suspend/resume can't do at related driver's suspend/resume API?

Nope...
The pwrseq library would have taken ownership of resources, so
related driver cannot suspend the device. Again it is architecture
specific, but we should have provision to handle this.

The system I am dealing with today, does need suspend/resume
callback. To be precise, suspend is close to off state for some devices or
they could enter standby or different low power state, but to do that,
we need same resource as used for ON/OFF functionality.


Thanks,
Vaibhav
>> There are some comments I have on the patches,
>> I will respond directly on respective patches, it would be useful
>> for next series.
>>
> Thanks.
>

-- 
Thanks,
Vaibhav

[toc] | [prev] | [next] | [standalone]

#1474844

FromPeter Chen <hzpeterchen@gmail.com>
Date2016-09-02 03:30 +0200
Message-ID<scLsR-17o-3@gated-at.bofh.it>
In reply to#1473706
On Wed, Aug 31, 2016 at 10:28:20PM +0530, Vaibhav Hiremath wrote:
> 
> 
> On Wednesday 31 August 2016 03:22 PM, Peter Chen wrote:
> >On Wed, Aug 31, 2016 at 01:46:30PM +0530, Vaibhav Hiremath wrote:
> >>
> >>On Monday 29 August 2016 04:40 PM, Peter Chen wrote:
> >>>On Wed, Aug 24, 2016 at 04:53:35PM +0800, Peter Chen wrote:
> >>>>On Tue, Aug 23, 2016 at 04:02:48PM +0530, Vaibhav Hiremath wrote:
> >>>>>On Monday 15 August 2016 02:43 PM, Peter Chen wrote:
> >>>>>>Hi all,
> >>>>>>
> >>>>>>This is a follow-up for my last power sequence framework patch set [1].
> >>>>>>According to Rob Herring and Ulf Hansson's comments[2], I use a generic
> >>>>>>power sequence library for parsing the power sequence elements on DT,
> >>>>>>and implement generic power sequence on library. The host driver
> >>>>>>can allocate power sequence instance, and calls pwrseq APIs accordingly.
> >>>>>>
> >>>>>>In future, if there are special power sequence requirements, the special
> >>>>>>power sequence library can be created.
> >>>>>>
> >>>>>>This patch set is tested on i.mx6 sabresx evk using a dts change, I use
> >>>>>>two hot-plug devices to simulate this use case, the related binding
> >>>>>>change is updated at patch [1/6], The udoo board changes were tested
> >>>>>>using my last power sequence patch set.[3]
> >>>>>>
> >>>>>>Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
> >>>>>>need to power on itself before it can be found by ULPI bus.
> >>>>>>
> >>>>>>[1]http://www.spinics.net/lists/linux-usb/msg142755.html
> >>>>>>[2]http://www.spinics.net/lists/linux-usb/msg143106.html
> >>>>>>[3]http://www.spinics.net/lists/linux-usb/msg142815.html
> >>>>>(Please ignore my response on V2)
> >>>>>
> >>>>>Sorry being so late in the discussion...
> >>>>>
> >>>>>If I am not missing anything, then I am afraid to say that the
> >>>>>generic library
> >>>>>implementation in this patch series is not going to solve many of
> >>>>>the custom
> >>>>>requirement of power on, off, etc...
> >>>>>I know you mentioned about adding another library when we come
> >>>>>across such platforms, but should we not keep provision (or easy
> >>>>>hooks/path)
> >>>>>to enable that ?
> >>>>>
> >>>>>Let me bring in the use case I am dealing with,
> >>>>>
> >>>>>
> >>>>>                               Host
> >>>>>                                |
> >>>>>                                V
> >>>>>                            USB port
> >>>>>------------------------------------------------------------
> >>>>>                                |
> >>>>>                                V
> >>>>>                       USB HUB device (May need custom on/off seq)
> >>>>>                                |
> >>>>>                                V
> >>>>>               =============================
> >>>>>              |                             |
> >>>>>              V                             V
> >>>>>          Device-1                       Device-2
> >>>>>(Needs special power               (Needs special power
> >>>>>  on/off sequence.                   on/off sequence.
> >>>>>  Also may need custom               Also, may need custom
> >>>>>  sequence for                       sequence for
> >>>>>  suspend/resume)                    suspend/resume)
> >>>>>
> >>>>>
> >>>>>Note: Both Devices are connected to HUB via HSIC and may differ
> >>>>>           in terms of functionality, features they support.
> >>>>>
> >>>>>In the above case, both Device-1 and Device-2, need separate
> >>>>>power on/off sequence. So generic library currently we have in this
> >>>>>patch series is not going to satisfy the need here.
> >>>>>
> >>>>>I looked at all 6 revisions of this patch-series, went through the
> >>>>>review comments, and looked at MMC power sequence code;
> >>>>>what I can say here is, we need something similar to
> >>>>>MMC power sequence here, where every device can have its own
> >>>>>power sequence (if needed).
> >>>>>
> >>>>>I know Rob is not in favor of creating platform device for
> >>>>>this, and I understand his comment.
> >>>>>If not platform device, but atleast we need mechanism to
> >>>>>connect each device back to its of_node and its respective
> >>>>>driver/library fns. For example, the Devices may support different
> >>>>>boot modes, and platform driver needs to make sure that
> >>>>>the right sequence is followed for booting.
> >>>>>
> >>>>>Peter, My apologies for taking you back again on this series.
> >>>>>I am OK, if you wish to address this in incremental addition,
> >>>>>but my point is, we know that the current generic way is not
> >>>>>enough for us, so I think we should try to fix it in initial phase only.
> >>>>>
> >>>>Rob, it seems generic power sequence can't cover all cases.
> >>>>Without information from DT, we can't know which power sequence
> >>>>for which device.
> >>>>
> >>>Vaibhav, do you agree that I create pwrseq library list using postcore_initcall
> >>>for each library, and choose pwrseq library according to compatible
> >>>string first, if there is no compatible string for this library, just
> >>>use generic pwrseq library.
> >>>
> >>Lets hear from MMC folks. Ulf, do you agree on approach
> >>for pwr seq ??
> >>
> >>
> >>With above approach, I kind of agree on it, but I have couple
> >>of comments,
> >>
> >>  - How DTS looks like now ?? Can you put example here ?
> >The dts is the same with current version.
> 
> How would consumer driver get the power sequence ?
> You must point to right power sequence driver.
> For example, in the above example, Device-1, should
> get handle to pwrseq-1, and Device-2 to pwrseq-2.

According to compatible string at device's of_node, we will have a list
for power sequence libraries which has index (or name), and matches
compatible string.

> 
> >>  - Should we merge MMC power sequence in next series ?
> >>    We also can take this as an incremental change, to avoid further
> >>    delay :)
> >We had an agreement that keep mmc's pwrseq framework unchanging.
> >Unless Ulf and rob both agree to change.
> 
> Why 2 separate approach for same problem ?
> And I see this as possible duplication of code/functionality :)

How the new kernel compatibles old dts? If we do not need to
consider this problem, the mmc can try to use power sequence library
too in future.

> 
> >>  - Lets also add suspend/resume callback to struct pwrseq
> >>
> >Why suspend/resume can't do at related driver's suspend/resume API?
> 
> Nope...
> The pwrseq library would have taken ownership of resources, so
> related driver cannot suspend the device. Again it is architecture
> specific, but we should have provision to handle this.
> 
> The system I am dealing with today, does need suspend/resume
> callback. To be precise, suspend is close to off state for some devices or
> they could enter standby or different low power state, but to do that,
> we need same resource as used for ON/OFF functionality.
> 

Ok, I will have API for suspend/resume. You can implement it at your own
library or generic one.

-- 

Best Regards,
Peter Chen

[toc] | [prev] | [next] | [standalone]

#1477245

FromVaibhav Hiremath <vaibhav.hiremath@linaro.org>
Date2016-09-06 12:20 +0200
Message-ID<selDX-HZ-13@gated-at.bofh.it>
In reply to#1474844
a

On Friday 02 September 2016 06:40 AM, Peter Chen wrote:
> On Wed, Aug 31, 2016 at 10:28:20PM +0530, Vaibhav Hiremath wrote:
>>
>> On Wednesday 31 August 2016 03:22 PM, Peter Chen wrote:
>>> On Wed, Aug 31, 2016 at 01:46:30PM +0530, Vaibhav Hiremath wrote:
>>>> On Monday 29 August 2016 04:40 PM, Peter Chen wrote:
>>>>> On Wed, Aug 24, 2016 at 04:53:35PM +0800, Peter Chen wrote:
>>>>>> On Tue, Aug 23, 2016 at 04:02:48PM +0530, Vaibhav Hiremath wrote:
>>>>>>> veOn Monday 15 August 2016 02:43 PM, Peter Chen wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> This is a follow-up for my last power sequence framework patch set [1].
>>>>>>>> According to Rob Herring and Ulf Hansson's comments[2], I use a generic
>>>>>>>> power sequence library for parsing the power sequence elements on DT,
>>>>>>>> and implement generic power sequence on library. The host driver
>>>>>>>> can allocate power sequence instance, and calls pwrseq APIs accordingly.
>>>>>>>>
>>>>>>>> In future, if there are special power sequence requirements, the special
>>>>>>>> power sequence library can be created.
>>>>>>>>
>>>>>>>> This patch set is tested on i.mx6 sabresx evk using a dts change, I use
>>>>>>>> two hot-plug devices to simulate this use case, the related binding
>>>>>>>> change is updated at patch [1/6], The udoo board changes were tested
>>>>>>>> using my last power sequence patch set.[3]
>>>>>>>>
>>>>>>>> Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
>>>>>>>> need to power on itself before it can be found by ULPI bus.
>>>>>>>>
>>>>>>>> [1]http://www.spinics.net/lists/linux-usb/msg142755.html
>>>>>>>> [2]http://www.spinics.net/lists/linux-usb/msg143106.html
>>>>>>>> [3]http://www.spinics.net/lists/linux-usb/msg142815.html
>>>>>>> (Please ignore my response on V2)
>>>>>>>
>>>>>>> Sorry being so late in the discussion...
>>>>>>>
>>>>>>> If I am not missing anything, then I am afraid to say that the
>>>>>>> generic library
>>>>>>> implementation in this patch series is not going to solve many of
>>>>>>> the custom
>>>>>>> requirement of power on, off, etc...
>>>>>>> I know you mentioned about adding another library when we come
>>>>>>> across such platforms, but should we not keep provision (or easy
>>>>>>> hooks/path)
>>>>>>> to enable that ?
>>>>>>>
>>>>>>> Let me bring in the use case I am dealing with,
>>>>>>>
>>>>>>>
>>>>>>>                                Host
>>>>>>>                                 |
>>>>>>>                                 V
>>>>>>>                             USB port
>>>>>>> ------------------------------------------------------------
>>>>>>>                                 |
>>>>>>>                                 V
>>>>>>>                        USB HUB device (May need custom on/off seq)
>>>>>>>                                 |
>>>>>>>                                 V
>>>>>>>                =============================
>>>>>>>               |                             |
>>>>>>>               V                             V
>>>>>>>           Device-1                       Device-2
>>>>>>> (Needs special power               (Needs special power
>>>>>>>   on/off sequence.                   on/off sequence.
>>>>>>>   Also may need custom               Also, may need custom
>>>>>>>   sequence for                       sequence for
>>>>>>>   suspend/resume)                    suspend/resume)
>>>>>>>
>>>>>>>
>>>>>>> Note: Both Devices are connected to HUB via HSIC and may differ
>>>>>>>            in terms of functionality, features they support.
>>>>>>>
>>>>>>> In the above case, both Device-1 and Device-2, need separate
>>>>>>> power on/off sequence. So generic library currently we have in this
>>>>>>> patch series is not going to satisfy the need here.
>>>>>>>
>>>>>>> I looked at all 6 revisions of this patch-series, went through the
>>>>>>> review comments, and looked at MMC power sequence code;
>>>>>>> what I can say here is, we need something similar to
>>>>>>> MMC power sequence here, where every device can have its own
>>>>>>> power sequence (if needed).
>>>>>>>
>>>>>>> I know Rob is not in favor of creating platform device for
>>>>>>> this, and I understand his comment.
>>>>>>> If not platform device, but atleast we need mechanism to
>>>>>>> connect each device back to its of_node and its respective
>>>>>>> driver/library fns. For example, the Devices may support different
>>>>>>> boot modes, and platform driver needs to make sure that
>>>>>>> the right sequence is followed for booting.
>>>>>>>
>>>>>>> Peter, My apologies for taking you back again on this series.
>>>>>>> I am OK, if you wish to address this in incremental addition,
>>>>>>> but my point is, we know that the current generic way is not
>>>>>>> enough for us, so I think we should try to fix it in initial phase only.
>>>>>>>
>>>>>> Rob, it seems generic power sequence can't cover all cases.
>>>>>> Without information from DT, we can't know which power sequence
>>>>>> for which device.
>>>>>>
>>>>> Vaibhav, do you agree that I create pwrseq library list using postcore_initcall
>>>>> for each library, and choose pwrseq library according to compatible
>>>>> string first, if there is no compatible string for this library, just
>>>>> use generic pwrseq library.
>>>>>
>>>> Lets hear from MMC folks. Ulf, do you agree on approach
>>>> for pwr seq ??
>>>>
>>>>
>>>> With above approach, I kind of agree on it, but I have couple
>>>> of comments,
>>>>
>>>>   - How DTS looks like now ?? Can you put example here ?
>>> The dts is the same with current version.
>> How would consumer driver get the power sequence ?
>> You must point to right power sequence driver.
>> For example, in the above example, Device-1, should
>> get handle to pwrseq-1, and Device-2 to pwrseq-2.
> According to compatible string at device's of_node, we will have a list
> for power sequence libraries which has index (or name), and matches
> compatible string.
>
>>>>   - Should we merge MMC power sequence in next series ?
>>>>     We also can take this as an incremental change, to avoid further
>>>>     delay :)
>>> We had an agreement that keep mmc's pwrseq framework unchanging.
>>> Unless Ulf and rob both agree to change.
>> Why 2 separate approach for same problem ?
>> And I see this as possible duplication of code/functionality :)
> How the new kernel compatibles old dts? If we do not need to
> consider this problem, the mmc can try to use power sequence library
> too in future.

I think we should attempt to get both MMC and USB power seq
come on one agreement, so that it can be reused.

MMC Power Seq :
  It is based on platform_device/platform_driver approach,

USB Power seq :
  We are trying to propose library approach, with compatible string match.

We should try to have one approach.
>
>>>>   - Lets also add suspend/resume callback to struct pwrseq
>>>>
>>> Why suspend/resume can't do at related driver's suspend/resume API?
>> Nope...
>> The pwrseq library would have taken ownership of resources, so
>> related driver cannot suspend the device. Again it is architecture
>> specific, but we should have provision to handle this.
>>
>> The system I am dealing with today, does need suspend/resume
>> callback. To be precise, suspend is close to off state for some devices or
>> they could enter standby or different low power state, but to do that,
>> we need same resource as used for ON/OFF functionality.
>>
> Ok, I will have API for suspend/resume. You can implement it at your own
> library or generic one.
>

Yeup. Sure.

-- 
Thanks,
Vaibhav

[toc] | [prev] | [next] | [standalone]

#1479755

FromUlf Hansson <ulf.hansson@linaro.org>
Date2016-09-09 10:50 +0200
Message-ID<sfpFv-zV-9@gated-at.bofh.it>
In reply to#1477245
[...]

>>>>
>>>> We had an agreement that keep mmc's pwrseq framework unchanging.
>>>> Unless Ulf and rob both agree to change.
>>>
>>> Why 2 separate approach for same problem ?
>>> And I see this as possible duplication of code/functionality :)
>>
>> How the new kernel compatibles old dts? If we do not need to
>> consider this problem, the mmc can try to use power sequence library
>> too in future.
>
>
> I think we should attempt to get both MMC and USB power seq
> come on one agreement, so that it can be reused.

That would be nice. Although, to do that you would have to allow some
DT bindings to be deprecated in the new generic power seq bindings, as
otherwise you would break existing DTBs.

I guess that is what Rob was objecting to!?

>
> MMC Power Seq :
>  It is based on platform_device/platform_driver approach,

We recently converted MMC to this. It's has a clear benefit as you can
rely on the behaviour from the driver core and PM core. So, it simply
avoids duplication of code.

>
> USB Power seq :
>  We are trying to propose library approach, with compatible string match.
>
> We should try to have one approach.
>>
>>
>>>>>   - Lets also add suspend/resume callback to struct pwrseq
>>>>>
>>>> Why suspend/resume can't do at related driver's suspend/resume API?
>>>
>>> Nope...
>>> The pwrseq library would have taken ownership of resources, so
>>> related driver cannot suspend the device. Again it is architecture
>>> specific, but we should have provision to handle this.
>>>
>>> The system I am dealing with today, does need suspend/resume
>>> callback. To be precise, suspend is close to off state for some devices
>>> or
>>> they could enter standby or different low power state, but to do that,
>>> we need same resource as used for ON/OFF functionality.
>>>
>> Ok, I will have API for suspend/resume. You can implement it at your own
>> library or generic one.

As stated, using a platform device + driver would simplify this, as
you wouldn't need an API but only a driver. I guess.

Kind regards
Uffe

[toc] | [prev] | [next] | [standalone]

#1486209

FromPeter Chen <hzpeterchen@gmail.com>
Date2016-09-19 09:50 +0200
Message-ID<sj1uV-2ya-1@gated-at.bofh.it>
In reply to#1479755
On Mon, Sep 19, 2016 at 01:09:10PM +0530, Vaibhav Hiremath wrote:
> 
> 
> On Friday 09 September 2016 02:17 PM, Ulf Hansson wrote:
> >[...]
> >
> >>>>>We had an agreement that keep mmc's pwrseq framework unchanging.
> >>>>>Unless Ulf and rob both agree to change.
> >>>>Why 2 separate approach for same problem ?
> >>>>And I see this as possible duplication of code/functionality :)
> >>>How the new kernel compatibles old dts? If we do not need to
> >>>consider this problem, the mmc can try to use power sequence library
> >>>too in future.
> >>
> >>I think we should attempt to get both MMC and USB power seq
> >>come on one agreement, so that it can be reused.
> >That would be nice. Although, to do that you would have to allow some
> >DT bindings to be deprecated in the new generic power seq bindings, as
> >otherwise you would break existing DTBs.
> >
> >I guess that is what Rob was objecting to!?
> 
> yeah, thats right.
> 
> So lets adopt similar implementation for USB as well instead of
> library, but keeping MMC untouched as of now.
> 
> What I am trying to propose here is,
> 
> Lets have power-sequence framework (similar to V1 of this series),
> with,
> 
> pwrseq: Core framework for power sequence.
> pwrseq_generic/simple: for all generic control, like reset and clock
> pwrseq_emmc: probably duplication of existing code - the idea
>                           here is, all future code should be using this new
>                           binding, so that we can deprecate the
> drivers/mmc/core/pwrseq
> pwrseq_arche: The usecase which I am dealing with today, which is more
>                          complex in nature.
> 
> Then the respective drivers can add their drivers (if needed) based on
> complexity.
> 
> comments ??

The key point here is DT maintainer (Rob) doesn't agree with adding new node
for power sequence at dts.

-- 

Best Regards,
Peter Chen

[toc] | [prev] | [next] | [standalone]

#1486242

FromVaibhav Hiremath <vaibhav.hiremath@linaro.org>
Date2016-09-19 10:20 +0200
Message-ID<sj1XX-2Zs-15@gated-at.bofh.it>
In reply to#1486209

On Monday 19 September 2016 01:16 PM, Peter Chen wrote:
> On Mon, Sep 19, 2016 at 01:09:10PM +0530, Vaibhav Hiremath wrote:
>>
>> On Friday 09 September 2016 02:17 PM, Ulf Hansson wrote:
>>> [...]
>>>
>>>>>>> We had an agreement that keep mmc's pwrseq framework unchanging.
>>>>>>> Unless Ulf and rob both agree to change.
>>>>>> Why 2 separate approach for same problem ?
>>>>>> And I see this as possible duplication of code/functionality :)
>>>>> How the new kernel compatibles old dts? If we do not need to
>>>>> consider this problem, the mmc can try to use power sequence library
>>>>> too in future.
>>>> I think we should attempt to get both MMC and USB power seq
>>>> come on one agreement, so that it can be reused.
>>> That would be nice. Although, to do that you would have to allow some
>>> DT bindings to be deprecated in the new generic power seq bindings, as
>>> otherwise you would break existing DTBs.
>>>
>>> I guess that is what Rob was objecting to!?
>> yeah, thats right.
>>
>> So lets adopt similar implementation for USB as well instead of
>> library, but keeping MMC untouched as of now.
>>
>> What I am trying to propose here is,
>>
>> Lets have power-sequence framework (similar to V1 of this series),
>> with,
>>
>> pwrseq: Core framework for power sequence.
>> pwrseq_generic/simple: for all generic control, like reset and clock
>> pwrseq_emmc: probably duplication of existing code - the idea
>>                            here is, all future code should be using this new
>>                            binding, so that we can deprecate the
>> drivers/mmc/core/pwrseq
>> pwrseq_arche: The usecase which I am dealing with today, which is more
>>                           complex in nature.
>>
>> Then the respective drivers can add their drivers (if needed) based on
>> complexity.
>>
>> comments ??
> The key point here is DT maintainer (Rob) doesn't agree with adding new node
> for power sequence at dts.
>

Hmmm.
We haven't heard from Rob lately especially after introduction of complex
usecases. I hope he would revisit on above proposal.

Thanks,
Vaibhav

[toc] | [prev] | [next] | [standalone]

#1486215

FromVaibhav Hiremath <vaibhav.hiremath@linaro.org>
Date2016-09-19 09:50 +0200
Message-ID<sj1uV-2ya-3@gated-at.bofh.it>
In reply to#1479755

On Friday 09 September 2016 02:17 PM, Ulf Hansson wrote:
> [...]
>
>>>>> We had an agreement that keep mmc's pwrseq framework unchanging.
>>>>> Unless Ulf and rob both agree to change.
>>>> Why 2 separate approach for same problem ?
>>>> And I see this as possible duplication of code/functionality :)
>>> How the new kernel compatibles old dts? If we do not need to
>>> consider this problem, the mmc can try to use power sequence library
>>> too in future.
>>
>> I think we should attempt to get both MMC and USB power seq
>> come on one agreement, so that it can be reused.
> That would be nice. Although, to do that you would have to allow some
> DT bindings to be deprecated in the new generic power seq bindings, as
> otherwise you would break existing DTBs.
>
> I guess that is what Rob was objecting to!?

yeah, thats right.

So lets adopt similar implementation for USB as well instead of
library, but keeping MMC untouched as of now.

What I am trying to propose here is,

Lets have power-sequence framework (similar to V1 of this series),
with,

pwrseq: Core framework for power sequence.
pwrseq_generic/simple: for all generic control, like reset and clock
pwrseq_emmc: probably duplication of existing code - the idea
                           here is, all future code should be using this new
                           binding, so that we can deprecate the 
drivers/mmc/core/pwrseq
pwrseq_arche: The usecase which I am dealing with today, which is more
                          complex in nature.

Then the respective drivers can add their drivers (if needed) based on
complexity.

comments ??

>> MMC Power Seq :
>>   It is based on platform_device/platform_driver approach,
> We recently converted MMC to this. It's has a clear benefit as you can
> rely on the behaviour from the driver core and PM core. So, it simply
> avoids duplication of code.

Agreed.

>> USB Power seq :
>>   We are trying to propose library approach, with compatible string match.
>>
>> We should try to have one approach.
>>>
>>>>>>    - Lets also add suspend/resume callback to struct pwrseq
>>>>>>
>>>>> Why suspend/resume can't do at related driver's suspend/resume API?
>>>> Nope...
>>>> The pwrseq library would have taken ownership of resources, so
>>>> related driver cannot suspend the device. Again it is architecture
>>>> specific, but we should have provision to handle this.
>>>>
>>>> The system I am dealing with today, does need suspend/resume
>>>> callback. To be precise, suspend is close to off state for some devices
>>>> or
>>>> they could enter standby or different low power state, but to do that,
>>>> we need same resource as used for ON/OFF functionality.
>>>>
>>> Ok, I will have API for suspend/resume. You can implement it at your own
>>> library or generic one.
> As stated, using a platform device + driver would simplify this, as
> you wouldn't need an API but only a driver. I guess.

Exactly.

Thanks,
Vaibhav

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web