All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Broadcom's PMB (Power Management Bus) support
@ 2020-12-11 21:59 Rafał Miłecki
  2020-12-11 21:59 ` [PATCH 1/2] dt-bindings: power: document Broadcom's PMB binding Rafał Miłecki
  2020-12-11 21:59 ` [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB Rafał Miłecki
  0 siblings, 2 replies; 11+ messages in thread
From: Rafał Miłecki @ 2020-12-11 21:59 UTC (permalink / raw)
  To: Florian Fainelli, Rob Herring
  Cc: Philipp Zabel, Rafael J . Wysocki, Kevin Hilman, Ulf Hansson,
	devicetree, bcm-kernel-feedback-list, linux-kernel,
	Rafał Miłecki

From: Rafał Miłecki <rafal@milecki.pl>

PMB is a hardware block used for powering SoC devices like PCIe, USB,
SATA. Initially I planned to treat it as a reset controller and Philipp
pointed out in review that PMB driver should use a power subsystem.

This is my refactored support.

***

Please note one difference when compared to the initial reset attempt.

As I store info about SoC devices in the driver now, I had to put
support for multiple buses there. That's required to avoid things like:

compatible = "brcm,bcm4908-pmb-no-1";
compatible = "brcm,bcm4908-pmb-no-2";

So now a single "reg" covers bigger buses (e.g. 0x40) in size, see:

reg = <0x802800e0 0x40>;

Other SoCs my use something like:

reg = <0x802800e0 0x20>;
reg = <0x802800e0 0x60>;

***

AFAIU this should go through Florian's tree. I based in on top of the
soc-arm64/next.

Rafał Miłecki (2):
  dt-bindings: power: document Broadcom's PMB binding
  soc: bcm: add PM driver for Broadcom's PMB

 .../bindings/power/brcm,bcm-pmb.yaml          |  50 +++
 drivers/soc/bcm/Kconfig                       |   8 +
 drivers/soc/bcm/Makefile                      |   1 +
 drivers/soc/bcm/bcm-pmb.c                     | 335 ++++++++++++++++++
 include/dt-bindings/soc/bcm-pmb.h             |  11 +
 5 files changed, 405 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/brcm,bcm-pmb.yaml
 create mode 100644 drivers/soc/bcm/bcm-pmb.c
 create mode 100644 include/dt-bindings/soc/bcm-pmb.h

-- 
2.26.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] dt-bindings: power: document Broadcom's PMB binding
  2020-12-11 21:59 [PATCH 0/2] Broadcom's PMB (Power Management Bus) support Rafał Miłecki
