This patchset adds Pin controller driver support for Meson-A1 Soc
which shares the same register layout of pinmux with previous
Meson-G12A, however there is difference for gpio and pin config
registers in A1.
Changes since v3 at [2]
- separate ao fixup from meson_pinctrl_parse_dt
- provide ao extra dt parse callback for each SoC
Changes since v2 at [1]:
- make dt parser callback as a separate patch
Changes since v1 at [0]:
- collect Reviewed-by
- modify commit log
- add an extra dt parser function for a1
[0] https://lore.kernel.org/linux-amlogic/[email protected]/
[1] https://lore.kernel.org/linux-amlogic/[email protected]/
[2] https://lore.kernel.org/linux-amlogic/[email protected]/
Qianggui Song (4):
pinctrl: add compatible for Amlogic Meson A1 pin controller
pinctrl: meson: add a new callback for SoCs fixup
pinctrl: meson: add pinctrl driver support for Meson-A1 SoC
arm64: dts: meson: a1: add pinctrl controller support
.../devicetree/bindings/pinctrl/meson,pinctrl.txt | 1 +
arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 18 +
drivers/pinctrl/meson/Kconfig | 6 +
drivers/pinctrl/meson/Makefile | 1 +
drivers/pinctrl/meson/pinctrl-meson-a1.c | 951 +++++++++++++++++++++
drivers/pinctrl/meson/pinctrl-meson-axg.c | 11 +
drivers/pinctrl/meson/pinctrl-meson-g12a.c | 9 +
drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 11 +
drivers/pinctrl/meson/pinctrl-meson-gxl.c | 11 +
drivers/pinctrl/meson/pinctrl-meson.c | 9 +-
drivers/pinctrl/meson/pinctrl-meson.h | 3 +
drivers/pinctrl/meson/pinctrl-meson8.c | 11 +
drivers/pinctrl/meson/pinctrl-meson8b.c | 11 +
include/dt-bindings/gpio/meson-a1-gpio.h | 73 ++
14 files changed, 1122 insertions(+), 4 deletions(-)
create mode 100644 drivers/pinctrl/meson/pinctrl-meson-a1.c
create mode 100644 include/dt-bindings/gpio/meson-a1-gpio.h
--
1.9.1
In meson_pinctrl_parse_dt, it contains two parts: reg parsing and
SoC relative fixup for AO. Several fixups in the same code make it hard
to maintain, so move all fixups to each SoC's callback and make
meson_pinctrl_parse_dt just do the reg parsing, separate these two
parts.Overview of all current Meson SoCs fixup is as below:
+------+--------------------------------------+--------------------------+
| | | |
| SoC | EE domain | AO domain |
+------+--------------------------------------+--------------------------+
|m8 | parse regs: | parse regs: |
|m8b | gpio,mux,pull,pull-enable(skip ds) | gpio,mux,pull(skip ds)|
|gxl | fixup: | fixup: |
|gxbb | no | pull-enable = pull |
|axg | | |
+------+--------------------------------------+--------------------------+
|g12a | parse regs: | parse regs: |
|sm1 | gpio,mux,pull,pull-enable,ds | gpio,mux,ds |
| | fixup: | fixup: |
| | no | pull = gpio |
| | | pull-enable = gpio |
+------+--------------------------------------+--------------------------+
|a1 or | parse regs: |
|later | gpio/mux (without ao domain) |
|SoCs | fixup: |
| | pull=gpio; pull-enable=gpio; ds=gpio |
+------+-----------------------------------------------------------------+
Signed-off-by: Qianggui Song <[email protected]>
---
drivers/pinctrl/meson/pinctrl-meson-axg.c | 11 +++++++++++
drivers/pinctrl/meson/pinctrl-meson-g12a.c | 9 +++++++++
drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 11 +++++++++++
drivers/pinctrl/meson/pinctrl-meson-gxl.c | 11 +++++++++++
drivers/pinctrl/meson/pinctrl-meson.c | 9 +++++----
drivers/pinctrl/meson/pinctrl-meson.h | 3 +++
drivers/pinctrl/meson/pinctrl-meson8.c | 11 +++++++++++
drivers/pinctrl/meson/pinctrl-meson8b.c | 11 +++++++++++
8 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg.c b/drivers/pinctrl/meson/pinctrl-meson-axg.c
index ad502eda4afa..9c07f4423c37 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-axg.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-axg.c
@@ -1040,6 +1040,16 @@
.num_pmx_banks = ARRAY_SIZE(meson_axg_aobus_pmx_banks),
};
+static int meson_axg_aobus_parse_dt_extra(struct meson_pinctrl *pc)
+{
+ if (!pc->reg_pull)
+ return -EINVAL;
+
+ pc->reg_pullen = pc->reg_pull;
+
+ return 0;
+}
+
static struct meson_pinctrl_data meson_axg_periphs_pinctrl_data = {
.name = "periphs-banks",
.pins = meson_axg_periphs_pins,
@@ -1066,6 +1076,7 @@
.num_banks = ARRAY_SIZE(meson_axg_aobus_banks),
.pmx_ops = &meson_axg_pmx_ops,
.pmx_data = &meson_axg_aobus_pmx_banks_data,
+ .parse_dt = meson_axg_aobus_parse_dt_extra,
};
static const struct of_device_id meson_axg_pinctrl_dt_match[] = {
diff --git a/drivers/pinctrl/meson/pinctrl-meson-g12a.c b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
index 582665fd362a..41850e3c0091 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-g12a.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
@@ -1362,6 +1362,14 @@
.num_pmx_banks = ARRAY_SIZE(meson_g12a_aobus_pmx_banks),
};
+static int meson_g12a_aobus_parse_dt_extra(struct meson_pinctrl *pc)
+{
+ pc->reg_pull = pc->reg_gpio;
+ pc->reg_pullen = pc->reg_gpio;
+
+ return 0;
+}
+
static struct meson_pinctrl_data meson_g12a_periphs_pinctrl_data = {
.name = "periphs-banks",
.pins = meson_g12a_periphs_pins,
@@ -1388,6 +1396,7 @@
.num_banks = ARRAY_SIZE(meson_g12a_aobus_banks),
.pmx_ops = &meson_axg_pmx_ops,
.pmx_data = &meson_g12a_aobus_pmx_banks_data,
+ .parse_dt = meson_g12a_aobus_parse_dt_extra,
};
static const struct of_device_id meson_g12a_pinctrl_dt_match[] = {
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
index 5bfa56f3847e..f5494a948200 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
@@ -827,6 +827,16 @@
BANK("AO", GPIOAO_0, GPIOAO_13, 0, 13, 0, 16, 0, 0, 0, 0, 0, 16, 1, 0),
};
+static int meson_gxbb_aobus_parse_dt_extra(struct meson_pinctrl *pc)
+{
+ if (!pc->reg_pull)
+ return -EINVAL;
+
+ pc->reg_pullen = pc->reg_pull;
+
+ return 0;
+}
+
static struct meson_pinctrl_data meson_gxbb_periphs_pinctrl_data = {
.name = "periphs-banks",
.pins = meson_gxbb_periphs_pins,
@@ -851,6 +861,7 @@
.num_funcs = ARRAY_SIZE(meson_gxbb_aobus_functions),
.num_banks = ARRAY_SIZE(meson_gxbb_aobus_banks),
.pmx_ops = &meson8_pmx_ops,
+ .parse_dt = meson_gxbb_aobus_parse_dt_extra,
};
static const struct of_device_id meson_gxbb_pinctrl_dt_match[] = {
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxl.c b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
index 72c5373c8dc1..943fb27dab08 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxl.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
@@ -796,6 +796,16 @@
BANK("AO", GPIOAO_0, GPIOAO_9, 0, 9, 0, 16, 0, 0, 0, 0, 0, 16, 1, 0),
};
+static int meson_gxl_aobus_parse_dt_extra(struct meson_pinctrl *pc)
+{
+ if (!pc->reg_pull)
+ return -EINVAL;
+
+ pc->reg_pullen = pc->reg_pull;
+
+ return 0;
+}
+
static struct meson_pinctrl_data meson_gxl_periphs_pinctrl_data = {
.name = "periphs-banks",
.pins = meson_gxl_periphs_pins,
@@ -820,6 +830,7 @@
.num_funcs = ARRAY_SIZE(meson_gxl_aobus_functions),
.num_banks = ARRAY_SIZE(meson_gxl_aobus_banks),
.pmx_ops = &meson8_pmx_ops,
+ .parse_dt = meson_gxl_aobus_parse_dt_extra,
};
static const struct of_device_id meson_gxl_pinctrl_dt_match[] = {
diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
index 8bba9d053d9f..e182583422a4 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.c
+++ b/drivers/pinctrl/meson/pinctrl-meson.c
@@ -677,14 +677,12 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
}
pc->reg_pull = meson_map_resource(pc, gpio_np, "pull");
- /* Use gpio region if pull one is not present */
if (IS_ERR(pc->reg_pull))
- pc->reg_pull = pc->reg_gpio;
+ pc->reg_pull = NULL;
pc->reg_pullen = meson_map_resource(pc, gpio_np, "pull-enable");
- /* Use pull region if pull-enable one is not present */
if (IS_ERR(pc->reg_pullen))
- pc->reg_pullen = pc->reg_pull;
+ pc->reg_pullen = NULL;
pc->reg_ds = meson_map_resource(pc, gpio_np, "ds");
if (IS_ERR(pc->reg_ds)) {
@@ -692,6 +690,9 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
pc->reg_ds = NULL;
}
+ if (pc->data->parse_dt)
+ return pc->data->parse_dt(pc);
+
return 0;
}
diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
index c696f3241a36..d570f7c53045 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.h
+++ b/drivers/pinctrl/meson/pinctrl-meson.h
@@ -11,6 +11,8 @@
#include <linux/regmap.h>
#include <linux/types.h>
+struct meson_pinctrl;
+
/**
* struct meson_pmx_group - a pinmux group
*
@@ -114,6 +116,7 @@ struct meson_pinctrl_data {
unsigned int num_banks;
const struct pinmux_ops *pmx_ops;
void *pmx_data;
+ int (*parse_dt)(struct meson_pinctrl *pc);
};
struct meson_pinctrl {
diff --git a/drivers/pinctrl/meson/pinctrl-meson8.c b/drivers/pinctrl/meson/pinctrl-meson8.c
index 0b97befa6335..65c70c9b7070 100644
--- a/drivers/pinctrl/meson/pinctrl-meson8.c
+++ b/drivers/pinctrl/meson/pinctrl-meson8.c
@@ -1079,6 +1079,16 @@
BANK("AO", GPIOAO_0, GPIO_TEST_N, 0, 13, 0, 16, 0, 0, 0, 0, 0, 16, 1, 0),
};
+static int meson8_aobus_parse_dt_extra(struct meson_pinctrl *pc)
+{
+ if (!pc->reg_pull)
+ return -EINVAL;
+
+ pc->reg_pullen = pc->reg_pull;
+
+ return 0;
+}
+
static struct meson_pinctrl_data meson8_cbus_pinctrl_data = {
.name = "cbus-banks",
.pins = meson8_cbus_pins,
@@ -1103,6 +1113,7 @@
.num_funcs = ARRAY_SIZE(meson8_aobus_functions),
.num_banks = ARRAY_SIZE(meson8_aobus_banks),
.pmx_ops = &meson8_pmx_ops,
+ .parse_dt = &meson8_aobus_parse_dt_extra,
};
static const struct of_device_id meson8_pinctrl_dt_match[] = {
diff --git a/drivers/pinctrl/meson/pinctrl-meson8b.c b/drivers/pinctrl/meson/pinctrl-meson8b.c
index a7de388388e6..85dc12e0c839 100644
--- a/drivers/pinctrl/meson/pinctrl-meson8b.c
+++ b/drivers/pinctrl/meson/pinctrl-meson8b.c
@@ -938,6 +938,16 @@
BANK("AO", GPIOAO_0, GPIO_TEST_N, 0, 13, 0, 16, 0, 0, 0, 0, 0, 16, 1, 0),
};
+static int meson8b_aobus_parse_dt_extra(struct meson_pinctrl *pc)
+{
+ if (!pc->reg_pull)
+ return -EINVAL;
+
+ pc->reg_pullen = pc->reg_pull;
+
+ return 0;
+}
+
static struct meson_pinctrl_data meson8b_cbus_pinctrl_data = {
.name = "cbus-banks",
.pins = meson8b_cbus_pins,
@@ -962,6 +972,7 @@
.num_funcs = ARRAY_SIZE(meson8b_aobus_functions),
.num_banks = ARRAY_SIZE(meson8b_aobus_banks),
.pmx_ops = &meson8_pmx_ops,
+ .parse_dt = &meson8b_aobus_parse_dt_extra,
};
static const struct of_device_id meson8b_pinctrl_dt_match[] = {
--
1.9.1
add peripheral pinctrl controller to a1 SoC
Signed-off-by: Qianggui Song <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index 7210ad049d1d..0965259af869 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -5,6 +5,7 @@
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/gpio/meson-a1-gpio.h>
/ {
compatible = "amlogic,a1";
@@ -74,6 +75,23 @@
#size-cells = <2>;
ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x1000000>;
+ periphs_pinctrl: pinctrl@0400 {
+ compatible = "amlogic,meson-a1-periphs-pinctrl";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ gpio: bank@0400 {
+ reg = <0x0 0x0400 0x0 0x003c>,
+ <0x0 0x0480 0x0 0x0118>;
+ reg-names = "mux", "gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&periphs_pinctrl 0 0 62>;
+ };
+
+ };
+
uart_AO: serial@1c00 {
compatible = "amlogic,meson-gx-uart",
"amlogic,meson-ao-uart";
--
1.9.1
Add new compatible name for Amlogic's Meson-A1 pin controller
add a dt-binding header file which document the detail pin names.
Note that A1 doesn't need DS bank reg any more, use gpio reg as
base.
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Neil Armstrong <[email protected]>
Signed-off-by: Qianggui Song <[email protected]>
---
.../devicetree/bindings/pinctrl/meson,pinctrl.txt | 1 +
include/dt-bindings/gpio/meson-a1-gpio.h | 73 ++++++++++++++++++++++
2 files changed, 74 insertions(+)
create mode 100644 include/dt-bindings/gpio/meson-a1-gpio.h
diff --git a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
index 10dc4f7176ca..0aff1f28495c 100644
--- a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
@@ -15,6 +15,7 @@ Required properties for the root node:
"amlogic,meson-axg-aobus-pinctrl"
"amlogic,meson-g12a-periphs-pinctrl"
"amlogic,meson-g12a-aobus-pinctrl"
+ "amlogic,meson-a1-periphs-pinctrl"
- reg: address and size of registers controlling irq functionality
=== GPIO sub-nodes ===
diff --git a/include/dt-bindings/gpio/meson-a1-gpio.h b/include/dt-bindings/gpio/meson-a1-gpio.h
new file mode 100644
index 000000000000..40e57a5ff1db
--- /dev/null
+++ b/include/dt-bindings/gpio/meson-a1-gpio.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ * Author: Qianggui Song <[email protected]>
+ */
+
+#ifndef _DT_BINDINGS_MESON_A1_GPIO_H
+#define _DT_BINDINGS_MESON_A1_GPIO_H
+
+#define GPIOP_0 0
+#define GPIOP_1 1
+#define GPIOP_2 2
+#define GPIOP_3 3
+#define GPIOP_4 4
+#define GPIOP_5 5
+#define GPIOP_6 6
+#define GPIOP_7 7
+#define GPIOP_8 8
+#define GPIOP_9 9
+#define GPIOP_10 10
+#define GPIOP_11 11
+#define GPIOP_12 12
+#define GPIOB_0 13
+#define GPIOB_1 14
+#define GPIOB_2 15
+#define GPIOB_3 16
+#define GPIOB_4 17
+#define GPIOB_5 18
+#define GPIOB_6 19
+#define GPIOX_0 20
+#define GPIOX_1 21
+#define GPIOX_2 22
+#define GPIOX_3 23
+#define GPIOX_4 24
+#define GPIOX_5 25
+#define GPIOX_6 26
+#define GPIOX_7 27
+#define GPIOX_8 28
+#define GPIOX_9 29
+#define GPIOX_10 30
+#define GPIOX_11 31
+#define GPIOX_12 32
+#define GPIOX_13 33
+#define GPIOX_14 34
+#define GPIOX_15 35
+#define GPIOX_16 36
+#define GPIOF_0 37
+#define GPIOF_1 38
+#define GPIOF_2 39
+#define GPIOF_3 40
+#define GPIOF_4 41
+#define GPIOF_5 42
+#define GPIOF_6 43
+#define GPIOF_7 44
+#define GPIOF_8 45
+#define GPIOF_9 46
+#define GPIOF_10 47
+#define GPIOF_11 48
+#define GPIOF_12 49
+#define GPIOA_0 50
+#define GPIOA_1 51
+#define GPIOA_2 52
+#define GPIOA_3 53
+#define GPIOA_4 54
+#define GPIOA_5 55
+#define GPIOA_6 56
+#define GPIOA_7 57
+#define GPIOA_8 58
+#define GPIOA_9 59
+#define GPIOA_10 60
+#define GPIOA_11 61
+
+#endif /* _DT_BINDINGS_MESON_A1_GPIO_H */
--
1.9.1
On Fri 25 Oct 2019 at 13:49, Qianggui Song <[email protected]> wrote:
> In meson_pinctrl_parse_dt, it contains two parts: reg parsing and
> SoC relative fixup for AO. Several fixups in the same code make it hard
> to maintain, so move all fixups to each SoC's callback and make
> meson_pinctrl_parse_dt just do the reg parsing, separate these two
> parts.Overview of all current Meson SoCs fixup is as below:
>
> +------+--------------------------------------+--------------------------+
> | | | |
> | SoC | EE domain | AO domain |
> +------+--------------------------------------+--------------------------+
> |m8 | parse regs: | parse regs: |
> |m8b | gpio,mux,pull,pull-enable(skip ds) | gpio,mux,pull(skip ds)|
> |gxl | fixup: | fixup: |
> |gxbb | no | pull-enable = pull |
> |axg | | |
> +------+--------------------------------------+--------------------------+
> |g12a | parse regs: | parse regs: |
> |sm1 | gpio,mux,pull,pull-enable,ds | gpio,mux,ds |
> | | fixup: | fixup: |
> | | no | pull = gpio |
> | | | pull-enable = gpio |
> +------+--------------------------------------+--------------------------+
> |a1 or | parse regs: |
> |later | gpio/mux (without ao domain) |
> |SoCs | fixup: |
> | | pull=gpio; pull-enable=gpio; ds=gpio |
> +------+-----------------------------------------------------------------+
>
> Signed-off-by: Qianggui Song <[email protected]>
> ---
> drivers/pinctrl/meson/pinctrl-meson-axg.c | 11 +++++++++++
> drivers/pinctrl/meson/pinctrl-meson-g12a.c | 9 +++++++++
> drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 11 +++++++++++
> drivers/pinctrl/meson/pinctrl-meson-gxl.c | 11 +++++++++++
> drivers/pinctrl/meson/pinctrl-meson.c | 9 +++++----
> drivers/pinctrl/meson/pinctrl-meson.h | 3 +++
> drivers/pinctrl/meson/pinctrl-meson8.c | 11 +++++++++++
> drivers/pinctrl/meson/pinctrl-meson8b.c | 11 +++++++++++
> 8 files changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg.c b/drivers/pinctrl/meson/pinctrl-meson-axg.c
> index ad502eda4afa..9c07f4423c37 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson-axg.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson-axg.c
> @@ -1040,6 +1040,16 @@
> .num_pmx_banks = ARRAY_SIZE(meson_axg_aobus_pmx_banks),
> };
>
> +static int meson_axg_aobus_parse_dt_extra(struct meson_pinctrl *pc)
> +{
> + if (!pc->reg_pull)
> + return -EINVAL;
> +
> + pc->reg_pullen = pc->reg_pull;
> +
> + return 0;
> +}
> +
> static struct meson_pinctrl_data meson_axg_periphs_pinctrl_data = {
> .name = "periphs-banks",
> .pins = meson_axg_periphs_pins,
> @@ -1066,6 +1076,7 @@
> .num_banks = ARRAY_SIZE(meson_axg_aobus_banks),
> .pmx_ops = &meson_axg_pmx_ops,
> .pmx_data = &meson_axg_aobus_pmx_banks_data,
> + .parse_dt = meson_axg_aobus_parse_dt_extra,
> };
>
> static const struct of_device_id meson_axg_pinctrl_dt_match[] = {
> diff --git a/drivers/pinctrl/meson/pinctrl-meson-g12a.c b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
> index 582665fd362a..41850e3c0091 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson-g12a.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
> @@ -1362,6 +1362,14 @@
> .num_pmx_banks = ARRAY_SIZE(meson_g12a_aobus_pmx_banks),
> };
>
> +static int meson_g12a_aobus_parse_dt_extra(struct meson_pinctrl *pc)
> +{
> + pc->reg_pull = pc->reg_gpio;
> + pc->reg_pullen = pc->reg_gpio;
> +
> + return 0;
> +}
> +
> static struct meson_pinctrl_data meson_g12a_periphs_pinctrl_data = {
> .name = "periphs-banks",
> .pins = meson_g12a_periphs_pins,
> @@ -1388,6 +1396,7 @@
> .num_banks = ARRAY_SIZE(meson_g12a_aobus_banks),
> .pmx_ops = &meson_axg_pmx_ops,
> .pmx_data = &meson_g12a_aobus_pmx_banks_data,
> + .parse_dt = meson_g12a_aobus_parse_dt_extra,
> };
>
> static const struct of_device_id meson_g12a_pinctrl_dt_match[] = {
> diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
> index 5bfa56f3847e..f5494a948200 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
> @@ -827,6 +827,16 @@
> BANK("AO", GPIOAO_0, GPIOAO_13, 0, 13, 0, 16, 0, 0, 0, 0, 0, 16, 1, 0),
> };
>
> +static int meson_gxbb_aobus_parse_dt_extra(struct meson_pinctrl *pc)
> +{
> + if (!pc->reg_pull)
> + return -EINVAL;
> +
> + pc->reg_pullen = pc->reg_pull;
> +
> + return 0;
> +}
Can you share the definition of this function instead of repeating it ?
> +
> static struct meson_pinctrl_data meson_gxbb_periphs_pinctrl_data = {
> .name = "periphs-banks",
> .pins = meson_gxbb_periphs_pins,
> @@ -851,6 +861,7 @@
> .num_funcs = ARRAY_SIZE(meson_gxbb_aobus_functions),
> .num_banks = ARRAY_SIZE(meson_gxbb_aobus_banks),
> .pmx_ops = &meson8_pmx_ops,
> + .parse_dt = meson_gxbb_aobus_parse_dt_extra,
> };
>
> static const struct of_device_id meson_gxbb_pinctrl_dt_match[] = {
> diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxl.c b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
> index 72c5373c8dc1..943fb27dab08 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson-gxl.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
> @@ -796,6 +796,16 @@
> BANK("AO", GPIOAO_0, GPIOAO_9, 0, 9, 0, 16, 0, 0, 0, 0, 0, 16, 1, 0),
> };
>
> +static int meson_gxl_aobus_parse_dt_extra(struct meson_pinctrl *pc)
> +{
> + if (!pc->reg_pull)
> + return -EINVAL;
> +
> + pc->reg_pullen = pc->reg_pull;
> +
> + return 0;
> +}
Same
> +
> static struct meson_pinctrl_data meson_gxl_periphs_pinctrl_data = {
> .name = "periphs-banks",
> .pins = meson_gxl_periphs_pins,
> @@ -820,6 +830,7 @@
> .num_funcs = ARRAY_SIZE(meson_gxl_aobus_functions),
> .num_banks = ARRAY_SIZE(meson_gxl_aobus_banks),
> .pmx_ops = &meson8_pmx_ops,
> + .parse_dt = meson_gxl_aobus_parse_dt_extra,
> };
>
> static const struct of_device_id meson_gxl_pinctrl_dt_match[] = {
> diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
> index 8bba9d053d9f..e182583422a4 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
> @@ -677,14 +677,12 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
> }
>
> pc->reg_pull = meson_map_resource(pc, gpio_np, "pull");
> - /* Use gpio region if pull one is not present */
> if (IS_ERR(pc->reg_pull))
> - pc->reg_pull = pc->reg_gpio;
> + pc->reg_pull = NULL;
Instead of doing this fixup, could modifhy meson_map_ressourse() to
return NULL instead of -ENOENT ?
Then you should IS_ERR_OR_NULL() for "mux" and "gpio" and just IS_ERR()
for the rest
>
> pc->reg_pullen = meson_map_resource(pc, gpio_np, "pull-enable");
> - /* Use pull region if pull-enable one is not present */
> if (IS_ERR(pc->reg_pullen))
> - pc->reg_pullen = pc->reg_pull;
> + pc->reg_pullen = NULL;
>
> pc->reg_ds = meson_map_resource(pc, gpio_np, "ds");
> if (IS_ERR(pc->reg_ds)) {
> @@ -692,6 +690,9 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
> pc->reg_ds = NULL;
> }
>
> + if (pc->data->parse_dt)
> + return pc->data->parse_dt(pc);
> +
> return 0;
> }
>
> diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
> index c696f3241a36..d570f7c53045 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson.h
> +++ b/drivers/pinctrl/meson/pinctrl-meson.h
> @@ -11,6 +11,8 @@
> #include <linux/regmap.h>
> #include <linux/types.h>
>
> +struct meson_pinctrl;
> +
> /**
> * struct meson_pmx_group - a pinmux group
> *
> @@ -114,6 +116,7 @@ struct meson_pinctrl_data {
> unsigned int num_banks;
> const struct pinmux_ops *pmx_ops;
> void *pmx_data;
> + int (*parse_dt)(struct meson_pinctrl *pc);
> };
>
> struct meson_pinctrl {
> diff --git a/drivers/pinctrl/meson/pinctrl-meson8.c b/drivers/pinctrl/meson/pinctrl-meson8.c
> index 0b97befa6335..65c70c9b7070 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson8.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson8.c
> @@ -1079,6 +1079,16 @@
> BANK("AO", GPIOAO_0, GPIO_TEST_N, 0, 13, 0, 16, 0, 0, 0, 0, 0, 16, 1, 0),
> };
>
> +static int meson8_aobus_parse_dt_extra(struct meson_pinctrl *pc)
> +{
> + if (!pc->reg_pull)
> + return -EINVAL;
> +
> + pc->reg_pullen = pc->reg_pull;
> +
> + return 0;
> +}
> +
> static struct meson_pinctrl_data meson8_cbus_pinctrl_data = {
> .name = "cbus-banks",
> .pins = meson8_cbus_pins,
> @@ -1103,6 +1113,7 @@
> .num_funcs = ARRAY_SIZE(meson8_aobus_functions),
> .num_banks = ARRAY_SIZE(meson8_aobus_banks),
> .pmx_ops = &meson8_pmx_ops,
> + .parse_dt = &meson8_aobus_parse_dt_extra,
> };
>
> static const struct of_device_id meson8_pinctrl_dt_match[] = {
> diff --git a/drivers/pinctrl/meson/pinctrl-meson8b.c b/drivers/pinctrl/meson/pinctrl-meson8b.c
> index a7de388388e6..85dc12e0c839 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson8b.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson8b.c
> @@ -938,6 +938,16 @@
> BANK("AO", GPIOAO_0, GPIO_TEST_N, 0, 13, 0, 16, 0, 0, 0, 0, 0, 16, 1, 0),
> };
>
> +static int meson8b_aobus_parse_dt_extra(struct meson_pinctrl *pc)
> +{
> + if (!pc->reg_pull)
> + return -EINVAL;
> +
> + pc->reg_pullen = pc->reg_pull;
> +
> + return 0;
> +}
> +
> static struct meson_pinctrl_data meson8b_cbus_pinctrl_data = {
> .name = "cbus-banks",
> .pins = meson8b_cbus_pins,
> @@ -962,6 +972,7 @@
> .num_funcs = ARRAY_SIZE(meson8b_aobus_functions),
> .num_banks = ARRAY_SIZE(meson8b_aobus_banks),
> .pmx_ops = &meson8_pmx_ops,
> + .parse_dt = &meson8b_aobus_parse_dt_extra,
> };
>
> static const struct of_device_id meson8b_pinctrl_dt_match[] = {
On Fri, Oct 25, 2019 at 1:49 PM Qianggui Song <[email protected]> wrote:
> Add new compatible name for Amlogic's Meson-A1 pin controller
> add a dt-binding header file which document the detail pin names.
> Note that A1 doesn't need DS bank reg any more, use gpio reg as
> base.
>
> Reviewed-by: Rob Herring <[email protected]>
> Reviewed-by: Neil Armstrong <[email protected]>
> Signed-off-by: Qianggui Song <[email protected]>
Patch applied.
Yours,
Linus Walleij
On Mon, Nov 4, 2019 at 4:31 PM Linus Walleij <[email protected]> wrote:
> On Fri, Oct 25, 2019 at 1:49 PM Qianggui Song <[email protected]> wrote:
>
> > Add new compatible name for Amlogic's Meson-A1 pin controller
> > add a dt-binding header file which document the detail pin names.
> > Note that A1 doesn't need DS bank reg any more, use gpio reg as
> > base.
> >
> > Reviewed-by: Rob Herring <[email protected]>
> > Reviewed-by: Neil Armstrong <[email protected]>
> > Signed-off-by: Qianggui Song <[email protected]>
>
> Patch applied.
I see there are still comments on the code patches but that is
fine, we can merge these bindings anyway. Just resend the
remaining 3 patches when you repost.
Yours,
Linus Walleij
On 2019/11/4 15:59, Jerome Brunet wrote:
>
> On Fri 25 Oct 2019 at 13:49, Qianggui Song <[email protected]> wrote:
>
>> In meson_pinctrl_parse_dt, it contains two parts: reg parsing and
>> SoC relative fixup for AO. Several fixups in the same code make it hard
>> to maintain, so move all fixups to each SoC's callback and make
>> meson_pinctrl_parse_dt just do the reg parsing, separate these two
>> parts.Overview of all current Meson SoCs fixup is as below:
>>
>> +------+--------------------------------------+--------------------------+
>> | | | |
>> | SoC | EE domain | AO domain |
>> +------+--------------------------------------+--------------------------+
>> |m8 | parse regs: | parse regs: |
>> |m8b | gpio,mux,pull,pull-enable(skip ds) | gpio,mux,pull(skip ds)|
>> |gxl | fixup: | fixup: |
>> |gxbb | no | pull-enable = pull |
>> |axg | | |
>> +------+--------------------------------------+--------------------------+
>> |g12a | parse regs: | parse regs: |
>> |sm1 | gpio,mux,pull,pull-enable,ds | gpio,mux,ds |
>> | | fixup: | fixup: |
>> | | no | pull = gpio |
>> | | | pull-enable = gpio |
>> +------+--------------------------------------+--------------------------+
>> |a1 or | parse regs: |
>> |later | gpio/mux (without ao domain) |
>> |SoCs | fixup: |
>> | | pull=gpio; pull-enable=gpio; ds=gpio |
>> +------+-----------------------------------------------------------------+
>>
>> Signed-off-by: Qianggui Song <[email protected]>
>> ---
>> drivers/pinctrl/meson/pinctrl-meson-axg.c | 11 +++++++++++
>> drivers/pinctrl/meson/pinctrl-meson-g12a.c | 9 +++++++++
>> drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 11 +++++++++++
>> drivers/pinctrl/meson/pinctrl-meson-gxl.c | 11 +++++++++++
>> drivers/pinctrl/meson/pinctrl-meson.c | 9 +++++----
>> drivers/pinctrl/meson/pinctrl-meson.h | 3 +++
>> drivers/pinctrl/meson/pinctrl-meson8.c | 11 +++++++++++
>> drivers/pinctrl/meson/pinctrl-meson8b.c | 11 +++++++++++
>> 8 files changed, 72 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg.c b/drivers/pinctrl/meson/pinctrl-meson-axg.c
>> index ad502eda4afa..9c07f4423c37 100644
>> --- a/drivers/pinctrl/meson/pinctrl-meson-axg.c
>> +++ b/drivers/pinctrl/meson/pinctrl-meson-axg.c
>> @@ -1040,6 +1040,16 @@
>> .num_pmx_banks = ARRAY_SIZE(meson_axg_aobus_pmx_banks),
>> };
>>
>> +static int meson_axg_aobus_parse_dt_extra(struct meson_pinctrl *pc)
>> +{
>> + if (!pc->reg_pull)
>> + return -EINVAL;
>> +
>> + pc->reg_pullen = pc->reg_pull;
>> +
>> + return 0;
>> +}
>> +
>> static struct meson_pinctrl_data meson_axg_periphs_pinctrl_data = {
>> .name = "periphs-banks",
>> .pins = meson_axg_periphs_pins,
>> @@ -1066,6 +1076,7 @@
>> .num_banks = ARRAY_SIZE(meson_axg_aobus_banks),
>> .pmx_ops = &meson_axg_pmx_ops,
>> .pmx_data = &meson_axg_aobus_pmx_banks_data,
>> + .parse_dt = meson_axg_aobus_parse_dt_extra,
>> };
>>
>> static const struct of_device_id meson_axg_pinctrl_dt_match[] = {
>> diff --git a/drivers/pinctrl/meson/pinctrl-meson-g12a.c b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
>> index 582665fd362a..41850e3c0091 100644
>> --- a/drivers/pinctrl/meson/pinctrl-meson-g12a.c
>> +++ b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
>> @@ -1362,6 +1362,14 @@
>> .num_pmx_banks = ARRAY_SIZE(meson_g12a_aobus_pmx_banks),
>> };
>>
>> +static int meson_g12a_aobus_parse_dt_extra(struct meson_pinctrl *pc)
>> +{
>> + pc->reg_pull = pc->reg_gpio;
>> + pc->reg_pullen = pc->reg_gpio;
>> +
>> + return 0;
>> +}
>> +
>> static struct meson_pinctrl_data meson_g12a_periphs_pinctrl_data = {
>> .name = "periphs-banks",
>> .pins = meson_g12a_periphs_pins,
>> @@ -1388,6 +1396,7 @@
>> .num_banks = ARRAY_SIZE(meson_g12a_aobus_banks),
>> .pmx_ops = &meson_axg_pmx_ops,
>> .pmx_data = &meson_g12a_aobus_pmx_banks_data,
>> + .parse_dt = meson_g12a_aobus_parse_dt_extra,
>> };
>>
>> static const struct of_device_id meson_g12a_pinctrl_dt_match[] = {
>> diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
>> index 5bfa56f3847e..f5494a948200 100644
>> --- a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
>> +++ b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
>> @@ -827,6 +827,16 @@
>> BANK("AO", GPIOAO_0, GPIOAO_13, 0, 13, 0, 16, 0, 0, 0, 0, 0, 16, 1, 0),
>> };
>>
>> +static int meson_gxbb_aobus_parse_dt_extra(struct meson_pinctrl *pc)
>> +{
>> + if (!pc->reg_pull)
>> + return -EINVAL;
>> +
>> + pc->reg_pullen = pc->reg_pull;
>> +
>> + return 0;
>> +}
>
> Can you share the definition of this function instead of repeating it ?
>
Seems like this common function can only be placed in pinctrl-meson.c, I
will try to write a meson_aobus_parse_dt_extra in pinctrl-meson.c and
assign it to each SoCs driver before g12a.
>> +
>> static struct meson_pinctrl_data meson_gxbb_periphs_pinctrl_data = {
>> .name = "periphs-banks",
>> .pins = meson_gxbb_periphs_pins,
>> @@ -851,6 +861,7 @@
>> .num_funcs = ARRAY_SIZE(meson_gxbb_aobus_functions),
>> .num_banks = ARRAY_SIZE(meson_gxbb_aobus_banks),
>> .pmx_ops = &meson8_pmx_ops,
>> + .parse_dt = meson_gxbb_aobus_parse_dt_extra,
>> };
>>
>> static const struct of_device_id meson_gxbb_pinctrl_dt_match[] = {
>> diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxl.c b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
>> index 72c5373c8dc1..943fb27dab08 100644
>> --- a/drivers/pinctrl/meson/pinctrl-meson-gxl.c
>> +++ b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
>> @@ -796,6 +796,16 @@
>> BANK("AO", GPIOAO_0, GPIOAO_9, 0, 9, 0, 16, 0, 0, 0, 0, 0, 16, 1, 0),
>> };
>>
>> +static int meson_gxl_aobus_parse_dt_extra(struct meson_pinctrl *pc)
>> +{
>> + if (!pc->reg_pull)
>> + return -EINVAL;
>> +
>> + pc->reg_pullen = pc->reg_pull;
>> +
>> + return 0;
>> +}
>
> Same
will do it for above and below other platform.
>
>> +
>> static struct meson_pinctrl_data meson_gxl_periphs_pinctrl_data = {
>> .name = "periphs-banks",
>> .pins = meson_gxl_periphs_pins,
>> @@ -820,6 +830,7 @@
>> .num_funcs = ARRAY_SIZE(meson_gxl_aobus_functions),
>> .num_banks = ARRAY_SIZE(meson_gxl_aobus_banks),
>> .pmx_ops = &meson8_pmx_ops,
>> + .parse_dt = meson_gxl_aobus_parse_dt_extra,
>> };
>>
>> static const struct of_device_id meson_gxl_pinctrl_dt_match[] = {
>> diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
>> index 8bba9d053d9f..e182583422a4 100644
>> --- a/drivers/pinctrl/meson/pinctrl-meson.c
>> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
>> @@ -677,14 +677,12 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
>> }
>>
>> pc->reg_pull = meson_map_resource(pc, gpio_np, "pull");
>> - /* Use gpio region if pull one is not present */
>> if (IS_ERR(pc->reg_pull))
>> - pc->reg_pull = pc->reg_gpio;
>> + pc->reg_pull = NULL;
>
> Instead of doing this fixup, could modifhy meson_map_ressourse() to
> return NULL instead of -ENOENT ?
>
> Then you should IS_ERR_OR_NULL() for "mux" and "gpio" and just IS_ERR()
> for the rest
>
Ok, will modify this in the next patch set
>>
>> pc->reg_pullen = meson_map_resource(pc, gpio_np, "pull-enable");
>> - /* Use pull region if pull-enable one is not present */
>> if (IS_ERR(pc->reg_pullen))
>> - pc->reg_pullen = pc->reg_pull;
>> + pc->reg_pullen = NULL;
>>
>> pc->reg_ds = meson_map_resource(pc, gpio_np, "ds");
>> if (IS_ERR(pc->reg_ds)) {
>> @@ -692,6 +690,9 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
>> pc->reg_ds = NULL;
>> }
>>
>> + if (pc->data->parse_dt)
>> + return pc->data->parse_dt(pc);
>> +
>> return 0;
>> }
>>
>> diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
>> index c696f3241a36..d570f7c53045 100644
>> --- a/drivers/pinctrl/meson/pinctrl-meson.h
>> +++ b/drivers/pinctrl/meson/pinctrl-meson.h
>> @@ -11,6 +11,8 @@
>> #include <linux/regmap.h>
>> #include <linux/types.h>
>>
>> +struct meson_pinctrl;
>> +
>> /**
>> * struct meson_pmx_group - a pinmux group
>> *
>> @@ -114,6 +116,7 @@ struct meson_pinctrl_data {
>> unsigned int num_banks;
>> const struct pinmux_ops *pmx_ops;
>> void *pmx_data;
>> + int (*parse_dt)(struct meson_pinctrl *pc);
>> };
>>
>> struct meson_pinctrl {
>> diff --git a/drivers/pinctrl/meson/pinctrl-meson8.c b/drivers/pinctrl/meson/pinctrl-meson8.c
>> index 0b97befa6335..65c70c9b7070 100644
>> --- a/drivers/pinctrl/meson/pinctrl-meson8.c
>> +++ b/drivers/pinctrl/meson/pinctrl-meson8.c
>> @@ -1079,6 +1079,16 @@
>> BANK("AO", GPIOAO_0, GPIO_TEST_N, 0, 13, 0, 16, 0, 0, 0, 0, 0, 16, 1, 0),
>> };
>>
>> +static int meson8_aobus_parse_dt_extra(struct meson_pinctrl *pc)
>> +{
>> + if (!pc->reg_pull)
>> + return -EINVAL;
>> +
>> + pc->reg_pullen = pc->reg_pull;
>> +
>> + return 0;
>> +}
>> +
>> static struct meson_pinctrl_data meson8_cbus_pinctrl_data = {
>> .name = "cbus-banks",
>> .pins = meson8_cbus_pins,
>> @@ -1103,6 +1113,7 @@
>> .num_funcs = ARRAY_SIZE(meson8_aobus_functions),
>> .num_banks = ARRAY_SIZE(meson8_aobus_banks),
>> .pmx_ops = &meson8_pmx_ops,
>> + .parse_dt = &meson8_aobus_parse_dt_extra,
>> };
>>
>> static const struct of_device_id meson8_pinctrl_dt_match[] = {
>> diff --git a/drivers/pinctrl/meson/pinctrl-meson8b.c b/drivers/pinctrl/meson/pinctrl-meson8b.c
>> index a7de388388e6..85dc12e0c839 100644
>> --- a/drivers/pinctrl/meson/pinctrl-meson8b.c
>> +++ b/drivers/pinctrl/meson/pinctrl-meson8b.c
>> @@ -938,6 +938,16 @@
>> BANK("AO", GPIOAO_0, GPIO_TEST_N, 0, 13, 0, 16, 0, 0, 0, 0, 0, 16, 1, 0),
>> };
>>
>> +static int meson8b_aobus_parse_dt_extra(struct meson_pinctrl *pc)
>> +{
>> + if (!pc->reg_pull)
>> + return -EINVAL;
>> +
>> + pc->reg_pullen = pc->reg_pull;
>> +
>> + return 0;
>> +}
>> +
>> static struct meson_pinctrl_data meson8b_cbus_pinctrl_data = {
>> .name = "cbus-banks",
>> .pins = meson8b_cbus_pins,
>> @@ -962,6 +972,7 @@
>> .num_funcs = ARRAY_SIZE(meson8b_aobus_functions),
>> .num_banks = ARRAY_SIZE(meson8b_aobus_banks),
>> .pmx_ops = &meson8_pmx_ops,
>> + .parse_dt = &meson8b_aobus_parse_dt_extra,
>> };
>>
>> static const struct of_device_id meson8b_pinctrl_dt_match[] = {
>
> .
>