From: Steven Lee <hidden> Date: 2021-06-08 10:26:01
AST2600 SoC has 2 SGPIO master interfaces one with 128 pins another one
with 80 pins, AST2500/AST2400 SoC has 1 SGPIO master interface that
supports up to 80 pins.
In the current driver design, the max number of sgpio pins is hardcoded
in macro MAX_NR_HW_SGPIO and the value is 80.
For supporting sgpio master interfaces of AST2600 SoC, the patch series
contains the following enhancement:
- Convert txt dt-bindings to yaml.
- Update aspeed-g6 dtsi to support the enhanced sgpio.
- Define max number of gpio pins in ast2600 platform data. Old chip
uses the original hardcoded value.
- Support muiltiple SGPIO master interfaces.
- Support up to 128 pins.
- Support wdt reset tolerance.
- Fix irq_chip issues which causes multiple sgpio devices use the same
irq_chip data.
- Replace all of_*() APIs with device_*().
Changes from v4:
* Remove ngpios from dtsi
* Add ast2400 and ast2500 platform data.
* Remove unused macros.
* Add ngpios check in a separate patch.
* Fix coding style issues.
Changes from v3:
* Split dt-bindings patch to 2 patches
* Rename ast2600-sgpiom1 compatible with ast2600-sgiom-128
* Rename ast2600-sgpiom2 compatible with ast2600-sgiom-80
* Correct the typo in commit messages.
* Fix coding style issues.
* Replace all of_*() APIs with device_*().
Changes from v2:
* Remove maximum/minimum of ngpios from bindings.
* Remove max-ngpios from bindings and dtsi.
* Remove ast2400-sgpiom and ast2500-sgpiom compatibles from dts and
driver.
* Add ast2600-sgpiom1 and ast2600-sgpiom2 compatibles as their max
number of available gpio pins are different.
* Modify functions to pass aspeed_sgpio struct instead of passing
max_ngpios.
* Split sgpio driver patch to 3 patches
Changes from v1:
* Fix yaml format issues.
* Fix issues reported by kernel test robot.
Please help to review.
Thanks,
Steven
Steven Lee (10):
dt-bindings: aspeed-sgpio: Convert txt bindings to yaml.
dt-bindings: aspeed-sgpio: Add ast2600 sgpio compatibles.
ARM: dts: aspeed-g6: Add SGPIO node.
ARM: dts: aspeed-g5: Remove ngpios from sgpio node.
gpio: gpio-aspeed-sgpio: Add AST2600 sgpio support
gpio: gpio-aspeed-sgpio: Add AST2400 and AST2500 platform data.
gpio: gpio-aspeed-sgpio: Add set_config function
gpio: gpio-aspeed-sgpio: Move irq_chip to aspeed-sgpio struct
gpio: gpio-aspeed-sgpio: Use generic device property APIs
gpio: gpio-aspeed-sgpio: Return error if ngpios is not multiple of 8.
.../bindings/gpio/aspeed,sgpio.yaml | 78 +++++++
.../devicetree/bindings/gpio/sgpio-aspeed.txt | 46 ----
arch/arm/boot/dts/aspeed-g5.dtsi | 1 -
arch/arm/boot/dts/aspeed-g6.dtsi | 28 +++
drivers/gpio/gpio-aspeed-sgpio.c | 206 ++++++++++++------
5 files changed, 248 insertions(+), 111 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
delete mode 100644 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
@@ -0,0 +1,75 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/gpio/aspeed,sgpio.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Aspeed SGPIO controller++maintainers:+-Andrew Jeffery <andrew@aj.id.au>++description:+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+support the following options+-Support interrupt option for each input port and various interrupt+sensitivity option (level-high, level-low, edge-high, edge-low)+-Support reset tolerance option for each output port+-Directly connected to APB bus and its shift clock is from APB bus clock+divided by a programmable value.+-Co-work with external signal-chained TTL components (74LV165/74LV595)++properties:+compatible:+enum:+-aspeed,ast2400-sgpio+-aspeed,ast2500-sgpio++reg:+maxItems:1++gpio-controller:true++'#gpio-cells':+const:2++interrupts:+maxItems:1++interrupt-controller:true++clocks:+maxItems:1++ngpios:true++bus-frequency:true++required:+-compatible+-reg+-gpio-controller+-'#gpio-cells'+-interrupts+-interrupt-controller+-ngpios+-clocks+-bus-frequency++additionalProperties:false++examples:+-|+#include <dt-bindings/clock/aspeed-clock.h>+sgpio:sgpio@1e780200 {+#gpio-cells = <2>;+compatible = "aspeed,ast2500-sgpio";+gpio-controller;+interrupts = <40>;+reg = <0x1e780200 0x0100>;+clocks = <&syscon ASPEED_CLK_APB>;+interrupt-controller;+ngpios = <80>;+bus-frequency = <12000000>;+};
@@ -1,46 +0,0 @@-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-support the following options:-- Support interrupt option for each input port and various interrupt- sensitivity option (level-high, level-low, edge-high, edge-low)-- Support reset tolerance option for each output port-- Directly connected to APB bus and its shift clock is from APB bus clock- divided by a programmable value.-- Co-work with external signal-chained TTL components (74LV165/74LV595)--Required properties:--- compatible : Should be one of- "aspeed,ast2400-sgpio", "aspeed,ast2500-sgpio"-- #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-- interrupts : Interrupt specifier, see interrupt-controller/interrupts.txt-- interrupt-controller : Mark the GPIO controller as an interrupt-controller-- ngpios : number of *hardware* GPIO lines, see gpio.txt. This will expose- 2 software GPIOs per hardware GPIO: one for hardware input, one for hardware- output. Up to 80 pins, must be a multiple of 8.-- clocks : A phandle to the APB clock for SGPM clock division-- bus-frequency : SGPM CLK frequency--The sgpio and interrupt properties are further described in their respective-bindings documentation:--- Documentation/devicetree/bindings/gpio/gpio.txt-- Documentation/devicetree/bindings/interrupt-controller/interrupts.txt-- Example:- sgpio: sgpio@1e780200 {- #gpio-cells = <2>;- compatible = "aspeed,ast2500-sgpio";- gpio-controller;- interrupts = <40>;- reg = <0x1e780200 0x0100>;- clocks = <&syscon ASPEED_CLK_APB>;- interrupt-controller;- ngpios = <8>;- bus-frequency = <12000000>;- };
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Steven Lee <hidden> Date: 2021-06-08 10:26:07
AST2600 SoC has 2 SGPIO master interfaces one with 128 pins another one
with 80 pins. Add ast2600-sgpiom0-80 and ast2600-sgpiom-128 compatibles
and update descriptions to introduce the max number of available gpio
pins that AST2600 supported.
Signed-off-by: Steven Lee <redacted>
Reviewed-by: Andrew Jeffery <redacted>
---
Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
@@ -10,9 +10,10 @@ maintainers:-Andrew Jeffery <andrew@aj.id.au>description:-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-support the following options+This SGPIO controller is for ASPEED AST2400, AST2500 and AST2600 SoC,+AST2600 have two sgpio master one with 128 pins another one with 80 pins,+AST2500/AST2400 have one sgpio master with 80 pins. Each of the Serial+GPIO pins can be programmed to support the following options-Support interrupt option for each input port and various interruptsensitivity option (level-high, level-low, edge-high, edge-low)-Support reset tolerance option for each output port
From: Steven Lee <hidden> Date: 2021-06-08 10:26:16
AST2600 supports 2 SGPIO master interfaces one with 128 pins another one
with 80 pins.
Signed-off-by: Steven Lee <redacted>
---
arch/arm/boot/dts/aspeed-g6.dtsi | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
From: Steven Lee <hidden> Date: 2021-06-08 10:26:18
Remove ngpios property from sgpio node as it should be defined in the
platform dts.
Signed-off-by: Steven Lee <redacted>
---
arch/arm/boot/dts/aspeed-g5.dtsi | 1 -
1 file changed, 1 deletion(-)
From: Steven Lee <hidden> Date: 2021-06-08 10:26:28
AST2600 SoC has 2 SGPIO master interfaces one with 128 pins another one
with 80 pins.
In the current driver, the maximum number of gpio pins of SoC is hardcoded
as 80 and the gpio pin count mask for GPIO Configuration register is
hardcode as GENMASK(9,6). In addition, some functions use the hardcoded
value to calculate the gpio offset.
The patch adds ast2600 compatibles and platform data that includes the
max number of gpio pins supported by ast2600 and gpio pin count mask for
GPIO Configuration register.
The patch also modifies some functions to pass aspeed_sgpio struct for
calculating gpio offset without using the hardcoded value.
Signed-off-by: Steven Lee <redacted>
---
drivers/gpio/gpio-aspeed-sgpio.c | 107 +++++++++++++++++++++----------
1 file changed, 73 insertions(+), 34 deletions(-)
@@ -139,18 +152,19 @@ static int aspeed_sgpio_init_valid_mask(struct gpio_chip *gc,unsignedlong*valid_mask,unsignedintngpios){structaspeed_sgpio*sgpio=gpiochip_get_data(gc);+intmax_ngpios=sgpio->max_ngpios;intn=sgpio->n_sgpio;-intc=SGPIO_OUTPUT_OFFSET-n;+intc=max_ngpios-n;-WARN_ON(ngpios<MAX_NR_HW_SGPIO*2);+WARN_ON(ngpios<max_ngpios*2);/* input GPIOs in the lower range */bitmap_set(valid_mask,0,n);bitmap_clear(valid_mask,n,c);-/* output GPIOS above SGPIO_OUTPUT_OFFSET */-bitmap_set(valid_mask,SGPIO_OUTPUT_OFFSET,n);-bitmap_clear(valid_mask,SGPIO_OUTPUT_OFFSET+n,c);+/* output GPIOS above max_ngpios */+bitmap_set(valid_mask,max_ngpios,n);+bitmap_clear(valid_mask,max_ngpios+n,c);return0;}
@@ -161,30 +175,30 @@ static void aspeed_sgpio_irq_init_valid_mask(struct gpio_chip *gc,structaspeed_sgpio*sgpio=gpiochip_get_data(gc);intn=sgpio->n_sgpio;-WARN_ON(ngpios<MAX_NR_HW_SGPIO*2);+WARN_ON(ngpios<sgpio->max_ngpios*2);/* input GPIOs in the lower range */bitmap_set(valid_mask,0,n);bitmap_clear(valid_mask,n,ngpios-n);}-staticboolaspeed_sgpio_is_input(unsignedintoffset)+staticboolaspeed_sgpio_is_input(unsignedintoffset,conststructaspeed_sgpio*gpio){-returnoffset<SGPIO_OUTPUT_OFFSET;+returnoffset<gpio->max_ngpios;}staticintaspeed_sgpio_get(structgpio_chip*gc,unsignedintoffset){structaspeed_sgpio*gpio=gpiochip_get_data(gc);-conststructaspeed_sgpio_bank*bank=to_bank(offset);+conststructaspeed_sgpio_bank*bank=to_bank(offset,gpio);unsignedlongflags;enumaspeed_sgpio_regreg;intrc=0;spin_lock_irqsave(&gpio->lock,flags);-reg=aspeed_sgpio_is_input(offset)?reg_val:reg_rdata;-rc=!!(ioread32(bank_reg(gpio,bank,reg))&GPIO_BIT(offset));+reg=aspeed_sgpio_is_input(offset,gpio)?reg_val:reg_rdata;+rc=!!(ioread32(bank_reg(gpio,bank,reg))&GPIO_BIT(gpio,offset));spin_unlock_irqrestore(&gpio->lock,flags);
@@ -194,11 +208,11 @@ static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)staticintsgpio_set_value(structgpio_chip*gc,unsignedintoffset,intval){structaspeed_sgpio*gpio=gpiochip_get_data(gc);-conststructaspeed_sgpio_bank*bank=to_bank(offset);+conststructaspeed_sgpio_bank*bank=to_bank(offset,gpio);void__iomem*addr_r,*addr_w;u32reg=0;-if(aspeed_sgpio_is_input(offset))+if(aspeed_sgpio_is_input(offset,gpio))return-EINVAL;/* Since this is an output, read the cached value from rdata, then
@@ -209,9 +223,9 @@ static int sgpio_set_value(struct gpio_chip *gc, unsigned int offset, int val)reg=ioread32(addr_r);if(val)-reg|=GPIO_BIT(offset);+reg|=GPIO_BIT(gpio,offset);else-reg&=~GPIO_BIT(offset);+reg&=~GPIO_BIT(gpio,offset);iowrite32(reg,addr_w);
@@ -232,7 +246,9 @@ static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int val)staticintaspeed_sgpio_dir_in(structgpio_chip*gc,unsignedintoffset){-returnaspeed_sgpio_is_input(offset)?0:-EINVAL;+structaspeed_sgpio*gpio=gpiochip_get_data(gc);++returnaspeed_sgpio_is_input(offset,gpio)?0:-EINVAL;}staticintaspeed_sgpio_dir_out(structgpio_chip*gc,unsignedintoffset,intval)
@@ -253,7 +269,9 @@ static int aspeed_sgpio_dir_out(struct gpio_chip *gc, unsigned int offset, int vstaticintaspeed_sgpio_get_direction(structgpio_chip*gc,unsignedintoffset){-return!!aspeed_sgpio_is_input(offset);+structaspeed_sgpio*gpio=gpiochip_get_data(gc);++return!!aspeed_sgpio_is_input(offset,gpio);}staticvoidirqd_to_aspeed_sgpio_data(structirq_data*d,
@@ -489,13 +520,22 @@ static int __init aspeed_sgpio_probe(struct platform_device *pdev)if(IS_ERR(gpio->base))returnPTR_ERR(gpio->base);+pdata=device_get_match_data(&pdev->dev);+if(pdata){+gpio->max_ngpios=pdata->max_ngpios;+pin_mask=pdata->pin_mask;+}else{+gpio->max_ngpios=MAX_NR_HW_SGPIO;+pin_mask=ASPEED_SGPIO_PINS_MASK;+}+rc=of_property_read_u32(pdev->dev.of_node,"ngpios",&nr_gpios);if(rc<0){dev_err(&pdev->dev,"Could not read ngpios property\n");return-EINVAL;-}elseif(nr_gpios>MAX_NR_HW_SGPIO){+}elseif(nr_gpios>gpio->max_ngpios){dev_err(&pdev->dev,"Number of GPIOs exceeds the maximum of %d: %d\n",-MAX_NR_HW_SGPIO,nr_gpios);+gpio->max_ngpios,nr_gpios);return-EINVAL;}gpio->n_sgpio=nr_gpios;
@@ -531,15 +571,14 @@ static int __init aspeed_sgpio_probe(struct platform_device *pdev)if(sgpio_clk_div>(1<<16)-1)return-EINVAL;-iowrite32(FIELD_PREP(ASPEED_SGPIO_CLK_DIV_MASK,sgpio_clk_div)|-FIELD_PREP(ASPEED_SGPIO_PINS_MASK,(nr_gpios/8))|-ASPEED_SGPIO_ENABLE,-gpio->base+ASPEED_SGPIO_CTRL);+gpio_cnt_regval=((nr_gpios/8)<<ASPEED_SGPIO_PINS_SHIFT)&pin_mask;+iowrite32(FIELD_PREP(ASPEED_SGPIO_CLK_DIV_MASK,sgpio_clk_div)|gpio_cnt_regval|+ASPEED_SGPIO_ENABLE,gpio->base+ASPEED_SGPIO_CTRL);spin_lock_init(&gpio->lock);gpio->chip.parent=&pdev->dev;-gpio->chip.ngpio=MAX_NR_HW_SGPIO*2;+gpio->chip.ngpio=gpio->max_ngpios*2;gpio->chip.init_valid_mask=aspeed_sgpio_init_valid_mask;gpio->chip.direction_input=aspeed_sgpio_dir_in;gpio->chip.direction_output=aspeed_sgpio_dir_out;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Steven Lee <hidden> Date: 2021-06-08 10:26:30
We use platform data to store GPIO pin mask and the max number of
available GPIO pins for AST2600.
Refactor driver to also add the platform data for AST2400/AST2500 and
remove unused MAX_NR_HW_SGPIO and ASPEED_SGPIO_PINS_MASK macros.
Signed-off-by: Steven Lee <redacted>
---
drivers/gpio/gpio-aspeed-sgpio.c | 34 +++++++++++---------------------
1 file changed, 12 insertions(+), 22 deletions(-)
From: Steven Lee <hidden> Date: 2021-06-08 10:26:43
The current design initializes irq->chip from a global irqchip struct,
which causes multiple sgpio devices use the same irq_chip.
The patch moves irq_chip to aspeed_sgpio struct for initializing
irq_chip from their private gpio struct.
Signed-off-by: Steven Lee <redacted>
Reviewed-by: Andrew Jeffery <redacted>
---
drivers/gpio/gpio-aspeed-sgpio.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
From: Steven Lee <hidden> Date: 2021-06-08 10:26:46
Replace all of_property_read_u32() with device_property_read_u32().
Signed-off-by: Steven Lee <redacted>
Acked-by: Andrew Jeffery <redacted>
---
drivers/gpio/gpio-aspeed-sgpio.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Steven Lee <hidden> Date: 2021-06-08 10:26:52
Add an else-if condition in the probe function to check whether ngpios is
multiple of 8.
Per AST datasheet, numbers of available serial GPIO pins in Serial GPIO
Configuration Register must be n bytes. For instance, if n = 1, it means
AST SoC supports 8 GPIO pins.
Signed-off-by: Steven Lee <redacted>
---
drivers/gpio/gpio-aspeed-sgpio.c | 4 ++++
1 file changed, 4 insertions(+)
@@ -572,6 +572,10 @@ static int __init aspeed_sgpio_probe(struct platform_device *pdev)dev_err(&pdev->dev,"Number of GPIOs exceeds the maximum of %d: %d\n",gpio->max_ngpios,nr_gpios);return-EINVAL;+}elseif(nr_gpios%8){+dev_err(&pdev->dev,"Number of GPIOs not multiple of 8: %d\n",+nr_gpios);+return-EINVAL;}gpio->n_sgpio=nr_gpios;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
@@ -0,0 +1,75 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/gpio/aspeed,sgpio.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Aspeed SGPIO controller++maintainers:+-Andrew Jeffery <andrew@aj.id.au>++description:+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
+ support the following options
+ - Support interrupt option for each input port and various interrupt
+ sensitivity option (level-high, level-low, edge-high, edge-low)
+ - Support reset tolerance option for each output port
+ - Directly connected to APB bus and its shift clock is from APB bus
clock
+ divided by a programmable value.
+ - Co-work with external signal-chained TTL components
(74LV165/74LV595)
+
+properties:
+ compatible:
+ enum:
+ - aspeed,ast2400-sgpio
+ - aspeed,ast2500-sgpio
+
+ reg:
+ maxItems: 1
+
+ gpio-controller: true
+
+ '#gpio-cells':
+ const: 2
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-controller: true
+
+ clocks:
+ maxItems: 1
+
+ ngpios: true
+
+ bus-frequency: true
I'm not familiar enough with dt-schema to know that this does what we need, so deferring to Rob.
Looks good otherwise.
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Andrew Jeffery <hidden> Date: 2021-06-09 00:44:16
On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote:
quoted hunk
AST2600 supports 2 SGPIO master interfaces one with 128 pins another one
with 80 pins.
Signed-off-by: Steven Lee <redacted>
---
arch/arm/boot/dts/aspeed-g6.dtsi | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
From: Andrew Jeffery <hidden> Date: 2021-06-09 00:45:25
On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote:
Remove ngpios property from sgpio node as it should be defined in the
platform dts.
Signed-off-by: Steven Lee <redacted>
I guess someone will make some noise if this breaks them.
Reviewed-by: Andrew Jeffery <redacted>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Andrew Jeffery <hidden> Date: 2021-06-09 00:52:51
On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote:
quoted hunk
AST2600 SoC has 2 SGPIO master interfaces one with 128 pins another one
with 80 pins.
In the current driver, the maximum number of gpio pins of SoC is hardcoded
as 80 and the gpio pin count mask for GPIO Configuration register is
hardcode as GENMASK(9,6). In addition, some functions use the hardcoded
value to calculate the gpio offset.
The patch adds ast2600 compatibles and platform data that includes the
max number of gpio pins supported by ast2600 and gpio pin count mask for
GPIO Configuration register.
The patch also modifies some functions to pass aspeed_sgpio struct for
calculating gpio offset without using the hardcoded value.
Signed-off-by: Steven Lee <redacted>
---
drivers/gpio/gpio-aspeed-sgpio.c | 107 +++++++++++++++++++++----------
1 file changed, 73 insertions(+), 34 deletions(-)
@@ -194,11 +208,11 @@ static int aspeed_sgpio_get(struct gpio_chip *gc,
unsigned int offset)
static int sgpio_set_value(struct gpio_chip *gc, unsigned int offset,
int val)
{
struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
- const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset, gpio);
void __iomem *addr_r, *addr_w;
u32 reg = 0;
- if (aspeed_sgpio_is_input(offset))
+ if (aspeed_sgpio_is_input(offset, gpio))
return -EINVAL;
/* Since this is an output, read the cached value from rdata, then
@@ -209,9 +223,9 @@ static int sgpio_set_value(struct gpio_chip *gc,
... because we should avoid introducing this branch if possible.
Aside from these two minor issues, it looks okay to me.
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Andrew Jeffery <hidden> Date: 2021-06-09 00:55:44
On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote:
quoted hunk
We use platform data to store GPIO pin mask and the max number of
available GPIO pins for AST2600.
Refactor driver to also add the platform data for AST2400/AST2500 and
remove unused MAX_NR_HW_SGPIO and ASPEED_SGPIO_PINS_MASK macros.
Signed-off-by: Steven Lee <redacted>
---
drivers/gpio/gpio-aspeed-sgpio.c | 34 +++++++++++---------------------
1 file changed, 12 insertions(+), 22 deletions(-)
Hmm, okay, maybe just re-order the patches so this commit comes before the previous one. That way we don't immediately rip out this condition that we just introduced in the previous patch.
I think I suggested squashing it into the previous patch, but with the removal of the comments and macros I think it's worth leaving it separate, just reordered.
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Andrew Jeffery <hidden> Date: 2021-06-09 00:56:34
On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote:
Add an else-if condition in the probe function to check whether ngpios is
multiple of 8.
Per AST datasheet, numbers of available serial GPIO pins in Serial GPIO
Configuration Register must be n bytes. For instance, if n = 1, it means
AST SoC supports 8 GPIO pins.
Signed-off-by: Steven Lee <redacted>
From: Steven Lee <hidden> Date: 2021-06-09 01:52:03
The 06/09/2021 08:43, Andrew Jeffery wrote:
On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote:
quoted
AST2600 supports 2 SGPIO master interfaces one with 128 pins another one
with 80 pins.
Signed-off-by: Steven Lee <redacted>
---
arch/arm/boot/dts/aspeed-g6.dtsi | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
The example in the binding document used ASPEED_CLK_APB. Which is correct? I assume ASPEED_CLK_APB2?
The example in the binding document is for aspeed-g5.
aspeed-g5 and aspeed-g6 use different clocks.
Should I add a new patch for adding an example for aspeed-g6?
From: Andrew Jeffery <hidden> Date: 2021-06-09 02:20:17
On Wed, 9 Jun 2021, at 11:21, Steven Lee wrote:
The 06/09/2021 08:43, Andrew Jeffery wrote:
quoted
On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote:
quoted
AST2600 supports 2 SGPIO master interfaces one with 128 pins another one
with 80 pins.
Signed-off-by: Steven Lee <redacted>
---
arch/arm/boot/dts/aspeed-g6.dtsi | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
The example in the binding document used ASPEED_CLK_APB. Which is correct? I assume ASPEED_CLK_APB2?
The example in the binding document is for aspeed-g5.
aspeed-g5 and aspeed-g6 use different clocks.
Should I add a new patch for adding an example for aspeed-g6?
Oh, I missed that. Never mind then!
Reviewed-by: Andrew Jeffery <redacted>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Steven Lee <hidden> Date: 2021-06-09 04:12:40
The 06/09/2021 08:55, Andrew Jeffery wrote:
On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote:
quoted
We use platform data to store GPIO pin mask and the max number of
available GPIO pins for AST2600.
Refactor driver to also add the platform data for AST2400/AST2500 and
remove unused MAX_NR_HW_SGPIO and ASPEED_SGPIO_PINS_MASK macros.
Signed-off-by: Steven Lee <redacted>
---
drivers/gpio/gpio-aspeed-sgpio.c | 34 +++++++++++---------------------
1 file changed, 12 insertions(+), 22 deletions(-)
Hmm, okay, maybe just re-order the patches so this commit comes before the previous one. That way we don't immediately rip out this condition that we just introduced in the previous patch.
I think I suggested squashing it into the previous patch, but with the removal of the comments and macros I think it's worth leaving it separate, just reordered.
I was wondering if I can squash patch-05 and patch-06 into one patch
as this patch(patch-06) requires macros, structures, and functions that
modified in the previous patch(patch-05).
Thanks,
Steven
From: Andrew Jeffery <hidden> Date: 2021-06-09 06:46:17
On Wed, 9 Jun 2021, at 13:42, Steven Lee wrote:
The 06/09/2021 08:55, Andrew Jeffery wrote:
quoted
On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote:
quoted
We use platform data to store GPIO pin mask and the max number of
available GPIO pins for AST2600.
Refactor driver to also add the platform data for AST2400/AST2500 and
remove unused MAX_NR_HW_SGPIO and ASPEED_SGPIO_PINS_MASK macros.
Signed-off-by: Steven Lee <redacted>
---
drivers/gpio/gpio-aspeed-sgpio.c | 34 +++++++++++---------------------
1 file changed, 12 insertions(+), 22 deletions(-)
Hmm, okay, maybe just re-order the patches so this commit comes before the previous one. That way we don't immediately rip out this condition that we just introduced in the previous patch.
I think I suggested squashing it into the previous patch, but with the removal of the comments and macros I think it's worth leaving it separate, just reordered.
I was wondering if I can squash patch-05 and patch-06 into one patch
as this patch(patch-06) requires macros, structures, and functions that
modified in the previous patch(patch-05).
On Tue, Jun 8, 2021 at 12:26 PM Steven Lee [off-list ref] wrote:
AST2600 SoC has 2 SGPIO master interfaces one with 128 pins another one
with 80 pins, AST2500/AST2400 SoC has 1 SGPIO master interface that
supports up to 80 pins.
In the current driver design, the max number of sgpio pins is hardcoded
in macro MAX_NR_HW_SGPIO and the value is 80.
For supporting sgpio master interfaces of AST2600 SoC, the patch series
contains the following enhancement:
- Convert txt dt-bindings to yaml.
- Update aspeed-g6 dtsi to support the enhanced sgpio.
- Define max number of gpio pins in ast2600 platform data. Old chip
uses the original hardcoded value.
- Support muiltiple SGPIO master interfaces.
- Support up to 128 pins.
- Support wdt reset tolerance.
- Fix irq_chip issues which causes multiple sgpio devices use the same
irq_chip data.
- Replace all of_*() APIs with device_*().
Changes from v4:
v5 looks good to me!
I just need Rob's or another DT persons nod on the bindings (or timeout)
before I merge it. Poke me if nothing happens.
These two need to be merged through the SoC tree, the rest I will handle.
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Steven Lee <hidden> Date: 2021-06-10 02:24:29
The 06/09/2021 18:54, Linus Walleij wrote:
On Tue, Jun 8, 2021 at 12:26 PM Steven Lee [off-list ref] wrote:
quoted
AST2600 SoC has 2 SGPIO master interfaces one with 128 pins another one
with 80 pins, AST2500/AST2400 SoC has 1 SGPIO master interface that
supports up to 80 pins.
In the current driver design, the max number of sgpio pins is hardcoded
in macro MAX_NR_HW_SGPIO and the value is 80.
For supporting sgpio master interfaces of AST2600 SoC, the patch series
contains the following enhancement:
- Convert txt dt-bindings to yaml.
- Update aspeed-g6 dtsi to support the enhanced sgpio.
- Define max number of gpio pins in ast2600 platform data. Old chip
uses the original hardcoded value.
- Support muiltiple SGPIO master interfaces.
- Support up to 128 pins.
- Support wdt reset tolerance.
- Fix irq_chip issues which causes multiple sgpio devices use the same
irq_chip data.
- Replace all of_*() APIs with device_*().
Changes from v4:
v5 looks good to me!
I just need Rob's or another DT persons nod on the bindings (or timeout)
before I merge it. Poke me if nothing happens.
These two need to be merged through the SoC tree, the rest I will handle.
Hi Linus, Andrew,
Per the comment in the following mail
https://lkml.org/lkml/2021/6/9/317
I was wondering if I should prepare v6 for the currnet solution or
I should drop this patch series then prepare another patch for the
new solution(piar GPIO input/output) which breaks userspace but is
better than the current solution.
Thanks,
Steven
On Thu, Jun 10, 2021 at 4:24 AM Steven Lee [off-list ref] wrote:
Per the comment in the following mail
https://lkml.org/lkml/2021/6/9/317
I was wondering if I should prepare v6 for the currnet solution or
I should drop this patch series then prepare another patch for the
new solution(piar GPIO input/output) which breaks userspace but is
better than the current solution.
I would say just go ahead with the new solution. AFAIK Aspeed
has pretty tight control over what kind of userspace run on these
systems.
BTW please influence Aspeed to use the GPIO character device
and ligpiod
https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/
if you are doing any kind of userspace GPIO control (which I
suspect that you do).
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Steven Lee <hidden> Date: 2021-06-10 08:39:45
The 06/10/2021 15:50, Linus Walleij wrote:
On Thu, Jun 10, 2021 at 4:24 AM Steven Lee [off-list ref] wrote:
quoted
Per the comment in the following mail
https://lkml.org/lkml/2021/6/9/317
I was wondering if I should prepare v6 for the currnet solution or
I should drop this patch series then prepare another patch for the
new solution(piar GPIO input/output) which breaks userspace but is
better than the current solution.
I would say just go ahead with the new solution. AFAIK Aspeed
has pretty tight control over what kind of userspace run on these
systems.
BTW please influence Aspeed to use the GPIO character device
and ligpiod
https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/
if you are doing any kind of userspace GPIO control (which I
suspect that you do).
We currently use gpioset and gpioget that provided by libgpiod to test
aspeed gpio and sgpio drivers.
For the current solution on AST2600,
the valid range of input pins is 0 ~ 127,
the valid range of output pins is 128 ~ 255.
So we access input pins by the following command
gpioget$chipId01234...127
and access output pins by the following command
gpioset$chipId128=1129=0130=1131=1...255=1
The new solution will change the gpio id order as follows
Input:
From: Rob Herring <robh@kernel.org> Date: 2021-06-10 16:23:27
On Tue, Jun 08, 2021 at 06:25:37PM +0800, Steven Lee wrote:
quoted hunk
AST2600 SoC has 2 SGPIO master interfaces one with 128 pins another one
with 80 pins. Add ast2600-sgpiom0-80 and ast2600-sgpiom-128 compatibles
and update descriptions to introduce the max number of available gpio
pins that AST2600 supported.
Signed-off-by: Steven Lee <redacted>
Reviewed-by: Andrew Jeffery <redacted>
---
Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
@@ -10,9 +10,10 @@ maintainers:-Andrew Jeffery <andrew@aj.id.au>description:-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-support the following options+This SGPIO controller is for ASPEED AST2400, AST2500 and AST2600 SoC,+AST2600 have two sgpio master one with 128 pins another one with 80 pins,+AST2500/AST2400 have one sgpio master with 80 pins. Each of the Serial+GPIO pins can be programmed to support the following options-Support interrupt option for each input port and various interruptsensitivity option (level-high, level-low, edge-high, edge-low)-Support reset tolerance option for each output port
If the number of GPIOs is the only difference, then I don't think you
should get rid of ngpios. It's one thing if it varies from one SoC to
the next, but if something is per instance we should have a property.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Andrew Jeffery <hidden> Date: 2021-06-10 23:28:00
On Fri, 11 Jun 2021, at 01:53, Rob Herring wrote:
On Tue, Jun 08, 2021 at 06:25:37PM +0800, Steven Lee wrote:
quoted
AST2600 SoC has 2 SGPIO master interfaces one with 128 pins another one
with 80 pins. Add ast2600-sgpiom0-80 and ast2600-sgpiom-128 compatibles
and update descriptions to introduce the max number of available gpio
pins that AST2600 supported.
Signed-off-by: Steven Lee <redacted>
Reviewed-by: Andrew Jeffery <redacted>
---
Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
@@ -10,9 +10,10 @@ maintainers:-Andrew Jeffery <andrew@aj.id.au>description:-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-support the following options+This SGPIO controller is for ASPEED AST2400, AST2500 and AST2600 SoC,+AST2600 have two sgpio master one with 128 pins another one with 80 pins,+AST2500/AST2400 have one sgpio master with 80 pins. Each of the Serial+GPIO pins can be programmed to support the following options-Support interrupt option for each input port and various interruptsensitivity option (level-high, level-low, edge-high, edge-low)-Support reset tolerance option for each output port
If the number of GPIOs is the only difference, then I don't think you
should get rid of ngpios. It's one thing if it varies from one SoC to
the next, but if something is per instance we should have a property.
There are two issues:
1. The maximum number of GPIOs supported by the controller
2. The maximum number of GPIOs supported by the platform
These are different because of what the controller does - here's some previous discussion on the topic:
https://lore.kernel.org/linux-gpio/f2875111-9ba9-43b7-b2a4-d00c8725f5a0@www.fastmail.com/
We've used ngpios to describe 2; this decision was made prior to the 2600 design - the SGPIO controller for both the 2400 and 2500 supported a maximum of 80 GPIOs. With the 2600 we have to differentiate between the two SGPIO controllers because they support a different maximum number of GPIOs. The proposed approach of different compatibles keeps the behaviour of ngpios the same across all controller implementations.
Cheers,
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Jun 9, 2021 at 8:46 AM Andrew Jeffery [off-list ref] wrote:
On Wed, 9 Jun 2021, at 13:42, Steven Lee wrote:
quoted
The 06/09/2021 08:55, Andrew Jeffery wrote:
quoted
On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote:
quoted
We use platform data to store GPIO pin mask and the max number of
available GPIO pins for AST2600.
Refactor driver to also add the platform data for AST2400/AST2500 and
remove unused MAX_NR_HW_SGPIO and ASPEED_SGPIO_PINS_MASK macros.
Signed-off-by: Steven Lee <redacted>
---
drivers/gpio/gpio-aspeed-sgpio.c | 34 +++++++++++---------------------
1 file changed, 12 insertions(+), 22 deletions(-)
Hmm, okay, maybe just re-order the patches so this commit comes before the previous one. That way we don't immediately rip out this condition that we just introduced in the previous patch.
I think I suggested squashing it into the previous patch, but with the removal of the comments and macros I think it's worth leaving it separate, just reordered.
I was wondering if I can squash patch-05 and patch-06 into one patch
as this patch(patch-06) requires macros, structures, and functions that
modified in the previous patch(patch-05).
Yeah, fair enough. Just squash them.
Cheers,
Andrew
I'm ready to pick this up as soon as you respin the series.
Bart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Steven Lee <hidden> Date: 2021-06-15 04:23:06
The 06/12/2021 03:02, Bartosz Golaszewski wrote:
On Wed, Jun 9, 2021 at 8:46 AM Andrew Jeffery [off-list ref] wrote:
quoted
On Wed, 9 Jun 2021, at 13:42, Steven Lee wrote:
quoted
The 06/09/2021 08:55, Andrew Jeffery wrote:
quoted
On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote:
quoted
We use platform data to store GPIO pin mask and the max number of
available GPIO pins for AST2600.
Refactor driver to also add the platform data for AST2400/AST2500 and
remove unused MAX_NR_HW_SGPIO and ASPEED_SGPIO_PINS_MASK macros.
Signed-off-by: Steven Lee <redacted>
---
drivers/gpio/gpio-aspeed-sgpio.c | 34 +++++++++++---------------------
1 file changed, 12 insertions(+), 22 deletions(-)
Hmm, okay, maybe just re-order the patches so this commit comes before the previous one. That way we don't immediately rip out this condition that we just introduced in the previous patch.
I think I suggested squashing it into the previous patch, but with the removal of the comments and macros I think it's worth leaving it separate, just reordered.
I was wondering if I can squash patch-05 and patch-06 into one patch
as this patch(patch-06) requires macros, structures, and functions that
modified in the previous patch(patch-05).
Yeah, fair enough. Just squash them.
Cheers,
Andrew
I'm ready to pick this up as soon as you respin the series.
Hi Bart,
Per the discussion in the following mail threads, I may redesign
aspeed sgpio driver for the new solution.
https://lkml.org/lkml/2021/6/3/1507https://lkml.org/lkml/2021/6/10/240
Patch02- Patch06 of this patch series will need to be modified for
the new solution, although some of them have Reviewed-by tag.
Thanks,
Steven
From: Rob Herring <robh@kernel.org> Date: 2021-06-16 15:21:02
On Thu, Jun 10, 2021 at 5:27 PM Andrew Jeffery [off-list ref] wrote:
On Fri, 11 Jun 2021, at 01:53, Rob Herring wrote:
quoted
On Tue, Jun 08, 2021 at 06:25:37PM +0800, Steven Lee wrote:
quoted
AST2600 SoC has 2 SGPIO master interfaces one with 128 pins another one
with 80 pins. Add ast2600-sgpiom0-80 and ast2600-sgpiom-128 compatibles
and update descriptions to introduce the max number of available gpio
pins that AST2600 supported.
Signed-off-by: Steven Lee <redacted>
Reviewed-by: Andrew Jeffery <redacted>
---
Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
@@ -10,9 +10,10 @@ maintainers:-Andrew Jeffery <andrew@aj.id.au>description:-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-support the following options+This SGPIO controller is for ASPEED AST2400, AST2500 and AST2600 SoC,+AST2600 have two sgpio master one with 128 pins another one with 80 pins,+AST2500/AST2400 have one sgpio master with 80 pins. Each of the Serial+GPIO pins can be programmed to support the following options-Support interrupt option for each input port and various interruptsensitivity option (level-high, level-low, edge-high, edge-low)-Support reset tolerance option for each output port
If the number of GPIOs is the only difference, then I don't think you
should get rid of ngpios. It's one thing if it varies from one SoC to
the next, but if something is per instance we should have a property.
There are two issues:
1. The maximum number of GPIOs supported by the controller
2. The maximum number of GPIOs supported by the platform
These are different because of what the controller does - here's some previous discussion on the topic:
https://lore.kernel.org/linux-gpio/f2875111-9ba9-43b7-b2a4-d00c8725f5a0@www.fastmail.com/
We've used ngpios to describe 2; this decision was made prior to the 2600 design - the SGPIO controller for both the 2400 and 2500 supported a maximum of 80 GPIOs. With the 2600 we have to differentiate between the two SGPIO controllers because they support a different maximum number of GPIOs. The proposed approach of different compatibles keeps the behaviour of ngpios the same across all controller implementations.
Okay, that makes sense.
Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel