* [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl
@ 2021-12-15 20:47 Rafał Miłecki
2021-12-15 20:47 ` [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux Rafał Miłecki
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Rafał Miłecki @ 2021-12-15 20:47 UTC (permalink / raw)
To: Rob Herring, Linus Walleij
Cc: Álvaro Fernández Rojas, Jonas Gorski, Randy Dunlap,
Florian Fainelli, linux-gpio, devicetree,
bcm-kernel-feedback-list, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
It's hardware block that is part of every SoC from BCM4908 family.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
.../pinctrl/brcm,bcm4908-pinctrl.yaml | 72 +++++++++++++++++++
MAINTAINERS | 7 ++
2 files changed, 79 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.yaml
diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.yaml
new file mode 100644
index 000000000000..175a992f15e1
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/brcm,bcm4908-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM4908 pin controller
+
+maintainers:
+ - Rafał Miłecki <rafal@milecki.pl>
+
+description:
+ Binding for pin controller present on BCM4908 family SoCs.
+
+properties:
+ compatible:
+ const: brcm,bcm4908-pinctrl
+
+ reg:
+ maxItems: 1
+
+patternProperties:
+ '-pins$':
+ type: object
+ $ref: pinmux-node.yaml#
+
+ properties:
+ function:
+ enum: [ led_0, led_1, led_2, led_3, led_4, led_5, led_6, led_7, led_8,
+ led_9, led_10, led_11, led_12, led_13, led_14, led_15, led_16,
+ led_17, led_18, led_19, led_20, led_21, led_22, led_23, led_24,
+ led_25, led_26, led_27, led_28, led_29, led_30, led_31,
+ hs_uart, i2c, i2s, nand_ctrl, nand_data, emmc_ctrl, usb0_pwr,
+ usb1_pwr ]
+
+ groups:
+ minItems: 1
+ maxItems: 2
+ items:
+ enum: [ led_0_grp_a, led_1_grp_a, led_2_grp_a, led_3_grp_a,
+ led_4_grp_a, led_5_grp_a, led_6_grp_a, led_7_grp_a,
+ led_8_grp_a, led_9_grp_a, led_10_grp_a, led_10_grp_b,
+ led_11_grp_a, led_11_grp_b, led_12_grp_a, led_12_grp_b,
+ led_13_grp_a, led_13_grp_b, led_14_grp_a, led_15_grp_a,
+ led_16_grp_a, led_17_grp_a, led_18_grp_a, led_19_grp_a,
+ led_20_grp_a, led_21_grp_a, led_22_grp_a, led_23_grp_a,
+ led_24_grp_a, led_25_grp_a, led_26_grp_a, led_27_grp_a,
+ led_28_grp_a, led_29_grp_a, led_30_grp_a, led_31_grp_a,
+ led_31_grp_b, hs_uart_grp, i2c_grp_a, i2c_grp_b, i2s_grp,
+ nand_ctrl_grp, nand_data_grp, emmc_ctrl_grp, usb0_pwr_grp,
+ usb1_pwr_grp ]
+
+allOf:
+ - $ref: pinctrl.yaml#
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ pinctrl@ff800560 {
+ compatible = "brcm,bcm4908-pinctrl";
+ reg = <0xff800560 0x10>;
+
+ led_0-a-pins {
+ function = "led_0";
+ groups = "led_0_grp_a";
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 52653c83de69..949cff4ee1e4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3666,6 +3666,13 @@ F: Documentation/devicetree/bindings/net/brcm,bcm4908-enet.yaml
F: drivers/net/ethernet/broadcom/bcm4908_enet.*
F: drivers/net/ethernet/broadcom/unimac.h
+BROADCOM BCM4908 PINMUX DRIVER
+M: Rafał Miłecki <rafal@milecki.pl>
+M: bcm-kernel-feedback-list@broadcom.com
+L: linux-gpio@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.yaml
+
BROADCOM BCM5301X ARM ARCHITECTURE
M: Hauke Mehrtens <hauke@hauke-m.de>
M: Rafał Miłecki <zajec5@gmail.com>
--
2.31.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux
2021-12-15 20:47 [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl Rafał Miłecki
@ 2021-12-15 20:47 ` Rafał Miłecki
2021-12-16 6:34 ` kernel test robot
` (2 more replies)
2021-12-15 22:27 ` [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl Rob Herring
2021-12-16 15:35 ` Rob Herring
2 siblings, 3 replies; 13+ messages in thread
From: Rafał Miłecki @ 2021-12-15 20:47 UTC (permalink / raw)
To: Rob Herring, Linus Walleij
Cc: Álvaro Fernández Rojas, Jonas Gorski, Randy Dunlap,
Florian Fainelli, linux-gpio, devicetree,
bcm-kernel-feedback-list, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
BCM4908 has its own pins layout so it needs a custom binding and a Linux
driver.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
MAINTAINERS | 1 +
drivers/pinctrl/bcm/Kconfig | 13 +
drivers/pinctrl/bcm/Makefile | 1 +
drivers/pinctrl/bcm/pinctrl-bcm4908.c | 561 ++++++++++++++++++++++++++
4 files changed, 576 insertions(+)
create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm4908.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 949cff4ee1e4..b906c094a4f6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3672,6 +3672,7 @@ M: bcm-kernel-feedback-list@broadcom.com
L: linux-gpio@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.yaml
+F: drivers/pinctrl/bcm/pinctrl-bcm4908.c
BROADCOM BCM5301X ARM ARCHITECTURE
M: Hauke Mehrtens <hauke@hauke-m.de>
diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig
index 5123f4c33854..ea5034cce644 100644
--- a/drivers/pinctrl/bcm/Kconfig
+++ b/drivers/pinctrl/bcm/Kconfig
@@ -29,6 +29,19 @@ config PINCTRL_BCM2835
help
Say Y here to enable the Broadcom BCM2835 GPIO driver.
+config PINCTRL_BCM4908
+ bool "Broadcom BCM4908 pinmux driver"
+ depends on OF && (ARCH_BCM4908 || COMPILE_TEST)
+ select PINMUX
+ select PINCONF
+ select GENERIC_PINCONF
+ select GENERIC_PINCTRL_GROUPS
+ select GENERIC_PINMUX_FUNCTIONS
+ default ARCH_BCM4908
+ help
+ Say Y here to enable driver for BCM4908 family SoCs with integrated
+ pin controller.
+
config PINCTRL_BCM63XX
bool
select PINMUX
diff --git a/drivers/pinctrl/bcm/Makefile b/drivers/pinctrl/bcm/Makefile
index 00c7b7775e63..82b868ec1471 100644
--- a/drivers/pinctrl/bcm/Makefile
+++ b/drivers/pinctrl/bcm/Makefile
@@ -3,6 +3,7 @@
obj-$(CONFIG_PINCTRL_BCM281XX) += pinctrl-bcm281xx.o
obj-$(CONFIG_PINCTRL_BCM2835) += pinctrl-bcm2835.o
+obj-$(CONFIG_PINCTRL_BCM4908) += pinctrl-bcm4908.o
obj-$(CONFIG_PINCTRL_BCM63XX) += pinctrl-bcm63xx.o
obj-$(CONFIG_PINCTRL_BCM6318) += pinctrl-bcm6318.o
obj-$(CONFIG_PINCTRL_BCM6328) += pinctrl-bcm6328.o
diff --git a/drivers/pinctrl/bcm/pinctrl-bcm4908.c b/drivers/pinctrl/bcm/pinctrl-bcm4908.c
new file mode 100644
index 000000000000..2de565c2ef3b
--- /dev/null
+++ b/drivers/pinctrl/bcm/pinctrl-bcm4908.c
@@ -0,0 +1,561 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 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>
+
+#include "../core.h"
+#include "../pinmux.h"
+
+#define BCM4908_NUM_PINS 86
+
+#define TEST_PORT_BLOCK_EN_LSB 0x00
+#define TEST_PORT_BLOCK_DATA_MSB 0x04
+#define TEST_PORT_BLOCK_DATA_LSB 0x08
+#define TEST_PORT_LSB_PINMUX_DATA_SHIFT 12
+#define TEST_PORT_COMMAND 0x0c
+#define TEST_PORT_CMD_LOAD_MUX_REG 0x00000021
+
+struct bcm4908_pinctrl {
+ struct device *dev;
+ struct pinctrl_dev *pctldev;
+ void __iomem *base;
+
+ struct pinctrl_desc pctldesc;
+};
+
+/*
+ * Groups
+ */
+
+struct bcm4908_pinctrl_pin_setup {
+ unsigned int number;
+ unsigned int function;
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_0_pins_a[] = {
+ { 0, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_1_pins_a[] = {
+ { 1, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_2_pins_a[] = {
+ { 2, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_3_pins_a[] = {
+ { 3, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_4_pins_a[] = {
+ { 4, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_5_pins_a[] = {
+ { 5, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_6_pins_a[] = {
+ { 6, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_7_pins_a[] = {
+ { 7, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_8_pins_a[] = {
+ { 8, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_9_pins_a[] = {
+ { 9, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_10_pins_a[] = {
+ { 10, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_11_pins_a[] = {
+ { 11, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_12_pins_a[] = {
+ { 12, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_13_pins_a[] = {
+ { 13, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_14_pins_a[] = {
+ { 14, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_15_pins_a[] = {
+ { 15, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_16_pins_a[] = {
+ { 16, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_17_pins_a[] = {
+ { 17, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_18_pins_a[] = {
+ { 18, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_19_pins_a[] = {
+ { 19, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_20_pins_a[] = {
+ { 20, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_21_pins_a[] = {
+ { 21, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_22_pins_a[] = {
+ { 22, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_23_pins_a[] = {
+ { 23, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_24_pins_a[] = {
+ { 24, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_25_pins_a[] = {
+ { 25, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_26_pins_a[] = {
+ { 26, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_27_pins_a[] = {
+ { 27, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_28_pins_a[] = {
+ { 28, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_29_pins_a[] = {
+ { 29, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_30_pins_a[] = {
+ { 30, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_31_pins_a[] = {
+ { 31, 3 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_10_pins_b[] = {
+ { 8, 2 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_11_pins_b[] = {
+ { 9, 2 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_12_pins_b[] = {
+ { 0, 2 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_13_pins_b[] = {
+ { 1, 2 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup led_31_pins_b[] = {
+ { 30, 2 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup hs_uart_pins[] = {
+ { 10, 0 }, /* CTS */
+ { 11, 0 }, /* RTS */
+ { 12, 0 }, /* RXD */
+ { 13, 0 }, /* TXD */
+};
+
+static const struct bcm4908_pinctrl_pin_setup i2c_pins_a[] = {
+ { 18, 0 }, /* SDA */
+ { 19, 0 }, /* SCL */
+};
+
+static const struct bcm4908_pinctrl_pin_setup i2c_pins_b[] = {
+ { 22, 0 }, /* SDA */
+ { 23, 0 }, /* SCL */
+};
+
+static const struct bcm4908_pinctrl_pin_setup i2s_pins[] = {
+ { 27, 0 }, /* MCLK */
+ { 28, 0 }, /* LRCK */
+ { 29, 0 }, /* SDATA */
+ { 30, 0 }, /* SCLK */
+};
+
+static const struct bcm4908_pinctrl_pin_setup nand_ctrl_pins[] = {
+ { 32, 0 },
+ { 33, 0 },
+ { 34, 0 },
+ { 43, 0 },
+ { 44, 0 },
+ { 45, 0 },
+ { 56, 1 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup nand_data_pins[] = {
+ { 35, 0 },
+ { 36, 0 },
+ { 37, 0 },
+ { 38, 0 },
+ { 39, 0 },
+ { 40, 0 },
+ { 41, 0 },
+ { 42, 0 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup emmc_ctrl_pins[] = {
+ { 46, 0 },
+ { 47, 0 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup usb0_pwr_pins[] = {
+ { 63, 0 },
+ { 64, 0 },
+};
+
+static const struct bcm4908_pinctrl_pin_setup usb1_pwr_pins[] = {
+ { 66, 0 },
+ { 67, 0 },
+};
+
+struct bcm4908_pinctrl_grp {
+ const char *name;
+ const struct bcm4908_pinctrl_pin_setup *pins;
+ const unsigned int num_pins;
+};
+
+static const struct bcm4908_pinctrl_grp bcm4908_pinctrl_grps[] = {
+ { "led_0_grp_a", led_0_pins_a, ARRAY_SIZE(led_0_pins_a) },
+ { "led_1_grp_a", led_1_pins_a, ARRAY_SIZE(led_1_pins_a) },
+ { "led_2_grp_a", led_2_pins_a, ARRAY_SIZE(led_2_pins_a) },
+ { "led_3_grp_a", led_3_pins_a, ARRAY_SIZE(led_3_pins_a) },
+ { "led_4_grp_a", led_4_pins_a, ARRAY_SIZE(led_4_pins_a) },
+ { "led_5_grp_a", led_5_pins_a, ARRAY_SIZE(led_5_pins_a) },
+ { "led_6_grp_a", led_6_pins_a, ARRAY_SIZE(led_6_pins_a) },
+ { "led_7_grp_a", led_7_pins_a, ARRAY_SIZE(led_7_pins_a) },
+ { "led_8_grp_a", led_8_pins_a, ARRAY_SIZE(led_8_pins_a) },
+ { "led_9_grp_a", led_9_pins_a, ARRAY_SIZE(led_9_pins_a) },
+ { "led_10_grp_a", led_10_pins_a, ARRAY_SIZE(led_10_pins_a) },
+ { "led_11_grp_a", led_11_pins_a, ARRAY_SIZE(led_11_pins_a) },
+ { "led_12_grp_a", led_12_pins_a, ARRAY_SIZE(led_12_pins_a) },
+ { "led_13_grp_a", led_13_pins_a, ARRAY_SIZE(led_13_pins_a) },
+ { "led_14_grp_a", led_14_pins_a, ARRAY_SIZE(led_14_pins_a) },
+ { "led_15_grp_a", led_15_pins_a, ARRAY_SIZE(led_15_pins_a) },
+ { "led_16_grp_a", led_16_pins_a, ARRAY_SIZE(led_16_pins_a) },
+ { "led_17_grp_a", led_17_pins_a, ARRAY_SIZE(led_17_pins_a) },
+ { "led_18_grp_a", led_18_pins_a, ARRAY_SIZE(led_18_pins_a) },
+ { "led_19_grp_a", led_19_pins_a, ARRAY_SIZE(led_19_pins_a) },
+ { "led_20_grp_a", led_20_pins_a, ARRAY_SIZE(led_20_pins_a) },
+ { "led_21_grp_a", led_21_pins_a, ARRAY_SIZE(led_21_pins_a) },
+ { "led_22_grp_a", led_22_pins_a, ARRAY_SIZE(led_22_pins_a) },
+ { "led_23_grp_a", led_23_pins_a, ARRAY_SIZE(led_23_pins_a) },
+ { "led_24_grp_a", led_24_pins_a, ARRAY_SIZE(led_24_pins_a) },
+ { "led_25_grp_a", led_25_pins_a, ARRAY_SIZE(led_25_pins_a) },
+ { "led_26_grp_a", led_26_pins_a, ARRAY_SIZE(led_26_pins_a) },
+ { "led_27_grp_a", led_27_pins_a, ARRAY_SIZE(led_27_pins_a) },
+ { "led_28_grp_a", led_28_pins_a, ARRAY_SIZE(led_28_pins_a) },
+ { "led_29_grp_a", led_29_pins_a, ARRAY_SIZE(led_29_pins_a) },
+ { "led_30_grp_a", led_30_pins_a, ARRAY_SIZE(led_30_pins_a) },
+ { "led_31_grp_a", led_31_pins_a, ARRAY_SIZE(led_31_pins_a) },
+ { "led_10_grp_b", led_10_pins_b, ARRAY_SIZE(led_10_pins_b) },
+ { "led_11_grp_b", led_11_pins_b, ARRAY_SIZE(led_11_pins_b) },
+ { "led_12_grp_b", led_12_pins_b, ARRAY_SIZE(led_12_pins_b) },
+ { "led_13_grp_b", led_13_pins_b, ARRAY_SIZE(led_13_pins_b) },
+ { "led_31_grp_b", led_31_pins_b, ARRAY_SIZE(led_31_pins_b) },
+ { "hs_uart_grp", hs_uart_pins, ARRAY_SIZE(hs_uart_pins) },
+ { "i2c_grp_a", i2c_pins_a, ARRAY_SIZE(i2c_pins_a) },
+ { "i2c_grp_b", i2c_pins_b, ARRAY_SIZE(i2c_pins_b) },
+ { "i2s_grp", i2s_pins, ARRAY_SIZE(i2s_pins) },
+ { "nand_ctrl_grp", nand_ctrl_pins, ARRAY_SIZE(nand_ctrl_pins) },
+ { "nand_data_grp", nand_data_pins, ARRAY_SIZE(nand_data_pins) },
+ { "emmc_ctrl_grp", emmc_ctrl_pins, ARRAY_SIZE(emmc_ctrl_pins) },
+ { "usb0_pwr_grp", usb0_pwr_pins, ARRAY_SIZE(usb0_pwr_pins) },
+ { "usb1_pwr_grp", usb1_pwr_pins, ARRAY_SIZE(usb1_pwr_pins) },
+};
+
+/*
+ * Functions
+ */
+
+struct bcm4908_pinctrl_function {
+ const char *name;
+ const char * const *groups;
+ const unsigned int num_groups;
+};
+
+static const char * const led_0_groups[] = { "led_0_grp_a" };
+static const char * const led_1_groups[] = { "led_1_grp_a" };
+static const char * const led_2_groups[] = { "led_2_grp_a" };
+static const char * const led_3_groups[] = { "led_3_grp_a" };
+static const char * const led_4_groups[] = { "led_4_grp_a" };
+static const char * const led_5_groups[] = { "led_5_grp_a" };
+static const char * const led_6_groups[] = { "led_6_grp_a" };
+static const char * const led_7_groups[] = { "led_7_grp_a" };
+static const char * const led_8_groups[] = { "led_8_grp_a" };
+static const char * const led_9_groups[] = { "led_9_grp_a" };
+static const char * const led_10_groups[] = { "led_10_grp_a", "led_10_grp_b" };
+static const char * const led_11_groups[] = { "led_11_grp_a", "led_11_grp_b" };
+static const char * const led_12_groups[] = { "led_12_grp_a", "led_12_grp_b" };
+static const char * const led_13_groups[] = { "led_13_grp_a", "led_13_grp_b" };
+static const char * const led_14_groups[] = { "led_14_grp_a" };
+static const char * const led_15_groups[] = { "led_15_grp_a" };
+static const char * const led_16_groups[] = { "led_16_grp_a" };
+static const char * const led_17_groups[] = { "led_17_grp_a" };
+static const char * const led_18_groups[] = { "led_18_grp_a" };
+static const char * const led_19_groups[] = { "led_19_grp_a" };
+static const char * const led_20_groups[] = { "led_20_grp_a" };
+static const char * const led_21_groups[] = { "led_21_grp_a" };
+static const char * const led_22_groups[] = { "led_22_grp_a" };
+static const char * const led_23_groups[] = { "led_23_grp_a" };
+static const char * const led_24_groups[] = { "led_24_grp_a" };
+static const char * const led_25_groups[] = { "led_25_grp_a" };
+static const char * const led_26_groups[] = { "led_26_grp_a" };
+static const char * const led_27_groups[] = { "led_27_grp_a" };
+static const char * const led_28_groups[] = { "led_28_grp_a" };
+static const char * const led_29_groups[] = { "led_29_grp_a" };
+static const char * const led_30_groups[] = { "led_30_grp_a" };
+static const char * const led_31_groups[] = { "led_31_grp_a", "led_31_grp_b" };
+static const char * const hs_uart_groups[] = { "hs_uart_grp" };
+static const char * const i2c_groups[] = { "i2c_grp_a", "i2c_grp_b" };
+static const char * const i2s_groups[] = { "i2s_grp" };
+static const char * const nand_ctrl_groups[] = { "nand_ctrl_grp" };
+static const char * const nand_data_groups[] = { "nand_data_grp" };
+static const char * const emmc_ctrl_groups[] = { "emmc_ctrl_grp" };
+static const char * const usb0_pwr_groups[] = { "usb0_pwr_grp" };
+static const char * const usb1_pwr_groups[] = { "usb1_pwr_grp" };
+
+static const struct bcm4908_pinctrl_function bcm4908_pinctrl_functions[] = {
+ { "led_0", led_0_groups, ARRAY_SIZE(led_0_groups) },
+ { "led_1", led_1_groups, ARRAY_SIZE(led_1_groups) },
+ { "led_2", led_2_groups, ARRAY_SIZE(led_2_groups) },
+ { "led_3", led_3_groups, ARRAY_SIZE(led_3_groups) },
+ { "led_4", led_4_groups, ARRAY_SIZE(led_4_groups) },
+ { "led_5", led_5_groups, ARRAY_SIZE(led_5_groups) },
+ { "led_6", led_6_groups, ARRAY_SIZE(led_6_groups) },
+ { "led_7", led_7_groups, ARRAY_SIZE(led_7_groups) },
+ { "led_8", led_8_groups, ARRAY_SIZE(led_8_groups) },
+ { "led_9", led_9_groups, ARRAY_SIZE(led_9_groups) },
+ { "led_10", led_10_groups, ARRAY_SIZE(led_10_groups) },
+ { "led_11", led_11_groups, ARRAY_SIZE(led_11_groups) },
+ { "led_12", led_12_groups, ARRAY_SIZE(led_12_groups) },
+ { "led_13", led_13_groups, ARRAY_SIZE(led_13_groups) },
+ { "led_14", led_14_groups, ARRAY_SIZE(led_14_groups) },
+ { "led_15", led_15_groups, ARRAY_SIZE(led_15_groups) },
+ { "led_16", led_16_groups, ARRAY_SIZE(led_16_groups) },
+ { "led_17", led_17_groups, ARRAY_SIZE(led_17_groups) },
+ { "led_18", led_18_groups, ARRAY_SIZE(led_18_groups) },
+ { "led_19", led_19_groups, ARRAY_SIZE(led_19_groups) },
+ { "led_20", led_20_groups, ARRAY_SIZE(led_20_groups) },
+ { "led_21", led_21_groups, ARRAY_SIZE(led_21_groups) },
+ { "led_22", led_22_groups, ARRAY_SIZE(led_22_groups) },
+ { "led_23", led_23_groups, ARRAY_SIZE(led_23_groups) },
+ { "led_24", led_24_groups, ARRAY_SIZE(led_24_groups) },
+ { "led_25", led_25_groups, ARRAY_SIZE(led_25_groups) },
+ { "led_26", led_26_groups, ARRAY_SIZE(led_26_groups) },
+ { "led_27", led_27_groups, ARRAY_SIZE(led_27_groups) },
+ { "led_28", led_28_groups, ARRAY_SIZE(led_28_groups) },
+ { "led_29", led_29_groups, ARRAY_SIZE(led_29_groups) },
+ { "led_30", led_30_groups, ARRAY_SIZE(led_30_groups) },
+ { "led_31", led_31_groups, ARRAY_SIZE(led_31_groups) },
+ { "hs_uart", hs_uart_groups, ARRAY_SIZE(hs_uart_groups) },
+ { "i2c", i2c_groups, ARRAY_SIZE(i2c_groups) },
+ { "i2s", i2s_groups, ARRAY_SIZE(i2s_groups) },
+ { "nand_ctrl", nand_ctrl_groups, ARRAY_SIZE(nand_ctrl_groups) },
+ { "nand_data", nand_data_groups, ARRAY_SIZE(nand_data_groups) },
+ { "emmc_ctrl", emmc_ctrl_groups, ARRAY_SIZE(emmc_ctrl_groups) },
+ { "usb0_pwr", usb0_pwr_groups, ARRAY_SIZE(usb0_pwr_groups) },
+ { "usb1_pwr", usb1_pwr_groups, ARRAY_SIZE(usb1_pwr_groups) },
+};
+
+/*
+ * Groups code
+ */
+
+static const struct pinctrl_ops bcm4908_pinctrl_ops = {
+ .get_groups_count = pinctrl_generic_get_group_count,
+ .get_group_name = pinctrl_generic_get_group_name,
+ .get_group_pins = pinctrl_generic_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 bcm4908_pinctrl_set_mux(struct pinctrl_dev *pctrl_dev,
+ unsigned int func_selector,
+ unsigned int group_selector)
+{
+ struct bcm4908_pinctrl *bcm4908_pinctrl = pinctrl_dev_get_drvdata(pctrl_dev);
+ const struct bcm4908_pinctrl_grp *group;
+ struct group_desc *group_desc;
+ int i;
+
+ group_desc = pinctrl_generic_get_group(pctrl_dev, group_selector);
+ if (!group_desc)
+ return -EINVAL;
+ group = group_desc->data;
+
+ for (i = 0; i < group->num_pins; i++) {
+ u32 lsb = 0;
+
+ lsb |= group->pins[i].number;
+ lsb |= group->pins[i].function << TEST_PORT_LSB_PINMUX_DATA_SHIFT;
+
+ writel(0x0, bcm4908_pinctrl->base + TEST_PORT_BLOCK_DATA_MSB);
+ writel(lsb, bcm4908_pinctrl->base + TEST_PORT_BLOCK_DATA_LSB);
+ writel(TEST_PORT_CMD_LOAD_MUX_REG, bcm4908_pinctrl->base + TEST_PORT_COMMAND);
+ }
+
+ return 0;
+}
+
+static const struct pinmux_ops bcm4908_pinctrl_pmxops = {
+ .get_functions_count = pinmux_generic_get_function_count,
+ .get_function_name = pinmux_generic_get_function_name,
+ .get_function_groups = pinmux_generic_get_function_groups,
+ .set_mux = bcm4908_pinctrl_set_mux,
+};
+
+/*
+ * Controller code
+ */
+
+static struct pinctrl_desc bcm4908_pinctrl_desc = {
+ .name = "bcm4908-pinctrl",
+ .pctlops = &bcm4908_pinctrl_ops,
+ .pmxops = &bcm4908_pinctrl_pmxops,
+};
+
+static const struct of_device_id bcm4908_pinctrl_of_match_table[] = {
+ { .compatible = "brcm,bcm4908-pinctrl", },
+ { }
+};
+
+static int bcm4908_pinctrl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct bcm4908_pinctrl *bcm4908_pinctrl;
+ struct pinctrl_desc *pctldesc;
+ struct pinctrl_pin_desc *pins;
+ int i;
+
+ bcm4908_pinctrl = devm_kzalloc(dev, sizeof(*bcm4908_pinctrl), GFP_KERNEL);
+ if (!bcm4908_pinctrl)
+ return -ENOMEM;
+ pctldesc = &bcm4908_pinctrl->pctldesc;
+ platform_set_drvdata(pdev, bcm4908_pinctrl);
+
+ /* Set basic properties */
+
+ bcm4908_pinctrl->dev = dev;
+
+ bcm4908_pinctrl->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(bcm4908_pinctrl->base)) {
+ dev_err(dev, "Failed to map pinctrl regs\n");
+ return PTR_ERR(bcm4908_pinctrl->base);
+ }
+
+ memcpy(pctldesc, &bcm4908_pinctrl_desc, sizeof(*pctldesc));
+
+ /* Set pinctrl properties */
+
+ pins = devm_kcalloc(dev, BCM4908_NUM_PINS,
+ sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
+ if (!pins)
+ return -ENOMEM;
+ for (i = 0; i < BCM4908_NUM_PINS; i++) {
+ pins[i].number = i;
+ pins[i].name = devm_kasprintf(dev, GFP_KERNEL, "pin%d", i);
+ if (!pins[i].name)
+ return -ENOMEM;
+ }
+ pctldesc->pins = pins;
+ pctldesc->npins = BCM4908_NUM_PINS;
+
+ /* Register */
+
+ bcm4908_pinctrl->pctldev = devm_pinctrl_register(dev, pctldesc, bcm4908_pinctrl);
+ if (IS_ERR(bcm4908_pinctrl->pctldev)) {
+ dev_err(dev, "Failed to register pinctrl\n");
+ return PTR_ERR(bcm4908_pinctrl->pctldev);
+ }
+
+ /* Groups */
+
+ for (i = 0; i < ARRAY_SIZE(bcm4908_pinctrl_grps); i++) {
+ const struct bcm4908_pinctrl_grp *group = &bcm4908_pinctrl_grps[i];
+ int *pins;
+ int j;
+
+ pins = devm_kcalloc(dev, group->num_pins, sizeof(*pins), GFP_KERNEL);
+ if (!pins)
+ return -ENOMEM;
+ for (j = 0; j < group->num_pins; j++)
+ pins[j] = group->pins[j].number;
+
+ pinctrl_generic_add_group(bcm4908_pinctrl->pctldev, group->name,
+ pins, group->num_pins, (void *)group);
+ }
+
+ /* Functions */
+
+ for (i = 0; i < ARRAY_SIZE(bcm4908_pinctrl_functions); i++) {
+ const struct bcm4908_pinctrl_function *function = &bcm4908_pinctrl_functions[i];
+
+ pinmux_generic_add_function(bcm4908_pinctrl->pctldev,
+ function->name,
+ function->groups,
+ function->num_groups, NULL);
+ }
+
+ return 0;
+}
+
+static struct platform_driver bcm4908_pinctrl_driver = {
+ .probe = bcm4908_pinctrl_probe,
+ .driver = {
+ .name = "bcm4908-pinctrl",
+ .of_match_table = bcm4908_pinctrl_of_match_table,
+ },
+};
+
+module_platform_driver(bcm4908_pinctrl_driver);
+
+MODULE_AUTHOR("Rafał Miłecki");
+MODULE_LICENSE("GPL v2");
+MODULE_DEVICE_TABLE(of, bcm4908_pinctrl_of_match_table);
--
2.31.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl
2021-12-15 20:47 [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl Rafał Miłecki
2021-12-15 20:47 ` [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux Rafał Miłecki
@ 2021-12-15 22:27 ` Rob Herring
2021-12-15 22:35 ` Rafał Miłecki
2021-12-16 15:35 ` Rob Herring
2 siblings, 1 reply; 13+ messages in thread
From: Rob Herring @ 2021-12-15 22:27 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Linus Walleij, Rafał Miłecki, Jonas Gorski,
Randy Dunlap, Álvaro Fernández Rojas,
bcm-kernel-feedback-list, devicetree, linux-gpio,
Florian Fainelli, Rob Herring
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1766 bytes --]
On Wed, 15 Dec 2021 21:47:52 +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> It's hardware block that is part of every SoC from BCM4908 family.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> .../pinctrl/brcm,bcm4908-pinctrl.yaml | 72 +++++++++++++++++++
> MAINTAINERS | 7 ++
> 2 files changed, 79 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/pinctrl/pinctrl.yaml'
xargs: dt-doc-validate: exited with status 255; aborting
make[1]: *** Deleting file 'Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.example.dt.yaml'
Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/pinctrl/pinctrl.yaml'
make[1]: *** [scripts/Makefile.lib:373: Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.example.dt.yaml] Error 255
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1413: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1568760
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl
2021-12-15 22:27 ` [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl Rob Herring
@ 2021-12-15 22:35 ` Rafał Miłecki
2021-12-16 15:32 ` Rob Herring
0 siblings, 1 reply; 13+ messages in thread
From: Rafał Miłecki @ 2021-12-15 22:35 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Rafał Miłecki, Jonas Gorski,
Randy Dunlap, Álvaro Fernández Rojas,
bcm-kernel-feedback-list, devicetree, linux-gpio,
Florian Fainelli, Rob Herring
Hi Rob,
On 15.12.2021 23:27, Rob Herring wrote:
> On Wed, 15 Dec 2021 21:47:52 +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> It's hardware block that is part of every SoC from BCM4908 family.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>> .../pinctrl/brcm,bcm4908-pinctrl.yaml | 72 +++++++++++++++++++
>> MAINTAINERS | 7 ++
>> 2 files changed, 79 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.yaml
>>
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/pinctrl/pinctrl.yaml'
> xargs: dt-doc-validate: exited with status 255; aborting
> make[1]: *** Deleting file 'Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.example.dt.yaml'
> Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/pinctrl/pinctrl.yaml'
> make[1]: *** [scripts/Makefile.lib:373: Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.example.dt.yaml] Error 255
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1413: dt_binding_check] Error 2
this patch targets Linus's git tree:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=for-next
Above tree contains 896568e5b9c8 ("dt-bindings: pinctrl: convert
controller description to the json-schema") which provides pinctrl.yaml.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux
2021-12-15 20:47 ` [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux Rafał Miłecki
@ 2021-12-16 6:34 ` kernel test robot
2021-12-16 14:26 ` kernel test robot
2021-12-16 19:55 ` Andy Shevchenko
2 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2021-12-16 6:34 UTC (permalink / raw)
To: Rafał Miłecki, Rob Herring, Linus Walleij
Cc: kbuild-all, Álvaro Fernández Rojas, Jonas Gorski,
Randy Dunlap, Florian Fainelli, linux-gpio, devicetree,
bcm-kernel-feedback-list, Rafał Miłecki
Hi "Rafał,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linusw-pinctrl/devel]
[also build test WARNING on robh/for-next linus/master v5.16-rc5 next-20211215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Rafa-Mi-ecki/dt-bindings-pinctrl-Add-binding-for-BCM4908-pinctrl/20211216-044855
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20211216/202112161405.FBvHqiWr-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/52ad0e1851a5d242cde2829eca853a7369807c42
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rafa-Mi-ecki/dt-bindings-pinctrl-Add-binding-for-BCM4908-pinctrl/20211216-044855
git checkout 52ad0e1851a5d242cde2829eca853a7369807c42
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=riscv SHELL=/bin/bash drivers/pinctrl/bcm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/pinctrl/bcm/pinctrl-bcm4908.c: In function 'bcm4908_pinctrl_probe':
>> drivers/pinctrl/bcm/pinctrl-bcm4908.c:542:53: warning: passing argument 3 of 'pinmux_generic_add_function' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
542 | function->groups,
| ~~~~~~~~^~~~~~~~
In file included from drivers/pinctrl/bcm/pinctrl-bcm4908.c:18:
drivers/pinctrl/bcm/../pinmux.h:153:46: note: expected 'const char **' but argument is of type 'const char * const*'
153 | const char **groups,
| ~~~~~~~~~~~~~^~~~~~
vim +542 drivers/pinctrl/bcm/pinctrl-bcm4908.c
468
469 static int bcm4908_pinctrl_probe(struct platform_device *pdev)
470 {
471 struct device *dev = &pdev->dev;
472 struct bcm4908_pinctrl *bcm4908_pinctrl;
473 struct pinctrl_desc *pctldesc;
474 struct pinctrl_pin_desc *pins;
475 int i;
476
477 bcm4908_pinctrl = devm_kzalloc(dev, sizeof(*bcm4908_pinctrl), GFP_KERNEL);
478 if (!bcm4908_pinctrl)
479 return -ENOMEM;
480 pctldesc = &bcm4908_pinctrl->pctldesc;
481 platform_set_drvdata(pdev, bcm4908_pinctrl);
482
483 /* Set basic properties */
484
485 bcm4908_pinctrl->dev = dev;
486
487 bcm4908_pinctrl->base = devm_platform_ioremap_resource(pdev, 0);
488 if (IS_ERR(bcm4908_pinctrl->base)) {
489 dev_err(dev, "Failed to map pinctrl regs\n");
490 return PTR_ERR(bcm4908_pinctrl->base);
491 }
492
493 memcpy(pctldesc, &bcm4908_pinctrl_desc, sizeof(*pctldesc));
494
495 /* Set pinctrl properties */
496
497 pins = devm_kcalloc(dev, BCM4908_NUM_PINS,
498 sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
499 if (!pins)
500 return -ENOMEM;
501 for (i = 0; i < BCM4908_NUM_PINS; i++) {
502 pins[i].number = i;
503 pins[i].name = devm_kasprintf(dev, GFP_KERNEL, "pin%d", i);
504 if (!pins[i].name)
505 return -ENOMEM;
506 }
507 pctldesc->pins = pins;
508 pctldesc->npins = BCM4908_NUM_PINS;
509
510 /* Register */
511
512 bcm4908_pinctrl->pctldev = devm_pinctrl_register(dev, pctldesc, bcm4908_pinctrl);
513 if (IS_ERR(bcm4908_pinctrl->pctldev)) {
514 dev_err(dev, "Failed to register pinctrl\n");
515 return PTR_ERR(bcm4908_pinctrl->pctldev);
516 }
517
518 /* Groups */
519
520 for (i = 0; i < ARRAY_SIZE(bcm4908_pinctrl_grps); i++) {
521 const struct bcm4908_pinctrl_grp *group = &bcm4908_pinctrl_grps[i];
522 int *pins;
523 int j;
524
525 pins = devm_kcalloc(dev, group->num_pins, sizeof(*pins), GFP_KERNEL);
526 if (!pins)
527 return -ENOMEM;
528 for (j = 0; j < group->num_pins; j++)
529 pins[j] = group->pins[j].number;
530
531 pinctrl_generic_add_group(bcm4908_pinctrl->pctldev, group->name,
532 pins, group->num_pins, (void *)group);
533 }
534
535 /* Functions */
536
537 for (i = 0; i < ARRAY_SIZE(bcm4908_pinctrl_functions); i++) {
538 const struct bcm4908_pinctrl_function *function = &bcm4908_pinctrl_functions[i];
539
540 pinmux_generic_add_function(bcm4908_pinctrl->pctldev,
541 function->name,
> 542 function->groups,
543 function->num_groups, NULL);
544 }
545
546 return 0;
547 }
548
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux
@ 2021-12-16 6:34 ` kernel test robot
0 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2021-12-16 6:34 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5376 bytes --]
Hi "Rafał,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linusw-pinctrl/devel]
[also build test WARNING on robh/for-next linus/master v5.16-rc5 next-20211215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Rafa-Mi-ecki/dt-bindings-pinctrl-Add-binding-for-BCM4908-pinctrl/20211216-044855
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20211216/202112161405.FBvHqiWr-lkp(a)intel.com/config)
compiler: riscv64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/52ad0e1851a5d242cde2829eca853a7369807c42
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rafa-Mi-ecki/dt-bindings-pinctrl-Add-binding-for-BCM4908-pinctrl/20211216-044855
git checkout 52ad0e1851a5d242cde2829eca853a7369807c42
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=riscv SHELL=/bin/bash drivers/pinctrl/bcm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/pinctrl/bcm/pinctrl-bcm4908.c: In function 'bcm4908_pinctrl_probe':
>> drivers/pinctrl/bcm/pinctrl-bcm4908.c:542:53: warning: passing argument 3 of 'pinmux_generic_add_function' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
542 | function->groups,
| ~~~~~~~~^~~~~~~~
In file included from drivers/pinctrl/bcm/pinctrl-bcm4908.c:18:
drivers/pinctrl/bcm/../pinmux.h:153:46: note: expected 'const char **' but argument is of type 'const char * const*'
153 | const char **groups,
| ~~~~~~~~~~~~~^~~~~~
vim +542 drivers/pinctrl/bcm/pinctrl-bcm4908.c
468
469 static int bcm4908_pinctrl_probe(struct platform_device *pdev)
470 {
471 struct device *dev = &pdev->dev;
472 struct bcm4908_pinctrl *bcm4908_pinctrl;
473 struct pinctrl_desc *pctldesc;
474 struct pinctrl_pin_desc *pins;
475 int i;
476
477 bcm4908_pinctrl = devm_kzalloc(dev, sizeof(*bcm4908_pinctrl), GFP_KERNEL);
478 if (!bcm4908_pinctrl)
479 return -ENOMEM;
480 pctldesc = &bcm4908_pinctrl->pctldesc;
481 platform_set_drvdata(pdev, bcm4908_pinctrl);
482
483 /* Set basic properties */
484
485 bcm4908_pinctrl->dev = dev;
486
487 bcm4908_pinctrl->base = devm_platform_ioremap_resource(pdev, 0);
488 if (IS_ERR(bcm4908_pinctrl->base)) {
489 dev_err(dev, "Failed to map pinctrl regs\n");
490 return PTR_ERR(bcm4908_pinctrl->base);
491 }
492
493 memcpy(pctldesc, &bcm4908_pinctrl_desc, sizeof(*pctldesc));
494
495 /* Set pinctrl properties */
496
497 pins = devm_kcalloc(dev, BCM4908_NUM_PINS,
498 sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
499 if (!pins)
500 return -ENOMEM;
501 for (i = 0; i < BCM4908_NUM_PINS; i++) {
502 pins[i].number = i;
503 pins[i].name = devm_kasprintf(dev, GFP_KERNEL, "pin%d", i);
504 if (!pins[i].name)
505 return -ENOMEM;
506 }
507 pctldesc->pins = pins;
508 pctldesc->npins = BCM4908_NUM_PINS;
509
510 /* Register */
511
512 bcm4908_pinctrl->pctldev = devm_pinctrl_register(dev, pctldesc, bcm4908_pinctrl);
513 if (IS_ERR(bcm4908_pinctrl->pctldev)) {
514 dev_err(dev, "Failed to register pinctrl\n");
515 return PTR_ERR(bcm4908_pinctrl->pctldev);
516 }
517
518 /* Groups */
519
520 for (i = 0; i < ARRAY_SIZE(bcm4908_pinctrl_grps); i++) {
521 const struct bcm4908_pinctrl_grp *group = &bcm4908_pinctrl_grps[i];
522 int *pins;
523 int j;
524
525 pins = devm_kcalloc(dev, group->num_pins, sizeof(*pins), GFP_KERNEL);
526 if (!pins)
527 return -ENOMEM;
528 for (j = 0; j < group->num_pins; j++)
529 pins[j] = group->pins[j].number;
530
531 pinctrl_generic_add_group(bcm4908_pinctrl->pctldev, group->name,
532 pins, group->num_pins, (void *)group);
533 }
534
535 /* Functions */
536
537 for (i = 0; i < ARRAY_SIZE(bcm4908_pinctrl_functions); i++) {
538 const struct bcm4908_pinctrl_function *function = &bcm4908_pinctrl_functions[i];
539
540 pinmux_generic_add_function(bcm4908_pinctrl->pctldev,
541 function->name,
> 542 function->groups,
543 function->num_groups, NULL);
544 }
545
546 return 0;
547 }
548
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux
2021-12-15 20:47 ` [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux Rafał Miłecki
@ 2021-12-16 14:26 ` kernel test robot
2021-12-16 14:26 ` kernel test robot
2021-12-16 19:55 ` Andy Shevchenko
2 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2021-12-16 14:26 UTC (permalink / raw)
To: Rafał Miłecki, Rob Herring, Linus Walleij
Cc: kbuild-all, Álvaro Fernández Rojas, Jonas Gorski,
Randy Dunlap, Florian Fainelli, linux-gpio, devicetree,
bcm-kernel-feedback-list, Rafał Miłecki
Hi "Rafał,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linusw-pinctrl/devel]
[also build test WARNING on robh/for-next linus/master v5.16-rc5 next-20211215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Rafa-Mi-ecki/dt-bindings-pinctrl-Add-binding-for-BCM4908-pinctrl/20211216-044855
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: alpha-randconfig-s031-20211216 (https://download.01.org/0day-ci/archive/20211216/202112162229.R65wpBRJ-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/52ad0e1851a5d242cde2829eca853a7369807c42
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rafa-Mi-ecki/dt-bindings-pinctrl-Add-binding-for-BCM4908-pinctrl/20211216-044855
git checkout 52ad0e1851a5d242cde2829eca853a7369807c42
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=alpha SHELL=/bin/bash drivers/perf/ drivers/pinctrl/bcm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/pinctrl/bcm/pinctrl-bcm4908.c:542:53: sparse: sparse: incorrect type in argument 3 (different modifiers) @@ expected char const **groups @@ got char const *const *const groups @@
drivers/pinctrl/bcm/pinctrl-bcm4908.c:542:53: sparse: expected char const **groups
drivers/pinctrl/bcm/pinctrl-bcm4908.c:542:53: sparse: got char const *const *const groups
vim +542 drivers/pinctrl/bcm/pinctrl-bcm4908.c
468
469 static int bcm4908_pinctrl_probe(struct platform_device *pdev)
470 {
471 struct device *dev = &pdev->dev;
472 struct bcm4908_pinctrl *bcm4908_pinctrl;
473 struct pinctrl_desc *pctldesc;
474 struct pinctrl_pin_desc *pins;
475 int i;
476
477 bcm4908_pinctrl = devm_kzalloc(dev, sizeof(*bcm4908_pinctrl), GFP_KERNEL);
478 if (!bcm4908_pinctrl)
479 return -ENOMEM;
480 pctldesc = &bcm4908_pinctrl->pctldesc;
481 platform_set_drvdata(pdev, bcm4908_pinctrl);
482
483 /* Set basic properties */
484
485 bcm4908_pinctrl->dev = dev;
486
487 bcm4908_pinctrl->base = devm_platform_ioremap_resource(pdev, 0);
488 if (IS_ERR(bcm4908_pinctrl->base)) {
489 dev_err(dev, "Failed to map pinctrl regs\n");
490 return PTR_ERR(bcm4908_pinctrl->base);
491 }
492
493 memcpy(pctldesc, &bcm4908_pinctrl_desc, sizeof(*pctldesc));
494
495 /* Set pinctrl properties */
496
497 pins = devm_kcalloc(dev, BCM4908_NUM_PINS,
498 sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
499 if (!pins)
500 return -ENOMEM;
501 for (i = 0; i < BCM4908_NUM_PINS; i++) {
502 pins[i].number = i;
503 pins[i].name = devm_kasprintf(dev, GFP_KERNEL, "pin%d", i);
504 if (!pins[i].name)
505 return -ENOMEM;
506 }
507 pctldesc->pins = pins;
508 pctldesc->npins = BCM4908_NUM_PINS;
509
510 /* Register */
511
512 bcm4908_pinctrl->pctldev = devm_pinctrl_register(dev, pctldesc, bcm4908_pinctrl);
513 if (IS_ERR(bcm4908_pinctrl->pctldev)) {
514 dev_err(dev, "Failed to register pinctrl\n");
515 return PTR_ERR(bcm4908_pinctrl->pctldev);
516 }
517
518 /* Groups */
519
520 for (i = 0; i < ARRAY_SIZE(bcm4908_pinctrl_grps); i++) {
521 const struct bcm4908_pinctrl_grp *group = &bcm4908_pinctrl_grps[i];
522 int *pins;
523 int j;
524
525 pins = devm_kcalloc(dev, group->num_pins, sizeof(*pins), GFP_KERNEL);
526 if (!pins)
527 return -ENOMEM;
528 for (j = 0; j < group->num_pins; j++)
529 pins[j] = group->pins[j].number;
530
531 pinctrl_generic_add_group(bcm4908_pinctrl->pctldev, group->name,
532 pins, group->num_pins, (void *)group);
533 }
534
535 /* Functions */
536
537 for (i = 0; i < ARRAY_SIZE(bcm4908_pinctrl_functions); i++) {
538 const struct bcm4908_pinctrl_function *function = &bcm4908_pinctrl_functions[i];
539
540 pinmux_generic_add_function(bcm4908_pinctrl->pctldev,
541 function->name,
> 542 function->groups,
543 function->num_groups, NULL);
544 }
545
546 return 0;
547 }
548
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux
@ 2021-12-16 14:26 ` kernel test robot
0 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2021-12-16 14:26 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5156 bytes --]
Hi "Rafał,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linusw-pinctrl/devel]
[also build test WARNING on robh/for-next linus/master v5.16-rc5 next-20211215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Rafa-Mi-ecki/dt-bindings-pinctrl-Add-binding-for-BCM4908-pinctrl/20211216-044855
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: alpha-randconfig-s031-20211216 (https://download.01.org/0day-ci/archive/20211216/202112162229.R65wpBRJ-lkp(a)intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/52ad0e1851a5d242cde2829eca853a7369807c42
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rafa-Mi-ecki/dt-bindings-pinctrl-Add-binding-for-BCM4908-pinctrl/20211216-044855
git checkout 52ad0e1851a5d242cde2829eca853a7369807c42
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=alpha SHELL=/bin/bash drivers/perf/ drivers/pinctrl/bcm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/pinctrl/bcm/pinctrl-bcm4908.c:542:53: sparse: sparse: incorrect type in argument 3 (different modifiers) @@ expected char const **groups @@ got char const *const *const groups @@
drivers/pinctrl/bcm/pinctrl-bcm4908.c:542:53: sparse: expected char const **groups
drivers/pinctrl/bcm/pinctrl-bcm4908.c:542:53: sparse: got char const *const *const groups
vim +542 drivers/pinctrl/bcm/pinctrl-bcm4908.c
468
469 static int bcm4908_pinctrl_probe(struct platform_device *pdev)
470 {
471 struct device *dev = &pdev->dev;
472 struct bcm4908_pinctrl *bcm4908_pinctrl;
473 struct pinctrl_desc *pctldesc;
474 struct pinctrl_pin_desc *pins;
475 int i;
476
477 bcm4908_pinctrl = devm_kzalloc(dev, sizeof(*bcm4908_pinctrl), GFP_KERNEL);
478 if (!bcm4908_pinctrl)
479 return -ENOMEM;
480 pctldesc = &bcm4908_pinctrl->pctldesc;
481 platform_set_drvdata(pdev, bcm4908_pinctrl);
482
483 /* Set basic properties */
484
485 bcm4908_pinctrl->dev = dev;
486
487 bcm4908_pinctrl->base = devm_platform_ioremap_resource(pdev, 0);
488 if (IS_ERR(bcm4908_pinctrl->base)) {
489 dev_err(dev, "Failed to map pinctrl regs\n");
490 return PTR_ERR(bcm4908_pinctrl->base);
491 }
492
493 memcpy(pctldesc, &bcm4908_pinctrl_desc, sizeof(*pctldesc));
494
495 /* Set pinctrl properties */
496
497 pins = devm_kcalloc(dev, BCM4908_NUM_PINS,
498 sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
499 if (!pins)
500 return -ENOMEM;
501 for (i = 0; i < BCM4908_NUM_PINS; i++) {
502 pins[i].number = i;
503 pins[i].name = devm_kasprintf(dev, GFP_KERNEL, "pin%d", i);
504 if (!pins[i].name)
505 return -ENOMEM;
506 }
507 pctldesc->pins = pins;
508 pctldesc->npins = BCM4908_NUM_PINS;
509
510 /* Register */
511
512 bcm4908_pinctrl->pctldev = devm_pinctrl_register(dev, pctldesc, bcm4908_pinctrl);
513 if (IS_ERR(bcm4908_pinctrl->pctldev)) {
514 dev_err(dev, "Failed to register pinctrl\n");
515 return PTR_ERR(bcm4908_pinctrl->pctldev);
516 }
517
518 /* Groups */
519
520 for (i = 0; i < ARRAY_SIZE(bcm4908_pinctrl_grps); i++) {
521 const struct bcm4908_pinctrl_grp *group = &bcm4908_pinctrl_grps[i];
522 int *pins;
523 int j;
524
525 pins = devm_kcalloc(dev, group->num_pins, sizeof(*pins), GFP_KERNEL);
526 if (!pins)
527 return -ENOMEM;
528 for (j = 0; j < group->num_pins; j++)
529 pins[j] = group->pins[j].number;
530
531 pinctrl_generic_add_group(bcm4908_pinctrl->pctldev, group->name,
532 pins, group->num_pins, (void *)group);
533 }
534
535 /* Functions */
536
537 for (i = 0; i < ARRAY_SIZE(bcm4908_pinctrl_functions); i++) {
538 const struct bcm4908_pinctrl_function *function = &bcm4908_pinctrl_functions[i];
539
540 pinmux_generic_add_function(bcm4908_pinctrl->pctldev,
541 function->name,
> 542 function->groups,
543 function->num_groups, NULL);
544 }
545
546 return 0;
547 }
548
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl
2021-12-15 22:35 ` Rafał Miłecki
@ 2021-12-16 15:32 ` Rob Herring
0 siblings, 0 replies; 13+ messages in thread
From: Rob Herring @ 2021-12-16 15:32 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Linus Walleij, Rafał Miłecki, Jonas Gorski,
Randy Dunlap, Álvaro Fernández Rojas,
bcm-kernel-feedback-list, devicetree, linux-gpio,
Florian Fainelli
On Wed, Dec 15, 2021 at 11:35:01PM +0100, Rafał Miłecki wrote:
> Hi Rob,
>
> On 15.12.2021 23:27, Rob Herring wrote:
> > On Wed, 15 Dec 2021 21:47:52 +0100, Rafał Miłecki wrote:
> > > From: Rafał Miłecki <rafal@milecki.pl>
> > >
> > > It's hardware block that is part of every SoC from BCM4908 family.
> > >
> > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > > ---
> > > .../pinctrl/brcm,bcm4908-pinctrl.yaml | 72 +++++++++++++++++++
> > > MAINTAINERS | 7 ++
> > > 2 files changed, 79 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.yaml
> > >
> >
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/pinctrl/pinctrl.yaml'
> > xargs: dt-doc-validate: exited with status 255; aborting
> > make[1]: *** Deleting file 'Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.example.dt.yaml'
> > Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/pinctrl/pinctrl.yaml'
> > make[1]: *** [scripts/Makefile.lib:373: Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.example.dt.yaml] Error 255
> > make[1]: *** Waiting for unfinished jobs....
> > make: *** [Makefile:1413: dt_binding_check] Error 2
>
> this patch targets Linus's git tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=for-next
>
> Above tree contains 896568e5b9c8 ("dt-bindings: pinctrl: convert
> controller description to the json-schema") which provides pinctrl.yaml.
Please note that in the patch if you don't want to get this message.
Otherwise, this is the alert that there's some dependency.
Rob
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl
2021-12-15 20:47 [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl Rafał Miłecki
2021-12-15 20:47 ` [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux Rafał Miłecki
2021-12-15 22:27 ` [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl Rob Herring
@ 2021-12-16 15:35 ` Rob Herring
2 siblings, 0 replies; 13+ messages in thread
From: Rob Herring @ 2021-12-16 15:35 UTC (permalink / raw)
To: Rafał Miłecki
Cc: bcm-kernel-feedback-list, Florian Fainelli,
Álvaro Fernández Rojas, linux-gpio, Rob Herring,
devicetree, Linus Walleij, Rafał Miłecki, Randy Dunlap,
Jonas Gorski
On Wed, 15 Dec 2021 21:47:52 +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> It's hardware block that is part of every SoC from BCM4908 family.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> .../pinctrl/brcm,bcm4908-pinctrl.yaml | 72 +++++++++++++++++++
> MAINTAINERS | 7 ++
> 2 files changed, 79 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm4908-pinctrl.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux
2021-12-15 20:47 ` [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux Rafał Miłecki
2021-12-16 6:34 ` kernel test robot
2021-12-16 14:26 ` kernel test robot
@ 2021-12-16 19:55 ` Andy Shevchenko
2021-12-22 10:19 ` Rafał Miłecki
2 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2021-12-16 19:55 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rob Herring, Linus Walleij, Álvaro Fernández Rojas,
Jonas Gorski, Randy Dunlap, Florian Fainelli, linux-gpio,
devicetree, bcm-kernel-feedback-list, Rafał Miłecki
On Thu, Dec 16, 2021 at 1:25 AM Rafał Miłecki <zajec5@gmail.com> wrote:
>
> From: Rafał Miłecki <rafal@milecki.pl>
>
> BCM4908 has its own pins layout so it needs a custom binding and a Linux
> driver.
...
> +config PINCTRL_BCM4908
> + bool "Broadcom BCM4908 pinmux driver"
Why not tristate?
> + depends on OF && (ARCH_BCM4908 || COMPILE_TEST)
Is there really dependency to OF?
> + select PINMUX
> + select PINCONF
> + select GENERIC_PINCONF
> + select GENERIC_PINCTRL_GROUPS
> + select GENERIC_PINMUX_FUNCTIONS
> + default ARCH_BCM4908
> + help
> + Say Y here to enable driver for BCM4908 family SoCs with integrated
> + pin controller.
With a module available it's good to mention its name here.
...
> +/*
> + * Copyright (C) 2021 Rafał Miłecki <rafal@milecki.pl>
> + */
One line?
...
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
No evidence of use of these headers.
But missed mod_devicetable.h.
> +#include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
Can you move this group...
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
...here?
> +#include "../core.h"
> +#include "../pinmux.h"
...
> +#define TEST_PORT_BLOCK_EN_LSB 0x00
> +#define TEST_PORT_BLOCK_DATA_MSB 0x04
> +#define TEST_PORT_BLOCK_DATA_LSB 0x08
> +#define TEST_PORT_LSB_PINMUX_DATA_SHIFT 12
> +#define TEST_PORT_COMMAND 0x0c
> +#define TEST_PORT_CMD_LOAD_MUX_REG 0x00000021
The prefix of all above doesn't match the module name.
...
> +struct bcm4908_pinctrl_grp {
> + const char *name;
> + const struct bcm4908_pinctrl_pin_setup *pins;
> + const unsigned int num_pins;
> +};
Why not to (re)use struct group_desc?
...
> + for (i = 0; i < group->num_pins; i++) {
> + u32 lsb = 0;
> +
> + lsb |= group->pins[i].number;
> + lsb |= group->pins[i].function << TEST_PORT_LSB_PINMUX_DATA_SHIFT;
> +
> + writel(0x0, bcm4908_pinctrl->base + TEST_PORT_BLOCK_DATA_MSB);
> + writel(lsb, bcm4908_pinctrl->base + TEST_PORT_BLOCK_DATA_LSB);
> + writel(TEST_PORT_CMD_LOAD_MUX_REG, bcm4908_pinctrl->base + TEST_PORT_COMMAND);
> + }
No serialization for IO, is it okay?
...
> + bcm4908_pinctrl->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(bcm4908_pinctrl->base)) {
> + dev_err(dev, "Failed to map pinctrl regs\n");
> + return PTR_ERR(bcm4908_pinctrl->base);
Besides of dev_err_probe(), why you duplicate message that already
printed by core?
> + }
...
> + /* Set pinctrl properties */
> +
Here and everywhere else, please drop redundant blank line.
...
> + pins = devm_kcalloc(dev, BCM4908_NUM_PINS,
> + sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
Please, use sizeof(*foo) form. Then put it on one line.
> + if (!pins)
> + return -ENOMEM;
...
> + for (i = 0; i < BCM4908_NUM_PINS; i++) {
> + pins[i].number = i;
> + pins[i].name = devm_kasprintf(dev, GFP_KERNEL, "pin%d", i);
> + if (!pins[i].name)
> + return -ENOMEM;
> + }
Can you use the kasprintf_strarray() to make the style unified, please?
...
> + bcm4908_pinctrl->pctldev = devm_pinctrl_register(dev, pctldesc, bcm4908_pinctrl);
> + if (IS_ERR(bcm4908_pinctrl->pctldev)) {
> + dev_err(dev, "Failed to register pinctrl\n");
> + return PTR_ERR(bcm4908_pinctrl->pctldev);
return dev_err_probe(...);
> + }
...
> +static struct platform_driver bcm4908_pinctrl_driver = {
> + .probe = bcm4908_pinctrl_probe,
> + .driver = {
> + .name = "bcm4908-pinctrl",
> + .of_match_table = bcm4908_pinctrl_of_match_table,
> + },
> +};
> +
No need.
> +module_platform_driver(bcm4908_pinctrl_driver);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux
2021-12-16 19:55 ` Andy Shevchenko
@ 2021-12-22 10:19 ` Rafał Miłecki
2021-12-22 12:12 ` Andy Shevchenko
0 siblings, 1 reply; 13+ messages in thread
From: Rafał Miłecki @ 2021-12-22 10:19 UTC (permalink / raw)
To: Andy Shevchenko, Rafał Miłecki
Cc: Rob Herring, Linus Walleij, Álvaro Fernández Rojas,
Jonas Gorski, Randy Dunlap, Florian Fainelli, linux-gpio,
devicetree, bcm-kernel-feedback-list
On 16.12.2021 20:55, Andy Shevchenko wrote:
>> +/*
>> + * Copyright (C) 2021 Rafał Miłecki <rafal@milecki.pl>
>> + */
>
> One line?
I don't think there's a rule for that. Not in coding-style.rst as much
as I'm aware of. checkpatch.pl also doesn't complain.
>> +#include <linux/pinctrl/pinconf-generic.h>
>> +#include <linux/pinctrl/pinctrl.h>
>> +#include <linux/pinctrl/pinmux.h>
>
> Can you move this group...
>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>
> ...here?
Any reason for that? For most of the time I keep my includes sorted
alphabetically. Now I checked coding-style.rst is actually seems to
recomment "clang-format" for the same reason: sorting includes.
>> +#define TEST_PORT_BLOCK_EN_LSB 0x00
>> +#define TEST_PORT_BLOCK_DATA_MSB 0x04
>> +#define TEST_PORT_BLOCK_DATA_LSB 0x08
>> +#define TEST_PORT_LSB_PINMUX_DATA_SHIFT 12
>> +#define TEST_PORT_COMMAND 0x0c
>> +#define TEST_PORT_CMD_LOAD_MUX_REG 0x00000021
>
> The prefix of all above doesn't match the module name.
Those are register names as in Broadcom's documentation. I don't think
those names can conflict with any included header defines but I can
change it.
>> +struct bcm4908_pinctrl_grp {
>> + const char *name;
>> + const struct bcm4908_pinctrl_pin_setup *pins;
>> + const unsigned int num_pins;
>> +};
>
> Why not to (re)use struct group_desc?
It's because "struct group_desc" has @pins that I can't fill statically
as I need struct instead of int.
I also need struct field for "const struct bcm4908_pinctrl_pin_setup"
and "void *data" doesn't fit that purpsose 100% because:
1. It's a void
2. It's not static
>> + /* Set pinctrl properties */
>> +
>
> Here and everywhere else, please drop redundant blank line.
No clear kernel rule for that.
I use blank line to indicate / suggest that comment applies to more than
just a single line that follows.
>> +static struct platform_driver bcm4908_pinctrl_driver = {
>> + .probe = bcm4908_pinctrl_probe,
>> + .driver = {
>> + .name = "bcm4908-pinctrl",
>> + .of_match_table = bcm4908_pinctrl_of_match_table,
>> + },
>> +};
>
>> +
>
> No need.
>
>> +module_platform_driver(bcm4908_pinctrl_driver);
You have 1344 other source files with empty line above
module_platform_driver(). coding-style.rst says to "separate functions
with one blank line". Are we supposed to argue now whether a macro can
be considered a functio nor not?
grep -B 1 -r "module_platform_driver" drivers/* | egrep -c "\.c-$"
1344
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux
2021-12-22 10:19 ` Rafał Miłecki
@ 2021-12-22 12:12 ` Andy Shevchenko
0 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2021-12-22 12:12 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rafał Miłecki, Rob Herring, Linus Walleij,
Álvaro Fernández Rojas, Jonas Gorski, Randy Dunlap,
Florian Fainelli, open list:GPIO SUBSYSTEM, devicetree,
bcm-kernel-feedback-list
On Wed, Dec 22, 2021 at 12:19 PM Rafał Miłecki <rafal@milecki.pl> wrote:
> On 16.12.2021 20:55, Andy Shevchenko wrote:
> >> +/*
> >> + * Copyright (C) 2021 Rafał Miłecki <rafal@milecki.pl>
> >> + */
> >
> > One line?
>
> I don't think there's a rule for that. Not in coding-style.rst as much
> as I'm aware of. checkpatch.pl also doesn't complain.
There is no rule, but common sense. Why occupy 3 LOCs instead of 1 LOC?
...
> >> +#include <linux/pinctrl/pinconf-generic.h>
> >> +#include <linux/pinctrl/pinctrl.h>
> >> +#include <linux/pinctrl/pinmux.h>
> >
> > Can you move this group...
> >
> >> +#include <linux/platform_device.h>
> >> +#include <linux/slab.h>
> >
> > ...here?
>
> Any reason for that? For most of the time I keep my includes sorted
> alphabetically. Now I checked coding-style.rst is actually seems to
> recomment "clang-format" for the same reason: sorting includes.
Yes. The reason is simple. With moving this group separately you
follow the rule of going from most generic to most particular headers
in the block. Grouping like this will show better that this code has
tighten relations with the pin control subsystem.
...
> >> +#define TEST_PORT_BLOCK_EN_LSB 0x00
> >> +#define TEST_PORT_BLOCK_DATA_MSB 0x04
> >> +#define TEST_PORT_BLOCK_DATA_LSB 0x08
> >> +#define TEST_PORT_LSB_PINMUX_DATA_SHIFT 12
> >> +#define TEST_PORT_COMMAND 0x0c
> >> +#define TEST_PORT_CMD_LOAD_MUX_REG 0x00000021
> >
> > The prefix of all above doesn't match the module name.
>
> Those are register names as in Broadcom's documentation. I don't think
> those names can conflict with any included header defines but I can
> change it.
They may easily conflict through headers with something more generic
not related to your driver or even GPIO. The TEST_PORT_COMMAND seems
one of this kind that might potentially collide.
...
> >> +
> >
> > Here and everywhere else, please drop redundant blank line.
>
> No clear kernel rule for that.
>
> I use blank line to indicate / suggest that comment applies to more than
> just a single line that follows.
Maybe these comments are not so useful after all?
...
> >> +
> >
> > No need.
> >
> >> +module_platform_driver(bcm4908_pinctrl_driver);
>
> You have 1344 other source files with empty line above
> module_platform_driver(). coding-style.rst says to "separate functions
> with one blank line". Are we supposed to argue now whether a macro can
> be considered a functio nor not?
>
> grep -B 1 -r "module_platform_driver" drivers/* | egrep -c "\.c-$"
> 1344
Same as above, common sense and the tight relationship between two.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-12-22 15:06 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-15 20:47 [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl Rafał Miłecki
2021-12-15 20:47 ` [PATCH 2/2] pinctrl: bcm: add driver for BCM4908 pinmux Rafał Miłecki
2021-12-16 6:34 ` kernel test robot
2021-12-16 6:34 ` kernel test robot
2021-12-16 14:26 ` kernel test robot
2021-12-16 14:26 ` kernel test robot
2021-12-16 19:55 ` Andy Shevchenko
2021-12-22 10:19 ` Rafał Miłecki
2021-12-22 12:12 ` Andy Shevchenko
2021-12-15 22:27 ` [PATCH 1/2] dt-bindings: pinctrl: Add binding for BCM4908 pinctrl Rob Herring
2021-12-15 22:35 ` Rafał Miłecki
2021-12-16 15:32 ` Rob Herring
2021-12-16 15:35 ` Rob Herring
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.