Messages in this thread | | | Subject | Re: [PATCH v6 0/8] power: add power sequence library | From | Vaibhav Hiremath <> | Date | Tue, 6 Sep 2016 15:48:34 +0530 |
| |
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
| |