All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288
@ 2016-12-14 13:52 Hans de Goede
  2016-12-14 13:52 ` [PATCH 2/5] mfd: axp20x: Add missing axp288 irqs Hans de Goede
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Hans de Goede @ 2016-12-14 13:52 UTC (permalink / raw)
  To: Lee Jones, Chen-Yu Tsai
  Cc: russianneuromancer @ ya . ru, linux-i2c, Hans de Goede

The interrupt line of the entire family of axp2xx pmics is active-low,
for devicetree enumerated irqs, this is dealt with in the devicetree.

ACPI irq resources have a flag field for this too, I tried using this
on my CUBE iwork8 Air tablet, but it does not contain the right data.

The dstd shows the irq listed as either ActiveLow or ActiveHigh,
depending on the OSID variable, which seems to be set by the
"OS IMAGE ID" in the BIOS/EFI setup screen.

Since the acpi-resource info is no good, simply pass in IRQF_TRIGGER_LOW
on the axp288.

Together with the other axp288 fixes in this series, this fixes the axp288
irq contineously triggering.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mfd/axp20x.c       | 6 +++---
 include/linux/mfd/axp20x.h | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index ba130be..3d76941 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -800,6 +800,7 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
 		axp20x->nr_cells = ARRAY_SIZE(axp288_cells);
 		axp20x->regmap_cfg = &axp288_regmap_config;
 		axp20x->regmap_irq_chip = &axp288_regmap_irq_chip;
+		axp20x->irq_flags = IRQF_TRIGGER_LOW;
 		break;
 	case AXP806_ID:
 		axp20x->nr_cells = ARRAY_SIZE(axp806_cells);
@@ -829,9 +830,8 @@ int axp20x_device_probe(struct axp20x_dev *axp20x)
 	int ret;
 
 	ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq,
-				  IRQF_ONESHOT | IRQF_SHARED, -1,
-				  axp20x->regmap_irq_chip,
-				  &axp20x->regmap_irqc);
+			  IRQF_ONESHOT | IRQF_SHARED | axp20x->irq_flags,
+			   -1, axp20x->regmap_irq_chip, &axp20x->regmap_irqc);
 	if (ret) {
 		dev_err(axp20x->dev, "failed to add irq chip: %d\n", ret);
 		return ret;
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index fec597f..199cce3 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -517,6 +517,7 @@ enum axp809_irqs {
 struct axp20x_dev {
 	struct device			*dev;
 	int				irq;
+	unsigned long			irq_flags;
 	struct regmap			*regmap;
 	struct regmap_irq_chip_data	*regmap_irqc;
 	long				variant;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/5] mfd: axp20x: Add missing axp288 irqs
  2016-12-14 13:52 [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288 Hans de Goede
@ 2016-12-14 13:52 ` Hans de Goede
  2016-12-14 14:21   ` Chen-Yu Tsai
  2017-01-03 17:54   ` Lee Jones
  2016-12-14 13:52 ` [PATCH 3/5] mfd: axp20x: Fix axp288 PEK_DBR and PEK_DBF irqs being swapped Hans de Goede
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: Hans de Goede @ 2016-12-14 13:52 UTC (permalink / raw)
  To: Lee Jones, Chen-Yu Tsai
  Cc: russianneuromancer @ ya . ru, linux-i2c, Hans de Goede

The axp288 has the following irqs 2 times: VBUS_FALL, VBUS_RISE,
VBUS_OV. On boot / reset the enable flags for both the normal and alt
version of these irqs is set.

Since we were only listing the normal version in the axp288 regmap_irq
struct, we were never disabling the alt versions of these irqs.

Add the alt versions to the axp288 regmap_irq struct, so that these
get properly disabled.

Together with the other axp288 fixes in this series, this fixes the axp288
irq contineously triggering.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mfd/axp20x.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 3d76941..9a81659 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -405,6 +405,9 @@ static const struct regmap_irq axp288_regmap_irqs[] = {
 	INIT_REGMAP_IRQ(AXP288, VBUS_FALL,              0, 2),
 	INIT_REGMAP_IRQ(AXP288, VBUS_RISE,              0, 3),
 	INIT_REGMAP_IRQ(AXP288, OV,                     0, 4),
+	INIT_REGMAP_IRQ(AXP288, FALLING_ALT,            0, 5),
+	INIT_REGMAP_IRQ(AXP288, RISING_ALT,             0, 6),
+	INIT_REGMAP_IRQ(AXP288, OV_ALT,                 0, 7),
 
 	INIT_REGMAP_IRQ(AXP288, DONE,                   1, 2),
 	INIT_REGMAP_IRQ(AXP288, CHARGING,               1, 3),
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/5] mfd: axp20x: Fix axp288 PEK_DBR and PEK_DBF irqs being swapped
  2016-12-14 13:52 [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288 Hans de Goede
  2016-12-14 13:52 ` [PATCH 2/5] mfd: axp20x: Add missing axp288 irqs Hans de Goede