@ 2020-12-11 21:59 ` Rafał Miłecki
  2020-12-11 21:59 ` [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB Rafał Miłecki
  1 sibling, 0 replies; 11+ messages in thread
From: Rafał Miłecki @ 2020-12-11 21:59 UTC (permalink / raw)
  To: Florian Fainelli, Rob Herring
  Cc: Philipp Zabel, Rafael J . Wysocki, Kevin Hilman, Ulf Hansson,
	devicetree, bcm-kernel-feedback-list, linux-kernel,
	Rafał Miłecki

From: Rafał Miłecki <rafal@milecki.pl>

Broadcom's PMB is power controller used for disabling and enabling SoC
devices.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../bindings/power/brcm,bcm-pmb.yaml          | 50 +++++++++++++++++++
 include/dt-bindings/soc/bcm-pmb.h             | 11 ++++
 2 files changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/brcm,bcm-pmb.yaml
 create mode 100644 include/dt-bindings/soc/bcm-pmb.h

diff --git a/Documentation/devicetree/bindings/power/brcm,bcm-pmb.yaml b/Documentation/devicetree/bindings/power/brcm,bcm-pmb.yaml
new file mode 100644
index 000000000000..40b08d83c80b
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/brcm,bcm-pmb.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/brcm,bcm-pmb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom PMB (Power Management Bus) controller
+
+description: This document describes Broadcom's PMB controller. It supports
+  powering various types of connected devices (e.g. PCIe, USB, SATA).
+
+maintainers:
+  - Rafał Miłecki <rafal@milecki.pl>
+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm4908-pmb
+
+  reg:
+    description: register space of one or more buses
+    maxItems: 1
+
+  big-endian:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: Flag to use for block working in big endian mode.
+
+  "#power-domain-cells":
+    description: cell specifies device ID (see bcm-pmb.h)
+    const: 1
+
+required:
+  - reg
+  - "#power-domain-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/soc/bcm-pmb.h>
+
+    pmb: power-controller@802800e0 {
+        compatible = "brcm,bcm4908-pmb";
+        reg = <0x802800e0 0x40>;
+        #power-domain-cells = <1>;
+    };
+
+    foo {
+        power-domains = <&pmb BCM_PMB_PCIE0>;
+    };
diff --git a/include/dt-bindings/soc/bcm-pmb.h b/include/dt-bindings/soc/bcm-pmb.h
new file mode 100644
index 000000000000..744dc3af4d41
--- /dev/null
+++ b/include/dt-bindings/soc/bcm-pmb.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later OR MIT */
+
+#ifndef __DT_BINDINGS_SOC_BCM_PMB_H
+#define __DT_BINDINGS_SOC_BCM_PMB_H
+
+#define BCM_PMB_PCIE0				0x01
+#define BCM_PMB_PCIE1				0x02
+#define BCM_PMB_PCIE2				0x03
+#define BCM_PMB_HOST_USB			0x04
+
+#endif
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB
  2020-12-11 21:59 [PATCH 0/2] Broadcom's PMB (Power Management Bus) support Rafał Miłecki
  2020-12-11 21:59 ` [PATCH 1/2] dt-bindings: power: document Broadcom's PMB binding Rafał Miłecki
@ 2020-12-11 21:59 ` Rafał Miłecki
  2020-12-11 22:02   ` Rafał Miłecki
                     ` (2 more replies)
  1 sibling, 3 replies; 11+ messages in thread
From: Rafał Miłecki @ 2020-12-11 21:59 UTC (permalink / raw)
  To: Florian Fainelli, Rob Herring
  Cc: Philipp Zabel, Rafael J . Wysocki, Kevin Hilman, Ulf Hansson,
	devicetree, bcm-kernel-feedback-list, linux-kernel,
	Rafał Miłecki

From: Rafał Miłecki <rafal@milecki.pl>

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 <rafal@milecki.pl>
---
 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 <rafal@milecki.pl>
+ */
+
+#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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB
  2020-12-11 21:59 ` [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB Rafał Miłecki
@ 2020-12-11 22:02   ` Rafał Miłecki
  2020-12-11 22:08   ` Florian Fainelli
  2020-12-12  3:26   ` Florian Fainelli
  2 siblings, 0 replies; 11+ messages in thread
From: Rafał Miłecki @ 2020-12-11 22:02 UTC (permalink / raw)
  To: Rafał Miłecki, Florian Fainelli, Rob Herring
  Cc: Philipp Zabel, Rafael J . Wysocki, Kevin Hilman, Ulf Hansson,
	devicetree, bcm-kernel-feedback-list, linux-kernel

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 ;)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB
  2020-12-11 21:59 ` [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB Rafał Miłecki
  2020-12-11 22:02   ` Rafał Miłecki
@ 2020-12-11 22:08   ` Florian Fainelli
  2020-12-11 22:10     ` Rafał Miłecki
  2020-12-14 12:24     ` Rafał Miłecki
  2020-12-12  3:26   ` Florian Fainelli
  2 siblings, 2 replies; 11+ messages in thread
From: Florian Fainelli @ 2020-12-11 22:08 UTC (permalink / raw)
  To: Rafał Miłecki, Florian Fainelli, Rob Herring
  Cc: Philipp Zabel, Rafael J . Wysocki, Kevin Hilman, Ulf Hansson,
	devicetree, bcm-kernel-feedback-list, linux-kernel,
	Rafał Miłecki

