* [PATCH v2 0/4] Add Aspeed G7 gpio support
@ 2024-08-30 3:40 Billy Tsai
2024-08-30 3:40 ` [PATCH v2 1/4] dt-bindings: gpio: aspeed,ast2400-gpio: Support ast2700 Billy Tsai
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Billy Tsai @ 2024-08-30 3:40 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, andrew,
linux-gpio, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel, BMC-SW
The Aspeed 7th generation SoC features two GPIO controllers: one with 12
GPIO pins and another with 216 GPIO pins. The main difference from the
previous generation is that the control logic has been updated to support
per-pin control, allowing each pin to have its own 32-bit register for
configuring value, direction, interrupt type, and more.
This patch serial also add low-level operations (llops) to abstract the
register access for GPIO registers and the coprocessor request/release in
gpio-aspeed.c making it easier to extend the driver to support different
hardware register layouts.
Changes since v1:
- Merge the gpio-aspeed-g7.c into the gpio-aspeed.c.
- Create the llops in gpio-aspeed.c for flexibility.
Billy Tsai (4):
dt-bindings: gpio: aspeed,ast2400-gpio: Support ast2700
gpio: aspeed: Remove the name for bank array
gpio: aspeed: Create llops to handle hardware access
gpio: aspeed: Support G7 Aspeed gpio controller
.../bindings/gpio/aspeed,ast2400-gpio.yaml | 46 +-
drivers/gpio/gpio-aspeed.c | 442 ++++++++++--------
2 files changed, 303 insertions(+), 185 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v2 1/4] dt-bindings: gpio: aspeed,ast2400-gpio: Support ast2700 2024-08-30 3:40 [PATCH v2 0/4] Add Aspeed G7 gpio support Billy Tsai @ 2024-08-30 3:40 ` Billy Tsai 2024-08-30 14:04 ` Conor Dooley 2024-08-31 5:49 ` Krzysztof Kozlowski 2024-08-30 3:40 ` [PATCH v2 2/4] gpio: aspeed: Remove the name for bank array Billy Tsai ` (2 subsequent siblings) 3 siblings, 2 replies; 15+ messages in thread From: Billy Tsai @ 2024-08-30 3:40 UTC (permalink / raw) To: linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, andrew, linux-gpio, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, BMC-SW The AST2700 is the 7th generation SoC from Aspeed, featuring two GPIO controllers: one with 12 GPIO pins and another with 216 GPIO pins. Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> --- .../bindings/gpio/aspeed,ast2400-gpio.yaml | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/gpio/aspeed,ast2400-gpio.yaml b/Documentation/devicetree/bindings/gpio/aspeed,ast2400-gpio.yaml index cf11aa7ec8c7..4d439972c14b 100644 --- a/Documentation/devicetree/bindings/gpio/aspeed,ast2400-gpio.yaml +++ b/Documentation/devicetree/bindings/gpio/aspeed,ast2400-gpio.yaml @@ -15,6 +15,7 @@ properties: - aspeed,ast2400-gpio - aspeed,ast2500-gpio - aspeed,ast2600-gpio + - aspeed,ast2700-gpio reg: maxItems: 1 @@ -42,7 +43,7 @@ properties: const: 2 ngpios: - minimum: 36 + minimum: 12 maximum: 232 required: @@ -93,6 +94,20 @@ allOf: enum: [ 36, 208 ] required: - ngpios + - if: + properties: + compatible: + contains: + const: aspeed,ast2700-gpio + then: + properties: + gpio-line-names: + minItems: 12 + maxItems: 216 + ngpios: + enum: [ 12, 216 ] + required: + - ngpios additionalProperties: false @@ -146,3 +161,32 @@ examples: gpio-ranges = <&pinctrl 0 208 36>; ngpios = <36>; }; + - | + soc { + #address-cells = <2>; + #size-cells = <2>; + #include <dt-bindings/interrupt-controller/arm-gic.h> + gpio2: gpio@14c0b000 { + compatible = "aspeed,ast2700-gpio"; + reg = <0x0 0x14c0b000 0x0 0x1000>; + interrupts-extended = <&soc1_intc2 18>; + interrupt-controller; + #interrupt-cells = <2>; + #gpio-cells = <2>; + gpio-controller; + gpio-ranges = <&pinctrl1 0 0 216>; + ngpios = <216>; + }; + + gpio3: gpio@12c11000 { + compatible = "aspeed,ast2700-gpio"; + reg = <0x0 0x12c11000 0x0 0x1000>; + interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>; + interrupt-controller; + #interrupt-cells = <2>; + #gpio-cells = <2>; + gpio-controller; + gpio-ranges = <&pinctrl0 0 0 12>; + ngpios = <12>; + }; + }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: gpio: aspeed,ast2400-gpio: Support ast2700 2024-08-30 3:40 ` [PATCH v2 1/4] dt-bindings: gpio: aspeed,ast2400-gpio: Support ast2700 Billy Tsai @ 2024-08-30 14:04 ` Conor Dooley 2024-09-02 2:32 ` Billy Tsai 2024-08-31 5:49 ` Krzysztof Kozlowski 1 sibling, 1 reply; 15+ messages in thread From: Conor Dooley @ 2024-08-30 14:04 UTC (permalink / raw) To: Billy Tsai Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, andrew, linux-gpio, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, BMC-SW [-- Attachment #1: Type: text/plain, Size: 1158 bytes --] On Fri, Aug 30, 2024 at 11:40:44AM +0800, Billy Tsai wrote: > The AST2700 is the 7th generation SoC from Aspeed, featuring two GPIO > controllers: one with 12 GPIO pins and another with 216 GPIO pins. > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> > --- > .../bindings/gpio/aspeed,ast2400-gpio.yaml | 46 ++++++++++++++++++- > 1 file changed, 45 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/gpio/aspeed,ast2400-gpio.yaml b/Documentation/devicetree/bindings/gpio/aspeed,ast2400-gpio.yaml > index cf11aa7ec8c7..4d439972c14b 100644 > --- a/Documentation/devicetree/bindings/gpio/aspeed,ast2400-gpio.yaml > +++ b/Documentation/devicetree/bindings/gpio/aspeed,ast2400-gpio.yaml > @@ -15,6 +15,7 @@ properties: > - aspeed,ast2400-gpio > - aspeed,ast2500-gpio > - aspeed,ast2600-gpio > + - aspeed,ast2700-gpio > > reg: > maxItems: 1 > @@ -42,7 +43,7 @@ properties: > const: 2 > > ngpios: > - minimum: 36 > + minimum: 12 > maximum: 232 Aren't you missing a corresponding update to gpio-line-names, which has a minimum of 36? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: gpio: aspeed,ast2400-gpio: Support ast2700 2024-08-30 14:04 ` Conor Dooley @ 2024-09-02 2:32 ` Billy Tsai 0 siblings, 0 replies; 15+ messages in thread From: Billy Tsai @ 2024-09-02 2:32 UTC (permalink / raw) To: Conor Dooley Cc: linus.walleij@linaro.org, brgl@bgdev.pl, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, joel@jms.id.au, andrew@codeconstruct.com.au, linux-gpio@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org, BMC-SW > > The AST2700 is the 7th generation SoC from Aspeed, featuring two GPIO > > controllers: one with 12 GPIO pins and another with 216 GPIO pins. > > > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> > > --- > > .../bindings/gpio/aspeed,ast2400-gpio.yaml | 46 ++++++++++++++++++- > > 1 file changed, 45 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/gpio/aspeed,ast2400-gpio.yaml b/Documentation/devicetree/bindings/gpio/aspeed,ast2400-gpio.yaml > > index cf11aa7ec8c7..4d439972c14b 100644 > > --- a/Documentation/devicetree/bindings/gpio/aspeed,ast2400-gpio.yaml > > +++ b/Documentation/devicetree/bindings/gpio/aspeed,ast2400-gpio.yaml > > @@ -15,6 +15,7 @@ properties: > > - aspeed,ast2400-gpio > > - aspeed,ast2500-gpio > > - aspeed,ast2600-gpio > > + - aspeed,ast2700-gpio > > > > reg: > > maxItems: 1 > > @@ -42,7 +43,7 @@ properties: > > const: 2 > > > > ngpios: > > - minimum: 36 > > + minimum: 12 > > maximum: 232 > Aren't you missing a corresponding update to gpio-line-names, which has > a minimum of 36? Yes, I missed updating the minimum of the gpio-line-names to 12. Thanks, I’ll fix it in the next version of the patch. Best regards, Billy Tsai ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: gpio: aspeed,ast2400-gpio: Support ast2700 2024-08-30 3:40 ` [PATCH v2 1/4] dt-bindings: gpio: aspeed,ast2400-gpio: Support ast2700 Billy Tsai 2024-08-30 14:04 ` Conor Dooley @ 2024-08-31 5:49 ` Krzysztof Kozlowski 1 sibling, 0 replies; 15+ messages in thread From: Krzysztof Kozlowski @ 2024-08-31 5:49 UTC (permalink / raw) To: Billy Tsai, linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, andrew, linux-gpio, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, BMC-SW On 30/08/2024 05:40, Billy Tsai wrote: > The AST2700 is the 7th generation SoC from Aspeed, featuring two GPIO > controllers: one with 12 GPIO pins and another with 216 GPIO pins. > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> > --- > .../bindings/gpio/aspeed,ast2400-gpio.yaml | 46 ++++++++++++++++++- > 1 file changed, 45 insertions(+), 1 deletion(-) > One more Aspeed patch where you ignore review feedback. I think almost all (except one) Aspeed submissions were doing this: ignoring reviewers. <form letter> This is a friendly reminder during the review process. It seems my or other reviewer's previous comments were not fully addressed. Maybe the feedback got lost between the quotes, maybe you just forgot to apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them. Thank you. </form letter> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/4] gpio: aspeed: Remove the name for bank array 2024-08-30 3:40 [PATCH v2 0/4] Add Aspeed G7 gpio support Billy Tsai 2024-08-30 3:40 ` [PATCH v2 1/4] dt-bindings: gpio: aspeed,ast2400-gpio: Support ast2700 Billy Tsai @ 2024-08-30 3:40 ` Billy Tsai 2024-09-12 7:51 ` Andrew Jeffery 2024-08-30 3:40 ` [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access Billy Tsai 2024-08-30 3:40 ` [PATCH v2 4/4] gpio: aspeed: Support G7 Aspeed gpio controller Billy Tsai 3 siblings, 1 reply; 15+ messages in thread From: Billy Tsai @ 2024-08-30 3:40 UTC (permalink / raw) To: linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, andrew, linux-gpio, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, BMC-SW The bank array name is only used to determine if the GPIO offset is valid, and this condition can be replaced by checking if the offset exceeds the ngpio property. Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> --- drivers/gpio/gpio-aspeed.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c index 04c03402db6d..24f50a0ea4ab 100644 --- a/drivers/gpio/gpio-aspeed.c +++ b/drivers/gpio/gpio-aspeed.c @@ -77,7 +77,6 @@ struct aspeed_gpio_bank { uint16_t debounce_regs; uint16_t tolerance_regs; uint16_t cmdsrc_regs; - const char names[4][3]; }; /* @@ -104,7 +103,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { .debounce_regs = 0x0040, .tolerance_regs = 0x001c, .cmdsrc_regs = 0x0060, - .names = { "A", "B", "C", "D" }, }, { .val_regs = 0x0020, @@ -113,7 +111,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { .debounce_regs = 0x0048, .tolerance_regs = 0x003c, .cmdsrc_regs = 0x0068, - .names = { "E", "F", "G", "H" }, }, { .val_regs = 0x0070, @@ -122,7 +119,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { .debounce_regs = 0x00b0, .tolerance_regs = 0x00ac, .cmdsrc_regs = 0x0090, - .names = { "I", "J", "K", "L" }, }, { .val_regs = 0x0078, @@ -131,7 +127,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { .debounce_regs = 0x0100, .tolerance_regs = 0x00fc, .cmdsrc_regs = 0x00e0, - .names = { "M", "N", "O", "P" }, }, { .val_regs = 0x0080, @@ -140,7 +135,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { .debounce_regs = 0x0130, .tolerance_regs = 0x012c, .cmdsrc_regs = 0x0110, - .names = { "Q", "R", "S", "T" }, }, { .val_regs = 0x0088, @@ -149,7 +143,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { .debounce_regs = 0x0160, .tolerance_regs = 0x015c, .cmdsrc_regs = 0x0140, - .names = { "U", "V", "W", "X" }, }, { .val_regs = 0x01E0, @@ -158,7 +151,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { .debounce_regs = 0x0190, .tolerance_regs = 0x018c, .cmdsrc_regs = 0x0170, - .names = { "Y", "Z", "AA", "AB" }, }, { .val_regs = 0x01e8, @@ -167,7 +159,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { .debounce_regs = 0x01c0, .tolerance_regs = 0x01bc, .cmdsrc_regs = 0x01a0, - .names = { "AC", "", "", "" }, }, }; @@ -280,11 +271,11 @@ static inline const struct aspeed_bank_props *find_bank_props( static inline bool have_gpio(struct aspeed_gpio *gpio, unsigned int offset) { const struct aspeed_bank_props *props = find_bank_props(gpio, offset); - const struct aspeed_gpio_bank *bank = to_bank(offset); - unsigned int group = GPIO_OFFSET(offset) / 8; - return bank->names[group][0] != '\0' && - (!props || ((props->input | props->output) & GPIO_BIT(offset))); + if (offset > gpio->chip.ngpio) + return false; + + return (!props || ((props->input | props->output) & GPIO_BIT(offset))); } static inline bool have_input(struct aspeed_gpio *gpio, unsigned int offset) -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] gpio: aspeed: Remove the name for bank array 2024-08-30 3:40 ` [PATCH v2 2/4] gpio: aspeed: Remove the name for bank array Billy Tsai @ 2024-09-12 7:51 ` Andrew Jeffery 2024-09-12 7:57 ` Billy Tsai 0 siblings, 1 reply; 15+ messages in thread From: Andrew Jeffery @ 2024-09-12 7:51 UTC (permalink / raw) To: Billy Tsai, linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, linux-gpio, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, BMC-SW On Fri, 2024-08-30 at 11:40 +0800, Billy Tsai wrote: > The bank array name is only used to determine if the GPIO offset is valid, > and this condition can be replaced by checking if the offset exceeds the > ngpio property. > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> > --- > drivers/gpio/gpio-aspeed.c | 17 ++++------------- > 1 file changed, 4 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c > index 04c03402db6d..24f50a0ea4ab 100644 > --- a/drivers/gpio/gpio-aspeed.c > +++ b/drivers/gpio/gpio-aspeed.c > @@ -77,7 +77,6 @@ struct aspeed_gpio_bank { > uint16_t debounce_regs; > uint16_t tolerance_regs; > uint16_t cmdsrc_regs; > - const char names[4][3]; > }; > > /* > @@ -104,7 +103,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > .debounce_regs = 0x0040, > .tolerance_regs = 0x001c, > .cmdsrc_regs = 0x0060, > - .names = { "A", "B", "C", "D" }, > }, > { > .val_regs = 0x0020, > @@ -113,7 +111,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > .debounce_regs = 0x0048, > .tolerance_regs = 0x003c, > .cmdsrc_regs = 0x0068, > - .names = { "E", "F", "G", "H" }, > }, > { > .val_regs = 0x0070, > @@ -122,7 +119,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > .debounce_regs = 0x00b0, > .tolerance_regs = 0x00ac, > .cmdsrc_regs = 0x0090, > - .names = { "I", "J", "K", "L" }, > }, > { > .val_regs = 0x0078, > @@ -131,7 +127,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > .debounce_regs = 0x0100, > .tolerance_regs = 0x00fc, > .cmdsrc_regs = 0x00e0, > - .names = { "M", "N", "O", "P" }, > }, > { > .val_regs = 0x0080, > @@ -140,7 +135,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > .debounce_regs = 0x0130, > .tolerance_regs = 0x012c, > .cmdsrc_regs = 0x0110, > - .names = { "Q", "R", "S", "T" }, > }, > { > .val_regs = 0x0088, > @@ -149,7 +143,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > .debounce_regs = 0x0160, > .tolerance_regs = 0x015c, > .cmdsrc_regs = 0x0140, > - .names = { "U", "V", "W", "X" }, > }, > { > .val_regs = 0x01E0, > @@ -158,7 +151,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > .debounce_regs = 0x0190, > .tolerance_regs = 0x018c, > .cmdsrc_regs = 0x0170, > - .names = { "Y", "Z", "AA", "AB" }, > }, > { > .val_regs = 0x01e8, > @@ -167,7 +159,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > .debounce_regs = 0x01c0, > .tolerance_regs = 0x01bc, > .cmdsrc_regs = 0x01a0, > - .names = { "AC", "", "", "" }, > }, > }; > > @@ -280,11 +271,11 @@ static inline const struct aspeed_bank_props *find_bank_props( > static inline bool have_gpio(struct aspeed_gpio *gpio, unsigned int offset) > { > const struct aspeed_bank_props *props = find_bank_props(gpio, offset); > - const struct aspeed_gpio_bank *bank = to_bank(offset); > - unsigned int group = GPIO_OFFSET(offset) / 8; > > - return bank->names[group][0] != '\0' && > - (!props || ((props->input | props->output) & GPIO_BIT(offset))); > + if (offset > gpio->chip.ngpio) Should this be `>=`? ngpio is a count. Andrew > + return false; > + > + return (!props || ((props->input | props->output) & GPIO_BIT(offset))); > } > > static inline bool have_input(struct aspeed_gpio *gpio, unsigned int offset) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] gpio: aspeed: Remove the name for bank array 2024-09-12 7:51 ` Andrew Jeffery @ 2024-09-12 7:57 ` Billy Tsai 0 siblings, 0 replies; 15+ messages in thread From: Billy Tsai @ 2024-09-12 7:57 UTC (permalink / raw) To: Andrew Jeffery, linus.walleij@linaro.org, brgl@bgdev.pl, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, joel@jms.id.au, linux-gpio@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org, BMC-SW > > The bank array name is only used to determine if the GPIO offset is valid, > > and this condition can be replaced by checking if the offset exceeds the > > ngpio property. > > > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> > > --- > > drivers/gpio/gpio-aspeed.c | 17 ++++------------- > > 1 file changed, 4 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c > > index 04c03402db6d..24f50a0ea4ab 100644 > > --- a/drivers/gpio/gpio-aspeed.c > > +++ b/drivers/gpio/gpio-aspeed.c > > @@ -77,7 +77,6 @@ struct aspeed_gpio_bank { > > uint16_t debounce_regs; > > uint16_t tolerance_regs; > > uint16_t cmdsrc_regs; > > - const char names[4][3]; > > }; > > > > /* > > @@ -104,7 +103,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > > .debounce_regs = 0x0040, > > .tolerance_regs = 0x001c, > > .cmdsrc_regs = 0x0060, > > - .names = { "A", "B", "C", "D" }, > > }, > > { > > .val_regs = 0x0020, > > @@ -113,7 +111,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > > .debounce_regs = 0x0048, > > .tolerance_regs = 0x003c, > > .cmdsrc_regs = 0x0068, > > - .names = { "E", "F", "G", "H" }, > > }, > > { > > .val_regs = 0x0070, > > @@ -122,7 +119,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > > .debounce_regs = 0x00b0, > > .tolerance_regs = 0x00ac, > > .cmdsrc_regs = 0x0090, > > - .names = { "I", "J", "K", "L" }, > > }, > > { > > .val_regs = 0x0078, > > @@ -131,7 +127,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > > .debounce_regs = 0x0100, > > .tolerance_regs = 0x00fc, > > .cmdsrc_regs = 0x00e0, > > - .names = { "M", "N", "O", "P" }, > > }, > > { > > .val_regs = 0x0080, > > @@ -140,7 +135,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > > .debounce_regs = 0x0130, > > .tolerance_regs = 0x012c, > > .cmdsrc_regs = 0x0110, > > - .names = { "Q", "R", "S", "T" }, > > }, > > { > > .val_regs = 0x0088, > > @@ -149,7 +143,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > > .debounce_regs = 0x0160, > > .tolerance_regs = 0x015c, > > .cmdsrc_regs = 0x0140, > > - .names = { "U", "V", "W", "X" }, > > }, > > { > > .val_regs = 0x01E0, > > @@ -158,7 +151,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > > .debounce_regs = 0x0190, > > .tolerance_regs = 0x018c, > > .cmdsrc_regs = 0x0170, > > - .names = { "Y", "Z", "AA", "AB" }, > > }, > > { > > .val_regs = 0x01e8, > > @@ -167,7 +159,6 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = { > > .debounce_regs = 0x01c0, > > .tolerance_regs = 0x01bc, > > .cmdsrc_regs = 0x01a0, > > - .names = { "AC", "", "", "" }, > > }, > > }; > > > > @@ -280,11 +271,11 @@ static inline const struct aspeed_bank_props *find_bank_props( > > static inline bool have_gpio(struct aspeed_gpio *gpio, unsigned int offset) > > { > > const struct aspeed_bank_props *props = find_bank_props(gpio, offset); > > - const struct aspeed_gpio_bank *bank = to_bank(offset); > > - unsigned int group = GPIO_OFFSET(offset) / 8; > > > > - return bank->names[group][0] != '\0' && > > - (!props || ((props->input | props->output) & GPIO_BIT(offset))); > > + if (offset > gpio->chip.ngpio) > Should this be `>=`? ngpio is a count. Yes, it should be `>=`. I will fix it. Thanks > > + return false; > > + > > + return (!props || ((props->input | props->output) & GPIO_BIT(offset))); > > } > > > > static inline bool have_input(struct aspeed_gpio *gpio, unsigned int offset) ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access 2024-08-30 3:40 [PATCH v2 0/4] Add Aspeed G7 gpio support Billy Tsai 2024-08-30 3:40 ` [PATCH v2 1/4] dt-bindings: gpio: aspeed,ast2400-gpio: Support ast2700 Billy Tsai 2024-08-30 3:40 ` [PATCH v2 2/4] gpio: aspeed: Remove the name for bank array Billy Tsai @ 2024-08-30 3:40 ` Billy Tsai 2024-08-30 16:07 ` kernel test robot ` (3 more replies) 2024-08-30 3:40 ` [PATCH v2 4/4] gpio: aspeed: Support G7 Aspeed gpio controller Billy Tsai 3 siblings, 4 replies; 15+ messages in thread From: Billy Tsai @ 2024-08-30 3:40 UTC (permalink / raw) To: linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, andrew, linux-gpio, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, BMC-SW Add low-level operations (llops) to abstract the register access for GPIO registers and the coprocessor request/release. With this abstraction layer, the driver can separate the hardware and software logic, making it easier to extend the driver to support different hardware register layouts. Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> --- drivers/gpio/gpio-aspeed.c | 309 +++++++++++++++++-------------------- 1 file changed, 138 insertions(+), 171 deletions(-) diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c index 24f50a0ea4ab..74c4e80958bf 100644 --- a/drivers/gpio/gpio-aspeed.c +++ b/drivers/gpio/gpio-aspeed.c @@ -39,6 +39,7 @@ struct aspeed_bank_props { struct aspeed_gpio_config { unsigned int nr_gpios; const struct aspeed_bank_props *props; + unsigned int version; }; /* @@ -62,10 +63,13 @@ struct aspeed_gpio { u8 *offset_timer; unsigned int timer_users[4]; + const int *debounce_timers_array; + int debounce_timers_num; struct clk *clk; u32 *dcache; u8 *cf_copro_bankmap; + const struct aspeed_gpio_llops *llops; }; struct aspeed_gpio_bank { @@ -178,6 +182,15 @@ enum aspeed_gpio_reg { reg_cmdsrc1, }; +struct aspeed_gpio_llops { + bool (*copro_request)(struct aspeed_gpio *gpio, unsigned int offset); + void (*copro_release)(struct aspeed_gpio *gpio, unsigned int offset); + void (*reg_bits_set)(struct aspeed_gpio *gpio, unsigned int offset, + const enum aspeed_gpio_reg reg, u32 val); + u32 (*reg_bits_read)(struct aspeed_gpio *gpio, unsigned int offset, + const enum aspeed_gpio_reg reg); +}; + #define GPIO_VAL_VALUE 0x00 #define GPIO_VAL_DIR 0x04 @@ -237,10 +250,6 @@ static inline void __iomem *bank_reg(struct aspeed_gpio *gpio, #define GPIO_OFFSET(x) ((x) & 0x1f) #define GPIO_BIT(x) BIT(GPIO_OFFSET(x)) -#define _GPIO_SET_DEBOUNCE(t, o, i) ((!!((t) & BIT(i))) << GPIO_OFFSET(o)) -#define GPIO_SET_DEBOUNCE1(t, o) _GPIO_SET_DEBOUNCE(t, o, 1) -#define GPIO_SET_DEBOUNCE2(t, o) _GPIO_SET_DEBOUNCE(t, o, 0) - static const struct aspeed_gpio_bank *to_bank(unsigned int offset) { unsigned int bank = GPIO_BANK(offset); @@ -296,35 +305,17 @@ static inline bool have_output(struct aspeed_gpio *gpio, unsigned int offset) } static void aspeed_gpio_change_cmd_source(struct aspeed_gpio *gpio, - const struct aspeed_gpio_bank *bank, - int bindex, int cmdsrc) + unsigned int offset, + int cmdsrc) { - void __iomem *c0 = bank_reg(gpio, bank, reg_cmdsrc0); - void __iomem *c1 = bank_reg(gpio, bank, reg_cmdsrc1); - u32 bit, reg; - /* - * Each register controls 4 banks, so take the bottom 2 - * bits of the bank index, and use them to select the - * right control bit (0, 8, 16 or 24). + * The command source register is only valid in bits 0, 8, 16, and 24, so we use + * (offset & ~(0x7)) to ensure that reg_bits_set always targets a valid bit. */ - bit = BIT((bindex & 3) << 3); - /* Source 1 first to avoid illegal 11 combination */ - reg = ioread32(c1); - if (cmdsrc & 2) - reg |= bit; - else - reg &= ~bit; - iowrite32(reg, c1); - + gpio->llops->reg_bits_set(gpio, offset & ~(0x7), reg_cmdsrc1, !!(cmdsrc & BIT(1))); /* Then Source 0 */ - reg = ioread32(c0); - if (cmdsrc & 1) - reg |= bit; - else - reg &= ~bit; - iowrite32(reg, c0); + gpio->llops->reg_bits_set(gpio, offset & ~(0x7), reg_cmdsrc0, !!(cmdsrc & BIT(0))); } static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio, @@ -343,7 +334,7 @@ static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio, copro_ops->request_access(copro_data); /* Change command source back to ARM */ - aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3, GPIO_CMDSRC_ARM); + aspeed_gpio_change_cmd_source(gpio, offset, GPIO_CMDSRC_ARM); /* Update cache */ gpio->dcache[GPIO_BANK(offset)] = ioread32(bank_reg(gpio, bank, reg_rdata)); @@ -354,8 +345,6 @@ static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio, static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio, unsigned int offset) { - const struct aspeed_gpio_bank *bank = to_bank(offset); - if (!copro_ops || !gpio->cf_copro_bankmap) return; if (!gpio->cf_copro_bankmap[offset >> 3]) @@ -364,7 +353,7 @@ static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio, return; /* Change command source back to ColdFire */ - aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3, + aspeed_gpio_change_cmd_source(gpio, offset, GPIO_CMDSRC_COLDFIRE); /* Restart the coprocessor */ @@ -374,29 +363,24 @@ static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio, static int aspeed_gpio_get(struct gpio_chip *gc, unsigned int offset) { struct aspeed_gpio *gpio = gpiochip_get_data(gc); - const struct aspeed_gpio_bank *bank = to_bank(offset); - return !!(ioread32(bank_reg(gpio, bank, reg_val)) & GPIO_BIT(offset)); + return gpio->llops->reg_bits_read(gpio, offset, reg_val); } static void __aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset, int val) { struct aspeed_gpio *gpio = gpiochip_get_data(gc); - const struct aspeed_gpio_bank *bank = to_bank(offset); - void __iomem *addr; u32 reg; - addr = bank_reg(gpio, bank, reg_val); reg = gpio->dcache[GPIO_BANK(offset)]; - if (val) reg |= GPIO_BIT(offset); else reg &= ~GPIO_BIT(offset); gpio->dcache[GPIO_BANK(offset)] = reg; - iowrite32(reg, addr); + gpio->llops->reg_bits_set(gpio, offset, reg_val, val); } static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset, @@ -407,36 +391,32 @@ static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset, bool copro; raw_spin_lock_irqsave(&gpio->lock, flags); - copro = aspeed_gpio_copro_request(gpio, offset); + if (gpio->llops->copro_request) + copro = gpio->llops->copro_request(gpio, offset); __aspeed_gpio_set(gc, offset, val); - if (copro) - aspeed_gpio_copro_release(gpio, offset); + if (copro && gpio->llops->copro_release) + gpio->llops->copro_release(gpio, offset); raw_spin_unlock_irqrestore(&gpio->lock, flags); } static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset) { struct aspeed_gpio *gpio = gpiochip_get_data(gc); - const struct aspeed_gpio_bank *bank = to_bank(offset); - void __iomem *addr = bank_reg(gpio, bank, reg_dir); unsigned long flags; bool copro; - u32 reg; if (!have_input(gpio, offset)) return -ENOTSUPP; raw_spin_lock_irqsave(&gpio->lock, flags); - reg = ioread32(addr); - reg &= ~GPIO_BIT(offset); - - copro = aspeed_gpio_copro_request(gpio, offset); - iowrite32(reg, addr); - if (copro) - aspeed_gpio_copro_release(gpio, offset); + if (gpio->llops->copro_request) + copro = gpio->llops->copro_request(gpio, offset); + gpio->llops->reg_bits_set(gpio, offset, reg_dir, 0); + if (copro && gpio->llops->copro_release) + gpio->llops->copro_release(gpio, offset); raw_spin_unlock_irqrestore(&gpio->lock, flags); @@ -447,26 +427,21 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc, unsigned int offset, int val) { struct aspeed_gpio *gpio = gpiochip_get_data(gc); - const struct aspeed_gpio_bank *bank = to_bank(offset); - void __iomem *addr = bank_reg(gpio, bank, reg_dir); unsigned long flags; bool copro; - u32 reg; if (!have_output(gpio, offset)) return -ENOTSUPP; raw_spin_lock_irqsave(&gpio->lock, flags); - reg = ioread32(addr); - reg |= GPIO_BIT(offset); - - copro = aspeed_gpio_copro_request(gpio, offset); + if (gpio->llops->copro_request) + copro = gpio->llops->copro_request(gpio, offset); __aspeed_gpio_set(gc, offset, val); - iowrite32(reg, addr); + gpio->llops->reg_bits_set(gpio, offset, reg_dir, 1); - if (copro) - aspeed_gpio_copro_release(gpio, offset); + if (copro && gpio->llops->copro_release) + gpio->llops->copro_release(gpio, offset); raw_spin_unlock_irqrestore(&gpio->lock, flags); return 0; @@ -475,7 +450,6 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc, static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset) { struct aspeed_gpio *gpio = gpiochip_get_data(gc); - const struct aspeed_gpio_bank *bank = to_bank(offset); unsigned long flags; u32 val; @@ -487,7 +461,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset) raw_spin_lock_irqsave(&gpio->lock, flags); - val = ioread32(bank_reg(gpio, bank, reg_dir)) & GPIO_BIT(offset); + val = gpio->llops->reg_bits_read(gpio, offset, reg_dir); raw_spin_unlock_irqrestore(&gpio->lock, flags); @@ -496,8 +470,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset) static inline int irqd_to_aspeed_gpio_data(struct irq_data *d, struct aspeed_gpio **gpio, - const struct aspeed_gpio_bank **bank, - u32 *bit, int *offset) + int *offset) { struct aspeed_gpio *internal; @@ -510,70 +483,55 @@ static inline int irqd_to_aspeed_gpio_data(struct irq_data *d, return -ENOTSUPP; *gpio = internal; - *bank = to_bank(*offset); - *bit = GPIO_BIT(*offset); return 0; } static void aspeed_gpio_irq_ack(struct irq_data *d) { - const struct aspeed_gpio_bank *bank; struct aspeed_gpio *gpio; unsigned long flags; - void __iomem *status_addr; int rc, offset; bool copro; - u32 bit; - rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset); + rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset); if (rc) return; - status_addr = bank_reg(gpio, bank, reg_irq_status); - raw_spin_lock_irqsave(&gpio->lock, flags); - copro = aspeed_gpio_copro_request(gpio, offset); + if (gpio->llops->copro_request) + copro = gpio->llops->copro_request(gpio, offset); - iowrite32(bit, status_addr); + gpio->llops->reg_bits_set(gpio, offset, reg_irq_status, 1); - if (copro) - aspeed_gpio_copro_release(gpio, offset); + if (copro && gpio->llops->copro_release) + gpio->llops->copro_release(gpio, offset); raw_spin_unlock_irqrestore(&gpio->lock, flags); } static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set) { - const struct aspeed_gpio_bank *bank; struct aspeed_gpio *gpio; unsigned long flags; - u32 reg, bit; - void __iomem *addr; int rc, offset; bool copro; - rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset); + rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset); if (rc) return; - addr = bank_reg(gpio, bank, reg_irq_enable); - /* Unmasking the IRQ */ if (set) gpiochip_enable_irq(&gpio->chip, irqd_to_hwirq(d)); raw_spin_lock_irqsave(&gpio->lock, flags); - copro = aspeed_gpio_copro_request(gpio, offset); + if (gpio->llops->copro_request) + copro = gpio->llops->copro_request(gpio, offset); - reg = ioread32(addr); - if (set) - reg |= bit; - else - reg &= ~bit; - iowrite32(reg, addr); + gpio->llops->reg_bits_set(gpio, offset, reg_irq_enable, set); - if (copro) - aspeed_gpio_copro_release(gpio, offset); + if (copro && gpio->llops->copro_release) + gpio->llops->copro_release(gpio, offset); raw_spin_unlock_irqrestore(&gpio->lock, flags); /* Masking the IRQ */ @@ -596,34 +554,31 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type) u32 type0 = 0; u32 type1 = 0; u32 type2 = 0; - u32 bit, reg; - const struct aspeed_gpio_bank *bank; irq_flow_handler_t handler; struct aspeed_gpio *gpio; unsigned long flags; - void __iomem *addr; int rc, offset; bool copro; - rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset); + rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset); if (rc) return -EINVAL; switch (type & IRQ_TYPE_SENSE_MASK) { case IRQ_TYPE_EDGE_BOTH: - type2 |= bit; + type2 = 1; fallthrough; case IRQ_TYPE_EDGE_RISING: - type0 |= bit; + type0 = 1; fallthrough; case IRQ_TYPE_EDGE_FALLING: handler = handle_edge_irq; break; case IRQ_TYPE_LEVEL_HIGH: - type0 |= bit; + type0 = 1; fallthrough; case IRQ_TYPE_LEVEL_LOW: - type1 |= bit; + type1 = 1; handler = handle_level_irq; break; default: @@ -631,25 +586,15 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type) } raw_spin_lock_irqsave(&gpio->lock, flags); - copro = aspeed_gpio_copro_request(gpio, offset); - - addr = bank_reg(gpio, bank, reg_irq_type0); - reg = ioread32(addr); - reg = (reg & ~bit) | type0; - iowrite32(reg, addr); + if (gpio->llops->copro_request) + copro = gpio->llops->copro_request(gpio, offset); - addr = bank_reg(gpio, bank, reg_irq_type1); - reg = ioread32(addr); - reg = (reg & ~bit) | type1; - iowrite32(reg, addr); + gpio->llops->reg_bits_set(gpio, offset, reg_irq_type0, type0); + gpio->llops->reg_bits_set(gpio, offset, reg_irq_type1, type1); + gpio->llops->reg_bits_set(gpio, offset, reg_irq_type2, type2); - addr = bank_reg(gpio, bank, reg_irq_type2); - reg = ioread32(addr); - reg = (reg & ~bit) | type2; - iowrite32(reg, addr); - - if (copro) - aspeed_gpio_copro_release(gpio, offset); + if (copro && gpio->llops->copro_release) + gpio->llops->copro_release(gpio, offset); raw_spin_unlock_irqrestore(&gpio->lock, flags); irq_set_handler_locked(d, handler); @@ -661,7 +606,6 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc) { struct gpio_chip *gc = irq_desc_get_handler_data(desc); struct irq_chip *ic = irq_desc_get_chip(desc); - struct aspeed_gpio *data = gpiochip_get_data(gc); unsigned int i, p, banks; unsigned long reg; struct aspeed_gpio *gpio = gpiochip_get_data(gc); @@ -670,9 +614,7 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc) banks = DIV_ROUND_UP(gpio->chip.ngpio, 32); for (i = 0; i < banks; i++) { - const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i]; - - reg = ioread32(bank_reg(data, bank, reg_irq_status)); + reg = gpio->llops->reg_bits_read(gpio, i, reg_irq_status); for_each_set_bit(p, ®, 32) generic_handle_domain_irq(gc->irq.domain, i * 32 + p); @@ -711,26 +653,16 @@ static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip, { struct aspeed_gpio *gpio = gpiochip_get_data(chip); unsigned long flags; - void __iomem *treg; bool copro; - u32 val; - - treg = bank_reg(gpio, to_bank(offset), reg_tolerance); raw_spin_lock_irqsave(&gpio->lock, flags); - copro = aspeed_gpio_copro_request(gpio, offset); + if (gpio->llops->copro_request) + copro = gpio->llops->copro_request(gpio, offset); - val = readl(treg); + gpio->llops->reg_bits_set(gpio, offset, reg_tolerance, enable); - if (enable) - val |= GPIO_BIT(offset); - else - val &= ~GPIO_BIT(offset); - - writel(val, treg); - - if (copro) - aspeed_gpio_copro_release(gpio, offset); + if (copro && gpio->llops->copro_release) + gpio->llops->copro_release(gpio, offset); raw_spin_unlock_irqrestore(&gpio->lock, flags); return 0; @@ -821,21 +753,11 @@ static inline bool timer_allocation_registered(struct aspeed_gpio *gpio, static void configure_timer(struct aspeed_gpio *gpio, unsigned int offset, unsigned int timer) { - const struct aspeed_gpio_bank *bank = to_bank(offset); - const u32 mask = GPIO_BIT(offset); - void __iomem *addr; - u32 val; - /* Note: Debounce timer isn't under control of the command * source registers, so no need to sync with the coprocessor */ - addr = bank_reg(gpio, bank, reg_debounce_sel1); - val = ioread32(addr); - iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE1(timer, offset), addr); - - addr = bank_reg(gpio, bank, reg_debounce_sel2); - val = ioread32(addr); - iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE2(timer, offset), addr); + gpio->llops->reg_bits_set(gpio, offset, reg_debounce_sel1, !!(timer & BIT(1))); + gpio->llops->reg_bits_set(gpio, offset, reg_debounce_sel2, !!(timer & BIT(0))); } static int enable_debounce(struct gpio_chip *chip, unsigned int offset, @@ -866,15 +788,15 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset, } /* Try to find a timer already configured for the debounce period */ - for (i = 1; i < ARRAY_SIZE(debounce_timers); i++) { + for (i = 1; i < gpio->debounce_timers_num; i++) { u32 cycles; - cycles = ioread32(gpio->base + debounce_timers[i]); + cycles = ioread32(gpio->base + gpio->debounce_timers_array[i]); if (requested_cycles == cycles) break; } - if (i == ARRAY_SIZE(debounce_timers)) { + if (i == gpio->debounce_timers_num) { int j; /* @@ -905,7 +827,7 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset, i = j; - iowrite32(requested_cycles, gpio->base + debounce_timers[i]); + iowrite32(requested_cycles, gpio->base + gpio->debounce_timers_array[i]); } if (WARN(i == 0, "Cannot register index of disabled timer\n")) { @@ -1027,7 +949,7 @@ int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc, /* Switch command source */ if (gpio->cf_copro_bankmap[bindex] == 1) - aspeed_gpio_change_cmd_source(gpio, bank, bindex, + aspeed_gpio_change_cmd_source(gpio, offset, GPIO_CMDSRC_COLDFIRE); if (vreg_offset) @@ -1051,7 +973,6 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc) struct gpio_chip *chip = gpiod_to_chip(desc); struct aspeed_gpio *gpio = gpiochip_get_data(chip); int rc = 0, bindex, offset = gpio_chip_hwgpio(desc); - const struct aspeed_gpio_bank *bank = to_bank(offset); unsigned long flags; if (!gpio->cf_copro_bankmap) @@ -1072,7 +993,7 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc) /* Switch command source */ if (gpio->cf_copro_bankmap[bindex] == 0) - aspeed_gpio_change_cmd_source(gpio, bank, bindex, + aspeed_gpio_change_cmd_source(gpio, offset, GPIO_CMDSRC_ARM); bail: raw_spin_unlock_irqrestore(&gpio->lock, flags); @@ -1082,12 +1003,10 @@ EXPORT_SYMBOL_GPL(aspeed_gpio_copro_release_gpio); static void aspeed_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p) { - const struct aspeed_gpio_bank *bank; struct aspeed_gpio *gpio; - u32 bit; int rc, offset; - rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset); + rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset); if (rc) return; @@ -1120,7 +1039,7 @@ static const struct aspeed_bank_props ast2400_bank_props[] = { static const struct aspeed_gpio_config ast2400_config = /* 220 for simplicity, really 216 with two 4-GPIO holes, four at end */ - { .nr_gpios = 220, .props = ast2400_bank_props, }; + { .nr_gpios = 220, .props = ast2400_bank_props, .version = 4}; static const struct aspeed_bank_props ast2500_bank_props[] = { /* input output */ @@ -1132,7 +1051,7 @@ static const struct aspeed_bank_props ast2500_bank_props[] = { static const struct aspeed_gpio_config ast2500_config = /* 232 for simplicity, actual number is 228 (4-GPIO hole in GPIOAB) */ - { .nr_gpios = 232, .props = ast2500_bank_props, }; + { .nr_gpios = 232, .props = ast2500_bank_props, .version = 4}; static const struct aspeed_bank_props ast2600_bank_props[] = { /* input output */ @@ -1148,7 +1067,7 @@ static const struct aspeed_gpio_config ast2600_config = * We expect ngpio being set in the device tree and this is a fallback * option. */ - { .nr_gpios = 208, .props = ast2600_bank_props, }; + { .nr_gpios = 208, .props = ast2600_bank_props, .version = 4}; static const struct of_device_id aspeed_gpio_of_table[] = { { .compatible = "aspeed,ast2400-gpio", .data = &ast2400_config, }, @@ -1158,6 +1077,40 @@ static const struct of_device_id aspeed_gpio_of_table[] = { }; MODULE_DEVICE_TABLE(of, aspeed_gpio_of_table); +static void aspeed_g4_reg_bits_set(struct aspeed_gpio *gpio, unsigned int offset, + const enum aspeed_gpio_reg reg, u32 val) +{ + const struct aspeed_gpio_bank *bank = to_bank(offset); + void __iomem *addr = bank_reg(gpio, bank, reg); + u32 temp; + + temp = ioread32(addr); + if (val) + temp |= GPIO_BIT(offset); + else + temp &= ~GPIO_BIT(offset); + + iowrite32(temp, addr); +} + +static u32 aspeed_g4_reg_bits_read(struct aspeed_gpio *gpio, unsigned int offset, + const enum aspeed_gpio_reg reg) +{ + const struct aspeed_gpio_bank *bank = to_bank(offset); + void __iomem *addr = bank_reg(gpio, bank, reg); + + if (reg == reg_rdata) + return ioread32(addr); + return !!(ioread32(addr) & GPIO_BIT(offset)); +} + +struct aspeed_gpio_llops aspeed_g4_llops = { + .copro_request = aspeed_gpio_copro_request, + .copro_release = aspeed_gpio_copro_release, + .reg_bits_set = aspeed_g4_reg_bits_set, + .reg_bits_read = aspeed_g4_reg_bits_read, +}; + static int __init aspeed_gpio_probe(struct platform_device *pdev) { const struct of_device_id *gpio_id; @@ -1191,6 +1144,18 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev) gpio->config = gpio_id->data; + if (gpio->config->version == 4) { + if (!gpio->llops) + gpio->llops = &aspeed_g4_llops; + + if (!gpio->debounce_timers_array) { + gpio->debounce_timers_array = debounce_timers; + gpio->debounce_timers_num = ARRAY_SIZE(debounce_timers); + } + } else { + return -EOPNOTSUPP; + } + gpio->chip.parent = &pdev->dev; err = of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpio); gpio->chip.ngpio = (u16) ngpio; @@ -1218,15 +1183,17 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev) * Populate it with initial values read from the HW and switch * all command sources to the ARM by default */ - for (i = 0; i < banks; i++) { - const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i]; - void __iomem *addr = bank_reg(gpio, bank, reg_rdata); - gpio->dcache[i] = ioread32(addr); - aspeed_gpio_change_cmd_source(gpio, bank, 0, GPIO_CMDSRC_ARM); - aspeed_gpio_change_cmd_source(gpio, bank, 1, GPIO_CMDSRC_ARM); - aspeed_gpio_change_cmd_source(gpio, bank, 2, GPIO_CMDSRC_ARM); - aspeed_gpio_change_cmd_source(gpio, bank, 3, GPIO_CMDSRC_ARM); - } + if (gpio->config->version == 4) + for (i = 0; i < banks; i++) { + const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i]; + void __iomem *addr = bank_reg(gpio, bank, reg_rdata); + + gpio->dcache[i] = ioread32(addr); + aspeed_gpio_change_cmd_source(gpio, i * 8 + 0, GPIO_CMDSRC_ARM); + aspeed_gpio_change_cmd_source(gpio, i * 8 + 8, GPIO_CMDSRC_ARM); + aspeed_gpio_change_cmd_source(gpio, i * 8 + 16, GPIO_CMDSRC_ARM); + aspeed_gpio_change_cmd_source(gpio, i * 8 + 24, GPIO_CMDSRC_ARM); + } /* Set up an irqchip */ irq = platform_get_irq(pdev, 0); -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access 2024-08-30 3:40 ` [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access Billy Tsai @ 2024-08-30 16:07 ` kernel test robot 2024-08-31 1:46 ` kernel test robot ` (2 subsequent siblings) 3 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2024-08-30 16:07 UTC (permalink / raw) To: Billy Tsai, linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, andrew, linux-gpio, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, BMC-SW Cc: llvm, oe-kbuild-all Hi Billy, kernel test robot noticed the following build warnings: [auto build test WARNING on brgl/gpio/for-next] [also build test WARNING on linus/master v6.11-rc5 next-20240830] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Billy-Tsai/dt-bindings-gpio-aspeed-ast2400-gpio-Support-ast2700/20240830-114325 base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next patch link: https://lore.kernel.org/r/20240830034047.2251482-4-billy_tsai%40aspeedtech.com patch subject: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access config: i386-buildonly-randconfig-004-20240830 (https://download.01.org/0day-ci/archive/20240830/202408302344.bCpCF6bu-lkp@intel.com/config) compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240830/202408302344.bCpCF6bu-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408302344.bCpCF6bu-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/gpio/gpio-aspeed.c:394:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 394 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-aspeed.c:399:6: note: uninitialized use occurs here 399 | if (copro && gpio->llops->copro_release) | ^~~~~ drivers/gpio/gpio-aspeed.c:394:2: note: remove the 'if' if its condition is always true 394 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 395 | copro = gpio->llops->copro_request(gpio, offset); drivers/gpio/gpio-aspeed.c:391:12: note: initialize the variable 'copro' to silence this warning 391 | bool copro; | ^ | = 0 drivers/gpio/gpio-aspeed.c:415:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 415 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-aspeed.c:418:6: note: uninitialized use occurs here 418 | if (copro && gpio->llops->copro_release) | ^~~~~ drivers/gpio/gpio-aspeed.c:415:2: note: remove the 'if' if its condition is always true 415 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 416 | copro = gpio->llops->copro_request(gpio, offset); drivers/gpio/gpio-aspeed.c:408:12: note: initialize the variable 'copro' to silence this warning 408 | bool copro; | ^ | = 0 drivers/gpio/gpio-aspeed.c:438:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 438 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-aspeed.c:443:6: note: uninitialized use occurs here 443 | if (copro && gpio->llops->copro_release) | ^~~~~ drivers/gpio/gpio-aspeed.c:438:2: note: remove the 'if' if its condition is always true 438 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 439 | copro = gpio->llops->copro_request(gpio, offset); drivers/gpio/gpio-aspeed.c:431:12: note: initialize the variable 'copro' to silence this warning 431 | bool copro; | ^ | = 0 drivers/gpio/gpio-aspeed.c:502:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 502 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-aspeed.c:507:6: note: uninitialized use occurs here 507 | if (copro && gpio->llops->copro_release) | ^~~~~ drivers/gpio/gpio-aspeed.c:502:2: note: remove the 'if' if its condition is always true 502 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 503 | copro = gpio->llops->copro_request(gpio, offset); drivers/gpio/gpio-aspeed.c:495:12: note: initialize the variable 'copro' to silence this warning 495 | bool copro; | ^ | = 0 drivers/gpio/gpio-aspeed.c:528:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 528 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-aspeed.c:533:6: note: uninitialized use occurs here 533 | if (copro && gpio->llops->copro_release) | ^~~~~ drivers/gpio/gpio-aspeed.c:528:2: note: remove the 'if' if its condition is always true 528 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 529 | copro = gpio->llops->copro_request(gpio, offset); drivers/gpio/gpio-aspeed.c:517:12: note: initialize the variable 'copro' to silence this warning 517 | bool copro; | ^ | = 0 drivers/gpio/gpio-aspeed.c:589:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 589 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-aspeed.c:596:6: note: uninitialized use occurs here 596 | if (copro && gpio->llops->copro_release) | ^~~~~ drivers/gpio/gpio-aspeed.c:589:2: note: remove the 'if' if its condition is always true 589 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 590 | copro = gpio->llops->copro_request(gpio, offset); drivers/gpio/gpio-aspeed.c:561:12: note: initialize the variable 'copro' to silence this warning 561 | bool copro; | ^ | = 0 drivers/gpio/gpio-aspeed.c:659:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 659 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-aspeed.c:664:6: note: uninitialized use occurs here 664 | if (copro && gpio->llops->copro_release) | ^~~~~ drivers/gpio/gpio-aspeed.c:659:2: note: remove the 'if' if its condition is always true 659 | if (gpio->llops->copro_request) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 660 | copro = gpio->llops->copro_request(gpio, offset); drivers/gpio/gpio-aspeed.c:656:12: note: initialize the variable 'copro' to silence this warning 656 | bool copro; | ^ | = 0 7 warnings generated. vim +394 drivers/gpio/gpio-aspeed.c 385 386 static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset, 387 int val) 388 { 389 struct aspeed_gpio *gpio = gpiochip_get_data(gc); 390 unsigned long flags; 391 bool copro; 392 393 raw_spin_lock_irqsave(&gpio->lock, flags); > 394 if (gpio->llops->copro_request) 395 copro = gpio->llops->copro_request(gpio, offset); 396 397 __aspeed_gpio_set(gc, offset, val); 398 399 if (copro && gpio->llops->copro_release) 400 gpio->llops->copro_release(gpio, offset); 401 raw_spin_unlock_irqrestore(&gpio->lock, flags); 402 } 403 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access 2024-08-30 3:40 ` [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access Billy Tsai 2024-08-30 16:07 ` kernel test robot @ 2024-08-31 1:46 ` kernel test robot 2024-08-31 16:05 ` Dan Carpenter 2024-09-12 8:18 ` Andrew Jeffery 3 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2024-08-31 1:46 UTC (permalink / raw) To: Billy Tsai, linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, andrew, linux-gpio, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, BMC-SW Cc: oe-kbuild-all Hi Billy, kernel test robot noticed the following build warnings: [auto build test WARNING on brgl/gpio/for-next] [also build test WARNING on linus/master v6.11-rc5 next-20240830] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Billy-Tsai/dt-bindings-gpio-aspeed-ast2400-gpio-Support-ast2700/20240830-114325 base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next patch link: https://lore.kernel.org/r/20240830034047.2251482-4-billy_tsai%40aspeedtech.com patch subject: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access config: openrisc-randconfig-r131-20240830 (https://download.01.org/0day-ci/archive/20240831/202408310953.Z0c7NIuz-lkp@intel.com/config) compiler: or1k-linux-gcc (GCC) 14.1.0 reproduce: (https://download.01.org/0day-ci/archive/20240831/202408310953.Z0c7NIuz-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408310953.Z0c7NIuz-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/gpio/gpio-aspeed.c:1107:26: sparse: sparse: symbol 'aspeed_g4_llops' was not declared. Should it be static? vim +/aspeed_g4_llops +1107 drivers/gpio/gpio-aspeed.c 1106 > 1107 struct aspeed_gpio_llops aspeed_g4_llops = { 1108 .copro_request = aspeed_gpio_copro_request, 1109 .copro_release = aspeed_gpio_copro_release, 1110 .reg_bits_set = aspeed_g4_reg_bits_set, 1111 .reg_bits_read = aspeed_g4_reg_bits_read, 1112 }; 1113 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access 2024-08-30 3:40 ` [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access Billy Tsai 2024-08-30 16:07 ` kernel test robot 2024-08-31 1:46 ` kernel test robot @ 2024-08-31 16:05 ` Dan Carpenter 2024-09-12 8:18 ` Andrew Jeffery 3 siblings, 0 replies; 15+ messages in thread From: Dan Carpenter @ 2024-08-31 16:05 UTC (permalink / raw) To: oe-kbuild, Billy Tsai, linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, andrew, linux-gpio, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, BMC-SW Cc: lkp, oe-kbuild-all Hi Billy, kernel test robot noticed the following build warnings: https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Billy-Tsai/dt-bindings-gpio-aspeed-ast2400-gpio-Support-ast2700/20240830-114325 base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next patch link: https://lore.kernel.org/r/20240830034047.2251482-4-billy_tsai%40aspeedtech.com patch subject: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access config: parisc-randconfig-r071-20240831 (https://download.01.org/0day-ci/archive/20240831/202408312313.HTx2vwvy-lkp@intel.com/config) compiler: hppa-linux-gcc (GCC) 14.1.0 If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> | Closes: https://lore.kernel.org/r/202408312313.HTx2vwvy-lkp@intel.com/ smatch warnings: drivers/gpio/gpio-aspeed.c:399 aspeed_gpio_set() error: uninitialized symbol 'copro'. drivers/gpio/gpio-aspeed.c:418 aspeed_gpio_dir_in() error: uninitialized symbol 'copro'. drivers/gpio/gpio-aspeed.c:443 aspeed_gpio_dir_out() error: uninitialized symbol 'copro'. drivers/gpio/gpio-aspeed.c:507 aspeed_gpio_irq_ack() error: uninitialized symbol 'copro'. drivers/gpio/gpio-aspeed.c:533 aspeed_gpio_irq_set_mask() error: uninitialized symbol 'copro'. drivers/gpio/gpio-aspeed.c:596 aspeed_gpio_set_type() error: uninitialized symbol 'copro'. drivers/gpio/gpio-aspeed.c:664 aspeed_gpio_reset_tolerance() error: uninitialized symbol 'copro'. vim +/copro +399 drivers/gpio/gpio-aspeed.c 361b79119a4b7f Joel Stanley 2016-08-30 386 static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset, 361b79119a4b7f Joel Stanley 2016-08-30 387 int val) 361b79119a4b7f Joel Stanley 2016-08-30 388 { 361b79119a4b7f Joel Stanley 2016-08-30 389 struct aspeed_gpio *gpio = gpiochip_get_data(gc); 361b79119a4b7f Joel Stanley 2016-08-30 390 unsigned long flags; a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 391 bool copro; 361b79119a4b7f Joel Stanley 2016-08-30 392 61a7904b6ace99 Iwona Winiarska 2021-12-04 393 raw_spin_lock_irqsave(&gpio->lock, flags); 0e6ca482ec6e28 Billy Tsai 2024-08-30 394 if (gpio->llops->copro_request) 0e6ca482ec6e28 Billy Tsai 2024-08-30 395 copro = gpio->llops->copro_request(gpio, offset); Uninitialized on else path 361b79119a4b7f Joel Stanley 2016-08-30 396 361b79119a4b7f Joel Stanley 2016-08-30 397 __aspeed_gpio_set(gc, offset, val); 361b79119a4b7f Joel Stanley 2016-08-30 398 0e6ca482ec6e28 Billy Tsai 2024-08-30 @399 if (copro && gpio->llops->copro_release) ^^^^^ 0e6ca482ec6e28 Billy Tsai 2024-08-30 400 gpio->llops->copro_release(gpio, offset); 61a7904b6ace99 Iwona Winiarska 2021-12-04 401 raw_spin_unlock_irqrestore(&gpio->lock, flags); 361b79119a4b7f Joel Stanley 2016-08-30 402 } 361b79119a4b7f Joel Stanley 2016-08-30 403 361b79119a4b7f Joel Stanley 2016-08-30 404 static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset) 361b79119a4b7f Joel Stanley 2016-08-30 405 { 361b79119a4b7f Joel Stanley 2016-08-30 406 struct aspeed_gpio *gpio = gpiochip_get_data(gc); 361b79119a4b7f Joel Stanley 2016-08-30 407 unsigned long flags; a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 408 bool copro; 361b79119a4b7f Joel Stanley 2016-08-30 409 1736f75d35e474 Andrew Jeffery 2017-01-24 410 if (!have_input(gpio, offset)) 1736f75d35e474 Andrew Jeffery 2017-01-24 411 return -ENOTSUPP; 1736f75d35e474 Andrew Jeffery 2017-01-24 412 61a7904b6ace99 Iwona Winiarska 2021-12-04 413 raw_spin_lock_irqsave(&gpio->lock, flags); 361b79119a4b7f Joel Stanley 2016-08-30 414 0e6ca482ec6e28 Billy Tsai 2024-08-30 415 if (gpio->llops->copro_request) 0e6ca482ec6e28 Billy Tsai 2024-08-30 416 copro = gpio->llops->copro_request(gpio, offset); 0e6ca482ec6e28 Billy Tsai 2024-08-30 417 gpio->llops->reg_bits_set(gpio, offset, reg_dir, 0); 0e6ca482ec6e28 Billy Tsai 2024-08-30 @418 if (copro && gpio->llops->copro_release) 0e6ca482ec6e28 Billy Tsai 2024-08-30 419 gpio->llops->copro_release(gpio, offset); 361b79119a4b7f Joel Stanley 2016-08-30 420 61a7904b6ace99 Iwona Winiarska 2021-12-04 421 raw_spin_unlock_irqrestore(&gpio->lock, flags); 361b79119a4b7f Joel Stanley 2016-08-30 422 361b79119a4b7f Joel Stanley 2016-08-30 423 return 0; 361b79119a4b7f Joel Stanley 2016-08-30 424 } 361b79119a4b7f Joel Stanley 2016-08-30 425 361b79119a4b7f Joel Stanley 2016-08-30 426 static int aspeed_gpio_dir_out(struct gpio_chip *gc, 361b79119a4b7f Joel Stanley 2016-08-30 427 unsigned int offset, int val) 361b79119a4b7f Joel Stanley 2016-08-30 428 { 361b79119a4b7f Joel Stanley 2016-08-30 429 struct aspeed_gpio *gpio = gpiochip_get_data(gc); 361b79119a4b7f Joel Stanley 2016-08-30 430 unsigned long flags; a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 431 bool copro; 361b79119a4b7f Joel Stanley 2016-08-30 432 1736f75d35e474 Andrew Jeffery 2017-01-24 433 if (!have_output(gpio, offset)) 1736f75d35e474 Andrew Jeffery 2017-01-24 434 return -ENOTSUPP; 1736f75d35e474 Andrew Jeffery 2017-01-24 435 61a7904b6ace99 Iwona Winiarska 2021-12-04 436 raw_spin_lock_irqsave(&gpio->lock, flags); 361b79119a4b7f Joel Stanley 2016-08-30 437 0e6ca482ec6e28 Billy Tsai 2024-08-30 438 if (gpio->llops->copro_request) 0e6ca482ec6e28 Billy Tsai 2024-08-30 439 copro = gpio->llops->copro_request(gpio, offset); af7949284910a1 Benjamin Herrenschmidt 2018-05-17 440 __aspeed_gpio_set(gc, offset, val); 0e6ca482ec6e28 Billy Tsai 2024-08-30 441 gpio->llops->reg_bits_set(gpio, offset, reg_dir, 1); 361b79119a4b7f Joel Stanley 2016-08-30 442 0e6ca482ec6e28 Billy Tsai 2024-08-30 @443 if (copro && gpio->llops->copro_release) 0e6ca482ec6e28 Billy Tsai 2024-08-30 444 gpio->llops->copro_release(gpio, offset); 61a7904b6ace99 Iwona Winiarska 2021-12-04 445 raw_spin_unlock_irqrestore(&gpio->lock, flags); 361b79119a4b7f Joel Stanley 2016-08-30 446 361b79119a4b7f Joel Stanley 2016-08-30 447 return 0; 361b79119a4b7f Joel Stanley 2016-08-30 448 } 361b79119a4b7f Joel Stanley 2016-08-30 449 361b79119a4b7f Joel Stanley 2016-08-30 450 static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset) 361b79119a4b7f Joel Stanley 2016-08-30 451 { 361b79119a4b7f Joel Stanley 2016-08-30 452 struct aspeed_gpio *gpio = gpiochip_get_data(gc); 361b79119a4b7f Joel Stanley 2016-08-30 453 unsigned long flags; 361b79119a4b7f Joel Stanley 2016-08-30 454 u32 val; 361b79119a4b7f Joel Stanley 2016-08-30 455 1736f75d35e474 Andrew Jeffery 2017-01-24 456 if (!have_input(gpio, offset)) e42615ec233b30 Matti Vaittinen 2019-11-06 457 return GPIO_LINE_DIRECTION_OUT; 1736f75d35e474 Andrew Jeffery 2017-01-24 458 1736f75d35e474 Andrew Jeffery 2017-01-24 459 if (!have_output(gpio, offset)) e42615ec233b30 Matti Vaittinen 2019-11-06 460 return GPIO_LINE_DIRECTION_IN; 1736f75d35e474 Andrew Jeffery 2017-01-24 461 61a7904b6ace99 Iwona Winiarska 2021-12-04 462 raw_spin_lock_irqsave(&gpio->lock, flags); 361b79119a4b7f Joel Stanley 2016-08-30 463 0e6ca482ec6e28 Billy Tsai 2024-08-30 464 val = gpio->llops->reg_bits_read(gpio, offset, reg_dir); 361b79119a4b7f Joel Stanley 2016-08-30 465 61a7904b6ace99 Iwona Winiarska 2021-12-04 466 raw_spin_unlock_irqrestore(&gpio->lock, flags); 361b79119a4b7f Joel Stanley 2016-08-30 467 e42615ec233b30 Matti Vaittinen 2019-11-06 468 return val ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN; 361b79119a4b7f Joel Stanley 2016-08-30 469 } 361b79119a4b7f Joel Stanley 2016-08-30 470 361b79119a4b7f Joel Stanley 2016-08-30 471 static inline int irqd_to_aspeed_gpio_data(struct irq_data *d, 361b79119a4b7f Joel Stanley 2016-08-30 472 struct aspeed_gpio **gpio, 0e6ca482ec6e28 Billy Tsai 2024-08-30 473 int *offset) 361b79119a4b7f Joel Stanley 2016-08-30 474 { 1736f75d35e474 Andrew Jeffery 2017-01-24 475 struct aspeed_gpio *internal; 361b79119a4b7f Joel Stanley 2016-08-30 476 a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 477 *offset = irqd_to_hwirq(d); 361b79119a4b7f Joel Stanley 2016-08-30 478 1736f75d35e474 Andrew Jeffery 2017-01-24 479 internal = irq_data_get_irq_chip_data(d); 1736f75d35e474 Andrew Jeffery 2017-01-24 480 1736f75d35e474 Andrew Jeffery 2017-01-24 481 /* This might be a bit of a questionable place to check */ a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 482 if (!have_irq(internal, *offset)) 1736f75d35e474 Andrew Jeffery 2017-01-24 483 return -ENOTSUPP; 1736f75d35e474 Andrew Jeffery 2017-01-24 484 1736f75d35e474 Andrew Jeffery 2017-01-24 485 *gpio = internal; 361b79119a4b7f Joel Stanley 2016-08-30 486 361b79119a4b7f Joel Stanley 2016-08-30 487 return 0; 361b79119a4b7f Joel Stanley 2016-08-30 488 } 361b79119a4b7f Joel Stanley 2016-08-30 489 361b79119a4b7f Joel Stanley 2016-08-30 490 static void aspeed_gpio_irq_ack(struct irq_data *d) 361b79119a4b7f Joel Stanley 2016-08-30 491 { 361b79119a4b7f Joel Stanley 2016-08-30 492 struct aspeed_gpio *gpio; 361b79119a4b7f Joel Stanley 2016-08-30 493 unsigned long flags; a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 494 int rc, offset; a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 495 bool copro; 361b79119a4b7f Joel Stanley 2016-08-30 496 0e6ca482ec6e28 Billy Tsai 2024-08-30 497 rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset); 361b79119a4b7f Joel Stanley 2016-08-30 498 if (rc) 361b79119a4b7f Joel Stanley 2016-08-30 499 return; 361b79119a4b7f Joel Stanley 2016-08-30 500 61a7904b6ace99 Iwona Winiarska 2021-12-04 501 raw_spin_lock_irqsave(&gpio->lock, flags); 0e6ca482ec6e28 Billy Tsai 2024-08-30 502 if (gpio->llops->copro_request) 0e6ca482ec6e28 Billy Tsai 2024-08-30 503 copro = gpio->llops->copro_request(gpio, offset); a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 504 0e6ca482ec6e28 Billy Tsai 2024-08-30 505 gpio->llops->reg_bits_set(gpio, offset, reg_irq_status, 1); a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 506 0e6ca482ec6e28 Billy Tsai 2024-08-30 @507 if (copro && gpio->llops->copro_release) 0e6ca482ec6e28 Billy Tsai 2024-08-30 508 gpio->llops->copro_release(gpio, offset); 61a7904b6ace99 Iwona Winiarska 2021-12-04 509 raw_spin_unlock_irqrestore(&gpio->lock, flags); 361b79119a4b7f Joel Stanley 2016-08-30 510 } 361b79119a4b7f Joel Stanley 2016-08-30 511 361b79119a4b7f Joel Stanley 2016-08-30 512 static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set) 361b79119a4b7f Joel Stanley 2016-08-30 513 { 361b79119a4b7f Joel Stanley 2016-08-30 514 struct aspeed_gpio *gpio; 361b79119a4b7f Joel Stanley 2016-08-30 515 unsigned long flags; a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 516 int rc, offset; a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 517 bool copro; 361b79119a4b7f Joel Stanley 2016-08-30 518 0e6ca482ec6e28 Billy Tsai 2024-08-30 519 rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset); 361b79119a4b7f Joel Stanley 2016-08-30 520 if (rc) 361b79119a4b7f Joel Stanley 2016-08-30 521 return; 361b79119a4b7f Joel Stanley 2016-08-30 522 061df08f063a97 Linus Walleij 2023-03-09 523 /* Unmasking the IRQ */ 061df08f063a97 Linus Walleij 2023-03-09 524 if (set) 061df08f063a97 Linus Walleij 2023-03-09 525 gpiochip_enable_irq(&gpio->chip, irqd_to_hwirq(d)); 061df08f063a97 Linus Walleij 2023-03-09 526 61a7904b6ace99 Iwona Winiarska 2021-12-04 527 raw_spin_lock_irqsave(&gpio->lock, flags); 0e6ca482ec6e28 Billy Tsai 2024-08-30 528 if (gpio->llops->copro_request) 0e6ca482ec6e28 Billy Tsai 2024-08-30 529 copro = gpio->llops->copro_request(gpio, offset); 361b79119a4b7f Joel Stanley 2016-08-30 530 0e6ca482ec6e28 Billy Tsai 2024-08-30 531 gpio->llops->reg_bits_set(gpio, offset, reg_irq_enable, set); 361b79119a4b7f Joel Stanley 2016-08-30 532 0e6ca482ec6e28 Billy Tsai 2024-08-30 @533 if (copro && gpio->llops->copro_release) 0e6ca482ec6e28 Billy Tsai 2024-08-30 534 gpio->llops->copro_release(gpio, offset); 61a7904b6ace99 Iwona Winiarska 2021-12-04 535 raw_spin_unlock_irqrestore(&gpio->lock, flags); 061df08f063a97 Linus Walleij 2023-03-09 536 061df08f063a97 Linus Walleij 2023-03-09 537 /* Masking the IRQ */ 061df08f063a97 Linus Walleij 2023-03-09 538 if (!set) 061df08f063a97 Linus Walleij 2023-03-09 539 gpiochip_disable_irq(&gpio->chip, irqd_to_hwirq(d)); 361b79119a4b7f Joel Stanley 2016-08-30 540 } 361b79119a4b7f Joel Stanley 2016-08-30 541 361b79119a4b7f Joel Stanley 2016-08-30 542 static void aspeed_gpio_irq_mask(struct irq_data *d) 361b79119a4b7f Joel Stanley 2016-08-30 543 { 361b79119a4b7f Joel Stanley 2016-08-30 544 aspeed_gpio_irq_set_mask(d, false); 361b79119a4b7f Joel Stanley 2016-08-30 545 } 361b79119a4b7f Joel Stanley 2016-08-30 546 361b79119a4b7f Joel Stanley 2016-08-30 547 static void aspeed_gpio_irq_unmask(struct irq_data *d) 361b79119a4b7f Joel Stanley 2016-08-30 548 { 361b79119a4b7f Joel Stanley 2016-08-30 549 aspeed_gpio_irq_set_mask(d, true); 361b79119a4b7f Joel Stanley 2016-08-30 550 } 361b79119a4b7f Joel Stanley 2016-08-30 551 361b79119a4b7f Joel Stanley 2016-08-30 552 static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type) 361b79119a4b7f Joel Stanley 2016-08-30 553 { 361b79119a4b7f Joel Stanley 2016-08-30 554 u32 type0 = 0; 361b79119a4b7f Joel Stanley 2016-08-30 555 u32 type1 = 0; 361b79119a4b7f Joel Stanley 2016-08-30 556 u32 type2 = 0; 361b79119a4b7f Joel Stanley 2016-08-30 557 irq_flow_handler_t handler; 361b79119a4b7f Joel Stanley 2016-08-30 558 struct aspeed_gpio *gpio; 361b79119a4b7f Joel Stanley 2016-08-30 559 unsigned long flags; a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 560 int rc, offset; a7ca13826e478f Benjamin Herrenschmidt 2018-06-29 561 bool copro; 361b79119a4b7f Joel Stanley 2016-08-30 562 0e6ca482ec6e28 Billy Tsai 2024-08-30 563 rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset); 361b79119a4b7f Joel Stanley 2016-08-30 564 if (rc) 361b79119a4b7f Joel Stanley 2016-08-30 565 return -EINVAL; 361b79119a4b7f Joel Stanley 2016-08-30 566 361b79119a4b7f Joel Stanley 2016-08-30 567 switch (type & IRQ_TYPE_SENSE_MASK) { 361b79119a4b7f Joel Stanley 2016-08-30 568 case IRQ_TYPE_EDGE_BOTH: 0e6ca482ec6e28 Billy Tsai 2024-08-30 569 type2 = 1; df561f6688fef7 Gustavo A. R. Silva 2020-08-23 570 fallthrough; 361b79119a4b7f Joel Stanley 2016-08-30 571 case IRQ_TYPE_EDGE_RISING: 0e6ca482ec6e28 Billy Tsai 2024-08-30 572 type0 = 1; df561f6688fef7 Gustavo A. R. Silva 2020-08-23 573 fallthrough; 361b79119a4b7f Joel Stanley 2016-08-30 574 case IRQ_TYPE_EDGE_FALLING: 361b79119a4b7f Joel Stanley 2016-08-30 575 handler = handle_edge_irq; 361b79119a4b7f Joel Stanley 2016-08-30 576 break; 361b79119a4b7f Joel Stanley 2016-08-30 577 case IRQ_TYPE_LEVEL_HIGH: 0e6ca482ec6e28 Billy Tsai 2024-08-30 578 type0 = 1; df561f6688fef7 Gustavo A. R. Silva 2020-08-23 579 fallthrough; 361b79119a4b7f Joel Stanley 2016-08-30 580 case IRQ_TYPE_LEVEL_LOW: 0e6ca482ec6e28 Billy Tsai 2024-08-30 581 type1 = 1; 361b79119a4b7f Joel Stanley 2016-08-30 582 handler = handle_level_irq; 361b79119a4b7f Joel Stanley 2016-08-30 583 break; 361b79119a4b7f Joel Stanley 2016-08-30 584 default: 361b79119a4b7f Joel Stanley 2016-08-30 585 return -EINVAL; 361b79119a4b7f Joel Stanley 2016-08-30 586 } 361b79119a4b7f Joel Stanley 2016-08-30 587 61a7904b6ace99 Iwona Winiarska 2021-12-04 588 raw_spin_lock_irqsave(&gpio->lock, flags); 0e6ca482ec6e28 Billy Tsai 2024-08-30 589 if (gpio->llops->copro_request) 0e6ca482ec6e28 Billy Tsai 2024-08-30 590 copro = gpio->llops->copro_request(gpio, offset); 361b79119a4b7f Joel Stanley 2016-08-30 591 0e6ca482ec6e28 Billy Tsai 2024-08-30 592 gpio->llops->reg_bits_set(gpio, offset, reg_irq_type0, type0); 0e6ca482ec6e28 Billy Tsai 2024-08-30 593 gpio->llops->reg_bits_set(gpio, offset, reg_irq_type1, type1); 0e6ca482ec6e28 Billy Tsai 2024-08-30 594 gpio->llops->reg_bits_set(gpio, offset, reg_irq_type2, type2); 361b79119a4b7f Joel Stanley 2016-08-30 595 0e6ca482ec6e28 Billy Tsai 2024-08-30 @596 if (copro && gpio->llops->copro_release) 0e6ca482ec6e28 Billy Tsai 2024-08-30 597 gpio->llops->copro_release(gpio, offset); 61a7904b6ace99 Iwona Winiarska 2021-12-04 598 raw_spin_unlock_irqrestore(&gpio->lock, flags); 361b79119a4b7f Joel Stanley 2016-08-30 599 361b79119a4b7f Joel Stanley 2016-08-30 600 irq_set_handler_locked(d, handler); 361b79119a4b7f Joel Stanley 2016-08-30 601 361b79119a4b7f Joel Stanley 2016-08-30 602 return 0; 361b79119a4b7f Joel Stanley 2016-08-30 603 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access 2024-08-30 3:40 ` [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access Billy Tsai ` (2 preceding siblings ...) 2024-08-31 16:05 ` Dan Carpenter @ 2024-09-12 8:18 ` Andrew Jeffery 3 siblings, 0 replies; 15+ messages in thread From: Andrew Jeffery @ 2024-09-12 8:18 UTC (permalink / raw) To: Billy Tsai, linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, linux-gpio, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, BMC-SW On Fri, 2024-08-30 at 11:40 +0800, Billy Tsai wrote: > Add low-level operations (llops) to abstract the register access for GPIO > registers and the coprocessor request/release. With this abstraction > layer, the driver can separate the hardware and software logic, making it > easier to extend the driver to support different hardware register > layouts. > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> > --- > drivers/gpio/gpio-aspeed.c | 309 +++++++++++++++++-------------------- > 1 file changed, 138 insertions(+), 171 deletions(-) > > diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c > index 24f50a0ea4ab..74c4e80958bf 100644 > --- a/drivers/gpio/gpio-aspeed.c > +++ b/drivers/gpio/gpio-aspeed.c > @@ -39,6 +39,7 @@ struct aspeed_bank_props { > struct aspeed_gpio_config { > unsigned int nr_gpios; > const struct aspeed_bank_props *props; > + unsigned int version; I'm interested to see whether this is necessary in practice. > }; > > /* > @@ -62,10 +63,13 @@ struct aspeed_gpio { > > u8 *offset_timer; > unsigned int timer_users[4]; > + const int *debounce_timers_array; > + int debounce_timers_num; > struct clk *clk; > > u32 *dcache; > u8 *cf_copro_bankmap; > + const struct aspeed_gpio_llops *llops; > }; > > struct aspeed_gpio_bank { > @@ -178,6 +182,15 @@ enum aspeed_gpio_reg { > reg_cmdsrc1, > }; > > +struct aspeed_gpio_llops { > + bool (*copro_request)(struct aspeed_gpio *gpio, unsigned int offset); > + void (*copro_release)(struct aspeed_gpio *gpio, unsigned int offset); > + void (*reg_bits_set)(struct aspeed_gpio *gpio, unsigned int offset, > + const enum aspeed_gpio_reg reg, u32 val); > + u32 (*reg_bits_read)(struct aspeed_gpio *gpio, unsigned int offset, > + const enum aspeed_gpio_reg reg); > +}; > + > #define GPIO_VAL_VALUE 0x00 > #define GPIO_VAL_DIR 0x04 > > @@ -237,10 +250,6 @@ static inline void __iomem *bank_reg(struct aspeed_gpio *gpio, > #define GPIO_OFFSET(x) ((x) & 0x1f) > #define GPIO_BIT(x) BIT(GPIO_OFFSET(x)) > > -#define _GPIO_SET_DEBOUNCE(t, o, i) ((!!((t) & BIT(i))) << GPIO_OFFSET(o)) > -#define GPIO_SET_DEBOUNCE1(t, o) _GPIO_SET_DEBOUNCE(t, o, 1) > -#define GPIO_SET_DEBOUNCE2(t, o) _GPIO_SET_DEBOUNCE(t, o, 0) > - > static const struct aspeed_gpio_bank *to_bank(unsigned int offset) > { > unsigned int bank = GPIO_BANK(offset); > @@ -296,35 +305,17 @@ static inline bool have_output(struct aspeed_gpio *gpio, unsigned int offset) > } > > static void aspeed_gpio_change_cmd_source(struct aspeed_gpio *gpio, > - const struct aspeed_gpio_bank *bank, > - int bindex, int cmdsrc) > + unsigned int offset, > + int cmdsrc) > { > - void __iomem *c0 = bank_reg(gpio, bank, reg_cmdsrc0); > - void __iomem *c1 = bank_reg(gpio, bank, reg_cmdsrc1); > - u32 bit, reg; > - > /* > - * Each register controls 4 banks, so take the bottom 2 > - * bits of the bank index, and use them to select the > - * right control bit (0, 8, 16 or 24). > + * The command source register is only valid in bits 0, 8, 16, and 24, so we use > + * (offset & ~(0x7)) to ensure that reg_bits_set always targets a valid bit. > */ > - bit = BIT((bindex & 3) << 3); > - > /* Source 1 first to avoid illegal 11 combination */ > - reg = ioread32(c1); > - if (cmdsrc & 2) > - reg |= bit; > - else > - reg &= ~bit; > - iowrite32(reg, c1); > - > + gpio->llops->reg_bits_set(gpio, offset & ~(0x7), reg_cmdsrc1, !!(cmdsrc & BIT(1))); > /* Then Source 0 */ > - reg = ioread32(c0); > - if (cmdsrc & 1) > - reg |= bit; > - else > - reg &= ~bit; > - iowrite32(reg, c0); > + gpio->llops->reg_bits_set(gpio, offset & ~(0x7), reg_cmdsrc0, !!(cmdsrc & BIT(0))); > } > > static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio, > @@ -343,7 +334,7 @@ static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio, > copro_ops->request_access(copro_data); > > /* Change command source back to ARM */ > - aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3, GPIO_CMDSRC_ARM); > + aspeed_gpio_change_cmd_source(gpio, offset, GPIO_CMDSRC_ARM); > > /* Update cache */ > gpio->dcache[GPIO_BANK(offset)] = ioread32(bank_reg(gpio, bank, reg_rdata)); > @@ -354,8 +345,6 @@ static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio, > static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio, > unsigned int offset) > { > - const struct aspeed_gpio_bank *bank = to_bank(offset); > - > if (!copro_ops || !gpio->cf_copro_bankmap) > return; > if (!gpio->cf_copro_bankmap[offset >> 3]) > @@ -364,7 +353,7 @@ static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio, > return; > > /* Change command source back to ColdFire */ > - aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3, > + aspeed_gpio_change_cmd_source(gpio, offset, > GPIO_CMDSRC_COLDFIRE); > > /* Restart the coprocessor */ > @@ -374,29 +363,24 @@ static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio, > static int aspeed_gpio_get(struct gpio_chip *gc, unsigned int offset) > { > struct aspeed_gpio *gpio = gpiochip_get_data(gc); > - const struct aspeed_gpio_bank *bank = to_bank(offset); > > - return !!(ioread32(bank_reg(gpio, bank, reg_val)) & GPIO_BIT(offset)); > + return gpio->llops->reg_bits_read(gpio, offset, reg_val); > } > > static void __aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset, > int val) > { > struct aspeed_gpio *gpio = gpiochip_get_data(gc); > - const struct aspeed_gpio_bank *bank = to_bank(offset); > - void __iomem *addr; > u32 reg; > > - addr = bank_reg(gpio, bank, reg_val); > reg = gpio->dcache[GPIO_BANK(offset)]; > - > if (val) > reg |= GPIO_BIT(offset); > else > reg &= ~GPIO_BIT(offset); > gpio->dcache[GPIO_BANK(offset)] = reg; > > - iowrite32(reg, addr); > + gpio->llops->reg_bits_set(gpio, offset, reg_val, val); > } > > static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset, > @@ -407,36 +391,32 @@ static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset, > bool copro; > > raw_spin_lock_irqsave(&gpio->lock, flags); > - copro = aspeed_gpio_copro_request(gpio, offset); > + if (gpio->llops->copro_request) > + copro = gpio->llops->copro_request(gpio, offset); A bit minor perhaps, but I'd add a static function to do the test and call: static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio, unsigned int offset) { if (gpio->llops->copro_request) return gpio->llops->copro_request(gpio, offset); return false; } static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio, unsigned int offset) { if (gpio->llops->copro_release) gpio->llops->copro_release(gpio, offset); } A bit less noise at the call-sites that way. That's only a suggestion for the optional copro function pointers though. For the reg_bits_read and reg_bits_set callbacks we should ensure they're not NULL in the driver probe() implementation, or error out if they are. > > __aspeed_gpio_set(gc, offset, val); > > - if (copro) > - aspeed_gpio_copro_release(gpio, offset); > + if (copro && gpio->llops->copro_release) > + gpio->llops->copro_release(gpio, offset); > raw_spin_unlock_irqrestore(&gpio->lock, flags); > } > > static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset) > { > struct aspeed_gpio *gpio = gpiochip_get_data(gc); > - const struct aspeed_gpio_bank *bank = to_bank(offset); > - void __iomem *addr = bank_reg(gpio, bank, reg_dir); > unsigned long flags; > bool copro; > - u32 reg; > > if (!have_input(gpio, offset)) > return -ENOTSUPP; > > raw_spin_lock_irqsave(&gpio->lock, flags); > > - reg = ioread32(addr); > - reg &= ~GPIO_BIT(offset); > - > - copro = aspeed_gpio_copro_request(gpio, offset); > - iowrite32(reg, addr); > - if (copro) > - aspeed_gpio_copro_release(gpio, offset); > + if (gpio->llops->copro_request) > + copro = gpio->llops->copro_request(gpio, offset); > + gpio->llops->reg_bits_set(gpio, offset, reg_dir, 0); > + if (copro && gpio->llops->copro_release) > + gpio->llops->copro_release(gpio, offset); > > raw_spin_unlock_irqrestore(&gpio->lock, flags); > > @@ -447,26 +427,21 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc, > unsigned int offset, int val) > { > struct aspeed_gpio *gpio = gpiochip_get_data(gc); > - const struct aspeed_gpio_bank *bank = to_bank(offset); > - void __iomem *addr = bank_reg(gpio, bank, reg_dir); > unsigned long flags; > bool copro; > - u32 reg; > > if (!have_output(gpio, offset)) > return -ENOTSUPP; > > raw_spin_lock_irqsave(&gpio->lock, flags); > > - reg = ioread32(addr); > - reg |= GPIO_BIT(offset); > - > - copro = aspeed_gpio_copro_request(gpio, offset); > + if (gpio->llops->copro_request) > + copro = gpio->llops->copro_request(gpio, offset); > __aspeed_gpio_set(gc, offset, val); > - iowrite32(reg, addr); > + gpio->llops->reg_bits_set(gpio, offset, reg_dir, 1); > > - if (copro) > - aspeed_gpio_copro_release(gpio, offset); > + if (copro && gpio->llops->copro_release) > + gpio->llops->copro_release(gpio, offset); > raw_spin_unlock_irqrestore(&gpio->lock, flags); > > return 0; > @@ -475,7 +450,6 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc, > static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset) > { > struct aspeed_gpio *gpio = gpiochip_get_data(gc); > - const struct aspeed_gpio_bank *bank = to_bank(offset); > unsigned long flags; > u32 val; > > @@ -487,7 +461,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset) > > raw_spin_lock_irqsave(&gpio->lock, flags); > > - val = ioread32(bank_reg(gpio, bank, reg_dir)) & GPIO_BIT(offset); > + val = gpio->llops->reg_bits_read(gpio, offset, reg_dir); > > raw_spin_unlock_irqrestore(&gpio->lock, flags); > > @@ -496,8 +470,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset) > > static inline int irqd_to_aspeed_gpio_data(struct irq_data *d, > struct aspeed_gpio **gpio, > - const struct aspeed_gpio_bank **bank, > - u32 *bit, int *offset) > + int *offset) > { > struct aspeed_gpio *internal; > > @@ -510,70 +483,55 @@ static inline int irqd_to_aspeed_gpio_data(struct irq_data *d, > return -ENOTSUPP; > > *gpio = internal; > - *bank = to_bank(*offset); > - *bit = GPIO_BIT(*offset); > > return 0; > } > > static void aspeed_gpio_irq_ack(struct irq_data *d) > { > - const struct aspeed_gpio_bank *bank; > struct aspeed_gpio *gpio; > unsigned long flags; > - void __iomem *status_addr; > int rc, offset; > bool copro; > - u32 bit; > > - rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset); > + rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset); > if (rc) > return; > > - status_addr = bank_reg(gpio, bank, reg_irq_status); > - > raw_spin_lock_irqsave(&gpio->lock, flags); > - copro = aspeed_gpio_copro_request(gpio, offset); > + if (gpio->llops->copro_request) > + copro = gpio->llops->copro_request(gpio, offset); > > - iowrite32(bit, status_addr); > + gpio->llops->reg_bits_set(gpio, offset, reg_irq_status, 1); > > - if (copro) > - aspeed_gpio_copro_release(gpio, offset); > + if (copro && gpio->llops->copro_release) > + gpio->llops->copro_release(gpio, offset); > raw_spin_unlock_irqrestore(&gpio->lock, flags); > } > > static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set) > { > - const struct aspeed_gpio_bank *bank; > struct aspeed_gpio *gpio; > unsigned long flags; > - u32 reg, bit; > - void __iomem *addr; > int rc, offset; > bool copro; > > - rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset); > + rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset); > if (rc) > return; > > - addr = bank_reg(gpio, bank, reg_irq_enable); > - > /* Unmasking the IRQ */ > if (set) > gpiochip_enable_irq(&gpio->chip, irqd_to_hwirq(d)); > > raw_spin_lock_irqsave(&gpio->lock, flags); > - copro = aspeed_gpio_copro_request(gpio, offset); > + if (gpio->llops->copro_request) > + copro = gpio->llops->copro_request(gpio, offset); > > - reg = ioread32(addr); > - if (set) > - reg |= bit; > - else > - reg &= ~bit; > - iowrite32(reg, addr); > + gpio->llops->reg_bits_set(gpio, offset, reg_irq_enable, set); > > - if (copro) > - aspeed_gpio_copro_release(gpio, offset); > + if (copro && gpio->llops->copro_release) > + gpio->llops->copro_release(gpio, offset); > raw_spin_unlock_irqrestore(&gpio->lock, flags); > > /* Masking the IRQ */ > @@ -596,34 +554,31 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type) > u32 type0 = 0; > u32 type1 = 0; > u32 type2 = 0; > - u32 bit, reg; > - const struct aspeed_gpio_bank *bank; > irq_flow_handler_t handler; > struct aspeed_gpio *gpio; > unsigned long flags; > - void __iomem *addr; > int rc, offset; > bool copro; > > - rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset); > + rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset); > if (rc) > return -EINVAL; > > switch (type & IRQ_TYPE_SENSE_MASK) { > case IRQ_TYPE_EDGE_BOTH: > - type2 |= bit; > + type2 = 1; > fallthrough; > case IRQ_TYPE_EDGE_RISING: > - type0 |= bit; > + type0 = 1; > fallthrough; > case IRQ_TYPE_EDGE_FALLING: > handler = handle_edge_irq; > break; > case IRQ_TYPE_LEVEL_HIGH: > - type0 |= bit; > + type0 = 1; > fallthrough; > case IRQ_TYPE_LEVEL_LOW: > - type1 |= bit; > + type1 = 1; > handler = handle_level_irq; > break; > default: > @@ -631,25 +586,15 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type) > } > > raw_spin_lock_irqsave(&gpio->lock, flags); > - copro = aspeed_gpio_copro_request(gpio, offset); > - > - addr = bank_reg(gpio, bank, reg_irq_type0); > - reg = ioread32(addr); > - reg = (reg & ~bit) | type0; > - iowrite32(reg, addr); > + if (gpio->llops->copro_request) > + copro = gpio->llops->copro_request(gpio, offset); > > - addr = bank_reg(gpio, bank, reg_irq_type1); > - reg = ioread32(addr); > - reg = (reg & ~bit) | type1; > - iowrite32(reg, addr); > + gpio->llops->reg_bits_set(gpio, offset, reg_irq_type0, type0); > + gpio->llops->reg_bits_set(gpio, offset, reg_irq_type1, type1); > + gpio->llops->reg_bits_set(gpio, offset, reg_irq_type2, type2); > > - addr = bank_reg(gpio, bank, reg_irq_type2); > - reg = ioread32(addr); > - reg = (reg & ~bit) | type2; > - iowrite32(reg, addr); > - > - if (copro) > - aspeed_gpio_copro_release(gpio, offset); > + if (copro && gpio->llops->copro_release) > + gpio->llops->copro_release(gpio, offset); > raw_spin_unlock_irqrestore(&gpio->lock, flags); > > irq_set_handler_locked(d, handler); > @@ -661,7 +606,6 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc) > { > struct gpio_chip *gc = irq_desc_get_handler_data(desc); > struct irq_chip *ic = irq_desc_get_chip(desc); > - struct aspeed_gpio *data = gpiochip_get_data(gc); > unsigned int i, p, banks; > unsigned long reg; > struct aspeed_gpio *gpio = gpiochip_get_data(gc); > @@ -670,9 +614,7 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc) > > banks = DIV_ROUND_UP(gpio->chip.ngpio, 32); > for (i = 0; i < banks; i++) { > - const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i]; > - > - reg = ioread32(bank_reg(data, bank, reg_irq_status)); > + reg = gpio->llops->reg_bits_read(gpio, i, reg_irq_status); > > for_each_set_bit(p, ®, 32) > generic_handle_domain_irq(gc->irq.domain, i * 32 + p); > @@ -711,26 +653,16 @@ static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip, > { > struct aspeed_gpio *gpio = gpiochip_get_data(chip); > unsigned long flags; > - void __iomem *treg; > bool copro; > - u32 val; > - > - treg = bank_reg(gpio, to_bank(offset), reg_tolerance); > > raw_spin_lock_irqsave(&gpio->lock, flags); > - copro = aspeed_gpio_copro_request(gpio, offset); > + if (gpio->llops->copro_request) > + copro = gpio->llops->copro_request(gpio, offset); > > - val = readl(treg); > + gpio->llops->reg_bits_set(gpio, offset, reg_tolerance, enable); > > - if (enable) > - val |= GPIO_BIT(offset); > - else > - val &= ~GPIO_BIT(offset); > - > - writel(val, treg); > - > - if (copro) > - aspeed_gpio_copro_release(gpio, offset); > + if (copro && gpio->llops->copro_release) > + gpio->llops->copro_release(gpio, offset); > raw_spin_unlock_irqrestore(&gpio->lock, flags); > > return 0; > @@ -821,21 +753,11 @@ static inline bool timer_allocation_registered(struct aspeed_gpio *gpio, > static void configure_timer(struct aspeed_gpio *gpio, unsigned int offset, > unsigned int timer) > { > - const struct aspeed_gpio_bank *bank = to_bank(offset); > - const u32 mask = GPIO_BIT(offset); > - void __iomem *addr; > - u32 val; > - > /* Note: Debounce timer isn't under control of the command > * source registers, so no need to sync with the coprocessor > */ > - addr = bank_reg(gpio, bank, reg_debounce_sel1); > - val = ioread32(addr); > - iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE1(timer, offset), addr); > - > - addr = bank_reg(gpio, bank, reg_debounce_sel2); > - val = ioread32(addr); > - iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE2(timer, offset), addr); > + gpio->llops->reg_bits_set(gpio, offset, reg_debounce_sel1, !!(timer & BIT(1))); > + gpio->llops->reg_bits_set(gpio, offset, reg_debounce_sel2, !!(timer & BIT(0))); > } > > static int enable_debounce(struct gpio_chip *chip, unsigned int offset, > @@ -866,15 +788,15 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset, > } > > /* Try to find a timer already configured for the debounce period */ > - for (i = 1; i < ARRAY_SIZE(debounce_timers); i++) { > + for (i = 1; i < gpio->debounce_timers_num; i++) { > u32 cycles; > > - cycles = ioread32(gpio->base + debounce_timers[i]); > + cycles = ioread32(gpio->base + gpio->debounce_timers_array[i]); > if (requested_cycles == cycles) > break; > } > > - if (i == ARRAY_SIZE(debounce_timers)) { > + if (i == gpio->debounce_timers_num) { > int j; > > /* > @@ -905,7 +827,7 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset, > > i = j; > > - iowrite32(requested_cycles, gpio->base + debounce_timers[i]); > + iowrite32(requested_cycles, gpio->base + gpio->debounce_timers_array[i]); > } > > if (WARN(i == 0, "Cannot register index of disabled timer\n")) { > @@ -1027,7 +949,7 @@ int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc, > > /* Switch command source */ > if (gpio->cf_copro_bankmap[bindex] == 1) > - aspeed_gpio_change_cmd_source(gpio, bank, bindex, > + aspeed_gpio_change_cmd_source(gpio, offset, > GPIO_CMDSRC_COLDFIRE); > > if (vreg_offset) > @@ -1051,7 +973,6 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc) > struct gpio_chip *chip = gpiod_to_chip(desc); > struct aspeed_gpio *gpio = gpiochip_get_data(chip); > int rc = 0, bindex, offset = gpio_chip_hwgpio(desc); > - const struct aspeed_gpio_bank *bank = to_bank(offset); > unsigned long flags; > > if (!gpio->cf_copro_bankmap) > @@ -1072,7 +993,7 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc) > > /* Switch command source */ > if (gpio->cf_copro_bankmap[bindex] == 0) > - aspeed_gpio_change_cmd_source(gpio, bank, bindex, > + aspeed_gpio_change_cmd_source(gpio, offset, > GPIO_CMDSRC_ARM); > bail: > raw_spin_unlock_irqrestore(&gpio->lock, flags); > @@ -1082,12 +1003,10 @@ EXPORT_SYMBOL_GPL(aspeed_gpio_copro_release_gpio); > > static void aspeed_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p) > { > - const struct aspeed_gpio_bank *bank; > struct aspeed_gpio *gpio; > - u32 bit; > int rc, offset; > > - rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset); > + rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset); > if (rc) > return; > > @@ -1120,7 +1039,7 @@ static const struct aspeed_bank_props ast2400_bank_props[] = { > > static const struct aspeed_gpio_config ast2400_config = > /* 220 for simplicity, really 216 with two 4-GPIO holes, four at end */ > - { .nr_gpios = 220, .props = ast2400_bank_props, }; > + { .nr_gpios = 220, .props = ast2400_bank_props, .version = 4}; > > static const struct aspeed_bank_props ast2500_bank_props[] = { > /* input output */ > @@ -1132,7 +1051,7 @@ static const struct aspeed_bank_props ast2500_bank_props[] = { > > static const struct aspeed_gpio_config ast2500_config = > /* 232 for simplicity, actual number is 228 (4-GPIO hole in GPIOAB) */ > - { .nr_gpios = 232, .props = ast2500_bank_props, }; > + { .nr_gpios = 232, .props = ast2500_bank_props, .version = 4}; > > static const struct aspeed_bank_props ast2600_bank_props[] = { > /* input output */ > @@ -1148,7 +1067,7 @@ static const struct aspeed_gpio_config ast2600_config = > * We expect ngpio being set in the device tree and this is a fallback > * option. > */ > - { .nr_gpios = 208, .props = ast2600_bank_props, }; > + { .nr_gpios = 208, .props = ast2600_bank_props, .version = 4}; > > static const struct of_device_id aspeed_gpio_of_table[] = { > { .compatible = "aspeed,ast2400-gpio", .data = &ast2400_config, }, > @@ -1158,6 +1077,40 @@ static const struct of_device_id aspeed_gpio_of_table[] = { > }; > MODULE_DEVICE_TABLE(of, aspeed_gpio_of_table); > > +static void aspeed_g4_reg_bits_set(struct aspeed_gpio *gpio, unsigned int offset, > + const enum aspeed_gpio_reg reg, u32 val) > +{ > + const struct aspeed_gpio_bank *bank = to_bank(offset); > + void __iomem *addr = bank_reg(gpio, bank, reg); > + u32 temp; > + > + temp = ioread32(addr); > + if (val) > + temp |= GPIO_BIT(offset); > + else > + temp &= ~GPIO_BIT(offset); > + > + iowrite32(temp, addr); > +} > + > +static u32 aspeed_g4_reg_bits_read(struct aspeed_gpio *gpio, unsigned int offset, > + const enum aspeed_gpio_reg reg) > +{ > + const struct aspeed_gpio_bank *bank = to_bank(offset); > + void __iomem *addr = bank_reg(gpio, bank, reg); > + > + if (reg == reg_rdata) > + return ioread32(addr); > + return !!(ioread32(addr) & GPIO_BIT(offset)); > +} > + > +struct aspeed_gpio_llops aspeed_g4_llops = { > + .copro_request = aspeed_gpio_copro_request, > + .copro_release = aspeed_gpio_copro_release, > + .reg_bits_set = aspeed_g4_reg_bits_set, > + .reg_bits_read = aspeed_g4_reg_bits_read, > +}; > + > static int __init aspeed_gpio_probe(struct platform_device *pdev) > { > const struct of_device_id *gpio_id; > @@ -1191,6 +1144,18 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev) > > gpio->config = gpio_id->data; > > + if (gpio->config->version == 4) { > + if (!gpio->llops) > + gpio->llops = &aspeed_g4_llops; > + > + if (!gpio->debounce_timers_array) { > + gpio->debounce_timers_array = debounce_timers; > + gpio->debounce_timers_num = ARRAY_SIZE(debounce_timers); > + } Why not embed the llops and debounce array pointer/size straight into the config struct that we provide via .data in the OF match table? I think that would just mean reordering some of the function and struct definitions in the source? That way we can get rid of the version member. Also, let's make sure here that the reg_bits_set and reg_bits_read callbacks are not NULL (and error out of aspeed_gpio_probe() if they are). On the whole though, I feel this change turns out to be a decent cleanup. It pushes the bank/offset bit-hackery down and away from the call-sites. Andrew > + } else { > + return -EOPNOTSUPP; > + } > + > gpio->chip.parent = &pdev->dev; > err = of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpio); > gpio->chip.ngpio = (u16) ngpio; > @@ -1218,15 +1183,17 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev) > * Populate it with initial values read from the HW and switch > * all command sources to the ARM by default > */ > - for (i = 0; i < banks; i++) { > - const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i]; > - void __iomem *addr = bank_reg(gpio, bank, reg_rdata); > - gpio->dcache[i] = ioread32(addr); > - aspeed_gpio_change_cmd_source(gpio, bank, 0, GPIO_CMDSRC_ARM); > - aspeed_gpio_change_cmd_source(gpio, bank, 1, GPIO_CMDSRC_ARM); > - aspeed_gpio_change_cmd_source(gpio, bank, 2, GPIO_CMDSRC_ARM); > - aspeed_gpio_change_cmd_source(gpio, bank, 3, GPIO_CMDSRC_ARM); > - } > + if (gpio->config->version == 4) > + for (i = 0; i < banks; i++) { > + const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i]; > + void __iomem *addr = bank_reg(gpio, bank, reg_rdata); > + > + gpio->dcache[i] = ioread32(addr); > + aspeed_gpio_change_cmd_source(gpio, i * 8 + 0, GPIO_CMDSRC_ARM); > + aspeed_gpio_change_cmd_source(gpio, i * 8 + 8, GPIO_CMDSRC_ARM); > + aspeed_gpio_change_cmd_source(gpio, i * 8 + 16, GPIO_CMDSRC_ARM); > + aspeed_gpio_change_cmd_source(gpio, i * 8 + 24, GPIO_CMDSRC_ARM); > + } > > /* Set up an irqchip */ > irq = platform_get_irq(pdev, 0); ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 4/4] gpio: aspeed: Support G7 Aspeed gpio controller 2024-08-30 3:40 [PATCH v2 0/4] Add Aspeed G7 gpio support Billy Tsai ` (2 preceding siblings ...) 2024-08-30 3:40 ` [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access Billy Tsai @ 2024-08-30 3:40 ` Billy Tsai 2024-08-31 3:39 ` kernel test robot 3 siblings, 1 reply; 15+ messages in thread From: Billy Tsai @ 2024-08-30 3:40 UTC (permalink / raw) To: linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, andrew, linux-gpio, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, BMC-SW In the 7th generation of the SoC from Aspeed, the control logic of the GPIO controller has been updated to support per-pin control. Each pin now has its own 32-bit register, allowing for individual control of the pin’s value, direction, interrupt type, and other settings. The permission for coprocessor access is supported by the hardware but hasn’t been implemented in the current patch. Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> --- drivers/gpio/gpio-aspeed.c | 116 +++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c index 74c4e80958bf..24f9c7312351 100644 --- a/drivers/gpio/gpio-aspeed.c +++ b/drivers/gpio/gpio-aspeed.c @@ -30,6 +30,23 @@ #include <linux/gpio/consumer.h> #include "gpiolib.h" +#define GPIO_G7_IRQ_STS_BASE 0x100 +#define GPIO_G7_IRQ_STS_OFFSET(x) (GPIO_G7_IRQ_STS_BASE + (x) * 0x4) +#define GPIO_G7_CTRL_REG_BASE 0x180 +#define GPIO_G7_CTRL_REG_OFFSET(x) (GPIO_G7_CTRL_REG_BASE + (x) * 0x4) +#define GPIO_G7_CTRL_OUT_DATA BIT(0) +#define GPIO_G7_CTRL_DIR BIT(1) +#define GPIO_G7_CTRL_IRQ_EN BIT(2) +#define GPIO_G7_CTRL_IRQ_TYPE0 BIT(3) +#define GPIO_G7_CTRL_IRQ_TYPE1 BIT(4) +#define GPIO_G7_CTRL_IRQ_TYPE2 BIT(5) +#define GPIO_G7_CTRL_RST_TOLERANCE BIT(6) +#define GPIO_G7_CTRL_DEBOUNCE_SEL2 BIT(7) +#define GPIO_G7_CTRL_DEBOUNCE_SEL1 BIT(8) +#define GPIO_G7_CTRL_INPUT_MASK BIT(9) +#define GPIO_G7_CTRL_IRQ_STS BIT(12) +#define GPIO_G7_CTRL_IN_DATA BIT(13) + struct aspeed_bank_props { unsigned int bank; u32 input; @@ -95,6 +112,7 @@ struct aspeed_gpio_bank { */ static const int debounce_timers[4] = { 0x00, 0x50, 0x54, 0x58 }; +static const int g7_debounce_timers[4] = { 0x00, 0x04, 0x00, 0x08 }; static const struct aspeed_gpio_copro_ops *copro_ops; static void *copro_data; @@ -246,6 +264,39 @@ static inline void __iomem *bank_reg(struct aspeed_gpio *gpio, BUG(); } +static inline u32 reg_mask(const enum aspeed_gpio_reg reg) +{ + switch (reg) { + case reg_val: + return GPIO_G7_CTRL_OUT_DATA; + case reg_dir: + return GPIO_G7_CTRL_DIR; + case reg_irq_enable: + return GPIO_G7_CTRL_IRQ_EN; + case reg_irq_type0: + return GPIO_G7_CTRL_IRQ_TYPE0; + case reg_irq_type1: + return GPIO_G7_CTRL_IRQ_TYPE1; + case reg_irq_type2: + return GPIO_G7_CTRL_IRQ_TYPE2; + case reg_tolerance: + return GPIO_G7_CTRL_RST_TOLERANCE; + case reg_debounce_sel1: + return GPIO_G7_CTRL_DEBOUNCE_SEL1; + case reg_debounce_sel2: + return GPIO_G7_CTRL_DEBOUNCE_SEL2; + case reg_rdata: + return GPIO_G7_CTRL_OUT_DATA; + case reg_irq_status: + return GPIO_G7_CTRL_IRQ_STS; + case reg_cmdsrc0: + case reg_cmdsrc1: + default: + WARN_ON_ONCE(1); + return 0; + } +} + #define GPIO_BANK(x) ((x) >> 5) #define GPIO_OFFSET(x) ((x) & 0x1f) #define GPIO_BIT(x) BIT(GPIO_OFFSET(x)) @@ -930,6 +981,9 @@ int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc, const struct aspeed_gpio_bank *bank = to_bank(offset); unsigned long flags; + if (gpio->config->version == 7) + return -EOPNOTSUPP; + if (!gpio->cf_copro_bankmap) gpio->cf_copro_bankmap = kzalloc(gpio->chip.ngpio >> 3, GFP_KERNEL); if (!gpio->cf_copro_bankmap) @@ -975,6 +1029,9 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc) int rc = 0, bindex, offset = gpio_chip_hwgpio(desc); unsigned long flags; + if (gpio->config->version == 7) + return -EOPNOTSUPP; + if (!gpio->cf_copro_bankmap) return -ENXIO; @@ -1069,10 +1126,27 @@ static const struct aspeed_gpio_config ast2600_config = */ { .nr_gpios = 208, .props = ast2600_bank_props, .version = 4}; +static const struct aspeed_bank_props ast2700_bank_props[] = { + /* input output */ + { 1, 0x0fffffff, 0x0fffffff }, /* E/F/G/H, 4-GPIO hole */ + { 6, 0x00ffffff, 0x00ffffff }, /* Y/Z/AA */ + {}, +}; + +static const struct aspeed_gpio_config ast2700_config = + /* + * ast2700 has two controllers one with 212 GPIOs and one with 16 GPIOs. + * 216 for simplicity, actual number is 212 (4-GPIO hole in GPIOH) + * We expect ngpio being set in the device tree and this is a fallback + * option. + */ + { .nr_gpios = 216, .props = ast2700_bank_props, .version = 7 }; + static const struct of_device_id aspeed_gpio_of_table[] = { { .compatible = "aspeed,ast2400-gpio", .data = &ast2400_config, }, { .compatible = "aspeed,ast2500-gpio", .data = &ast2500_config, }, { .compatible = "aspeed,ast2600-gpio", .data = &ast2600_config, }, + { .compatible = "aspeed,ast2700-gpio", .data = &ast2700_config, }, {} }; MODULE_DEVICE_TABLE(of, aspeed_gpio_of_table); @@ -1111,6 +1185,40 @@ struct aspeed_gpio_llops aspeed_g4_llops = { .reg_bits_read = aspeed_g4_reg_bits_read, }; +static void aspeed_g7_reg_bits_set(struct aspeed_gpio *gpio, unsigned int offset, + const enum aspeed_gpio_reg reg, u32 val) +{ + u32 mask = reg_mask(reg); + void __iomem *addr = gpio->base + GPIO_G7_CTRL_REG_OFFSET(offset); + u32 write_val = (ioread32(addr) & ~(mask)) | (((val) << (ffs(mask) - 1)) & (mask)); + + iowrite32(write_val, addr); +} + +static u32 aspeed_g7_reg_bits_read(struct aspeed_gpio *gpio, unsigned int offset, + const enum aspeed_gpio_reg reg) +{ + u32 mask = reg_mask(reg); + void __iomem *addr; + + if (reg == reg_irq_status) { + addr = gpio->base + GPIO_G7_IRQ_STS_OFFSET(offset); + return ioread32(addr); + } + addr = gpio->base + GPIO_G7_CTRL_REG_OFFSET(offset); + if (reg == reg_val) + mask = GPIO_G7_CTRL_IN_DATA; + + return (((ioread32(addr)) & (mask)) >> (ffs(mask) - 1)); +} + +struct aspeed_gpio_llops aspeed_g7_llops = { + .copro_request = NULL, + .copro_release = NULL, + .reg_bits_set = aspeed_g7_reg_bits_set, + .reg_bits_read = aspeed_g7_reg_bits_read, +}; + static int __init aspeed_gpio_probe(struct platform_device *pdev) { const struct of_device_id *gpio_id; @@ -1152,6 +1260,14 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev) gpio->debounce_timers_array = debounce_timers; gpio->debounce_timers_num = ARRAY_SIZE(debounce_timers); } + } else if (gpio->config->version == 7) { + if (!gpio->llops) + gpio->llops = &aspeed_g7_llops; + + if (!gpio->debounce_timers_array) { + gpio->debounce_timers_array = g7_debounce_timers; + gpio->debounce_timers_num = ARRAY_SIZE(g7_debounce_timers); + } } else { return -EOPNOTSUPP; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/4] gpio: aspeed: Support G7 Aspeed gpio controller 2024-08-30 3:40 ` [PATCH v2 4/4] gpio: aspeed: Support G7 Aspeed gpio controller Billy Tsai @ 2024-08-31 3:39 ` kernel test robot 0 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2024-08-31 3:39 UTC (permalink / raw) To: Billy Tsai, linus.walleij, brgl, robh, krzk+dt, conor+dt, joel, andrew, linux-gpio, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, BMC-SW Cc: oe-kbuild-all Hi Billy, kernel test robot noticed the following build warnings: [auto build test WARNING on brgl/gpio/for-next] [also build test WARNING on linus/master v6.11-rc5 next-20240830] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Billy-Tsai/dt-bindings-gpio-aspeed-ast2400-gpio-Support-ast2700/20240830-114325 base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next patch link: https://lore.kernel.org/r/20240830034047.2251482-5-billy_tsai%40aspeedtech.com patch subject: [PATCH v2 4/4] gpio: aspeed: Support G7 Aspeed gpio controller config: openrisc-randconfig-r131-20240830 (https://download.01.org/0day-ci/archive/20240831/202408311104.T3i0AxEf-lkp@intel.com/config) compiler: or1k-linux-gcc (GCC) 14.1.0 reproduce: (https://download.01.org/0day-ci/archive/20240831/202408311104.T3i0AxEf-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408311104.T3i0AxEf-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) drivers/gpio/gpio-aspeed.c:1181:26: sparse: sparse: symbol 'aspeed_g4_llops' was not declared. Should it be static? >> drivers/gpio/gpio-aspeed.c:1215:26: sparse: sparse: symbol 'aspeed_g7_llops' was not declared. Should it be static? vim +/aspeed_g7_llops +1215 drivers/gpio/gpio-aspeed.c 1214 > 1215 struct aspeed_gpio_llops aspeed_g7_llops = { 1216 .copro_request = NULL, 1217 .copro_release = NULL, 1218 .reg_bits_set = aspeed_g7_reg_bits_set, 1219 .reg_bits_read = aspeed_g7_reg_bits_read, 1220 }; 1221 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-09-12 8:18 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-30 3:40 [PATCH v2 0/4] Add Aspeed G7 gpio support Billy Tsai 2024-08-30 3:40 ` [PATCH v2 1/4] dt-bindings: gpio: aspeed,ast2400-gpio: Support ast2700 Billy Tsai 2024-08-30 14:04 ` Conor Dooley 2024-09-02 2:32 ` Billy Tsai 2024-08-31 5:49 ` Krzysztof Kozlowski 2024-08-30 3:40 ` [PATCH v2 2/4] gpio: aspeed: Remove the name for bank array Billy Tsai 2024-09-12 7:51 ` Andrew Jeffery 2024-09-12 7:57 ` Billy Tsai 2024-08-30 3:40 ` [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access Billy Tsai 2024-08-30 16:07 ` kernel test robot 2024-08-31 1:46 ` kernel test robot 2024-08-31 16:05 ` Dan Carpenter 2024-09-12 8:18 ` Andrew Jeffery 2024-08-30 3:40 ` [PATCH v2 4/4] gpio: aspeed: Support G7 Aspeed gpio controller Billy Tsai 2024-08-31 3:39 ` kernel test robot
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).