David Collins (1):
pinctrl: qcom: spmi-gpio: correct parent irqspec translation
satya priya (1):
arm64: dts: sc7280: Add volume up support for sc7280-idp
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 31 ++++++++++++++++++++++++++
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 37 +++++++++++++++++++++++++++++---
2 files changed, 65 insertions(+), 3 deletions(-)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Add pm7325 PMIC gpio support for vol+ on sc7280-idp.
Signed-off-by: satya priya <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 371a2a9..52bcbbc 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -199,6 +199,37 @@
modem-init;
};
+&soc {
+ gpio_keys {
+ compatible = "gpio-keys";
+ label = "gpio-keys";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&key_vol_up_default>;
+
+ vol_up {
+ label = "volume_up";
+ gpios = <&pm7325_gpios 6 GPIO_ACTIVE_LOW>;
+ linux,input-type = <1>;
+ linux,code = <KEY_VOLUMEUP>;
+ gpio-key,wakeup;
+ debounce-interval = <15>;
+ linux,can-disable;
+ };
+ };
+};
+
+&pm7325_gpios {
+ key_vol_up_default: key_vol_up_default {
+ pins = "gpio6";
+ function = "normal";
+ input-enable;
+ bias-pull-up;
+ power-source = <0>;
+ qcom,drive-strength = <3>;
+ };
+};
+
&pmk8350_vadc {
pmk8350_die_temp {
reg = <PMK8350_ADC7_DIE_TEMP>;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
From: David Collins <[email protected]>
pmic_gpio_child_to_parent_hwirq() and
gpiochip_populate_parent_fwspec_fourcell() translate a pinctrl-
spmi-gpio irqspec to an SPMI controller irqspec. When they do
this, they use a fixed SPMI slave ID of 0 and a fixed GPIO
peripheral offset of 0xC0 (corresponding to SPMI address 0xC000).
This translation results in an incorrect irqspec for secondary
PMICs that don't have a slave ID of 0 as well as for PMIC chips
which have GPIO peripherals located at a base address other than
0xC000.
Correct this issue by passing the slave ID of the pinctrl-spmi-
gpio device's parent in the SPMI controller irqspec and by
calculating the peripheral ID base from the device tree 'reg'
property of the pinctrl-spmi-gpio device.
Signed-off-by: David Collins <[email protected]>
Signed-off-by: satya priya <[email protected]>
---
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 37 +++++++++++++++++++++++++++++---
1 file changed, 34 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index 98bf0e2..dbae168 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2012-2014, 2016-2021 The Linux Foundation. All rights reserved.
*/
#include <linux/gpio/driver.h>
@@ -14,6 +14,7 @@
#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/slab.h>
+#include <linux/spmi.h>
#include <linux/types.h>
#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
@@ -171,6 +172,8 @@ struct pmic_gpio_state {
struct pinctrl_dev *ctrl;
struct gpio_chip chip;
struct irq_chip irq;
+ u8 usid;
+ u8 pid_base;
};
static const struct pinconf_generic_params pmic_gpio_bindings[] = {
@@ -949,12 +952,36 @@ static int pmic_gpio_child_to_parent_hwirq(struct gpio_chip *chip,
unsigned int *parent_hwirq,
unsigned int *parent_type)
{
- *parent_hwirq = child_hwirq + 0xc0;
+ struct pmic_gpio_state *state = gpiochip_get_data(chip);
+
+ *parent_hwirq = child_hwirq + state->pid_base;
*parent_type = child_type;
return 0;
}
+static void *pmic_gpio_populate_parent_fwspec(struct gpio_chip *chip,
+ unsigned int parent_hwirq,
+ unsigned int parent_type)
+{
+ struct pmic_gpio_state *state = gpiochip_get_data(chip);
+ struct irq_fwspec *fwspec;
+
+ fwspec = kzalloc(sizeof(*fwspec), GFP_KERNEL);
+ if (!fwspec)
+ return NULL;
+
+ fwspec->fwnode = chip->irq.parent_domain->fwnode;
+
+ fwspec->param_count = 4;
+ fwspec->param[0] = state->usid;
+ fwspec->param[1] = parent_hwirq;
+ fwspec->param[2] = 0;
+ fwspec->param[3] = parent_type;
+
+ return fwspec;
+}
+
static int pmic_gpio_probe(struct platform_device *pdev)
{
struct irq_domain *parent_domain;
@@ -965,6 +992,7 @@ static int pmic_gpio_probe(struct platform_device *pdev)
struct pmic_gpio_pad *pad, *pads;
struct pmic_gpio_state *state;
struct gpio_irq_chip *girq;
+ const struct spmi_device *parent_spmi_dev;
int ret, npins, i;
u32 reg;
@@ -984,6 +1012,9 @@ static int pmic_gpio_probe(struct platform_device *pdev)
state->dev = &pdev->dev;
state->map = dev_get_regmap(dev->parent, NULL);
+ parent_spmi_dev = to_spmi_device(dev->parent);
+ state->usid = parent_spmi_dev->usid;
+ state->pid_base = reg >> 8;
pindesc = devm_kcalloc(dev, npins, sizeof(*pindesc), GFP_KERNEL);
if (!pindesc)
@@ -1059,7 +1090,7 @@ static int pmic_gpio_probe(struct platform_device *pdev)
girq->fwnode = of_node_to_fwnode(state->dev->of_node);
girq->parent_domain = parent_domain;
girq->child_to_parent_hwirq = pmic_gpio_child_to_parent_hwirq;
- girq->populate_parent_alloc_arg = gpiochip_populate_parent_fwspec_fourcell;
+ girq->populate_parent_alloc_arg = pmic_gpio_populate_parent_fwspec;
girq->child_offset_to_irq = pmic_gpio_child_offset_to_irq;
girq->child_irq_domain_ops.translate = pmic_gpio_domain_translate;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Hi Satya/David,
nice work on identifying this bug!
On Fri, Aug 13, 2021 at 6:56 AM satya priya <[email protected]> wrote:
>
> From: David Collins <[email protected]>
>
> pmic_gpio_child_to_parent_hwirq() and
> gpiochip_populate_parent_fwspec_fourcell() translate a pinctrl-
> spmi-gpio irqspec to an SPMI controller irqspec. When they do
> this, they use a fixed SPMI slave ID of 0 and a fixed GPIO
> peripheral offset of 0xC0 (corresponding to SPMI address 0xC000).
> This translation results in an incorrect irqspec for secondary
> PMICs that don't have a slave ID of 0 as well as for PMIC chips
> which have GPIO peripherals located at a base address other than
> 0xC000.
>
> Correct this issue by passing the slave ID of the pinctrl-spmi-
> gpio device's parent in the SPMI controller irqspec and by
> calculating the peripheral ID base from the device tree 'reg'
> property of the pinctrl-spmi-gpio device.
>
> Signed-off-by: David Collins <[email protected]>
> Signed-off-by: satya priya <[email protected]>
Is this a regression or is it fine if I just apply it for v5.15?
I was thinking v5.15 since it isn't yet used in device trees.
Yours,
Linus Walleij
Hi Linus,
On 2021-08-13 14:27, Linus Walleij wrote:
> Hi Satya/David,
>
> nice work on identifying this bug!
>
> On Fri, Aug 13, 2021 at 6:56 AM satya priya <[email protected]>
> wrote:
>>
>> From: David Collins <[email protected]>
>>
>> pmic_gpio_child_to_parent_hwirq() and
>> gpiochip_populate_parent_fwspec_fourcell() translate a pinctrl-
>> spmi-gpio irqspec to an SPMI controller irqspec. When they do
>> this, they use a fixed SPMI slave ID of 0 and a fixed GPIO
>> peripheral offset of 0xC0 (corresponding to SPMI address 0xC000).
>> This translation results in an incorrect irqspec for secondary
>> PMICs that don't have a slave ID of 0 as well as for PMIC chips
>> which have GPIO peripherals located at a base address other than
>> 0xC000.
>>
>> Correct this issue by passing the slave ID of the pinctrl-spmi-
>> gpio device's parent in the SPMI controller irqspec and by
>> calculating the peripheral ID base from the device tree 'reg'
>> property of the pinctrl-spmi-gpio device.
>>
>> Signed-off-by: David Collins <[email protected]>
>> Signed-off-by: satya priya <[email protected]>
>
> Is this a regression or is it fine if I just apply it for v5.15?
> I was thinking v5.15 since it isn't yet used in device trees.
>
Without this fix, [2/2] Vol+ support is failing. If possible please
merge it on current branch.
> Yours,
> Linus Walleij
Quoting [email protected] (2021-08-15 23:50:37)
> Hi Linus,
>
> On 2021-08-13 14:27, Linus Walleij wrote:
> > Hi Satya/David,
> >
> > nice work on identifying this bug!
> >
> > On Fri, Aug 13, 2021 at 6:56 AM satya priya <[email protected]>
> > wrote:
> >>
> >> From: David Collins <[email protected]>
> >>
> >> pmic_gpio_child_to_parent_hwirq() and
> >> gpiochip_populate_parent_fwspec_fourcell() translate a pinctrl-
> >> spmi-gpio irqspec to an SPMI controller irqspec. When they do
> >> this, they use a fixed SPMI slave ID of 0 and a fixed GPIO
> >> peripheral offset of 0xC0 (corresponding to SPMI address 0xC000).
> >> This translation results in an incorrect irqspec for secondary
> >> PMICs that don't have a slave ID of 0 as well as for PMIC chips
> >> which have GPIO peripherals located at a base address other than
> >> 0xC000.
> >>
> >> Correct this issue by passing the slave ID of the pinctrl-spmi-
> >> gpio device's parent in the SPMI controller irqspec and by
> >> calculating the peripheral ID base from the device tree 'reg'
> >> property of the pinctrl-spmi-gpio device.
> >>
> >> Signed-off-by: David Collins <[email protected]>
> >> Signed-off-by: satya priya <[email protected]>
Can you please add an appropriate Fixes tag?
> >
> > Is this a regression or is it fine if I just apply it for v5.15?
> > I was thinking v5.15 since it isn't yet used in device trees.
> >
>
> Without this fix, [2/2] Vol+ support is failing. If possible please
> merge it on current branch.
>
Are there any boards supported upstream that have a gpio block that
isn't at 0xc000?
On 2021-08-17 02:38, Stephen Boyd wrote:
> Quoting [email protected] (2021-08-15 23:50:37)
>> Hi Linus,
>>
>> On 2021-08-13 14:27, Linus Walleij wrote:
>> > Hi Satya/David,
>> >
>> > nice work on identifying this bug!
>> >
>> > On Fri, Aug 13, 2021 at 6:56 AM satya priya <[email protected]>
>> > wrote:
>> >>
>> >> From: David Collins <[email protected]>
>> >>
>> >> pmic_gpio_child_to_parent_hwirq() and
>> >> gpiochip_populate_parent_fwspec_fourcell() translate a pinctrl-
>> >> spmi-gpio irqspec to an SPMI controller irqspec. When they do
>> >> this, they use a fixed SPMI slave ID of 0 and a fixed GPIO
>> >> peripheral offset of 0xC0 (corresponding to SPMI address 0xC000).
>> >> This translation results in an incorrect irqspec for secondary
>> >> PMICs that don't have a slave ID of 0 as well as for PMIC chips
>> >> which have GPIO peripherals located at a base address other than
>> >> 0xC000.
>> >>
>> >> Correct this issue by passing the slave ID of the pinctrl-spmi-
>> >> gpio device's parent in the SPMI controller irqspec and by
>> >> calculating the peripheral ID base from the device tree 'reg'
>> >> property of the pinctrl-spmi-gpio device.
>> >>
>> >> Signed-off-by: David Collins <[email protected]>
>> >> Signed-off-by: satya priya <[email protected]>
>
> Can you please add an appropriate Fixes tag?
>
Okay.
>> >
>> > Is this a regression or is it fine if I just apply it for v5.15?
>> > I was thinking v5.15 since it isn't yet used in device trees.
>> >
>>
>> Without this fix, [2/2] Vol+ support is failing. If possible please
>> merge it on current branch.
>>
>
> Are there any boards supported upstream that have a gpio block that
> isn't at 0xc000?
yes, all the pmics used in sm8350-mtp.dts board have gpio block at
addresses different than 0xc000.
Thanks,
Satya Priya
Quoting [email protected] (2021-08-17 02:06:42)
> On 2021-08-17 02:38, Stephen Boyd wrote:
> >
> > Are there any boards supported upstream that have a gpio block that
> > isn't at 0xc000?
>
> yes, all the pmics used in sm8350-mtp.dts board have gpio block at
> addresses different than 0xc000.
>
So maybe
Fixes: f67cc6a91d88 ("arm64: dts: qcom: sm8350-mtp: Add PMICs")
is appropriate then?
On 2021-08-18 00:45, Stephen Boyd wrote:
> Quoting [email protected] (2021-08-17 02:06:42)
>> On 2021-08-17 02:38, Stephen Boyd wrote:
>> >
>> > Are there any boards supported upstream that have a gpio block that
>> > isn't at 0xc000?
>>
>> yes, all the pmics used in sm8350-mtp.dts board have gpio block at
>> addresses different than 0xc000.
>>
>
> So maybe
>
> Fixes: f67cc6a91d88 ("arm64: dts: qcom: sm8350-mtp: Add PMICs")
>
> is appropriate then?
This patch is actually fixing the pinctrl-spmi-gpio.c driver.
So, I think we should add
Fixes: ca69e2d165eb ("qcom: spmi-gpio: add support for hierarchical IRQ
chip")
Quoting [email protected] (2021-08-17 22:26:18)
> On 2021-08-18 00:45, Stephen Boyd wrote:
> > Quoting [email protected] (2021-08-17 02:06:42)
> >> On 2021-08-17 02:38, Stephen Boyd wrote:
> >> >
> >> > Are there any boards supported upstream that have a gpio block that
> >> > isn't at 0xc000?
> >>
> >> yes, all the pmics used in sm8350-mtp.dts board have gpio block at
> >> addresses different than 0xc000.
> >>
> >
> > So maybe
> >
> > Fixes: f67cc6a91d88 ("arm64: dts: qcom: sm8350-mtp: Add PMICs")
> >
> > is appropriate then?
>
> This patch is actually fixing the pinctrl-spmi-gpio.c driver.
> So, I think we should add
>
> Fixes: ca69e2d165eb ("qcom: spmi-gpio: add support for hierarchical IRQ
> chip")
OK. Were you going to resend this patch? I don't see it in linux-next so
I worry that Linus dropped it while the Fixes tag was figured out.