2020-12-13 03:58:46

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB

From: Rafał Miłecki <[email protected]>

PMB can be found on BCM4908 and many other chipsets (e.g. BCM63138).
It's needed to power on and off SoC blocks like PCIe, SATA, USB.

Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/soc/bcm/Kconfig | 8 +
drivers/soc/bcm/Makefile | 1 +
drivers/soc/bcm/bcm-pmb.c | 335 ++++++++++++++++++++++++++++++++++++++
3 files changed, 344 insertions(+)
create mode 100644 drivers/soc/bcm/bcm-pmb.c

diff --git a/drivers/soc/bcm/Kconfig b/drivers/soc/bcm/Kconfig
index 24f92a6e882a..327cfbd88471 100644
--- a/drivers/soc/bcm/Kconfig
+++ b/drivers/soc/bcm/Kconfig
@@ -13,6 +13,14 @@ config BCM2835_POWER
firmware means that Linux usage of the same power domain
must be accessed using the RASPBERRYPI_POWER driver

+config BCM_PMB
+ bool "Broadcom PMB (Power Management Bus) driver"
+ depends on ARCH_BCM4908 || (COMPILE_TEST && OF)
+ default ARCH_BCM4908
+ select PM_GENERIC_DOMAINS if PM
+ help
+ Foo
+
config RASPBERRYPI_POWER
bool "Raspberry Pi power domain driver"
depends on ARCH_BCM2835 || (COMPILE_TEST && OF)
diff --git a/drivers/soc/bcm/Makefile b/drivers/soc/bcm/Makefile
index 7bc90e0bd773..c451ee76259f 100644
--- a/drivers/soc/bcm/Makefile
+++ b/drivers/soc/bcm/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_BCM2835_POWER) += bcm2835-power.o
+obj-$(CONFIG_BCM_PMB) += bcm-pmb.o
obj-$(CONFIG_RASPBERRYPI_POWER) += raspberrypi-power.o
obj-$(CONFIG_SOC_BCM63XX) += bcm63xx/
obj-$(CONFIG_SOC_BRCMSTB) += brcmstb/
diff --git a/drivers/soc/bcm/bcm-pmb.c b/drivers/soc/bcm/bcm-pmb.c
new file mode 100644
index 000000000000..31820e56418d
--- /dev/null
+++ b/drivers/soc/bcm/bcm-pmb.c
@@ -0,0 +1,335 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2013 Broadcom
+ * Copyright (C) 2020 Rafał Miłecki <[email protected]>
+ */
+
+#include <dt-bindings/soc/bcm-pmb.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/reset/bcm63xx_pmb.h>
+
+#define BPCM_ID_REG 0x00
+#define BPCM_CAPABILITIES 0x04
+#define BPCM_CAP_NUM_ZONES 0x000000ff
+#define BPCM_CAP_SR_REG_BITS 0x0000ff00
+#define BPCM_CAP_PLLTYPE 0x00030000
+#define BPCM_CAP_UBUS 0x00080000
+#define BPCM_CONTROL 0x08
+#define BPCM_STATUS 0x0c
+#define BPCM_ROSC_CONTROL 0x10
+#define BPCM_ROSC_THRESH_H 0x14
+#define BPCM_ROSC_THRESHOLD_BCM6838 0x14
+#define BPCM_ROSC_THRESH_S 0x18
+#define BPCM_ROSC_COUNT_BCM6838 0x18
+#define BPCM_ROSC_COUNT 0x1c
+#define BPCM_PWD_CONTROL_BCM6838 0x1c
+#define BPCM_PWD_CONTROL 0x20
+#define BPCM_SR_CONTROL_BCM6838 0x20
+#define BPCM_PWD_ACCUM_CONTROL 0x24
+#define BPCM_SR_CONTROL 0x28
+#define BPCM_GLOBAL_CONTROL 0x2c
+#define BPCM_MISC_CONTROL 0x30
+#define BPCM_MISC_CONTROL2 0x34
+#define BPCM_SGPHY_CNTL 0x38
+#define BPCM_SGPHY_STATUS 0x3c
+#define BPCM_ZONE0 0x40
+#define BPCM_ZONE_CONTROL 0x00
+#define BPCM_ZONE_CONTROL_MANUAL_CLK_EN 0x00000001
+#define BPCM_ZONE_CONTROL_MANUAL_RESET_CTL 0x00000002
+#define BPCM_ZONE_CONTROL_FREQ_SCALE_USED 0x00000004 /* R/O */
+#define BPCM_ZONE_CONTROL_DPG_CAPABLE 0x00000008 /* R/O */
+#define BPCM_ZONE_CONTROL_MANUAL_MEM_PWR 0x00000030
+#define BPCM_ZONE_CONTROL_MANUAL_ISO_CTL 0x00000040
+#define BPCM_ZONE_CONTROL_MANUAL_CTL 0x00000080
+#define BPCM_ZONE_CONTROL_DPG_CTL_EN 0x00000100
+#define BPCM_ZONE_CONTROL_PWR_DN_REQ 0x00000200
+#define BPCM_ZONE_CONTROL_PWR_UP_REQ 0x00000400
+#define BPCM_ZONE_CONTROL_MEM_PWR_CTL_EN 0x00000800
+#define BPCM_ZONE_CONTROL_BLK_RESET_ASSERT 0x00001000
+#define BPCM_ZONE_CONTROL_MEM_STBY 0x00002000
+#define BPCM_ZONE_CONTROL_RESERVED 0x0007c000
+#define BPCM_ZONE_CONTROL_PWR_CNTL_STATE 0x00f80000
+#define BPCM_ZONE_CONTROL_FREQ_SCALAR_DYN_SEL 0x01000000 /* R/O */
+#define BPCM_ZONE_CONTROL_PWR_OFF_STATE 0x02000000 /* R/O */
+#define BPCM_ZONE_CONTROL_PWR_ON_STATE 0x04000000 /* R/O */
+#define BPCM_ZONE_CONTROL_PWR_GOOD 0x08000000 /* R/O */
+#define BPCM_ZONE_CONTROL_DPG_PWR_STATE 0x10000000 /* R/O */
+#define BPCM_ZONE_CONTROL_MEM_PWR_STATE 0x20000000 /* R/O */
+#define BPCM_ZONE_CONTROL_ISO_STATE 0x40000000 /* R/O */
+#define BPCM_ZONE_CONTROL_RESET_STATE 0x80000000 /* R/O */
+#define BPCM_ZONE_CONFIG1 0x04
+#define BPCM_ZONE_CONFIG2 0x08
+#define BPCM_ZONE_FREQ_SCALAR_CONTROL 0x0c
+#define BPCM_ZONE_SIZE 0x10
+
+struct bcm_pmb {
+ struct device *dev;
+ void __iomem *base;
+ spinlock_t lock;
+ bool little_endian;
+ struct genpd_onecell_data genpd_onecell_data;
+};
+
+struct bcm_pmb_pd_data {
+ const char * const name;
+ int id;
+ u8 bus;
+ u8 device;
+};
+
+struct bcm_pmb_pm_domain {
+ struct bcm_pmb *pmb;
+ const struct bcm_pmb_pd_data *data;
+ struct generic_pm_domain genpd;
+};
+
+static int bcm_pmb_bpcm_read(struct bcm_pmb *pmb, int bus, u8 device,
+ int offset, u32 *val)
+{
+ void __iomem *base = pmb->base + bus * 0x20;
+ unsigned long flags;
+ int err;
+
+ spin_lock_irqsave(&pmb->lock, flags);
+ err = bpcm_rd(base, device, offset, val);
+ spin_unlock_irqrestore(&pmb->lock, flags);
+
+ if (!err)
+ *val = pmb->little_endian ? le32_to_cpu(*val) : be32_to_cpu(*val);
+
+ return err;
+}
+
+static int bcm_pmb_bpcm_write(struct bcm_pmb *pmb, int bus, u8 device,
+ int offset, u32 val)
+{
+ void __iomem *base = pmb->base + bus * 0x20;
+ unsigned long flags;
+ int err;
+
+ val = pmb->little_endian ? cpu_to_le32(val) : cpu_to_be32(val);
+
+ spin_lock_irqsave(&pmb->lock, flags);
+ err = bpcm_wr(base, device, offset, val);
+ spin_unlock_irqrestore(&pmb->lock, flags);
+
+ return err;
+}
+
+static int bcm_pmb_power_off_zone(struct bcm_pmb *pmb, int bus, u8 device,
+ int zone)
+{
+ int offset;
+ u32 val;
+ int err;
+
+ offset = BPCM_ZONE0 + zone * BPCM_ZONE_SIZE + BPCM_ZONE_CONTROL;
+
+ err = bcm_pmb_bpcm_read(pmb, bus, device, offset, &val);
+ if (err)
+ return err;
+
+ val |= BPCM_ZONE_CONTROL_PWR_DN_REQ;
+ val &= ~BPCM_ZONE_CONTROL_PWR_UP_REQ;
+
+ err = bcm_pmb_bpcm_write(pmb, bus, device, offset, val);
+
+ return err;
+}
+
+static int bcm_pmb_power_on_zone(struct bcm_pmb *pmb, int bus, u8 device,
+ int zone)
+{
+ int offset;
+ u32 val;
+ int err;
+
+ offset = BPCM_ZONE0 + zone * BPCM_ZONE_SIZE + BPCM_ZONE_CONTROL;
+
+ err = bcm_pmb_bpcm_read(pmb, bus, device, offset, &val);
+ if (err)
+ return err;
+
+ if (!(val & BPCM_ZONE_CONTROL_PWR_ON_STATE)) {
+ val &= ~BPCM_ZONE_CONTROL_PWR_DN_REQ;
+ val |= BPCM_ZONE_CONTROL_DPG_CTL_EN;
+ val |= BPCM_ZONE_CONTROL_PWR_UP_REQ;
+ val |= BPCM_ZONE_CONTROL_MEM_PWR_CTL_EN;
+ val |= BPCM_ZONE_CONTROL_BLK_RESET_ASSERT;
+
+ err = bcm_pmb_bpcm_write(pmb, bus, device, offset, val);
+ }
+
+ return err;
+}
+
+static int bcm_pmb_power_off_device(struct bcm_pmb *pmb, int bus, u8 device)
+{
+ int offset;
+ u32 val;
+ int err;
+
+ /* Entire device can be powered off by powering off the 0th zone */
+ offset = BPCM_ZONE0 + BPCM_ZONE_CONTROL;
+
+ err = bcm_pmb_bpcm_read(pmb, bus, device, offset, &val);
+ if (err)
+ return err;
+
+ if (!(val & BPCM_ZONE_CONTROL_PWR_OFF_STATE)) {
+ val = BPCM_ZONE_CONTROL_PWR_DN_REQ;
+
+ err = bcm_pmb_bpcm_write(pmb, bus, device, offset, val);
+ }
+
+ return err;
+}
+
+static int bcm_pmb_power_on_device(struct bcm_pmb *pmb, int bus, u8 device)
+{
+ u32 val;
+ int err;
+ int i;
+
+ err = bcm_pmb_bpcm_read(pmb, bus, device, BPCM_CAPABILITIES, &val);
+ if (err)
+ return err;
+
+ for (i = 0; i < (val & BPCM_CAP_NUM_ZONES); i++) {
+ err = bcm_pmb_power_on_zone(pmb, bus, device, i);
+ if (err)
+ return err;
+ }
+
+ return err;
+}
+
+static int bcm_pmb_power_on(struct generic_pm_domain *genpd)
+{
+ struct bcm_pmb_pm_domain *pd = container_of(genpd, struct bcm_pmb_pm_domain, genpd);
+ const struct bcm_pmb_pd_data *data = pd->data;
+ struct bcm_pmb *pmb = pd->pmb;
+
+ switch (data->id) {
+ case BCM_PMB_PCIE0:
+ case BCM_PMB_PCIE1:
+ case BCM_PMB_PCIE2:
+ return bcm_pmb_power_on_zone(pmb, data->bus, data->device, 0);
+ case BCM_PMB_HOST_USB:
+ return bcm_pmb_power_on_device(pmb, data->bus, data->device);
+ default:
+ dev_err(pmb->dev, "unsupported device id: %d\n", data->id);
+ return -EINVAL;
+ }
+}
+
+static int bcm_pmb_power_off(struct generic_pm_domain *genpd)
+{
+ struct bcm_pmb_pm_domain *pd = container_of(genpd, struct bcm_pmb_pm_domain, genpd);
+ const struct bcm_pmb_pd_data *data = pd->data;
+ struct bcm_pmb *pmb = pd->pmb;
+
+ switch (data->id) {
+ case BCM_PMB_PCIE0:
+ case BCM_PMB_PCIE1:
+ case BCM_PMB_PCIE2:
+ return bcm_pmb_power_off_zone(pmb, data->bus, data->device, 0);
+ case BCM_PMB_HOST_USB:
+ return bcm_pmb_power_off_device(pmb, data->bus, data->device);
+ default:
+ dev_err(pmb->dev, "unsupported device id: %d\n", data->id);
+ return -EINVAL;
+ }
+}
+
+static int bcm_pmb_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const struct bcm_pmb_pd_data *table;
+ const struct bcm_pmb_pd_data *e;
+ struct resource *res;
+ struct bcm_pmb *pmb;
+ int max_id;
+ int err;
+
+ dev_info(dev, "START\n");
+
+ pmb = devm_kzalloc(dev, sizeof(*pmb), GFP_KERNEL);
+ if (!pmb)
+ return -ENOMEM;
+
+ pmb->dev = dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ pmb->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(pmb->base))
+ return PTR_ERR(pmb->base);
+
+ spin_lock_init(&pmb->lock);
+
+ pmb->little_endian = !of_device_is_big_endian(dev->of_node);
+
+ table = of_device_get_match_data(dev);
+ if (!table)
+ return -EINVAL;
+
+ max_id = 0;
+ for (e = table; e->name; e++)
+ max_id = max(max_id, e->id);
+
+ pmb->genpd_onecell_data.num_domains = max_id + 1;
+ pmb->genpd_onecell_data.domains =
+ devm_kcalloc(dev, pmb->genpd_onecell_data.num_domains,
+ sizeof(struct generic_pm_domain *), GFP_KERNEL);
+ if (!pmb->genpd_onecell_data.domains)
+ return -ENOMEM;
+
+ for (e = table; e->name; e++) {
+ struct bcm_pmb_pm_domain *pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+
+ pd->pmb = pmb;
+ pd->data = e;
+ pd->genpd.name = e->name;
+ pd->genpd.power_on = bcm_pmb_power_on;
+ pd->genpd.power_off = bcm_pmb_power_off;
+
+ pm_genpd_init(&pd->genpd, NULL, true);
+ pmb->genpd_onecell_data.domains[e->id] = &pd->genpd;
+ }
+
+ err = of_genpd_add_provider_onecell(dev->of_node, &pmb->genpd_onecell_data);
+ if (err) {
+ dev_err(dev, "failed to add genpd provider: %d\n", err);
+ return err;
+ }
+
+ return 0;
+}
+
+static const struct bcm_pmb_pd_data bcm_pmb_bcm4908_data[] = {
+ { .name = "pcie2", .id = BCM_PMB_PCIE2, .bus = 0, .device = 2, },
+ { .name = "pcie0", .id = BCM_PMB_PCIE0, .bus = 1, .device = 14, },
+ { .name = "pcie1", .id = BCM_PMB_PCIE1, .bus = 1, .device = 15, },
+ { .name = "usb", .id = BCM_PMB_HOST_USB, .bus = 1, .device = 17, },
+ { },
+};
+
+static const struct of_device_id bcm_pmb_of_match[] = {
+ { .compatible = "brcm,bcm4908-pmb", .data = &bcm_pmb_bcm4908_data, },
+ { },
+};
+
+static struct platform_driver bcm_pmb_driver = {
+ .driver = {
+ .name = "bcm-pmb",
+ .of_match_table = bcm_pmb_of_match,
+ },
+ .probe = bcm_pmb_probe,
+};
+
+builtin_platform_driver(bcm_pmb_driver);
--
2.26.2


