LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Andy Gross <agross@kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Marcel Holtmann <marcel@holtmann.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
	Kalle Valo <kvalo@codeaurora.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Stanimir Varbanov <svarbanov@mm-sol.com>,
	"open list:DRM DRIVER FOR MSM ADRENO GPU" 
	<linux-arm-msm@vger.kernel.org>,
	linux-mmc@vger.kernel.org,
	open list <linux-kernel@vger.kernel.org>,
	linux-bluetooth@vger.kernel.org, ath10k@lists.infradead.org,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [RFC PATCH 10/15] pwrseq: add support for QCA BT+WiFi power sequencer
Date: Fri, 20 Aug 2021 11:10:29 +0300	[thread overview]
Message-ID: <CAA8EJpr+=Yg2B_DzQWntW0GgvBfaSpAu0K+UD3NowdkusiYxrQ@mail.gmail.com> (raw)
In-Reply-To: <YR7m43mURVJ8YufC@ripper>

Hi,

On Fri, 20 Aug 2021 at 02:17, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Mon 16 Aug 17:55 PDT 2021, Dmitry Baryshkov wrote:
> [..]
> > diff --git a/drivers/power/pwrseq/pwrseq_qca.c b/drivers/power/pwrseq/pwrseq_qca.c
> > new file mode 100644
> > index 000000000000..3421a4821126
> > --- /dev/null
> > +++ b/drivers/power/pwrseq/pwrseq_qca.c
> > @@ -0,0 +1,290 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2021, Linaro Ltd.
> > + *
> > + * Author: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > + *
> > + * Power Sequencer for Qualcomm WiFi + BT SoCs
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pwrseq/driver.h>
> > +#include <linux/regulator/consumer.h>
> > +
> > +/*
> > + * Voltage regulator information required for configuring the
> > + * QCA WiFi+Bluetooth chipset
> > + */
> > +struct qca_vreg {
> > +     const char *name;
> > +     unsigned int load_uA;
> > +};
> > +
> > +struct qca_device_data {
> > +     struct qca_vreg vddio;
>
> Any particular reason why this isn't just the first entry in vregs and
> operated as part of the bulk API?

Because VDDIO should be up before bringing the rest of the power
sources (at least for wcn39xx). This is usually the case since VDDIO
is S4A, but I'd still prefer to express this in the code.
And register_bulk_enable powers up all the supplies asynchronously,
thus it can not guarantee that the first entry would be powered up
first.

>
> > +     struct qca_vreg *vregs;
> > +     size_t num_vregs;
> > +     bool has_bt_en;
> > +     bool has_wifi_en;
> > +};
> > +
> > +struct pwrseq_qca;
> > +struct pwrseq_qca_one {
> > +     struct pwrseq_qca *common;
> > +     struct gpio_desc *enable;
> > +};
> > +
> > +#define PWRSEQ_QCA_WIFI 0
> > +#define PWRSEQ_QCA_BT 1
> > +
> > +#define PWRSEQ_QCA_MAX 2
> > +
> > +struct pwrseq_qca {
> > +     struct regulator *vddio;
> > +     struct gpio_desc *sw_ctrl;
> > +     struct pwrseq_qca_one pwrseq_qcas[PWRSEQ_QCA_MAX];
> > +     int num_vregs;
> > +     struct regulator_bulk_data vregs[];
> > +};
> > +
> > +static int pwrseq_qca_power_on(struct pwrseq *pwrseq)
> > +{
> > +     struct pwrseq_qca_one *qca_one = pwrseq_get_data(pwrseq);
> > +     int ret;
> > +
> > +     if (qca_one->common->vddio) {
>
> devm_regulator_get() doesn't return NULL, so this is always true.

This is more of the safety guard for the cases when the qca doesn't
have the special vddio supply.

>
> > +             ret = regulator_enable(qca_one->common->vddio);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     ret = regulator_bulk_enable(qca_one->common->num_vregs, qca_one->common->vregs);
> > +     if (ret)
> > +             goto vddio_off;
> > +
> > +     if (qca_one->enable) {
> > +             gpiod_set_value_cansleep(qca_one->enable, 0);
> > +             msleep(50);
> > +             gpiod_set_value_cansleep(qca_one->enable, 1);
> > +             msleep(150);
> > +     }
> > +
> > +     if (qca_one->common->sw_ctrl) {
> > +             bool sw_ctrl_state = gpiod_get_value_cansleep(qca_one->common->sw_ctrl);
> > +             dev_dbg(&pwrseq->dev, "SW_CTRL is %d", sw_ctrl_state);
> > +     }
> > +
> > +     return 0;
> > +
> > +vddio_off:
> > +     regulator_disable(qca_one->common->vddio);
> > +
> > +     return ret;
> > +}
> [..]
> > +static int pwrseq_qca_probe(struct platform_device *pdev)
> > +{
> > +     struct pwrseq_qca *pwrseq_qca;
> > +     struct pwrseq *pwrseq;
> > +     struct pwrseq_provider *provider;
> > +     struct device *dev = &pdev->dev;
> > +     struct pwrseq_onecell_data *onecell;
> > +     const struct qca_device_data *data;
> > +     int ret, i;
> > +
> > +     data = device_get_match_data(dev);
> > +     if (!data)
> > +             return -EINVAL;
> > +
> > +     pwrseq_qca = devm_kzalloc(dev, struct_size(pwrseq_qca, vregs, data->num_vregs), GFP_KERNEL);
> > +     if (!pwrseq_qca)
> > +             return -ENOMEM;
> > +
> > +     onecell = devm_kzalloc(dev, struct_size(onecell, pwrseqs, PWRSEQ_QCA_MAX), GFP_KERNEL);
> > +     if (!onecell)
> > +             return -ENOMEM;
> > +
> > +     ret = pwrseq_qca_regulators_init(dev, pwrseq_qca, data);
> > +     if (ret)
> > +             return ret;
> > +
> > +     if (data->has_wifi_en) {
> > +             pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_WIFI].enable = devm_gpiod_get(dev, "wifi-enable", GPIOD_OUT_LOW);
> > +             if (IS_ERR(pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_WIFI].enable)) {
> > +                     return dev_err_probe(dev, PTR_ERR(pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_WIFI].enable),
> > +                                     "failed to acquire WIFI enable GPIO\n");
> > +             }
> > +     }
> > +
> > +     if (data->has_bt_en) {
> > +             pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_BT].enable = devm_gpiod_get(dev, "bt-enable", GPIOD_OUT_LOW);
> > +             if (IS_ERR(pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_BT].enable)) {
> > +                     return dev_err_probe(dev, PTR_ERR(pwrseq_qca->pwrseq_qcas[PWRSEQ_QCA_BT].enable),
> > +                                     "failed to acquire BT enable GPIO\n");
> > +             }
> > +     }
> > +
> > +     pwrseq_qca->sw_ctrl = devm_gpiod_get_optional(dev, "swctrl", GPIOD_IN);
> > +     if (IS_ERR(pwrseq_qca->sw_ctrl)) {
> > +             return dev_err_probe(dev, PTR_ERR(pwrseq_qca->sw_ctrl),
> > +                             "failed to acquire SW_CTRL gpio\n");
> > +     } else if (!pwrseq_qca->sw_ctrl)
> > +             dev_info(dev, "No SW_CTRL gpio\n");
>
> Some {} around the else as well please.

