2015-04-02 14:40:44

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 0/2] Arndale Octa: Fix S2MPS11 RTC alarm IRQ

Hi,

After long struggle this finally fixes the S2MPS11 RTC alarm IRQ
on Arndale Octa board. It can easily be tested with:
rtcwake -d rtc0 -m on -s 5 -v

The patches are independent.

Best regards,
Krzysztof

Krzysztof Kozlowski (2):
mfd: sec: Fix RTC alarm interrupt number on S2MPS11
ARM: dts: Fix pinctrl settings for S2MPS11 RTC alarm IRQ on Arndale
Octa

arch/arm/boot/dts/exynos5420-arndale-octa.dts | 13 ++++++++++++-
drivers/mfd/sec-irq.c | 14 ++++++++++----
include/linux/mfd/samsung/irq.h | 2 +-
3 files changed, 23 insertions(+), 6 deletions(-)

--
1.9.1


2015-04-02 14:36:37

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 1/2] mfd: sec: Fix RTC alarm interrupt number on S2MPS11

The RTC on S2MPS11 is the same as S2MPS14. However interrupt numbers of
RTC alarms 0 and 1 were inversed between these two devices. So when
rtc-s5m driver requested S2MPS14_IRQ_RTCA0 interrupt, it matched to
S2MPS11_IRQ_RTCA1, not RTCA0.

Fix this by using consistent RTC alarm interrupt numbers and adding a
BUILD_BUG_ON for future generations.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/mfd/sec-irq.c | 14 ++++++++++----
include/linux/mfd/samsung/irq.h | 2 +-
2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
index ba86a918c2da..806fa8dbb22d 100644
--- a/drivers/mfd/sec-irq.c
+++ b/drivers/mfd/sec-irq.c
@@ -61,14 +61,14 @@ static const struct regmap_irq s2mps11_irqs[] = {
.reg_offset = 1,
.mask = S2MPS11_IRQ_RTC60S_MASK,
},
- [S2MPS11_IRQ_RTCA0] = {
- .reg_offset = 1,
- .mask = S2MPS11_IRQ_RTCA0_MASK,
- },
[S2MPS11_IRQ_RTCA1] = {
.reg_offset = 1,
.mask = S2MPS11_IRQ_RTCA1_MASK,
},
+ [S2MPS11_IRQ_RTCA0] = {
+ .reg_offset = 1,
+ .mask = S2MPS11_IRQ_RTCA0_MASK,
+ },
[S2MPS11_IRQ_SMPL] = {
.reg_offset = 1,
.mask = S2MPS11_IRQ_SMPL_MASK,
@@ -484,6 +484,12 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
return ret;
}

+ /*
+ * The rtc-s5m driver requests S2MPS14_IRQ_RTCA0 also for S2MPS11
+ * so the interrupt number must be consistent.
+ */
+ BUILD_BUG_ON(((enum s2mps14_irq)S2MPS11_IRQ_RTCA0) != S2MPS14_IRQ_RTCA0);
+
return 0;
}

diff --git a/include/linux/mfd/samsung/irq.h b/include/linux/mfd/samsung/irq.h
index f35af7361b60..667aa40486dd 100644
--- a/include/linux/mfd/samsung/irq.h
+++ b/include/linux/mfd/samsung/irq.h
@@ -74,8 +74,8 @@ enum s2mps11_irq {
S2MPS11_IRQ_MRB,

S2MPS11_IRQ_RTC60S,
- S2MPS11_IRQ_RTCA0,
S2MPS11_IRQ_RTCA1,
+ S2MPS11_IRQ_RTCA0,
S2MPS11_IRQ_SMPL,
S2MPS11_IRQ_RTC1S,
S2MPS11_IRQ_WTSR,
--
1.9.1

2015-04-02 14:40:05

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 2/2] ARM: dts: Fix pinctrl settings for S2MPS11 RTC alarm IRQ on Arndale Octa

On Arndale Octa the S2MPS11 RTC alarm interrupt was not handled at all
because of wrong configuration of interrupt and gpx3-2.
1. Interrupt is signaled by falling edge.
2. This GPIO line is hard-wired on the board to PVDD_APIO_1V8 through a
resistor so pull-up/down must be disabled.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
arch/arm/boot/dts/exynos5420-arndale-octa.dts | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos5420-arndale-octa.dts b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
index d78fcd997ce6..97346df31d41 100644
--- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts
+++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
@@ -87,7 +87,9 @@
s2mps11,buck4-ramp-enable = <1>;

interrupt-parent = <&gpx3>;
- interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&s2mps11_irq>;

s2mps11_osc: clocks {
#clock-cells = <1>;
@@ -379,3 +381,12 @@
clocks = <&clock CLK_RTC>, <&s2mps11_osc S2MPS11_CLK_AP>;
clock-names = "rtc", "rtc_src";
};
+
+&pinctrl_0 {
+ s2mps11_irq: s2mps11-irq {
+ samsung,pins = "gpx3-2";
+ samsung,pin-function = <0xf>;
+ samsung,pin-pud = <0>;
+ samsung,pin-drv = <0>;
+ };
+};
--
1.9.1

2015-04-07 07:37:35

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 1/2] mfd: sec: Fix RTC alarm interrupt number on S2MPS11

On Thu, 02 Apr 2015, Krzysztof Kozlowski wrote:

> The RTC on S2MPS11 is the same as S2MPS14. However interrupt numbers of
> RTC alarms 0 and 1 were inversed between these two devices. So when
> rtc-s5m driver requested S2MPS14_IRQ_RTCA0 interrupt, it matched to
> S2MPS11_IRQ_RTCA1, not RTCA0.
>
> Fix this by using consistent RTC alarm interrupt numbers and adding a
> BUILD_BUG_ON for future generations.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> drivers/mfd/sec-irq.c | 14 ++++++++++----
> include/linux/mfd/samsung/irq.h | 2 +-
> 2 files changed, 11 insertions(+), 5 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
> index ba86a918c2da..806fa8dbb22d 100644
> --- a/drivers/mfd/sec-irq.c
> +++ b/drivers/mfd/sec-irq.c
> @@ -61,14 +61,14 @@ static const struct regmap_irq s2mps11_irqs[] = {
> .reg_offset = 1,
> .mask = S2MPS11_IRQ_RTC60S_MASK,
> },
> - [S2MPS11_IRQ_RTCA0] = {
> - .reg_offset = 1,
> - .mask = S2MPS11_IRQ_RTCA0_MASK,
> - },
> [S2MPS11_IRQ_RTCA1] = {
> .reg_offset = 1,
> .mask = S2MPS11_IRQ_RTCA1_MASK,
> },
> + [S2MPS11_IRQ_RTCA0] = {
> + .reg_offset = 1,
> + .mask = S2MPS11_IRQ_RTCA0_MASK,
> + },
> [S2MPS11_IRQ_SMPL] = {
> .reg_offset = 1,
> .mask = S2MPS11_IRQ_SMPL_MASK,
> @@ -484,6 +484,12 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
> return ret;
> }
>
> + /*
> + * The rtc-s5m driver requests S2MPS14_IRQ_RTCA0 also for S2MPS11
> + * so the interrupt number must be consistent.
> + */
> + BUILD_BUG_ON(((enum s2mps14_irq)S2MPS11_IRQ_RTCA0) != S2MPS14_IRQ_RTCA0);
> +
> return 0;
> }
>
> diff --git a/include/linux/mfd/samsung/irq.h b/include/linux/mfd/samsung/irq.h
> index f35af7361b60..667aa40486dd 100644
> --- a/include/linux/mfd/samsung/irq.h
> +++ b/include/linux/mfd/samsung/irq.h
> @@ -74,8 +74,8 @@ enum s2mps11_irq {
> S2MPS11_IRQ_MRB,
>
> S2MPS11_IRQ_RTC60S,
> - S2MPS11_IRQ_RTCA0,
> S2MPS11_IRQ_RTCA1,
> + S2MPS11_IRQ_RTCA0,
> S2MPS11_IRQ_SMPL,
> S2MPS11_IRQ_RTC1S,
> S2MPS11_IRQ_WTSR,

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2015-04-17 14:50:05

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: Fix pinctrl settings for S2MPS11 RTC alarm IRQ on Arndale Octa

2015-04-02 23:36 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
> On Arndale Octa the S2MPS11 RTC alarm interrupt was not handled at all
> because of wrong configuration of interrupt and gpx3-2.
> 1. Interrupt is signaled by falling edge.
> 2. This GPIO line is hard-wired on the board to PVDD_APIO_1V8 through a
> resistor so pull-up/down must be disabled.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> arch/arm/boot/dts/exynos5420-arndale-octa.dts | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)

Dear Kukjin,

Any comments on this and other patches. A lot of emails waits for
your opinion. Is there anything I could do to help you in smooth
review or applying?

Best regards,
Krzysztof

>
> diff --git a/arch/arm/boot/dts/exynos5420-arndale-octa.dts b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
> index d78fcd997ce6..97346df31d41 100644
> --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts
> +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
> @@ -87,7 +87,9 @@
> s2mps11,buck4-ramp-enable = <1>;
>
> interrupt-parent = <&gpx3>;
> - interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
> + interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&s2mps11_irq>;
>
> s2mps11_osc: clocks {
> #clock-cells = <1>;
> @@ -379,3 +381,12 @@
> clocks = <&clock CLK_RTC>, <&s2mps11_osc S2MPS11_CLK_AP>;
> clock-names = "rtc", "rtc_src";
> };
> +
> +&pinctrl_0 {
> + s2mps11_irq: s2mps11-irq {
> + samsung,pins = "gpx3-2";
> + samsung,pin-function = <0xf>;
> + samsung,pin-pud = <0>;
> + samsung,pin-drv = <0>;
> + };
> +};
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2015-04-27 00:59:51

by Kukjin Kim