@ 2016-12-14 13:52 ` Hans de Goede
  2016-12-14 14:28   ` Chen-Yu Tsai
  2017-01-03 17:54   ` Lee Jones
  2016-12-14 13:52 ` [PATCH 4/5] mfd: axp20x: Drop wrong AXP288_PMIC_ADC_EN define Hans de Goede
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: Hans de Goede @ 2016-12-14 13:52 UTC (permalink / raw)
  To: Lee Jones, Chen-Yu Tsai
  Cc: russianneuromancer @ ya . ru, linux-i2c, Hans de Goede

The R in PEK_DBR stands for rising, so it should be mapped to
AXP288_IRQ_POKP where the last P stands for positive edge.

Likewise PEK_DBF should be mapped to the falling edge, aka the
_N_egative edge, so it should be mapped to AXP288_IRQ_POKN.

This fixes the inverted powerbutton status reporting by the
axp20x-pek driver.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mfd/axp20x.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 9a81659..a294121 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -205,14 +205,14 @@ static struct resource axp22x_pek_resources[] = {
 static struct resource axp288_power_button_resources[] = {
 	{
 		.name	= "PEK_DBR",
-		.start	= AXP288_IRQ_POKN,
-		.end	= AXP288_IRQ_POKN,
+		.start	= AXP288_IRQ_POKP,
+		.end	= AXP288_IRQ_POKP,
 		.flags	= IORESOURCE_IRQ,
 	},
 	{
 		.name	= "PEK_DBF",
-		.start	= AXP288_IRQ_POKP,
-		.end	= AXP288_IRQ_POKP,
+		.start	= AXP288_IRQ_POKN,
+		.end	= AXP288_IRQ_POKN,
 		.flags	= IORESOURCE_IRQ,
 	},
 };
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/5] mfd: axp20x: Drop wrong AXP288_PMIC_ADC_EN define
  2016-12-14 13:52 [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288 Hans de Goede
  2016-12-14 13:52 ` [PATCH 2/5] mfd: axp20x: Add missing axp288 irqs Hans de Goede
  2016-12-14 13:52 ` [PATCH 3/5] mfd: axp20x: Fix axp288 PEK_DBR and PEK_DBF irqs being swapped Hans de Goede
@ 2016-12-14 13:52 ` Hans de Goede
  2016-12-14 14:29   ` Chen-Yu Tsai
  2017-01-03 17:54   ` Lee Jones
  2016-12-14 13:52 ` [PATCH 5/5] mfd: axp20x: Fix axp288 volatile ranges Hans de Goede
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: Hans de Goede @ 2016-12-14 13:52 UTC (permalink / raw)
  To: Lee Jones, Chen-Yu Tsai
  Cc: russianneuromancer @ ya . ru, linux-i2c, Hans de Goede

The adc-enable register for the axp288 is 0x82, not 0x84.
0x82 is already defined as AXP20X_ADC_EN1 and that is what the
axp288_adc driver is actually using, so simply drop the wrong
AXP288_PMIC_ADC_EN define.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 include/linux/mfd/axp20x.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 199cce3..fe93e00 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -232,7 +232,6 @@ enum {
 #define AXP288_PMIC_ADC_H               0x56
 #define AXP288_PMIC_ADC_L               0x57
 #define AXP288_ADC_TS_PIN_CTRL          0x84
-#define AXP288_PMIC_ADC_EN              0x84
 
 /* Fuel Gauge */
 #define AXP288_FG_RDC1_REG          0xba
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/5] mfd: axp20x: Fix axp288 volatile ranges
  2016-12-14 13:52 [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288 Hans de Goede
                   ` (2 preceding siblings ...)
  2016-12-14 13:52 ` [PATCH 4/5] mfd: axp20x: Drop wrong AXP288_PMIC_ADC_EN define Hans de Goede
@ 2016-12-14 13:52 ` Hans de Goede
  2016-12-14 14:41   ` Chen-Yu Tsai
  2016-12-14 14:19 ` [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288 Chen-Yu Tsai
  2017-01-03 17:53 ` Lee Jones
  5 siblings, 1 reply; 16+ messages in thread
From: Hans de Goede @ 2016-12-14 13:52 UTC (permalink / raw)
  To: Lee Jones, Chen-Yu Tsai
  Cc: russianneuromancer @ ya . ru, linux-i2c, Hans de Goede

The axp288 pmic has a lot more volatile registers then we were
listing in axp288_volatile_ranges, fix this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mfd/axp20x.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index a294121..b9c1adf 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -117,7 +117,10 @@ static const struct regmap_range axp288_writeable_ranges[] = {
 };
 
 static const struct regmap_range axp288_volatile_ranges[] = {
+	regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP20X_PWR_OP_MODE),
 	regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IPSOUT_V_HIGH_L),
+	regmap_reg_range(AXP22X_GPIO_STATE, AXP22X_GPIO_STATE),
+	regmap_reg_range(AXP20X_FG_RES, AXP288_FG_CC_CAP_REG),
 };
 
 static const struct regmap_access_table axp288_writeable_table = {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288
  2016-12-14 13:52 [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288 Hans de Goede
                   ` (3 preceding siblings ...)
  2016-12-14 13:52 ` [PATCH 5/5] mfd: axp20x: Fix axp288 volatile ranges Hans de Goede
@ 2016-12-14 14:19 ` Chen-Yu Tsai
  2017-01-03 17:53 ` Lee Jones
  5 siblings, 0 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2016-12-14 14:19 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Chen-Yu Tsai, russianneuromancer @ ya . ru, linux-i2c

On Wed, Dec 14, 2016 at 9:52 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> The interrupt line of the entire family of axp2xx pmics is active-low,
> for devicetree enumerated irqs, this is dealt with in the devicetree.
>
> ACPI irq resources have a flag field for this too, I tried using this
> on my CUBE iwork8 Air tablet, but it does not contain the right data.
>
> The dstd shows the irq listed as either ActiveLow or ActiveHigh,
> depending on the OSID variable, which seems to be set by the
> "OS IMAGE ID" in the BIOS/EFI setup screen.
>
> Since the acpi-resource info is no good, simply pass in IRQF_TRIGGER_LOW
> on the axp288.
>
> Together with the other axp288 fixes in this series, this fixes the axp288
> irq contineously triggering.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

This patch looks good to me. However, I do not have any AXP288 hardware
to test it on, nor am I familiar with the ACPI stuff.

Acked-by: Chen-Yu Tsai <wens@csie.org>

P.S. I don't think we're handling IRQ trigger types at all. The hardware
default for the NMI interrupt in the Allwinner chips just happens to be
active low.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] mfd: axp20x: Add missing axp288 irqs
  2016-12-14 13:52 ` [PATCH 2/5] mfd: axp20x: Add missing axp288 irqs Hans de Goede
@ 2016-12-14 14:21   ` Chen-Yu Tsai
  2017-01-03 17:54   ` Lee Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2016-12-14 14:21 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Chen-Yu Tsai, russianneuromancer @ ya . ru, linux-i2c

On Wed, Dec 14, 2016 at 9:52 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> The axp288 has the following irqs 2 times: VBUS_FALL, VBUS_RISE,
> VBUS_OV. On boot / reset the enable flags for both the normal and alt
> version of these irqs is set.
>
> Since we were only listing the normal version in the axp288 regmap_irq
> struct, we were never disabling the alt versions of these irqs.
>
> Add the alt versions to the axp288 regmap_irq struct, so that these
> get properly disabled.
>
> Together with the other axp288 fixes in this series, this fixes the axp288
> irq contineously triggering.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/5] mfd: axp20x: Fix axp288 PEK_DBR and PEK_DBF irqs being swapped
  2016-12-14 13:52 ` [PATCH 3/5] mfd: axp20x: Fix axp288 PEK_DBR and PEK_DBF irqs being swapped Hans de Goede
@ 2016-12-14 14:28   ` Chen-Yu Tsai
  2017-01-03 17:54   ` Lee Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2016-12-14 14:28 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Chen-Yu Tsai, russianneuromancer @ ya . ru, linux-i2c

On Wed, Dec 14, 2016 at 9:52 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> The R in PEK_DBR stands for rising, so it should be mapped to
> AXP288_IRQ_POKP where the last P stands for positive edge.
>
> Likewise PEK_DBF should be mapped to the falling edge, aka the
> _N_egative edge, so it should be mapped to AXP288_IRQ_POKN.
>
> This fixes the inverted powerbutton status reporting by the
> axp20x-pek driver.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] mfd: axp20x: Drop wrong AXP288_PMIC_ADC_EN define
  2016-12-14 13:52 ` [PATCH 4/5] mfd: axp20x: Drop wrong AXP288_PMIC_ADC_EN define Hans de Goede
@ 2016-12-14 14:29   ` Chen-Yu Tsai
  2017-01-03 17:54   ` Lee Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2016-12-14 14:29 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Chen-Yu Tsai, russianneuromancer @ ya . ru, linux-i2c

On Wed, Dec 14, 2016 at 9:52 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> The adc-enable register for the axp288 is 0x82, not 0x84.
> 0x82 is already defined as AXP20X_ADC_EN1 and that is what the
> axp288_adc driver is actually using, so simply drop the wrong
> AXP288_PMIC_ADC_EN define.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] mfd: axp20x: Fix axp288 volatile ranges
  2016-12-14 13:52 ` [PATCH 5/5] mfd: axp20x: Fix axp288 volatile ranges Hans de Goede
@ 2016-12-14 14:41   ` Chen-Yu Tsai
  2016-12-14 15:07     ` Hans de Goede
  2016-12-15 15:44     ` Hans de Goede
  0 siblings, 2 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2016-12-14 14:41 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Chen-Yu Tsai, russianneuromancer @ ya . ru, linux-i2c

On Wed, Dec 14, 2016 at 9:52 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> The axp288 pmic has a lot more volatile registers then we were
> listing in axp288_volatile_ranges, fix this.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mfd/axp20x.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index a294121..b9c1adf 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -117,7 +117,10 @@ static const struct regmap_range axp288_writeable_ranges[] = {
>  };
>
>  static const struct regmap_range axp288_volatile_ranges[] = {
> +       regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP20X_PWR_OP_MODE),

Actually register 0x02 is volatile as well. Various fields say "write
1 to clear".
You might need a new define for it though, as the usage is different.

>         regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IPSOUT_V_HIGH_L),
> +       regmap_reg_range(AXP22X_GPIO_STATE, AXP22X_GPIO_STATE),
> +       regmap_reg_range(AXP20X_FG_RES, AXP288_FG_CC_CAP_REG),

Could you also add AXP20X_TIMER_CTRL and 0xa0 ~ 0xa1 (real time
battery voltage),
if you're adding defines?

Thanks
ChenYu

>  };
>
>  static const struct regmap_access_table axp288_writeable_table = {
> --
> 2.9.3
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] mfd: axp20x: Fix axp288 volatile ranges
  2016-12-14 14:41   ` Chen-Yu Tsai
@ 2016-12-14 15:07     ` Hans de Goede
  2016-12-15 15:44     ` Hans de Goede
  1 sibling, 0 replies; 16+ messages in thread
From: Hans de Goede @ 2016-12-14 15:07 UTC (permalink / raw)
  To: Chen-Yu Tsai; +Cc: Lee Jones, russianneuromancer @ ya . ru, linux-i2c

Hi,

On 14-12-16 15:41, Chen-Yu Tsai wrote:
> On Wed, Dec 14, 2016 at 9:52 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> The axp288 pmic has a lot more volatile registers then we were
>> listing in axp288_volatile_ranges, fix this.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/mfd/axp20x.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index a294121..b9c1adf 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
>> @@ -117,7 +117,10 @@ static const struct regmap_range axp288_writeable_ranges[] = {
>>  };
>>
>>  static const struct regmap_range axp288_volatile_ranges[] = {
>> +       regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP20X_PWR_OP_MODE),
>
> Actually register 0x02 is volatile as well. Various fields say "write
> 1 to clear".
> You might need a new define for it though, as the usage is different.
>
>>         regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IPSOUT_V_HIGH_L),
>> +       regmap_reg_range(AXP22X_GPIO_STATE, AXP22X_GPIO_STATE),
>> +       regmap_reg_range(AXP20X_FG_RES, AXP288_FG_CC_CAP_REG),
>
> Could you also add AXP20X_TIMER_CTRL and 0xa0 ~ 0xa1 (real time
> battery voltage),
> if you're adding defines?

Hmm, ok, I'll do 2 new patches then 1 adding the defines and a v2
of this one. Lee, can you pick up 1 - 4 as they currently are
please? Then I'll just send these 2 new patches.

Regards,

Hans

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] mfd: axp20x: Fix axp288 volatile ranges
  2016-12-14 14:41   ` Chen-Yu Tsai
  2016-12-14 15:07     ` Hans de Goede
@ 2016-12-15 15:44     ` Hans de Goede
  1 sibling, 0 replies; 16+ messages in thread
From: Hans de Goede @ 2016-12-15 15:44 UTC (permalink / raw)
  To: Chen-Yu Tsai; +Cc: Lee Jones, russianneuromancer @ ya . ru, linux-i2c

Hi,

On 14-12-16 15:41, Chen-Yu Tsai wrote:
> On Wed, Dec 14, 2016 at 9:52 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> The axp288 pmic has a lot more volatile registers then we were
>> listing in axp288_volatile_ranges, fix this.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/mfd/axp20x.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index a294121..b9c1adf 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
>> @@ -117,7 +117,10 @@ static const struct regmap_range axp288_writeable_ranges[] = {
>>  };
>>
>>  static const struct regmap_range axp288_volatile_ranges[] = {
>> +       regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP20X_PWR_OP_MODE),
>
> Actually register 0x02 is volatile as well. Various fields say "write
> 1 to clear".
> You might need a new define for it though, as the usage is different.
>
>>         regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IPSOUT_V_HIGH_L),
>> +       regmap_reg_range(AXP22X_GPIO_STATE, AXP22X_GPIO_STATE),
>> +       regmap_reg_range(AXP20X_FG_RES, AXP288_FG_CC_CAP_REG),
>
> Could you also add AXP20X_TIMER_CTRL and 0xa0 ~ 0xa1 (real time
> battery voltage),
> if you're adding defines?

We also need to add 0xbc "BC Detect Status Register" to be able
to get the results of the charger detect logic in the axp288.

Self-nak for v2, I will send a v3 later.

Regards,

Hans

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288
  2016-12-14 13:52 [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288 Hans de Goede
                   ` (4 preceding siblings ...)
  2016-12-14 14:19 ` [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288 Chen-Yu Tsai
@ 2017-01-03 17:53 ` Lee Jones
  5 siblings, 0 replies; 16+ messages in thread
From: Lee Jones @ 2017-01-03 17:53 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Chen-Yu Tsai, russianneuromancer @ ya . ru, linux-i2c

On Wed, 14 Dec 2016, Hans de Goede wrote:

> The interrupt line of the entire family of axp2xx pmics is active-low,
> for devicetree enumerated irqs, this is dealt with in the devicetree.
> 
> ACPI irq resources have a flag field for this too, I tried using this
> on my CUBE iwork8 Air tablet, but it does not contain the right data.
> 
> The dstd shows the irq listed as either ActiveLow or ActiveHigh,
> depending on the OSID variable, which seems to be set by the
> "OS IMAGE ID" in the BIOS/EFI setup screen.
> 
> Since the acpi-resource info is no good, simply pass in IRQF_TRIGGER_LOW
> on the axp288.
> 
> Together with the other axp288 fixes in this series, this fixes the axp288
> irq contineously triggering.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mfd/axp20x.c       | 6 +++---
>  include/linux/mfd/axp20x.h | 1 +
>  2 files changed, 4 insertions(+), 3 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index ba130be..3d76941 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -800,6 +800,7 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
>  		axp20x->nr_cells = ARRAY_SIZE(axp288_cells);
>  		axp20x->regmap_cfg = &axp288_regmap_config;
>  		axp20x->regmap_irq_chip = &axp288_regmap_irq_chip;
> +		axp20x->irq_flags = IRQF_TRIGGER_LOW;
>  		break;
>  	case AXP806_ID:
>  		axp20x->nr_cells = ARRAY_SIZE(axp806_cells);
> @@ -829,9 +830,8 @@ int axp20x_device_probe(struct axp20x_dev *axp20x)
>  	int ret;
>  
>  	ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq,
> -				  IRQF_ONESHOT | IRQF_SHARED, -1,
> -				  axp20x->regmap_irq_chip,
> -				  &axp20x->regmap_irqc);
> +			  IRQF_ONESHOT | IRQF_SHARED | axp20x->irq_flags,
> +			   -1, axp20x->regmap_irq_chip, &axp20x->regmap_irqc);
>  	if (ret) {
>  		dev_err(axp20x->dev, "failed to add irq chip: %d\n", ret);
>  		return ret;
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index fec597f..199cce3 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -517,6 +517,7 @@ enum axp809_irqs {
>  struct axp20x_dev {
>  	struct device			*dev;
>  	int				irq;
> +	unsigned long			irq_flags;
>  	struct regmap			*regmap;
>  	struct regmap_irq_chip_data	*regmap_irqc;
>  	long				variant;

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] mfd: axp20x: Add missing axp288 irqs
  2016-12-14 13:52 ` [PATCH 2/5] mfd: axp20x: Add missing axp288 irqs Hans de Goede
  2016-12-14 14:21   ` Chen-Yu Tsai
@ 2017-01-03 17:54   ` Lee Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Lee Jones @ 2017-01-03 17:54 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Chen-Yu Tsai, russianneuromancer @ ya . ru, linux-i2c

On Wed, 14 Dec 2016, Hans de Goede wrote:

> The axp288 has the following irqs 2 times: VBUS_FALL, VBUS_RISE,
> VBUS_OV. On boot / reset the enable flags for both the normal and alt
> version of these irqs is set.
> 
> Since we were only listing the normal version in the axp288 regmap_irq
> struct, we were never disabling the alt versions of these irqs.
> 
> Add the alt versions to the axp288 regmap_irq struct, so that these
> get properly disabled.
> 
> Together with the other axp288 fixes in this series, this fixes the axp288
> irq contineously triggering.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mfd/axp20x.c | 3 +++
>  1 file changed, 3 insertions(+)

Applied, thanks.

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 3d76941..9a81659 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -405,6 +405,9 @@ static const struct regmap_irq axp288_regmap_irqs[] = {
>  	INIT_REGMAP_IRQ(AXP288, VBUS_FALL,              0, 2),
>  	INIT_REGMAP_IRQ(AXP288, VBUS_RISE,              0, 3),
>  	INIT_REGMAP_IRQ(AXP288, OV,                     0, 4),
> +	INIT_REGMAP_IRQ(AXP288, FALLING_ALT,            0, 5),
> +	INIT_REGMAP_IRQ(AXP288, RISING_ALT,             0, 6),
> +	INIT_REGMAP_IRQ(AXP288, OV_ALT,                 0, 7),
>  
>  	INIT_REGMAP_IRQ(AXP288, DONE,                   1, 2),
>  	INIT_REGMAP_IRQ(AXP288, CHARGING,               1, 3),

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/5] mfd: axp20x: Fix axp288 PEK_DBR and PEK_DBF irqs being swapped
  2016-12-14 13:52 ` [PATCH 3/5] mfd: axp20x: Fix axp288 PEK_DBR and PEK_DBF irqs being swapped Hans de Goede
  2016-12-14 14:28   ` Chen-Yu Tsai
@ 2017-01-03 17:54   ` Lee Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Lee Jones @ 2017-01-03 17:54 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Chen-Yu Tsai, russianneuromancer @ ya . ru, linux-i2c

On Wed, 14 Dec 2016, Hans de Goede wrote:

> The R in PEK_DBR stands for rising, so it should be mapped to
> AXP288_IRQ_POKP where the last P stands for positive edge.
> 
> Likewise PEK_DBF should be mapped to the falling edge, aka the
> _N_egative edge, so it should be mapped to AXP288_IRQ_POKN.
> 
> This fixes the inverted powerbutton status reporting by the
> axp20x-pek driver.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mfd/axp20x.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 9a81659..a294121 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -205,14 +205,14 @@ static struct resource axp22x_pek_resources[] = {
>  static struct resource axp288_power_button_resources[] = {
>  	{
>  		.name	= "PEK_DBR",
> -		.start	= AXP288_IRQ_POKN,
> -		.end	= AXP288_IRQ_POKN,
> +		.start	= AXP288_IRQ_POKP,
> +		.end	= AXP288_IRQ_POKP,
>  		.flags	= IORESOURCE_IRQ,
>  	},
>  	{
>  		.name	= "PEK_DBF",
> -		.start	= AXP288_IRQ_POKP,
> -		.end	= AXP288_IRQ_POKP,
> +		.start	= AXP288_IRQ_POKN,
> +		.end	= AXP288_IRQ_POKN,
>  		.flags	= IORESOURCE_IRQ,
>  	},
>  };

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] mfd: axp20x: Drop wrong AXP288_PMIC_ADC_EN define
  2016-12-14 13:52 ` [PATCH 4/5] mfd: axp20x: Drop wrong AXP288_PMIC_ADC_EN define Hans de Goede
  2016-12-14 14:29   ` Chen-Yu Tsai
@ 2017-01-03 17:54   ` Lee Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Lee Jones @ 2017-01-03 17:54 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Chen-Yu Tsai, russianneuromancer @ ya . ru, linux-i2c

On Wed, 14 Dec 2016, Hans de Goede wrote:

> The adc-enable register for the axp288 is 0x82, not 0x84.
> 0x82 is already defined as AXP20X_ADC_EN1 and that is what the
> axp288_adc driver is actually using, so simply drop the wrong
> AXP288_PMIC_ADC_EN define.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  include/linux/mfd/axp20x.h | 1 -
>  1 file changed, 1 deletion(-)

Applied, thanks.

> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 199cce3..fe93e00 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -232,7 +232,6 @@ enum {
>  #define AXP288_PMIC_ADC_H               0x56
>  #define AXP288_PMIC_ADC_L               0x57
>  #define AXP288_ADC_TS_PIN_CTRL          0x84
> -#define AXP288_PMIC_ADC_EN              0x84
>  
>  /* Fuel Gauge */
>  #define AXP288_FG_RDC1_REG          0xba

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-01-03 17:50 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-14 13:52 [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288 Hans de Goede
2016-12-14 13:52 ` [PATCH 2/5] mfd: axp20x: Add missing axp288 irqs Hans de Goede
2016-12-14 14:21   ` Chen-Yu Tsai
2017-01-03 17:54   ` Lee Jones
2016-12-14 13:52 ` [PATCH 3/5] mfd: axp20x: Fix axp288 PEK_DBR and PEK_DBF irqs being swapped Hans de Goede
2016-12-14 14:28   ` Chen-Yu Tsai
2017-01-03 17:54   ` Lee Jones
2016-12-14 13:52 ` [PATCH 4/5] mfd: axp20x: Drop wrong AXP288_PMIC_ADC_EN define Hans de Goede
2016-12-14 14:29   ` Chen-Yu Tsai
2017-01-03 17:54   ` Lee Jones
2016-12-14 13:52 ` [PATCH 5/5] mfd: axp20x: Fix axp288 volatile ranges Hans de Goede
2016-12-14 14:41   ` Chen-Yu Tsai
2016-12-14 15:07     ` Hans de Goede
2016-12-15 15:44     ` Hans de Goede
2016-12-14 14:19 ` [PATCH 1/5] mfd: axp20x: Use IRQF_TRIGGER_LOW on the axp288 Chen-Yu Tsai
2017-01-03 17:53 ` Lee Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.