lkml.org 
[lkml]   [2018]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
    [...]

    >>
    >> +/**
    >> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
    >> + * @dev: Device to attach.
    >> + * @index: The index of the PM domain.
    >> + *
    >> + * Parse device's OF node to find a PM domain specifier at the provided @index.
    >> + * If such is found, allocates a new device and attaches it to retrieved
    >> + * pm_domain ops.
    >> + *
    >> + * Returns the allocated device if successfully attached PM domain, NULL when
    >> + * the device don't need a PM domain or have a single PM domain, else PTR_ERR()
    >> + * in case of failures. Note that if a power-domain exists for the device, but
    >> + * cannot be found or turned on, then return PTR_ERR(-EPROBE_DEFER) to ensure
    >> + * that the device is not probed and to re-try again later.
    >> + */
    >> +struct device *genpd_dev_pm_attach_by_id(struct device *dev,
    >> + unsigned int index)
    >> +{
    >> + struct device *genpd_dev;
    >> + int num_domains;
    >> + int ret;
    >> +
    >> + if (!dev->of_node)
    >> + return NULL;
    >> +
    >> + /* Deal only with devices using multiple PM domains. */
    >> + num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
    >> + "#power-domain-cells");
    >> + if (num_domains < 2 || index >= num_domains)
    >> + return NULL;
    >> +
    >> + /* Allocate and register device on the genpd bus. */
    >> + genpd_dev = kzalloc(sizeof(*genpd_dev), GFP_KERNEL);
    >> + if (!genpd_dev)
    >> + return ERR_PTR(-ENOMEM);
    >> +
    >> + dev_set_name(genpd_dev, "genpd:%u:%s", index, dev_name(dev));
    >> + genpd_dev->bus = &genpd_bus_type;
    >> + genpd_dev->release = genpd_release_dev;
    >> +
    >> + ret = device_register(genpd_dev);
    >> + if (ret) {
    >> + kfree(genpd_dev);
    >> + return ERR_PTR(ret);
    >> + }
    >> +
    >> + /* Try to attach the device to the PM domain at the specified index. */
    >> + ret = __genpd_dev_pm_attach(genpd_dev, dev->of_node, index);
    >> + if (ret < 1) {
    >> + device_unregister(genpd_dev);
    >> + return ret ? ERR_PTR(ret) : NULL;
    >> + }
    >> +
    >> + pm_runtime_set_active(genpd_dev);
    >> + pm_runtime_enable(genpd_dev);
    >> +
    >> + return genpd_dev;
    >> +}
    >> +EXPORT_SYMBOL_GPL(genpd_dev_pm_attach_by_id);
    >
    > Thanks for sending this. Believe it or not this has still been on my to-do list
    > and so we definitely need a solution for Tegra.
    >
    > Looking at the above it appears that additional power-domains exposed as devices
    > to the client device. So I assume that this means that the drivers for devices
    > with multiple power-domains will need to call RPM APIs for each of these
    > additional power-domains. Is that correct?

    They can, but should not!

    Instead, the driver shall use device_link_add() and device_link_del(),
    dynamically, depending on what PM domain that their original device
    needs for the current running use case.

    In that way, they keep existing runtime PM deployment, operating on
    its original device.

    >
    > If so, I can see that that would work, but it does not seem to fit the RPM model
    > where currently for something like device clocks, the RPM callbacks can handle
    > all clocks at once.
    >
    > I was wondering why we could not add a list of genpd domains to the
    > dev_pm_domain struct for the device? For example ...

    See above answer, hopefully that explains it.

    FYI, that's why I also discovered the bug around using device links
    with runtime PM during probe.
    https://patchwork.kernel.org/patch/10408825/

    >
    > diff --git a/include/linux/pm.h b/include/linux/pm.h
    > index e723b78d8357..a11d6db3c077 100644
    > --- a/include/linux/pm.h
    > +++ b/include/linux/pm.h
    > @@ -659,6 +659,7 @@ extern void dev_pm_put_subsys_data(struct device *dev);
    > * subsystem-level and driver-level callbacks.
    > */
    > struct dev_pm_domain {
    > + struct list_head genpd_list;
    > struct dev_pm_ops ops;
    > void (*detach)(struct device *dev, bool power_off);
    > int (*activate)(struct device *dev);
    > @@ -666,6 +667,11 @@ struct dev_pm_domain {
    > void (*dismiss)(struct device *dev);
    > };
    >
    > +struct dev_pm_domain_link {
    > + struct generic_pm_domain *genpd;
    > + struct list_head node;
    > +};
    > +
    > /*
    > * The PM_EVENT_ messages are also used by drivers implementing the legacy
    > * suspend framework, based on the ->suspend() and ->resume() callbacks common
    > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
    > index e61b5cd79fe2..019593804670 100644
    > --- a/include/linux/pm_domain.h
    > +++ b/include/linux/pm_domain.h
    > @@ -51,7 +51,6 @@ struct dev_pm_opp;
    >
    > struct generic_pm_domain {
    > struct device dev;
    > - struct dev_pm_domain domain; /* PM domain operations */
    > struct list_head gpd_list_node; /* Node in the global PM domains list */
    > struct list_head master_links; /* Links with PM domain as a master */
    > struct list_head slave_links; /* Links with PM domain as a slave */
    > @@ -99,11 +98,6 @@ struct generic_pm_domain {
    >
    > };
    >
    > -static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
    > -{
    > - return container_of(pd, struct generic_pm_domain, domain);
    > -}
    > -
    >
    > Obviously the above will not compile but the intent would be to allocate a
    > dev_pm_domain_link struct per power-domain that the device needs and add
    > to the genpd_list for the device. It would be a much bigger change in
    > having to iterate through all the power-domains when turning devices on
    > and off, however, it would simplify the client driver.
    >
    > Cheers
    > Jon
    >
    > --
    > nvpublic

    Kind regards
    Uffe

    \
     
     \ /
      Last update: 2018-05-22 16:48    [W:2.953 / U:0.280 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site