On 12/11/20 1:59 PM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> 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 <rafal@milecki.pl>

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB
  2020-12-11 22:08   ` Florian Fainelli
@ 2020-12-11 22:10     ` Rafał Miłecki
  2020-12-14 12:24     ` Rafał Miłecki
  1 sibling, 0 replies; 11+ messages in thread
From: Rafał Miłecki @ 2020-12-11 22:10 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rob Herring, Philipp Zabel, Rafael J . Wysocki, Kevin Hilman,
	Ulf Hansson,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	bcm-kernel-feedback-list, Linux Kernel Mailing List,
	Rafał Miłecki

On Fri, 11 Dec 2020 at 23:08, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 12/11/20 1:59 PM, Rafał Miłecki wrote:
> > From: Rafał Miłecki <rafal@milecki.pl>
> >
> > 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 <rafal@milecki.pl>
>
> 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ł

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB
  2020-12-11 21:59 ` [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB Rafał Miłecki
  2020-12-11 22:02   ` Rafał Miłecki
  2020-12-11 22:08   ` Florian Fainelli
@ 2020-12-12  3:26   ` Florian Fainelli
  2020-12-14  8:00     ` Rafał Miłecki
  2 siblings, 1 reply; 11+ messages in thread
From: Florian Fainelli @ 2020-12-12  3:26 UTC (permalink / raw)
  To: Rafał Miłecki, Florian Fainelli, Rob Herring
  Cc: Philipp Zabel, Rafael J . Wysocki, Kevin Hilman, Ulf Hansson,
	devicetree, bcm-kernel-feedback-list, linux-kernel,
	Rafał Miłecki



On 12/11/2020 1:59 PM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> 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 <rafal@milecki.pl>

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB
  2020-12-12  3:26   ` Florian Fainelli
@ 2020-12-14  8:00     ` Rafał Miłecki
  0 siblings, 0 replies; 11+ messages in thread
From: Rafał Miłecki @ 2020-12-14  8:00 UTC (permalink / raw)
  To: Florian Fainelli, Rafał Miłecki, Rob Herring
  Cc: Philipp Zabel, Rafael J . Wysocki, Kevin Hilman, Ulf Hansson,
	devicetree, bcm-kernel-feedback-list, linux-kernel

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB
  2020-12-11 22:08   ` Florian Fainelli
  2020-12-11 22:10     ` Rafał Miłecki
@ 2020-12-14 12:24     ` Rafał Miłecki
  2020-12-14 17:32       ` Florian Fainelli
  1 sibling, 1 reply; 11+ messages in thread
From: Rafał Miłecki @ 2020-12-14 12:24 UTC (permalink / raw)
  To: Florian Fainelli, Rafał Miłecki, Rob Herring
  Cc: Philipp Zabel, Rafael J . Wysocki, Kevin Hilman, Ulf Hansson,
	devicetree, bcm-kernel-feedback-list, linux-kernel

On 11.12.2020 23:08, Florian Fainelli wrote:
> On 12/11/20 1:59 PM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> 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 <rafal@milecki.pl>
> 
> 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?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB
  2020-12-14 12:24     ` Rafał Miłecki
@ 2020-12-14 17:32       ` Florian Fainelli
  2020-12-14 17:34         ` Rafał Miłecki
  0 siblings, 1 reply; 11+ messages in thread
From: Florian Fainelli @ 2020-12-14 17:32 UTC (permalink / raw)
  To: Rafał Miłecki, Rafał Miłecki, Rob Herring
  Cc: Philipp Zabel, Rafael J . Wysocki, Kevin Hilman, Ulf Hansson,
	devicetree, bcm-kernel-feedback-list, linux-kernel

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 <rafal@milecki.pl>
>>>
>>> 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 <rafal@milecki.pl>
>>
>> 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



-- 
Florian

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB
  2020-12-14 17:32       ` Florian Fainelli
@ 2020-12-14 17:34         ` Rafał Miłecki
  0 siblings, 0 replies; 11+ messages in thread
From: Rafał Miłecki @ 2020-12-14 17:34 UTC (permalink / raw)
  To: Florian Fainelli, Rafał Miłecki, Rob Herring
  Cc: Philipp Zabel, Rafael J . Wysocki, Kevin Hilman, Ulf Hansson,
	devicetree, bcm-kernel-feedback-list, linux-kernel

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 <rafal@milecki.pl>
>>>>
>>>> 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 <rafal@milecki.pl>
>>>
>>> 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!

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-12-14 21:51 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-11 21:59 [PATCH 0/2] Broadcom's PMB (Power Management Bus) support Rafał Miłecki
2020-12-11 21:59 ` [PATCH 1/2] dt-bindings: power: document Broadcom's PMB binding Rafał Miłecki
2020-12-11 21:59 ` [PATCH 2/2] soc: bcm: add PM driver for Broadcom's PMB Rafał Miłecki
2020-12-11 22:02   ` Rafał Miłecki
2020-12-11 22:08   ` Florian Fainelli
2020-12-11 22:10     ` Rafał Miłecki
2020-12-14 12:24     ` Rafał Miłecki
2020-12-14 17:32       ` Florian Fainelli
2020-12-14 17:34         ` Rafał Miłecki
2020-12-12  3:26   ` Florian Fainelli
2020-12-14  8:00     ` Rafał Miłecki

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.