On Mon 16 Aug 17:54 PDT 2021, Dmitry Baryshkov wrote: > Basing on MMC's pwrseq support code, add separate power sequencer > subsystem. It will be used by other drivers to handle device power up > requirements. > Some more background to why we need a pwrseq framework wouldn't hurt. [..] > diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c > new file mode 100644 > index 000000000000..20485cae29aa > --- /dev/null > +++ b/drivers/power/pwrseq/core.c > @@ -0,0 +1,411 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// Copyright 2021 (c) Linaro Ltd. > +// Author: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > +// > +// Based on phy-core.c: > +// Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com The typical format is: // SPDX using C++ style comment /* * Copyright stuff using C style comment */ > + > +#include <linux/device.h> > +#include <linux/idr.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/pm_runtime.h> > +#include <linux/pwrseq/consumer.h> > +#include <linux/pwrseq/driver.h> > +#include <linux/slab.h> > + > +#define to_pwrseq(a) (container_of((a), struct pwrseq, dev)) No need for the extra parenthesis around container_of() > + > +static DEFINE_IDA(pwrseq_ida); > +static DEFINE_MUTEX(pwrseq_provider_mutex); > +static LIST_HEAD(pwrseq_provider_list); > + > +struct pwrseq_provider { > + struct device *dev; > + struct module *owner; > + struct list_head list; > + void *data; > + struct pwrseq * (*of_xlate)(void *data, struct of_phandle_args *args); > +}; > + > +void pwrseq_put(struct device *dev, struct pwrseq *pwrseq) > +{ > + device_link_remove(dev, &pwrseq->dev); > + > + module_put(pwrseq->owner); > + put_device(&pwrseq->dev); > +} > +EXPORT_SYMBOL_GPL(pwrseq_put); > + > +static struct pwrseq_provider *of_pwrseq_provider_lookup(struct device_node *node) > +{ > + struct pwrseq_provider *pwrseq_provider; > + > + list_for_each_entry(pwrseq_provider, &pwrseq_provider_list, list) { > + if (pwrseq_provider->dev->of_node == node) > + return pwrseq_provider; > + } > + > + return ERR_PTR(-EPROBE_DEFER); > +} > + > +static struct pwrseq *_of_pwrseq_get(struct device *dev, const char *id) > +{ > + struct pwrseq_provider *pwrseq_provider; > + struct pwrseq *pwrseq; > + struct of_phandle_args args; > + char prop_name[64]; /* 64 is max size of property name */ > + int ret; > + > + snprintf(prop_name, 64, "%s-pwrseq", id); sizeof(prop_name), to avoid giving others a chance to "fix" it later? > + ret = of_parse_phandle_with_args(dev->of_node, prop_name, "#pwrseq-cells", 0, &args); > + if (ret) { > + struct device_node *dn; > + > + /* > + * Parsing failed. Try locating old bindings for mmc-pwrseq, > + * which did not use #pwrseq-cells. > + */ > + if (strcmp(id, "mmc")) > + return ERR_PTR(-ENODEV); > + > + dn = of_parse_phandle(dev->of_node, prop_name, 0); > + if (!dn) > + return ERR_PTR(-ENODEV); > + > + args.np = dn; > + args.args_count = 0; > + } > + > + mutex_lock(&pwrseq_provider_mutex); > + pwrseq_provider = of_pwrseq_provider_lookup(args.np); > + if (IS_ERR(pwrseq_provider) || !try_module_get(pwrseq_provider->owner)) { > + pwrseq = ERR_PTR(-EPROBE_DEFER); > + goto out_unlock; > + } > + > + if (!of_device_is_available(args.np)) { > + dev_warn(pwrseq_provider->dev, "Requested pwrseq is disabled\n"); > + pwrseq = ERR_PTR(-ENODEV); > + goto out_put_module; > + } > + > + pwrseq = pwrseq_provider->of_xlate(pwrseq_provider->data, &args); > + > +out_put_module: > + module_put(pwrseq_provider->owner); > + > +out_unlock: > + mutex_unlock(&pwrseq_provider_mutex); > + of_node_put(args.np); > + > + return pwrseq; > +} > + [..] > +int pwrseq_pre_power_on(struct pwrseq *pwrseq) > +{ > + if (pwrseq && pwrseq->ops->pre_power_on) > + return pwrseq->ops->pre_power_on(pwrseq); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(pwrseq_pre_power_on); > + > +int pwrseq_power_on(struct pwrseq *pwrseq) Wouldn't it make sense to refcount the power on/off operations and at least warn about unbalanced disables? My concern is related to the qca-driver's reliance on the regulator framework to refcount the on/off of the shared resources and additional power_off from either the WiFi or BT client would result in the other client getting its power disabled unexpectedly - which might be annoying to debug. > +{ > + if (pwrseq && pwrseq->ops->power_on) > + return pwrseq->ops->power_on(pwrseq); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(pwrseq_power_on); > + > +void pwrseq_power_off(struct pwrseq *pwrseq) > +{ > + if (pwrseq && pwrseq->ops->power_off) > + pwrseq->ops->power_off(pwrseq); > +} > +EXPORT_SYMBOL_GPL(pwrseq_power_off); > + > +void pwrseq_reset(struct pwrseq *pwrseq) > +{ > + if (pwrseq && pwrseq->ops->reset) > + pwrseq->ops->reset(pwrseq); > +} > +EXPORT_SYMBOL_GPL(pwrseq_reset); > + Regards, Bjorn
