Re: [PATCH v6 0/8] power: add power sequence library
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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] power: add power sequence library
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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] power: add power sequence library
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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] power: add power sequence library
[...] 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] power: add power sequence library
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
Re: [PATCH v6 0/8] power: add power sequence library
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 compatibl
Re: [PATCH v6 0/8] power: add power sequence library
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 unchangi
Re: [PATCH v6 0/8] power: add power sequence library
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
Re: [PATCH v6 0/8] power: add power sequence library
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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] power: add power sequence library
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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] power: add power sequence library
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. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] power: add power sequence library
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. Thanks, Vaibhav Changes for v6: - Add Matthias Kaehlcke's Reviewed-by and Tested-by. (patch [2/6]) - Change chipidea core of_node assignment for coming user. (patch [5/6]) - Applies Joshua Clayton's three dts changes for two boards, the USB device's reg has only #address-cells, but without #size-cells. Changes for v5: - Delete pwrseq_register/pwrseq_unregister, which is useless currently - Fix the linker error when the pwrseq user is compiled as module Changes for v4: - Create the patch on next-20160722 - Fix the of_node is not NULL after chipidea driver is unbinded [Patch 5/6] - Using more friendly wait method for reset gpio [Patch 2/6] - Support multiple input clocks [Patch 2/6] - Add Rob Herring's ack for DT changes - Add Joshua Clayton's Tested-by Changes for v3: - Delete "power-sequence" property at binding-doc, and change related code at both library and user code. - Change binding-doc example node name with Rob's comments - of_get_named_gpio_flags only gets the gpio, but without setting gpio flags, add additional code request gpio with proper gpio flags - Add Philipp Zabel's Ack and MAINTAINER's entry Changes for v2: - Delete "pwrseq" prefix and clock-names for properties at dt binding - Should use structure not but its pointer for kzalloc - Since chipidea core has no of_node, let core's of_node equals glue layer'
[PATCH v6 0/8] power: add power sequence library
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 Changes for v6: - Add Matthias Kaehlcke's Reviewed-by and Tested-by. (patch [2/6]) - Change chipidea core of_node assignment for coming user. (patch [5/6]) - Applies Joshua Clayton's three dts changes for two boards, the USB device's reg has only #address-cells, but without #size-cells. Changes for v5: - Delete pwrseq_register/pwrseq_unregister, which is useless currently - Fix the linker error when the pwrseq user is compiled as module Changes for v4: - Create the patch on next-20160722 - Fix the of_node is not NULL after chipidea driver is unbinded [Patch 5/6] - Using more friendly wait method for reset gpio [Patch 2/6] - Support multiple input clocks [Patch 2/6] - Add Rob Herring's ack for DT changes - Add Joshua Clayton's Tested-by Changes for v3: - Delete "power-sequence" property at binding-doc, and change related code at both library and user code. - Change binding-doc example node name with Rob's comments - of_get_named_gpio_flags only gets the gpio, but without setting gpio flags, add additional code request gpio with proper gpio flags - Add Philipp Zabel's Ack and MAINTAINER's entry Changes for v2: - Delete "pwrseq" prefix and clock-names for properties at dt binding - Should use structure not but its pointer for kzalloc - Since chipidea core has no of_node, let core's of_node equals glue layer's at core's probe Joshua Clayton (2): ARM: dts: imx6qdl: Enable usb node children with ARM: dts: imx6q-evi: Fix onboard hub reset line Peter Chen (6): binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library power: add power sequence library binding-doc: usb: usb-device: add optional properties for power sequence usb: core: add power sequence handling for USB devices usb: chipidea: let chipidea core device of_node equal's glue layer device of_node ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property .../bindings/power/pwrseq/pwrseq-generic.txt | 48 ++ .../devicetree/bindings/usb/usb-device.txt | 10 +- MAINTAINERS| 9 ++ arch/arm/boot/dts/imx6q-evi.dts| 25 +-- arch/arm/boot/dts/imx6qdl-udoo.dtsi| 26 ++-- arch/arm/boot/dts/imx6qdl.dtsi | 6 + drivers/power/Kconfig | 1 + drivers/power/Makefile | 1 + drivers/power/pwrseq/Kconfig | 20 +++ drivers/power/pwrseq/Makefile | 2 + drivers/power/pwrseq/core.c| 62 drivers/power/pwrseq/pwrseq_generic.c | 168 + drivers/usb/chipidea/core.c| 27 +++- drivers/usb/core/Makefile | 1 + drivers/usb/core/hub.c | 12 +- drivers/usb/core/hub.h | 12 ++ drivers/usb/core/pwrseq.c | 100 include/linux/power/pwrseq.h | 47 ++ 18 files changed, 536 insertions(+), 41 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt create mode 100644 drivers/power/pwrseq/Kconfig create mode 100644 drivers/power/pwrseq/Makefile create mode 100644 drivers/power/pwrseq/core.c create mode 100644 drivers/power/pwrseq/pwrseq_generic.c create mode 100644 drivers/usb/core/pwrseq.c create mode 100644 include/linux/power/pwrseq.h -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html