2022-11-09 06:57:07

by Yinbo Zhu

[permalink] [raw]
Subject: [PATCH v8 1/2] pinctrl: pinctrl-loongson2: add pinctrl driver support

The Loongson-2 SoC has a few pins that can be used as GPIOs or take
multiple other functions. Add a driver for the pinmuxing.

There is currently no support for GPIO pin pull-up and pull-down.

Signed-off-by: zhanghongchen <[email protected]>
Signed-off-by: Yinbo Zhu <[email protected]>
---
Change in v8:
1. Add #include <linux/pinctrl/pinctrl.h>.
2. Add #include <linux/seq_file.h>.
Change in v7:
1. Add all history change log information.
Change in v6:
1. Add #include <asm-generic/io.h>.
Change in v5:
1. NO change, but other patch in this series of patches set has
change.
Change in v4:
1. Replace Loongson2/loongson2 with Loongson-2/loongson-2/LOONGSON-2
but except c file.
2. Add a helper combining two calls and that helper is
"devm_platform_ioremap_resource".
Change in v3:
1. NO change, but other patch in this series of patches set has
change.
Change in v2:
1. Add "depends LOONGARCH || COMPILE_TEST" in Kconfig.
2. Fixup the odd indentation in Kconfig.
3. Make coma goes to previous line. in Kconfig.
4. Add items in alphabetical order in Makefile.
5. Add const type for loongson2_pmx_functions.
6. Add static for loongson2_pmx_ops.
7. Replace sizeof(struct loongson2_pinctrl) with sizeof(*pctrl).
8. Use dev_err_probe as helper to replace "dev_err" and
"return PTR_ERR()".
9. Replace raw spinlock with ordinary spinlock.

MAINTAINERS | 7 +
drivers/pinctrl/Kconfig | 11 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-loongson2.c | 331 ++++++++++++++++++++++++++++
4 files changed, 350 insertions(+)
create mode 100644 drivers/pinctrl/pinctrl-loongson2.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6234b5c57a9a..7c71cf2a6ddd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12036,6 +12036,13 @@ S: Maintained
F: Documentation/devicetree/bindings/timer/loongson,ls2k-hpet.yaml
F: drivers/clocksource/loongson2_hpet.c

+LOONGSON-2 SOC SERIES PINCTRL DRIVER
+M: zhanghongchen <[email protected]>
+M: Yinbo Zhu <[email protected]>
+L: [email protected]
+S: Maintained
+F: drivers/pinctrl/pinctrl-loongson2.c
+
LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI)
M: Sathya Prakash <[email protected]>
M: Sreekanth Reddy <[email protected]>
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index f71fefff400f..a053952e2482 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -512,6 +512,17 @@ config PINCTRL_ZYNQMP
This driver can also be built as a module. If so, the module
will be called pinctrl-zynqmp.

+config PINCTRL_LOONGSON2
+ tristate "Pinctrl driver for the Loongson-2 SoC"
+ depends on LOONGARCH || COMPILE_TEST
+ select PINMUX
+ select GENERIC_PINCONF
+ help
+ This selects pin control driver for the Loongson-2 SoC. It
+ provides pin config functions multiplexing. GPIO pin pull-up,
+ pull-down functions are not supported. Say yes to enable
+ pinctrl for Loongson-2 SoC.
+
source "drivers/pinctrl/actions/Kconfig"
source "drivers/pinctrl/aspeed/Kconfig"
source "drivers/pinctrl/bcm/Kconfig"
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 89bfa01b5231..37b1f9bb85e5 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_PINCTRL_KEEMBAY) += pinctrl-keembay.o
obj-$(CONFIG_PINCTRL_LANTIQ) += pinctrl-lantiq.o
obj-$(CONFIG_PINCTRL_FALCON) += pinctrl-falcon.o
obj-$(CONFIG_PINCTRL_XWAY) += pinctrl-xway.o
+obj-$(CONFIG_PINCTRL_LOONGSON2) += pinctrl-loongson2.o
obj-$(CONFIG_PINCTRL_LPC18XX) += pinctrl-lpc18xx.o
obj-$(CONFIG_PINCTRL_MAX77620) += pinctrl-max77620.o
obj-$(CONFIG_PINCTRL_MCP23S08_I2C) += pinctrl-mcp23s08_i2c.o
diff --git a/drivers/pinctrl/pinctrl-loongson2.c b/drivers/pinctrl/pinctrl-loongson2.c
new file mode 100644
index 000000000000..8d2813cb60cf
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-loongson2.c
@@ -0,0 +1,331 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Author: zhanghongchen <[email protected]>
+ * Yinbo Zhu <[email protected]>
+ * Copyright (C) 2022-2023 Loongson Technology Corporation Limited
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/seq_file.h>
+#include <asm-generic/io.h>
+#include "core.h"
+#include "pinctrl-utils.h"
+
+#define PMX_GROUP(grp, offset, bitv) \
+ { \
+ .name = #grp, \
+ .pins = grp ## _pins, \
+ .num_pins = ARRAY_SIZE(grp ## _pins), \
+ .reg = offset, \
+ .bit = bitv, \
+ }
+
+#define SPECIFIC_GROUP(group) \
+ static const char * const group##_groups[] = { \
+ #group \
+ }
+
+#define FUNCTION(fn) \
+ { \
+ .name = #fn, \
+ .groups = fn ## _groups, \
+ .num_groups = ARRAY_SIZE(fn ## _groups), \
+ }
+
+struct loongson2_pinctrl {
+ struct device *dev;
+ struct pinctrl_dev *pcdev;
+ struct pinctrl_desc desc;
+ struct device_node *of_node;
+ spinlock_t lock;
+ void * __iomem reg_base;
+};
+
+struct loongson2_pmx_group {
+ const char *name;
+ const unsigned int *pins;
+ unsigned int num_pins;
+ unsigned int reg;
+ unsigned int bit;
+};
+
+struct loongson2_pmx_func {
+ const char *name;
+ const char * const *groups;
+ unsigned int num_groups;
+};
+
+#define LOONGSON2_PIN(x) PINCTRL_PIN(x, "gpio"#x)
+static const struct pinctrl_pin_desc loongson2_pctrl_pins[] = {
+ LOONGSON2_PIN(0), LOONGSON2_PIN(1), LOONGSON2_PIN(2), LOONGSON2_PIN(3),
+ LOONGSON2_PIN(4), LOONGSON2_PIN(5), LOONGSON2_PIN(6), LOONGSON2_PIN(7),
+ LOONGSON2_PIN(8), LOONGSON2_PIN(9), LOONGSON2_PIN(10), LOONGSON2_PIN(11),
+ LOONGSON2_PIN(12), LOONGSON2_PIN(13), LOONGSON2_PIN(14),
+ LOONGSON2_PIN(16), LOONGSON2_PIN(17), LOONGSON2_PIN(18), LOONGSON2_PIN(19),
+ LOONGSON2_PIN(20), LOONGSON2_PIN(21), LOONGSON2_PIN(22), LOONGSON2_PIN(23),
+ LOONGSON2_PIN(24), LOONGSON2_PIN(25), LOONGSON2_PIN(26), LOONGSON2_PIN(27),
+ LOONGSON2_PIN(28), LOONGSON2_PIN(29), LOONGSON2_PIN(30),
+ LOONGSON2_PIN(32), LOONGSON2_PIN(33), LOONGSON2_PIN(34), LOONGSON2_PIN(35),
+ LOONGSON2_PIN(36), LOONGSON2_PIN(37), LOONGSON2_PIN(38), LOONGSON2_PIN(39),
+ LOONGSON2_PIN(40), LOONGSON2_PIN(41),
+ LOONGSON2_PIN(44), LOONGSON2_PIN(45), LOONGSON2_PIN(46), LOONGSON2_PIN(47),
+ LOONGSON2_PIN(48), LOONGSON2_PIN(49), LOONGSON2_PIN(50), LOONGSON2_PIN(51),
+ LOONGSON2_PIN(52), LOONGSON2_PIN(53), LOONGSON2_PIN(54), LOONGSON2_PIN(55),
+ LOONGSON2_PIN(56), LOONGSON2_PIN(57), LOONGSON2_PIN(58), LOONGSON2_PIN(59),
+ LOONGSON2_PIN(60), LOONGSON2_PIN(61), LOONGSON2_PIN(62), LOONGSON2_PIN(63),
+};
+
+static const unsigned int gpio_pins[] = {0, 1, 2, 3, 4, 5, 6, 7,
+ 8, 9, 10, 11, 12, 13, 14,
+ 16, 17, 18, 19, 20, 21, 22, 23,
+ 24, 25, 26, 27, 28, 29, 30,
+ 32, 33, 34, 35, 36, 37, 38, 39,
+ 40, 43, 44, 45, 46, 47,
+ 48, 49, 50, 51, 52, 53, 46, 55,
+ 56, 57, 58, 59, 60, 61, 62, 63};
+static const unsigned int sdio_pins[] = {36, 37, 38, 39, 40, 41};
+static const unsigned int can1_pins[] = {34, 35};
+static const unsigned int can0_pins[] = {32, 33};
+static const unsigned int pwm3_pins[] = {23};
+static const unsigned int pwm2_pins[] = {22};
+static const unsigned int pwm1_pins[] = {21};
+static const unsigned int pwm0_pins[] = {20};
+static const unsigned int i2c1_pins[] = {18, 19};
+static const unsigned int i2c0_pins[] = {16, 17};
+static const unsigned int nand_pins[] = {44, 45, 46, 47, 48, 49, 50, 51,
+ 52, 53, 54, 55, 56, 57, 58, 59, 60,
+ 61, 62, 63};
+static const unsigned int sata_led_pins[] = {14};
+static const unsigned int lio_pins[] = {};
+static const unsigned int i2s_pins[] = {24, 25, 26, 27, 28};
+static const unsigned int hda_pins[] = {24, 25, 26, 27, 28, 29, 30};
+static const unsigned int uart2_pins[] = {};
+static const unsigned int uart1_pins[] = {};
+static const unsigned int camera_pins[] = {};
+static const unsigned int dvo1_pins[] = {};
+static const unsigned int dvo0_pins[] = {};
+
+static struct loongson2_pmx_group loongson2_pmx_groups[] = {
+ PMX_GROUP(gpio, 0x0, 64),
+ PMX_GROUP(sdio, 0x0, 20),
+ PMX_GROUP(can1, 0x0, 17),
+ PMX_GROUP(can0, 0x0, 16),
+ PMX_GROUP(pwm3, 0x0, 15),
+ PMX_GROUP(pwm2, 0x0, 14),
+ PMX_GROUP(pwm1, 0x0, 13),
+ PMX_GROUP(pwm0, 0x0, 12),
+ PMX_GROUP(i2c1, 0x0, 11),
+ PMX_GROUP(i2c0, 0x0, 10),
+ PMX_GROUP(nand, 0x0, 9),
+ PMX_GROUP(sata_led, 0x0, 8),
+ PMX_GROUP(lio, 0x0, 7),
+ PMX_GROUP(i2s, 0x0, 6),
+ PMX_GROUP(hda, 0x0, 4),
+ PMX_GROUP(uart2, 0x8, 13),
+ PMX_GROUP(uart1, 0x8, 12),
+ PMX_GROUP(camera, 0x10, 5),
+ PMX_GROUP(dvo1, 0x10, 4),
+ PMX_GROUP(dvo0, 0x10, 1),
+
+};
+
+SPECIFIC_GROUP(sdio);
+SPECIFIC_GROUP(can1);
+SPECIFIC_GROUP(can0);
+SPECIFIC_GROUP(pwm3);
+SPECIFIC_GROUP(pwm2);
+SPECIFIC_GROUP(pwm1);
+SPECIFIC_GROUP(pwm0);
+SPECIFIC_GROUP(i2c1);
+SPECIFIC_GROUP(i2c0);
+SPECIFIC_GROUP(nand);
+SPECIFIC_GROUP(sata_led);
+SPECIFIC_GROUP(lio);
+SPECIFIC_GROUP(i2s);
+SPECIFIC_GROUP(hda);
+SPECIFIC_GROUP(uart2);
+SPECIFIC_GROUP(uart1);
+SPECIFIC_GROUP(camera);
+SPECIFIC_GROUP(dvo1);
+SPECIFIC_GROUP(dvo0);
+
+static const char * const gpio_groups[] = {
+ "sdio", "can1", "can0", "pwm3", "pwm2", "pwm1", "pwm0", "i2c1",
+ "i2c0", "nand", "sata_led", "lio", "i2s", "hda", "uart2", "uart1",
+ "camera", "dvo1", "dvo0"
+};
+
+static const struct loongson2_pmx_func loongson2_pmx_functions[] = {
+ FUNCTION(gpio),
+ FUNCTION(sdio),
+ FUNCTION(can1),
+ FUNCTION(can0),
+ FUNCTION(pwm3),
+ FUNCTION(pwm2),
+ FUNCTION(pwm1),
+ FUNCTION(pwm0),
+ FUNCTION(i2c1),
+ FUNCTION(i2c0),
+ FUNCTION(nand),
+ FUNCTION(sata_led),
+ FUNCTION(lio),
+ FUNCTION(i2s),
+ FUNCTION(hda),
+ FUNCTION(uart2),
+ FUNCTION(uart1),
+ FUNCTION(camera),
+ FUNCTION(dvo1),
+ FUNCTION(dvo0),
+};
+
+static int loongson2_get_groups_count(struct pinctrl_dev *pcdev)
+{
+ return ARRAY_SIZE(loongson2_pmx_groups);
+}
+
+static const char *loongson2_get_group_name(struct pinctrl_dev *pcdev,
+ unsigned int selector)
+{
+ return loongson2_pmx_groups[selector].name;
+}
+
+static int loongson2_get_group_pins(struct pinctrl_dev *pcdev, unsigned int selector,
+ const unsigned int **pins, unsigned int *num_pins)
+{
+ *pins = loongson2_pmx_groups[selector].pins;
+ *num_pins = loongson2_pmx_groups[selector].num_pins;
+
+ return 0;
+}
+
+static void loongson2_pin_dbg_show(struct pinctrl_dev *pcdev, struct seq_file *s,
+ unsigned int offset)
+{
+ seq_printf(s, " %s", dev_name(pcdev->dev));
+}
+
+static const struct pinctrl_ops loongson2_pctrl_ops = {
+ .get_groups_count = loongson2_get_groups_count,
+ .get_group_name = loongson2_get_group_name,
+ .get_group_pins = loongson2_get_group_pins,
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_all,
+ .dt_free_map = pinctrl_utils_free_map,
+ .pin_dbg_show = loongson2_pin_dbg_show,
+};
+
+static int loongson2_pmx_set_mux(struct pinctrl_dev *pcdev, unsigned int func_num,
+ unsigned int group_num)
+{
+ struct loongson2_pinctrl *pctrl = pinctrl_dev_get_drvdata(pcdev);
+ unsigned long reg = (unsigned long)pctrl->reg_base +
+ loongson2_pmx_groups[group_num].reg;
+ unsigned int mux_bit = loongson2_pmx_groups[group_num].bit;
+ unsigned int val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&pctrl->lock, flags);
+ val = readl((void *)reg);
+ if (func_num == 0)
+ val &= ~(1<<mux_bit);
+ else
+ val |= (1<<mux_bit);
+ writel(val, (void *)reg);
+ spin_unlock_irqrestore(&pctrl->lock, flags);
+
+ return 0;
+}
+
+static int loongson2_pmx_get_funcs_count(struct pinctrl_dev *pcdev)
+{
+ return ARRAY_SIZE(loongson2_pmx_functions);
+}
+
+static const char *loongson2_pmx_get_func_name(struct pinctrl_dev *pcdev,
+ unsigned int selector)
+{
+ return loongson2_pmx_functions[selector].name;
+}
+
+static int loongson2_pmx_get_groups(struct pinctrl_dev *pcdev,
+ unsigned int selector,
+ const char * const **groups,
+ unsigned int * const num_groups)
+{
+ *groups = loongson2_pmx_functions[selector].groups;
+ *num_groups = loongson2_pmx_functions[selector].num_groups;
+
+ return 0;
+}
+
+static const struct pinmux_ops loongson2_pmx_ops = {
+ .set_mux = loongson2_pmx_set_mux,
+ .get_functions_count = loongson2_pmx_get_funcs_count,
+ .get_function_name = loongson2_pmx_get_func_name,
+ .get_function_groups = loongson2_pmx_get_groups,
+};
+
+static int loongson2_pinctrl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct loongson2_pinctrl *pctrl;
+
+ pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL);
+ if (!pctrl)
+ return -ENOMEM;
+
+ pctrl->reg_base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(pctrl->reg_base))
+ return dev_err_probe(pctrl->dev, PTR_ERR(pctrl->reg_base),
+ "unable to map I/O memory");
+
+ spin_lock_init(&pctrl->lock);
+
+ pctrl->dev = dev;
+ pctrl->desc.name = "pinctrl-loongson2";
+ pctrl->desc.owner = THIS_MODULE;
+ pctrl->desc.pctlops = &loongson2_pctrl_ops;
+ pctrl->desc.pmxops = &loongson2_pmx_ops;
+ pctrl->desc.confops = NULL;
+ pctrl->desc.pins = loongson2_pctrl_pins;
+ pctrl->desc.npins = ARRAY_SIZE(loongson2_pctrl_pins);
+
+ pctrl->pcdev = devm_pinctrl_register(pctrl->dev, &pctrl->desc, pctrl);
+ if (IS_ERR(pctrl->pcdev))
+ return dev_err_probe(pctrl->dev, PTR_ERR(pctrl->pcdev),
+ "can't register pinctrl device");
+
+ return 0;
+}
+
+static const struct of_device_id loongson2_pinctrl_dt_match[] = {
+ {
+ .compatible = "loongson,ls2k-pinctrl",
+ },
+ { },
+};
+
+static struct platform_driver loongson2_pinctrl_driver = {
+ .probe = loongson2_pinctrl_probe,
+ .driver = {
+ .name = "loongson2-pinctrl",
+ .of_match_table = loongson2_pinctrl_dt_match,
+ },
+};
+
+static int __init loongson2_pinctrl_init(void)
+{
+ return platform_driver_register(&loongson2_pinctrl_driver);
+}
+arch_initcall(loongson2_pinctrl_init);
+
+static void __exit loongson2_pinctrl_exit(void)
+{
+ platform_driver_unregister(&loongson2_pinctrl_driver);
+}
+module_exit(loongson2_pinctrl_exit);
--
2.20.1