ack

>
> Regards,
> Bjorn



-- 
With best wishes
Dmitry

  reply	other threads:[~2021-08-20  8:10 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-17  0:54 [RFC PATCH 00/15] create power sequencing subsystem Dmitry Baryshkov
2021-08-17  0:54 ` [RFC PATCH 01/15] power: add power sequencer subsystem Dmitry Baryshkov
2021-08-19 23:37   ` Bjorn Andersson
2021-08-17  0:54 ` [RFC PATCH 02/15] pwrseq: port MMC's pwrseq drivers to new pwrseq subsystem Dmitry Baryshkov
2021-08-17  0:54 ` [RFC PATCH 03/15] mmc: core: switch " Dmitry Baryshkov
2021-08-17  0:54 ` [RFC PATCH 04/15] ath10k: add support for pwrseq sequencing Dmitry Baryshkov
2021-08-17  0:54 ` [RFC PATCH 05/15] Bluetooth: hci_qca: merge qca_power into qca_serdev Dmitry Baryshkov
2021-08-17  0:54 ` [RFC PATCH 06/15] Bluetooth: hci_qca: merge init paths Dmitry Baryshkov
2021-08-17  0:54 ` [RFC PATCH 07/15] Bluetooth: hci_qca: merge qca_power_on with qca_regulators_init Dmitry Baryshkov
2021-08-17  0:55 ` [RFC PATCH 08/15] Bluetooth: hci_qca: futher rework of power on/off handling Dmitry Baryshkov
2021-08-17  0:55 ` [RFC PATCH 09/15] Bluetooth: hci_qca: add support for pwrseq Dmitry Baryshkov
2021-08-17  0:55 ` [RFC PATCH 10/15] pwrseq: add support for QCA BT+WiFi power sequencer Dmitry Baryshkov
2021-08-19 23:18   ` Bjorn Andersson
2021-08-20  8:10     ` Dmitry Baryshkov [this message]
2021-08-20 16:35       ` Bjorn Andersson
2021-08-17  0:55 ` [RFC PATCH 11/15] arm64: dts: qcom: sdm845-db845c: switch bt+wifi to qca " Dmitry Baryshkov
2021-08-19 23:40   ` Bjorn Andersson
2021-08-17  0:55 ` [RFC PATCH 12/15] arm64: dts: qcom: qrb5165-rb5: add bluetooth support Dmitry Baryshkov
2021-08-17  0:55 ` [RFC PATCH 13/15] arm64: dts: qcom: sdm845-db845c: add second channel support to qca power sequencer Dmitry Baryshkov
2021-08-17  0:55 ` [RFC PATCH 14/15] WIP: PCI: qcom: use pwrseq to power up bus devices Dmitry Baryshkov
2021-08-19 23:44   ` Bjorn Andersson
2021-08-20  8:50     ` Dmitry Baryshkov
2021-08-17  0:55 ` [RFC PATCH 15/15] WIP: arm64: dts: qcom: qrb5165-rb5: add bus-pwrseq property to pcie0 Dmitry Baryshkov
2021-08-19 15:23 ` [RFC PATCH 00/15] create power sequencing subsystem Marcel Holtmann
2021-08-20 13:08   ` Dmitry Baryshkov
2021-08-20 17:02     ` Bjorn Andersson
2021-08-20 18:06       ` Dmitry Baryshkov
2021-08-21  6:50     ` Marcel Holtmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAA8EJpr+=Yg2B_DzQWntW0GgvBfaSpAu0K+UD3NowdkusiYxrQ@mail.gmail.com' \
    --to=dmitry.baryshkov@linaro.org \
    --cc=agross@kernel.org \
    --cc=ath10k@lists.infradead.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=davem@davemloft.net \
    --cc=johan.hedberg@gmail.com \
    --cc=kuba@kernel.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.org \
    --cc=netdev@vger.kernel.org \
    --cc=svarbanov@mm-sol.com \
    --cc=ulf.hansson@linaro.org \
    --subject='Re: [RFC PATCH 10/15] pwrseq: add support for QCA BT+WiFi power sequencer' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).