[PATCH V3 1/2] dt-bindings: pinctrl: document Broadcom Northstar pin mux controller

[PATCH V3 1/2] dt-bindings: pinctrl: document Broadcom Northstar pin mux controller

[Rafał Miłecki]

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

Northstar has mux controller just like Northstar Plus and Northstar2.
It's a bit different though (different registers & pins) so it requires
its own binding.

It's needed to allow other block bindings specify required mux setup.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
V2: Use "cru_gpio_control"
    Add more functions & groups
    Include Florian's Reviewed-by
V3: Use 3 different bindings as available pins depend on the chipset.
    Strings match Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.txt
---
 .../bindings/pinctrl/brcm,bcm4708-pinmux.txt       | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt

diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt
new file mode 100644
index 000000000000..af906f196e8c
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt
@@ -0,0 +1,42 @@
+Broadcom Northstar pins mux controller
+
+Some of Northstar SoCs's pins can be used for various purposes thanks to the mux
+controller. This binding allows describing mux controller and listing available
+functions. They can be referenced later by other bindings to let system
+configure controller correctly.
+
+A list of pins varies across chipsets so few bindings are available.
+
+Required properties:
+- compatible: must be one of:
+	"brcm,bcm4708-pinmux"
+	"brcm,bcm4709-pinmux"
+	"brcm,bcm53012-pinmux"
+- reg: iomem address range of CRU (Central Resource Unit) pin registers
+- reg-names: "cru_gpio_control" - the only needed & supported reg right now
+
+Functions and their groups available for all chipsets:
+- "spi": "spi_grp"
+- "i2c": "i2c_grp"
+- "pwm": "pwm0_grp", "pwm1_grp", "pwm2_grp", "pwm3_grp"
+- "uart1": "uart1_grp"
+
+Additionally available on BCM4709 and BCM53012:
+- "mdio": "mdio_grp"
+- "uart2": "uart2_grp"
+- "sdio": "sdio_pwr_grp", "sdio_1p8v_grp"
+
+For documentation of subnodes see:
+Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+
+Example:
+	pinctrl@1800c1c0 {
+		compatible = "brcm,bcm4708-pinmux";
+		reg = <0x1800c1c0 0x24>;
+		reg-names = "cru_gpio_control";
+
+		spi {
+			function = "spi";
+			groups = "spi_grp";
+		};
+	};
-- 
2.13.7


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V3 2/2] pinctrl: bcm: add Northstar driver

[Rafał Miłecki]

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

This driver provides support for Northstar mux controller. It differs
from Northstar Plus one so a new binding and driver were needed.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Define more pins, groups, functions
    Simplify ns_pinctrl_set_mux() as Northstar uses 1:1 mapping
V3: Support 3 different bindings and different sets of pins, groups
    and functions. This requires building arrays dynamically on init.
---
 drivers/pinctrl/bcm/Kconfig      |  13 ++
 drivers/pinctrl/bcm/Makefile     |   1 +
 drivers/pinctrl/bcm/pinctrl-ns.c | 372 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 386 insertions(+)
 create mode 100644 drivers/pinctrl/bcm/pinctrl-ns.c

diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig
index 0f38d51f47c6..c8575399d6f7 100644
--- a/drivers/pinctrl/bcm/Kconfig
+++ b/drivers/pinctrl/bcm/Kconfig
@@ -73,6 +73,19 @@ config PINCTRL_CYGNUS_MUX
 	  configuration, with the exception that certain individual pins
 	  can be overridden to GPIO function
 
+config PINCTRL_NS
+	bool "Broadcom Northstar pins driver"
+	depends on OF && (ARCH_BCM_5301X || COMPILE_TEST)
+	select PINMUX
+	select GENERIC_PINCONF
+	default ARCH_BCM_5301X
+	help
+	  Say yes here to enable the Broadcom NS SoC pins driver.
+
+	  The Broadcom Northstar pins driver supports muxing multi-purpose pins
+	  that can be used for various functions (e.g. SPI, I2C, UART) as well
+	  as GPIOs.
+
 config PINCTRL_NSP_GPIO
 	bool "Broadcom NSP GPIO (with PINCONF) driver"
 	depends on OF_GPIO && (ARCH_BCM_NSP || COMPILE_TEST)
