[PATCH 0/3] mfd: bd718x7: Set button press duration via dt only

[PATCH 0/3] mfd: bd718x7: Set button press duration via dt only

[Leonard Crestez]

The reset button on imx8mm-evk is tied to PWRON_B pin of bd71847 and the
long press duration is set to zero from OTP. The linux driver overrides
those values and breaks reset from button.

Overwriting OTP or bootloader configuration with some hardcoded defaults
is not desirable, power button should behave identically from bootloader
to kernel.

Add DT properties to allow explicitly overriding press durations but
don't do anything by default.

Leonard Crestez (3):
  mfd: bd718x7: Remove hardcoded config for button press duration
  dt-bindings: mfd: Document short/long press duration for bd718x7
  mfd: bd718x7: Make power button press duration configurable

 .../bindings/mfd/rohm,bd71837-pmic.txt        |  8 +++
 drivers/mfd/rohm-bd718x7.c                    | 61 ++++++++++++-------
 2 files changed, 48 insertions(+), 21 deletions(-)

-- 
2.17.1

[PATCH 0/3] mfd: bd718x7: Set button press duration via dt only

[Leonard Crestez]

The reset button on imx8mm-evk is tied to PWRON_B pin of bd71847 and the
long press duration is set to zero from OTP. The linux driver overrides
those values and breaks reset from button.

Overwriting OTP or bootloader configuration with some hardcoded defaults
is not desirable, power button should behave identically from bootloader
to kernel.

Add DT properties to allow explicitly overriding press durations but
don't do anything by default.

Leonard Crestez (3):
  mfd: bd718x7: Remove hardcoded config for button press duration
  dt-bindings: mfd: Document short/long press duration for bd718x7
  mfd: bd718x7: Make power button press duration configurable

 .../bindings/mfd/rohm,bd71837-pmic.txt        |  8 +++
 drivers/mfd/rohm-bd718x7.c                    | 61 ++++++++++++-------
 2 files changed, 48 insertions(+), 21 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] mfd: bd718x7: Remove hardcoded config for button press duration

[Leonard Crestez]

The reset button on imx8mm-evk is tied to PWRON_B pin of bd71847 and the
long press duration is set to zero from OTP. The linux driver overrides
those values and breaks reset from button.

Overwriting OTP or bootloader configuration with some hardcoded defaults
is not desirable, keep already programmed values instead.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/mfd/rohm-bd718x7.c | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
index a29d529a96f4..cdbef83884f0 100644
--- a/drivers/mfd/rohm-bd718x7.c
+++ b/drivers/mfd/rohm-bd718x7.c
@@ -115,33 +115,10 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c,
 	if (ret) {
 		dev_err(&i2c->dev, "Failed to add irq_chip\n");
 		return ret;
 	}
 
