| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMRc=Md=U_xEByDJSrYsTRVn=gtP-SR+-gjV-FynALWv64x5RA@mail.gmail.com>
Date: Fri, 2 Jan 2026 13:58:14 +0000
From: Bartosz Golaszewski <brgl@...nel.org>
To: manivannan.sadhasivam@....qualcomm.com
Cc: Manivannan Sadhasivam via B4 Relay <devnull+manivannan.sadhasivam.oss.qualcomm.com@...nel.org>,
linux-pci@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org, Chen-Yu Tsai <wens@...nel.org>,
Brian Norris <briannorris@...omium.org>,
Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>, Niklas Cassel <cassel@...nel.org>,
Alex Elder <elder@...cstar.com>, Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
Chen-Yu Tsai <wenst@...omium.org>, Manivannan Sadhasivam <mani@...nel.org>,
Lorenzo Pieralisi <lpieralisi@...nel.org>, Krzysztof Wilczyński <kwilczynski@...nel.org>,
Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
Bartosz Golaszewski <brgl@...ev.pl>
Subject: Re: [PATCH v3 2/7] PCI/pwrctrl: Add 'struct pci_pwrctrl::power_{on/off}'
callbacks
On Mon, 29 Dec 2025 18:26:53 +0100, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@...nel.org> said:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
>
> To allow the pwrctrl core to control the power on/off sequences of the
> pwrctrl drivers, add the 'struct pci_pwrctrl::power_{on/off}' callbacks and
> populate them in the respective pwrctrl drivers.
>
> The pwrctrl drivers still power on the resources on their own now. So there
> is no functional change.
>
> Co-developed-by: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
> Tested-by: Chen-Yu Tsai <wenst@...omium.org>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
> ---
> drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c | 27 +++++++++++++++---
> drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 20 +++++++++----
> drivers/pci/pwrctrl/slot.c | 48 ++++++++++++++++++++++----------
> include/linux/pci-pwrctrl.h | 4 +++
> 4 files changed, 75 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> index 4e664e7b8dd2..0fb9038a1d18 100644
> --- a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> @@ -52,11 +52,27 @@ static const struct pci_pwrctrl_pwrseq_pdata pci_pwrctrl_pwrseq_qcom_wcn_pdata =
> .validate_device = pci_pwrctrl_pwrseq_qcm_wcn_validate_device,
> };
>
> +static int pci_pwrctrl_pwrseq_power_on(struct pci_pwrctrl *ctx)
> +{
> + struct pci_pwrctrl_pwrseq_data *data = container_of(ctx, struct pci_pwrctrl_pwrseq_data,
> + ctx);
> +
> + return pwrseq_power_on(data->pwrseq);
> +}
> +
> +static void pci_pwrctrl_pwrseq_power_off(struct pci_pwrctrl *ctx)
> +{
> + struct pci_pwrctrl_pwrseq_data *data = container_of(ctx, struct pci_pwrctrl_pwrseq_data,
> + ctx);
> +
> + pwrseq_power_off(data->pwrseq);
> +}
> +
> static void devm_pci_pwrctrl_pwrseq_power_off(void *data)
> {
> - struct pwrseq_desc *pwrseq = data;
> + struct pci_pwrctrl_pwrseq_data *pwrseq_data = data;
>
> - pwrseq_power_off(pwrseq);
> + pci_pwrctrl_pwrseq_power_off(&pwrseq_data->ctx);
> }
>
> static int pci_pwrctrl_pwrseq_probe(struct platform_device *pdev)
> @@ -85,16 +101,19 @@ static int pci_pwrctrl_pwrseq_probe(struct platform_device *pdev)
> return dev_err_probe(dev, PTR_ERR(data->pwrseq),
> "Failed to get the power sequencer\n");
>
> - ret = pwrseq_power_on(data->pwrseq);
> + ret = pci_pwrctrl_pwrseq_power_on(&data->ctx);
> if (ret)
> return dev_err_probe(dev, ret,
> "Failed to power-on the device\n");
>
> ret = devm_add_action_or_reset(dev, devm_pci_pwrctrl_pwrseq_power_off,
> - data->pwrseq);
> + data);
> if (ret)
> return ret;
>
> + data->ctx.power_on = pci_pwrctrl_pwrseq_power_on;
> + data->ctx.power_off = pci_pwrctrl_pwrseq_power_off;
> +
> pci_pwrctrl_init(&data->ctx, dev);
>
> ret = devm_pci_pwrctrl_device_set_ready(dev, &data->ctx);
> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> index 0a63add84d09..0393af2a099c 100644
> --- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> @@ -434,15 +434,20 @@ static int tc9563_pwrctrl_parse_device_dt(struct tc9563_pwrctrl_ctx *ctx, struct
> return 0;
> }
>
> -static void tc9563_pwrctrl_power_off(struct tc9563_pwrctrl_ctx *ctx)
> +static void tc9563_pwrctrl_power_off(struct pci_pwrctrl *pwrctrl)
> {
> + struct tc9563_pwrctrl_ctx *ctx = container_of(pwrctrl,
> + struct tc9563_pwrctrl_ctx, pwrctrl);
> +
> gpiod_set_value(ctx->reset_gpio, 1);
>
> regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> }
>
> -static int tc9563_pwrctrl_bring_up(struct tc9563_pwrctrl_ctx *ctx)
> +static int tc9563_pwrctrl_power_on(struct pci_pwrctrl *pwrctrl)
> {
> + struct tc9563_pwrctrl_ctx *ctx = container_of(pwrctrl,
> + struct tc9563_pwrctrl_ctx, pwrctrl);
> struct tc9563_pwrctrl_cfg *cfg;
> int ret, i;
>
> @@ -502,7 +507,7 @@ static int tc9563_pwrctrl_bring_up(struct tc9563_pwrctrl_ctx *ctx)
> return 0;
>
> power_off:
> - tc9563_pwrctrl_power_off(ctx);
> + tc9563_pwrctrl_power_off(&ctx->pwrctrl);
> return ret;
> }
>
> @@ -591,7 +596,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
> goto remove_i2c;
> }
>
> - ret = tc9563_pwrctrl_bring_up(ctx);
> + ret = tc9563_pwrctrl_power_on(&ctx->pwrctrl);
> if (ret)
> goto remove_i2c;
>
> @@ -601,6 +606,9 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
> goto power_off;
> }
>
> + ctx->pwrctrl.power_on = tc9563_pwrctrl_power_on;
> + ctx->pwrctrl.power_off = tc9563_pwrctrl_power_off;
> +
> ret = devm_pci_pwrctrl_device_set_ready(dev, &ctx->pwrctrl);
> if (ret)
> goto power_off;
> @@ -610,7 +618,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
> return 0;
>
> power_off:
> - tc9563_pwrctrl_power_off(ctx);
> + tc9563_pwrctrl_power_off(&ctx->pwrctrl);
> remove_i2c:
> i2c_unregister_device(ctx->client);
> put_device(&ctx->adapter->dev);
> @@ -621,7 +629,7 @@ static void tc9563_pwrctrl_remove(struct platform_device *pdev)
> {
> struct tc9563_pwrctrl_ctx *ctx = platform_get_drvdata(pdev);
>
> - tc9563_pwrctrl_power_off(ctx);
> + tc9563_pwrctrl_power_off(&ctx->pwrctrl);
> i2c_unregister_device(ctx->client);
> put_device(&ctx->adapter->dev);
> }
> diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
> index 3320494b62d8..14701f65f1f2 100644
> --- a/drivers/pci/pwrctrl/slot.c
> +++ b/drivers/pci/pwrctrl/slot.c
> @@ -17,13 +17,36 @@ struct pci_pwrctrl_slot_data {
> struct pci_pwrctrl ctx;
> struct regulator_bulk_data *supplies;
> int num_supplies;
> + struct clk *clk;
> };
>
> -static void devm_pci_pwrctrl_slot_power_off(void *data)
> +static int pci_pwrctrl_slot_power_on(struct pci_pwrctrl *ctx)
> {
> - struct pci_pwrctrl_slot_data *slot = data;
> + struct pci_pwrctrl_slot_data *slot = container_of(ctx, struct pci_pwrctrl_slot_data, ctx);
> + int ret;
> +
> + ret = regulator_bulk_enable(slot->num_supplies, slot->supplies);
> + if (ret < 0) {
> + dev_err(slot->ctx.dev, "Failed to enable slot regulators\n");
> + return ret;
> + }
> +
> + return clk_prepare_enable(slot->clk);
> +}
> +
> +static void pci_pwrctrl_slot_power_off(struct pci_pwrctrl *ctx)
> +{
> + struct pci_pwrctrl_slot_data *slot = container_of(ctx, struct pci_pwrctrl_slot_data, ctx);
>
> regulator_bulk_disable(slot->num_supplies, slot->supplies);
> + clk_disable_unprepare(slot->clk);
> +}
> +
> +static void devm_pci_pwrctrl_slot_release(void *data)
> +{
> + struct pci_pwrctrl_slot_data *slot = data;
> +
> + pci_pwrctrl_slot_power_off(&slot->ctx);
> regulator_bulk_free(slot->num_supplies, slot->supplies);
> }
>
> @@ -31,7 +54,6 @@ static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
> {
> struct pci_pwrctrl_slot_data *slot;
> struct device *dev = &pdev->dev;
> - struct clk *clk;
> int ret;
>
> slot = devm_kzalloc(dev, sizeof(*slot), GFP_KERNEL);
> @@ -46,23 +68,21 @@ static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
> }
>
> slot->num_supplies = ret;
> - ret = regulator_bulk_enable(slot->num_supplies, slot->supplies);
> - if (ret < 0) {
> - dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
> - regulator_bulk_free(slot->num_supplies, slot->supplies);
> - return ret;
> - }
>
> - ret = devm_add_action_or_reset(dev, devm_pci_pwrctrl_slot_power_off,
> + ret = devm_add_action_or_reset(dev, devm_pci_pwrctrl_slot_release,
> slot);
> if (ret)
> return ret;
>
> - clk = devm_clk_get_optional_enabled(dev, NULL);
> - if (IS_ERR(clk)) {
> - return dev_err_probe(dev, PTR_ERR(clk),
> + slot->clk = devm_clk_get_optional(dev, NULL);
> + if (IS_ERR(slot->clk))
> + return dev_err_probe(dev, PTR_ERR(slot->clk),
> "Failed to enable slot clock\n");
> - }
> +
> + pci_pwrctrl_slot_power_on(&slot->ctx);
> +
> + slot->ctx.power_on = pci_pwrctrl_slot_power_on;
> + slot->ctx.power_off = pci_pwrctrl_slot_power_off;
>
> pci_pwrctrl_init(&slot->ctx, dev);
>
> diff --git a/include/linux/pci-pwrctrl.h b/include/linux/pci-pwrctrl.h
> index 4aefc7901cd1..bd0ee9998125 100644
> --- a/include/linux/pci-pwrctrl.h
> +++ b/include/linux/pci-pwrctrl.h
> @@ -31,6 +31,8 @@ struct device_link;
> /**
> * struct pci_pwrctrl - PCI device power control context.
> * @dev: Address of the power controlling device.
> + * @power_on: Callback to power on the power controlling device.
> + * @power_off: Callback to power off the power controlling device.
> *
> * An object of this type must be allocated by the PCI power control device and
> * passed to the pwrctrl subsystem to trigger a bus rescan and setup a device
> @@ -38,6 +40,8 @@ struct device_link;
> */
> struct pci_pwrctrl {
> struct device *dev;
> + int (*power_on)(struct pci_pwrctrl *pwrctrl);
> + void (*power_off)(struct pci_pwrctrl *pwrctrl);
I'd say that returning int here would not hurt and could spare you changing
the code in the future if anyone requires error reporting.
Bart
>
> /* private: internal use only */
> struct notifier_block nb;
>
> --
> 2.48.1
>
>
>
Powered by blists - more mailing lists