Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1411545 > unrolled thread

[RFC][PATCH 0/5] Hi655x powerkey support for HiKey

Back to article view | Back to linux.kernel

Contents

  [RFC][PATCH 0/5] Hi655x powerkey support for HiKey John Stultz <john.stultz@linaro.org> - 2016-06-01 23:30 +0200
    [RFC][PATCH 5/5] arm64: dts: Add powerkey info to pmic for hi6220-hikey John Stultz <john.stultz@linaro.org> - 2016-06-01 23:30 +0200
      Re: [RFC][PATCH 5/5] arm64: dts: Add powerkey info to pmic for hi6220-hikey Rob Herring <robh+dt@kernel.org> - 2016-06-01 23:50 +0200
    [RFC][PATCH 2/5] hi655x-pmic: Make hi655x pmic logic probe child nodes in the dt John Stultz <john.stultz@linaro.org> - 2016-06-01 23:30 +0200
      Re: [RFC][PATCH 2/5] hi655x-pmic: Make hi655x pmic logic probe child  nodes in the dt Lee Jones <lee.jones@linaro.org> - 2016-06-08 16:40 +0200
        Re: [RFC][PATCH 2/5] hi655x-pmic: Make hi655x pmic logic probe child  nodes in the dt John Stultz <john.stultz@linaro.org> - 2016-06-08 19:30 +0200
          Re: [RFC][PATCH 2/5] hi655x-pmic: Make hi655x pmic logic probe child  nodes in the dt Lee Jones <lee.jones@linaro.org> - 2016-06-09 16:50 +0200
    [RFC][PATCH 3/5] hi655x-pmic: Fixup issue with un-acked interrupts John Stultz <john.stultz@linaro.org> - 2016-06-01 23:30 +0200
      Re: [RFC][PATCH 3/5] hi655x-pmic: Fixup issue with un-acked  interrupts Lee Jones <lee.jones@linaro.org> - 2016-06-08 16:40 +0200

#1411545 — [RFC][PATCH 0/5] Hi655x powerkey support for HiKey

This patchset enables the pmic powerkey to function on HiKey.

I wanted to submit it for some initial review. Feedback would
be greatly appreciated!

thanks
-john

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>

Guodong Xu (1):
  arm64: dts: Add powerkey info to pmic for hi6220-hikey

John Stultz (3):
  dts: bindings: Add binding documentation for hisilicon,hi6552-powerkey
  hi655x-pmic: Make hi655x pmic logic probe child nodes in the dt
  hi655x-pmic: Fixup issue with un-acked interrupts

Jorge Ramirez-Ortiz (1):
  drivers: input: powerkey for HISI 65xx SoC

 .../bindings/input/hisilicon,hi6552-powerkey.txt   |  40 ++++
 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts     |   7 +
 drivers/input/misc/Kconfig                         |   8 +
 drivers/input/misc/Makefile                        |   1 +
 drivers/input/misc/hisi_powerkey.c                 | 228 +++++++++++++++++++++
 drivers/mfd/hi655x-pmic.c                          |   9 +
 6 files changed, 293 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/hisilicon,hi6552-powerkey.txt
 create mode 100644 drivers/input/misc/hisi_powerkey.c

-- 
1.9.1

[toc] | [next] | [standalone]

#1411546 — [RFC][PATCH 5/5] arm64: dts: Add powerkey info to pmic for hi6220-hikey

From: Guodong Xu <guodong.xu@linaro.org>

Add powerkey entry to the HiKey dts.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index e92a30c..c8e49a6 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -144,6 +144,13 @@
 		#interrupt-cells = <2>;
 		pmic-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
 
+		powerkey:powerkey@b1{
+			compatible = "hisilicon,hi6552-powerkey";
+			interrupt-parent = <&pmic>;
+			interrupts = <6 0>, <5 0>, <4 0>;
+			interrupt-names = "down", "up", "hold 4s";
+		};
+
 		regulators {
 			ldo2: LDO2 {
 				regulator-name = "LDO2_2V8";
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1411567 — Re: [RFC][PATCH 5/5] arm64: dts: Add powerkey info to pmic for hi6220-hikey

On Wed, Jun 1, 2016 at 4:27 PM, John Stultz <john.stultz@linaro.org> wrote:
> From: Guodong Xu <guodong.xu@linaro.org>
>
> Add powerkey entry to the HiKey dts.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
> index e92a30c..c8e49a6 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
> @@ -144,6 +144,13 @@
>                 #interrupt-cells = <2>;
>                 pmic-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
>
> +               powerkey:powerkey@b1{

nit: you probably don't need the label and unit-address is not needed, so just:

powerkey {

> +                       compatible = "hisilicon,hi6552-powerkey";
> +                       interrupt-parent = <&pmic>;
> +                       interrupts = <6 0>, <5 0>, <4 0>;
> +                       interrupt-names = "down", "up", "hold 4s";
> +               };
> +
>                 regulators {
>                         ldo2: LDO2 {
>                                 regulator-name = "LDO2_2V8";
> --
> 1.9.1
>

[toc] | [prev] | [next] | [standalone]

#1411547 — [RFC][PATCH 2/5] hi655x-pmic: Make hi655x pmic logic probe child nodes in the dt

In trying to wire up the powerkey driver, I found I
needed to add this to get the pmic logic to probe
child nodes in the dt data.

With this patch, child nodes get properly probed.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/mfd/hi655x-pmic.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mfd/hi655x-pmic.c b/drivers/mfd/hi655x-pmic.c
index 05ddc78..3511035 100644
--- a/drivers/mfd/hi655x-pmic.c
+++ b/drivers/mfd/hi655x-pmic.c
@@ -39,6 +39,11 @@ static const struct regmap_irq hi655x_irqs[] = {
 	{ .reg_offset = 0, .mask = RESERVE_INT },
 };
 
+static const struct of_device_id of_hi655x_pmic_child_match_tbl[] = {
+	{ .compatible = "hisilicon,hi6552-powerkey", },
+	{ /* end */ }
+};
+
 static const struct regmap_irq_chip hi655x_irq_chip = {
 	.name = "hi655x-pmic",
 	.irqs = hi655x_irqs,
@@ -122,6 +127,9 @@ static int hi655x_pmic_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pmic);
 
+	/* populate sub nodes */
+	of_platform_populate(np, of_hi655x_pmic_child_match_tbl, NULL, dev);
+
 	ret = mfd_add_devices(dev, PLATFORM_DEVID_AUTO, hi655x_pmic_devs,
 			      ARRAY_SIZE(hi655x_pmic_devs), NULL, 0, NULL);
 	if (ret) {
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1417533 — Re: [RFC][PATCH 2/5] hi655x-pmic: Make hi655x pmic logic probe child nodes in the dt

Subject should indicate "mfd".

`git log --oneline -- drivers/mfd`

> In trying to wire up the powerkey driver, I found I
> needed to add this to get the pmic logic to probe

PMIC

> child nodes in the dt data.

DT.

Please use full buffer width.  Wrapping at 50 chars is not necessary.

> With this patch, child nodes get properly probed.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  drivers/mfd/hi655x-pmic.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mfd/hi655x-pmic.c b/drivers/mfd/hi655x-pmic.c
> index 05ddc78..3511035 100644
> --- a/drivers/mfd/hi655x-pmic.c
> +++ b/drivers/mfd/hi655x-pmic.c
> @@ -39,6 +39,11 @@ static const struct regmap_irq hi655x_irqs[] = {
>  	{ .reg_offset = 0, .mask = RESERVE_INT },
>  };
>  
> +static const struct of_device_id of_hi655x_pmic_child_match_tbl[] = {
> +	{ .compatible = "hisilicon,hi6552-powerkey", },
> +	{ /* end */ }
> +};
> +
>  static const struct regmap_irq_chip hi655x_irq_chip = {
>  	.name = "hi655x-pmic",
>  	.irqs = hi655x_irqs,
> @@ -122,6 +127,9 @@ static int hi655x_pmic_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, pmic);
>  
> +	/* populate sub nodes */
> +	of_platform_populate(np, of_hi655x_pmic_child_match_tbl, NULL, dev);
> +

Oh, holey poop, no!

Please don't mix DT and MFD registration like this.

Drivers should contain either of_platform_populate() or
mfd_add_devices(), but never both.

>  	ret = mfd_add_devices(dev, PLATFORM_DEVID_AUTO, hi655x_pmic_devs,
>  			      ARRAY_SIZE(hi655x_pmic_devs), NULL, 0, NULL);
>  	if (ret) {

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

[toc] | [prev] | [next] | [standalone]

#1417727 — Re: [RFC][PATCH 2/5] hi655x-pmic: Make hi655x pmic logic probe child nodes in the dt

On Wed, Jun 8, 2016 at 7:31 AM, Lee Jones <lee.jones@linaro.org> wrote:
>
> Please use full buffer width.  Wrapping at 50 chars is not necessary.

But then how would you be able to review patches on your phone?! :)


>> --- a/drivers/mfd/hi655x-pmic.c
>> +++ b/drivers/mfd/hi655x-pmic.c
>> @@ -39,6 +39,11 @@ static const struct regmap_irq hi655x_irqs[] = {
>>       { .reg_offset = 0, .mask = RESERVE_INT },
>>  };
>>
>> +static const struct of_device_id of_hi655x_pmic_child_match_tbl[] = {
>> +     { .compatible = "hisilicon,hi6552-powerkey", },
>> +     { /* end */ }
>> +};
>> +
>>  static const struct regmap_irq_chip hi655x_irq_chip = {
>>       .name = "hi655x-pmic",
>>       .irqs = hi655x_irqs,
>> @@ -122,6 +127,9 @@ static int hi655x_pmic_probe(struct platform_device *pdev)
>>
>>       platform_set_drvdata(pdev, pmic);
>>
>> +     /* populate sub nodes */
>> +     of_platform_populate(np, of_hi655x_pmic_child_match_tbl, NULL, dev);
>> +
>
> Oh, holey poop, no!
>
> Please don't mix DT and MFD registration like this.
>
> Drivers should contain either of_platform_populate() or
> mfd_add_devices(), but never both.
>
>>       ret = mfd_add_devices(dev, PLATFORM_DEVID_AUTO, hi655x_pmic_devs,
>>                             ARRAY_SIZE(hi655x_pmic_devs), NULL, 0, NULL);
>>       if (ret) {


So apologies for my ignorance here. I'm not quite sure I see what the
right thing to do. I tried adding a "hi65xx-powerkey" entry in the
hi655x_pmic_devs, but then mfd_add_devices doesn't seem to provide any
of the irq info from the DT.

The only way I can think to make it work w/o calling to populate
sub-nodes is to pull the powerkey subnode out of the pmic node. It
seems to work, but I'm not sure if it properly describes the hardware
then.  Sorry again for being a bit clueless here.

Thanks for the review!
-john

[toc] | [prev] | [next] | [standalone]

#1418403 — Re: [RFC][PATCH 2/5] hi655x-pmic: Make hi655x pmic logic probe child nodes in the dt

On Wed, 08 Jun 2016, John Stultz wrote:

> On Wed, Jun 8, 2016 at 7:31 AM, Lee Jones <lee.jones@linaro.org> wrote:
> >
> > Please use full buffer width.  Wrapping at 50 chars is not necessary.
> 
> But then how would you be able to review patches on your phone?! :)

Sideways! ;)

> >> --- a/drivers/mfd/hi655x-pmic.c
> >> +++ b/drivers/mfd/hi655x-pmic.c
> >> @@ -39,6 +39,11 @@ static const struct regmap_irq hi655x_irqs[] = {
> >>       { .reg_offset = 0, .mask = RESERVE_INT },
> >>  };
> >>
> >> +static const struct of_device_id of_hi655x_pmic_child_match_tbl[] = {
> >> +     { .compatible = "hisilicon,hi6552-powerkey", },
> >> +     { /* end */ }
> >> +};
> >> +
> >>  static const struct regmap_irq_chip hi655x_irq_chip = {
> >>       .name = "hi655x-pmic",
> >>       .irqs = hi655x_irqs,
> >> @@ -122,6 +127,9 @@ static int hi655x_pmic_probe(struct platform_device *pdev)
> >>
> >>       platform_set_drvdata(pdev, pmic);
> >>
> >> +     /* populate sub nodes */
> >> +     of_platform_populate(np, of_hi655x_pmic_child_match_tbl, NULL, dev);
> >> +
> >
> > Oh, holey poop, no!
> >
> > Please don't mix DT and MFD registration like this.
> >
> > Drivers should contain either of_platform_populate() or
> > mfd_add_devices(), but never both.
> >
> >>       ret = mfd_add_devices(dev, PLATFORM_DEVID_AUTO, hi655x_pmic_devs,
> >>                             ARRAY_SIZE(hi655x_pmic_devs), NULL, 0, NULL);
> >>       if (ret) {
> 
> 
> So apologies for my ignorance here. I'm not quite sure I see what the
> right thing to do. I tried adding a "hi65xx-powerkey" entry in the
> hi655x_pmic_devs, but then mfd_add_devices doesn't seem to provide any
> of the irq info from the DT.
> 
> The only way I can think to make it work w/o calling to populate
> sub-nodes is to pull the powerkey subnode out of the pmic node. It
> seems to work, but I'm not sure if it properly describes the hardware
> then.  Sorry again for being a bit clueless here.

Since we use mfd_add_devices() already, you need to create a cell in
this file for the power-key driver and omit it from DT altogether.  If
your priority is to have each sub-device represented in DT, then you
need to stop using mfd_add_devices() and have of_platform_populate()
do your bidding.

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

[toc] | [prev] | [next] | [standalone]

#1411551 — [RFC][PATCH 3/5] hi655x-pmic: Fixup issue with un-acked interrupts

While trying to get the powerkey to function, I found
when pressing the key, I would get infinitely repeating
interrupts.

After digging around a bit, it seems we didn't set the
ack_base value for the regmap irqchip logic, so nothing
was acking the interrupt.

This patch adds the ack_base, which seems to make things
work.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/mfd/hi655x-pmic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/hi655x-pmic.c b/drivers/mfd/hi655x-pmic.c
index 3511035..4a3fdbc 100644
--- a/drivers/mfd/hi655x-pmic.c
+++ b/drivers/mfd/hi655x-pmic.c
@@ -50,6 +50,7 @@ static const struct regmap_irq_chip hi655x_irq_chip = {
 	.num_regs = 1,
 	.num_irqs = ARRAY_SIZE(hi655x_irqs),
 	.status_base = HI655X_IRQ_STAT_BASE,
+	.ack_base = HI655X_IRQ_STAT_BASE,
 	.mask_base = HI655X_IRQ_MASK_BASE,
 };
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1417529 — Re: [RFC][PATCH 3/5] hi655x-pmic: Fixup issue with un-acked interrupts

On Wed, 01 Jun 2016, John Stultz wrote:

> While trying to get the powerkey to function, I found
> when pressing the key, I would get infinitely repeating
> interrupts.
> 
> After digging around a bit, it seems we didn't set the
> ack_base value for the regmap irqchip logic, so nothing
> was acking the interrupt.
> 
> This patch adds the ack_base, which seems to make things
> work.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  drivers/mfd/hi655x-pmic.c | 1 +
>  1 file changed, 1 insertion(+)

Seems reasonable.

Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/drivers/mfd/hi655x-pmic.c b/drivers/mfd/hi655x-pmic.c
> index 3511035..4a3fdbc 100644
> --- a/drivers/mfd/hi655x-pmic.c
> +++ b/drivers/mfd/hi655x-pmic.c
> @@ -50,6 +50,7 @@ static const struct regmap_irq_chip hi655x_irq_chip = {
>  	.num_regs = 1,
>  	.num_irqs = ARRAY_SIZE(hi655x_irqs),
>  	.status_base = HI655X_IRQ_STAT_BASE,
> +	.ack_base = HI655X_IRQ_STAT_BASE,
>  	.mask_base = HI655X_IRQ_MASK_BASE,
>  };
>  

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web