-	/* Configure short press to 10 milliseconds */
-	ret = regmap_update_bits(bd718xx->regmap,
-				 BD718XX_REG_PWRONCONFIG0,
-				 BD718XX_PWRBTN_PRESS_DURATION_MASK,
-				 BD718XX_PWRBTN_SHORT_PRESS_10MS);
-	if (ret) {
-		dev_err(&i2c->dev,
-			"Failed to configure button short press timeout\n");
-		return ret;
-	}
-
-	/* Configure long press to 10 seconds */
-	ret = regmap_update_bits(bd718xx->regmap,
-				 BD718XX_REG_PWRONCONFIG1,
-				 BD718XX_PWRBTN_PRESS_DURATION_MASK,
-				 BD718XX_PWRBTN_LONG_PRESS_10S);
-
-	if (ret) {
-		dev_err(&i2c->dev,
-			"Failed to configure button long press timeout\n");
-		return ret;
-	}
-
 	ret = regmap_irq_get_virq(bd718xx->irq_data, BD718XX_INT_PWRBTN_S);
 
 	if (ret < 0) {
 		dev_err(&i2c->dev, "Failed to get the IRQ\n");
 		return ret;
-- 
2.17.1

[PATCH 1/3] mfd: bd718x7: Remove hardcoded config for button press duration

[Leonard Crestez]

The reset button on imx8mm-evk is tied to PWRON_B pin of bd71847 and the
long press duration is set to zero from OTP. The linux driver overrides
those values and breaks reset from button.

Overwriting OTP or bootloader configuration with some hardcoded defaults
is not desirable, keep already programmed values instead.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/mfd/rohm-bd718x7.c | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
index a29d529a96f4..cdbef83884f0 100644
--- a/drivers/mfd/rohm-bd718x7.c
+++ b/drivers/mfd/rohm-bd718x7.c
@@ -115,33 +115,10 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c,
 	if (ret) {
 		dev_err(&i2c->dev, "Failed to add irq_chip\n");
 		return ret;
 	}
 
-	/* Configure short press to 10 milliseconds */
-	ret = regmap_update_bits(bd718xx->regmap,
-				 BD718XX_REG_PWRONCONFIG0,
-				 BD718XX_PWRBTN_PRESS_DURATION_MASK,
-				 BD718XX_PWRBTN_SHORT_PRESS_10MS);
-	if (ret) {
-		dev_err(&i2c->dev,
-			"Failed to configure button short press timeout\n");
-		return ret;
-	}
-
-	/* Configure long press to 10 seconds */
-	ret = regmap_update_bits(bd718xx->regmap,
-				 BD718XX_REG_PWRONCONFIG1,
-				 BD718XX_PWRBTN_PRESS_DURATION_MASK,
-				 BD718XX_PWRBTN_LONG_PRESS_10S);
-
-	if (ret) {
-		dev_err(&i2c->dev,
-			"Failed to configure button long press timeout\n");
-		return ret;
-	}
-
 	ret = regmap_irq_get_virq(bd718xx->irq_data, BD718XX_INT_PWRBTN_S);
 
 	if (ret < 0) {
 		dev_err(&i2c->dev, "Failed to get the IRQ\n");
 		return ret;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] mfd: bd718x7: Remove hardcoded config for button press duration

[Vaittinen, Matti]

On Mon, 2019-05-20 at 08:55 +0000, Leonard Crestez wrote:
> The reset button on imx8mm-evk is tied to PWRON_B pin of bd71847 and
> the
> long press duration is set to zero from OTP. The linux driver
> overrides
> those values and breaks reset from button.
> 
> Overwriting OTP or bootloader configuration with some hardcoded
> defaults
> is not desirable, keep already programmed values instead.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Acked-By: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Re: [PATCH 1/3] mfd: bd718x7: Remove hardcoded config for button press duration

[Vaittinen, Matti]

On Mon, 2019-05-20 at 08:55 +0000, Leonard Crestez wrote:
> The reset button on imx8mm-evk is tied to PWRON_B pin of bd71847 and
> the
> long press duration is set to zero from OTP. The linux driver
> overrides
> those values and breaks reset from button.
> 
> Overwriting OTP or bootloader configuration with some hardcoded
> defaults
> is not desirable, keep already programmed values instead.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Acked-By: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] dt-bindings: mfd: Document short/long press duration for bd718x7

[Leonard Crestez]

These valeus can be overwritten at probe time.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt         | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
index d5f68ac78d15..3649c1191432 100644
--- a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
@@ -36,10 +36,18 @@ switched power state to SNVS. If reset is done via READY state the power
 outputs will be returned to HW control by OTP loading. Thus the reset
 target state is set to READY by default. If SNVS state is used the boot
 crucial regulators must have the regulator-always-on and regulator-boot-on
 properties set in regulator node.
 
+- rohm,short-press-ms	: Short press duration in milliseconds
+- rohm,long-press-ms	: Long press duration in milliseconds
+
+Configure the "short press" and "long press" timers for the power button.
+Values are rounded to what hardware supports (500ms multiple for short and
+1000ms multiple for long). If these properties are not present the existing
+configuration (from bootloader or OTP) is not touched.
+
 Example:
 
 	/* external oscillator node */
 	osc: oscillator {
 		compatible = "fixed-clock";
-- 
2.17.1

[PATCH 2/3] dt-bindings: mfd: Document short/long press duration for bd718x7

[Leonard Crestez]

These valeus can be overwritten at probe time.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt         | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
index d5f68ac78d15..3649c1191432 100644
--- a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
@@ -36,10 +36,18 @@ switched power state to SNVS. If reset is done via READY state the power
 outputs will be returned to HW control by OTP loading. Thus the reset
 target state is set to READY by default. If SNVS state is used the boot
 crucial regulators must have the regulator-always-on and regulator-boot-on
 properties set in regulator node.
 
+- rohm,short-press-ms	: Short press duration in milliseconds
+- rohm,long-press-ms	: Long press duration in milliseconds
+
+Configure the "short press" and "long press" timers for the power button.
+Values are rounded to what hardware supports (500ms multiple for short and
+1000ms multiple for long). If these properties are not present the existing
+configuration (from bootloader or OTP) is not touched.
+
 Example:
 
 	/* external oscillator node */
 	osc: oscillator {
 		compatible = "fixed-clock";
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] dt-bindings: mfd: Document short/long press duration for bd718x7

[Vaittinen, Matti]

Thanks Leonard!

This is how it should've been done from the beginning =)

On Mon, 2019-05-20 at 08:55 +0000, Leonard Crestez wrote:
> These valeus can be overwritten at probe time.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Acked-By: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Re: [PATCH 2/3] dt-bindings: mfd: Document short/long press duration for bd718x7

[Vaittinen, Matti]

Thanks Leonard!

This is how it should've been done from the beginning =)

On Mon, 2019-05-20 at 08:55 +0000, Leonard Crestez wrote:
> These valeus can be overwritten at probe time.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Acked-By: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] mfd: bd718x7: Make power button press duration configurable

