On 11/02/16 10:13, Jon Hunter wrote: > > On 11/02/16 09:57, Ulf Hansson wrote: >> On 11 February 2016 at 10:13, Jon Hunter <jonathanh@xxxxxxxxxx> wrote: >>> >>> On 10/02/16 18:25, Ulf Hansson wrote: >>> >>> [snip] >>> >>>>>> Perhaps there's a way to allow the generic PM domain to control this >>>>>> by itself. If we for example used the struct device corresponding to >>>>>> the powergate driver, genpd could use it to distinguish between >>>>>> various instances of genpd structs..!? Maybe it would simplify the way >>>>>> to deal with removing domains? >>>>> >>>>> Yes, that would be ideal. However, would have require changing >>>>> genpd_init()? I am not sure how genpd would be able to access the device >>>>> struct for the powergate driver because we don't provide this via any >>>>> API I am aware of? And I am guessing that you don't wish to expose the >>>>> gpd_list to the world either. >>>>> >>>>> If there is an easy way, I am open to it, but looking at it today, I am >>>>> not sure I see a simple way in which we could add a new API to do this. >>>>> However, may be I am missing something! >>>> >>>> If we add a new __pm_genpd_init() API, that could require a struct >>>> device to be provided. That API will thus invoke the existing >>>> pm_genpd_init() but also deal with the extra things needed here. >>>> >>>> I would also allow such an API to return an error code. >>>> >>>> Correspondingly, pm_genpd_remove() could be required to be provided >>>> with a struct device. >>>> >>>> Existing users of pm_genpd_init() can then convert to >>>> __pm_genpd_init() whenever suitable. >>>> >>>> Of course, another option is just to add new member in the genpd >>>> struct for the struct *device. The caller of pm_genpd_init() could >>>> check it, but allow it to be NULL. Although, the pm_genpd_remove() API >>>> would require that pointer to the struct device to be set... >>>> >>>> What do you think? >>> >>> Yes, sounds good. May be it is simpler just to add a new member and let >>> the platform genpd driver handle it. >>> >>> I am wondering if in addition to pm_genpd_remove(), we then just have a >>> function called pm_genpd_remove_tail(), which allows you to pass the >>> struct device pointer and will remove the last pm-domain from the >>> gpd_list and return the genpd pointer if successful. Internally, it will >>> call pm_genpd_remove(). It seems to me that if there are nested >>> pm-domains, then we probably want to remove them starting from the tail >>> as opposed to the head. >>> >>> How does that sound? >> >> Why not make pm_genpd_remove() to behave as you describe for >> pm_genpd_remove_tail()? >> That's probably the only sane way to remove genpds anyhow!? > > Simply to offer flexibility. I could see that for some devices that have > no dependencies between pm-domains and have a static list of pm-domains, > they can simply call pm_genpd_remove() for a given pm-domain. However, > that said, I can envision a case where a single pm-domain would be > removed by itself and so may be there is no benefit? By the way, do you think that instead of passing the struct device * to pm_genpd_remove(), we should just have a void *dev_id in the same way the request_irq()/free_irq() work? In other words, it would allow people to use the struct device or struct device_node, etc? Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