[permalink] [raw]
Subject: RE: [PATCH 2/2] ARM: dts: Fix pinctrl settings for S2MPS11 RTC alarm IRQ on Arndale Octa

Krzysztof Kozlowski wrote:
>
> 2015-04-02 23:36 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
> > On Arndale Octa the S2MPS11 RTC alarm interrupt was not handled at all
> > because of wrong configuration of interrupt and gpx3-2.
> > 1. Interrupt is signaled by falling edge.
> > 2. This GPIO line is hard-wired on the board to PVDD_APIO_1V8 through a
> > resistor so pull-up/down must be disabled.
> >
> > Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > ---
> > arch/arm/boot/dts/exynos5420-arndale-octa.dts | 13 ++++++++++++-
> > 1 file changed, 12 insertions(+), 1 deletion(-)
>
> Dear Kukjin,
>
> Any comments on this and other patches. A lot of emails waits for
> your opinion. Is there anything I could do to help you in smooth
> review or applying?
>
Sorry for the delay and looks good to me.

Will apply into fixes branch and thanks.

- Kukjin

> Best regards,
> Krzysztof
>
> >
> > diff --git a/arch/arm/boot/dts/exynos5420-arndale-octa.dts b/arch/arm/boot/dts/exynos5420-arndale-
> octa.dts
> > index d78fcd997ce6..97346df31d41 100644
> > --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts
> > +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
> > @@ -87,7 +87,9 @@
> > s2mps11,buck4-ramp-enable = <1>;
> >
> > interrupt-parent = <&gpx3>;
> > - interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&s2mps11_irq>;
> >
> > s2mps11_osc: clocks {
> > #clock-cells = <1>;
> > @@ -379,3 +381,12 @@
> > clocks = <&clock CLK_RTC>, <&s2mps11_osc S2MPS11_CLK_AP>;
> > clock-names = "rtc", "rtc_src";
> > };
> > +
> > +&pinctrl_0 {
> > + s2mps11_irq: s2mps11-irq {
> > + samsung,pins = "gpx3-2";
> > + samsung,pin-function = <0xf>;
> > + samsung,pin-pud = <0>;
> > + samsung,pin-drv = <0>;
> > + };
> > +};
> > --
> > 1.9.1

2015-04-30 15:58:23

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: Fix pinctrl settings for S2MPS11 RTC alarm IRQ on Arndale Octa

Hi Krzystof,

Krzysztof Kozlowski <[email protected]> writes:

> 2015-04-02 23:36 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
>> On Arndale Octa the S2MPS11 RTC alarm interrupt was not handled at all
>> because of wrong configuration of interrupt and gpx3-2.
>> 1. Interrupt is signaled by falling edge.
>> 2. This GPIO line is hard-wired on the board to PVDD_APIO_1V8 through a
>> resistor so pull-up/down must be disabled.
>>
>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>> ---
>> arch/arm/boot/dts/exynos5420-arndale-octa.dts | 13 ++++++++++++-
>> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> Dear Kukjin,
>
> Any comments on this and other patches. A lot of emails waits for
> your opinion. Is there anything I could do to help you in smooth
> review or applying?

IMO, I think you you should just start collecting fixes and features and
queuing them for Kukjin and then start working as a co-maintainer.

The samsung platforms have been in a near constant state of breakage
over the last *several* cycles, and something really needs to change in
how these are being monitored and maintained.

If someone else is paying closer attention, especially to important
fixes like this and the recent ones for other imprecies aborts etc.,
all of us who are trying to use these Exynos platforms with mainline
will be in much better shape.

Kevin

2015-05-01 08:04:59

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: Fix pinctrl settings for S2MPS11 RTC alarm IRQ on Arndale Octa

2015-05-01 0:58 GMT+09:00 Kevin Hilman <[email protected]>:
> Hi Krzystof,
>
> Krzysztof Kozlowski <[email protected]> writes:
>
>> 2015-04-02 23:36 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
>>> On Arndale Octa the S2MPS11 RTC alarm interrupt was not handled at all
>>> because of wrong configuration of interrupt and gpx3-2.
>>> 1. Interrupt is signaled by falling edge.
>>> 2. This GPIO line is hard-wired on the board to PVDD_APIO_1V8 through a
>>> resistor so pull-up/down must be disabled.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>> ---
>>> arch/arm/boot/dts/exynos5420-arndale-octa.dts | 13 ++++++++++++-
>>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> Dear Kukjin,
>>
>> Any comments on this and other patches. A lot of emails waits for
>> your opinion. Is there anything I could do to help you in smooth
>> review or applying?
>
> IMO, I think you you should just start collecting fixes and features and
> queuing them for Kukjin and then start working as a co-maintainer.

Seems an interesting and challenging idea. Let's try it and see if it helps.

>
> The samsung platforms have been in a near constant state of breakage
> over the last *several* cycles, and something really needs to change in
> how these are being monitored and maintained.

We have been struggling with this for some time... It is annoying also to me.

Best regards,
Krzysztof