diff --git a/drivers/pinctrl/bcm/Makefile b/drivers/pinctrl/bcm/Makefile
index 80ceb9dae944..79d5e49fdd9a 100644
--- a/drivers/pinctrl/bcm/Makefile
+++ b/drivers/pinctrl/bcm/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_PINCTRL_BCM281XX)		+= pinctrl-bcm281xx.o
 obj-$(CONFIG_PINCTRL_BCM2835)		+= pinctrl-bcm2835.o
 obj-$(CONFIG_PINCTRL_IPROC_GPIO)	+= pinctrl-iproc-gpio.o
 obj-$(CONFIG_PINCTRL_CYGNUS_MUX)	+= pinctrl-cygnus-mux.o
+obj-$(CONFIG_PINCTRL_NS)		+= pinctrl-ns.o
 obj-$(CONFIG_PINCTRL_NSP_GPIO)		+= pinctrl-nsp-gpio.o
 obj-$(CONFIG_PINCTRL_NS2_MUX)		+= pinctrl-ns2-mux.o
 obj-$(CONFIG_PINCTRL_NSP_MUX)		+= pinctrl-nsp-mux.o
diff --git a/drivers/pinctrl/bcm/pinctrl-ns.c b/drivers/pinctrl/bcm/pinctrl-ns.c
new file mode 100644
index 000000000000..aedbb2813c50
--- /dev/null
+++ b/drivers/pinctrl/bcm/pinctrl-ns.c
@@ -0,0 +1,372 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Rafał Miłecki <rafal@milecki.pl>
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define FLAG_BCM4708		BIT(1)
+#define FLAG_BCM4709		BIT(2)
+#define FLAG_BCM53012		BIT(3)
+
+struct ns_pinctrl {
+	struct device *dev;
+	unsigned int chipset_flag;
+	struct pinctrl_dev *pctldev;
+	void __iomem *base;
+
+	struct pinctrl_desc pctldesc;
+	struct ns_pinctrl_group *groups;
+	unsigned int num_groups;
+	struct ns_pinctrl_function *functions;
+	unsigned int num_functions;
+};
+
+/*
+ * Pins
+ */
+
+static const struct pinctrl_pin_desc ns_pinctrl_pins[] = {
+	{ 0, "spi_clk", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 1, "spi_ss", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 2, "spi_mosi", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 3, "spi_miso", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 4, "i2c_scl", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 5, "i2c_sda", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 6, "mdc", (void *)(FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 7, "mdio", (void *)(FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 8, "pwm0", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 9, "pwm1", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 10, "pwm2", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 11, "pwm3", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 12, "uart1_rx", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 13, "uart1_tx", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 14, "uart1_cts", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 15, "uart1_rts", (void *)(FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 16, "uart2_rx", (void *)(FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 17, "uart2_tx", (void *)(FLAG_BCM4709 | FLAG_BCM53012) },
+/* TODO { ??, "xtal_out", (void *)(FLAG_BCM4709) }, */
+	{ 22, "sdio_pwr", (void *)(FLAG_BCM4709 | FLAG_BCM53012) },
+	{ 23, "sdio_en_1p8v", (void *)(FLAG_BCM4709 | FLAG_BCM53012) },
+};
+
+/*
+ * Groups
+ */
+
+struct ns_pinctrl_group {
+	const char *name;
+	const unsigned int *pins;
+	const unsigned int num_pins;
+	unsigned int chipsets;
+};
+
+static const unsigned int spi_pins[] = { 0, 1, 2, 3 };
+static const unsigned int i2c_pins[] = { 4, 5 };
+static const unsigned int mdio_pins[] = { 6, 7 };
+static const unsigned int pwm0_pins[] = { 8 };
+static const unsigned int pwm1_pins[] = { 9 };
+static const unsigned int pwm2_pins[] = { 10 };
+static const unsigned int pwm3_pins[] = { 11 };
+static const unsigned int uart1_pins[] = { 12, 13, 14, 15 };
+static const unsigned int uart2_pins[] = { 16, 17 };
+static const unsigned int sdio_pwr_pins[] = { 22 };
+static const unsigned int sdio_1p8v_pins[] = { 23 };
+
+#define NS_GROUP(_name, _pins, _chipsets)		\
+{							\
+	.name = _name,					\
+	.pins = _pins,					\
+	.num_pins = ARRAY_SIZE(_pins),			\
+	.chipsets = _chipsets,				\
+}
+
+static const struct ns_pinctrl_group ns_pinctrl_groups[] = {
+	NS_GROUP("spi_grp", spi_pins, FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012),
+	NS_GROUP("i2c_grp", i2c_pins, FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012),
+	NS_GROUP("mdio_grp", mdio_pins, FLAG_BCM4709 | FLAG_BCM53012),
+	NS_GROUP("pwm0_grp", pwm0_pins, FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012),
+	NS_GROUP("pwm1_grp", pwm1_pins, FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012),
+	NS_GROUP("pwm2_grp", pwm2_pins, FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012),
+	NS_GROUP("pwm3_grp", pwm3_pins, FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012),
+	NS_GROUP("uart1_grp", uart1_pins, FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012),
+	NS_GROUP("uart2_grp", uart2_pins, FLAG_BCM4709 | FLAG_BCM53012),
+	NS_GROUP("sdio_pwr_grp", sdio_pwr_pins, FLAG_BCM4709 | FLAG_BCM53012),
+	NS_GROUP("sdio_1p8v_grp", sdio_1p8v_pins, FLAG_BCM4709 | FLAG_BCM53012),
+};
+
+/*
+ * Functions
+ */
+
+struct ns_pinctrl_function {
+	const char *name;
+	const char * const *groups;
+	const unsigned int num_groups;
+	unsigned int chipsets;
+};
+
+static const char * const spi_groups[] = { "spi_grp" };
+static const char * const i2c_groups[] = { "i2c_grp" };
+static const char * const mdio_groups[] = { "mdio_grp" };
+static const char * const pwm_groups[] = { "pwm0_grp", "pwm1_grp", "pwm2_grp",
+					   "pwm3_grp" };
+static const char * const uart1_groups[] = { "uart1_grp" };
+static const char * const uart2_groups[] = { "uart2_grp" };
+static const char * const sdio_groups[] = { "sdio_pwr_grp", "sdio_1p8v_grp" };
+
+#define NS_FUNCTION(_name, _groups, _chipsets)		\
+{							\
+	.name = _name,					\
+	.groups = _groups,				\
+	.num_groups = ARRAY_SIZE(_groups),		\
+	.chipsets = _chipsets,				\
+}
+
+static const struct ns_pinctrl_function ns_pinctrl_functions[] = {
+	NS_FUNCTION("spi", spi_groups, FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012),
+	NS_FUNCTION("i2c", i2c_groups, FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012),
+	NS_FUNCTION("mdio", mdio_groups, FLAG_BCM4709 | FLAG_BCM53012),
+	NS_FUNCTION("pwm", pwm_groups, FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012),
+	NS_FUNCTION("uart1", uart1_groups, FLAG_BCM4708 | FLAG_BCM4709 | FLAG_BCM53012),
+	NS_FUNCTION("uart2", uart2_groups, FLAG_BCM4709 | FLAG_BCM53012),
+	NS_FUNCTION("sdio", sdio_groups, FLAG_BCM4709 | FLAG_BCM53012),
+};
+
+/*
+ * Groups code
+ */
+
+static int ns_pinctrl_get_groups_count(struct pinctrl_dev *pctrl_dev)
+{
+	struct ns_pinctrl *ns_pinctrl = pinctrl_dev_get_drvdata(pctrl_dev);
+
+	return ns_pinctrl->num_groups;
+}
+
+static const char *ns_pinctrl_get_group_name(struct pinctrl_dev *pctrl_dev,
+					     unsigned int selector)
+{
+	struct ns_pinctrl *ns_pinctrl = pinctrl_dev_get_drvdata(pctrl_dev);
+
+	return ns_pinctrl->groups[selector].name;
+}
+
+static int ns_pinctrl_get_group_pins(struct pinctrl_dev *pctrl_dev,
+				     unsigned int selector,
+				     const unsigned int **pins,
+				     unsigned int *num_pins)
+{
+	struct ns_pinctrl *ns_pinctrl = pinctrl_dev_get_drvdata(pctrl_dev);
+
+	*pins = ns_pinctrl->groups[selector].pins;
+	*num_pins = ns_pinctrl->groups[selector].num_pins;
+
+	return 0;
+}
+
+static const struct pinctrl_ops ns_pinctrl_ops = {
+	.get_groups_count = ns_pinctrl_get_groups_count,
+	.get_group_name = ns_pinctrl_get_group_name,
+	.get_group_pins = ns_pinctrl_get_group_pins,
+	.dt_node_to_map = pinconf_generic_dt_node_to_map_group,
+	.dt_free_map = pinconf_generic_dt_free_map,
+};
+
+/*
+ * Functions code
+ */
+
+static int ns_pinctrl_get_functions_count(struct pinctrl_dev *pctrl_dev)
+{
+	struct ns_pinctrl *ns_pinctrl = pinctrl_dev_get_drvdata(pctrl_dev);
+
+	return ns_pinctrl->num_functions;
+}
+
+static const char *ns_pinctrl_get_function_name(struct pinctrl_dev *pctrl_dev,
+						unsigned int selector)
+{
+	struct ns_pinctrl *ns_pinctrl = pinctrl_dev_get_drvdata(pctrl_dev);
+
+	return ns_pinctrl->functions[selector].name;
+}
+
+static int ns_pinctrl_get_function_groups(struct pinctrl_dev *pctrl_dev,
+					  unsigned int selector,
+					  const char * const **groups,
+					  unsigned * const num_groups)
+{
+	struct ns_pinctrl *ns_pinctrl = pinctrl_dev_get_drvdata(pctrl_dev);
+
+	*groups = ns_pinctrl->functions[selector].groups;
+	*num_groups = ns_pinctrl->functions[selector].num_groups;
+
+	return 0;
+}
+
+static int ns_pinctrl_set_mux(struct pinctrl_dev *pctrl_dev,
+			      unsigned int func_select,
+			      unsigned int grp_select)
+{
+	struct ns_pinctrl *ns_pinctrl = pinctrl_dev_get_drvdata(pctrl_dev);
+	u32 unset = 0;
+	u32 tmp;
+	int i;
+
+	for (i = 0; i < ns_pinctrl->groups[grp_select].num_pins; i++) {
+		int pin_number = ns_pinctrl->groups[grp_select].pins[i];
+
+		unset |= BIT(pin_number);
+	}
+
+	tmp = readl(ns_pinctrl->base);
+	tmp &= ~unset;
+	writel(tmp, ns_pinctrl->base);
+
+	return 0;
+}
+
+static const struct pinmux_ops ns_pinctrl_pmxops = {
+	.get_functions_count = ns_pinctrl_get_functions_count,
+	.get_function_name = ns_pinctrl_get_function_name,
+	.get_function_groups = ns_pinctrl_get_function_groups,
+	.set_mux = ns_pinctrl_set_mux,
+};
+
+/*
+ * Controller code
+ */
+
+static struct pinctrl_desc ns_pinctrl_desc = {
+	.name = "pinctrl-ns",
+	.pctlops = &ns_pinctrl_ops,
+	.pmxops = &ns_pinctrl_pmxops,
+};
+
+static const struct of_device_id ns_pinctrl_of_match_table[] = {
+	{ .compatible = "brcm,bcm4708-pinmux", .data = (void *)FLAG_BCM4708, },
+	{ .compatible = "brcm,bcm4709-pinmux", .data = (void *)FLAG_BCM4709, },
+	{ .compatible = "brcm,bcm53012-pinmux", .data = (void *)FLAG_BCM53012, },
+	{ }
+};
+
+static int ns_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *of_id;
+	struct ns_pinctrl *ns_pinctrl;
+	struct pinctrl_desc *pctldesc;
+	struct pinctrl_pin_desc *pin;
+	struct ns_pinctrl_group *group;
+	struct ns_pinctrl_function *function;
+	struct resource *res;
+	int i;
+
+	ns_pinctrl = devm_kzalloc(dev, sizeof(*ns_pinctrl), GFP_KERNEL);
+	if (!ns_pinctrl)
+		return -ENOMEM;
+	pctldesc = &ns_pinctrl->pctldesc;
+	platform_set_drvdata(pdev, ns_pinctrl);
+
+	/* Set basic properties */
+
+	ns_pinctrl->dev = dev;
+
+	of_id = of_match_device(ns_pinctrl_of_match_table, dev);
+	if (!of_id)
+		return -EINVAL;
+	ns_pinctrl->chipset_flag = (unsigned int)of_id->data;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+					   "cru_gpio_control");
+	ns_pinctrl->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(ns_pinctrl->base)) {
+		dev_err(dev, "Failed to map pinctrl regs\n");
+		return PTR_ERR(ns_pinctrl->base);
+	}
+
+	memcpy(pctldesc, &ns_pinctrl_desc, sizeof(*pctldesc));
+
+	/* Set pinctrl properties */
+
+	pctldesc->pins = devm_kcalloc(dev, ARRAY_SIZE(ns_pinctrl_pins),
+				      sizeof(struct pinctrl_pin_desc),
+				      GFP_KERNEL);
+	if (!pctldesc->pins)
+		return -ENOMEM;
+	for (i = 0, pin = (struct pinctrl_pin_desc *)&pctldesc->pins[0];
+	     i < ARRAY_SIZE(ns_pinctrl_pins); i++) {
+		const struct pinctrl_pin_desc *src = &ns_pinctrl_pins[i];
+		unsigned int chipsets = (unsigned int)src->drv_data;
+
+		if (chipsets & ns_pinctrl->chipset_flag) {
+			memcpy(pin++, src, sizeof(*src));
+			pctldesc->npins++;
+		}
+	}
+
+	ns_pinctrl->groups = devm_kcalloc(dev, ARRAY_SIZE(ns_pinctrl_groups),
+					  sizeof(struct ns_pinctrl_group),
+					  GFP_KERNEL);
+	if (!ns_pinctrl->groups)
+		return -ENOMEM;
+	for (i = 0, group = &ns_pinctrl->groups[0];
+	     i < ARRAY_SIZE(ns_pinctrl_groups); i++) {
+		const struct ns_pinctrl_group *src = &ns_pinctrl_groups[i];
+
+		if (src->chipsets & ns_pinctrl->chipset_flag) {
+			memcpy(group++, src, sizeof(*src));
+			ns_pinctrl->num_groups++;
+		}
+	}
+
+	ns_pinctrl->functions = devm_kcalloc(dev,
+					     ARRAY_SIZE(ns_pinctrl_functions),
+					     sizeof(struct ns_pinctrl_function),
+					     GFP_KERNEL);
+	if (!ns_pinctrl->functions)
+		return -ENOMEM;
+	for (i = 0, function = &ns_pinctrl->functions[0];
+	     i < ARRAY_SIZE(ns_pinctrl_functions); i++) {
+		const struct ns_pinctrl_function *src = &ns_pinctrl_functions[i];
+
+		if (src->chipsets & ns_pinctrl->chipset_flag) {
+			memcpy(function++, src, sizeof(*src));
+			ns_pinctrl->num_functions++;
+		}
+	}
+
+	/* Register */
+
+	ns_pinctrl->pctldev = devm_pinctrl_register(dev, pctldesc, ns_pinctrl);
+	if (IS_ERR(ns_pinctrl->pctldev)) {
+		dev_err(dev, "Failed to register pinctrl\n");
+		return PTR_ERR(ns_pinctrl->pctldev);
+	}
+
+	return 0;
+}
+
+static struct platform_driver ns_pinctrl_driver = {
+	.probe = ns_pinctrl_probe,
+	.driver = {
+		.name = "ns-pinmux",
+		.of_match_table = ns_pinctrl_of_match_table,
+	},
+};
+
+module_platform_driver(ns_pinctrl_driver);
+
+MODULE_AUTHOR("Rafał Miłecki");
+MODULE_LICENSE("GPL v2");
+MODULE_DEVICE_TABLE(of, ns_pinctrl_of_match_table);
-- 
2.13.7


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 2/2] pinctrl: bcm: add Northstar driver