2022-11-09 06:58:45

by Yinbo Zhu

[permalink] [raw]
Subject: [PATCH v8 2/2] dt-bindings: pinctrl: add loongson-2 pinctrl

Add the Loongson-2 pinctrl binding with DT schema format using
json-schema.

Signed-off-by: Yinbo Zhu <[email protected]>
---
Change in v8:
1. NO change, but other patch in this series patches set has
change.
Change in v7:
1. Add all change log information.
Change in v6:
1. NO change, but other patch in this series patches set has
change.
Change in v5:
1. Drop dependencies.
2. Add spaces after '='.
3. Replace string loongson2 with loongson-2 in title.
Change in v4:
1. Replace Loongson2 with Loongson-2.
Change in v3:
1. Drop the quotes in "pinctrl.yaml#".
2. Remove the items in function node.
3. Add requird node for "group" and "function" in properties.
Change in v2:
1. Add "$ref to pinctrl.yaml".
2. Put required after patternProperties.
3. Add "additionalProperties: false" after '-pins$'
4. Add "unevaluatedProperties: false" after 'pinmux$'
5. Fixup the broken indentation in patternProperties node.
6. Use 4 spaces for example indentation.

.../pinctrl/loongson,ls2k-pinctrl.yaml | 125 ++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 126 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/loongson,ls2k-pinctrl.yaml

diff --git a/Documentation/devicetree/bindings/pinctrl/loongson,ls2k-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/loongson,ls2k-pinctrl.yaml
new file mode 100644
index 000000000000..34683a4856ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/loongson,ls2k-pinctrl.yaml
@@ -0,0 +1,125 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/loongson,ls2k-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Loongson-2 SoC Pinctrl Controller
+
+maintainers:
+ - zhanghongchen <[email protected]>
+ - Yinbo Zhu <[email protected]>
+
+allOf:
+ - $ref: pinctrl.yaml#
+
+properties:
+ compatible:
+ const: loongson,ls2k-pinctrl
+
+ reg:
+ maxItems: 1
+
+patternProperties:
+ '-pins$':
+ type: object
+
+ additionalProperties: false
+
+ patternProperties:
+ 'pinmux$':
+ type: object
+ description: node for pinctrl.
+ $ref: pinmux-node.yaml#
+
+ unevaluatedProperties: false
+
+ properties:
+ groups:
+ description:
+ One or more groups of pins to mux to a certain function
+ items:
+ enum: [gpio, sdio, can1, can0, pwm3, pwm2, pwm1, pwm0, i2c1, i2c0,
+ nand, sata_led, lio, i2s, hda, uart2, uart1, camera, dv01,
+ dvo0]
+ function:
+ description:
+ The function that a group of pins is muxed to
+ enum: [gpio, sdio, can1, can0, pwm3, pwm2, pwm1, pwm0, i2c1, i2c0,
+ nand, sata_led, lio, i2s, hda, uart2, uart1, camera, dv01,
+ dvo0]
+
+ required:
+ - groups
+ - function
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ pctrl: pinctrl@1fe00420 {
+ compatible = "loongson,ls2k-pinctrl";
+ reg = <0x1fe00420 0x18>;
+ sdio_pins_default: sdio-pins {
+ sdio-pinmux {
+ groups = "sdio";
+ function = "sdio";
+ };
+
+ sdio-det-pinmux {
+ groups = "pwm2";
+ function = "gpio";
+ };
+ };
+
+ pwm1_pins_default: pwm1-pins {
+ pinmux {
+ groups = "pwm1";
+ function = "pwm1";
+ };
+ };
+
+ pwm0_pins_default: pwm0-pins {
+ pinmux {
+ groups = "pwm0";
+ function = "pwm0";
+ };
+ };
+
+ i2c1_pins_default: i2c1-pins {
+ pinmux {
+ groups = "i2c1";
+ function = "i2c1";
+ };
+ };
+
+ i2c0_pins_default: i2c0-pins {
+ pinmux {
+ groups = "i2c0";
+ function = "i2c0";
+ };
+ };
+
+ nand_pins_default: nand-pins {
+ pinmux {
+ groups = "nand";
+ function = "nand";
+ };
+ };
+
+ hda_pins_default: hda-pins {
+ grp0-pinmux {
+ groups = "hda";
+ function = "hda";
+ };
+
+ grp1-pinmux {
+ groups = "i2s";
+ function = "gpio";
+ };
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 7c71cf2a6ddd..c4fc50eb260b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12041,6 +12041,7 @@ M: zhanghongchen <[email protected]>
M: Yinbo Zhu <[email protected]>
L: [email protected]
S: Maintained
+F: Documentation/devicetree/bindings/pinctrl/loongson,ls2k-pinctrl.yaml
F: drivers/pinctrl/pinctrl-loongson2.c

LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI)
--
2.20.1


2022-11-09 07:23:26

by Yinbo Zhu

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] pinctrl: pinctrl-loongson2: add pinctrl driver support


Hi Linus Walleij,

I had added some changes in these series patch in v8, please help add my
change and merge it into your tree and sync it to linux-next.

Thanks,
Yinbo.
?? 2022/11/9 ????2:11, Yinbo Zhu д??:
> The Loongson-2 SoC has a few pins that can be used as GPIOs or take
> multiple other functions. Add a driver for the pinmuxing.
>
> There is currently no support for GPIO pin pull-up and pull-down.
>
> Signed-off-by: zhanghongchen <[email protected]>
> Signed-off-by: Yinbo Zhu <[email protected]>
> ---
> Change in v8:
> 1. Add #include <linux/pinctrl/pinctrl.h>.
> 2. Add #include <linux/seq_file.h>.
> Change in v7:
> 1. Add all history change log information.
> Change in v6:
> 1. Add #include <asm-generic/io.h>.
> Change in v5:
> 1. NO change, but other patch in this series of patches set has
> change.
> Change in v4:
> 1. Replace Loongson2/loongson2 with Loongson-2/loongson-2/LOONGSON-2
> but except c file.
> 2. Add a helper combining two calls and that helper is
> "devm_platform_ioremap_resource".
> Change in v3:
> 1. NO change, but other patch in this series of patches set has
> change.
> Change in v2:
> 1. Add "depends LOONGARCH || COMPILE_TEST" in Kconfig.
> 2. Fixup the odd indentation in Kconfig.
> 3. Make coma goes to previous line. in Kconfig.
> 4. Add items in alphabetical order in Makefile.
> 5. Add const type for loongson2_pmx_functions.
> 6. Add static for loongson2_pmx_ops.
> 7. Replace sizeof(struct loongson2_pinctrl) with sizeof(*pctrl).
> 8. Use dev_err_probe as helper to replace "dev_err" and
> "return PTR_ERR()".
> 9. Replace raw spinlock with ordinary spinlock.
>
> MAINTAINERS | 7 +
> drivers/pinctrl/Kconfig | 11 +
> drivers/pinctrl/Makefile | 1 +
> drivers/pinctrl/pinctrl-loongson2.c | 331 ++++++++++++++++++++++++++++
> 4 files changed, 350 insertions(+)
> create mode 100644 drivers/pinctrl/pinctrl-loongson2.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6234b5c57a9a..7c71cf2a6ddd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12036,6 +12036,13 @@ S: Maintained
> F: Documentation/devicetree/bindings/timer/loongson,ls2k-hpet.yaml
> F: drivers/clocksource/loongson2_hpet.c
>
> +LOONGSON-2 SOC SERIES PINCTRL DRIVER
> +M: zhanghongchen <[email protected]>
> +M: Yinbo Zhu <[email protected]>
> +L: [email protected]
> +S: Maintained
> +F: drivers/pinctrl/pinctrl-loongson2.c
> +
> LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI)
> M: Sathya Prakash <[email protected]>
> M: Sreekanth Reddy <[email protected]>
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index f71fefff400f..a053952e2482 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -512,6 +512,17 @@ config PINCTRL_ZYNQMP
> This driver can also be built as a module. If so, the module
> will be called pinctrl-zynqmp.
>
> +config PINCTRL_LOONGSON2
> + tristate "Pinctrl driver for the Loongson-2 SoC"
> + depends on LOONGARCH || COMPILE_TEST
> + select PINMUX
> + select GENERIC_PINCONF
> + help
> + This selects pin control driver for the Loongson-2 SoC. It
> + provides pin config functions multiplexing. GPIO pin pull-up,
> + pull-down functions are not supported. Say yes to enable
> + pinctrl for Loongson-2 SoC.
> +
> source "drivers/pinctrl/actions/Kconfig"
> source "drivers/pinctrl/aspeed/Kconfig"
> source "drivers/pinctrl/bcm/Kconfig"
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index 89bfa01b5231..37b1f9bb85e5 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_PINCTRL_KEEMBAY) += pinctrl-keembay.o
> obj-$(CONFIG_PINCTRL_LANTIQ) += pinctrl-lantiq.o
> obj-$(CONFIG_PINCTRL_FALCON) += pinctrl-falcon.o
> obj-$(CONFIG_PINCTRL_XWAY) += pinctrl-xway.o
> +obj-$(CONFIG_PINCTRL_LOONGSON2) += pinctrl-loongson2.o
> obj-$(CONFIG_PINCTRL_LPC18XX) += pinctrl-lpc18xx.o
> obj-$(CONFIG_PINCTRL_MAX77620) += pinctrl-max77620.o
> obj-$(CONFIG_PINCTRL_MCP23S08_I2C) += pinctrl-mcp23s08_i2c.o
> diff --git a/drivers/pinctrl/pinctrl-loongson2.c b/drivers/pinctrl/pinctrl-loongson2.c
> new file mode 100644
> index 000000000000..8d2813cb60cf
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-loongson2.c
> @@ -0,0 +1,331 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Author: zhanghongchen <[email protected]>
> + * Yinbo Zhu <[email protected]>
> + * Copyright (C) 2022-2023 Loongson Technology Corporation Limited
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/pinctrl/pinmux.h>
> +#include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/seq_file.h>
> +#include <asm-generic/io.h>
> +#include "core.h"
> +#include "pinctrl-utils.h"
> +
> +#define PMX_GROUP(grp, offset, bitv) \
> + { \
> + .name = #grp, \
> + .pins = grp ## _pins, \
> + .num_pins = ARRAY_SIZE(grp ## _pins), \
> + .reg = offset, \
> + .bit = bitv, \
> + }
> +
> +#define SPECIFIC_GROUP(group) \
> + static const char * const group##_groups[] = { \
> + #group \
> + }
> +
> +#define FUNCTION(fn) \
> + { \
> + .name = #fn, \
> + .groups = fn ## _groups, \
> + .num_groups = ARRAY_SIZE(fn ## _groups), \
> + }
> +
> +struct loongson2_pinctrl {
> + struct device *dev;
> + struct pinctrl_dev *pcdev;
> + struct pinctrl_desc desc;
> + struct device_node *of_node;
> + spinlock_t lock;
> + void * __iomem reg_base;
> +};
> +
> +struct loongson2_pmx_group {
> + const char *name;
> + const unsigned int *pins;
> + unsigned int num_pins;
> + unsigned int reg;
> + unsigned int bit;
> +};
> +
> +struct loongson2_pmx_func {
> + const char *name;
> + const char * const *groups;
> + unsigned int num_groups;
> +};
> +
> +#define LOONGSON2_PIN(x) PINCTRL_PIN(x, "gpio"#x)
> +static const struct pinctrl_pin_desc loongson2_pctrl_pins[] = {
> + LOONGSON2_PIN(0), LOONGSON2_PIN(1), LOONGSON2_PIN(2), LOONGSON2_PIN(3),
> + LOONGSON2_PIN(4), LOONGSON2_PIN(5), LOONGSON2_PIN(6), LOONGSON2_PIN(7),
> + LOONGSON2_PIN(8), LOONGSON2_PIN(9), LOONGSON2_PIN(10), LOONGSON2_PIN(11),
> + LOONGSON2_PIN(12), LOONGSON2_PIN(13), LOONGSON2_PIN(14),
> + LOONGSON2_PIN(16), LOONGSON2_PIN(17), LOONGSON2_PIN(18), LOONGSON2_PIN(19),
> + LOONGSON2_PIN(20), LOONGSON2_PIN(21), LOONGSON2_PIN(22), LOONGSON2_PIN(23),
> + LOONGSON2_PIN(24), LOONGSON2_PIN(25), LOONGSON2_PIN(26), LOONGSON2_PIN(27),
> + LOONGSON2_PIN(28), LOONGSON2_PIN(29), LOONGSON2_PIN(30),
> + LOONGSON2_PIN(32), LOONGSON2_PIN(33), LOONGSON2_PIN(34), LOONGSON2_PIN(35),
> + LOONGSON2_PIN(36), LOONGSON2_PIN(37), LOONGSON2_PIN(38), LOONGSON2_PIN(39),
> + LOONGSON2_PIN(40), LOONGSON2_PIN(41),
> + LOONGSON2_PIN(44), LOONGSON2_PIN(45), LOONGSON2_PIN(46), LOONGSON2_PIN(47),
> + LOONGSON2_PIN(48), LOONGSON2_PIN(49), LOONGSON2_PIN(50), LOONGSON2_PIN(51),
> + LOONGSON2_PIN(52), LOONGSON2_PIN(53), LOONGSON2_PIN(54), LOONGSON2_PIN(55),
> + LOONGSON2_PIN(56), LOONGSON2_PIN(57), LOONGSON2_PIN(58), LOONGSON2_PIN(59),
> + LOONGSON2_PIN(60), LOONGSON2_PIN(61), LOONGSON2_PIN(62), LOONGSON2_PIN(63),
> +};
> +
> +static const unsigned int gpio_pins[] = {0, 1, 2, 3, 4, 5, 6, 7,
> + 8, 9, 10, 11, 12, 13, 14,
> + 16, 17, 18, 19, 20, 21, 22, 23,
> + 24, 25, 26, 27, 28, 29, 30,
> + 32, 33, 34, 35, 36, 37, 38, 39,
> + 40, 43, 44, 45, 46, 47,
> + 48, 49, 50, 51, 52, 53, 46, 55,
> + 56, 57, 58, 59, 60, 61, 62, 63};
> +static const unsigned int sdio_pins[] = {36, 37, 38, 39, 40, 41};
> +static const unsigned int can1_pins[] = {34, 35};
> +static const unsigned int can0_pins[] = {32, 33};
> +static const unsigned int pwm3_pins[] = {23};
> +static const unsigned int pwm2_pins[] = {22};
> +static const unsigned int pwm1_pins[] = {21};
> +static const unsigned int pwm0_pins[] = {20};
> +static const unsigned int i2c1_pins[] = {18, 19};
> +static const unsigned int i2c0_pins[] = {16, 17};
> +static const unsigned int nand_pins[] = {44, 45, 46, 47, 48, 49, 50, 51,
> + 52, 53, 54, 55, 56, 57, 58, 59, 60,
> + 61, 62, 63};
> +static const unsigned int sata_led_pins[] = {14};
> +static const unsigned int lio_pins[] = {};
> +static const unsigned int i2s_pins[] = {24, 25, 26, 27, 28};
> +static const unsigned int hda_pins[] = {24, 25, 26, 27, 28, 29, 30};
> +static const unsigned int uart2_pins[] = {};
> +static const unsigned int uart1_pins[] = {};
> +static const unsigned int camera_pins[] = {};
> +static const unsigned int dvo1_pins[] = {};
> +static const unsigned int dvo0_pins[] = {};
> +
> +static struct loongson2_pmx_group loongson2_pmx_groups[] = {
> + PMX_GROUP(gpio, 0x0, 64),
> + PMX_GROUP(sdio, 0x0, 20),
> + PMX_GROUP(can1, 0x0, 17),
> + PMX_GROUP(can0, 0x0, 16),
> + PMX_GROUP(pwm3, 0x0, 15),
> + PMX_GROUP(pwm2, 0x0, 14),
> + PMX_GROUP(pwm1, 0x0, 13),
> + PMX_GROUP(pwm0, 0x0, 12),
> + PMX_GROUP(i2c1, 0x0, 11),
> + PMX_GROUP(i2c0, 0x0, 10),
> + PMX_GROUP(nand, 0x0, 9),
> + PMX_GROUP(sata_led, 0x0, 8),
> + PMX_GROUP(lio, 0x0, 7),
> + PMX_GROUP(i2s, 0x0, 6),
> + PMX_GROUP(hda, 0x0, 4),
> + PMX_GROUP(uart2, 0x8, 13),
> + PMX_GROUP(uart1, 0x8, 12),
> + PMX_GROUP(camera, 0x10, 5),
> + PMX_GROUP(dvo1, 0x10, 4),
> + PMX_GROUP(dvo0, 0x10, 1),
> +
> +};
> +
> +SPECIFIC_GROUP(sdio);
> +SPECIFIC_GROUP(can1);
> +SPECIFIC_GROUP(can0);
> +SPECIFIC_GROUP(pwm3);
> +SPECIFIC_GROUP(pwm2);
> +SPECIFIC_GROUP(pwm1);
> +SPECIFIC_GROUP(pwm0);
> +SPECIFIC_GROUP(i2c1);
> +SPECIFIC_GROUP(i2c0);
> +SPECIFIC_GROUP(nand);
> +SPECIFIC_GROUP(sata_led);
> +SPECIFIC_GROUP(lio);
> +SPECIFIC_GROUP(i2s);
> +SPECIFIC_GROUP(hda);
> +SPECIFIC_GROUP(uart2);
> +SPECIFIC_GROUP(uart1);
> +SPECIFIC_GROUP(camera);
> +SPECIFIC_GROUP(dvo1);
> +SPECIFIC_GROUP(dvo0);
> +
> +static const char * const gpio_groups[] = {
> + "sdio", "can1", "can0", "pwm3", "pwm2", "pwm1", "pwm0", "i2c1",
> + "i2c0", "nand", "sata_led", "lio", "i2s", "hda", "uart2", "uart1",
> + "camera", "dvo1", "dvo0"
> +};
> +
> +static const struct loongson2_pmx_func loongson2_pmx_functions[] = {
> + FUNCTION(gpio),
> + FUNCTION(sdio),
> + FUNCTION(can1),
> + FUNCTION(can0),
> + FUNCTION(pwm3),
> + FUNCTION(pwm2),
> + FUNCTION(pwm1),
> + FUNCTION(pwm0),
> + FUNCTION(i2c1),
> + FUNCTION(i2c0),
> + FUNCTION(nand),
> + FUNCTION(sata_led),
> + FUNCTION(lio),
> + FUNCTION(i2s),
> + FUNCTION(hda),
> + FUNCTION(uart2),
> + FUNCTION(uart1),
> + FUNCTION(camera),
> + FUNCTION(dvo1),
> + FUNCTION(dvo0),
> +};
> +
> +static int loongson2_get_groups_count(struct pinctrl_dev *pcdev)
> +{
> + return ARRAY_SIZE(loongson2_pmx_groups);
> +}
> +
> +static const char *loongson2_get_group_name(struct pinctrl_dev *pcdev,
> + unsigned int selector)
> +{
> + return loongson2_pmx_groups[selector].name;
> +}
> +
> +static int loongson2_get_group_pins(struct pinctrl_dev *pcdev, unsigned int selector,
> + const unsigned int **pins, unsigned int *num_pins)
> +{
> + *pins = loongson2_pmx_groups[selector].pins;
> + *num_pins = loongson2_pmx_groups[selector].num_pins;
> +
> + return 0;
> +}
> +
> +static void loongson2_pin_dbg_show(struct pinctrl_dev *pcdev, struct seq_file *s,
> + unsigned int offset)
> +{
> + seq_printf(s, " %s", dev_name(pcdev->dev));
> +}
> +
> +static const struct pinctrl_ops loongson2_pctrl_ops = {
> + .get_groups_count = loongson2_get_groups_count,
> + .get_group_name = loongson2_get_group_name,
> + .get_group_pins = loongson2_get_group_pins,
> + .dt_node_to_map = pinconf_generic_dt_node_to_map_all,
> + .dt_free_map = pinctrl_utils_free_map,
> + .pin_dbg_show = loongson2_pin_dbg_show,
> +};
> +
> +static int loongson2_pmx_set_mux(struct pinctrl_dev *pcdev, unsigned int func_num,
> + unsigned int group_num)
> +{
> + struct loongson2_pinctrl *pctrl = pinctrl_dev_get_drvdata(pcdev);
> + unsigned long reg = (unsigned long)pctrl->reg_base +
> + loongson2_pmx_groups[group_num].reg;
> + unsigned int mux_bit = loongson2_pmx_groups[group_num].bit;
> + unsigned int val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&pctrl->lock, flags);
> + val = readl((void *)reg);
> + if (func_num == 0)
> + val &= ~(1<<mux_bit);
> + else
> + val |= (1<<mux_bit);
> + writel(val, (void *)reg);
> + spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> + return 0;
> +}
> +
> +static int loongson2_pmx_get_funcs_count(struct pinctrl_dev *pcdev)
> +{
> + return ARRAY_SIZE(loongson2_pmx_functions);
> +}
> +
> +static const char *loongson2_pmx_get_func_name(struct pinctrl_dev *pcdev,
> + unsigned int selector)
> +{
> + return loongson2_pmx_functions[selector].name;
> +}
> +
> +static int loongson2_pmx_get_groups(struct pinctrl_dev *pcdev,
> + unsigned int selector,
> + const char * const **groups,
> + unsigned int * const num_groups)
> +{
> + *groups = loongson2_pmx_functions[selector].groups;
> + *num_groups = loongson2_pmx_functions[selector].num_groups;
> +
> + return 0;
> +}
> +
> +static const struct pinmux_ops loongson2_pmx_ops = {
> + .set_mux = loongson2_pmx_set_mux,
> + .get_functions_count = loongson2_pmx_get_funcs_count,
> + .get_function_name = loongson2_pmx_get_func_name,
> + .get_function_groups = loongson2_pmx_get_groups,
> +};
> +
> +static int loongson2_pinctrl_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct loongson2_pinctrl *pctrl;
> +
> + pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL);
> + if (!pctrl)
> + return -ENOMEM;
> +
> + pctrl->reg_base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(pctrl->reg_base))
> + return dev_err_probe(pctrl->dev, PTR_ERR(pctrl->reg_base),
> + "unable to map I/O memory");
> +
> + spin_lock_init(&pctrl->lock);
> +
> + pctrl->dev = dev;
> + pctrl->desc.name = "pinctrl-loongson2";
> + pctrl->desc.owner = THIS_MODULE;
> + pctrl->desc.pctlops = &loongson2_pctrl_ops;
> + pctrl->desc.pmxops = &loongson2_pmx_ops;
> + pctrl->desc.confops = NULL;
> + pctrl->desc.pins = loongson2_pctrl_pins;
> + pctrl->desc.npins = ARRAY_SIZE(loongson2_pctrl_pins);
> +
> + pctrl->pcdev = devm_pinctrl_register(pctrl->dev, &pctrl->desc, pctrl);
> + if (IS_ERR(pctrl->pcdev))
> + return dev_err_probe(pctrl->dev, PTR_ERR(pctrl->pcdev),
> + "can't register pinctrl device");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id loongson2_pinctrl_dt_match[] = {
> + {
> + .compatible = "loongson,ls2k-pinctrl",
> + },
> + { },
> +};
> +
> +static struct platform_driver loongson2_pinctrl_driver = {
> + .probe = loongson2_pinctrl_probe,
> + .driver = {
> + .name = "loongson2-pinctrl",
> + .of_match_table = loongson2_pinctrl_dt_match,
> + },
> +};
> +
> +static int __init loongson2_pinctrl_init(void)
> +{
> + return platform_driver_register(&loongson2_pinctrl_driver);
> +}
> +arch_initcall(loongson2_pinctrl_init);
> +
> +static void __exit loongson2_pinctrl_exit(void)
> +{
> + platform_driver_unregister(&loongson2_pinctrl_driver);
> +}
> +module_exit(loongson2_pinctrl_exit);
>


2022-11-09 08:43:28

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] pinctrl: pinctrl-loongson2: add pinctrl driver support

On Wed, Nov 9, 2022 at 7:42 AM Yinbo Zhu <[email protected]> wrote:

> I had added some changes in these series patch in v8, please help add my
> change and merge it into your tree and sync it to linux-next.

Yeah no problem, I took out the v7 version and applied this one instead.

Yours,
Linus Walleij

2022-11-09 15:51:57

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] pinctrl: pinctrl-loongson2: add pinctrl driver support

On Wed, Nov 09, 2022 at 09:30:03AM +0100, Linus Walleij wrote:
> On Wed, Nov 9, 2022 at 7:42 AM Yinbo Zhu <[email protected]> wrote:
>
> > I had added some changes in these series patch in v8, please help add my
> > change and merge it into your tree and sync it to linux-next.
>
> Yeah no problem, I took out the v7 version and applied this one instead.

It needs more work.

--
With Best Regards,
Andy Shevchenko



2022-11-09 16:41:21

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] pinctrl: pinctrl-loongson2: add pinctrl driver support

On Wed, Nov 09, 2022 at 02:11:21PM +0800, Yinbo Zhu wrote:
> The Loongson-2 SoC has a few pins that can be used as GPIOs or take
> multiple other functions. Add a driver for the pinmuxing.
>
> There is currently no support for GPIO pin pull-up and pull-down.

