linux-kernel - Re: [PATCH v7 5/7] PCI: brcmstb: Add control of subdevice voltage regulators

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAL_Jsq+7vER_VkHJZt3vzz6qaqGvF3ts0NQQ4KCR4fi95+ZVZg@mail.gmail.com>
Date:   Thu, 4 Nov 2021 10:13:56 -0500
From:   Rob Herring <robh@...nel.org>
To:     Jim Quinlan <jim2101024@...il.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>
Cc:     PCI <linux-pci@...r.kernel.org>,
        Nicolas Saenz Julienne <nsaenz@...nel.org>,
        Mark Brown <broonie@...nel.org>,
        "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" 
        <bcm-kernel-feedback-list@...adcom.com>,
        Jim Quinlan <james.quinlan@...adcom.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Krzysztof Wilczyński <kw@...ux.com>,
        "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
        <linux-rpi-kernel@...ts.infradead.org>,
        "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v7 5/7] PCI: brcmstb: Add control of subdevice voltage regulators

On Wed, Nov 3, 2021 at 1:49 PM Jim Quinlan <jim2101024@...il.com> wrote:
>
> This Broadcom STB PCIe RC driver has one port and connects directly to one
> device, be it a switch or an endpoint.  We want to be able to turn on/off
> any regulators for that device.  Control of regulators is needed because of
> the chicken-and-egg situation: although the regulator is "owned" by the
> device and would be best handled by its driver, the device cannot be
> discovered and probed unless its regulator is already turned on.
>
> Signed-off-by: Jim Quinlan <jim2101024@...il.com>
> ---
>  drivers/pci/controller/pcie-brcmstb.c | 156 +++++++++++++++++++++++++-
>  1 file changed, 154 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index ba4d6daf312c..aaf6a4cbeb78 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -24,6 +24,7 @@
>  #include <linux/pci.h>
>  #include <linux/pci-ecam.h>
>  #include <linux/printk.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
> @@ -191,6 +192,15 @@ static inline void brcm_pcie_bridge_sw_init_set_generic(struct brcm_pcie *pcie,
>  static inline void brcm_pcie_perst_set_4908(struct brcm_pcie *pcie, u32 val);
>  static inline void brcm_pcie_perst_set_7278(struct brcm_pcie *pcie, u32 val);
>  static inline void brcm_pcie_perst_set_generic(struct brcm_pcie *pcie, u32 val);
> +static int brcm_pcie_add_bus(struct pci_bus *bus);
> +static void brcm_pcie_remove_bus(struct pci_bus *bus);
> +static bool brcm_pcie_link_up(struct brcm_pcie *pcie);
> +
> +static const char * const supplies[] = {
> +       "vpcie3v3",
> +       "vpcie3v3aux",
> +       "vpcie12v",

Common DT properties, so they should be in common DT code.

> +};
>
>  enum {
>         RGR1_SW_INIT_1,
> @@ -295,8 +305,38 @@ 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(supplies)];
> +       unsigned int            num_supplies;

Humm, this will need to be stored somewhere, but the host bridge is
not the right place. That doesn't scale to more than 1 bridge/bus. I'm
not exactly sure where though. pci_bus.self->sys_data,
pci_bus.self->dev.driver_data, pci_bus->sysdata,
pci_bus->dev.driver_data are possible options. Bjorn?

However, given suspend/resume hooks are also needed, maybe
portdrv_pci.c driver is the better spot for all this? The host bridge
wouldn't be in control of opting in, but presence of a DT node ptr for
the port device may be sufficient.

Rob