[Linus Walleij]

On Wed, Sep 26, 2018 at 9:31 PM Rafał Miłecki <zajec5@gmail.com> wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
>
> This driver provides support for Northstar mux controller. It differs
> from Northstar Plus one so a new binding and driver were needed.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Define more pins, groups, functions
>     Simplify ns_pinctrl_set_mux() as Northstar uses 1:1 mapping
> V3: Support 3 different bindings and different sets of pins, groups
>     and functions. This requires building arrays dynamically on init.

Patch applied.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 1/2] dt-bindings: pinctrl: document Broadcom Northstar pin mux controller

[Linus Walleij]

On Wed, Sep 26, 2018 at 9:31 PM Rafał Miłecki <zajec5@gmail.com> wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
>
> Northstar has mux controller just like Northstar Plus and Northstar2.
> It's a bit different though (different registers & pins) so it requires
> its own binding.
>
> It's needed to allow other block bindings specify required mux setup.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> V2: Use "cru_gpio_control"
>     Add more functions & groups
>     Include Florian's Reviewed-by
> V3: Use 3 different bindings as available pins depend on the chipset.
>     Strings match Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.txt

Patch applied.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 1/2] dt-bindings: pinctrl: document Broadcom Northstar pin mux controller

[Rob Herring]

On Wed, Sep 26, 2018 at 09:31:02PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Northstar has mux controller just like Northstar Plus and Northstar2.
> It's a bit different though (different registers & pins) so it requires
> its own binding.
> 
> It's needed to allow other block bindings specify required mux setup.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> V2: Use "cru_gpio_control"
>     Add more functions & groups
>     Include Florian's Reviewed-by
> V3: Use 3 different bindings as available pins depend on the chipset.
>     Strings match Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.txt
> ---
>  .../bindings/pinctrl/brcm,bcm4708-pinmux.txt       | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt
> new file mode 100644
> index 000000000000..af906f196e8c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt
> @@ -0,0 +1,42 @@
> +Broadcom Northstar pins mux controller
> +
> +Some of Northstar SoCs's pins can be used for various purposes thanks to the mux
> +controller. This binding allows describing mux controller and listing available
> +functions. They can be referenced later by other bindings to let system
> +configure controller correctly.
> +
> +A list of pins varies across chipsets so few bindings are available.
> +
> +Required properties:
> +- compatible: must be one of:
> +	"brcm,bcm4708-pinmux"
> +	"brcm,bcm4709-pinmux"
> +	"brcm,bcm53012-pinmux"
> +- reg: iomem address range of CRU (Central Resource Unit) pin registers