[Leonard Crestez]

Allow overwriting the values in BD718XX_REG_PWRONCONFIG0 and
BD718XX_REG_PWRONCONFIG1 via devicetree.

Keep existing values (from bootloader or OTP) if property is not
present.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/mfd/rohm-bd718x7.c | 42 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
index cdbef83884f0..dffa3f4bffad 100644
--- a/drivers/mfd/rohm-bd718x7.c
+++ b/drivers/mfd/rohm-bd718x7.c
@@ -79,10 +79,48 @@ static const struct regmap_config bd718xx_regmap_config = {
 	.volatile_table = &volatile_regs,
 	.max_register = BD718XX_MAX_REGISTER - 1,
 	.cache_type = REGCACHE_RBTREE,
 };
 
+static int bd718xx_init_press_duration(struct bd718xx *bd718xx)
+{
+	struct device* dev = bd718xx->dev;
+	u32 short_press_ms, long_press_ms;
+	u32 short_press_value, long_press_value;
+	int ret;
+
+	ret = of_property_read_u32(dev->of_node, "rohm,short-press-ms",
+				   &short_press_ms);
+	if (!ret) {
+		short_press_value = min(15u, short_press_ms / 500);
+		ret = regmap_update_bits(bd718xx->regmap,
+					 BD718XX_REG_PWRONCONFIG0,
+					 BD718XX_PWRBTN_PRESS_DURATION_MASK,
+					 short_press_value);
+		if (ret) {
+			dev_err(dev, "Failed to init pwron short press\n");
+			return ret;
+		}
+	}
+
+	ret = of_property_read_u32(dev->of_node, "rohm,long-press-ms",
+				   &long_press_ms);
+	if (!ret) {
+		long_press_value = min(15u, long_press_ms / 1000);
+		ret = regmap_update_bits(bd718xx->regmap,
+					 BD718XX_REG_PWRONCONFIG1,
+					 BD718XX_PWRBTN_PRESS_DURATION_MASK,
+					 long_press_value);
+		if (ret) {
+			dev_err(dev, "Failed to init pwron long press\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static int bd718xx_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
 	struct bd718xx *bd718xx;
 	int ret;
@@ -115,10 +153,14 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c,
 	if (ret) {
 		dev_err(&i2c->dev, "Failed to add irq_chip\n");
 		return ret;
 	}
 
+	ret = bd718xx_init_press_duration(bd718xx);
+	if (ret)
+		return ret;
+
 	ret = regmap_irq_get_virq(bd718xx->irq_data, BD718XX_INT_PWRBTN_S);
 
 	if (ret < 0) {
 		dev_err(&i2c->dev, "Failed to get the IRQ\n");
 		return ret;
-- 
2.17.1

[PATCH 3/3] mfd: bd718x7: Make power button press duration configurable

[Leonard Crestez]

Allow overwriting the values in BD718XX_REG_PWRONCONFIG0 and
BD718XX_REG_PWRONCONFIG1 via devicetree.

Keep existing values (from bootloader or OTP) if property is not
present.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/mfd/rohm-bd718x7.c | 42 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
index cdbef83884f0..dffa3f4bffad 100644
--- a/drivers/mfd/rohm-bd718x7.c
+++ b/drivers/mfd/rohm-bd718x7.c
@@ -79,10 +79,48 @@ static const struct regmap_config bd718xx_regmap_config = {
 	.volatile_table = &volatile_regs,
 	.max_register = BD718XX_MAX_REGISTER - 1,
 	.cache_type = REGCACHE_RBTREE,
 };
 
+static int bd718xx_init_press_duration(struct bd718xx *bd718xx)
+{
+	struct device* dev = bd718xx->dev;
+	u32 short_press_ms, long_press_ms;
+	u32 short_press_value, long_press_value;
+	int ret;
+
+	ret = of_property_read_u32(dev->of_node, "rohm,short-press-ms",
+				   &short_press_ms);
+	if (!ret) {
+		short_press_value = min(15u, short_press_ms / 500);
+		ret = regmap_update_bits(bd718xx->regmap,
+					 BD718XX_REG_PWRONCONFIG0,
+					 BD718XX_PWRBTN_PRESS_DURATION_MASK,
+					 short_press_value);
+		if (ret) {
+			dev_err(dev, "Failed to init pwron short press\n");
+			return ret;
+		}
+	}
+
+	ret = of_property_read_u32(dev->of_node, "rohm,long-press-ms",
+				   &long_press_ms);
+	if (!ret) {
+		long_press_value = min(15u, long_press_ms / 1000);
+		ret = regmap_update_bits(bd718xx->regmap,
+					 BD718XX_REG_PWRONCONFIG1,
+					 BD718XX_PWRBTN_PRESS_DURATION_MASK,
+					 long_press_value);
+		if (ret) {
+			dev_err(dev, "Failed to init pwron long press\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static int bd718xx_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
 	struct bd718xx *bd718xx;
 	int ret;
@@ -115,10 +153,14 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c,
 	if (ret) {
 		dev_err(&i2c->dev, "Failed to add irq_chip\n");
 		return ret;
 	}
 
+	ret = bd718xx_init_press_duration(bd718xx);
+	if (ret)
+		return ret;
+
 	ret = regmap_irq_get_virq(bd718xx->irq_data, BD718XX_INT_PWRBTN_S);
 
 	if (ret < 0) {
 		dev_err(&i2c->dev, "Failed to get the IRQ\n");
 		return ret;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] mfd: bd718x7: Make power button press duration configurable

[Vaittinen, Matti]

Thanks for implementing this Leonard. This is how it should have been
done right away. Just one little thing.

On Mon, 2019-05-20 at 08:55 +0000, Leonard Crestez wrote:
> Allow overwriting the values in BD718XX_REG_PWRONCONFIG0 and
> BD718XX_REG_PWRONCONFIG1 via devicetree.
> 
> Keep existing values (from bootloader or OTP) if property is not
> present.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/mfd/rohm-bd718x7.c | 42
> ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
> index cdbef83884f0..dffa3f4bffad 100644
> --- a/drivers/mfd/rohm-bd718x7.c
> +++ b/drivers/mfd/rohm-bd718x7.c
> @@ -79,10 +79,48 @@ static const struct regmap_config
> bd718xx_regmap_config = {
>  	.volatile_table = &volatile_regs,
>  	.max_register = BD718XX_MAX_REGISTER - 1,
>  	.cache_type = REGCACHE_RBTREE,
>  };
>  
> +static int bd718xx_init_press_duration(struct bd718xx *bd718xx)
> +{
> +	struct device* dev = bd718xx->dev;
> +	u32 short_press_ms, long_press_ms;
> +	u32 short_press_value, long_press_value;
> +	int ret;
> +
> +	ret = of_property_read_u32(dev->of_node, "rohm,short-press-ms",
> +				   &short_press_ms);
> +	if (!ret) {
> +		short_press_value = min(15u, short_press_ms / 500);

This should rather be implemented as rounding not flooring. Giving for
example 1400ms from DT should rather yield timeout 1500ms than 1000ms.
I'd say it makes the difference at least for long press tmo.

> +		ret = regmap_update_bits(bd718xx->regmap,
> +					 BD718XX_REG_PWRONCONFIG0,
> +					 BD718XX_PWRBTN_PRESS_DURATION_
> MASK,
> +					 short_press_value);
> +		if (ret) {
> +			dev_err(dev, "Failed to init pwron short
> press\n");
> +			return ret;
> +		}
> +	}
> +
> +	ret = of_property_read_u32(dev->of_node, "rohm,long-press-ms",
> +				   &long_press_ms);
> +	if (!ret) {
> +		long_press_value = min(15u, long_press_ms / 1000);

Also here we should do rounding. 900ms from DT should rather be 1s than
10ms. Especially when the long press causes forced power down and short
press may be 10 ms...

> +		ret = regmap_update_bits(bd718xx->regmap,
> +					 BD718XX_REG_PWRONCONFIG1,
> +					 BD718XX_PWRBTN_PRESS_DURATION_
> MASK,
> +					 long_press_value);
> +		if (ret) {
> +			dev_err(dev, "Failed to init pwron long
> press\n");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int bd718xx_i2c_probe(struct i2c_client *i2c,
>  			    const struct i2c_device_id *id)
>  {
>  	struct bd718xx *bd718xx;
>  	int ret;
> @@ -115,10 +153,14 @@ static int bd718xx_i2c_probe(struct i2c_client
> *i2c,
>  	if (ret) {
>  		dev_err(&i2c->dev, "Failed to add irq_chip\n");
>  		return ret;
>  	}
>  
> +	ret = bd718xx_init_press_duration(bd718xx);
> +	if (ret)
> +		return ret;
> +
>  	ret = regmap_irq_get_virq(bd718xx->irq_data,
> BD718XX_INT_PWRBTN_S);
>  
>  	if (ret < 0) {
>  		dev_err(&i2c->dev, "Failed to get the IRQ\n");
>  		return ret;

Other than that:
Reviewed-By: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>


Best Regards
	Matti Vaittinen

Re: [PATCH 3/3] mfd: bd718x7: Make power button press duration configurable

[Vaittinen, Matti]

Thanks for implementing this Leonard. This is how it should have been
done right away. Just one little thing.

On Mon, 2019-05-20 at 08:55 +0000, Leonard Crestez wrote:
> Allow overwriting the values in BD718XX_REG_PWRONCONFIG0 and
> BD718XX_REG_PWRONCONFIG1 via devicetree.
> 
> Keep existing values (from bootloader or OTP) if property is not
> present.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/mfd/rohm-bd718x7.c | 42
> ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
> index cdbef83884f0..dffa3f4bffad 100644
> --- a/drivers/mfd/rohm-bd718x7.c
> +++ b/drivers/mfd/rohm-bd718x7.c
> @@ -79,10 +79,48 @@ static const struct regmap_config
> bd718xx_regmap_config = {
>  	.volatile_table = &volatile_regs,
>  	.max_register = BD718XX_MAX_REGISTER - 1,
>  	.cache_type = REGCACHE_RBTREE,
>  };
>  
> +static int bd718xx_init_press_duration(struct bd718xx *bd718xx)
> +{
> +	struct device* dev = bd718xx->dev;
> +	u32 short_press_ms, long_press_ms;
> +	u32 short_press_value, long_press_value;
> +	int ret;
> +
> +	ret = of_property_read_u32(dev->of_node, "rohm,short-press-ms",
> +				   &short_press_ms);
> +	if (!ret) {
> +		short_press_value = min(15u, short_press_ms / 500);

This should rather be implemented as rounding not flooring. Giving for
example 1400ms from DT should rather yield timeout 1500ms than 1000ms.
I'd say it makes the difference at least for long press tmo.

> +		ret = regmap_update_bits(bd718xx->regmap,
> +					 BD718XX_REG_PWRONCONFIG0,
> +					 BD718XX_PWRBTN_PRESS_DURATION_
> MASK,
> +					 short_press_value);
> +		if (ret) {
> +			dev_err(dev, "Failed to init pwron short
> press\n");
> +			return ret;
> +		}
> +	}
> +
> +	ret = of_property_read_u32(dev->of_node, "rohm,long-press-ms",
> +				   &long_press_ms);
> +	if (!ret) {
> +		long_press_value = min(15u, long_press_ms / 1000);

Also here we should do rounding. 900ms from DT should rather be 1s than
10ms. Especially when the long press causes forced power down and short
press may be 10 ms...

> +		ret = regmap_update_bits(bd718xx->regmap,
> +					 BD718XX_REG_PWRONCONFIG1,
> +					 BD718XX_PWRBTN_PRESS_DURATION_
> MASK,
> +					 long_press_value);
> +		if (ret) {
> +			dev_err(dev, "Failed to init pwron long
> press\n");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int bd718xx_i2c_probe(struct i2c_client *i2c,
>  			    const struct i2c_device_id *id)
>  {
>  	struct bd718xx *bd718xx;
>  	int ret;
> @@ -115,10 +153,14 @@ static int bd718xx_i2c_probe(struct i2c_client
> *i2c,
>  	if (ret) {
>  		dev_err(&i2c->dev, "Failed to add irq_chip\n");
>  		return ret;
>  	}
>  
> +	ret = bd718xx_init_press_duration(bd718xx);
> +	if (ret)
> +		return ret;
> +
>  	ret = regmap_irq_get_virq(bd718xx->irq_data,
> BD718XX_INT_PWRBTN_S);
>  
>  	if (ret < 0) {
>  		dev_err(&i2c->dev, "Failed to get the IRQ\n");
>  		return ret;

Other than that:
Reviewed-By: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>


Best Regards
	Matti Vaittinen

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel