* [PATCH 0/2] MPM pin mappings for MSM8226 and MSM8974 @ 2023-09-22 22:40 Matti Lehtimäki 2023-09-22 22:40 ` [PATCH 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings Matti Lehtimäki 2023-09-22 22:40 ` [PATCH 2/2] pinctrl: qcom: msm8974: " Matti Lehtimäki 0 siblings, 2 replies; 9+ messages in thread From: Matti Lehtimäki @ 2023-09-22 22:40 UTC (permalink / raw) To: linux-arm-msm Cc: ~postmarketos/upstreaming, phone-devel, Matti Lehtimäki, linux-gpio, linux-kernel This series adds the MPM wakeirq mappings for MSM8226 and MSM8974. Matti Lehtimäki (2): pinctrl: qcom: msm8226: Add MPM pin mappings pinctrl: qcom: msm8974: Add MPM pin mappings drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++ drivers/pinctrl/qcom/pinctrl-msm8x74.c | 12 ++++++++++++ 2 files changed, 24 insertions(+) -- 2.39.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings 2023-09-22 22:40 [PATCH 0/2] MPM pin mappings for MSM8226 and MSM8974 Matti Lehtimäki @ 2023-09-22 22:40 ` Matti Lehtimäki 2023-09-23 9:32 ` Luca Weiss 2023-09-22 22:40 ` [PATCH 2/2] pinctrl: qcom: msm8974: " Matti Lehtimäki 1 sibling, 1 reply; 9+ messages in thread From: Matti Lehtimäki @ 2023-09-22 22:40 UTC (permalink / raw) To: linux-arm-msm Cc: ~postmarketos/upstreaming, phone-devel, Matti Lehtimäki, Bjorn Andersson, Andy Gross, Konrad Dybcio, Linus Walleij, linux-gpio, linux-kernel Add pin <-> wakeirq mappings to allow for waking up the AP from sleep through MPM-connected pins. Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> --- drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/pinctrl/qcom/pinctrl-msm8226.c b/drivers/pinctrl/qcom/pinctrl-msm8226.c index 994619840a70..1e46a9ab382f 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm8226.c +++ b/drivers/pinctrl/qcom/pinctrl-msm8226.c @@ -612,6 +612,16 @@ static const struct msm_pingroup msm8226_groups[] = { #define NUM_GPIO_PINGROUPS 117 +static const struct msm_gpio_wakeirq_map msm8226_mpm_map[] = { + { 1, 3 }, { 4, 4 }, { 5, 5 }, { 9, 6 }, { 13, 7 }, { 17, 8 }, + { 21, 9 }, { 27, 10 }, { 29, 11 }, { 31, 12 }, { 33, 13 }, { 35, 14 }, + { 37, 15 }, { 38, 16 }, { 39, 17 }, { 41, 18 }, { 46, 19 }, { 48, 20 }, + { 49, 21 }, { 50, 22 }, { 51, 23 }, { 52, 24 }, { 54, 25 }, { 62, 26 }, + { 63, 27 }, { 64, 28 }, { 65, 29 }, { 66, 30 }, { 67, 31 }, { 68, 32 }, + { 69, 33 }, { 71, 34 }, { 72, 35 }, { 106, 36 }, { 107, 37 }, + { 108, 38 }, { 109, 39 }, { 110, 40 }, { 111, 54 }, { 113, 55 }, +}; + static const struct msm_pinctrl_soc_data msm8226_pinctrl = { .pins = msm8226_pins, .npins = ARRAY_SIZE(msm8226_pins), @@ -620,6 +630,8 @@ static const struct msm_pinctrl_soc_data msm8226_pinctrl = { .groups = msm8226_groups, .ngroups = ARRAY_SIZE(msm8226_groups), .ngpios = NUM_GPIO_PINGROUPS, + .wakeirq_map = msm8226_mpm_map, + .nwakeirq_map = ARRAY_SIZE(msm8226_mpm_map), }; static int msm8226_pinctrl_probe(struct platform_device *pdev) -- 2.39.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings 2023-09-22 22:40 ` [PATCH 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings Matti Lehtimäki @ 2023-09-23 9:32 ` Luca Weiss 2023-09-23 10:00 ` Stephan Gerhold 0 siblings, 1 reply; 9+ messages in thread From: Luca Weiss @ 2023-09-23 9:32 UTC (permalink / raw) To: linux-arm-msm, ~postmarketos/upstreaming Cc: ~postmarketos/upstreaming, phone-devel, Matti Lehtimäki, Bjorn Andersson, Andy Gross, Konrad Dybcio, Linus Walleij, linux-gpio, linux-kernel, Matti Lehtimäki Hi Matti, On Samstag, 23. September 2023 00:40:26 CEST Matti Lehtimäki wrote: > Add pin <-> wakeirq mappings to allow for waking up the AP from sleep > through MPM-connected pins. > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> > --- > drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm8226.c > b/drivers/pinctrl/qcom/pinctrl-msm8226.c index 994619840a70..1e46a9ab382f > 100644 > --- a/drivers/pinctrl/qcom/pinctrl-msm8226.c > +++ b/drivers/pinctrl/qcom/pinctrl-msm8226.c > @@ -612,6 +612,16 @@ static const struct msm_pingroup msm8226_groups[] = { > > #define NUM_GPIO_PINGROUPS 117 > > +static const struct msm_gpio_wakeirq_map msm8226_mpm_map[] = { > + { 1, 3 }, { 4, 4 }, { 5, 5 }, { 9, 6 }, { 13, 7 }, { 17, 8 }, I'm not really convinced this is the correct order of values... Let's look at downstream: qcom,gpio-map = <3 1>, <4 4 >, <5 5 >, <6 9 >, [...] From Documentation/devicetree/bindings/arm/msm/mpm.txt downstream: Each tuple represents a MPM pin and which GIC interrupt is routed to it. So first is pin number, second is interrupt number. And check mainline: /** * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins * @gpio: The GPIOs that are wakeup capable * @wakeirq: The interrupt at the always-on interrupt controller */ struct msm_gpio_wakeirq_map { unsigned int gpio; unsigned int wakeirq; }; So here we also have the order pin-interrupt, not the reverse order. Therefore I believe the order in this patch is incorrect, and it should rather be: { 3, 1 }, { 4, 4 }, { 5, 5 }, { 6, 9 }, { 7, 13 }, { 8, 17 }, [...] Or do you think I'm missing something? Regards Luca > + { 21, 9 }, { 27, 10 }, { 29, 11 }, { 31, 12 }, { 33, 13 }, { 35, 14 }, > + { 37, 15 }, { 38, 16 }, { 39, 17 }, { 41, 18 }, { 46, 19 }, { 48, 20 }, > + { 49, 21 }, { 50, 22 }, { 51, 23 }, { 52, 24 }, { 54, 25 }, { 62, 26 }, > + { 63, 27 }, { 64, 28 }, { 65, 29 }, { 66, 30 }, { 67, 31 }, { 68, 32 }, > + { 69, 33 }, { 71, 34 }, { 72, 35 }, { 106, 36 }, { 107, 37 }, > + { 108, 38 }, { 109, 39 }, { 110, 40 }, { 111, 54 }, { 113, 55 }, > +}; > + > static const struct msm_pinctrl_soc_data msm8226_pinctrl = { > .pins = msm8226_pins, > .npins = ARRAY_SIZE(msm8226_pins), > @@ -620,6 +630,8 @@ static const struct msm_pinctrl_soc_data msm8226_pinctrl > = { .groups = msm8226_groups, > .ngroups = ARRAY_SIZE(msm8226_groups), > .ngpios = NUM_GPIO_PINGROUPS, > + .wakeirq_map = msm8226_mpm_map, > + .nwakeirq_map = ARRAY_SIZE(msm8226_mpm_map), > }; > > static int msm8226_pinctrl_probe(struct platform_device *pdev) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings 2023-09-23 9:32 ` Luca Weiss @ 2023-09-23 10:00 ` Stephan Gerhold 2023-09-23 11:19 ` Luca Weiss 0 siblings, 1 reply; 9+ messages in thread From: Stephan Gerhold @ 2023-09-23 10:00 UTC (permalink / raw) To: Luca Weiss Cc: linux-arm-msm, ~postmarketos/upstreaming, phone-devel, Matti Lehtimäki, Bjorn Andersson, Andy Gross, Konrad Dybcio, Linus Walleij, linux-gpio, linux-kernel On Sat, Sep 23, 2023 at 11:32:47AM +0200, Luca Weiss wrote: > Hi Matti, > > On Samstag, 23. September 2023 00:40:26 CEST Matti Lehtimäki wrote: > > Add pin <-> wakeirq mappings to allow for waking up the AP from sleep > > through MPM-connected pins. > > > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> > > --- > > drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > b/drivers/pinctrl/qcom/pinctrl-msm8226.c index 994619840a70..1e46a9ab382f > > 100644 > > --- a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > +++ b/drivers/pinctrl/qcom/pinctrl-msm8226.c > > @@ -612,6 +612,16 @@ static const struct msm_pingroup msm8226_groups[] = { > > > > #define NUM_GPIO_PINGROUPS 117 > > > > +static const struct msm_gpio_wakeirq_map msm8226_mpm_map[] = { > > + { 1, 3 }, { 4, 4 }, { 5, 5 }, { 9, 6 }, { 13, 7 }, { 17, 8 }, > > I'm not really convinced this is the correct order of values... > > Let's look at downstream: > > qcom,gpio-map = <3 1>, > <4 4 >, > <5 5 >, > <6 9 >, > [...] > > From Documentation/devicetree/bindings/arm/msm/mpm.txt downstream: > > Each tuple represents a MPM pin and which GIC interrupt is routed to it. > > So first is pin number, second is interrupt number. > > And check mainline: > > /** > * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins > * @gpio: The GPIOs that are wakeup capable > * @wakeirq: The interrupt at the always-on interrupt controller > */ > struct msm_gpio_wakeirq_map { > unsigned int gpio; > unsigned int wakeirq; > }; > > So here we also have the order pin-interrupt, not the reverse order. > > Therefore I believe the order in this patch is incorrect, and it should rather > be: > > { 3, 1 }, { 4, 4 }, { 5, 5 }, { 6, 9 }, { 7, 13 }, { 8, 17 }, > [...] > > Or do you think I'm missing something? > Yes :) Let's look at the later entries: > > + { 21, 9 }, { 27, 10 }, { 29, 11 }, { 31, 12 }, { 33, 13 }, { 35, 14 > }, > > + { 37, 15 }, { 38, 16 }, { 39, 17 }, { 41, 18 }, { 46, 19 }, { 48, 20 > }, > > + { 49, 21 }, { 50, 22 }, { 51, 23 }, { 52, 24 }, { 54, 25 }, { 62, 26 > }, > > + { 63, 27 }, { 64, 28 }, { 65, 29 }, { 66, 30 }, { 67, 31 }, { 68, 32 > }, > > + { 69, 33 }, { 71, 34 }, { 72, 35 }, { 106, 36 }, { 107, 37 }, > > + { 108, 38 }, { 109, 39 }, { 110, 40 }, { 111, 54 }, { 113, 55 }, > > +}; > > + For example: { 113, 55 }, i.e. { .gpio = 113, .wakeirq = 55 }. MSM8226 has GPIOs 0-116 and 64 MPM pins/interrupts. The order in this patch is the only one that can be correct because the definition would be invalid the other way around. 113 must be the GPIO number because it is larger than the 64 available MPM interrupt pins. :) Thanks, Stephan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings 2023-09-23 10:00 ` Stephan Gerhold @ 2023-09-23 11:19 ` Luca Weiss 2023-09-23 11:35 ` Stephan Gerhold 0 siblings, 1 reply; 9+ messages in thread From: Luca Weiss @ 2023-09-23 11:19 UTC (permalink / raw) To: Stephan Gerhold Cc: linux-arm-msm, ~postmarketos/upstreaming, phone-devel, Matti Lehtimäki, Bjorn Andersson, Andy Gross, Konrad Dybcio, Linus Walleij, linux-gpio, linux-kernel On Samstag, 23. September 2023 12:00:52 CEST Stephan Gerhold wrote: > On Sat, Sep 23, 2023 at 11:32:47AM +0200, Luca Weiss wrote: > > Hi Matti, > > > > On Samstag, 23. September 2023 00:40:26 CEST Matti Lehtimäki wrote: > > > Add pin <-> wakeirq mappings to allow for waking up the AP from sleep > > > through MPM-connected pins. > > > > > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> > > > --- > > > > > > drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > b/drivers/pinctrl/qcom/pinctrl-msm8226.c index > > > 994619840a70..1e46a9ab382f > > > 100644 > > > --- a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > +++ b/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > @@ -612,6 +612,16 @@ static const struct msm_pingroup msm8226_groups[] = > > > { > > > > > > #define NUM_GPIO_PINGROUPS 117 > > > > > > +static const struct msm_gpio_wakeirq_map msm8226_mpm_map[] = { > > > + { 1, 3 }, { 4, 4 }, { 5, 5 }, { 9, 6 }, { 13, 7 }, { 17, 8 }, > > > > I'm not really convinced this is the correct order of values... > > > > Let's look at downstream: > > qcom,gpio-map = <3 1>, > > > > <4 4 >, > > <5 5 >, > > <6 9 >, > > [...] > > > > From Documentation/devicetree/bindings/arm/msm/mpm.txt downstream: > > Each tuple represents a MPM pin and which GIC interrupt is routed to it. > > > > So first is pin number, second is interrupt number. > > > > And check mainline: > > /** > > > > * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins > > * @gpio: The GPIOs that are wakeup capable > > * @wakeirq: The interrupt at the always-on interrupt controller > > */ > > > > struct msm_gpio_wakeirq_map { > > > > unsigned int gpio; > > unsigned int wakeirq; > > > > }; > > > > So here we also have the order pin-interrupt, not the reverse order. > > > > Therefore I believe the order in this patch is incorrect, and it should > > rather> > > be: > > { 3, 1 }, { 4, 4 }, { 5, 5 }, { 6, 9 }, { 7, 13 }, { 8, 17 }, > > [...] > > > > Or do you think I'm missing something? > > Yes :) > > Let's look at the later entries: > > > + { 21, 9 }, { 27, 10 }, { 29, 11 }, { 31, 12 }, { 33, 13 }, { 35, 14 > > > > }, > > > > > + { 37, 15 }, { 38, 16 }, { 39, 17 }, { 41, 18 }, { 46, 19 }, { 48, 20 > > > > }, > > > > > + { 49, 21 }, { 50, 22 }, { 51, 23 }, { 52, 24 }, { 54, 25 }, { 62, 26 > > > > }, > > > > > + { 63, 27 }, { 64, 28 }, { 65, 29 }, { 66, 30 }, { 67, 31 }, { 68, 32 > > > > }, > > > > > + { 69, 33 }, { 71, 34 }, { 72, 35 }, { 106, 36 }, { 107, 37 }, > > > + { 108, 38 }, { 109, 39 }, { 110, 40 }, { 111, 54 }, { 113, 55 }, > > > +}; > > > + > > For example: { 113, 55 }, i.e. { .gpio = 113, .wakeirq = 55 }. > > MSM8226 has GPIOs 0-116 and 64 MPM pins/interrupts. The order in this > patch is the only one that can be correct because the definition would > be invalid the other way around. 113 must be the GPIO number because it > is larger than the 64 available MPM interrupt pins. :) So basically you're saying downstream is wrong / buggy? From qcom,gpio-map = [...], <55 113>; it's taking the properties like this (drivers/soc/qcom/mpm-of.c): unsigned long pin = be32_to_cpup(list++); irq_hw_number_t hwirq = be32_to_cpup(list++); Your explanation does make sense I guess but somewhere the link downstream -> mainline must be broken, no? Regards Luca > > Thanks, > Stephan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings 2023-09-23 11:19 ` Luca Weiss @ 2023-09-23 11:35 ` Stephan Gerhold 2023-09-23 11:49 ` Luca Weiss 0 siblings, 1 reply; 9+ messages in thread From: Stephan Gerhold @ 2023-09-23 11:35 UTC (permalink / raw) To: Luca Weiss Cc: linux-arm-msm, ~postmarketos/upstreaming, phone-devel, Matti Lehtimäki, Bjorn Andersson, Andy Gross, Konrad Dybcio, Linus Walleij, linux-gpio, linux-kernel On Sat, Sep 23, 2023 at 01:19:46PM +0200, Luca Weiss wrote: > On Samstag, 23. September 2023 12:00:52 CEST Stephan Gerhold wrote: > > On Sat, Sep 23, 2023 at 11:32:47AM +0200, Luca Weiss wrote: > > > Hi Matti, > > > > > > On Samstag, 23. September 2023 00:40:26 CEST Matti Lehtimäki wrote: > > > > Add pin <-> wakeirq mappings to allow for waking up the AP from sleep > > > > through MPM-connected pins. > > > > > > > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> > > > > --- > > > > > > > > drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++ > > > > 1 file changed, 12 insertions(+) > > > > > > > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > > b/drivers/pinctrl/qcom/pinctrl-msm8226.c index > > > > 994619840a70..1e46a9ab382f > > > > 100644 > > > > --- a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > > +++ b/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > > @@ -612,6 +612,16 @@ static const struct msm_pingroup msm8226_groups[] = > > > > { > > > > > > > > #define NUM_GPIO_PINGROUPS 117 > > > > > > > > +static const struct msm_gpio_wakeirq_map msm8226_mpm_map[] = { > > > > + { 1, 3 }, { 4, 4 }, { 5, 5 }, { 9, 6 }, { 13, 7 }, { 17, 8 }, > > > > > > I'm not really convinced this is the correct order of values... > > > > > > Let's look at downstream: > > > qcom,gpio-map = <3 1>, > > > > > > <4 4 >, > > > <5 5 >, > > > <6 9 >, > > > [...] > > > > > > From Documentation/devicetree/bindings/arm/msm/mpm.txt downstream: > > > Each tuple represents a MPM pin and which GIC interrupt is routed to it. > > > > > > So first is pin number, second is interrupt number. > > > > > > And check mainline: > > > /** > > > > > > * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins > > > * @gpio: The GPIOs that are wakeup capable > > > * @wakeirq: The interrupt at the always-on interrupt controller > > > */ > > > > > > struct msm_gpio_wakeirq_map { > > > > > > unsigned int gpio; > > > unsigned int wakeirq; > > > > > > }; > > > > > > So here we also have the order pin-interrupt, not the reverse order. > > > > > > Therefore I believe the order in this patch is incorrect, and it should > > > rather> > > > be: > > > { 3, 1 }, { 4, 4 }, { 5, 5 }, { 6, 9 }, { 7, 13 }, { 8, 17 }, > > > [...] > > > > > > Or do you think I'm missing something? > > > > Yes :) > > > > Let's look at the later entries: > > > > + { 21, 9 }, { 27, 10 }, { 29, 11 }, { 31, 12 }, { 33, 13 }, { 35, 14 > > > > > > }, > > > > > > > + { 37, 15 }, { 38, 16 }, { 39, 17 }, { 41, 18 }, { 46, 19 }, { 48, 20 > > > > > > }, > > > > > > > + { 49, 21 }, { 50, 22 }, { 51, 23 }, { 52, 24 }, { 54, 25 }, { 62, 26 > > > > > > }, > > > > > > > + { 63, 27 }, { 64, 28 }, { 65, 29 }, { 66, 30 }, { 67, 31 }, { 68, 32 > > > > > > }, > > > > > > > + { 69, 33 }, { 71, 34 }, { 72, 35 }, { 106, 36 }, { 107, 37 }, > > > > + { 108, 38 }, { 109, 39 }, { 110, 40 }, { 111, 54 }, { 113, 55 }, > > > > +}; > > > > + > > > > For example: { 113, 55 }, i.e. { .gpio = 113, .wakeirq = 55 }. > > > > MSM8226 has GPIOs 0-116 and 64 MPM pins/interrupts. The order in this > > patch is the only one that can be correct because the definition would > > be invalid the other way around. 113 must be the GPIO number because it > > is larger than the 64 available MPM interrupt pins. :) > > So basically you're saying downstream is wrong / buggy? > "Misleading" or "confusing" would be the words I would use. :-) > From qcom,gpio-map = [...], <55 113>; it's taking the properties like this > (drivers/soc/qcom/mpm-of.c): > > unsigned long pin = be32_to_cpup(list++); > irq_hw_number_t hwirq = be32_to_cpup(list++); > > Your explanation does make sense I guess but somewhere the link downstream -> > mainline must be broken, no? > After staring at mpm-of.c for a while I would say that there: - downstream "pin" = MPM pin = mainline "wakeirq" - because this is used as index to msm_mpm_irqs_m2a, which has a size of MSM_MPM_NR_MPM_IRQS (64) - downstream "hwirq" = GPIO / GIC IRQ = mainline "gpio" This means for <55 113>: pin = wakeirq = 55 and hwirq = gpio = 113. Which matches the definition in this patch: { .gpio = 113, .wakeirq = 55 } = { 113, 55 } Stephan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings 2023-09-23 11:35 ` Stephan Gerhold @ 2023-09-23 11:49 ` Luca Weiss 0 siblings, 0 replies; 9+ messages in thread From: Luca Weiss @ 2023-09-23 11:49 UTC (permalink / raw) To: Stephan Gerhold Cc: linux-arm-msm, ~postmarketos/upstreaming, phone-devel, Matti Lehtimäki, Bjorn Andersson, Andy Gross, Konrad Dybcio, Linus Walleij, linux-gpio, linux-kernel On Samstag, 23. September 2023 13:35:25 CEST Stephan Gerhold wrote: > On Sat, Sep 23, 2023 at 01:19:46PM +0200, Luca Weiss wrote: > > On Samstag, 23. September 2023 12:00:52 CEST Stephan Gerhold wrote: > > > On Sat, Sep 23, 2023 at 11:32:47AM +0200, Luca Weiss wrote: > > > > Hi Matti, > > > > > > > > On Samstag, 23. September 2023 00:40:26 CEST Matti Lehtimäki wrote: > > > > > Add pin <-> wakeirq mappings to allow for waking up the AP from > > > > > sleep > > > > > through MPM-connected pins. > > > > > > > > > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> > > > > > --- > > > > > > > > > > drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++ > > > > > 1 file changed, 12 insertions(+) > > > > > > > > > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > > > b/drivers/pinctrl/qcom/pinctrl-msm8226.c index > > > > > 994619840a70..1e46a9ab382f > > > > > 100644 > > > > > --- a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > > > +++ b/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > > > @@ -612,6 +612,16 @@ static const struct msm_pingroup > > > > > msm8226_groups[] = > > > > > { > > > > > > > > > > #define NUM_GPIO_PINGROUPS 117 > > > > > > > > > > +static const struct msm_gpio_wakeirq_map msm8226_mpm_map[] = { > > > > > + { 1, 3 }, { 4, 4 }, { 5, 5 }, { 9, 6 }, { 13, 7 }, { 17, 8 }, > > > > > > > > I'm not really convinced this is the correct order of values... > > > > > > > > Let's look at downstream: > > > > qcom,gpio-map = <3 1>, > > > > > > > > <4 4 >, > > > > <5 5 >, > > > > <6 9 >, > > > > [...] > > > > > > > > From Documentation/devicetree/bindings/arm/msm/mpm.txt downstream: > > > > Each tuple represents a MPM pin and which GIC interrupt is routed to > > > > it. > > > > > > > > So first is pin number, second is interrupt number. > > > > > > > > And check mainline: > > > > /** > > > > > > > > * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins > > > > * @gpio: The GPIOs that are wakeup capable > > > > * @wakeirq: The interrupt at the always-on interrupt > > > > controller > > > > */ > > > > > > > > struct msm_gpio_wakeirq_map { > > > > > > > > unsigned int gpio; > > > > unsigned int wakeirq; > > > > > > > > }; > > > > > > > > So here we also have the order pin-interrupt, not the reverse order. > > > > > > > > Therefore I believe the order in this patch is incorrect, and it > > > > should > > > > rather> > > > > > > > > be: > > > > { 3, 1 }, { 4, 4 }, { 5, 5 }, { 6, 9 }, { 7, 13 }, { 8, 17 }, > > > > [...] > > > > > > > > Or do you think I'm missing something? > > > > > > Yes :) > > > > > > Let's look at the later entries: > > > > > + { 21, 9 }, { 27, 10 }, { 29, 11 }, { 31, 12 }, { 33, 13 }, { 35, > > > > > 14 > > > > > > > > }, > > > > > > > > > + { 37, 15 }, { 38, 16 }, { 39, 17 }, { 41, 18 }, { 46, 19 }, { 48, > > > > > 20 > > > > > > > > }, > > > > > > > > > + { 49, 21 }, { 50, 22 }, { 51, 23 }, { 52, 24 }, { 54, 25 }, { 62, > > > > > 26 > > > > > > > > }, > > > > > > > > > + { 63, 27 }, { 64, 28 }, { 65, 29 }, { 66, 30 }, { 67, 31 }, { 68, > > > > > 32 > > > > > > > > }, > > > > > > > > > + { 69, 33 }, { 71, 34 }, { 72, 35 }, { 106, 36 }, { 107, 37 }, > > > > > + { 108, 38 }, { 109, 39 }, { 110, 40 }, { 111, 54 }, { 113, 55 }, > > > > > +}; > > > > > + > > > > > > For example: { 113, 55 }, i.e. { .gpio = 113, .wakeirq = 55 }. > > > > > > MSM8226 has GPIOs 0-116 and 64 MPM pins/interrupts. The order in this > > > patch is the only one that can be correct because the definition would > > > be invalid the other way around. 113 must be the GPIO number because it > > > is larger than the 64 available MPM interrupt pins. :) > > > > So basically you're saying downstream is wrong / buggy? > > "Misleading" or "confusing" would be the words I would use. :-) ;) > > > From qcom,gpio-map = [...], <55 113>; it's taking the properties like this > > > > (drivers/soc/qcom/mpm-of.c): > > unsigned long pin = be32_to_cpup(list++); > > irq_hw_number_t hwirq = be32_to_cpup(list++); > > > > Your explanation does make sense I guess but somewhere the link downstream > > -> mainline must be broken, no? > > After staring at mpm-of.c for a while I would say that there: > - downstream "pin" = MPM pin = mainline "wakeirq" > - because this is used as index to msm_mpm_irqs_m2a, which has a size > of MSM_MPM_NR_MPM_IRQS (64) > - downstream "hwirq" = GPIO / GIC IRQ = mainline "gpio" > > This means for <55 113>: pin = wakeirq = 55 and hwirq = gpio = 113. > Which matches the definition in this patch: > { .gpio = 113, .wakeirq = 55 } = { 113, 55 } Fun, thanks for digging into it! @Matti: I think I see one missing entry here "<41 115>," on downstream, so { 115, 41 } appears to be missing in this patch? Or is there a reason you omitted that one? The rest looks correct :) Regards Luca > > Stephan ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] pinctrl: qcom: msm8974: Add MPM pin mappings 2023-09-22 22:40 [PATCH 0/2] MPM pin mappings for MSM8226 and MSM8974 Matti Lehtimäki 2023-09-22 22:40 ` [PATCH 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings Matti Lehtimäki @ 2023-09-22 22:40 ` Matti Lehtimäki 2023-09-23 11:52 ` Luca Weiss 1 sibling, 1 reply; 9+ messages in thread From: Matti Lehtimäki @ 2023-09-22 22:40 UTC (permalink / raw) To: linux-arm-msm Cc: ~postmarketos/upstreaming, phone-devel, Matti Lehtimäki, Bjorn Andersson, Andy Gross, Konrad Dybcio, Linus Walleij, linux-gpio, linux-kernel Add pin <-> wakeirq mappings to allow for waking up the AP from sleep through MPM-connected pins. Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> --- drivers/pinctrl/qcom/pinctrl-msm8x74.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/pinctrl/qcom/pinctrl-msm8x74.c b/drivers/pinctrl/qcom/pinctrl-msm8x74.c index d5fe62992849..238c83f6ec4f 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm8x74.c +++ b/drivers/pinctrl/qcom/pinctrl-msm8x74.c @@ -1045,6 +1045,16 @@ static const struct msm_pingroup msm8x74_groups[] = { #define NUM_GPIO_PINGROUPS 146 +static const struct msm_gpio_wakeirq_map msm8x74_mpm_map[] = { + { 1, 4 }, { 5, 5 }, { 9, 6 }, { 18, 7 }, { 20, 8 }, { 24, 9 }, + { 27, 10 }, { 28, 11 }, { 34, 12 }, { 35, 13 }, { 37, 14 }, { 42, 15 }, + { 44, 16 }, { 46, 17 }, { 50, 18 }, { 54, 19 }, { 59, 20 }, { 61, 21 }, + { 62, 22 }, { 64, 23 }, { 65, 24 }, { 66, 25 }, { 67, 26 }, { 68, 27 }, + { 71, 28 }, { 72, 29 }, { 73, 30 }, { 74, 31 }, { 75, 32 }, { 77, 33 }, + { 79, 34 }, { 80, 35 }, { 82, 36 }, { 86, 37 }, { 92, 38 }, { 93, 39 }, + { 95, 40 }, { 102, 3 }, { 144, 41 }, +}; + static const struct msm_pinctrl_soc_data msm8x74_pinctrl = { .pins = msm8x74_pins, .npins = ARRAY_SIZE(msm8x74_pins), @@ -1053,6 +1063,8 @@ static const struct msm_pinctrl_soc_data msm8x74_pinctrl = { .groups = msm8x74_groups, .ngroups = ARRAY_SIZE(msm8x74_groups), .ngpios = NUM_GPIO_PINGROUPS, + .wakeirq_map = msm8x74_mpm_map, + .nwakeirq_map = ARRAY_SIZE(msm8x74_mpm_map), }; static int msm8x74_pinctrl_probe(struct platform_device *pdev) -- 2.39.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] pinctrl: qcom: msm8974: Add MPM pin mappings 2023-09-22 22:40 ` [PATCH 2/2] pinctrl: qcom: msm8974: " Matti Lehtimäki @ 2023-09-23 11:52 ` Luca Weiss 0 siblings, 0 replies; 9+ messages in thread From: Luca Weiss @ 2023-09-23 11:52 UTC (permalink / raw) To: linux-arm-msm, ~postmarketos/upstreaming Cc: ~postmarketos/upstreaming, phone-devel, Matti Lehtimäki, Bjorn Andersson, Andy Gross, Konrad Dybcio, Linus Walleij, linux-gpio, linux-kernel, Matti Lehtimäki On Samstag, 23. September 2023 00:40:27 CEST Matti Lehtimäki wrote: > Add pin <-> wakeirq mappings to allow for waking up the AP from sleep > through MPM-connected pins. > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> Based on the discussion in the 8226 patch, this looks correct, compared with downstream arch/arm/boot/dts/msm8974pro-pm.dtsi. Reviewed-by: Luca Weiss <luca@z3ntu.xyz> > --- > drivers/pinctrl/qcom/pinctrl-msm8x74.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm8x74.c > b/drivers/pinctrl/qcom/pinctrl-msm8x74.c index d5fe62992849..238c83f6ec4f > 100644 > --- a/drivers/pinctrl/qcom/pinctrl-msm8x74.c > +++ b/drivers/pinctrl/qcom/pinctrl-msm8x74.c > @@ -1045,6 +1045,16 @@ static const struct msm_pingroup msm8x74_groups[] = { > > #define NUM_GPIO_PINGROUPS 146 > > +static const struct msm_gpio_wakeirq_map msm8x74_mpm_map[] = { > + { 1, 4 }, { 5, 5 }, { 9, 6 }, { 18, 7 }, { 20, 8 }, { 24, 9 }, > + { 27, 10 }, { 28, 11 }, { 34, 12 }, { 35, 13 }, { 37, 14 }, { 42, 15 }, > + { 44, 16 }, { 46, 17 }, { 50, 18 }, { 54, 19 }, { 59, 20 }, { 61, 21 }, > + { 62, 22 }, { 64, 23 }, { 65, 24 }, { 66, 25 }, { 67, 26 }, { 68, 27 }, > + { 71, 28 }, { 72, 29 }, { 73, 30 }, { 74, 31 }, { 75, 32 }, { 77, 33 }, > + { 79, 34 }, { 80, 35 }, { 82, 36 }, { 86, 37 }, { 92, 38 }, { 93, 39 }, > + { 95, 40 }, { 102, 3 }, { 144, 41 }, > +}; > + > static const struct msm_pinctrl_soc_data msm8x74_pinctrl = { > .pins = msm8x74_pins, > .npins = ARRAY_SIZE(msm8x74_pins), > @@ -1053,6 +1063,8 @@ static const struct msm_pinctrl_soc_data > msm8x74_pinctrl = { .groups = msm8x74_groups, > .ngroups = ARRAY_SIZE(msm8x74_groups), > .ngpios = NUM_GPIO_PINGROUPS, > + .wakeirq_map = msm8x74_mpm_map, > + .nwakeirq_map = ARRAY_SIZE(msm8x74_mpm_map), > }; > > static int msm8x74_pinctrl_probe(struct platform_device *pdev) ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-09-23 11:53 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-09-22 22:40 [PATCH 0/2] MPM pin mappings for MSM8226 and MSM8974 Matti Lehtimäki 2023-09-22 22:40 ` [PATCH 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings Matti Lehtimäki 2023-09-23 9:32 ` Luca Weiss 2023-09-23 10:00 ` Stephan Gerhold 2023-09-23 11:19 ` Luca Weiss 2023-09-23 11:35 ` Stephan Gerhold 2023-09-23 11:49 ` Luca Weiss 2023-09-22 22:40 ` [PATCH 2/2] pinctrl: qcom: msm8974: " Matti Lehtimäki 2023-09-23 11:52 ` Luca Weiss
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).