2020-12-13 04:06:28

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB

On 12/11/20 1:59 PM, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> PMB can be found on BCM4908 and many other chipsets (e.g. BCM63138).
> It's needed to power on and off SoC blocks like PCIe, SATA, USB.
>
> Signed-off-by: Rafał Miłecki <[email protected]>

I will do a more thorough review tonight, however do you mind moving the
driver under drives/soc/bcm/bcm63xx? The first SoC that had PMB was
63138 and that one is DSL.

And we would probably need a MAINTAINERS file update for this driver?

Thanks!
--
Florian

2020-12-13 04:21:29

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB

On Fri, 11 Dec 2020 at 23:08, Florian Fainelli <[email protected]> wrote:
> On 12/11/20 1:59 PM, Rafał Miłecki wrote:
> > From: Rafał Miłecki <[email protected]>
> >
> > PMB can be found on BCM4908 and many other chipsets (e.g. BCM63138).
> > It's needed to power on and off SoC blocks like PCIe, SATA, USB.
> >
> > Signed-off-by: Rafał Miłecki <[email protected]>
>
> I will do a more thorough review tonight, however do you mind moving the
> driver under drives/soc/bcm/bcm63xx? The first SoC that had PMB was
> 63138 and that one is DSL.
>
> And we would probably need a MAINTAINERS file update for this driver?

