* [V2 PATCH 0/3] Fix the memory layout and add sgpio node for aspeed g6
@ 2020-10-12 3:31 Billy Tsai
2020-10-12 3:31 ` [PATCH 1/3] Arm: dts: aspeed-g6: Fix the register range of gpio Billy Tsai
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Billy Tsai @ 2020-10-12 3:31 UTC (permalink / raw)
To: robh+dt, joel, andrew, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel, linus.walleij, bgolaszewski,
linux-gpio, openbmc
Cc: BMC-SW
This patch series is used to add sgpiom and sgpios nodes and add pinctrl
setting for sgpiom1
v2:
- Split the change of dts and pinctrl to two commit.
- Add the compatible string for aspeed,ast2600-sgpiom.
aspeed,ast2600-sgpios will implement in the future.
Billy Tsai (3):
Arm: dts: aspeed-g6: Fix the register range of gpio
Arm: dts: aspeed-g6: Add sgpio node
pinctrl: aspeed-g6: Add sgpiom2 pinctrl setting
.../devicetree/bindings/gpio/sgpio-aspeed.txt | 8 +--
arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi | 5 ++
arch/arm/boot/dts/aspeed-g6.dtsi | 54 ++++++++++++++++++-
drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 30 +++++++++--
4 files changed, 89 insertions(+), 8 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] Arm: dts: aspeed-g6: Fix the register range of gpio
2020-10-12 3:31 [V2 PATCH 0/3] Fix the memory layout and add sgpio node for aspeed g6 Billy Tsai
@ 2020-10-12 3:31 ` Billy Tsai
2020-10-12 4:30 ` Joel Stanley
2020-10-26 1:05 ` Andrew Jeffery
2020-10-12 3:31 ` [PATCH 2/3] Arm: dts: aspeed-g6: Add sgpio node Billy Tsai
2020-10-12 3:31 ` [PATCH 3/3] pinctrl: aspeed-g6: Add sgpiom2 pinctrl setting Billy Tsai
2 siblings, 2 replies; 16+ messages in thread
From: Billy Tsai @ 2020-10-12 3:31 UTC (permalink / raw)
To: robh+dt, joel, andrew, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel, linus.walleij, bgolaszewski,
linux-gpio, openbmc
Cc: BMC-SW
This patch is used to fix the memory range of gpio0
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
arch/arm/boot/dts/aspeed-g6.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index 97ca743363d7..ad19dce038ea 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -357,7 +357,7 @@
#gpio-cells = <2>;
gpio-controller;
compatible = "aspeed,ast2600-gpio";
- reg = <0x1e780000 0x800>;
+ reg = <0x1e780000 0x400>;
interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
gpio-ranges = <&pinctrl 0 0 208>;
ngpios = <208>;
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] Arm: dts: aspeed-g6: Add sgpio node
2020-10-12 3:31 [V2 PATCH 0/3] Fix the memory layout and add sgpio node for aspeed g6 Billy Tsai
2020-10-12 3:31 ` [PATCH 1/3] Arm: dts: aspeed-g6: Fix the register range of gpio Billy Tsai
@ 2020-10-12 3:31 ` Billy Tsai
2020-10-12 4:35 ` Joel Stanley
2020-10-12 3:31 ` [PATCH 3/3] pinctrl: aspeed-g6: Add sgpiom2 pinctrl setting Billy Tsai
2 siblings, 1 reply; 16+ messages in thread
From: Billy Tsai @ 2020-10-12 3:31 UTC (permalink / raw)
To: robh+dt, joel, andrew, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel, linus.walleij, bgolaszewski,
linux-gpio, openbmc
Cc: BMC-SW
This patch is used to add sgpiom and sgpios nodes and add compatiable
string for sgpiom.
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
.../devicetree/bindings/gpio/sgpio-aspeed.txt | 8 +--
arch/arm/boot/dts/aspeed-g6.dtsi | 52 +++++++++++++++++++
2 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
index d4d83916c09d..815d9b5167a5 100644
--- a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
+++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
@@ -1,8 +1,10 @@
Aspeed SGPIO controller Device Tree Bindings
--------------------------------------------
-This SGPIO controller is for ASPEED AST2500 SoC, it supports up to 80 full
-featured Serial GPIOs. Each of the Serial GPIO pins can be programmed to
+This SGPIO controller is for ASPEED AST2500/AST2600 SoC, it supports 2 master.
+One is up to 128 SGPIO input ports and 128 output ports concurrently(after AST2600A1)
+and Second one is up to 80.
+Each of the Serial GPIO pins can be programmed to
support the following options:
- Support interrupt option for each input port and various interrupt
sensitivity option (level-high, level-low, edge-high, edge-low)
@@ -14,7 +16,7 @@ support the following options:
Required properties:
- compatible : Should be one of
- "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio"
+ "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio", "aspeed,ast2600-sgpiom"
- #gpio-cells : Should be 2, see gpio.txt
- reg : Address and length of the register set for the device
- gpio-controller : Marks the device node as a GPIO controller
diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index ad19dce038ea..cb053a996e87 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -366,6 +366,58 @@
#interrupt-cells = <2>;
};
+ sgpiom0: sgpiom@1e780500 {
+ #gpio-cells = <2>;
+ gpio-controller;
+ compatible = "aspeed,ast2600-sgpiom";
+ reg = <0x1e780500 0x100>;
+ interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
+ ngpios = <128>;
+ clocks = <&syscon ASPEED_CLK_APB2>;
+ interrupt-controller;
+ bus-frequency = <12000000>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sgpm1_default>;
+ status = "disabled";
+ };
+
+ sgpiom1: sgpiom@1e780600 {
+ #gpio-cells = <2>;
+ gpio-controller;
+ compatible = "aspeed,ast2600-sgpiom";
+ reg = <0x1e780600 0x100>;
+ interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>;
+ ngpios = <80>;
+ clocks = <&syscon ASPEED_CLK_APB2>;
+ interrupt-controller;
+ bus-frequency = <12000000>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sgpm2_default>;
+ status = "disabled";
+ };
+
+ sgpios0: sgpios@1e780700 {
+ #gpio-cells = <2>;
+ gpio-controller;
+ compatible = "aspeed,ast2600-sgpios";
+ reg = <0x1e780700 0x40>;
+ interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&syscon ASPEED_CLK_APB2>;
+ status = "disabled";
+ };
+
+ sgpios1: sgpios@1e780740 {
+ #gpio-cells = <2>;
+ gpio-controller;
+ compatible = "aspeed,ast2600-sgpios";
+ reg = <0x1e780740 0x40>;
+ interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&syscon ASPEED_CLK_APB2>;
+ status = "disabled";
+ };
+
gpio1: gpio@1e780800 {
#gpio-cells = <2>;
gpio-controller;
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] pinctrl: aspeed-g6: Add sgpiom2 pinctrl setting
2020-10-12 3:31 [V2 PATCH 0/3] Fix the memory layout and add sgpio node for aspeed g6 Billy Tsai
2020-10-12 3:31 ` [PATCH 1/3] Arm: dts: aspeed-g6: Fix the register range of gpio Billy Tsai
2020-10-12 3:31 ` [PATCH 2/3] Arm: dts: aspeed-g6: Add sgpio node Billy Tsai
@ 2020-10-12 3:31 ` Billy Tsai
2020-10-12 4:36 ` Joel Stanley
2020-10-26 1:26 ` Andrew Jeffery
2 siblings, 2 replies; 16+ messages in thread
From: Billy Tsai @ 2020-10-12 3:31 UTC (permalink / raw)
To: robh+dt, joel, andrew, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel, linus.walleij, bgolaszewski,
linux-gpio, openbmc
Cc: BMC-SW
At ast2600a1 we change feature of master sgpio to 2 sets.
So this patch is used to add the pinctrl setting of the new sgpio.
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi | 5 ++++
drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 30 +++++++++++++++++++---
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi b/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
index 7028e21bdd98..a16ecf08e307 100644
--- a/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
@@ -862,6 +862,11 @@
groups = "SGPM1";
};
+ pinctrl_sgpm2_default: sgpm2_default {
+ function = "SGPM2";
+ groups = "SGPM2";
+ };
+
pinctrl_sgps1_default: sgps1_default {
function = "SGPS1";
groups = "SGPS1";
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
index 34803a6c7664..b673a44ffa3b 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
@@ -46,8 +46,10 @@
#define SCU620 0x620 /* Disable GPIO Internal Pull-Down #4 */
#define SCU634 0x634 /* Disable GPIO Internal Pull-Down #5 */
#define SCU638 0x638 /* Disable GPIO Internal Pull-Down #6 */
+#define SCU690 0x690 /* Multi-function Pin Control #24 */
#define SCU694 0x694 /* Multi-function Pin Control #25 */
#define SCU69C 0x69C /* Multi-function Pin Control #27 */
+#define SCU6D0 0x6D0 /* Multi-function Pin Control #28 */
#define SCUC20 0xC20 /* PCIE configuration Setting Control */
#define ASPEED_G6_NR_PINS 256
@@ -81,13 +83,21 @@ FUNC_GROUP_DECL(I2C12, L26, K24);
#define K26 4
SIG_EXPR_LIST_DECL_SESG(K26, MACLINK1, MACLINK1, SIG_DESC_SET(SCU410, 4));
SIG_EXPR_LIST_DECL_SESG(K26, SCL13, I2C13, SIG_DESC_SET(SCU4B0, 4));
-PIN_DECL_2(K26, GPIOA4, MACLINK1, SCL13);
+/*SGPM2 is A1 Only */
+SIG_EXPR_LIST_DECL_SESG(K26, SGPM2CLK, SGPM2, SIG_DESC_SET(SCU6D0, 4),
+ SIG_DESC_CLEAR(SCU410, 4), SIG_DESC_CLEAR(SCU4B0, 4),
+ SIG_DESC_CLEAR(SCU690, 4));
+PIN_DECL_3(K26, GPIOA4, SGPM2CLK, MACLINK1, SCL13);
FUNC_GROUP_DECL(MACLINK1, K26);
#define L24 5
SIG_EXPR_LIST_DECL_SESG(L24, MACLINK2, MACLINK2, SIG_DESC_SET(SCU410, 5));
SIG_EXPR_LIST_DECL_SESG(L24, SDA13, I2C13, SIG_DESC_SET(SCU4B0, 5));
-PIN_DECL_2(L24, GPIOA5, MACLINK2, SDA13);
+/*SGPM2 is A1 Only */
+SIG_EXPR_LIST_DECL_SESG(L24, SGPM2LD, SGPM2, SIG_DESC_SET(SCU6D0, 5),
+ SIG_DESC_CLEAR(SCU410, 5), SIG_DESC_CLEAR(SCU4B0, 5),
+ SIG_DESC_CLEAR(SCU690, 5));
+PIN_DECL_3(L24, GPIOA5, SGPM2LD, MACLINK2, SDA13);
FUNC_GROUP_DECL(MACLINK2, L24);
FUNC_GROUP_DECL(I2C13, K26, L24);
@@ -95,16 +105,26 @@ FUNC_GROUP_DECL(I2C13, K26, L24);
#define L23 6
SIG_EXPR_LIST_DECL_SESG(L23, MACLINK3, MACLINK3, SIG_DESC_SET(SCU410, 6));
SIG_EXPR_LIST_DECL_SESG(L23, SCL14, I2C14, SIG_DESC_SET(SCU4B0, 6));
-PIN_DECL_2(L23, GPIOA6, MACLINK3, SCL14);
+/*SGPM2 is A1 Only */
+SIG_EXPR_LIST_DECL_SESG(L23, SGPM2O, SGPM2, SIG_DESC_SET(SCU6D0, 6),
+ SIG_DESC_CLEAR(SCU410, 6), SIG_DESC_CLEAR(SCU4B0, 6),
+ SIG_DESC_CLEAR(SCU690, 6));
+PIN_DECL_3(L23, GPIOA6, SGPM2O, MACLINK3, SCL14);
FUNC_GROUP_DECL(MACLINK3, L23);
#define K25 7
SIG_EXPR_LIST_DECL_SESG(K25, MACLINK4, MACLINK4, SIG_DESC_SET(SCU410, 7));
SIG_EXPR_LIST_DECL_SESG(K25, SDA14, I2C14, SIG_DESC_SET(SCU4B0, 7));
-PIN_DECL_2(K25, GPIOA7, MACLINK4, SDA14);
+/*SGPM2 is A1 Only */
+SIG_EXPR_LIST_DECL_SESG(K25, SGPM2I, SGPM2, SIG_DESC_SET(SCU6D0, 7),
+ SIG_DESC_CLEAR(SCU410, 7), SIG_DESC_CLEAR(SCU4B0, 7),
+ SIG_DESC_CLEAR(SCU690, 7));
+PIN_DECL_3(K25, GPIOA7, SGPM2I, MACLINK4, SDA14);
FUNC_GROUP_DECL(MACLINK4, K25);
FUNC_GROUP_DECL(I2C14, L23, K25);
+/*SGPM2 is A1 Only */
+FUNC_GROUP_DECL(SGPM2, K26, L24, L23, K25);
#define J26 8
SIG_EXPR_LIST_DECL_SESG(J26, SALT1, SALT1, SIG_DESC_SET(SCU410, 8));
@@ -2060,6 +2080,7 @@ static const struct aspeed_pin_group aspeed_g6_groups[] = {
ASPEED_PINCTRL_GROUP(EMMCG4),
ASPEED_PINCTRL_GROUP(EMMCG8),
ASPEED_PINCTRL_GROUP(SGPM1),
+ ASPEED_PINCTRL_GROUP(SGPM2),
ASPEED_PINCTRL_GROUP(SGPS1),
ASPEED_PINCTRL_GROUP(SIOONCTRL),
ASPEED_PINCTRL_GROUP(SIOPBI),
@@ -2276,6 +2297,7 @@ static const struct aspeed_pin_function aspeed_g6_functions[] = {
ASPEED_PINCTRL_FUNC(SD1),
ASPEED_PINCTRL_FUNC(SD2),
ASPEED_PINCTRL_FUNC(SGPM1),
+ ASPEED_PINCTRL_FUNC(SGPM2),
ASPEED_PINCTRL_FUNC(SGPS1),
ASPEED_PINCTRL_FUNC(SIOONCTRL),
ASPEED_PINCTRL_FUNC(SIOPBI),
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] Arm: dts: aspeed-g6: Fix the register range of gpio
2020-10-12 3:31 ` [PATCH 1/3] Arm: dts: aspeed-g6: Fix the register range of gpio Billy Tsai
@ 2020-10-12 4:30 ` Joel Stanley
2020-10-26 1:05 ` Andrew Jeffery
1 sibling, 0 replies; 16+ messages in thread
From: Joel Stanley @ 2020-10-12 4:30 UTC (permalink / raw)
To: Billy Tsai
Cc: devicetree, linux-aspeed, open list:GPIO SUBSYSTEM,
Andrew Jeffery, Linus Walleij, Linux Kernel Mailing List,
Bartosz Golaszewski, Rob Herring, BMC-SW, OpenBMC Maillist,
Linux ARM
On Mon, 12 Oct 2020 at 03:32, Billy Tsai <billy_tsai@aspeedtech.com> wrote:
>
> This patch is used to fix the memory range of gpio0
>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> arch/arm/boot/dts/aspeed-g6.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
> index 97ca743363d7..ad19dce038ea 100644
> --- a/arch/arm/boot/dts/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
> @@ -357,7 +357,7 @@
> #gpio-cells = <2>;
> gpio-controller;
> compatible = "aspeed,ast2600-gpio";
> - reg = <0x1e780000 0x800>;
> + reg = <0x1e780000 0x400>;
> interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> gpio-ranges = <&pinctrl 0 0 208>;
> ngpios = <208>;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] Arm: dts: aspeed-g6: Add sgpio node
2020-10-12 3:31 ` [PATCH 2/3] Arm: dts: aspeed-g6: Add sgpio node Billy Tsai
@ 2020-10-12 4:35 ` Joel Stanley
2020-10-12 4:53 ` Billy Tsai
2020-10-26 1:33 ` Andrew Jeffery
0 siblings, 2 replies; 16+ messages in thread
From: Joel Stanley @ 2020-10-12 4:35 UTC (permalink / raw)
To: Billy Tsai
Cc: devicetree, linux-aspeed, open list:GPIO SUBSYSTEM,
Andrew Jeffery, Linus Walleij, Linux Kernel Mailing List,
Bartosz Golaszewski, Rob Herring, BMC-SW, OpenBMC Maillist,
Linux ARM
On Mon, 12 Oct 2020 at 03:32, Billy Tsai <billy_tsai@aspeedtech.com> wrote:
>
> This patch is used to add sgpiom and sgpios nodes and add compatiable
> string for sgpiom.
You also need to add sgpios documentation to the bindings docs.
Whenever you add new device tree bindings to the kernel tree you
should add documentation for them.
When preparing patches for submission, use scripts/checkpatch.pl to
check for common issues. It will warn you if you are adding strings
that are not documented.
Cheers,
Joel
>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
> .../devicetree/bindings/gpio/sgpio-aspeed.txt | 8 +--
> arch/arm/boot/dts/aspeed-g6.dtsi | 52 +++++++++++++++++++
> 2 files changed, 57 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> index d4d83916c09d..815d9b5167a5 100644
> --- a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> +++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> @@ -1,8 +1,10 @@
> Aspeed SGPIO controller Device Tree Bindings
> --------------------------------------------
>
> -This SGPIO controller is for ASPEED AST2500 SoC, it supports up to 80 full
> -featured Serial GPIOs. Each of the Serial GPIO pins can be programmed to
> +This SGPIO controller is for ASPEED AST2500/AST2600 SoC, it supports 2 master.
> +One is up to 128 SGPIO input ports and 128 output ports concurrently(after AST2600A1)
> +and Second one is up to 80.
> +Each of the Serial GPIO pins can be programmed to
> support the following options:
> - Support interrupt option for each input port and various interrupt
> sensitivity option (level-high, level-low, edge-high, edge-low)
> @@ -14,7 +16,7 @@ support the following options:
> Required properties:
>
> - compatible : Should be one of
> - "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio"
> + "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio", "aspeed,ast2600-sgpiom"
I think we should add sgpiom strings for the ast2500 (and ast2400?)
too, as this is how they should have been named in the first place:
> - compatible : Should be one of
> "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio"
> "aspeed,ast2400-sgpiom", "aspeed,ast2500-sgpiom", "aspeed,ast2600-sgpiom"
> - #gpio-cells : Should be 2, see gpio.txt
> - reg : Address and length of the register set for the device
> - gpio-controller : Marks the device node as a GPIO controller
> diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
> index ad19dce038ea..cb053a996e87 100644
> --- a/arch/arm/boot/dts/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
> @@ -366,6 +366,58 @@
> #interrupt-cells = <2>;
> };
>
> + sgpiom0: sgpiom@1e780500 {
> + #gpio-cells = <2>;
> + gpio-controller;
> + compatible = "aspeed,ast2600-sgpiom";
> + reg = <0x1e780500 0x100>;
> + interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
> + ngpios = <128>;
> + clocks = <&syscon ASPEED_CLK_APB2>;
> + interrupt-controller;
> + bus-frequency = <12000000>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_sgpm1_default>;
> + status = "disabled";
> + };
> +
> + sgpiom1: sgpiom@1e780600 {
> + #gpio-cells = <2>;
> + gpio-controller;
> + compatible = "aspeed,ast2600-sgpiom";
> + reg = <0x1e780600 0x100>;
> + interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>;
> + ngpios = <80>;
> + clocks = <&syscon ASPEED_CLK_APB2>;
> + interrupt-controller;
> + bus-frequency = <12000000>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_sgpm2_default>;
> + status = "disabled";
> + };
> +
> + sgpios0: sgpios@1e780700 {
> + #gpio-cells = <2>;
> + gpio-controller;
> + compatible = "aspeed,ast2600-sgpios";
> + reg = <0x1e780700 0x40>;
> + interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&syscon ASPEED_CLK_APB2>;
> + status = "disabled";
> + };
> +
> + sgpios1: sgpios@1e780740 {
> + #gpio-cells = <2>;
> + gpio-controller;
> + compatible = "aspeed,ast2600-sgpios";
> + reg = <0x1e780740 0x40>;
> + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&syscon ASPEED_CLK_APB2>;
> + status = "disabled";
> + };
> +
> gpio1: gpio@1e780800 {
> #gpio-cells = <2>;
> gpio-controller;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] pinctrl: aspeed-g6: Add sgpiom2 pinctrl setting
2020-10-12 3:31 ` [PATCH 3/3] pinctrl: aspeed-g6: Add sgpiom2 pinctrl setting Billy Tsai
@ 2020-10-12 4:36 ` Joel Stanley
2020-10-26 1:26 ` Andrew Jeffery
1 sibling, 0 replies; 16+ messages in thread
From: Joel Stanley @ 2020-10-12 4:36 UTC (permalink / raw)
To: Billy Tsai, Linus Walleij
Cc: devicetree, linux-aspeed, open list:GPIO SUBSYSTEM,
Andrew Jeffery, OpenBMC Maillist, Linux Kernel Mailing List,
Bartosz Golaszewski, Rob Herring, BMC-SW, Linux ARM
On Mon, 12 Oct 2020 at 03:32, Billy Tsai <billy_tsai@aspeedtech.com> wrote:
>
> At ast2600a1 we change feature of master sgpio to 2 sets.
> So this patch is used to add the pinctrl setting of the new sgpio.
>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Linus, can you take this through the pinctrl tree? The patch to the
will be fine to come through your tree as we rarely update that file.
> ---
> arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi | 5 ++++
> drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 30 +++++++++++++++++++---
> 2 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi b/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> index 7028e21bdd98..a16ecf08e307 100644
> --- a/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> @@ -862,6 +862,11 @@
> groups = "SGPM1";
> };
>
> + pinctrl_sgpm2_default: sgpm2_default {
> + function = "SGPM2";
> + groups = "SGPM2";
> + };
> +
> pinctrl_sgps1_default: sgps1_default {
> function = "SGPS1";
> groups = "SGPS1";
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> index 34803a6c7664..b673a44ffa3b 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> @@ -46,8 +46,10 @@
> #define SCU620 0x620 /* Disable GPIO Internal Pull-Down #4 */
> #define SCU634 0x634 /* Disable GPIO Internal Pull-Down #5 */
> #define SCU638 0x638 /* Disable GPIO Internal Pull-Down #6 */
> +#define SCU690 0x690 /* Multi-function Pin Control #24 */
> #define SCU694 0x694 /* Multi-function Pin Control #25 */
> #define SCU69C 0x69C /* Multi-function Pin Control #27 */
> +#define SCU6D0 0x6D0 /* Multi-function Pin Control #28 */
> #define SCUC20 0xC20 /* PCIE configuration Setting Control */
>
> #define ASPEED_G6_NR_PINS 256
> @@ -81,13 +83,21 @@ FUNC_GROUP_DECL(I2C12, L26, K24);
> #define K26 4
> SIG_EXPR_LIST_DECL_SESG(K26, MACLINK1, MACLINK1, SIG_DESC_SET(SCU410, 4));
> SIG_EXPR_LIST_DECL_SESG(K26, SCL13, I2C13, SIG_DESC_SET(SCU4B0, 4));
> -PIN_DECL_2(K26, GPIOA4, MACLINK1, SCL13);
> +/*SGPM2 is A1 Only */
> +SIG_EXPR_LIST_DECL_SESG(K26, SGPM2CLK, SGPM2, SIG_DESC_SET(SCU6D0, 4),
> + SIG_DESC_CLEAR(SCU410, 4), SIG_DESC_CLEAR(SCU4B0, 4),
> + SIG_DESC_CLEAR(SCU690, 4));
> +PIN_DECL_3(K26, GPIOA4, SGPM2CLK, MACLINK1, SCL13);
> FUNC_GROUP_DECL(MACLINK1, K26);
>
> #define L24 5
> SIG_EXPR_LIST_DECL_SESG(L24, MACLINK2, MACLINK2, SIG_DESC_SET(SCU410, 5));
> SIG_EXPR_LIST_DECL_SESG(L24, SDA13, I2C13, SIG_DESC_SET(SCU4B0, 5));
> -PIN_DECL_2(L24, GPIOA5, MACLINK2, SDA13);
> +/*SGPM2 is A1 Only */
> +SIG_EXPR_LIST_DECL_SESG(L24, SGPM2LD, SGPM2, SIG_DESC_SET(SCU6D0, 5),
> + SIG_DESC_CLEAR(SCU410, 5), SIG_DESC_CLEAR(SCU4B0, 5),
> + SIG_DESC_CLEAR(SCU690, 5));
> +PIN_DECL_3(L24, GPIOA5, SGPM2LD, MACLINK2, SDA13);
> FUNC_GROUP_DECL(MACLINK2, L24);
>
> FUNC_GROUP_DECL(I2C13, K26, L24);
> @@ -95,16 +105,26 @@ FUNC_GROUP_DECL(I2C13, K26, L24);
> #define L23 6
> SIG_EXPR_LIST_DECL_SESG(L23, MACLINK3, MACLINK3, SIG_DESC_SET(SCU410, 6));
> SIG_EXPR_LIST_DECL_SESG(L23, SCL14, I2C14, SIG_DESC_SET(SCU4B0, 6));
> -PIN_DECL_2(L23, GPIOA6, MACLINK3, SCL14);
> +/*SGPM2 is A1 Only */
> +SIG_EXPR_LIST_DECL_SESG(L23, SGPM2O, SGPM2, SIG_DESC_SET(SCU6D0, 6),
> + SIG_DESC_CLEAR(SCU410, 6), SIG_DESC_CLEAR(SCU4B0, 6),
> + SIG_DESC_CLEAR(SCU690, 6));
> +PIN_DECL_3(L23, GPIOA6, SGPM2O, MACLINK3, SCL14);
> FUNC_GROUP_DECL(MACLINK3, L23);
>
> #define K25 7
> SIG_EXPR_LIST_DECL_SESG(K25, MACLINK4, MACLINK4, SIG_DESC_SET(SCU410, 7));
> SIG_EXPR_LIST_DECL_SESG(K25, SDA14, I2C14, SIG_DESC_SET(SCU4B0, 7));
> -PIN_DECL_2(K25, GPIOA7, MACLINK4, SDA14);
> +/*SGPM2 is A1 Only */
> +SIG_EXPR_LIST_DECL_SESG(K25, SGPM2I, SGPM2, SIG_DESC_SET(SCU6D0, 7),
> + SIG_DESC_CLEAR(SCU410, 7), SIG_DESC_CLEAR(SCU4B0, 7),
> + SIG_DESC_CLEAR(SCU690, 7));
> +PIN_DECL_3(K25, GPIOA7, SGPM2I, MACLINK4, SDA14);
> FUNC_GROUP_DECL(MACLINK4, K25);
>
> FUNC_GROUP_DECL(I2C14, L23, K25);
> +/*SGPM2 is A1 Only */
> +FUNC_GROUP_DECL(SGPM2, K26, L24, L23, K25);
>
> #define J26 8
> SIG_EXPR_LIST_DECL_SESG(J26, SALT1, SALT1, SIG_DESC_SET(SCU410, 8));
> @@ -2060,6 +2080,7 @@ static const struct aspeed_pin_group aspeed_g6_groups[] = {
> ASPEED_PINCTRL_GROUP(EMMCG4),
> ASPEED_PINCTRL_GROUP(EMMCG8),
> ASPEED_PINCTRL_GROUP(SGPM1),
> + ASPEED_PINCTRL_GROUP(SGPM2),
> ASPEED_PINCTRL_GROUP(SGPS1),
> ASPEED_PINCTRL_GROUP(SIOONCTRL),
> ASPEED_PINCTRL_GROUP(SIOPBI),
> @@ -2276,6 +2297,7 @@ static const struct aspeed_pin_function aspeed_g6_functions[] = {
> ASPEED_PINCTRL_FUNC(SD1),
> ASPEED_PINCTRL_FUNC(SD2),
> ASPEED_PINCTRL_FUNC(SGPM1),
> + ASPEED_PINCTRL_FUNC(SGPM2),
> ASPEED_PINCTRL_FUNC(SGPS1),
> ASPEED_PINCTRL_FUNC(SIOONCTRL),
> ASPEED_PINCTRL_FUNC(SIOPBI),
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] Arm: dts: aspeed-g6: Add sgpio node
2020-10-12 4:35 ` Joel Stanley
@ 2020-10-12 4:53 ` Billy Tsai
2020-10-28 5:10 ` Joel Stanley
2020-10-26 1:33 ` Andrew Jeffery
1 sibling, 1 reply; 16+ messages in thread
From: Billy Tsai @ 2020-10-12 4:53 UTC (permalink / raw)
To: Joel Stanley
Cc: devicetree, linux-aspeed, open list:GPIO SUBSYSTEM,
Andrew Jeffery, Linus Walleij, Linux Kernel Mailing List,
Bartosz Golaszewski, Rob Herring, BMC-SW, OpenBMC Maillist,
Linux ARM
Hi Joel,
Thanks for the review.
On 2020/10/12, 12:35 PM, Joel Stanley wrote:
> On Mon, 12 Oct 2020 at 03:32, Billy Tsai <billy_tsai@aspeedtech.com> wrote:
> >
> > This patch is used to add sgpiom and sgpios nodes and add compatible
> > string for sgpiom.
>
> You also need to add sgpios documentation to the bindings docs.
>
> Whenever you add new device tree bindings to the kernel tree you
> should add documentation for them.
>
> When preparing patches for submission, use scripts/checkpatch.pl to
> check for common issues. It will warn you if you are adding strings
> that are not documented.
>
> Cheers,
>
> Joel
>
Because the driver of sgpios doesn't be implemented, so I don't know how to describe it at sgpio-aspeed.txt.
Can I just add compatible string " aspeed,ast2600-sgpios " to the document for bypassing the warning of checkpatch?
> >
> > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> > ---
> > .../devicetree/bindings/gpio/sgpio-aspeed.txt | 8 +--
> > arch/arm/boot/dts/aspeed-g6.dtsi | 52 +++++++++++++++++++
> > 2 files changed, 57 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > index d4d83916c09d..815d9b5167a5 100644
> > --- a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > +++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > @@ -1,8 +1,10 @@
> > Aspeed SGPIO controller Device Tree Bindings
> > --------------------------------------------
> >
> > -This SGPIO controller is for ASPEED AST2500 SoC, it supports up to 80 full
> > -featured Serial GPIOs. Each of the Serial GPIO pins can be programmed to
> > +This SGPIO controller is for ASPEED AST2500/AST2600 SoC, it supports 2 master.
> > +One is up to 128 SGPIO input ports and 128 output ports concurrently(after AST2600A1)
> > +and Second one is up to 80.
> > +Each of the Serial GPIO pins can be programmed to
> > support the following options:
> > - Support interrupt option for each input port and various interrupt
> > sensitivity option (level-high, level-low, edge-high, edge-low)
> > @@ -14,7 +16,7 @@ support the following options:
> > Required properties:
> >
> > - compatible : Should be one of
> > - "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio"
> > + "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio", "aspeed,ast2600-sgpiom"
>
> I think we should add sgpiom strings for the ast2500 (and ast2400?)
> too, as this is how they should have been named in the first place:
>
If I change the document whether I also need to send the patch for sgpio driver and g5/g4.dtsi?
> > - compatible : Should be one of
> > "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio"
> > "aspeed,ast2400-sgpiom", "aspeed,ast2500-sgpiom", "aspeed,ast2600-sgpiom"
>
>
> > - #gpio-cells : Should be 2, see gpio.txt
> > - reg : Address and length of the register set for the device
> > - gpio-controller : Marks the device node as a GPIO controller
> > diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
> > index ad19dce038ea..cb053a996e87 100644
> > --- a/arch/arm/boot/dts/aspeed-g6.dtsi
> > +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
> > @@ -366,6 +366,58 @@
> > #interrupt-cells = <2>;
> > };
> >
> > + sgpiom0: sgpiom@1e780500 {
> > + #gpio-cells = <2>;
> > + gpio-controller;
> > + compatible = "aspeed,ast2600-sgpiom";
> > + reg = <0x1e780500 0x100>;
> > + interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
> > + ngpios = <128>;
> > + clocks = <&syscon ASPEED_CLK_APB2>;
> > + interrupt-controller;
> > + bus-frequency = <12000000>;
> > +
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_sgpm1_default>;
> > + status = "disabled";
> > + };
> > +
> > + sgpiom1: sgpiom@1e780600 {
> > + #gpio-cells = <2>;
> > + gpio-controller;
> > + compatible = "aspeed,ast2600-sgpiom";
> > + reg = <0x1e780600 0x100>;
> > + interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>;
> > + ngpios = <80>;
> > + clocks = <&syscon ASPEED_CLK_APB2>;
> > + interrupt-controller;
> > + bus-frequency = <12000000>;
> > +
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_sgpm2_default>;
> > + status = "disabled";
> > + };
> > +
> > + sgpios0: sgpios@1e780700 {
> > + #gpio-cells = <2>;
> > + gpio-controller;
> > + compatible = "aspeed,ast2600-sgpios";
> > + reg = <0x1e780700 0x40>;
> > + interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&syscon ASPEED_CLK_APB2>;
> > + status = "disabled";
> > + };
> > +
> > + sgpios1: sgpios@1e780740 {
> > + #gpio-cells = <2>;
> > + gpio-controller;
> > + compatible = "aspeed,ast2600-sgpios";
> > + reg = <0x1e780740 0x40>;
> > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&syscon ASPEED_CLK_APB2>;
> > + status = "disabled";
> > + };
> > +
> > gpio1: gpio@1e780800 {
> > #gpio-cells = <2>;
> > gpio-controller;
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] Arm: dts: aspeed-g6: Fix the register range of gpio
2020-10-12 3:31 ` [PATCH 1/3] Arm: dts: aspeed-g6: Fix the register range of gpio Billy Tsai
2020-10-12 4:30 ` Joel Stanley
@ 2020-10-26 1:05 ` Andrew Jeffery
2020-10-28 5:12 ` Joel Stanley
1 sibling, 1 reply; 16+ messages in thread
From: Andrew Jeffery @ 2020-10-26 1:05 UTC (permalink / raw)
To: Billy Tsai, Rob Herring, Joel Stanley, devicetree,
linux-arm-kernel, linux-aspeed, linux-kernel, Linus Walleij,
Bartosz Golaszewski, linux-gpio, openbmc
Cc: BMC-SW
On Mon, 12 Oct 2020, at 14:01, Billy Tsai wrote:
> This patch is used to fix the memory range of gpio0
>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] pinctrl: aspeed-g6: Add sgpiom2 pinctrl setting
2020-10-12 3:31 ` [PATCH 3/3] pinctrl: aspeed-g6: Add sgpiom2 pinctrl setting Billy Tsai
2020-10-12 4:36 ` Joel Stanley
@ 2020-10-26 1:26 ` Andrew Jeffery
2020-10-26 2:03 ` Billy Tsai
1 sibling, 1 reply; 16+ messages in thread
From: Andrew Jeffery @ 2020-10-26 1:26 UTC (permalink / raw)
To: Billy Tsai, Rob Herring, Joel Stanley, devicetree,
linux-arm-kernel, linux-aspeed, linux-kernel, Linus Walleij,
Bartosz Golaszewski, linux-gpio, openbmc
Cc: BMC-SW
On Mon, 12 Oct 2020, at 14:01, Billy Tsai wrote:
> At ast2600a1 we change feature of master sgpio to 2 sets.
> So this patch is used to add the pinctrl setting of the new sgpio.
>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
> arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi | 5 ++++
> drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 30 +++++++++++++++++++---
> 2 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> b/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> index 7028e21bdd98..a16ecf08e307 100644
> --- a/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> @@ -862,6 +862,11 @@
> groups = "SGPM1";
> };
>
> + pinctrl_sgpm2_default: sgpm2_default {
> + function = "SGPM2";
> + groups = "SGPM2";
> + };
> +
> pinctrl_sgps1_default: sgps1_default {
> function = "SGPS1";
> groups = "SGPS1";
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> index 34803a6c7664..b673a44ffa3b 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> @@ -46,8 +46,10 @@
> #define SCU620 0x620 /* Disable GPIO Internal Pull-Down #4 */
> #define SCU634 0x634 /* Disable GPIO Internal Pull-Down #5 */
> #define SCU638 0x638 /* Disable GPIO Internal Pull-Down #6 */
> +#define SCU690 0x690 /* Multi-function Pin Control #24 */
> #define SCU694 0x694 /* Multi-function Pin Control #25 */
> #define SCU69C 0x69C /* Multi-function Pin Control #27 */
> +#define SCU6D0 0x6D0 /* Multi-function Pin Control #28 */
> #define SCUC20 0xC20 /* PCIE configuration Setting Control */
>
> #define ASPEED_G6_NR_PINS 256
> @@ -81,13 +83,21 @@ FUNC_GROUP_DECL(I2C12, L26, K24);
> #define K26 4
> SIG_EXPR_LIST_DECL_SESG(K26, MACLINK1, MACLINK1, SIG_DESC_SET(SCU410, 4));
> SIG_EXPR_LIST_DECL_SESG(K26, SCL13, I2C13, SIG_DESC_SET(SCU4B0, 4));
> -PIN_DECL_2(K26, GPIOA4, MACLINK1, SCL13);
> +/*SGPM2 is A1 Only */
> +SIG_EXPR_LIST_DECL_SESG(K26, SGPM2CLK, SGPM2, SIG_DESC_SET(SCU6D0, 4),
> + SIG_DESC_CLEAR(SCU410, 4), SIG_DESC_CLEAR(SCU4B0, 4),
> + SIG_DESC_CLEAR(SCU690, 4));
> +PIN_DECL_3(K26, GPIOA4, SGPM2CLK, MACLINK1, SCL13);
> FUNC_GROUP_DECL(MACLINK1, K26);
>
> #define L24 5
> SIG_EXPR_LIST_DECL_SESG(L24, MACLINK2, MACLINK2, SIG_DESC_SET(SCU410, 5));
> SIG_EXPR_LIST_DECL_SESG(L24, SDA13, I2C13, SIG_DESC_SET(SCU4B0, 5));
> -PIN_DECL_2(L24, GPIOA5, MACLINK2, SDA13);
> +/*SGPM2 is A1 Only */
> +SIG_EXPR_LIST_DECL_SESG(L24, SGPM2LD, SGPM2, SIG_DESC_SET(SCU6D0, 5),
> + SIG_DESC_CLEAR(SCU410, 5), SIG_DESC_CLEAR(SCU4B0, 5),
> + SIG_DESC_CLEAR(SCU690, 5));
A few things:
1. It looks like the Multi-function Pins Mapping and Control table in section 5.1 of the datasheet only tells part of the story. It lists SGPS2 on the pins you've modified in this patch but not SGPM2. However, the table in section 2.1 (Pin Description) does outline SGPM2 and SGPS2 are routed via the same pins, though this does not listed the associated registers and bit fields. Can we fix the table in 5.1 so it's easier to review this patch?
2. We don't need to specify the _CLEAR() behaviour here as this is implied by the process to disable the higher priority mux configurations. It should be enough to do:
SIG_EXPR_LIST_DECL_SESG(L24, SGPM2LD, SGPM2, SIG_DESC_SET(SCU6D0, 5));
However, this requires that we also define the priorities correctly, so:
3. Can we add both the SGPS2 and SGPM2 configurations so we have a complete definition for the pins?
Cheers,
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] Arm: dts: aspeed-g6: Add sgpio node
2020-10-12 4:35 ` Joel Stanley
2020-10-12 4:53 ` Billy Tsai
@ 2020-10-26 1:33 ` Andrew Jeffery
1 sibling, 0 replies; 16+ messages in thread
From: Andrew Jeffery @ 2020-10-26 1:33 UTC (permalink / raw)
To: Joel Stanley, Billy Tsai
Cc: devicetree, linux-aspeed, open list:GPIO SUBSYSTEM,
Linus Walleij, Linux Kernel Mailing List, Bartosz Golaszewski,
Rob Herring, BMC-SW, OpenBMC Maillist, Linux ARM
On Mon, 12 Oct 2020, at 15:05, Joel Stanley wrote:
> On Mon, 12 Oct 2020 at 03:32, Billy Tsai <billy_tsai@aspeedtech.com> wrote:
> >
> > This patch is used to add sgpiom and sgpios nodes and add compatiable
> > string for sgpiom.
>
> You also need to add sgpios documentation to the bindings docs.
>
> Whenever you add new device tree bindings to the kernel tree you
> should add documentation for them.
>
> When preparing patches for submission, use scripts/checkpatch.pl to
> check for common issues. It will warn you if you are adding strings
> that are not documented.
>
> Cheers,
>
> Joel
>
> >
> > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> > ---
> > .../devicetree/bindings/gpio/sgpio-aspeed.txt | 8 +--
> > arch/arm/boot/dts/aspeed-g6.dtsi | 52 +++++++++++++++++++
> > 2 files changed, 57 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > index d4d83916c09d..815d9b5167a5 100644
> > --- a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > +++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > @@ -1,8 +1,10 @@
> > Aspeed SGPIO controller Device Tree Bindings
> > --------------------------------------------
> >
> > -This SGPIO controller is for ASPEED AST2500 SoC, it supports up to 80 full
> > -featured Serial GPIOs. Each of the Serial GPIO pins can be programmed to
> > +This SGPIO controller is for ASPEED AST2500/AST2600 SoC, it supports 2 master.
> > +One is up to 128 SGPIO input ports and 128 output ports concurrently(after AST2600A1)
> > +and Second one is up to 80.
> > +Each of the Serial GPIO pins can be programmed to
> > support the following options:
> > - Support interrupt option for each input port and various interrupt
> > sensitivity option (level-high, level-low, edge-high, edge-low)
> > @@ -14,7 +16,7 @@ support the following options:
> > Required properties:
> >
> > - compatible : Should be one of
> > - "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio"
> > + "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio", "aspeed,ast2600-sgpiom"
>
> I think we should add sgpiom strings for the ast2500 (and ast2400?)
> too, as this is how they should have been named in the first place:
Can we defer this discussion until someone adds support for the slave
interface? Maybe the existing compatible could have done with some more
thought, but it is what it is, and I don't think we need to muddy the waters
further right now?
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] pinctrl: aspeed-g6: Add sgpiom2 pinctrl setting
2020-10-26 1:26 ` Andrew Jeffery
@ 2020-10-26 2:03 ` Billy Tsai
2020-10-26 2:20 ` Andrew Jeffery
0 siblings, 1 reply; 16+ messages in thread
From: Billy Tsai @ 2020-10-26 2:03 UTC (permalink / raw)
To: Andrew Jeffery, Rob Herring, Joel Stanley, devicetree,
linux-arm-kernel, linux-aspeed, linux-kernel, Linus Walleij,
Bartosz Golaszewski, linux-gpio, openbmc
Cc: BMC-SW
On 2020/10/26, 9:27 AM, Andrew Jeffery wrote:
On Mon, 12 Oct 2020, at 14:01, Billy Tsai wrote:
> > At ast2600a1 we change feature of master sgpio to 2 sets.
> > So this patch is used to add the pinctrl setting of the new sgpio.
> >
> > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> > ---
> > arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi | 5 ++++
> > drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 30 +++++++++++++++++++---
> > 2 files changed, 31 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> > b/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> > index 7028e21bdd98..a16ecf08e307 100644
> > --- a/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> > +++ b/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> > @@ -862,6 +862,11 @@
> > groups = "SGPM1";
> > };
> >
> > + pinctrl_sgpm2_default: sgpm2_default {
> > + function = "SGPM2";
> > + groups = "SGPM2";
> > + };
> > +
> > pinctrl_sgps1_default: sgps1_default {
> > function = "SGPS1";
> > groups = "SGPS1";
> > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > index 34803a6c7664..b673a44ffa3b 100644
> > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > @@ -46,8 +46,10 @@
> > #define SCU620 0x620 /* Disable GPIO Internal Pull-Down #4 */
> > #define SCU634 0x634 /* Disable GPIO Internal Pull-Down #5 */
> > #define SCU638 0x638 /* Disable GPIO Internal Pull-Down #6 */
> > +#define SCU690 0x690 /* Multi-function Pin Control #24 */
> > #define SCU694 0x694 /* Multi-function Pin Control #25 */
> > #define SCU69C 0x69C /* Multi-function Pin Control #27 */
> > +#define SCU6D0 0x6D0 /* Multi-function Pin Control #28 */
> > #define SCUC20 0xC20 /* PCIE configuration Setting Control */
> >
> > #define ASPEED_G6_NR_PINS 256
> > @@ -81,13 +83,21 @@ FUNC_GROUP_DECL(I2C12, L26, K24);
> > #define K26 4
> > SIG_EXPR_LIST_DECL_SESG(K26, MACLINK1, MACLINK1, SIG_DESC_SET(SCU410, 4));
> > SIG_EXPR_LIST_DECL_SESG(K26, SCL13, I2C13, SIG_DESC_SET(SCU4B0, 4));
> > -PIN_DECL_2(K26, GPIOA4, MACLINK1, SCL13);
> > +/*SGPM2 is A1 Only */
> > +SIG_EXPR_LIST_DECL_SESG(K26, SGPM2CLK, SGPM2, SIG_DESC_SET(SCU6D0, 4),
> > + SIG_DESC_CLEAR(SCU410, 4), SIG_DESC_CLEAR(SCU4B0, 4),
> > + SIG_DESC_CLEAR(SCU690, 4));
> > +PIN_DECL_3(K26, GPIOA4, SGPM2CLK, MACLINK1, SCL13);
> > FUNC_GROUP_DECL(MACLINK1, K26);
> >
> > #define L24 5
> > SIG_EXPR_LIST_DECL_SESG(L24, MACLINK2, MACLINK2, SIG_DESC_SET(SCU410, 5));
> > SIG_EXPR_LIST_DECL_SESG(L24, SDA13, I2C13, SIG_DESC_SET(SCU4B0, 5));
> > -PIN_DECL_2(L24, GPIOA5, MACLINK2, SDA13);
> > +/*SGPM2 is A1 Only */
> > +SIG_EXPR_LIST_DECL_SESG(L24, SGPM2LD, SGPM2, SIG_DESC_SET(SCU6D0, 5),
> > + SIG_DESC_CLEAR(SCU410, 5), SIG_DESC_CLEAR(SCU4B0, 5),
> > + SIG_DESC_CLEAR(SCU690, 5));
>
> A few things:
>
> 1. It looks like the Multi-function Pins Mapping and Control table in section 5.1 of the datasheet only tells part of the story. It lists SGPS2 on the pins you've modified in this patch but not SGPM2. However, the table in section 2.1 (Pin Description) does outline SGPM2 and SGPS2 are routed via the same pins, though this does not listed the associated registers and bit fields. Can we fix the table in 5.1 so it's easier to review this patch?
You can see section 5.2 to find SGPIO master interface table. It lists balls and register setting information of the SGPIOM1 and SGPIOM2.
>
> 2. We don't need to specify the _CLEAR() behaviour here as this is implied by the process to disable the higher priority mux configurations. It should be enough to do:
>
> SIG_EXPR_LIST_DECL_SESG(L24, SGPM2LD, SGPM2, SIG_DESC_SET(SCU6D0, 5));
>
> However, this requires that we also define the priorities correctly, so:
>
> 3. Can we add both the SGPS2 and SGPM2 configurations so we have a complete definition for the pins?
>
Thank you for your advice. I will complete the full configurations and send the V2 patch.
> Cheers,
>
> Andrew
Best Regards,
Billy Tsai
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] pinctrl: aspeed-g6: Add sgpiom2 pinctrl setting
2020-10-26 2:03 ` Billy Tsai
@ 2020-10-26 2:20 ` Andrew Jeffery
2020-10-26 2:56 ` Billy Tsai
0 siblings, 1 reply; 16+ messages in thread
From: Andrew Jeffery @ 2020-10-26 2:20 UTC (permalink / raw)
To: Billy Tsai, Rob Herring, Joel Stanley, devicetree,
linux-arm-kernel, linux-aspeed, linux-kernel, Linus Walleij,
Bartosz Golaszewski, linux-gpio, openbmc
Cc: BMC-SW
On Mon, 26 Oct 2020, at 12:33, Billy Tsai wrote:
>
> On 2020/10/26, 9:27 AM, Andrew Jeffery wrote:
>
> On Mon, 12 Oct 2020, at 14:01, Billy Tsai wrote:
> > > At ast2600a1 we change feature of master sgpio to 2 sets.
> > > So this patch is used to add the pinctrl setting of the new
> sgpio.
> > >
> > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> > > ---
> > > arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi | 5 ++++
> > > drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 30
> +++++++++++++++++++---
> > > 2 files changed, 31 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> > > b/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> > > index 7028e21bdd98..a16ecf08e307 100644
> > > --- a/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> > > +++ b/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> > > @@ -862,6 +862,11 @@
> > > groups = "SGPM1";
> > > };
> > >
> > > + pinctrl_sgpm2_default: sgpm2_default {
> > > + function = "SGPM2";
> > > + groups = "SGPM2";
> > > + };
> > > +
> > > pinctrl_sgps1_default: sgps1_default {
> > > function = "SGPS1";
> > > groups = "SGPS1";
> > > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > > b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > > index 34803a6c7664..b673a44ffa3b 100644
> > > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > > @@ -46,8 +46,10 @@
> > > #define SCU620 0x620 /* Disable GPIO Internal Pull-Down #4 */
> > > #define SCU634 0x634 /* Disable GPIO Internal Pull-Down #5 */
> > > #define SCU638 0x638 /* Disable GPIO Internal Pull-Down #6 */
> > > +#define SCU690 0x690 /* Multi-function Pin Control #24 */
> > > #define SCU694 0x694 /* Multi-function Pin Control #25 */
> > > #define SCU69C 0x69C /* Multi-function Pin Control #27 */
> > > +#define SCU6D0 0x6D0 /* Multi-function Pin Control #28 */
> > > #define SCUC20 0xC20 /* PCIE configuration Setting Control */
> > >
> > > #define ASPEED_G6_NR_PINS 256
> > > @@ -81,13 +83,21 @@ FUNC_GROUP_DECL(I2C12, L26, K24);
> > > #define K26 4
> > > SIG_EXPR_LIST_DECL_SESG(K26, MACLINK1, MACLINK1,
> SIG_DESC_SET(SCU410, 4));
> > > SIG_EXPR_LIST_DECL_SESG(K26, SCL13, I2C13,
> SIG_DESC_SET(SCU4B0, 4));
> > > -PIN_DECL_2(K26, GPIOA4, MACLINK1, SCL13);
> > > +/*SGPM2 is A1 Only */
> > > +SIG_EXPR_LIST_DECL_SESG(K26, SGPM2CLK, SGPM2,
> SIG_DESC_SET(SCU6D0, 4),
> > > + SIG_DESC_CLEAR(SCU410, 4), SIG_DESC_CLEAR(SCU4B0, 4),
> > > + SIG_DESC_CLEAR(SCU690, 4));
> > > +PIN_DECL_3(K26, GPIOA4, SGPM2CLK, MACLINK1, SCL13);
> > > FUNC_GROUP_DECL(MACLINK1, K26);
> > >
> > > #define L24 5
> > > SIG_EXPR_LIST_DECL_SESG(L24, MACLINK2, MACLINK2,
> SIG_DESC_SET(SCU410, 5));
> > > SIG_EXPR_LIST_DECL_SESG(L24, SDA13, I2C13,
> SIG_DESC_SET(SCU4B0, 5));
> > > -PIN_DECL_2(L24, GPIOA5, MACLINK2, SDA13);
> > > +/*SGPM2 is A1 Only */
> > > +SIG_EXPR_LIST_DECL_SESG(L24, SGPM2LD, SGPM2,
> SIG_DESC_SET(SCU6D0, 5),
> > > + SIG_DESC_CLEAR(SCU410, 5), SIG_DESC_CLEAR(SCU4B0, 5),
> > > + SIG_DESC_CLEAR(SCU690, 5));
> >
> > A few things:
> >
> > 1. It looks like the Multi-function Pins Mapping and Control
> table in section 5.1 of the datasheet only tells part of the story. It
> lists SGPS2 on the pins you've modified in this patch but not SGPM2.
> However, the table in section 2.1 (Pin Description) does outline SGPM2
> and SGPS2 are routed via the same pins, though this does not listed the
> associated registers and bit fields. Can we fix the table in 5.1 so
> it's easier to review this patch?
> You can see section 5.2 to find SGPIO master interface table. It lists
> balls and register setting information of the SGPIOM1 and SGPIOM2.
Yep, though typically I use the table in 5.1 to figure out the pinctrl details.
It appears you'd need to add another two columns to the table to capture the
info - is Aspeed planning to do that in a future release of the datasheet?
> >
> > 2. We don't need to specify the _CLEAR() behaviour here as this
> is implied by the process to disable the higher priority mux
> configurations. It should be enough to do:
> >
> > SIG_EXPR_LIST_DECL_SESG(L24, SGPM2LD, SGPM2, SIG_DESC_SET(SCU6D0,
> 5));
> >
> > However, this requires that we also define the priorities
> correctly, so:
> >
> > 3. Can we add both the SGPS2 and SGPM2 configurations so we have
> a complete definition for the pins?
> >
> Thank you for your advice. I will complete the full configurations and
> send the V2 patch.
Thanks!
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] pinctrl: aspeed-g6: Add sgpiom2 pinctrl setting
2020-10-26 2:20 ` Andrew Jeffery
@ 2020-10-26 2:56 ` Billy Tsai
0 siblings, 0 replies; 16+ messages in thread
From: Billy Tsai @ 2020-10-26 2:56 UTC (permalink / raw)
To: Andrew Jeffery, Rob Herring, Joel Stanley, devicetree,
linux-arm-kernel, linux-aspeed, linux-kernel, Linus Walleij,
Bartosz Golaszewski, linux-gpio, openbmc
Cc: BMC-SW
On 2020/10/26, 10:21 AM, Andrew Jeffery wrote:
On Mon, 26 Oct 2020, at 12:33, Billy Tsai wrote:
>
> On 2020/10/26, 9:27 AM, Andrew Jeffery wrote:
>
> On Mon, 12 Oct 2020, at 14:01, Billy Tsai wrote:
> > > At ast2600a1 we change feature of master sgpio to 2 sets.
> > > So this patch is used to add the pinctrl setting of the new
> sgpio.
> > >
> > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> > > ---
> > > arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi | 5 ++++
> > > drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 30
> +++++++++++++++++++---
> > > 2 files changed, 31 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> > > b/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> > > index 7028e21bdd98..a16ecf08e307 100644
> > > --- a/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> > > +++ b/arch/arm/boot/dts/aspeed-g6-pinctrl.dtsi
> > > @@ -862,6 +862,11 @@
> > > groups = "SGPM1";
> > > };
> > >
> > > + pinctrl_sgpm2_default: sgpm2_default {
> > > + function = "SGPM2";
> > > + groups = "SGPM2";
> > > + };
> > > +
> > > pinctrl_sgps1_default: sgps1_default {
> > > function = "SGPS1";
> > > groups = "SGPS1";
> > > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > > b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > > index 34803a6c7664..b673a44ffa3b 100644
> > > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > > @@ -46,8 +46,10 @@
> > > #define SCU620 0x620 /* Disable GPIO Internal Pull-Down #4 */
> > > #define SCU634 0x634 /* Disable GPIO Internal Pull-Down #5 */
> > > #define SCU638 0x638 /* Disable GPIO Internal Pull-Down #6 */
> > > +#define SCU690 0x690 /* Multi-function Pin Control #24 */
> > > #define SCU694 0x694 /* Multi-function Pin Control #25 */
> > > #define SCU69C 0x69C /* Multi-function Pin Control #27 */
> > > +#define SCU6D0 0x6D0 /* Multi-function Pin Control #28 */
> > > #define SCUC20 0xC20 /* PCIE configuration Setting Control */
> > >
> > > #define ASPEED_G6_NR_PINS 256
> > > @@ -81,13 +83,21 @@ FUNC_GROUP_DECL(I2C12, L26, K24);
> > > #define K26 4
> > > SIG_EXPR_LIST_DECL_SESG(K26, MACLINK1, MACLINK1,
> SIG_DESC_SET(SCU410, 4));
> > > SIG_EXPR_LIST_DECL_SESG(K26, SCL13, I2C13,
> SIG_DESC_SET(SCU4B0, 4));
> > > -PIN_DECL_2(K26, GPIOA4, MACLINK1, SCL13);
> > > +/*SGPM2 is A1 Only */
> > > +SIG_EXPR_LIST_DECL_SESG(K26, SGPM2CLK, SGPM2,
> SIG_DESC_SET(SCU6D0, 4),
> > > + SIG_DESC_CLEAR(SCU410, 4), SIG_DESC_CLEAR(SCU4B0, 4),
> > > + SIG_DESC_CLEAR(SCU690, 4));
> > > +PIN_DECL_3(K26, GPIOA4, SGPM2CLK, MACLINK1, SCL13);
> > > FUNC_GROUP_DECL(MACLINK1, K26);
> > >
> > > #define L24 5
> > > SIG_EXPR_LIST_DECL_SESG(L24, MACLINK2, MACLINK2,
> SIG_DESC_SET(SCU410, 5));
> > > SIG_EXPR_LIST_DECL_SESG(L24, SDA13, I2C13,
> SIG_DESC_SET(SCU4B0, 5));
> > > -PIN_DECL_2(L24, GPIOA5, MACLINK2, SDA13);
> > > +/*SGPM2 is A1 Only */
> > > +SIG_EXPR_LIST_DECL_SESG(L24, SGPM2LD, SGPM2,
> SIG_DESC_SET(SCU6D0, 5),
> > > + SIG_DESC_CLEAR(SCU410, 5), SIG_DESC_CLEAR(SCU4B0, 5),
> > > + SIG_DESC_CLEAR(SCU690, 5));
> >
> > A few things:
> >
> > 1. It looks like the Multi-function Pins Mapping and Control
> table in section 5.1 of the datasheet only tells part of the story. It
> lists SGPS2 on the pins you've modified in this patch but not SGPM2.
> However, the table in section 2.1 (Pin Description) does outline SGPM2
> and SGPS2 are routed via the same pins, though this does not listed the
> associated registers and bit fields. Can we fix the table in 5.1 so
> it's easier to review this patch?
> You can see section 5.2 to find SGPIO master interface table. It lists
> balls and register setting information of the SGPIOM1 and SGPIOM2.
Yep, though typically I use the table in 5.1 to figure out the pinctrl details.
It appears you'd need to add another two columns to the table to capture the
info - is Aspeed planning to do that in a future release of the datasheet?
Yes, we will update the datasheet to add another two columns.
> >
> > 2. We don't need to specify the _CLEAR() behaviour here as this
> is implied by the process to disable the higher priority mux
> configurations. It should be enough to do:
> >
> > SIG_EXPR_LIST_DECL_SESG(L24, SGPM2LD, SGPM2, SIG_DESC_SET(SCU6D0,
> 5));
> >
> > However, this requires that we also define the priorities
> correctly, so:
> >
> > 3. Can we add both the SGPS2 and SGPM2 configurations so we have
> a complete definition for the pins?
> >
> Thank you for your advice. I will complete the full configurations and
> send the V2 patch.
Thanks!
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] Arm: dts: aspeed-g6: Add sgpio node
2020-10-12 4:53 ` Billy Tsai
@ 2020-10-28 5:10 ` Joel Stanley
0 siblings, 0 replies; 16+ messages in thread
From: Joel Stanley @ 2020-10-28 5:10 UTC (permalink / raw)
To: Billy Tsai
Cc: devicetree, linux-aspeed, open list:GPIO SUBSYSTEM,
Andrew Jeffery, Linus Walleij, Linux Kernel Mailing List,
Bartosz Golaszewski, Rob Herring, BMC-SW, OpenBMC Maillist,
Linux ARM
On Mon, 12 Oct 2020 at 04:56, Billy Tsai <billy_tsai@aspeedtech.com> wrote:
>
> Hi Joel,
>
> Thanks for the review.
>
> On 2020/10/12, 12:35 PM, Joel Stanley wrote:
>
> > On Mon, 12 Oct 2020 at 03:32, Billy Tsai <billy_tsai@aspeedtech.com> wrote:
> > >
> > > This patch is used to add sgpiom and sgpios nodes and add compatible
> > > string for sgpiom.
> >
> > You also need to add sgpios documentation to the bindings docs.
> >
> > Whenever you add new device tree bindings to the kernel tree you
> > should add documentation for them.
> >
> > When preparing patches for submission, use scripts/checkpatch.pl to
> > check for common issues. It will warn you if you are adding strings
> > that are not documented.
> >
> > Cheers,
> >
> > Joel
> >
> Because the driver of sgpios doesn't be implemented, so I don't know how to describe it at sgpio-aspeed.txt.
> Can I just add compatible string " aspeed,ast2600-sgpios " to the document for bypassing the warning of checkpatch?
Ignore the sgpios issue for now; we don't have a driver for it so
there's no need to add strings. Drop that part from your dts patch.
> > >
> > > - compatible : Should be one of
> > > - "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio"
> > > + "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio", "aspeed,ast2600-sgpiom"
> >
> > I think we should add sgpiom strings for the ast2500 (and ast2400?)
> > too, as this is how they should have been named in the first place:
> >
> If I change the document whether I also need to send the patch for sgpio driver and g5/g4.dtsi?
For the sgpiom? We already have a driver for that.
As I said above, make this about fixing the sgpio master and put aside
the sgpio slave issue for now.
Cheers,
Joel
> > > - compatible : Should be one of
> > > "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio"
> > > "aspeed,ast2400-sgpiom", "aspeed,ast2500-sgpiom", "aspeed,ast2600-sgpiom"
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] Arm: dts: aspeed-g6: Fix the register range of gpio
2020-10-26 1:05 ` Andrew Jeffery
@ 2020-10-28 5:12 ` Joel Stanley
0 siblings, 0 replies; 16+ messages in thread
From: Joel Stanley @ 2020-10-28 5:12 UTC (permalink / raw)
To: Andrew Jeffery
Cc: devicetree, linux-aspeed, open list:GPIO SUBSYSTEM,
Linus Walleij, Linux Kernel Mailing List, Billy Tsai,
Bartosz Golaszewski, Rob Herring, BMC-SW, OpenBMC Maillist,
Linux ARM
On Mon, 26 Oct 2020 at 01:05, Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Mon, 12 Oct 2020, at 14:01, Billy Tsai wrote:
> > This patch is used to fix the memory range of gpio0
> >
> > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
>
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
I've applied this with:
Fixes: 8dbcb5b709b9 ("ARM: dts: aspeed-g6: Add gpio devices")
Cheers,
Joel
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-10-28 5:15 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-12 3:31 [V2 PATCH 0/3] Fix the memory layout and add sgpio node for aspeed g6 Billy Tsai
2020-10-12 3:31 ` [PATCH 1/3] Arm: dts: aspeed-g6: Fix the register range of gpio Billy Tsai
2020-10-12 4:30 ` Joel Stanley
2020-10-26 1:05 ` Andrew Jeffery
2020-10-28 5:12 ` Joel Stanley
2020-10-12 3:31 ` [PATCH 2/3] Arm: dts: aspeed-g6: Add sgpio node Billy Tsai
2020-10-12 4:35 ` Joel Stanley
2020-10-12 4:53 ` Billy Tsai
2020-10-28 5:10 ` Joel Stanley
2020-10-26 1:33 ` Andrew Jeffery
2020-10-12 3:31 ` [PATCH 3/3] pinctrl: aspeed-g6: Add sgpiom2 pinctrl setting Billy Tsai
2020-10-12 4:36 ` Joel Stanley
2020-10-26 1:26 ` Andrew Jeffery
2020-10-26 2:03 ` Billy Tsai
2020-10-26 2:20 ` Andrew Jeffery
2020-10-26 2:56 ` Billy Tsai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).