| Message ID | 20201210212903.216728-9-krzk@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/9] ARM: dts: exynos: correct PMIC interrupt trigger level on Artik 5 | expand |
On 10.12.2020 22:29, Krzysztof Kozlowski wrote: > Interrupt line can be configured on different hardware in different way, > even inverted. Therefore driver should not enforce specific trigger > type - edge falling - but instead rely on Devicetree to configure it. > > The Samsung PMIC drivers are used only on Devicetree boards. > > Additionally, the PMIC datasheets describe the interrupt line as active > low with a requirement of acknowledge from the CPU therefore the edge > falling is not correct. > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> It looks that this together with DTS change fixes RTC alarm failure that I've observed from time to time on TM2e board! > --- > > This patch should wait till DTS changes are merged, as it relies on > proper Devicetree. > --- > drivers/mfd/sec-irq.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c > index a98c5d165039..760f88a865ab 100644 > --- a/drivers/mfd/sec-irq.c > +++ b/drivers/mfd/sec-irq.c > @@ -480,8 +480,7 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic) > } > > ret = devm_regmap_add_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic, > - sec_pmic->irq, > - IRQF_TRIGGER_FALLING | IRQF_ONESHOT, > + sec_pmic->irq, IRQF_ONESHOT, > sec_pmic->irq_base, sec_irq_chip, > &sec_pmic->irq_data); > if (ret != 0) { Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
On Fri, Dec 18, 2020 at 02:25:39PM +0100, Marek Szyprowski wrote: > On 10.12.2020 22:29, Krzysztof Kozlowski wrote: > > Interrupt line can be configured on different hardware in different way, > > even inverted. Therefore driver should not enforce specific trigger > > type - edge falling - but instead rely on Devicetree to configure it. > > > > The Samsung PMIC drivers are used only on Devicetree boards. > > > > Additionally, the PMIC datasheets describe the interrupt line as active > > low with a requirement of acknowledge from the CPU therefore the edge > > falling is not correct. > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > > It looks that this together with DTS change fixes RTC alarm failure that > I've observed from time to time on TM2e board! Great! I'll add this to the commit msg. Thanks for testing. Best regards, Krzysztof > > > --- > > > > This patch should wait till DTS changes are merged, as it relies on > > proper Devicetree. > > ---
Hi Krzysztof, On 18.12.2020 15:22, Krzysztof Kozlowski wrote: > On Fri, Dec 18, 2020 at 02:25:39PM +0100, Marek Szyprowski wrote: >> On 10.12.2020 22:29, Krzysztof Kozlowski wrote: >>> Interrupt line can be configured on different hardware in different way, >>> even inverted. Therefore driver should not enforce specific trigger >>> type - edge falling - but instead rely on Devicetree to configure it. >>> >>> The Samsung PMIC drivers are used only on Devicetree boards. >>> >>> Additionally, the PMIC datasheets describe the interrupt line as active >>> low with a requirement of acknowledge from the CPU therefore the edge >>> falling is not correct. >>> >>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> >> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> >> >> It looks that this together with DTS change fixes RTC alarm failure that >> I've observed from time to time on TM2e board! > Great! I'll add this to the commit msg. > > Thanks for testing. BTW, while playing with this, maybe it would make sense to fix the reported interrupt type for the PMIC sub-interrupts: # grep s2mps /proc/interrupts 120: 0 gpa0 7 Level s2mps13 121: 0 s2mps13 10 Edge rtc-alarm0 Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
On Mon, Dec 21, 2020 at 08:36:02AM +0100, Marek Szyprowski wrote: > Hi Krzysztof, > > On 18.12.2020 15:22, Krzysztof Kozlowski wrote: > > On Fri, Dec 18, 2020 at 02:25:39PM +0100, Marek Szyprowski wrote: > >> On 10.12.2020 22:29, Krzysztof Kozlowski wrote: > >>> Interrupt line can be configured on different hardware in different way, > >>> even inverted. Therefore driver should not enforce specific trigger > >>> type - edge falling - but instead rely on Devicetree to configure it. > >>> > >>> The Samsung PMIC drivers are used only on Devicetree boards. > >>> > >>> Additionally, the PMIC datasheets describe the interrupt line as active > >>> low with a requirement of acknowledge from the CPU therefore the edge > >>> falling is not correct. > >>> > >>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > >> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > >> > >> It looks that this together with DTS change fixes RTC alarm failure that > >> I've observed from time to time on TM2e board! > > Great! I'll add this to the commit msg. > > > > Thanks for testing. > > BTW, while playing with this, maybe it would make sense to fix the > reported interrupt type for the PMIC sub-interrupts: > > # grep s2mps /proc/interrupts > 120: 0 gpa0 7 Level s2mps13 > 121: 0 s2mps13 10 Edge rtc-alarm0 I also spotted this. It's a virtual interrupt and I am not sure whether we can actually configure it when the hardware does not allow to set the type (the regmap_irq_type requires register offsets). Best regards, Krzysztof
On 21.12.2020 08:55, Krzysztof Kozlowski wrote: > On Mon, Dec 21, 2020 at 08:36:02AM +0100, Marek Szyprowski wrote: >> On 18.12.2020 15:22, Krzysztof Kozlowski wrote: >>> On Fri, Dec 18, 2020 at 02:25:39PM +0100, Marek Szyprowski wrote: >>>> On 10.12.2020 22:29, Krzysztof Kozlowski wrote: >>>>> Interrupt line can be configured on different hardware in different way, >>>>> even inverted. Therefore driver should not enforce specific trigger >>>>> type - edge falling - but instead rely on Devicetree to configure it. >>>>> >>>>> The Samsung PMIC drivers are used only on Devicetree boards. >>>>> >>>>> Additionally, the PMIC datasheets describe the interrupt line as active >>>>> low with a requirement of acknowledge from the CPU therefore the edge >>>>> falling is not correct. >>>>> >>>>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> >>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>> >>>> It looks that this together with DTS change fixes RTC alarm failure that >>>> I've observed from time to time on TM2e board! >>> Great! I'll add this to the commit msg. >>> >>> Thanks for testing. >> BTW, while playing with this, maybe it would make sense to fix the >> reported interrupt type for the PMIC sub-interrupts: >> >> # grep s2mps /proc/interrupts >> 120: 0 gpa0 7 Level s2mps13 >> 121: 0 s2mps13 10 Edge rtc-alarm0 > I also spotted this. It's a virtual interrupt and I am not sure whether > we can actually configure it when the hardware does not allow to set the > type (the regmap_irq_type requires register offsets). I know that it is virtual, but maybe the regmap code could simply copy the interrupt type from its parent interrupt? Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c index a98c5d165039..760f88a865ab 100644 --- a/drivers/mfd/sec-irq.c +++ b/drivers/mfd/sec-irq.c @@ -480,8 +480,7 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic) } ret = devm_regmap_add_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic, - sec_pmic->irq, - IRQF_TRIGGER_FALLING | IRQF_ONESHOT, + sec_pmic->irq, IRQF_ONESHOT, sec_pmic->irq_base, sec_irq_chip, &sec_pmic->irq_data); if (ret != 0) {
Interrupt line can be configured on different hardware in different way, even inverted. Therefore driver should not enforce specific trigger type - edge falling - but instead rely on Devicetree to configure it. The Samsung PMIC drivers are used only on Devicetree boards. Additionally, the PMIC datasheets describe the interrupt line as active low with a requirement of acknowledge from the CPU therefore the edge falling is not correct. Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- This patch should wait till DTS changes are merged, as it relies on proper Devicetree. --- drivers/mfd/sec-irq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)