Sounds good!

--
Rafał

2020-12-13 04:28:32

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB

On 11.12.2020 22:59, Rafał Miłecki wrote:
> @@ -13,6 +13,14 @@ config BCM2835_POWER
> firmware means that Linux usage of the same power domain
> must be accessed using the RASPBERRYPI_POWER driver
>
> +config BCM_PMB
> + bool "Broadcom PMB (Power Management Bus) driver"
> + depends on ARCH_BCM4908 || (COMPILE_TEST && OF)
> + default ARCH_BCM4908
> + select PM_GENERIC_DOMAINS if PM
> + help
> + Foo

I'll improve description a bit in V2 ;)

2020-12-13 14:07:09

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB



On 12/11/2020 1:59 PM, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> PMB can be found on BCM4908 and many other chipsets (e.g. BCM63138).
> It's needed to power on and off SoC blocks like PCIe, SATA, USB.
>
> Signed-off-by: Rafał Miłecki <[email protected]>

This looks good to me, just a few nipicks below.

[snip]

> +static int bcm_pmb_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + const struct bcm_pmb_pd_data *table;
> + const struct bcm_pmb_pd_data *e;
> + struct resource *res;
> + struct bcm_pmb *pmb;
> + int max_id;
> + int err;
> +
> + dev_info(dev, "START\n");