> Signed-off-by: zhanghongchen <[email protected]>
> Signed-off-by: Yinbo Zhu <[email protected]>

Why two SoBs? Who is(are) the author(s)?

...

> + help
> + This selects pin control driver for the Loongson-2 SoC. It

One space is enough.

> + provides pin config functions multiplexing. GPIO pin pull-up,
> + pull-down functions are not supported. Say yes to enable
> + pinctrl for Loongson-2 SoC.

Perhaps keep your entry in order?

> source "drivers/pinctrl/actions/Kconfig"
> source "drivers/pinctrl/aspeed/Kconfig"
> source "drivers/pinctrl/bcm/Kconfig"

...

> @@ -29,6 +29,7 @@ obj-$(CONFIG_PINCTRL_KEEMBAY) += pinctrl-keembay.o
> obj-$(CONFIG_PINCTRL_LANTIQ) += pinctrl-lantiq.o
> obj-$(CONFIG_PINCTRL_FALCON) += pinctrl-falcon.o
> obj-$(CONFIG_PINCTRL_XWAY) += pinctrl-xway.o
> +obj-$(CONFIG_PINCTRL_LOONGSON2) += pinctrl-loongson2.o

I would expect more order here...

> obj-$(CONFIG_PINCTRL_LPC18XX) += pinctrl-lpc18xx.o
> obj-$(CONFIG_PINCTRL_MAX77620) += pinctrl-max77620.o
> obj-$(CONFIG_PINCTRL_MCP23S08_I2C) += pinctrl-mcp23s08_i2c.o

...

> + * Author: zhanghongchen <[email protected]>
> + * Yinbo Zhu <[email protected]>

Missed Co-developed-by tag above?

> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>

> +#include <linux/of.h>

I found no user of this header.
But you missed mod_devicetable.h.

> +#include <linux/pinctrl/pinmux.h>
> +#include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/pinctrl/pinctrl.h>

Can we keep it as a separate group after generic linux/* ones?

> +#include <linux/seq_file.h>

+ Blank line.

> +#include <asm-generic/io.h>

No, use linux/io.h.

...

> +#define PMX_GROUP(grp, offset, bitv) \
> + { \
> + .name = #grp, \
> + .pins = grp ## _pins, \
> + .num_pins = ARRAY_SIZE(grp ## _pins), \
> + .reg = offset, \
> + .bit = bitv, \
> + }

Use PINCTRL_PINGROUP() and associated data structure.

...

> +static const unsigned int lio_pins[] = {};
> +static const unsigned int uart2_pins[] = {};
> +static const unsigned int uart1_pins[] = {};
> +static const unsigned int camera_pins[] = {};
> +static const unsigned int dvo1_pins[] = {};
> +static const unsigned int dvo0_pins[] = {};

No sure what this means.

...

> +static struct loongson2_pmx_group loongson2_pmx_groups[] = {
> + PMX_GROUP(gpio, 0x0, 64),
> + PMX_GROUP(sdio, 0x0, 20),
> + PMX_GROUP(can1, 0x0, 17),
> + PMX_GROUP(can0, 0x0, 16),
> + PMX_GROUP(pwm3, 0x0, 15),
> + PMX_GROUP(pwm2, 0x0, 14),
> + PMX_GROUP(pwm1, 0x0, 13),
> + PMX_GROUP(pwm0, 0x0, 12),
> + PMX_GROUP(i2c1, 0x0, 11),
> + PMX_GROUP(i2c0, 0x0, 10),
> + PMX_GROUP(nand, 0x0, 9),
> + PMX_GROUP(sata_led, 0x0, 8),
> + PMX_GROUP(lio, 0x0, 7),
> + PMX_GROUP(i2s, 0x0, 6),
> + PMX_GROUP(hda, 0x0, 4),
> + PMX_GROUP(uart2, 0x8, 13),
> + PMX_GROUP(uart1, 0x8, 12),
> + PMX_GROUP(camera, 0x10, 5),
> + PMX_GROUP(dvo1, 0x10, 4),
> + PMX_GROUP(dvo0, 0x10, 1),

> +

Redundant blank line.

> +};

...

> +static const char * const gpio_groups[] = {
> + "sdio", "can1", "can0", "pwm3", "pwm2", "pwm1", "pwm0", "i2c1",
> + "i2c0", "nand", "sata_led", "lio", "i2s", "hda", "uart2", "uart1",
> + "camera", "dvo1", "dvo0"

Leave trailing comma.

Also it would be nice to have that grouped like

"sdio",
"can1", "can0",
"pwm3", "pwm2", "pwm1", "pwm0",
"i2c1", "i2c0",
"nand",
"sata_led",
"lio",
"i2s", "hda",
"uart2", "uart1",
"camera",
"dvo1", "dvo0",

> +};


...

> + unsigned long reg = (unsigned long)pctrl->reg_base +
> + loongson2_pmx_groups[group_num].reg;

Why casting?!

...

> + val = readl((void *)reg);

Ouch.

> + if (func_num == 0)
> + val &= ~(1<<mux_bit);
> + else
> + val |= (1<<mux_bit);

Why not using __assign_bit() or similar? Or at least BIT() ?

...

> + writel(val, (void *)reg);

Ouch!

...

> + pctrl->reg_base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(pctrl->reg_base))

> + return dev_err_probe(pctrl->dev, PTR_ERR(pctrl->reg_base),
> + "unable to map I/O memory");

Message duplicates what core does.

...

> + pctrl->desc.confops = NULL;

Redundant.

...

> +static const struct of_device_id loongson2_pinctrl_dt_match[] = {
> + {
> + .compatible = "loongson,ls2k-pinctrl",
> + },

> + { },

No comma for the terminator line.

> +};

--
With Best Regards,
Andy Shevchenko



2022-11-09 16:53:44

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] pinctrl: pinctrl-loongson2: add pinctrl driver support

On Wed, Nov 09, 2022 at 05:01:39PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 09, 2022 at 09:30:03AM +0100, Linus Walleij wrote:
> > On Wed, Nov 9, 2022 at 7:42 AM Yinbo Zhu <[email protected]> wrote:
> >
> > > I had added some changes in these series patch in v8, please help add my
> > > change and merge it into your tree and sync it to linux-next.
> >
> > Yeah no problem, I took out the v7 version and applied this one instead.
>
> It needs more work.

I mean it still needs more work, the ugliest part is the castings for IO
accessors. Must be fixed beforehand.

--
With Best Regards,
Andy Shevchenko



2022-11-10 08:42:17

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] pinctrl: pinctrl-loongson2: add pinctrl driver support

On Wed, Nov 9, 2022 at 4:01 PM Andy Shevchenko
<[email protected]> wrote:
> On Wed, Nov 09, 2022 at 09:30:03AM +0100, Linus Walleij wrote:
> > On Wed, Nov 9, 2022 at 7:42 AM Yinbo Zhu <[email protected]> wrote:
> >
> > > I had added some changes in these series patch in v8, please help add my
> > > change and merge it into your tree and sync it to linux-next.
> >
> > Yeah no problem, I took out the v7 version and applied this one instead.
>
> It needs more work.

It also failed in linux-next because of missing MODULE_LICENSE()
so I took it out again, thanks for poking me.

Yinbo: don't lose your spirit, keep at it!

Yours,
Linus Walleij

2022-11-11 04:02:34

by Yinbo Zhu

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] pinctrl: pinctrl-loongson2: add pinctrl driver support



在 2022/11/10 下午4:17, Linus Walleij 写道:
> On Wed, Nov 9, 2022 at 4:01 PM Andy Shevchenko
> <[email protected]> wrote:
>> On Wed, Nov 09, 2022 at 09:30:03AM +0100, Linus Walleij wrote:
>>> On Wed, Nov 9, 2022 at 7:42 AM Yinbo Zhu <[email protected]> wrote:
>>>
>>>> I had added some changes in these series patch in v8, please help add my
>>>> change and merge it into your tree and sync it to linux-next.
>>>
>>> Yeah no problem, I took out the v7 version and applied this one instead.
>>
>> It needs more work.
>
> It also failed in linux-next because of missing MODULE_LICENSE()
> so I took it out again, thanks for poking me.
>
> Yinbo: don't lose your spirit, keep at it!
>
> Yours,
> Linus Walleij
Hi Walleij,

Thanks for your understanding.

I had add v9 and verified that base your pinctrl tree and compile it use
W=1 that no any warning and error.

BRs,
Yinbo.
>


2022-11-11 04:12:47

by Yinbo Zhu

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] pinctrl: pinctrl-loongson2: add pinctrl driver support


Hi Andy,

I had add v9 that following your advice, but there are some explain
about your doubt as follows:

?? 2022/11/9 ????11:40, Andy Shevchenko д??:
> On Wed, Nov 09, 2022 at 02:11:21PM +0800, Yinbo Zhu wrote:
>> The Loongson-2 SoC has a few pins that can be used as GPIOs or take
>> multiple other functions. Add a driver for the pinmuxing.
>>
>> There is currently no support for GPIO pin pull-up and pull-down.
>
>> Signed-off-by: zhanghongchen <[email protected]>
>> Signed-off-by: Yinbo Zhu <[email protected]>
>
> Why two SoBs? Who is(are) the author(s)?
>
> ...
>
>> + help
>> + This selects pin control driver for the Loongson-2 SoC. It
>
> One space is enough.
>
>> + provides pin config functions multiplexing. GPIO pin pull-up,
>> + pull-down functions are not supported. Say yes to enable
>> + pinctrl for Loongson-2 SoC.
>
> Perhaps keep your entry in order?
>
>> source "drivers/pinctrl/actions/Kconfig"
>> source "drivers/pinctrl/aspeed/Kconfig"
>> source "drivers/pinctrl/bcm/Kconfig"
>
> ...
>
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_PINCTRL_KEEMBAY) += pinctrl-keembay.o
>> obj-$(CONFIG_PINCTRL_LANTIQ) += pinctrl-lantiq.o
>> obj-$(CONFIG_PINCTRL_FALCON) += pinctrl-falcon.o
>> obj-$(CONFIG_PINCTRL_XWAY) += pinctrl-xway.o
>> +obj-$(CONFIG_PINCTRL_LOONGSON2) += pinctrl-loongson2.o
>
> I would expect more order here...
>
>> obj-$(CONFIG_PINCTRL_LPC18XX) += pinctrl-lpc18xx.o
>> obj-$(CONFIG_PINCTRL_MAX77620) += pinctrl-max77620.o
>> obj-$(CONFIG_PINCTRL_MCP23S08_I2C) += pinctrl-mcp23s08_i2c.o
>
> ...
>
>> + * Author: zhanghongchen <[email protected]>
>> + * Yinbo Zhu <[email protected]>
>
> Missed Co-developed-by tag above?
>
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>
>> +#include <linux/of.h>
>
> I found no user of this header.
> But you missed mod_devicetable.h.
>
>> +#include <linux/pinctrl/pinmux.h>
>> +#include <linux/pinctrl/pinconf-generic.h>
>> +#include <linux/pinctrl/pinctrl.h>
>
> Can we keep it as a separate group after generic linux/* ones?
>
>> +#include <linux/seq_file.h>
>
> + Blank line.
>
>> +#include <asm-generic/io.h>
>
> No, use linux/io.h.
>
> ...
>
>> +#define PMX_GROUP(grp, offset, bitv) \
>> + { \
>> + .name = #grp, \
>> + .pins = grp ## _pins, \
>> + .num_pins = ARRAY_SIZE(grp ## _pins), \
>> + .reg = offset, \
>> + .bit = bitv, \
>> + }
>
> Use PINCTRL_PINGROUP() and associated data structure.
>
> ...
>
>> +static const unsigned int lio_pins[] = {};
>> +static const unsigned int uart2_pins[] = {};
>> +static const unsigned int uart1_pins[] = {};
>> +static const unsigned int camera_pins[] = {};
>> +static const unsigned int dvo1_pins[] = {};
>> +static const unsigned int dvo0_pins[] = {};
>
> No sure what this means.
There are some fuzzy reuse relationships on loongson2, e.g., lio can be
reused as gpio, which is one of the reserved gpios but the manual not
indicates this gpio index.
>
> ...
>
>> +static struct loongson2_pmx_group loongson2_pmx_groups[] = {
>> + PMX_GROUP(gpio, 0x0, 64),
>> + PMX_GROUP(sdio, 0x0, 20),
>> + PMX_GROUP(can1, 0x0, 17),
>> + PMX_GROUP(can0, 0x0, 16),
>> + PMX_GROUP(pwm3, 0x0, 15),
>> + PMX_GROUP(pwm2, 0x0, 14),
>> + PMX_GROUP(pwm1, 0x0, 13),
>> + PMX_GROUP(pwm0, 0x0, 12),
>> + PMX_GROUP(i2c1, 0x0, 11),
>> + PMX_GROUP(i2c0, 0x0, 10),
>> + PMX_GROUP(nand, 0x0, 9),
>> + PMX_GROUP(sata_led, 0x0, 8),
>> + PMX_GROUP(lio, 0x0, 7),
>> + PMX_GROUP(i2s, 0x0, 6),
>> + PMX_GROUP(hda, 0x0, 4),
>> + PMX_GROUP(uart2, 0x8, 13),
>> + PMX_GROUP(uart1, 0x8, 12),
>> + PMX_GROUP(camera, 0x10, 5),
>> + PMX_GROUP(dvo1, 0x10, 4),
>> + PMX_GROUP(dvo0, 0x10, 1),
>
>> +
>
> Redundant blank line.
>
>> +};
>
> ...
>
>> +static const char * const gpio_groups[] = {
>> + "sdio", "can1", "can0", "pwm3", "pwm2", "pwm1", "pwm0", "i2c1",
>> + "i2c0", "nand", "sata_led", "lio", "i2s", "hda", "uart2", "uart1",
>> + "camera", "dvo1", "dvo0"
>
> Leave trailing comma.
>
> Also it would be nice to have that grouped like
>
> "sdio",
> "can1", "can0",
> "pwm3", "pwm2", "pwm1", "pwm0",
> "i2c1", "i2c0",
> "nand",
> "sata_led",
> "lio",
> "i2s", "hda",
> "uart2", "uart1",
> "camera",
> "dvo1", "dvo0",
>
>> +};
>
>
> ...
>
>> + unsigned long reg = (unsigned long)pctrl->reg_base +
>> + loongson2_pmx_groups[group_num].reg;
>
> Why casting?!
There are some registers that determine the pin reuse relationship. For
example, register A determines the sdio reuse relationship, and register
A+offset determines the uart reuse relationship
>
> ...
>
>> + val = readl((void *)reg);
>
> Ouch.
>
>> + if (func_num == 0)
>> + val &= ~(1<<mux_bit);
>> + else
>> + val |= (1<<mux_bit);
>
> Why not using __assign_bit() or similar? Or at least BIT() ?
>
> ...
>
>> + writel(val, (void *)reg);
>
> Ouch!
I will keep writel/readl.
>
> ...
>
>> + pctrl->reg_base = devm_platform_ioremap_resource(pdev, 0);
>> + if (IS_ERR(pctrl->reg_base))
>
>> + return dev_err_probe(pctrl->dev, PTR_ERR(pctrl->reg_base),
>> + "unable to map I/O memory");
>
> Message duplicates what core does.
>
> ...
>
>> + pctrl->desc.confops = NULL;
>
> Redundant.
>
> ...
>
>> +static const struct of_device_id loongson2_pinctrl_dt_match[] = {
>> + {
>> + .compatible = "loongson,ls2k-pinctrl",
>> + },
>
>> + { },
>
> No comma for the terminator line.
>
>> +};
>


2022-11-11 11:36:46

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] pinctrl: pinctrl-loongson2: add pinctrl driver support

On Fri, Nov 11, 2022 at 11:53:37AM +0800, Yinbo Zhu wrote:
> ?? 2022/11/9 ????11:40, Andy Shevchenko д??:
> > On Wed, Nov 09, 2022 at 02:11:21PM +0800, Yinbo Zhu wrote:

...

> > > +static const unsigned int lio_pins[] = {};
> > > +static const unsigned int uart2_pins[] = {};
> > > +static const unsigned int uart1_pins[] = {};
> > > +static const unsigned int camera_pins[] = {};
> > > +static const unsigned int dvo1_pins[] = {};
> > > +static const unsigned int dvo0_pins[] = {};
> >
> > No sure what this means.
> There are some fuzzy reuse relationships on loongson2, e.g., lio can be
> reused as gpio, which is one of the reserved gpios but the manual not
> indicates this gpio index.

The rule of thumb is to avoid putting something you are not able to test.

...

> > > + unsigned long reg = (unsigned long)pctrl->reg_base +
> > > + loongson2_pmx_groups[group_num].reg;
> >
> > Why casting?!
> There are some registers that determine the pin reuse relationship. For
> example, register A determines the sdio reuse relationship, and register
> A+offset determines the uart reuse relationship

I'm asking about something different. Why do you need casting for iomem offset?

...

> > > + val = readl((void *)reg);
> >
> > Ouch.

^^^ You see, this is a real issue with code, without fix which it's no go.

...

> > > + writel(val, (void *)reg);
> >
> > Ouch!
> I will keep writel/readl.

Yes, but you need to take care about annotations of the addresses (__iomem)
and remove ugly castings.

--
With Best Regards,
Andy Shevchenko