Perhaps 'cru' in the compatible if that's what the h/w is called?

Also, if this is a sub-block, then it should be a child of the block 
which should be defined here.

> +- reg-names: "cru_gpio_control" - the only needed & supported reg right now
> +
> +Functions and their groups available for all chipsets:
> +- "spi": "spi_grp"
> +- "i2c": "i2c_grp"
> +- "pwm": "pwm0_grp", "pwm1_grp", "pwm2_grp", "pwm3_grp"
> +- "uart1": "uart1_grp"
> +
> +Additionally available on BCM4709 and BCM53012:
> +- "mdio": "mdio_grp"
> +- "uart2": "uart2_grp"
> +- "sdio": "sdio_pwr_grp", "sdio_1p8v_grp"
> +
> +For documentation of subnodes see:
> +Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> +
> +Example:
> +	pinctrl@1800c1c0 {
> +		compatible = "brcm,bcm4708-pinmux";
> +		reg = <0x1800c1c0 0x24>;
> +		reg-names = "cru_gpio_control";
> +
> +		spi {

You'll find this now causes dtc warnings. 'spi' is reserved for SPI 
controller nodes. So 'spi-pins' perhaps.

> +			function = "spi";
> +			groups = "spi_grp";
> +		};
> +	};
> -- 
> 2.13.7
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 1/2] dt-bindings: pinctrl: document Broadcom Northstar pin mux controller