Stray debugging.

[snip]

> +
> +static const struct bcm_pmb_pd_data bcm_pmb_bcm4908_data[] = {
> + { .name = "pcie2", .id = BCM_PMB_PCIE2, .bus = 0, .device = 2, },
> + { .name = "pcie0", .id = BCM_PMB_PCIE0, .bus = 1, .device = 14, },
> + { .name = "pcie1", .id = BCM_PMB_PCIE1, .bus = 1, .device = 15, },
> + { .name = "usb", .id = BCM_PMB_HOST_USB, .bus = 1, .device = 17, },

Do you have to be more specific and spell out whether this is the host
controller (xhci) or device (bdc)? If not, then this looks good to me.
--
Florian

2020-12-14 14:37:34

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB

On 12.12.2020 04:26, Florian Fainelli wrote:
>> +
>> +static const struct bcm_pmb_pd_data bcm_pmb_bcm4908_data[] = {
>> + { .name = "pcie2", .id = BCM_PMB_PCIE2, .bus = 0, .device = 2, },
>> + { .name = "pcie0", .id = BCM_PMB_PCIE0, .bus = 1, .device = 14, },
>> + { .name = "pcie1", .id = BCM_PMB_PCIE1, .bus = 1, .device = 15, },
>> + { .name = "usb", .id = BCM_PMB_HOST_USB, .bus = 1, .device = 17, },
>
> Do you have to be more specific and spell out whether this is the host
> controller (xhci) or device (bdc)? If not, then this looks good to me.

I believe I have to and I believe I already do. I used BCM_PMB_HOST_USB
which clearly (I believe) points to the HOST controller.

In 6838 part of pmc_drv.h from Broadcom's SDK I found:

enum {
USB30_2X_Zone_Common,
USB30_2X_Zone_USB_Host,
USB30_2X_Zone_USB_Device,
};

and that's what makes me believe we need to specify HOST explicitly.

2020-12-14 21:56:15

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB

On 14.12.2020 18:32, Florian Fainelli wrote:
> On 12/14/20 4:24 AM, Rafał Miłecki wrote:
>> On 11.12.2020 23:08, Florian Fainelli wrote:
>>> On 12/11/20 1:59 PM, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <[email protected]>
>>>>
>>>> PMB can be found on BCM4908 and many other chipsets (e.g. BCM63138).
>>>> It's needed to power on and off SoC blocks like PCIe, SATA, USB.
>>>>
>>>> Signed-off-by: Rafał Miłecki <[email protected]>
>>>
>>> I will do a more thorough review tonight, however do you mind moving the
>>> driver under drives/soc/bcm/bcm63xx? The first SoC that had PMB was
>>> 63138 and that one is DSL.
>>
>> I now realized that bcm63xx's:
>> * Kconfig is wrapper in: if SOC_BCM63XX
>> * Makefile is conditional: obj-$(CONFIG_SOC_BCM63XX)
>>
>> So it means I've to either:
>> 1. Refactor bcm63xx structure
>> 2. Make SOC_BCM63XX selectable on ARCH_BCM4908 and select it
>>
>> I'm not sure if any of above is a really good idea. Any further thought,
>> ideas?
>
> Well, I was thinking about thing along those lines below, it's really
> about putting drivers that belong to the same SoC family in the same
> basket for easier maintenance while submitting patches/pull requests
> from my side.
>
> diff --git a/drivers/soc/bcm/Makefile b/drivers/soc/bcm/Makefile
> index 7bc90e0bd773..0f0efa28d92b 100644
> --- a/drivers/soc/bcm/Makefile
> +++ b/drivers/soc/bcm/Makefile
> @@ -1,5 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0-only
> obj-$(CONFIG_BCM2835_POWER) += bcm2835-power.o
> obj-$(CONFIG_RASPBERRYPI_POWER) += raspberrypi-power.o
> -obj-$(CONFIG_SOC_BCM63XX) += bcm63xx/
> +obj-y += bcm63xx/
> obj-$(CONFIG_SOC_BRCMSTB) += brcmstb/
> diff --git a/drivers/soc/bcm/bcm63xx/Kconfig
> b/drivers/soc/bcm/bcm63xx/Kconfig
> index 16f648a6c70a..76fb61e7377e 100644
> --- a/drivers/soc/bcm/bcm63xx/Kconfig
> +++ b/drivers/soc/bcm/bcm63xx/Kconfig
> @@ -10,3 +10,8 @@ config BCM63XX_POWER
> BCM6318, BCM6328, BCM6362 and BCM63268 SoCs.
>
> endif # SOC_BCM63XX
> +
> +config BCM_PMB_POWER
> + tristate "Broadcom PMB bus power domain driver"
> + depends on BMIPS_GENERIC || ARCH_BCM4908 || COMPILE_TEST
> + default ARCH_BCM4908

Sounds OK to me, thanks for clarifying what approach you're OK with!

2020-12-14 23:48:10

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB

On 11.12.2020 23:08, Florian Fainelli wrote:
> On 12/11/20 1:59 PM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <[email protected]>
>>
>> PMB can be found on BCM4908 and many other chipsets (e.g. BCM63138).
>> It's needed to power on and off SoC blocks like PCIe, SATA, USB.
>>
>> Signed-off-by: Rafał Miłecki <[email protected]>
>
> I will do a more thorough review tonight, however do you mind moving the
> driver under drives/soc/bcm/bcm63xx? The first SoC that had PMB was
> 63138 and that one is DSL.

I now realized that bcm63xx's:
* Kconfig is wrapper in: if SOC_BCM63XX
* Makefile is conditional: obj-$(CONFIG_SOC_BCM63XX)

So it means I've to either:
1. Refactor bcm63xx structure
2. Make SOC_BCM63XX selectable on ARCH_BCM4908 and select it

I'm not sure if any of above is a really good idea. Any further thought, ideas?