* [PATCH V2 0/4] gpio: sprd: Modification of UNISOC Platform EIC Driver
@ 2023-09-21 9:00 Wenhua Lin
2023-09-21 9:00 ` [PATCH V2 1/4] gpio: sprd: In the sleep state, the eic debounce clk must be forced open Wenhua Lin
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Wenhua Lin @ 2023-09-21 9:00 UTC (permalink / raw)
To: Linus Walleij, Andy Shevchenko, Bartosz Golaszewski
Cc: Orson Zhai, Baolin Wang, Chunyan Zhang, linux-gpio, linux-kernel,
wenhua lin, Wenhua Lin, Xiongpeng Wu
Recently, some bugs have been discovered during use, and patch1
and patch2 are bug fixes. Also, this patchset add optimization:
patch3 optimization the calculation method of eic number,
and patch4 Support 8 banks EIC controller.
Change in V2:
-Using thread send 4 patches
-The dbnc of the title is changed to debounce in PATCH 1/4.
-Add Fixes tag in PATCH 1/4.
-Change commit message and title in PATCH 2/4.
-Change commit message in PATCH 3/4.
-Remove modifications to SPRD_EIC_MAX_BANK macro in PATCH 3/4.
-Remove modifications to fallthrough in PATCH 3/4.
-Add Fixes tag in PATCH 3/4.
-PATCH 3/4 macro modify split into a separate patch in PATCH 4/4.
-Change related comments in PATCH 4/4.
Wenhua Lin (4):
gpio: sprd: In the sleep state, the eic debounce clk must be forced
open
gpio: sprd: Clear interrupt after set the interrupt type
gpio: sprd: Modify the calculation method of eic number
gpio: sprd: Support 8 banks EIC controller
drivers/gpio/gpio-eic-sprd.c | 63 ++++++++++++++++++++++--------------
1 file changed, 38 insertions(+), 25 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V2 1/4] gpio: sprd: In the sleep state, the eic debounce clk must be forced open
2023-09-21 9:00 [PATCH V2 0/4] gpio: sprd: Modification of UNISOC Platform EIC Driver Wenhua Lin
@ 2023-09-21 9:00 ` Wenhua Lin
2023-09-21 11:23 ` Andy Shevchenko
2023-09-27 9:08 ` Baolin Wang
2023-09-21 9:00 ` [PATCH V2 2/4] gpio: sprd: Clear interrupt after set the interrupt type Wenhua Lin
` (2 subsequent siblings)
3 siblings, 2 replies; 18+ messages in thread
From: Wenhua Lin @ 2023-09-21 9:00 UTC (permalink / raw)
To: Linus Walleij, Andy Shevchenko, Bartosz Golaszewski
Cc: Orson Zhai, Baolin Wang, Chunyan Zhang, linux-gpio, linux-kernel,
wenhua lin, Wenhua Lin, Xiongpeng Wu
In the sleep state, Eic debounce has no clock and the clk of
debounce needs to be forced open, so that eic can wake up normally.
Fixes: 2788938b7946 ("gpio: eic-sprd: Make the irqchip immutable")
Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
---
drivers/gpio/gpio-eic-sprd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
index 84352a6f4973..bfa8a4c7515a 100644
--- a/drivers/gpio/gpio-eic-sprd.c
+++ b/drivers/gpio/gpio-eic-sprd.c
@@ -23,6 +23,7 @@
#define SPRD_EIC_DBNC_IC 0x24
#define SPRD_EIC_DBNC_TRIG 0x28
#define SPRD_EIC_DBNC_CTRL0 0x40
+#define SPRD_EIC_DBNC_FORCE_CLK 0x8000
#define SPRD_EIC_LATCH_INTEN 0x0
#define SPRD_EIC_LATCH_INTRAW 0x4
@@ -214,6 +215,7 @@ static int sprd_eic_set_debounce(struct gpio_chip *chip, unsigned int offset,
u32 value = readl_relaxed(base + reg) & ~SPRD_EIC_DBNC_MASK;
value |= (debounce / 1000) & SPRD_EIC_DBNC_MASK;
+ value |= SPRD_EIC_DBNC_FORCE_CLK;
writel_relaxed(value, base + reg);
return 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH V2 2/4] gpio: sprd: Clear interrupt after set the interrupt type
2023-09-21 9:00 [PATCH V2 0/4] gpio: sprd: Modification of UNISOC Platform EIC Driver Wenhua Lin
2023-09-21 9:00 ` [PATCH V2 1/4] gpio: sprd: In the sleep state, the eic debounce clk must be forced open Wenhua Lin
@ 2023-09-21 9:00 ` Wenhua Lin
2023-09-21 11:24 ` Andy Shevchenko
2023-09-27 9:15 ` Baolin Wang
2023-09-21 9:00 ` [PATCH V2 3/4] gpio: sprd: Modify the calculation method of eic number Wenhua Lin
2023-09-21 9:00 ` [PATCH V2 4/4] gpio: sprd: Support 8 banks EIC controller Wenhua Lin
3 siblings, 2 replies; 18+ messages in thread
From: Wenhua Lin @ 2023-09-21 9:00 UTC (permalink / raw)
To: Linus Walleij, Andy Shevchenko, Bartosz Golaszewski
Cc: Orson Zhai, Baolin Wang, Chunyan Zhang, linux-gpio, linux-kernel,
wenhua lin, Wenhua Lin, Xiongpeng Wu
The initialization state of the EIC module is a high level trigger.
If it is currently a high level, the interrupt condition is met at
this time, and the EIC interrupt has a latch capability, which will
cause an interrupt to occur after booting. To avoid this, When setting
the EIC interrupt trigger type, clear the interrupt once.
Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
---
drivers/gpio/gpio-eic-sprd.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
index bfa8a4c7515a..96f1c7fd3988 100644
--- a/drivers/gpio/gpio-eic-sprd.c
+++ b/drivers/gpio/gpio-eic-sprd.c
@@ -375,29 +375,34 @@ static int sprd_eic_irq_set_type(struct irq_data *data, unsigned int flow_type)
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 0);
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 0);
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTPOL, 1);
+ sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTCLR, 1);
irq_set_handler_locked(data, handle_edge_irq);
break;
case IRQ_TYPE_EDGE_FALLING:
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 0);
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 0);
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTPOL, 0);
+ sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTCLR, 1);
irq_set_handler_locked(data, handle_edge_irq);
break;
case IRQ_TYPE_EDGE_BOTH:
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 0);
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 1);
+ sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTCLR, 1);
irq_set_handler_locked(data, handle_edge_irq);
break;
case IRQ_TYPE_LEVEL_HIGH:
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 0);
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 1);
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTPOL, 1);
+ sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTCLR, 1);
irq_set_handler_locked(data, handle_level_irq);
break;
case IRQ_TYPE_LEVEL_LOW:
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 0);
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 1);
sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTPOL, 0);
+ sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTCLR, 1);
irq_set_handler_locked(data, handle_level_irq);
break;
default:
@@ -410,29 +415,34 @@ static int sprd_eic_irq_set_type(struct irq_data *data, unsigned int flow_type)
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 0);
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 0);
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTPOL, 1);
+ sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTCLR, 1);
irq_set_handler_locked(data, handle_edge_irq);
break;
case IRQ_TYPE_EDGE_FALLING:
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 0);
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 0);
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTPOL, 0);
+ sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTCLR, 1);
irq_set_handler_locked(data, handle_edge_irq);
break;
case IRQ_TYPE_EDGE_BOTH:
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 0);
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 1);
+ sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTCLR, 1);
irq_set_handler_locked(data, handle_edge_irq);
break;
case IRQ_TYPE_LEVEL_HIGH:
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 0);
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 1);
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTPOL, 1);
+ sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTCLR, 1);
irq_set_handler_locked(data, handle_level_irq);
break;
case IRQ_TYPE_LEVEL_LOW:
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 0);
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 1);
sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTPOL, 0);
+ sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTCLR, 1);
irq_set_handler_locked(data, handle_level_irq);
break;
default:
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH V2 3/4] gpio: sprd: Modify the calculation method of eic number
2023-09-21 9:00 [PATCH V2 0/4] gpio: sprd: Modification of UNISOC Platform EIC Driver Wenhua Lin
2023-09-21 9:00 ` [PATCH V2 1/4] gpio: sprd: In the sleep state, the eic debounce clk must be forced open Wenhua Lin
2023-09-21 9:00 ` [PATCH V2 2/4] gpio: sprd: Clear interrupt after set the interrupt type Wenhua Lin
@ 2023-09-21 9:00 ` Wenhua Lin
2023-09-21 11:27 ` Andy Shevchenko
2023-09-27 9:24 ` Baolin Wang
2023-09-21 9:00 ` [PATCH V2 4/4] gpio: sprd: Support 8 banks EIC controller Wenhua Lin
3 siblings, 2 replies; 18+ messages in thread
From: Wenhua Lin @ 2023-09-21 9:00 UTC (permalink / raw)
To: Linus Walleij, Andy Shevchenko, Bartosz Golaszewski
Cc: Orson Zhai, Baolin Wang, Chunyan Zhang, linux-gpio, linux-kernel,
wenhua lin, Wenhua Lin, Xiongpeng Wu
When the soc changes, the corresponding gpio-eic-sprd.c
code needs to be modified, and the corresponding
Document must also be modified, which is quite troublesome.
To avoid modifying the driver file, the number of EICs
is automatically calculated by matching dts nodes.
Fixes: 2788938b7946 ("gpio: eic-sprd: Make the irqchip immutable")
Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
---
drivers/gpio/gpio-eic-sprd.c | 43 ++++++++++++++++++------------------
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
index 96f1c7fd3988..e85addbdf8aa 100644
--- a/drivers/gpio/gpio-eic-sprd.c
+++ b/drivers/gpio/gpio-eic-sprd.c
@@ -100,33 +100,32 @@ struct sprd_eic {
struct sprd_eic_variant_data {
enum sprd_eic_type type;
- u32 num_eics;
};
+#define SPRD_EIC_VAR_DATA(soc_name) \
+static const struct sprd_eic_variant_data soc_name##_eic_dbnc_data = { \
+ .type = SPRD_EIC_DEBOUNCE, \
+}; \
+ \
+static const struct sprd_eic_variant_data soc_name##_eic_latch_data = { \
+ .type = SPRD_EIC_LATCH, \
+}; \
+ \
+static const struct sprd_eic_variant_data soc_name##_eic_async_data = { \
+ .type = SPRD_EIC_ASYNC, \
+}; \
+ \
+static const struct sprd_eic_variant_data soc_name##_eic_sync_data = { \
+ .type = SPRD_EIC_SYNC, \
+}
+
+SPRD_EIC_VAR_DATA(sc9860);
+
static const char *sprd_eic_label_name[SPRD_EIC_MAX] = {
"eic-debounce", "eic-latch", "eic-async",
"eic-sync",
};
-static const struct sprd_eic_variant_data sc9860_eic_dbnc_data = {
- .type = SPRD_EIC_DEBOUNCE,
- .num_eics = 8,
-};
-
-static const struct sprd_eic_variant_data sc9860_eic_latch_data = {
- .type = SPRD_EIC_LATCH,
- .num_eics = 8,
-};
-
-static const struct sprd_eic_variant_data sc9860_eic_async_data = {
- .type = SPRD_EIC_ASYNC,
- .num_eics = 8,
-};
-
-static const struct sprd_eic_variant_data sc9860_eic_sync_data = {
- .type = SPRD_EIC_SYNC,
- .num_eics = 8,
-};
static inline void __iomem *sprd_eic_offset_base(struct sprd_eic *sprd_eic,
unsigned int bank)
@@ -595,6 +594,7 @@ static int sprd_eic_probe(struct platform_device *pdev)
struct sprd_eic *sprd_eic;
struct resource *res;
int ret, i;
+ u16 num_banks = 0;
pdata = of_device_get_match_data(&pdev->dev);
if (!pdata) {
@@ -625,12 +625,13 @@ static int sprd_eic_probe(struct platform_device *pdev)
break;
sprd_eic->base[i] = devm_ioremap_resource(&pdev->dev, res);
+ num_banks++;
if (IS_ERR(sprd_eic->base[i]))
return PTR_ERR(sprd_eic->base[i]);
}
sprd_eic->chip.label = sprd_eic_label_name[sprd_eic->type];
- sprd_eic->chip.ngpio = pdata->num_eics;
+ sprd_eic->chip.ngpio = num_banks * SPRD_EIC_PER_BANK_NR;
sprd_eic->chip.base = -1;
sprd_eic->chip.parent = &pdev->dev;
sprd_eic->chip.direction_input = sprd_eic_direction_input;
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH V2 4/4] gpio: sprd: Support 8 banks EIC controller
2023-09-21 9:00 [PATCH V2 0/4] gpio: sprd: Modification of UNISOC Platform EIC Driver Wenhua Lin
` (2 preceding siblings ...)
2023-09-21 9:00 ` [PATCH V2 3/4] gpio: sprd: Modify the calculation method of eic number Wenhua Lin
@ 2023-09-21 9:00 ` Wenhua Lin
2023-09-27 9:28 ` Baolin Wang
3 siblings, 1 reply; 18+ messages in thread
From: Wenhua Lin @ 2023-09-21 9:00 UTC (permalink / raw)
To: Linus Walleij, Andy Shevchenko, Bartosz Golaszewski
Cc: Orson Zhai, Baolin Wang, Chunyan Zhang, linux-gpio, linux-kernel,
wenhua lin, Wenhua Lin, Xiongpeng Wu
In order to solve the problem of insufficient eic,
it supports 8 banks of eic controller, each bank contains 8 eic.
Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
---
drivers/gpio/gpio-eic-sprd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
index e85addbdf8aa..6bb002060c3e 100644
--- a/drivers/gpio/gpio-eic-sprd.c
+++ b/drivers/gpio/gpio-eic-sprd.c
@@ -51,10 +51,10 @@
#define SPRD_EIC_SYNC_DATA 0x1c
/*
- * The digital-chip EIC controller can support maximum 3 banks, and each bank
+ * The digital-chip EIC controller can support maximum 8 banks, and each bank
* contains 8 EICs.
*/
-#define SPRD_EIC_MAX_BANK 3
+#define SPRD_EIC_MAX_BANK 8
#define SPRD_EIC_PER_BANK_NR 8
#define SPRD_EIC_DATA_MASK GENMASK(7, 0)
#define SPRD_EIC_BIT(x) ((x) & (SPRD_EIC_PER_BANK_NR - 1))
@@ -615,9 +615,9 @@ static int sprd_eic_probe(struct platform_device *pdev)
for (i = 0; i < SPRD_EIC_MAX_BANK; i++) {
/*
- * We can have maximum 3 banks EICs, and each EIC has
+ * We can have maximum 8 banks EICs, and each EIC has
* its own base address. But some platform maybe only
- * have one bank EIC, thus base[1] and base[2] can be
+ * have one bank EIC, thus base[1] and base[7] can be
* optional.
*/
res = platform_get_resource(pdev, IORESOURCE_MEM, i);
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH V2 1/4] gpio: sprd: In the sleep state, the eic debounce clk must be forced open
2023-09-21 9:00 ` [PATCH V2 1/4] gpio: sprd: In the sleep state, the eic debounce clk must be forced open Wenhua Lin
@ 2023-09-21 11:23 ` Andy Shevchenko
2024-01-02 8:34 ` wenhua lin
2023-09-27 9:08 ` Baolin Wang
1 sibling, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2023-09-21 11:23 UTC (permalink / raw)
To: Wenhua Lin
Cc: Linus Walleij, Bartosz Golaszewski, Orson Zhai, Baolin Wang,
Chunyan Zhang, linux-gpio, linux-kernel, wenhua lin, Xiongpeng Wu
On Thu, Sep 21, 2023 at 05:00:24PM +0800, Wenhua Lin wrote:
> In the sleep state, Eic debounce has no clock and the clk of
Eic --> The eic
clk --> clock
> debounce needs to be forced open, so that eic can wake up normally.
> Fixes: 2788938b7946 ("gpio: eic-sprd: Make the irqchip immutable")
>
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
The tag block mustn't have blank lines.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V2 2/4] gpio: sprd: Clear interrupt after set the interrupt type
2023-09-21 9:00 ` [PATCH V2 2/4] gpio: sprd: Clear interrupt after set the interrupt type Wenhua Lin
@ 2023-09-21 11:24 ` Andy Shevchenko
2024-01-03 12:06 ` wenhua lin
2023-09-27 9:15 ` Baolin Wang
1 sibling, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2023-09-21 11:24 UTC (permalink / raw)
To: Wenhua Lin
Cc: Linus Walleij, Bartosz Golaszewski, Orson Zhai, Baolin Wang,
Chunyan Zhang, linux-gpio, linux-kernel, wenhua lin, Xiongpeng Wu
On Thu, Sep 21, 2023 at 05:00:25PM +0800, Wenhua Lin wrote:
> The initialization state of the EIC module is a high level trigger.
Here you use EIC, in other places eic. Can you, please, be consistent?
> If it is currently a high level, the interrupt condition is met at
> this time, and the EIC interrupt has a latch capability, which will
> cause an interrupt to occur after booting. To avoid this, When setting
> the EIC interrupt trigger type, clear the interrupt once.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V2 3/4] gpio: sprd: Modify the calculation method of eic number
2023-09-21 9:00 ` [PATCH V2 3/4] gpio: sprd: Modify the calculation method of eic number Wenhua Lin
@ 2023-09-21 11:27 ` Andy Shevchenko
2024-01-04 2:31 ` wenhua lin
2023-09-27 9:24 ` Baolin Wang
1 sibling, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2023-09-21 11:27 UTC (permalink / raw)
To: Wenhua Lin
Cc: Linus Walleij, Bartosz Golaszewski, Orson Zhai, Baolin Wang,
Chunyan Zhang, linux-gpio, linux-kernel, wenhua lin, Xiongpeng Wu
On Thu, Sep 21, 2023 at 05:00:26PM +0800, Wenhua Lin wrote:
> When the soc changes, the corresponding gpio-eic-sprd.c
> code needs to be modified, and the corresponding
> Document must also be modified, which is quite troublesome.
> To avoid modifying the driver file, the number of EICs
> is automatically calculated by matching dts nodes.
> Fixes: 2788938b7946 ("gpio: eic-sprd: Make the irqchip immutable")
>
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
Same comment as per patch 1. Also, fixes needs to be grouped at the beginning
of the series. (I don't remember seeing Fixes tag in the patch 2.)
...
> +#define SPRD_EIC_VAR_DATA(soc_name) \
Misindented \
> +static const struct sprd_eic_variant_data soc_name##_eic_dbnc_data = { \
> + .type = SPRD_EIC_DEBOUNCE, \
> +}; \
...
> struct sprd_eic *sprd_eic;
> struct resource *res;
> int ret, i;
> + u16 num_banks = 0;
Preserve reversed xmas tree order.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V2 1/4] gpio: sprd: In the sleep state, the eic debounce clk must be forced open
2023-09-21 9:00 ` [PATCH V2 1/4] gpio: sprd: In the sleep state, the eic debounce clk must be forced open Wenhua Lin
2023-09-21 11:23 ` Andy Shevchenko
@ 2023-09-27 9:08 ` Baolin Wang
2024-01-02 8:42 ` wenhua lin
1 sibling, 1 reply; 18+ messages in thread
From: Baolin Wang @ 2023-09-27 9:08 UTC (permalink / raw)
To: Wenhua Lin, Linus Walleij, Andy Shevchenko, Bartosz Golaszewski
Cc: Orson Zhai, Chunyan Zhang, linux-gpio, linux-kernel, wenhua lin,
Xiongpeng Wu
On 9/21/2023 5:00 PM, Wenhua Lin wrote:
> In the sleep state, Eic debounce has no clock and the clk of
> debounce needs to be forced open, so that eic can wake up normally.
>
> Fixes: 2788938b7946 ("gpio: eic-sprd: Make the irqchip immutable")
Are you sure this is the right Fixes tag? This commit did not change EIC
debounce logics.
The changes look good to me.
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> ---
> drivers/gpio/gpio-eic-sprd.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
> index 84352a6f4973..bfa8a4c7515a 100644
> --- a/drivers/gpio/gpio-eic-sprd.c
> +++ b/drivers/gpio/gpio-eic-sprd.c
> @@ -23,6 +23,7 @@
> #define SPRD_EIC_DBNC_IC 0x24
> #define SPRD_EIC_DBNC_TRIG 0x28
> #define SPRD_EIC_DBNC_CTRL0 0x40
> +#define SPRD_EIC_DBNC_FORCE_CLK 0x8000
>
> #define SPRD_EIC_LATCH_INTEN 0x0
> #define SPRD_EIC_LATCH_INTRAW 0x4
> @@ -214,6 +215,7 @@ static int sprd_eic_set_debounce(struct gpio_chip *chip, unsigned int offset,
> u32 value = readl_relaxed(base + reg) & ~SPRD_EIC_DBNC_MASK;
>
> value |= (debounce / 1000) & SPRD_EIC_DBNC_MASK;
> + value |= SPRD_EIC_DBNC_FORCE_CLK;
> writel_relaxed(value, base + reg);
>
> return 0;
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V2 2/4] gpio: sprd: Clear interrupt after set the interrupt type
2023-09-21 9:00 ` [PATCH V2 2/4] gpio: sprd: Clear interrupt after set the interrupt type Wenhua Lin
2023-09-21 11:24 ` Andy Shevchenko
@ 2023-09-27 9:15 ` Baolin Wang
1 sibling, 0 replies; 18+ messages in thread
From: Baolin Wang @ 2023-09-27 9:15 UTC (permalink / raw)
To: Wenhua Lin, Linus Walleij, Andy Shevchenko, Bartosz Golaszewski
Cc: Orson Zhai, Chunyan Zhang, linux-gpio, linux-kernel, wenhua lin,
Xiongpeng Wu
On 9/21/2023 5:00 PM, Wenhua Lin wrote:
> The initialization state of the EIC module is a high level trigger.
> If it is currently a high level, the interrupt condition is met at
> this time, and the EIC interrupt has a latch capability, which will
> cause an interrupt to occur after booting. To avoid this, When setting
> the EIC interrupt trigger type, clear the interrupt once.
With Andy's comments,
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> ---
> drivers/gpio/gpio-eic-sprd.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
> index bfa8a4c7515a..96f1c7fd3988 100644
> --- a/drivers/gpio/gpio-eic-sprd.c
> +++ b/drivers/gpio/gpio-eic-sprd.c
> @@ -375,29 +375,34 @@ static int sprd_eic_irq_set_type(struct irq_data *data, unsigned int flow_type)
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTPOL, 1);
> + sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTCLR, 1);
> irq_set_handler_locked(data, handle_edge_irq);
> break;
> case IRQ_TYPE_EDGE_FALLING:
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTPOL, 0);
> + sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTCLR, 1);
> irq_set_handler_locked(data, handle_edge_irq);
> break;
> case IRQ_TYPE_EDGE_BOTH:
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 1);
> + sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTCLR, 1);
> irq_set_handler_locked(data, handle_edge_irq);
> break;
> case IRQ_TYPE_LEVEL_HIGH:
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 1);
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTPOL, 1);
> + sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTCLR, 1);
> irq_set_handler_locked(data, handle_level_irq);
> break;
> case IRQ_TYPE_LEVEL_LOW:
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 1);
> sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTPOL, 0);
> + sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTCLR, 1);
> irq_set_handler_locked(data, handle_level_irq);
> break;
> default:
> @@ -410,29 +415,34 @@ static int sprd_eic_irq_set_type(struct irq_data *data, unsigned int flow_type)
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTPOL, 1);
> + sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTCLR, 1);
> irq_set_handler_locked(data, handle_edge_irq);
> break;
> case IRQ_TYPE_EDGE_FALLING:
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTPOL, 0);
> + sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTCLR, 1);
> irq_set_handler_locked(data, handle_edge_irq);
> break;
> case IRQ_TYPE_EDGE_BOTH:
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 1);
> + sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTCLR, 1);
> irq_set_handler_locked(data, handle_edge_irq);
> break;
> case IRQ_TYPE_LEVEL_HIGH:
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 1);
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTPOL, 1);
> + sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTCLR, 1);
> irq_set_handler_locked(data, handle_level_irq);
> break;
> case IRQ_TYPE_LEVEL_LOW:
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 0);
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 1);
> sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTPOL, 0);
> + sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTCLR, 1);
> irq_set_handler_locked(data, handle_level_irq);
> break;
> default:
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V2 3/4] gpio: sprd: Modify the calculation method of eic number
2023-09-21 9:00 ` [PATCH V2 3/4] gpio: sprd: Modify the calculation method of eic number Wenhua Lin
2023-09-21 11:27 ` Andy Shevchenko
@ 2023-09-27 9:24 ` Baolin Wang
2024-01-03 12:04 ` wenhua lin
1 sibling, 1 reply; 18+ messages in thread
From: Baolin Wang @ 2023-09-27 9:24 UTC (permalink / raw)
To: Wenhua Lin, Linus Walleij, Andy Shevchenko, Bartosz Golaszewski
Cc: Orson Zhai, Chunyan Zhang, linux-gpio, linux-kernel, wenhua lin,
Xiongpeng Wu
On 9/21/2023 5:00 PM, Wenhua Lin wrote:
> When the soc changes, the corresponding gpio-eic-sprd.c
> code needs to be modified, and the corresponding
> Document must also be modified, which is quite troublesome.
> To avoid modifying the driver file, the number of EICs
> is automatically calculated by matching dts nodes.
>
> Fixes: 2788938b7946 ("gpio: eic-sprd: Make the irqchip immutable")
This is not a bugfix and you still use an incorrect Fixes tag.
>
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> ---
> drivers/gpio/gpio-eic-sprd.c | 43 ++++++++++++++++++------------------
> 1 file changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
> index 96f1c7fd3988..e85addbdf8aa 100644
> --- a/drivers/gpio/gpio-eic-sprd.c
> +++ b/drivers/gpio/gpio-eic-sprd.c
> @@ -100,33 +100,32 @@ struct sprd_eic {
>
> struct sprd_eic_variant_data {
> enum sprd_eic_type type;
> - u32 num_eics;
> };
>
> +#define SPRD_EIC_VAR_DATA(soc_name) \
> +static const struct sprd_eic_variant_data soc_name##_eic_dbnc_data = { \
> + .type = SPRD_EIC_DEBOUNCE, \
> +}; \
> + \
> +static const struct sprd_eic_variant_data soc_name##_eic_latch_data = { \
> + .type = SPRD_EIC_LATCH, \
> +}; \
> + \
> +static const struct sprd_eic_variant_data soc_name##_eic_async_data = { \
> + .type = SPRD_EIC_ASYNC, \
> +}; \
> + \
> +static const struct sprd_eic_variant_data soc_name##_eic_sync_data = { \
> + .type = SPRD_EIC_SYNC, \
> +}
> +
> +SPRD_EIC_VAR_DATA(sc9860);
> +
> static const char *sprd_eic_label_name[SPRD_EIC_MAX] = {
> "eic-debounce", "eic-latch", "eic-async",
> "eic-sync",
> };
>
> -static const struct sprd_eic_variant_data sc9860_eic_dbnc_data = {
> - .type = SPRD_EIC_DEBOUNCE,
> - .num_eics = 8,
> -};
> -
> -static const struct sprd_eic_variant_data sc9860_eic_latch_data = {
> - .type = SPRD_EIC_LATCH,
> - .num_eics = 8,
> -};
> -
> -static const struct sprd_eic_variant_data sc9860_eic_async_data = {
> - .type = SPRD_EIC_ASYNC,
> - .num_eics = 8,
> -};
> -
> -static const struct sprd_eic_variant_data sc9860_eic_sync_data = {
> - .type = SPRD_EIC_SYNC,
> - .num_eics = 8,
> -};
>
> static inline void __iomem *sprd_eic_offset_base(struct sprd_eic *sprd_eic,
> unsigned int bank)
> @@ -595,6 +594,7 @@ static int sprd_eic_probe(struct platform_device *pdev)
> struct sprd_eic *sprd_eic;
> struct resource *res;
> int ret, i;
> + u16 num_banks = 0;
>
> pdata = of_device_get_match_data(&pdev->dev);
> if (!pdata) {
> @@ -625,12 +625,13 @@ static int sprd_eic_probe(struct platform_device *pdev)
> break;
>
> sprd_eic->base[i] = devm_ioremap_resource(&pdev->dev, res);
> + num_banks++;
Please move this after the validation of the sprd_eic->base.
> if (IS_ERR(sprd_eic->base[i]))
> return PTR_ERR(sprd_eic->base[i]);
> }
>
> sprd_eic->chip.label = sprd_eic_label_name[sprd_eic->type];
> - sprd_eic->chip.ngpio = pdata->num_eics;
> + sprd_eic->chip.ngpio = num_banks * SPRD_EIC_PER_BANK_NR;
> sprd_eic->chip.base = -1;
> sprd_eic->chip.parent = &pdev->dev;
> sprd_eic->chip.direction_input = sprd_eic_direction_input;
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V2 4/4] gpio: sprd: Support 8 banks EIC controller
2023-09-21 9:00 ` [PATCH V2 4/4] gpio: sprd: Support 8 banks EIC controller Wenhua Lin
@ 2023-09-27 9:28 ` Baolin Wang
2024-01-03 12:03 ` wenhua lin
0 siblings, 1 reply; 18+ messages in thread
From: Baolin Wang @ 2023-09-27 9:28 UTC (permalink / raw)
To: Wenhua Lin, Linus Walleij, Andy Shevchenko, Bartosz Golaszewski
Cc: Orson Zhai, Chunyan Zhang, linux-gpio, linux-kernel, wenhua lin,
Xiongpeng Wu
On 9/21/2023 5:00 PM, Wenhua Lin wrote:
> In order to solve the problem of insufficient eic,
> it supports 8 banks of eic controller, each bank contains 8 eic.
>
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> ---
> drivers/gpio/gpio-eic-sprd.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
> index e85addbdf8aa..6bb002060c3e 100644
> --- a/drivers/gpio/gpio-eic-sprd.c
> +++ b/drivers/gpio/gpio-eic-sprd.c
> @@ -51,10 +51,10 @@
> #define SPRD_EIC_SYNC_DATA 0x1c
>
> /*
> - * The digital-chip EIC controller can support maximum 3 banks, and each bank
> + * The digital-chip EIC controller can support maximum 8 banks, and each bank
> * contains 8 EICs.
> */
> -#define SPRD_EIC_MAX_BANK 3
> +#define SPRD_EIC_MAX_BANK 8
> #define SPRD_EIC_PER_BANK_NR 8
> #define SPRD_EIC_DATA_MASK GENMASK(7, 0)
> #define SPRD_EIC_BIT(x) ((x) & (SPRD_EIC_PER_BANK_NR - 1))
> @@ -615,9 +615,9 @@ static int sprd_eic_probe(struct platform_device *pdev)
>
> for (i = 0; i < SPRD_EIC_MAX_BANK; i++) {
> /*
> - * We can have maximum 3 banks EICs, and each EIC has
> + * We can have maximum 8 banks EICs, and each EIC has
> * its own base address. But some platform maybe only
> - * have one bank EIC, thus base[1] and base[2] can be
> + * have one bank EIC, thus base[1] and base[7] can be
Should be "base[1] to base[7]"
> * optional.
> */
> res = platform_get_resource(pdev, IORESOURCE_MEM, i);
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V2 1/4] gpio: sprd: In the sleep state, the eic debounce clk must be forced open
2023-09-21 11:23 ` Andy Shevchenko
@ 2024-01-02 8:34 ` wenhua lin
0 siblings, 0 replies; 18+ messages in thread
From: wenhua lin @ 2024-01-02 8:34 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wenhua Lin, Linus Walleij, Bartosz Golaszewski, Orson Zhai,
Baolin Wang, Chunyan Zhang, linux-gpio, linux-kernel,
Xiongpeng Wu
On Thu, Sep 21, 2023 at 7:25 PM Andy Shevchenko <andy@kernel.org> wrote:
>
> On Thu, Sep 21, 2023 at 05:00:24PM +0800, Wenhua Lin wrote:
> > In the sleep state, Eic debounce has no clock and the clk of
>
> Eic --> The eic
>
> clk --> clock
>
> > debounce needs to be forced open, so that eic can wake up normally.
>
> > Fixes: 2788938b7946 ("gpio: eic-sprd: Make the irqchip immutable")
> >
> > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
>
> The tag block mustn't have blank lines.
>
Thank you very much for your review.
I will fix this issue in patch v3.
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V2 1/4] gpio: sprd: In the sleep state, the eic debounce clk must be forced open
2023-09-27 9:08 ` Baolin Wang
@ 2024-01-02 8:42 ` wenhua lin
0 siblings, 0 replies; 18+ messages in thread
From: wenhua lin @ 2024-01-02 8:42 UTC (permalink / raw)
To: Baolin Wang
Cc: Wenhua Lin, Linus Walleij, Andy Shevchenko, Bartosz Golaszewski,
Orson Zhai, Chunyan Zhang, linux-gpio, linux-kernel, Xiongpeng Wu
On Wed, Sep 27, 2023 at 5:08 PM Baolin Wang
<baolin.wang@linux.alibaba.com> wrote:
>
>
>
> On 9/21/2023 5:00 PM, Wenhua Lin wrote:
> > In the sleep state, Eic debounce has no clock and the clk of
> > debounce needs to be forced open, so that eic can wake up normally.
> >
> > Fixes: 2788938b7946 ("gpio: eic-sprd: Make the irqchip immutable")
>
> Are you sure this is the right Fixes tag? This commit did not change EIC
> debounce logics.
>
> The changes look good to me.
This modification turns on the debounce clock, and we will modify the
submission
description on patch v3. Thank you for your review.
>
> > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> > ---
> > drivers/gpio/gpio-eic-sprd.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
> > index 84352a6f4973..bfa8a4c7515a 100644
> > --- a/drivers/gpio/gpio-eic-sprd.c
> > +++ b/drivers/gpio/gpio-eic-sprd.c
> > @@ -23,6 +23,7 @@
> > #define SPRD_EIC_DBNC_IC 0x24
> > #define SPRD_EIC_DBNC_TRIG 0x28
> > #define SPRD_EIC_DBNC_CTRL0 0x40
> > +#define SPRD_EIC_DBNC_FORCE_CLK 0x8000
> >
> > #define SPRD_EIC_LATCH_INTEN 0x0
> > #define SPRD_EIC_LATCH_INTRAW 0x4
> > @@ -214,6 +215,7 @@ static int sprd_eic_set_debounce(struct gpio_chip *chip, unsigned int offset,
> > u32 value = readl_relaxed(base + reg) & ~SPRD_EIC_DBNC_MASK;
> >
> > value |= (debounce / 1000) & SPRD_EIC_DBNC_MASK;
> > + value |= SPRD_EIC_DBNC_FORCE_CLK;
> > writel_relaxed(value, base + reg);
> >
> > return 0;
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V2 4/4] gpio: sprd: Support 8 banks EIC controller
2023-09-27 9:28 ` Baolin Wang
@ 2024-01-03 12:03 ` wenhua lin
0 siblings, 0 replies; 18+ messages in thread
From: wenhua lin @ 2024-01-03 12:03 UTC (permalink / raw)
To: Baolin Wang
Cc: Wenhua Lin, Linus Walleij, Andy Shevchenko, Bartosz Golaszewski,
Orson Zhai, Chunyan Zhang, linux-gpio, linux-kernel, Xiongpeng Wu
On Wed, Sep 27, 2023 at 5:28 PM Baolin Wang
<baolin.wang@linux.alibaba.com> wrote:
>
>
>
> On 9/21/2023 5:00 PM, Wenhua Lin wrote:
> > In order to solve the problem of insufficient eic,
> > it supports 8 banks of eic controller, each bank contains 8 eic.
> >
> > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> > ---
> > drivers/gpio/gpio-eic-sprd.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
> > index e85addbdf8aa..6bb002060c3e 100644
> > --- a/drivers/gpio/gpio-eic-sprd.c
> > +++ b/drivers/gpio/gpio-eic-sprd.c
> > @@ -51,10 +51,10 @@
> > #define SPRD_EIC_SYNC_DATA 0x1c
> >
> > /*
> > - * The digital-chip EIC controller can support maximum 3 banks, and each bank
> > + * The digital-chip EIC controller can support maximum 8 banks, and each bank
> > * contains 8 EICs.
> > */
> > -#define SPRD_EIC_MAX_BANK 3
> > +#define SPRD_EIC_MAX_BANK 8
> > #define SPRD_EIC_PER_BANK_NR 8
> > #define SPRD_EIC_DATA_MASK GENMASK(7, 0)
> > #define SPRD_EIC_BIT(x) ((x) & (SPRD_EIC_PER_BANK_NR - 1))
> > @@ -615,9 +615,9 @@ static int sprd_eic_probe(struct platform_device *pdev)
> >
> > for (i = 0; i < SPRD_EIC_MAX_BANK; i++) {
> > /*
> > - * We can have maximum 3 banks EICs, and each EIC has
> > + * We can have maximum 8 banks EICs, and each EIC has
> > * its own base address. But some platform maybe only
> > - * have one bank EIC, thus base[1] and base[2] can be
> > + * have one bank EIC, thus base[1] and base[7] can be
>
> Should be "base[1] to base[7]"
Thank you very much for your review.
I will fix this issue in patch v3.
>
> > * optional.
> > */
> > res = platform_get_resource(pdev, IORESOURCE_MEM, i);
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V2 3/4] gpio: sprd: Modify the calculation method of eic number
2023-09-27 9:24 ` Baolin Wang
@ 2024-01-03 12:04 ` wenhua lin
0 siblings, 0 replies; 18+ messages in thread
From: wenhua lin @ 2024-01-03 12:04 UTC (permalink / raw)
To: Baolin Wang
Cc: Wenhua Lin, Linus Walleij, Andy Shevchenko, Bartosz Golaszewski,
Orson Zhai, Chunyan Zhang, linux-gpio, linux-kernel, Xiongpeng Wu
On Wed, Sep 27, 2023 at 5:24 PM Baolin Wang
<baolin.wang@linux.alibaba.com> wrote:
>
>
>
> On 9/21/2023 5:00 PM, Wenhua Lin wrote:
> > When the soc changes, the corresponding gpio-eic-sprd.c
> > code needs to be modified, and the corresponding
> > Document must also be modified, which is quite troublesome.
> > To avoid modifying the driver file, the number of EICs
> > is automatically calculated by matching dts nodes.
> >
> > Fixes: 2788938b7946 ("gpio: eic-sprd: Make the irqchip immutable")
>
> This is not a bugfix and you still use an incorrect Fixes tag.
>
> >
> > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> > ---
> > drivers/gpio/gpio-eic-sprd.c | 43 ++++++++++++++++++------------------
> > 1 file changed, 22 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
> > index 96f1c7fd3988..e85addbdf8aa 100644
> > --- a/drivers/gpio/gpio-eic-sprd.c
> > +++ b/drivers/gpio/gpio-eic-sprd.c
> > @@ -100,33 +100,32 @@ struct sprd_eic {
> >
> > struct sprd_eic_variant_data {
> > enum sprd_eic_type type;
> > - u32 num_eics;
> > };
> >
> > +#define SPRD_EIC_VAR_DATA(soc_name) \
> > +static const struct sprd_eic_variant_data soc_name##_eic_dbnc_data = { \
> > + .type = SPRD_EIC_DEBOUNCE, \
> > +}; \
> > + \
> > +static const struct sprd_eic_variant_data soc_name##_eic_latch_data = { \
> > + .type = SPRD_EIC_LATCH, \
> > +}; \
> > + \
> > +static const struct sprd_eic_variant_data soc_name##_eic_async_data = { \
> > + .type = SPRD_EIC_ASYNC, \
> > +}; \
> > + \
> > +static const struct sprd_eic_variant_data soc_name##_eic_sync_data = { \
> > + .type = SPRD_EIC_SYNC, \
> > +}
> > +
> > +SPRD_EIC_VAR_DATA(sc9860);
> > +
> > static const char *sprd_eic_label_name[SPRD_EIC_MAX] = {
> > "eic-debounce", "eic-latch", "eic-async",
> > "eic-sync",
> > };
> >
> > -static const struct sprd_eic_variant_data sc9860_eic_dbnc_data = {
> > - .type = SPRD_EIC_DEBOUNCE,
> > - .num_eics = 8,
> > -};
> > -
> > -static const struct sprd_eic_variant_data sc9860_eic_latch_data = {
> > - .type = SPRD_EIC_LATCH,
> > - .num_eics = 8,
> > -};
> > -
> > -static const struct sprd_eic_variant_data sc9860_eic_async_data = {
> > - .type = SPRD_EIC_ASYNC,
> > - .num_eics = 8,
> > -};
> > -
> > -static const struct sprd_eic_variant_data sc9860_eic_sync_data = {
> > - .type = SPRD_EIC_SYNC,
> > - .num_eics = 8,
> > -};
> >
> > static inline void __iomem *sprd_eic_offset_base(struct sprd_eic *sprd_eic,
> > unsigned int bank)
> > @@ -595,6 +594,7 @@ static int sprd_eic_probe(struct platform_device *pdev)
> > struct sprd_eic *sprd_eic;
> > struct resource *res;
> > int ret, i;
> > + u16 num_banks = 0;
> >
> > pdata = of_device_get_match_data(&pdev->dev);
> > if (!pdata) {
> > @@ -625,12 +625,13 @@ static int sprd_eic_probe(struct platform_device *pdev)
> > break;
> >
> > sprd_eic->base[i] = devm_ioremap_resource(&pdev->dev, res);
> > + num_banks++;
>
> Please move this after the validation of the sprd_eic->base.
Thank you very much for your review.
I will fix this issue in patch v3.
>
> > if (IS_ERR(sprd_eic->base[i]))
> > return PTR_ERR(sprd_eic->base[i]);
> > }
> >
> > sprd_eic->chip.label = sprd_eic_label_name[sprd_eic->type];
> > - sprd_eic->chip.ngpio = pdata->num_eics;
> > + sprd_eic->chip.ngpio = num_banks * SPRD_EIC_PER_BANK_NR;
> > sprd_eic->chip.base = -1;
> > sprd_eic->chip.parent = &pdev->dev;
> > sprd_eic->chip.direction_input = sprd_eic_direction_input;
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V2 2/4] gpio: sprd: Clear interrupt after set the interrupt type
2023-09-21 11:24 ` Andy Shevchenko
@ 2024-01-03 12:06 ` wenhua lin
0 siblings, 0 replies; 18+ messages in thread
From: wenhua lin @ 2024-01-03 12:06 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wenhua Lin, Linus Walleij, Bartosz Golaszewski, Orson Zhai,
Baolin Wang, Chunyan Zhang, linux-gpio, linux-kernel,
Xiongpeng Wu
On Thu, Sep 21, 2023 at 7:24 PM Andy Shevchenko <andy@kernel.org> wrote:
>
> On Thu, Sep 21, 2023 at 05:00:25PM +0800, Wenhua Lin wrote:
> > The initialization state of the EIC module is a high level trigger.
>
> Here you use EIC, in other places eic. Can you, please, be consistent?
Thank you very much for your review.
I will fix this issue in patch v3.
>
> > If it is currently a high level, the interrupt condition is met at
> > this time, and the EIC interrupt has a latch capability, which will
> > cause an interrupt to occur after booting. To avoid this, When setting
> > the EIC interrupt trigger type, clear the interrupt once.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V2 3/4] gpio: sprd: Modify the calculation method of eic number
2023-09-21 11:27 ` Andy Shevchenko
@ 2024-01-04 2:31 ` wenhua lin
0 siblings, 0 replies; 18+ messages in thread
From: wenhua lin @ 2024-01-04 2:31 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wenhua Lin, Linus Walleij, Bartosz Golaszewski, Orson Zhai,
Baolin Wang, Chunyan Zhang, linux-gpio, linux-kernel,
Xiongpeng Wu
On Thu, Sep 21, 2023 at 7:27 PM Andy Shevchenko <andy@kernel.org> wrote:
>
> On Thu, Sep 21, 2023 at 05:00:26PM +0800, Wenhua Lin wrote:
> > When the soc changes, the corresponding gpio-eic-sprd.c
> > code needs to be modified, and the corresponding
> > Document must also be modified, which is quite troublesome.
> > To avoid modifying the driver file, the number of EICs
> > is automatically calculated by matching dts nodes.
>
> > Fixes: 2788938b7946 ("gpio: eic-sprd: Make the irqchip immutable")
> >
> > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
>
> Same comment as per patch 1. Also, fixes needs to be grouped at the beginning
> of the series. (I don't remember seeing Fixes tag in the patch 2.)
>
> ...
>
> > +#define SPRD_EIC_VAR_DATA(soc_name) \
>
> Misindented \
Thank you very much for your review.
I will fix this issue in patch v3.
>
> > +static const struct sprd_eic_variant_data soc_name##_eic_dbnc_data = { \
> > + .type = SPRD_EIC_DEBOUNCE, \
> > +}; \
>
> ...
>
> > struct sprd_eic *sprd_eic;
> > struct resource *res;
>
> > int ret, i;
> > + u16 num_banks = 0;
>
> Preserve reversed xmas tree order.
Thank you very much for your review.
I will fix this issue in patch v3.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2024-01-04 2:31 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-21 9:00 [PATCH V2 0/4] gpio: sprd: Modification of UNISOC Platform EIC Driver Wenhua Lin
2023-09-21 9:00 ` [PATCH V2 1/4] gpio: sprd: In the sleep state, the eic debounce clk must be forced open Wenhua Lin
2023-09-21 11:23 ` Andy Shevchenko
2024-01-02 8:34 ` wenhua lin
2023-09-27 9:08 ` Baolin Wang
2024-01-02 8:42 ` wenhua lin
2023-09-21 9:00 ` [PATCH V2 2/4] gpio: sprd: Clear interrupt after set the interrupt type Wenhua Lin
2023-09-21 11:24 ` Andy Shevchenko
2024-01-03 12:06 ` wenhua lin
2023-09-27 9:15 ` Baolin Wang
2023-09-21 9:00 ` [PATCH V2 3/4] gpio: sprd: Modify the calculation method of eic number Wenhua Lin
2023-09-21 11:27 ` Andy Shevchenko
2024-01-04 2:31 ` wenhua lin
2023-09-27 9:24 ` Baolin Wang
2024-01-03 12:04 ` wenhua lin
2023-09-21 9:00 ` [PATCH V2 4/4] gpio: sprd: Support 8 banks EIC controller Wenhua Lin
2023-09-27 9:28 ` Baolin Wang
2024-01-03 12:03 ` wenhua lin
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).