[Linus Walleij]

Rafał, could you patch the bindings with Rob comments so I don't have
to pull the patch out of my tree or revert it?

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 1/2] dt-bindings: pinctrl: document Broadcom Northstar pin mux controller

[Rafał Miłecki]

On 2018-10-12 00:22, Rob Herring wrote:
> On Wed, Sep 26, 2018 at 09:31:02PM +0200, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>> 
>> Northstar has mux controller just like Northstar Plus and Northstar2.
>> It's a bit different though (different registers & pins) so it 
>> requires
>> its own binding.
>> 
>> It's needed to allow other block bindings specify required mux setup.
>> 
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> V2: Use "cru_gpio_control"
>>     Add more functions & groups
>>     Include Florian's Reviewed-by
>> V3: Use 3 different bindings as available pins depend on the chipset.
>>     Strings match 
>> Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.txt
>> ---
>>  .../bindings/pinctrl/brcm,bcm4708-pinmux.txt       | 42 
>> ++++++++++++++++++++++
>>  1 file changed, 42 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt 
>> b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt
>> new file mode 100644
>> index 000000000000..af906f196e8c
>> --- /dev/null
>> +++ 
>> b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt
>> @@ -0,0 +1,42 @@
>> +Broadcom Northstar pins mux controller
>> +
>> +Some of Northstar SoCs's pins can be used for various purposes thanks 
>> to the mux
>> +controller. This binding allows describing mux controller and listing 
>> available
>> +functions. They can be referenced later by other bindings to let 
>> system
>> +configure controller correctly.
>> +
>> +A list of pins varies across chipsets so few bindings are available.
>> +
>> +Required properties:
>> +- compatible: must be one of:
>> +	"brcm,bcm4708-pinmux"
>> +	"brcm,bcm4709-pinmux"
>> +	"brcm,bcm53012-pinmux"
>> +- reg: iomem address range of CRU (Central Resource Unit) pin 
>> registers
> 
> Perhaps 'cru' in the compatible if that's what the h/w is called?
> 
> Also, if this is a sub-block, then it should be a child of the block
> which should be defined here.

It took me some time to do some extra research on the whole CRU thing.

Valuable resources:
[1] bcm5301x_dmu.c (from the SDK)
[2] bcm5301x_pcie.c (from the SDK)
[3] https://patchwork.kernel.org/patch/7888651/
[4] https://patchwork.kernel.org/patch/5051471/

First of all CRU seems to be a sub-block of the DMU (which stands for
"Device Management Unit" according to the [1]). DMU seems to be
independent block at 0x1800c000 with a size of 0x1000.

It isn't actually clear what the CRU stands for. In Broadcom's case:
1. According to the [3] it's "Clock and Reset Unit"
2. According to the [4] it's a "central resource unit"
Other vendors seem to use CRU name for "Clock and Reset Unit".

In any case, you're right Rob, it's a sub-block, a set of random
registers that control SoC.

So I think that:
1) We should have a node for DMU and CRU
2) We should not include "cru" in bindings as pinmuxing seems to be part
    of SoC that just happens to be controlled using CRU registers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 1/2] dt-bindings: pinctrl: document Broadcom Northstar pin mux controller

[Linus Walleij]

On Mon, Oct 15, 2018 at 10:56 AM Rafał Miłecki <rafal@milecki.pl> wrote:

> > Perhaps 'cru' in the compatible if that's what the h/w is called?
> >
> > Also, if this is a sub-block, then it should be a child of the block
> > which should be defined here.
>
> It took me some time to do some extra research on the whole CRU thing.
>
> Valuable resources:
> [1] bcm5301x_dmu.c (from the SDK)
> [2] bcm5301x_pcie.c (from the SDK)
> [3] https://patchwork.kernel.org/patch/7888651/
> [4] https://patchwork.kernel.org/patch/5051471/
>
> First of all CRU seems to be a sub-block of the DMU (which stands for
> "Device Management Unit" according to the [1]). DMU seems to be
> independent block at 0x1800c000 with a size of 0x1000.
>
> It isn't actually clear what the CRU stands for. In Broadcom's case:
> 1. According to the [3] it's "Clock and Reset Unit"
> 2. According to the [4] it's a "central resource unit"
> Other vendors seem to use CRU name for "Clock and Reset Unit".
>
> In any case, you're right Rob, it's a sub-block, a set of random
> registers that control SoC.
>
> So I think that:
> 1) We should have a node for DMU and CRU
> 2) We should not include "cru" in bindings as pinmuxing seems to be part
>     of SoC that just happens to be controlled using CRU registers

