* [PATCH] gpio: sprd: Two-dimensional arrays maintain pmic eic
@ 2023-08-07 9:18 Wenhua Lin
2023-08-07 9:26 ` Baolin Wang
0 siblings, 1 reply; 3+ messages in thread
From: Wenhua Lin @ 2023-08-07 9:18 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko, Orson Zhai,
Baolin Wang, Chunyan Zhang
Cc: linux-serial, linux-kernel, wenhua lin, Wenhua Lin, Xiongpeng Wu
Maintain the registers of each pmic eic through a Two-dimensional
array to avoid mutual interference.
Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
---
drivers/gpio/gpio-pmic-eic-sprd.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/gpio/gpio-pmic-eic-sprd.c b/drivers/gpio/gpio-pmic-eic-sprd.c
index c3e4d90f6b18..8d67d130cbcf 100644
--- a/drivers/gpio/gpio-pmic-eic-sprd.c
+++ b/drivers/gpio/gpio-pmic-eic-sprd.c
@@ -57,7 +57,7 @@ struct sprd_pmic_eic {
struct gpio_chip chip;
struct regmap *map;
u32 offset;
- u8 reg[CACHE_NR_REGS];
+ u8 reg[SPRD_PMIC_EIC_NR][CACHE_NR_REGS];
struct mutex buslock;
int irq;
};
@@ -151,8 +151,8 @@ static void sprd_pmic_eic_irq_mask(struct irq_data *data)
struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
u32 offset = irqd_to_hwirq(data);
- pmic_eic->reg[REG_IE] = 0;
- pmic_eic->reg[REG_TRIG] = 0;
+ pmic_eic->reg[offset][REG_IE] = 0;
+ pmic_eic->reg[offset][REG_TRIG] = 0;
gpiochip_disable_irq(chip, offset);
}
@@ -165,8 +165,8 @@ static void sprd_pmic_eic_irq_unmask(struct irq_data *data)
gpiochip_enable_irq(chip, offset);
- pmic_eic->reg[REG_IE] = 1;
- pmic_eic->reg[REG_TRIG] = 1;
+ pmic_eic->reg[offset][REG_IE] = 1;
+ pmic_eic->reg[offset][REG_TRIG] = 1;
}
static int sprd_pmic_eic_irq_set_type(struct irq_data *data,
@@ -174,13 +174,14 @@ static int sprd_pmic_eic_irq_set_type(struct irq_data *data,
{
struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
+ u32 offset = irqd_to_hwirq(data);
switch (flow_type) {
case IRQ_TYPE_LEVEL_HIGH:
- pmic_eic->reg[REG_IEV] = 1;
+ pmic_eic->reg[offset][REG_IEV] = 1;
break;
case IRQ_TYPE_LEVEL_LOW:
- pmic_eic->reg[REG_IEV] = 0;
+ pmic_eic->reg[offset][REG_IEV] = 0;
break;
case IRQ_TYPE_EDGE_RISING:
case IRQ_TYPE_EDGE_FALLING:
@@ -222,15 +223,15 @@ static void sprd_pmic_eic_bus_sync_unlock(struct irq_data *data)
sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_IEV, 1);
} else {
sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_IEV,
- pmic_eic->reg[REG_IEV]);
+ pmic_eic->reg[offset][REG_IEV]);
}
/* Set irq unmask */
sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_IE,
- pmic_eic->reg[REG_IE]);
+ pmic_eic->reg[offset][REG_IE]);
/* Generate trigger start pulse for debounce EIC */
sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_TRIG,
- pmic_eic->reg[REG_TRIG]);
+ pmic_eic->reg[offset][REG_TRIG]);
mutex_unlock(&pmic_eic->buslock);
}
@@ -335,6 +336,7 @@ static int sprd_pmic_eic_probe(struct platform_device *pdev)
ret = devm_request_threaded_irq(&pdev->dev, pmic_eic->irq, NULL,
sprd_pmic_eic_irq_handler,
+ IRQF_TRIGGER_LOW |
IRQF_ONESHOT | IRQF_NO_SUSPEND,
dev_name(&pdev->dev), pmic_eic);
if (ret) {
@@ -352,6 +354,7 @@ static int sprd_pmic_eic_probe(struct platform_device *pdev)
pmic_eic->chip.set_config = sprd_pmic_eic_set_config;
pmic_eic->chip.set = sprd_pmic_eic_set;
pmic_eic->chip.get = sprd_pmic_eic_get;
+ pmic_eic->chip.can_sleep = true;
irq = &pmic_eic->chip.irq;
gpio_irq_chip_set_chip(irq, &pmic_eic_irq_chip);
--
2.17.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gpio: sprd: Two-dimensional arrays maintain pmic eic
2023-08-07 9:18 [PATCH] gpio: sprd: Two-dimensional arrays maintain pmic eic Wenhua Lin
@ 2023-08-07 9:26 ` Baolin Wang
2023-08-09 6:00 ` wenhua lin
0 siblings, 1 reply; 3+ messages in thread
From: Baolin Wang @ 2023-08-07 9:26 UTC (permalink / raw)
To: Wenhua Lin, Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
Orson Zhai, Chunyan Zhang
Cc: linux-serial, linux-kernel, wenhua lin, Xiongpeng Wu
On 8/7/2023 5:18 PM, Wenhua Lin wrote:
> Maintain the registers of each pmic eic through a Two-dimensional
> array to avoid mutual interference.
>
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
NAK. See below.
> ---
> drivers/gpio/gpio-pmic-eic-sprd.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpio/gpio-pmic-eic-sprd.c b/drivers/gpio/gpio-pmic-eic-sprd.c
> index c3e4d90f6b18..8d67d130cbcf 100644
> --- a/drivers/gpio/gpio-pmic-eic-sprd.c
> +++ b/drivers/gpio/gpio-pmic-eic-sprd.c
> @@ -57,7 +57,7 @@ struct sprd_pmic_eic {
> struct gpio_chip chip;
> struct regmap *map;
> u32 offset;
> - u8 reg[CACHE_NR_REGS];
> + u8 reg[SPRD_PMIC_EIC_NR][CACHE_NR_REGS];
We have recorded the offset, no need two-dimensional array unless you
have other strong reasons.
> struct mutex buslock;
> int irq;
> };
> @@ -151,8 +151,8 @@ static void sprd_pmic_eic_irq_mask(struct irq_data *data)
> struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
> u32 offset = irqd_to_hwirq(data);
>
> - pmic_eic->reg[REG_IE] = 0;
> - pmic_eic->reg[REG_TRIG] = 0;
> + pmic_eic->reg[offset][REG_IE] = 0;
> + pmic_eic->reg[offset][REG_TRIG] = 0;
>
> gpiochip_disable_irq(chip, offset);
> }
> @@ -165,8 +165,8 @@ static void sprd_pmic_eic_irq_unmask(struct irq_data *data)
>
> gpiochip_enable_irq(chip, offset);
>
> - pmic_eic->reg[REG_IE] = 1;
> - pmic_eic->reg[REG_TRIG] = 1;
> + pmic_eic->reg[offset][REG_IE] = 1;
> + pmic_eic->reg[offset][REG_TRIG] = 1;
> }
>
> static int sprd_pmic_eic_irq_set_type(struct irq_data *data,
> @@ -174,13 +174,14 @@ static int sprd_pmic_eic_irq_set_type(struct irq_data *data,
> {
> struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
> struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
> + u32 offset = irqd_to_hwirq(data);
>
> switch (flow_type) {
> case IRQ_TYPE_LEVEL_HIGH:
> - pmic_eic->reg[REG_IEV] = 1;
> + pmic_eic->reg[offset][REG_IEV] = 1;
> break;
> case IRQ_TYPE_LEVEL_LOW:
> - pmic_eic->reg[REG_IEV] = 0;
> + pmic_eic->reg[offset][REG_IEV] = 0;
> break;
> case IRQ_TYPE_EDGE_RISING:
> case IRQ_TYPE_EDGE_FALLING:
> @@ -222,15 +223,15 @@ static void sprd_pmic_eic_bus_sync_unlock(struct irq_data *data)
> sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_IEV, 1);
> } else {
> sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_IEV,
> - pmic_eic->reg[REG_IEV]);
> + pmic_eic->reg[offset][REG_IEV]);
> }
>
> /* Set irq unmask */
> sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_IE,
> - pmic_eic->reg[REG_IE]);
> + pmic_eic->reg[offset][REG_IE]);
> /* Generate trigger start pulse for debounce EIC */
> sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_TRIG,
> - pmic_eic->reg[REG_TRIG]);
> + pmic_eic->reg[offset][REG_TRIG]);
>
> mutex_unlock(&pmic_eic->buslock);
> }
> @@ -335,6 +336,7 @@ static int sprd_pmic_eic_probe(struct platform_device *pdev)
>
> ret = devm_request_threaded_irq(&pdev->dev, pmic_eic->irq, NULL,
> sprd_pmic_eic_irq_handler,
> + IRQF_TRIGGER_LOW |
Why? you did not explain this in the commit log.
> IRQF_ONESHOT | IRQF_NO_SUSPEND,
> dev_name(&pdev->dev), pmic_eic);
> if (ret) {
> @@ -352,6 +354,7 @@ static int sprd_pmic_eic_probe(struct platform_device *pdev)
> pmic_eic->chip.set_config = sprd_pmic_eic_set_config;
> pmic_eic->chip.set = sprd_pmic_eic_set;
> pmic_eic->chip.get = sprd_pmic_eic_get;
> + pmic_eic->chip.can_sleep = true;
Ditto. Why?
Please DO NOT squash different fixes into one patch.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gpio: sprd: Two-dimensional arrays maintain pmic eic
2023-08-07 9:26 ` Baolin Wang
@ 2023-08-09 6:00 ` wenhua lin
0 siblings, 0 replies; 3+ messages in thread
From: wenhua lin @ 2023-08-09 6:00 UTC (permalink / raw)
To: Baolin Wang
Cc: Wenhua Lin, Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
Orson Zhai, Chunyan Zhang, linux-serial, linux-kernel,
Xiongpeng Wu
Hi baolin:
1.We have recorded the offset, no need two-dimensional array unless you
have other strong reasons.
One-dimensional array reg[CACHE_NR_REGS] , the array to cache the EIC
registers., pmic eic has different channels, if the pmic eic does not
increase the offset two-dimensional array to maintain separately, it
will cause one of the eic channels to close the interrupt enable, and
it will be synchronized Disable other eic channel interrupt enable.
2.Why? you did not explain this in the commit log.
We will re-split the patch submission and explain our reasons for
modification in the submission information, thank you very much for
your review
3.Ditto. Why?
> + pmic_eic->chip.can_sleep = true;
We will re-split the patch submission, thank you very much for your review
Baolin Wang <baolin.wang@linux.alibaba.com> 于2023年8月7日周一 17:27写道：
>
>
>
> On 8/7/2023 5:18 PM, Wenhua Lin wrote:
> > Maintain the registers of each pmic eic through a Two-dimensional
> > array to avoid mutual interference.
> >
> > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
>
> NAK. See below.
>
> > ---
> > drivers/gpio/gpio-pmic-eic-sprd.c | 23 +++++++++++++----------
> > 1 file changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-pmic-eic-sprd.c b/drivers/gpio/gpio-pmic-eic-sprd.c
> > index c3e4d90f6b18..8d67d130cbcf 100644
> > --- a/drivers/gpio/gpio-pmic-eic-sprd.c
> > +++ b/drivers/gpio/gpio-pmic-eic-sprd.c
> > @@ -57,7 +57,7 @@ struct sprd_pmic_eic {
> > struct gpio_chip chip;
> > struct regmap *map;
> > u32 offset;
> > - u8 reg[CACHE_NR_REGS];
> > + u8 reg[SPRD_PMIC_EIC_NR][CACHE_NR_REGS];
>
> We have recorded the offset, no need two-dimensional array unless you
> have other strong reasons.
>
> > struct mutex buslock;
> > int irq;
> > };
> > @@ -151,8 +151,8 @@ static void sprd_pmic_eic_irq_mask(struct irq_data *data)
> > struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
> > u32 offset = irqd_to_hwirq(data);
> >
> > - pmic_eic->reg[REG_IE] = 0;
> > - pmic_eic->reg[REG_TRIG] = 0;
> > + pmic_eic->reg[offset][REG_IE] = 0;
> > + pmic_eic->reg[offset][REG_TRIG] = 0;
> >
> > gpiochip_disable_irq(chip, offset);
> > }
> > @@ -165,8 +165,8 @@ static void sprd_pmic_eic_irq_unmask(struct irq_data *data)
> >
> > gpiochip_enable_irq(chip, offset);
> >
> > - pmic_eic->reg[REG_IE] = 1;
> > - pmic_eic->reg[REG_TRIG] = 1;
> > + pmic_eic->reg[offset][REG_IE] = 1;
> > + pmic_eic->reg[offset][REG_TRIG] = 1;
> > }
> >
> > static int sprd_pmic_eic_irq_set_type(struct irq_data *data,
> > @@ -174,13 +174,14 @@ static int sprd_pmic_eic_irq_set_type(struct irq_data *data,
> > {
> > struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
> > struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
> > + u32 offset = irqd_to_hwirq(data);
> >
> > switch (flow_type) {
> > case IRQ_TYPE_LEVEL_HIGH:
> > - pmic_eic->reg[REG_IEV] = 1;
> > + pmic_eic->reg[offset][REG_IEV] = 1;
> > break;
> > case IRQ_TYPE_LEVEL_LOW:
> > - pmic_eic->reg[REG_IEV] = 0;
> > + pmic_eic->reg[offset][REG_IEV] = 0;
> > break;
> > case IRQ_TYPE_EDGE_RISING:
> > case IRQ_TYPE_EDGE_FALLING:
> > @@ -222,15 +223,15 @@ static void sprd_pmic_eic_bus_sync_unlock(struct irq_data *data)
> > sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_IEV, 1);
> > } else {
> > sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_IEV,
> > - pmic_eic->reg[REG_IEV]);
> > + pmic_eic->reg[offset][REG_IEV]);
> > }
> >
> > /* Set irq unmask */
> > sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_IE,
> > - pmic_eic->reg[REG_IE]);
> > + pmic_eic->reg[offset][REG_IE]);
> > /* Generate trigger start pulse for debounce EIC */
> > sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_TRIG,
> > - pmic_eic->reg[REG_TRIG]);
> > + pmic_eic->reg[offset][REG_TRIG]);
> >
> > mutex_unlock(&pmic_eic->buslock);
> > }
> > @@ -335,6 +336,7 @@ static int sprd_pmic_eic_probe(struct platform_device *pdev)
> >
> > ret = devm_request_threaded_irq(&pdev->dev, pmic_eic->irq, NULL,
> > sprd_pmic_eic_irq_handler,
> > + IRQF_TRIGGER_LOW |
>
> Why? you did not explain this in the commit log.
>
> > IRQF_ONESHOT | IRQF_NO_SUSPEND,
> > dev_name(&pdev->dev), pmic_eic);
> > if (ret) {
> > @@ -352,6 +354,7 @@ static int sprd_pmic_eic_probe(struct platform_device *pdev)
> > pmic_eic->chip.set_config = sprd_pmic_eic_set_config;
> > pmic_eic->chip.set = sprd_pmic_eic_set;
> > pmic_eic->chip.get = sprd_pmic_eic_get;
> > + pmic_eic->chip.can_sleep = true;
>
> Ditto. Why?
>
> Please DO NOT squash different fixes into one patch.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-08-09 6:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-07 9:18 [PATCH] gpio: sprd: Two-dimensional arrays maintain pmic eic Wenhua Lin
2023-08-07 9:26 ` Baolin Wang
2023-08-09 6:00 ` wenhua lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®