Hi Rafael, Kevin, Ulf,
Looks like there is still some interest/needs in/for this. Any thoughts
on how we can move this forward?
Cheers
Jon
On 28/02/17 15:29, Geert Uytterhoeven wrote:
> Hi Jon,
>
> On Tue, Feb 28, 2017 at 4:18 PM, Jon Hunter <[email protected]> wrote:
>> On 20/09/16 11:28, Jon Hunter wrote:
>>> The Tegra124/210 XUSB subsystem (that consists of both host and device
>>> controllers) is partitioned across 3 PM domains which are:
>>> - XUSBA: Superspeed logic (for USB 3.0)
>>> - XUSBB: Device controller
>>> - XUSBC: Host controller
>>>
>>> These power domains are not nested and can be powered-up and down
>>> independently of one another. In practice different scenarios require
>>> different combinations of the power domains, for example:
>>> - Superspeed host: XUSBA and XUSBC
>>> - Superspeed device: XUSBA and XUSBB
>>>
>>> Although it could be possible to logically nest both the XUSBB and XUSBC
>>> domains under the XUSBA, superspeed may not always be used/required and
>>> so this would keep it on unnecessarily.
>>>
>>> Given that Tegra uses device-tree for describing the hardware, it would
>>> be ideal that the device-tree 'power-domains' property for generic PM
>>> domains could be extended to allow more than one PM domain to be
>>> specified. For example, define the following the Tegra210 xHCI device ...
>>>
>>> usb@70090000 {
>>> compatible = "nvidia,tegra210-xusb";
>>> ...
>>> power-domains = <&pd_xusbhost>, <&pd_xusbss>;
>>> };
>>>
>>> This RFC extends the generic PM domain framework to allow a device to
>>> define more than one PM domain in the device-tree 'power-domains'
>>> property.
>>
>> I wanted to kick this thread again now in the new year and see if there
>> is still some interest in pursuing this?
>>
>> There is still very much a need from a Tegra perspective. I have put all
>> those who responded on TO.
>>
>> I know that a lot of time has passed since we discuss this and so if you
>> are scratching your head wondering what I am harping on about,
>> essentially with this RFC I was looking for a way to support devices
>> that require multiple power domains where the domains do not have a
>> parent-child relationship and so not are nested in anyway.
>>
>> If you need me to elaborate on the need for this, I am happy to do this.
>> My take away from when we discussed this last year, was that there was a
>> need for this.
>
> It definitely makes sense to me, as the "power-domains" DT binding is not
> limited to plain "power areas", but may refer to clock domains, too
> (cfr. the Linux "PM Domain" notion).
>
> For my (Renesas) use case, we have devices that are part of both a power
> area and a clock domain. Currently this is handled by the power area driver
> calling into the clock driver.
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
--
nvpublic
+Björn
On 13 March 2017 at 10:37, Jon Hunter <[email protected]> wrote:
> Hi Rafael, Kevin, Ulf,
>
> Looks like there is still some interest/needs in/for this. Any thoughts
> on how we can move this forward?
At the Linaro Connect last week, I was talking to Björn, Rajendra and
Stephen more about these related issues.
It definitely seems like we need to progress with this somehow,
meaning we need a solution for being able to associate a device with
more than one PM domain. In that context, I don't think genpd based on
its current design, is a good fit to solve the problem.
Instead I think we need something entirely new (perhaps some code can
be borrowed from genpd), which is more similar to the clock/regulator
framework. In other words, what you also were suggesting in a earlier
reply.
In this way, the driver/subsystem gains full flexibility of managing
its device's PM domains, which seems like the best future-proof
solution.
Kind regards
Uffe
>
> Cheers
> Jon
>
> On 28/02/17 15:29, Geert Uytterhoeven wrote:
>> Hi Jon,
>>
>> On Tue, Feb 28, 2017 at 4:18 PM, Jon Hunter <[email protected]> wrote:
>>> On 20/09/16 11:28, Jon Hunter wrote:
>>>> The Tegra124/210 XUSB subsystem (that consists of both host and device
>>>> controllers) is partitioned across 3 PM domains which are:
>>>> - XUSBA: Superspeed logic (for USB 3.0)
>>>> - XUSBB: Device controller
>>>> - XUSBC: Host controller
>>>>
>>>> These power domains are not nested and can be powered-up and down
>>>> independently of one another. In practice different scenarios require
>>>> different combinations of the power domains, for example:
>>>> - Superspeed host: XUSBA and XUSBC
>>>> - Superspeed device: XUSBA and XUSBB
>>>>
>>>> Although it could be possible to logically nest both the XUSBB and XUSBC
>>>> domains under the XUSBA, superspeed may not always be used/required and
>>>> so this would keep it on unnecessarily.
>>>>
>>>> Given that Tegra uses device-tree for describing the hardware, it would
>>>> be ideal that the device-tree 'power-domains' property for generic PM
>>>> domains could be extended to allow more than one PM domain to be
>>>> specified. For example, define the following the Tegra210 xHCI device ...
>>>>
>>>> usb@70090000 {
>>>> compatible = "nvidia,tegra210-xusb";
>>>> ...
>>>> power-domains = <&pd_xusbhost>, <&pd_xusbss>;
>>>> };
>>>>
>>>> This RFC extends the generic PM domain framework to allow a device to
>>>> define more than one PM domain in the device-tree 'power-domains'
>>>> property.
>>>
>>> I wanted to kick this thread again now in the new year and see if there
>>> is still some interest in pursuing this?
>>>
>>> There is still very much a need from a Tegra perspective. I have put all
>>> those who responded on TO.
>>>
>>> I know that a lot of time has passed since we discuss this and so if you
>>> are scratching your head wondering what I am harping on about,
>>> essentially with this RFC I was looking for a way to support devices
>>> that require multiple power domains where the domains do not have a
>>> parent-child relationship and so not are nested in anyway.
>>>
>>> If you need me to elaborate on the need for this, I am happy to do this.
>>> My take away from when we discussed this last year, was that there was a
>>> need for this.
>>
>> It definitely makes sense to me, as the "power-domains" DT binding is not
>> limited to plain "power areas", but may refer to clock domains, too
>> (cfr. the Linux "PM Domain" notion).
>>
>> For my (Renesas) use case, we have devices that are part of both a power
>> area and a clock domain. Currently this is handled by the power area driver
>> calling into the clock driver.
>>
>> Thanks!
>>
>> Gr{oetje,eeting}s,
>>
>> Geert
>>
>> --
>> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>>
>> In personal conversations with technical people, I call myself a hacker. But
>> when I'm talking to journalists I just say "programmer" or something like that.
>> -- Linus Torvalds
>>
>
> --
> nvpublic
Hi Ulf,
On 13/03/17 11:45, Ulf Hansson wrote:
> +Björn
>
> On 13 March 2017 at 10:37, Jon Hunter <[email protected]> wrote:
>> Hi Rafael, Kevin, Ulf,
>>
>> Looks like there is still some interest/needs in/for this. Any thoughts
>> on how we can move this forward?
>
> At the Linaro Connect last week, I was talking to Björn, Rajendra and
> Stephen more about these related issues.
>
> It definitely seems like we need to progress with this somehow,
> meaning we need a solution for being able to associate a device with
> more than one PM domain. In that context, I don't think genpd based on
> its current design, is a good fit to solve the problem.
>
> Instead I think we need something entirely new (perhaps some code can
> be borrowed from genpd), which is more similar to the clock/regulator
> framework. In other words, what you also were suggesting in a earlier
> reply.
> In this way, the driver/subsystem gains full flexibility of managing
> its device's PM domains, which seems like the best future-proof
> solution.
I agree, I think that that would give us the most flexibility to handle
whatever scenario. However, I was thinking that we could still use the
genpd core to register pm-domains with and control. My thought was to
allow devices to have a bindings with multiple pm-domains ...
dev-xyz {
...
power-domains = <&domain-a>, <&domain-b>;
};
Then in the genpd core we do having something like ...
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index e697dec9d25b..d1ae6ddf4903 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2026,6 +2026,15 @@ int genpd_dev_pm_attach(struct device *dev)
"samsung,power-domain", 0);
if (!pd_args.np)
return -ENOENT;
+ } else if (ret > 1) {
+ /*
+ * If there are more than one PM domain defined for a device,
+ * then these need to be manually controlled by the device
+ * driver because the genpd core cannot bind a device with
+ * more than one PM domain.
+ */
+ dev_dbg(dev, "cannot add PM domains, %d detected!\n", ret);
+ return 0;
}
Then add some new public APIs for getting and controlling the pm-domains ...
struct generic_pm_domain *pm_genpd_get(struct device *dev, char *name);
- Use 'dev->of_node' to look-up pm-domain if populated, else uses name.
struct generic_pm_domain *of_pm_genpd_get(struct device *dev, int index);
void pm_genpd_put(struct generic_pm_domain *pd);
int pm_genpd_power_on(struct generic_pm_domain *pd);
int pm_genpd_power_off(struct generic_pm_domain *pd);
- Power on/off APIs would be synchronous types
Are there any potential pitfalls of the above?
Cheers
Jon
--
nvpublic
Hi Jon,
On Mon, Mar 13, 2017 at 3:09 PM, Jon Hunter <[email protected]> wrote:
> On 13/03/17 11:45, Ulf Hansson wrote:
>> +Björn
>>
>> On 13 March 2017 at 10:37, Jon Hunter <[email protected]> wrote:
>>> Looks like there is still some interest/needs in/for this. Any thoughts
>>> on how we can move this forward?
>>
>> At the Linaro Connect last week, I was talking to Björn, Rajendra and
>> Stephen more about these related issues.
>>
>> It definitely seems like we need to progress with this somehow,
>> meaning we need a solution for being able to associate a device with
>> more than one PM domain. In that context, I don't think genpd based on
>> its current design, is a good fit to solve the problem.
>>
>> Instead I think we need something entirely new (perhaps some code can
>> be borrowed from genpd), which is more similar to the clock/regulator
>> framework. In other words, what you also were suggesting in a earlier
>> reply.
>> In this way, the driver/subsystem gains full flexibility of managing
>> its device's PM domains, which seems like the best future-proof
>> solution.
>
> I agree, I think that that would give us the most flexibility to handle
> whatever scenario. However, I was thinking that we could still use the
> genpd core to register pm-domains with and control. My thought was to
> allow devices to have a bindings with multiple pm-domains ...
>
> dev-xyz {
> ...
> power-domains = <&domain-a>, <&domain-b>;
> };
>
> Then in the genpd core we do having something like ...
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index e697dec9d25b..d1ae6ddf4903 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2026,6 +2026,15 @@ int genpd_dev_pm_attach(struct device *dev)
> "samsung,power-domain", 0);
> if (!pd_args.np)
> return -ENOENT;
> + } else if (ret > 1) {
> + /*
> + * If there are more than one PM domain defined for a device,
> + * then these need to be manually controlled by the device
> + * driver because the genpd core cannot bind a device with
Which device driver?
The driver for the device that belongs to multiple PM domains?
The PM domain providers?
> + * more than one PM domain.
> + */
> + dev_dbg(dev, "cannot add PM domains, %d detected!\n", ret);
> + return 0;
> }
>
> Then add some new public APIs for getting and controlling the pm-domains ...
>
> struct generic_pm_domain *pm_genpd_get(struct device *dev, char *name);
> - Use 'dev->of_node' to look-up pm-domain if populated, else uses name.
>
> struct generic_pm_domain *of_pm_genpd_get(struct device *dev, int index);
> void pm_genpd_put(struct generic_pm_domain *pd);
> int pm_genpd_power_on(struct generic_pm_domain *pd);
> int pm_genpd_power_off(struct generic_pm_domain *pd);
> - Power on/off APIs would be synchronous types
>
> Are there any potential pitfalls of the above?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On 13/03/17 14:19, Geert Uytterhoeven wrote:
> Hi Jon,
>
> On Mon, Mar 13, 2017 at 3:09 PM, Jon Hunter <[email protected]> wrote:
>> On 13/03/17 11:45, Ulf Hansson wrote:
>>> +Björn
>>>
>>> On 13 March 2017 at 10:37, Jon Hunter <[email protected]> wrote:
>>>> Looks like there is still some interest/needs in/for this. Any thoughts
>>>> on how we can move this forward?
>>>
>>> At the Linaro Connect last week, I was talking to Björn, Rajendra and
>>> Stephen more about these related issues.
>>>
>>> It definitely seems like we need to progress with this somehow,
>>> meaning we need a solution for being able to associate a device with
>>> more than one PM domain. In that context, I don't think genpd based on
>>> its current design, is a good fit to solve the problem.
>>>
>>> Instead I think we need something entirely new (perhaps some code can
>>> be borrowed from genpd), which is more similar to the clock/regulator
>>> framework. In other words, what you also were suggesting in a earlier
>>> reply.
>>> In this way, the driver/subsystem gains full flexibility of managing
>>> its device's PM domains, which seems like the best future-proof
>>> solution.
>>
>> I agree, I think that that would give us the most flexibility to handle
>> whatever scenario. However, I was thinking that we could still use the
>> genpd core to register pm-domains with and control. My thought was to
>> allow devices to have a bindings with multiple pm-domains ...
>>
>> dev-xyz {
>> ...
>> power-domains = <&domain-a>, <&domain-b>;
>> };
>>
>> Then in the genpd core we do having something like ...
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index e697dec9d25b..d1ae6ddf4903 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -2026,6 +2026,15 @@ int genpd_dev_pm_attach(struct device *dev)
>> "samsung,power-domain", 0);
>> if (!pd_args.np)
>> return -ENOENT;
>> + } else if (ret > 1) {
>> + /*
>> + * If there are more than one PM domain defined for a device,
>> + * then these need to be manually controlled by the device
>> + * driver because the genpd core cannot bind a device with
>
> Which device driver?
> The driver for the device that belongs to multiple PM domains?
Yes, exactly. So maybe I would need to say ... "manually controlled by
the driver for *this* device ..."
Jon
--
nvpublic
Hi Jon,
On Mon, Mar 13, 2017 at 3:27 PM, Jon Hunter <[email protected]> wrote:
> On 13/03/17 14:19, Geert Uytterhoeven wrote:
>> On Mon, Mar 13, 2017 at 3:09 PM, Jon Hunter <[email protected]> wrote:
>>> On 13/03/17 11:45, Ulf Hansson wrote:
>>>> +Björn
>>>>
>>>> On 13 March 2017 at 10:37, Jon Hunter <[email protected]> wrote:
>>>>> Looks like there is still some interest/needs in/for this. Any thoughts
>>>>> on how we can move this forward?
>>>>
>>>> At the Linaro Connect last week, I was talking to Björn, Rajendra and
>>>> Stephen more about these related issues.
>>>>
>>>> It definitely seems like we need to progress with this somehow,
>>>> meaning we need a solution for being able to associate a device with
>>>> more than one PM domain. In that context, I don't think genpd based on
>>>> its current design, is a good fit to solve the problem.
>>>>
>>>> Instead I think we need something entirely new (perhaps some code can
>>>> be borrowed from genpd), which is more similar to the clock/regulator
>>>> framework. In other words, what you also were suggesting in a earlier
>>>> reply.
>>>> In this way, the driver/subsystem gains full flexibility of managing
>>>> its device's PM domains, which seems like the best future-proof
>>>> solution.
>>>
>>> I agree, I think that that would give us the most flexibility to handle
>>> whatever scenario. However, I was thinking that we could still use the
>>> genpd core to register pm-domains with and control. My thought was to
>>> allow devices to have a bindings with multiple pm-domains ...
>>>
>>> dev-xyz {
>>> ...
>>> power-domains = <&domain-a>, <&domain-b>;
>>> };
>>>
>>> Then in the genpd core we do having something like ...
>>>
>>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>> index e697dec9d25b..d1ae6ddf4903 100644
>>> --- a/drivers/base/power/domain.c
>>> +++ b/drivers/base/power/domain.c
>>> @@ -2026,6 +2026,15 @@ int genpd_dev_pm_attach(struct device *dev)
>>> "samsung,power-domain", 0);
>>> if (!pd_args.np)
>>> return -ENOENT;
>>> + } else if (ret > 1) {
>>> + /*
>>> + * If there are more than one PM domain defined for a device,
>>> + * then these need to be manually controlled by the device
>>> + * driver because the genpd core cannot bind a device with
>>
>> Which device driver?
>> The driver for the device that belongs to multiple PM domains?
>
> Yes, exactly. So maybe I would need to say ... "manually controlled by
> the driver for *this* device ..."
That looks a bit cumbersome to me.
Power (and clock) domains are platform features. Any IP core may show up
in a new SoC, and suddenly have become part of one or more PM Domains.
Having to handle that in each individual driver will cause lots of churn.
Especially as the multiple PM Domains a device may belong to may be
fairly orthogonal to each other.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 13 March 2017 at 15:09, Jon Hunter <[email protected]> wrote:
> Hi Ulf,
>
> On 13/03/17 11:45, Ulf Hansson wrote:
>> +Björn
>>
>> On 13 March 2017 at 10:37, Jon Hunter <[email protected]> wrote:
>>> Hi Rafael, Kevin, Ulf,
>>>
>>> Looks like there is still some interest/needs in/for this. Any thoughts
>>> on how we can move this forward?
>>
>> At the Linaro Connect last week, I was talking to Björn, Rajendra and
>> Stephen more about these related issues.
>>
>> It definitely seems like we need to progress with this somehow,
>> meaning we need a solution for being able to associate a device with
>> more than one PM domain. In that context, I don't think genpd based on
>> its current design, is a good fit to solve the problem.
>>
>> Instead I think we need something entirely new (perhaps some code can
>> be borrowed from genpd), which is more similar to the clock/regulator
>> framework. In other words, what you also were suggesting in a earlier
>> reply.
>> In this way, the driver/subsystem gains full flexibility of managing
>> its device's PM domains, which seems like the best future-proof
>> solution.
>
> I agree, I think that that would give us the most flexibility to handle
> whatever scenario. However, I was thinking that we could still use the
> genpd core to register pm-domains with and control. My thought was to
> allow devices to have a bindings with multiple pm-domains ...
>
> dev-xyz {
> ...
> power-domains = <&domain-a>, <&domain-b>;
> };
This could work. However, let's involve DT maintainers to make sure we
get their input to this. Perhaps they prefer a different approach.
>
> Then in the genpd core we do having something like ...
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index e697dec9d25b..d1ae6ddf4903 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2026,6 +2026,15 @@ int genpd_dev_pm_attach(struct device *dev)
> "samsung,power-domain", 0);
> if (!pd_args.np)
> return -ENOENT;
> + } else if (ret > 1) {
> + /*
> + * If there are more than one PM domain defined for a device,
> + * then these need to be manually controlled by the device
> + * driver because the genpd core cannot bind a device with
> + * more than one PM domain.
> + */
> + dev_dbg(dev, "cannot add PM domains, %d detected!\n", ret);
> + return 0;
> }
>
> Then add some new public APIs for getting and controlling the pm-domains ...
>
> struct generic_pm_domain *pm_genpd_get(struct device *dev, char *name);
> - Use 'dev->of_node' to look-up pm-domain if populated, else uses name.
>
> struct generic_pm_domain *of_pm_genpd_get(struct device *dev, int index);
> void pm_genpd_put(struct generic_pm_domain *pd);
> int pm_genpd_power_on(struct generic_pm_domain *pd);
> int pm_genpd_power_off(struct generic_pm_domain *pd);
> - Power on/off APIs would be synchronous types
>
> Are there any potential pitfalls of the above?
So if I understand correctly, you would like to extend genpd with some
new APIs. It's worth a try, however my main worries are these:
1) These new API must not be allowed to be abused.
I have seen that before as when people try to handle some corner
cases, I don't want to that to happen again. To avoid that, perhaps we
should continue the re-structuring and thus move structures/datas that
are currently public, to be internal to genpd. To get a clean
interface.
2) I wouldn't be surprised if we run into some tricky corner cases, as
we get a mixture of devices handled by runtime PM and in some other
cases via new APIs. Perhaps that can be sorted out!?
Kind regards
Uffe
Hi Geert,
On 13/03/17 14:38, Geert Uytterhoeven wrote:
> Hi Jon,
>
> On Mon, Mar 13, 2017 at 3:27 PM, Jon Hunter <[email protected]> wrote:
>> On 13/03/17 14:19, Geert Uytterhoeven wrote:
>>> On Mon, Mar 13, 2017 at 3:09 PM, Jon Hunter <[email protected]> wrote:
>>>> On 13/03/17 11:45, Ulf Hansson wrote:
>>>>> +Björn
>>>>>
>>>>> On 13 March 2017 at 10:37, Jon Hunter <[email protected]> wrote:
>>>>>> Looks like there is still some interest/needs in/for this. Any thoughts
>>>>>> on how we can move this forward?
>>>>>
>>>>> At the Linaro Connect last week, I was talking to Björn, Rajendra and
>>>>> Stephen more about these related issues.
>>>>>
>>>>> It definitely seems like we need to progress with this somehow,
>>>>> meaning we need a solution for being able to associate a device with
>>>>> more than one PM domain. In that context, I don't think genpd based on
>>>>> its current design, is a good fit to solve the problem.
>>>>>
>>>>> Instead I think we need something entirely new (perhaps some code can
>>>>> be borrowed from genpd), which is more similar to the clock/regulator
>>>>> framework. In other words, what you also were suggesting in a earlier
>>>>> reply.
>>>>> In this way, the driver/subsystem gains full flexibility of managing
>>>>> its device's PM domains, which seems like the best future-proof
>>>>> solution.
>>>>
>>>> I agree, I think that that would give us the most flexibility to handle
>>>> whatever scenario. However, I was thinking that we could still use the
>>>> genpd core to register pm-domains with and control. My thought was to
>>>> allow devices to have a bindings with multiple pm-domains ...
>>>>
>>>> dev-xyz {
>>>> ...
>>>> power-domains = <&domain-a>, <&domain-b>;
>>>> };
>>>>
>>>> Then in the genpd core we do having something like ...
>>>>
>>>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>>> index e697dec9d25b..d1ae6ddf4903 100644
>>>> --- a/drivers/base/power/domain.c
>>>> +++ b/drivers/base/power/domain.c
>>>> @@ -2026,6 +2026,15 @@ int genpd_dev_pm_attach(struct device *dev)
>>>> "samsung,power-domain", 0);
>>>> if (!pd_args.np)
>>>> return -ENOENT;
>>>> + } else if (ret > 1) {
>>>> + /*
>>>> + * If there are more than one PM domain defined for a device,
>>>> + * then these need to be manually controlled by the device
>>>> + * driver because the genpd core cannot bind a device with
>>>
>>> Which device driver?
>>> The driver for the device that belongs to multiple PM domains?
>>
>> Yes, exactly. So maybe I would need to say ... "manually controlled by
>> the driver for *this* device ..."
>
> That looks a bit cumbersome to me.
>
> Power (and clock) domains are platform features. Any IP core may show up
> in a new SoC, and suddenly have become part of one or more PM Domains.
> Having to handle that in each individual driver will cause lots of churn.
> Especially as the multiple PM Domains a device may belong to may be
> fairly orthogonal to each other.
Yes that's true. However, in order to make this work for everyone and
have a generic solution I am not sure how that can be avoided. If there
are cases where devices require multiple PM domains but the usage is
quite simple (ie. all on when device in use and all off when device is
not in use), I could see the APIs I proposed being extended to include
some _bulk() versions like we have for regulators.
Cheers
Jon
--
nvpublic
Hey Jon,
>>> Looks like there is still some interest/needs in/for this. Any thoughts
>>> on how we can move this forward?
>>
>> At the Linaro Connect last week, I was talking to Björn, Rajendra and
>> Stephen more about these related issues.
>>
>> It definitely seems like we need to progress with this somehow,
>> meaning we need a solution for being able to associate a device with
>> more than one PM domain. In that context, I don't think genpd based on
>> its current design, is a good fit to solve the problem.
>>
>> Instead I think we need something entirely new (perhaps some code can
>> be borrowed from genpd), which is more similar to the clock/regulator
>> framework. In other words, what you also were suggesting in a earlier
>> reply.
>> In this way, the driver/subsystem gains full flexibility of managing
>> its device's PM domains, which seems like the best future-proof
>> solution.
>
> I agree, I think that that would give us the most flexibility to handle
> whatever scenario. However, I was thinking that we could still use the
> genpd core to register pm-domains with and control. My thought was to
> allow devices to have a bindings with multiple pm-domains ...
>
> dev-xyz {
> ...
> power-domains = <&domain-a>, <&domain-b>;
> };
>
> Then in the genpd core we do having something like ...
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index e697dec9d25b..d1ae6ddf4903 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2026,6 +2026,15 @@ int genpd_dev_pm_attach(struct device *dev)
> "samsung,power-domain", 0);
> if (!pd_args.np)
> return -ENOENT;
> + } else if (ret > 1) {
> + /*
> + * If there are more than one PM domain defined for a device,
> + * then these need to be manually controlled by the device
> + * driver because the genpd core cannot bind a device with
> + * more than one PM domain.
> + */
> + dev_dbg(dev, "cannot add PM domains, %d detected!\n", ret);
> + return 0;
> }
>
> Then add some new public APIs for getting and controlling the pm-domains ...
>
> struct generic_pm_domain *pm_genpd_get(struct device *dev, char *name);
> - Use 'dev->of_node' to look-up pm-domain if populated, else uses name.
>
> struct generic_pm_domain *of_pm_genpd_get(struct device *dev, int index);
> void pm_genpd_put(struct generic_pm_domain *pd);
> int pm_genpd_power_on(struct generic_pm_domain *pd);
> int pm_genpd_power_off(struct generic_pm_domain *pd);
> - Power on/off APIs would be synchronous types
These would also need some kind of usecounting I guess, since genpd
otherwise relies on runtime PM to do the usecounting.
This overall seems like a reasonable approach to solve the problem we
have. While we discussed this approach at connect, we thought it would
be a good idea to bring out some RFC on these lines to get the
discussion going. Do you think you would be able to work on some quick
RFC around these lines, else if you think you would be busy in the near
term I can help with hacking up the changes as well.
regards,
Rajendra
>
> Are there any potential pitfalls of the above?
>
> Cheers
> Jon
>
On 13/03/17 14:42, Ulf Hansson wrote:
> On 13 March 2017 at 15:09, Jon Hunter <[email protected]> wrote:
>> Hi Ulf,
>>
>> On 13/03/17 11:45, Ulf Hansson wrote:
>>> +Björn
>>>
>>> On 13 March 2017 at 10:37, Jon Hunter <[email protected]> wrote:
>>>> Hi Rafael, Kevin, Ulf,
>>>>
>>>> Looks like there is still some interest/needs in/for this. Any thoughts
>>>> on how we can move this forward?
>>>
>>> At the Linaro Connect last week, I was talking to Björn, Rajendra and
>>> Stephen more about these related issues.
>>>
>>> It definitely seems like we need to progress with this somehow,
>>> meaning we need a solution for being able to associate a device with
>>> more than one PM domain. In that context, I don't think genpd based on
>>> its current design, is a good fit to solve the problem.
>>>
>>> Instead I think we need something entirely new (perhaps some code can
>>> be borrowed from genpd), which is more similar to the clock/regulator
>>> framework. In other words, what you also were suggesting in a earlier
>>> reply.
>>> In this way, the driver/subsystem gains full flexibility of managing
>>> its device's PM domains, which seems like the best future-proof
>>> solution.
>>
>> I agree, I think that that would give us the most flexibility to handle
>> whatever scenario. However, I was thinking that we could still use the
>> genpd core to register pm-domains with and control. My thought was to
>> allow devices to have a bindings with multiple pm-domains ...
>>
>> dev-xyz {
>> ...
>> power-domains = <&domain-a>, <&domain-b>;
>> };
>
> This could work. However, let's involve DT maintainers to make sure we
> get their input to this. Perhaps they prefer a different approach.
No problem. I should point out the above is for the #power-domain-cells
= <0> case.
>>
>> Then in the genpd core we do having something like ...
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index e697dec9d25b..d1ae6ddf4903 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -2026,6 +2026,15 @@ int genpd_dev_pm_attach(struct device *dev)
>> "samsung,power-domain", 0);
>> if (!pd_args.np)
>> return -ENOENT;
>> + } else if (ret > 1) {
>> + /*
>> + * If there are more than one PM domain defined for a device,
>> + * then these need to be manually controlled by the device
>> + * driver because the genpd core cannot bind a device with
>> + * more than one PM domain.
>> + */
>> + dev_dbg(dev, "cannot add PM domains, %d detected!\n", ret);
>> + return 0;
>> }
>>
>> Then add some new public APIs for getting and controlling the pm-domains ...
>>
>> struct generic_pm_domain *pm_genpd_get(struct device *dev, char *name);
>> - Use 'dev->of_node' to look-up pm-domain if populated, else uses name.
>>
>> struct generic_pm_domain *of_pm_genpd_get(struct device *dev, int index);
>> void pm_genpd_put(struct generic_pm_domain *pd);
>> int pm_genpd_power_on(struct generic_pm_domain *pd);
>> int pm_genpd_power_off(struct generic_pm_domain *pd);
>> - Power on/off APIs would be synchronous types
>>
>> Are there any potential pitfalls of the above?
>
> So if I understand correctly, you would like to extend genpd with some
> new APIs. It's worth a try, however my main worries are these:
>
> 1) These new API must not be allowed to be abused.
> I have seen that before as when people try to handle some corner
> cases, I don't want to that to happen again. To avoid that, perhaps we
> should continue the re-structuring and thus move structures/datas that
> are currently public, to be internal to genpd. To get a clean
> interface.
OK, fair enough. Any in particular you are concerned about?
> 2) I wouldn't be surprised if we run into some tricky corner cases, as
> we get a mixture of devices handled by runtime PM and in some other
> cases via new APIs. Perhaps that can be sorted out!?
Right that is a concern, however, I think that in the long-term we would
be better off with the power-domains being controlled by the same
underlying code as opposed to something different.
Cheers
Jon
--
nvpublic
Hi Rajendra,
On 15/03/17 03:47, Nayak, Rajendra wrote:
> Hey Jon,
>
>>>> Looks like there is still some interest/needs in/for this. Any thoughts
>>>> on how we can move this forward?
>>>
>>> At the Linaro Connect last week, I was talking to Björn, Rajendra and
>>> Stephen more about these related issues.
>>>
>>> It definitely seems like we need to progress with this somehow,
>>> meaning we need a solution for being able to associate a device with
>>> more than one PM domain. In that context, I don't think genpd based on
>>> its current design, is a good fit to solve the problem.
>>>
>>> Instead I think we need something entirely new (perhaps some code can
>>> be borrowed from genpd), which is more similar to the clock/regulator
>>> framework. In other words, what you also were suggesting in a earlier
>>> reply.
>>> In this way, the driver/subsystem gains full flexibility of managing
>>> its device's PM domains, which seems like the best future-proof
>>> solution.
>>
>> I agree, I think that that would give us the most flexibility to handle
>> whatever scenario. However, I was thinking that we could still use the
>> genpd core to register pm-domains with and control. My thought was to
>> allow devices to have a bindings with multiple pm-domains ...
>>
>> dev-xyz {
>> ...
>> power-domains = <&domain-a>, <&domain-b>;
>> };
>>
>> Then in the genpd core we do having something like ...
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index e697dec9d25b..d1ae6ddf4903 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -2026,6 +2026,15 @@ int genpd_dev_pm_attach(struct device *dev)
>>
>> "samsung,power-domain", 0);
>> if (!pd_args.np)
>> return -ENOENT;
>> + } else if (ret > 1) {
>> + /*
>> + * If there are more than one PM domain defined for a
>> device,
>> + * then these need to be manually controlled by the
>> device
>> + * driver because the genpd core cannot bind a device
>> with
>> + * more than one PM domain.
>> + */
>> + dev_dbg(dev, "cannot add PM domains, %d detected!\n",
>> ret);
>> + return 0;
>> }
>>
>> Then add some new public APIs for getting and controlling the
>> pm-domains ...
>>
>> struct generic_pm_domain *pm_genpd_get(struct device *dev, char *name);
>> - Use 'dev->of_node' to look-up pm-domain if populated, else uses name.
>>
>> struct generic_pm_domain *of_pm_genpd_get(struct device *dev, int index);
>> void pm_genpd_put(struct generic_pm_domain *pd);
>> int pm_genpd_power_on(struct generic_pm_domain *pd);
>> int pm_genpd_power_off(struct generic_pm_domain *pd);
>> - Power on/off APIs would be synchronous types
>
> These would also need some kind of usecounting I guess, since genpd
> otherwise relies on runtime PM to do the usecounting.
Yes exactly.
> This overall seems like a reasonable approach to solve the problem we
> have. While we discussed this approach at connect, we thought it would
> be a good idea to bring out some RFC on these lines to get the
> discussion going. Do you think you would be able to work on some quick
> RFC around these lines, else if you think you would be busy in the near
> term I can help with hacking up the changes as well.
Yes I plan too. I will let you know if I get side tracked on something else.
Cheers!
Jon
--
nvpublic