OK do you want me to revert the patches or do you want to
fix this stuff on top of the patches that are already in the tree?
I'm fine either way.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 1/2] dt-bindings: pinctrl: document Broadcom Northstar pin mux controller

[Rafał Miłecki]

On Tue, 16 Oct 2018 at 09:41, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Oct 15, 2018 at 10:56 AM Rafał Miłecki <rafal@milecki.pl> wrote:
>
> > > Perhaps 'cru' in the compatible if that's what the h/w is called?
> > >
> > > Also, if this is a sub-block, then it should be a child of the block
> > > which should be defined here.
> >
> > It took me some time to do some extra research on the whole CRU thing.
> >
> > Valuable resources:
> > [1] bcm5301x_dmu.c (from the SDK)
> > [2] bcm5301x_pcie.c (from the SDK)
> > [3] https://patchwork.kernel.org/patch/7888651/
> > [4] https://patchwork.kernel.org/patch/5051471/
> >
> > First of all CRU seems to be a sub-block of the DMU (which stands for
> > "Device Management Unit" according to the [1]). DMU seems to be
> > independent block at 0x1800c000 with a size of 0x1000.
> >
> > It isn't actually clear what the CRU stands for. In Broadcom's case:
> > 1. According to the [3] it's "Clock and Reset Unit"
> > 2. According to the [4] it's a "central resource unit"
> > Other vendors seem to use CRU name for "Clock and Reset Unit".
> >
> > In any case, you're right Rob, it's a sub-block, a set of random
> > registers that control SoC.
> >
> > So I think that:
> > 1) We should have a node for DMU and CRU
> > 2) We should not include "cru" in bindings as pinmuxing seems to be part
> >     of SoC that just happens to be controlled using CRU registers
>
> OK do you want me to revert the patches or do you want to
> fix this stuff on top of the patches that are already in the tree?
> I'm fine either way.

I'm fine either way, thanks for taking care of my work!

-- 
Rafał

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel