From: Florian Fainelli <f.fainelli@gmail.com> To: Jim Quinlan <james.quinlan@broadcom.com>, linux-pci@vger.kernel.org, Nicolas Saenz Julienne <nsaenzjulienne@suse.de>, broonie@kernel.org, bcm-kernel-feedback-list@broadcom.com Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Rob Herring <robh@kernel.org>, Bjorn Helgaas <bhelgaas@google.com>, Florian Fainelli <f.fainelli@gmail.com>, "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-rpi-kernel@lists.infradead.org>, "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org>, open list <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v2 2/6] PCI: brcmstb: Add control of EP voltage regulator(s) Date: Mon, 30 Nov 2020 13:32:26 -0800 [thread overview] Message-ID: <154a809d-9320-f0a1-45ad-78af0303a9ac@gmail.com> (raw) In-Reply-To: <20201130211145.3012-3-james.quinlan@broadcom.com> On 11/30/2020 1:11 PM, Jim Quinlan wrote: > Control of EP regulators by the RC is needed because of the chicken-and-egg > situation: although the regulator is "owned" by the EP and would be best > handled on its driver, the EP cannot be discovered and probed unless its > regulator is already turned on. > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> > --- > drivers/pci/controller/pcie-brcmstb.c | 38 ++++++++++++++++++++++++++- > 1 file changed, 37 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > index bea86899bd5d..9d4ac42b3bee 100644 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -23,6 +23,7 @@ > #include <linux/of_platform.h> > #include <linux/pci.h> > #include <linux/printk.h> > +#include <linux/regulator/consumer.h> > #include <linux/reset.h> > #include <linux/sizes.h> > #include <linux/slab.h> > @@ -210,6 +211,10 @@ enum pcie_type { > BCM2711, > }; > > +static const char * const ep_regulator_names[] = { > + "vpcie12v", "vpcie3v3", "vpcie1v8", "vpcie0v9", Only if you need to re-spin this patch series, I would be keen on putting each string on its own line, that way when adding a subsequent regulator name, it is just a matter of an one line d > +}; > + > struct pcie_cfg_data { > const int *offsets; > const enum pcie_type type; > @@ -287,8 +292,25 @@ struct brcm_pcie { > u32 hw_rev; > void (*perst_set)(struct brcm_pcie *pcie, u32 val); > void (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val); > + struct regulator_bulk_data supplies[ARRAY_SIZE(ep_regulator_names)]; > }; > > +static void brcm_set_regulators(struct brcm_pcie *pcie, bool on) > +{ > + struct device *dev = pcie->dev; > + int ret; > + > + if (on) > + ret = regulator_bulk_enable(ARRAY_SIZE(ep_regulator_names), > + pcie->supplies); > + else > + ret = regulator_bulk_disable(ARRAY_SIZE(ep_regulator_names), > + pcie->supplies); > + if (ret) > + dev_err(dev, "failed to %s EP regulators\n", > + on ? "enable" : "disable"); Should not you propagate the return value to the caller here? -- Florian
WARNING: multiple messages have this Message-ID (diff)
From: Florian Fainelli <f.fainelli@gmail.com> To: Jim Quinlan <james.quinlan@broadcom.com>, linux-pci@vger.kernel.org, Nicolas Saenz Julienne <nsaenzjulienne@suse.de>, broonie@kernel.org, bcm-kernel-feedback-list@broadcom.com Cc: Rob Herring <robh@kernel.org>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, open list <linux-kernel@vger.kernel.org>, Florian Fainelli <f.fainelli@gmail.com>, "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-rpi-kernel@lists.infradead.org>, Bjorn Helgaas <bhelgaas@google.com>, "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v2 2/6] PCI: brcmstb: Add control of EP voltage regulator(s) Date: Mon, 30 Nov 2020 13:32:26 -0800 [thread overview] Message-ID: <154a809d-9320-f0a1-45ad-78af0303a9ac@gmail.com> (raw) In-Reply-To: <20201130211145.3012-3-james.quinlan@broadcom.com> On 11/30/2020 1:11 PM, Jim Quinlan wrote: > Control of EP regulators by the RC is needed because of the chicken-and-egg > situation: although the regulator is "owned" by the EP and would be best > handled on its driver, the EP cannot be discovered and probed unless its > regulator is already turned on. > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> > --- > drivers/pci/controller/pcie-brcmstb.c | 38 ++++++++++++++++++++++++++- > 1 file changed, 37 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > index bea86899bd5d..9d4ac42b3bee 100644 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -23,6 +23,7 @@ > #include <linux/of_platform.h> > #include <linux/pci.h> > #include <linux/printk.h> > +#include <linux/regulator/consumer.h> > #include <linux/reset.h> > #include <linux/sizes.h> > #include <linux/slab.h> > @@ -210,6 +211,10 @@ enum pcie_type { > BCM2711, > }; > > +static const char * const ep_regulator_names[] = { > + "vpcie12v", "vpcie3v3", "vpcie1v8", "vpcie0v9", Only if you need to re-spin this patch series, I would be keen on putting each string on its own line, that way when adding a subsequent regulator name, it is just a matter of an one line d > +}; > + > struct pcie_cfg_data { > const int *offsets; > const enum pcie_type type; > @@ -287,8 +292,25 @@ struct brcm_pcie { > u32 hw_rev; > void (*perst_set)(struct brcm_pcie *pcie, u32 val); > void (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val); > + struct regulator_bulk_data supplies[ARRAY_SIZE(ep_regulator_names)]; > }; > > +static void brcm_set_regulators(struct brcm_pcie *pcie, bool on) > +{ > + struct device *dev = pcie->dev; > + int ret; > + > + if (on) > + ret = regulator_bulk_enable(ARRAY_SIZE(ep_regulator_names), > + pcie->supplies); > + else > + ret = regulator_bulk_disable(ARRAY_SIZE(ep_regulator_names), > + pcie->supplies); > + if (ret) > + dev_err(dev, "failed to %s EP regulators\n", > + on ? "enable" : "disable"); Should not you propagate the return value to the caller here? -- Florian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-11-30 21:33 UTC|newest] Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-30 21:11 [PATCH v2 0/6] brcmstb: add EP regulators and panic handler Jim Quinlan 2020-11-30 21:11 ` Jim Quinlan 2020-11-30 21:11 ` [PATCH v2 1/6] dt-bindings: PCI: Add bindings for Brcmstb EP voltage regulators Jim Quinlan 2020-11-30 21:11 ` Jim Quinlan 2020-12-09 14:01 ` Rob Herring 2020-12-09 14:01 ` Rob Herring 2021-01-04 22:12 ` Jim Quinlan 2021-01-04 22:12 ` Jim Quinlan 2021-01-05 14:01 ` Mark Brown 2021-01-05 14:01 ` Mark Brown 2021-01-05 15:09 ` Jim Quinlan 2021-01-05 15:09 ` Jim Quinlan 2021-01-05 15:33 ` Mark Brown 2021-01-05 15:33 ` Mark Brown 2021-01-07 22:31 ` Rob Herring 2021-01-07 22:31 ` Rob Herring 2020-11-30 21:11 ` [PATCH v2 2/6] PCI: brcmstb: Add control of EP voltage regulator(s) Jim Quinlan 2020-11-30 21:11 ` Jim Quinlan 2020-11-30 21:32 ` Florian Fainelli [this message] 2020-11-30 21:32 ` Florian Fainelli 2020-11-30 21:11 ` [PATCH v2 3/6] PCI: brcmstb: Do not turn off regulators if EP can wake up Jim Quinlan 2020-11-30 21:11 ` Jim Quinlan 2020-11-30 21:29 ` Florian Fainelli 2020-11-30 21:29 ` Florian Fainelli 2020-11-30 21:11 ` [PATCH v2 4/6] PCI: brcmstb: Give 7216 SOCs their own config type Jim Quinlan 2020-11-30 21:11 ` Jim Quinlan 2020-11-30 21:24 ` Florian Fainelli 2020-11-30 21:24 ` Florian Fainelli 2020-11-30 21:11 ` [PATCH v2 5/6] PCI: brcmstb: Add panic/die handler to RC driver Jim Quinlan 2020-11-30 21:11 ` Jim Quinlan 2020-11-30 21:11 ` Jim Quinlan 2020-11-30 21:28 ` Florian Fainelli 2020-11-30 21:28 ` Florian Fainelli 2020-12-01 18:05 ` Bjorn Helgaas 2020-12-01 18:05 ` Bjorn Helgaas 2020-12-01 20:12 ` Jim Quinlan 2020-12-01 20:12 ` Jim Quinlan 2021-01-06 19:19 ` Bjorn Helgaas 2021-01-06 19:19 ` Bjorn Helgaas [not found] ` <CA+-6iNzARUT63Mv7qFzk_g5wep4v6aPuN8f8yjQcgozVcKhVTw@mail.gmail.com> 2021-01-06 19:57 ` Jim Quinlan 2021-01-06 19:57 ` Jim Quinlan 2021-01-06 23:11 ` Bjorn Helgaas 2021-01-06 23:11 ` Bjorn Helgaas 2020-11-30 21:11 ` [PATCH v2 6/6] PCI: brcmstb: check return value of clk_prepare_enable() Jim Quinlan 2020-11-30 21:11 ` Jim Quinlan 2020-11-30 21:24 ` Florian Fainelli 2020-11-30 21:24 ` Florian Fainelli 2021-01-06 19:19 ` [PATCH v2 0/6] brcmstb: add EP regulators and panic handler Bjorn Helgaas 2021-01-06 19:19 ` Bjorn Helgaas
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=154a809d-9320-f0a1-45ad-78af0303a9ac@gmail.com \ --to=f.fainelli@gmail.com \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=bhelgaas@google.com \ --cc=broonie@kernel.org \ --cc=james.quinlan@broadcom.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=linux-rpi-kernel@lists.infradead.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=nsaenzjulienne@suse.de \ --cc=robh@kernel.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.