[PATCH V3 2/3] devicetree: Add bindings for DA9063 OnKey

[PATCH V3 2/3] devicetree: Add bindings for DA9063 OnKey

[S Twiss]

From: Steve Twiss <stwiss.opensource@diasemi.com>

Add device tree bindings for the DA9063 OnKey driver


Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>

---
Version History

Changes in V3
 - No change

Changes in V2
 - Remove the circular dependency comment linking patches in the main e-mail
 - Search and replace 'keyword' with 'property' in onkey sub-node description
 - Reformat onkey sub-node content to move the description for key-power into
   a new optional properties sub-section so it has a more promiment position

This patch applies against linux-next and v4.1-rc1 


 Documentation/devicetree/bindings/mfd/da9063.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/da9063.txt b/Documentation/devicetree/bindings/mfd/da9063.txt
index 42c6fa6..05b21bc 100644
--- a/Documentation/devicetree/bindings/mfd/da9063.txt
+++ b/Documentation/devicetree/bindings/mfd/da9063.txt
@@ -5,6 +5,7 @@ DA9093 consists of a large and varied group of sub-devices (I2C Only):
 Device                   Supply Names    Description
 ------                   ------------    -----------
 da9063-regulator        :               : LDOs & BUCKs
+da9063-onkey            :               : On Key
 da9063-rtc              :               : Real-Time Clock
 da9063-watchdog         :               : Watchdog
 
@@ -51,6 +52,18 @@ Sub-nodes:
   the DA9063. There are currently no entries in this binding, however
   compatible = "dlg,da9063-rtc" should be added if a node is created.
 
+- onkey : This node defines the OnKey settings for controlling the key
+  functionality of the device. The node should contain the compatible property
+  with the value "dlg,da9063-onkey".
+
+  Optional onkey properties:
+
+  - dlg,disable-key-power : Disable power-down using a long key-press. If this
+    entry exists the OnKey driver will remove support for the KEY_POWER key
+    press. If this entry does not exist then by default the key-press
+    triggered power down is enabled and the OnKey will support both KEY_POWER
+    and KEY_SLEEP.
+
 - watchdog : This node defines settings for the Watchdog timer associated
   with the DA9063. There are currently no entries in this binding, however
   compatible = "dlg,da9063-watchdog" should be added if a node is created.
@@ -73,6 +86,11 @@ Example:
 			compatible = "dlg,da9063-watchdog";
 		};
 
+		onkey {
+			compatible = "dlg,da9063-onkey";
+			dlg,disable-key-power;
+		};
+
 		regulators {
 			DA9063_BCORE1: bcore1 {
 				regulator-name = "BCORE1";
-- 
end-of-patch for PATCH V3

[PATCH V3 1/3] input: misc: da9063: OnKey driver

[S Twiss]

From: Steve Twiss <stwiss.opensource@diasemi.com>

Add OnKey driver support for DA9063


Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>

---
The changes made here have been taken from the DA9062 OnKey review thread. 
Please see https://lkml.org/lkml/2015/4/29/406 for more information on this.

Version History

Changes in V3
 - Move the MFD code changes into a separate patch [PATCH V3 3/3]
 - Rename OnKey driver to use underscore: da9063_onkey.c
 - Header and MODULE_LICENSE macro update for GPL and filename change
 - Remove MAINTAINERS file (no longer needed due to rename of onkey driver)
 - Delete input_unregister_device call from platform driver remove function
 - Replaced dev_notice() with dev_dbg() for key press notification

Changes in V2
 - Remove the circular dependency comment in the main e-mail body
   linking PATCH V1 1/2 and 2/2
 - Alter the copyright header information to match expected GPLv2
   text from http://www.gnu.org/licenses/gpl-2.0.html

This patch applies against linux-next and v4.1-rc1 


 drivers/input/misc/Kconfig        |  10 ++
 drivers/input/misc/Makefile       |   1 +
 drivers/input/misc/da9063_onkey.c | 227 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 238 insertions(+)
 create mode 100644 drivers/input/misc/da9063_onkey.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 4436ab1..d917883 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -610,6 +610,16 @@ config INPUT_DA9055_ONKEY
 	  To compile this driver as a module, choose M here: the module
 	  will be called da9055_onkey.
 
+config INPUT_DA9063_ONKEY
+	tristate "Dialog DA9063 OnKey"
+	depends on MFD_DA9063
+	help
+	  Support the ONKEY of Dialog DA9063 Power Management IC as an
+	  input device reporting power button statue.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called da9063-onkey.
+
 config INPUT_DM355EVM
 	tristate "TI DaVinci DM355 EVM Keypad and IR Remote"
 	depends on MFD_DM355EVM_MSP
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 78ba4c1..4f95c56 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_INPUT_CMA3000_I2C)		+= cma3000_d0x_i2c.o
 obj-$(CONFIG_INPUT_COBALT_BTNS)		+= cobalt_btns.o
 obj-$(CONFIG_INPUT_DA9052_ONKEY)	+= da9052_onkey.o
 obj-$(CONFIG_INPUT_DA9055_ONKEY)	+= da9055_onkey.o
+obj-$(CONFIG_INPUT_DA9063_ONKEY)	+= da9063_onkey.o
 obj-$(CONFIG_INPUT_DM355EVM)		+= dm355evm_keys.o
 obj-$(CONFIG_INPUT_E3X0_BUTTON)		+= e3x0-button.o
 obj-$(CONFIG_INPUT_DRV260X_HAPTICS)	+= drv260x.o
diff --git a/drivers/input/misc/da9063_onkey.c b/drivers/input/misc/da9063_onkey.c
new file mode 100644
index 0000000..250595c
--- /dev/null
+++ b/drivers/input/misc/da9063_onkey.c
@@ -0,0 +1,227 @@
+/*
+ * da9063_onkey.c - Onkey device driver for DA9063
+ * Copyright (C) 2015  Dialog Semiconductor Ltd.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/workqueue.h>
+#include <linux/regmap.h>
+#include <linux/of.h>
+#include <linux/mfd/da9063/core.h>
+#include <linux/mfd/da9063/pdata.h>
+#include <linux/mfd/da9063/registers.h>
+
+struct da9063_onkey {
+	struct	da9063 *hw;
+	struct delayed_work work;
+	struct	input_dev *input;
+	int irq;
+	bool key_power;
+};
+
+static void da9063_poll_on(struct work_struct *work)
+{
+	struct da9063_onkey *onkey = container_of(work, struct da9063_onkey,
+						  work.work);
+	unsigned int val;
+	int fault_log = 0;
+	bool poll = true;
+	int ret;
+
+	/* poll to see when the pin is released */
+	ret = regmap_read(onkey->hw->regmap, DA9063_REG_STATUS_A, &val);
+	if (ret < 0) {
+		dev_err(&onkey->input->dev,
+			"Failed to read ON status: %d\n", ret);
+		goto err_poll;
+	}
+
+	if (!(val & DA9063_NONKEY)) {
+		ret = regmap_update_bits(onkey->hw->regmap,
+					 DA9063_REG_CONTROL_B,
+					 DA9063_NONKEY_LOCK, 0);
+		if (ret < 0) {
+			dev_err(&onkey->input->dev,
+				"Failed to reset the Key Delay %d\n", ret);
+			goto err_poll;
+		}
+
+		input_report_key(onkey->input, KEY_POWER, 0);
+		input_sync(onkey->input);
+
+		poll = false;
+	}
+
+	/* if the fault log KEY_RESET is detected,
+	 * then clear it and shutdown via I2C
+	 */
+	ret = regmap_read(onkey->hw->regmap, DA9063_REG_FAULT_LOG, &fault_log);
+	if (ret < 0)
+		dev_warn(&onkey->input->dev, "Cannot read FAULT_LOG\n");
+
+	if (fault_log & DA9063_KEY_RESET) {
+		ret = regmap_write(onkey->hw->regmap,
+				   DA9063_REG_FAULT_LOG,
+				   DA9063_KEY_RESET);
+		if (ret < 0)
+			dev_warn(&onkey->input->dev,
+				 "Cannot reset KEY_RESET fault log\n");
+		else {
+			/* at this point we do any S/W housekeeping
+			 * and then send shutdown command
+			 */
+			dev_dbg(&onkey->input->dev,
+				 "Sending SHUTDOWN to DA9063 ...\n");
+			ret = regmap_write(onkey->hw->regmap,
+					   DA9063_REG_CONTROL_F,
+					   DA9063_SHUTDOWN);
+			if (ret < 0)
+				dev_err(&onkey->input->dev,
+					"Cannot SHUTDOWN DA9063\n");
+		}
+	}
+
+err_poll:
+	if (poll)
+		schedule_delayed_work(&onkey->work, 50);
+}
+
+static irqreturn_t da9063_onkey_irq_handler(int irq, void *data)
+{
+	struct da9063_onkey *onkey = data;
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(onkey->hw->regmap, DA9063_REG_STATUS_A, &val);
+	if (onkey->key_power && (ret >= 0) && (val & DA9063_NONKEY)) {
+		input_report_key(onkey->input, KEY_POWER, 1);
+		input_sync(onkey->input);
+		schedule_delayed_work(&onkey->work, 0);
+		dev_dbg(&onkey->input->dev, "KEY_POWER pressed.\n");
+	} else {
+		input_report_key(onkey->input, KEY_SLEEP, 1);
+		input_sync(onkey->input);
+		input_report_key(onkey->input, KEY_SLEEP, 0);
+		input_sync(onkey->input);
+		dev_dbg(&onkey->input->dev, "KEY_SLEEP pressed.\n");
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int da9063_onkey_probe(struct platform_device *pdev)
+{
+	struct da9063 *da9063 = dev_get_drvdata(pdev->dev.parent);
+	struct da9063_pdata *pdata = dev_get_platdata(da9063->dev);
+	struct da9063_onkey *onkey;
+	bool kp_tmp = true;
+	int ret = 0;
+
+	if (pdata)
+		kp_tmp = pdata->key_power;
+	else {
+		kp_tmp = of_property_read_bool((&pdev->dev)->of_node,
+					       "dlg,disable-key-power");
+		kp_tmp = !kp_tmp;
+	}
+
+	onkey = devm_kzalloc(&pdev->dev, sizeof(struct da9063_onkey),
+			     GFP_KERNEL);
+	if (!onkey) {
+		dev_err(&pdev->dev, "Failed to allocate memory.\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	INIT_DELAYED_WORK(&onkey->work, da9063_poll_on);
+
+	onkey->input = devm_input_allocate_device(&pdev->dev);
+	if (!onkey->input) {
+		dev_err(&pdev->dev, "Failed to allocated input device.\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	ret = platform_get_irq_byname(pdev, "ONKEY");
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to get platform IRQ.\n");
+		goto err;
+	}
+	onkey->irq = ret;
+
+	ret = request_threaded_irq(onkey->irq, NULL,
+				   da9063_onkey_irq_handler,
+				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+				   "ONKEY", onkey);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Failed to request input device IRQ.\n");
+		goto err;
+	}
+
+	onkey->hw = da9063;
+	onkey->key_power = kp_tmp;
+	onkey->input->evbit[0] = BIT_MASK(EV_KEY);
+	onkey->input->name = DA9063_DRVNAME_ONKEY;
+	onkey->input->phys = DA9063_DRVNAME_ONKEY "/input0";
+	onkey->input->dev.parent = &pdev->dev;
+
+	if (onkey->key_power)
+		input_set_capability(onkey->input, EV_KEY, KEY_POWER);
+	input_set_capability(onkey->input, EV_KEY, KEY_SLEEP);
+
+	ret = input_register_device(onkey->input);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Failed to register input device.\n");
+		goto err_irq;
+	}
+
+	platform_set_drvdata(pdev, onkey);
+	return 0;
+
+err_irq:
+	free_irq(onkey->irq, onkey);
+	cancel_delayed_work_sync(&onkey->work);
+err:
+	return ret;
+}
+
+static int da9063_onkey_remove(struct platform_device *pdev)
+{
+	struct	da9063_onkey *onkey = platform_get_drvdata(pdev);
+
+	free_irq(onkey->irq, onkey);
+	cancel_delayed_work_sync(&onkey->work);
+	return 0;
+}
+
+static struct platform_driver da9063_onkey_driver = {
+	.probe	= da9063_onkey_probe,
+	.remove	= da9063_onkey_remove,
+	.driver	= {
+		.name	= DA9063_DRVNAME_ONKEY,
+		.owner	= THIS_MODULE,
+	},
+};
+
+module_platform_driver(da9063_onkey_driver);
+
+MODULE_AUTHOR("S Twiss <stwiss.opensource@diasemi.com>");
+MODULE_DESCRIPTION("Onkey device driver for Dialog DA9063");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DA9063_DRVNAME_ONKEY);
-- 
end-of-patch for PATCH V3

[PATCH V3 0/3] Add OnKey support for DA9063

[S Twiss]

From: Steve Twiss <stwiss.opensource@diasemi.com>

This patch set adds OnKey driver support for the Dialog
Semiconductor DA9063 PMIC.

[PATCH V3 1/3]: kernel driver onkey support
[PATCH V3 2/3]: device tree bindings document
[PATCH V3 3/3]: mfd core support

Thank you,
Steve Twiss, Dialog Semiconductor Ltd.

S Twiss (3):
  input: misc: da9063: OnKey driver
  devicetree: Add bindings for DA9063 OnKey
  mfd: da9063: MFD support for OnKey driver

 Documentation/devicetree/bindings/mfd/da9063.txt |  18 ++
 drivers/input/misc/Kconfig                       |  10 +
 drivers/input/misc/Makefile                      |   1 +
 drivers/input/misc/da9063_onkey.c                | 227 +++++++++++++++++++++++
 drivers/mfd/da9063-core.c                        |  54 ++++++
 include/linux/mfd/da9063/pdata.h                 |   1 +
 6 files changed, 311 insertions(+)
 create mode 100644 drivers/input/misc/da9063_onkey.c

-- 
end-of-patch for PATCH V3

[PATCH V3 3/3] mfd: da9063: MFD support for OnKey driver

[S Twiss]

From: Steve Twiss <stwiss.opensource@diasemi.com>

Add MFD support for the DA9063 OnKey driver


Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>

---
Version History

Changes in V3
 - The MFD code was originally in [PATCH V2 1/2]. This is now been moved into
   this patch [PATCH V3 3/3].
 - Whitespace cleanup

This patch applies against linux-next and v4.1-rc1 


 drivers/mfd/da9063-core.c        | 54 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/da9063/pdata.h |  1 +
 2 files changed, 55 insertions(+)

diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
index facd361..af841c1 100644
--- a/drivers/mfd/da9063-core.c
+++ b/drivers/mfd/da9063-core.c
@@ -60,6 +60,7 @@ static struct resource da9063_rtc_resources[] = {
 
 static struct resource da9063_onkey_resources[] = {
 	{
+		.name	= "ONKEY",
 		.start	= DA9063_IRQ_ONKEY,
 		.end	= DA9063_IRQ_ONKEY,
 		.flags	= IORESOURCE_IRQ,
@@ -97,6 +98,7 @@ static const struct mfd_cell da9063_devs[] = {
 		.name		= DA9063_DRVNAME_ONKEY,
 		.num_resources	= ARRAY_SIZE(da9063_onkey_resources),
 		.resources	= da9063_onkey_resources,
+		.of_compatible = "dlg,da9063-onkey",
 	},
 	{
 		.name		= DA9063_DRVNAME_RTC,
@@ -109,12 +111,64 @@ static const struct mfd_cell da9063_devs[] = {
 	},
 };
 
+static int da9063_clear_fault_log(struct da9063 *da9063)
+{
+	int ret = 0;
+	int fault_log = 0;
+
+	ret = regmap_read(da9063->regmap, DA9063_REG_FAULT_LOG, &fault_log);
+	if (ret < 0) {
+		dev_err(da9063->dev, "Cannot read FAULT_LOG.\n");
+		return -EIO;
+	}
+
+	if (fault_log) {
+		if (fault_log & DA9063_TWD_ERROR)
+			dev_dbg(da9063->dev,
+				"Fault log entry detected: DA9063_TWD_ERROR\n");
+		if (fault_log & DA9063_POR)
+			dev_dbg(da9063->dev,
+				"Fault log entry detected: DA9063_POR\n");
+		if (fault_log & DA9063_VDD_FAULT)
+			dev_dbg(da9063->dev,
+				"Fault log entry detected: DA9063_VDD_FAULT\n");
+		if (fault_log & DA9063_VDD_START)
+			dev_dbg(da9063->dev,
+				"Fault log entry detected: DA9063_VDD_START\n");
+		if (fault_log & DA9063_TEMP_CRIT)
+			dev_dbg(da9063->dev,
+				"Fault log entry detected: DA9063_TEMP_CRIT\n");
+		if (fault_log & DA9063_KEY_RESET)
+			dev_dbg(da9063->dev,
+				"Fault log entry detected: DA9063_KEY_RESET\n");
+		if (fault_log & DA9063_NSHUTDOWN)
+			dev_dbg(da9063->dev,
+				"Fault log entry detected: DA9063_NSHUTDOWN\n");
+		if (fault_log & DA9063_WAIT_SHUT)
+			dev_dbg(da9063->dev,
+				"Fault log entry detected: DA9063_WAIT_SHUT\n");
+	}
+
+	ret = regmap_write(da9063->regmap,
+			   DA9063_REG_FAULT_LOG,
+			   fault_log);
+	if (ret < 0)
+		dev_err(da9063->dev,
+			"Cannot reset FAULT_LOG values %d\n", ret);
+
+	return ret;
+}
+
 int da9063_device_init(struct da9063 *da9063, unsigned int irq)
 {
 	struct da9063_pdata *pdata = da9063->dev->platform_data;
 	int model, variant_id, variant_code;
 	int ret;
 
+	ret = da9063_clear_fault_log(da9063);
+	if (ret < 0)
+		dev_err(da9063->dev, "Cannot clear fault log\n");
+
 	if (pdata) {
 		da9063->flags = pdata->flags;
 		da9063->irq_base = pdata->irq_base;
diff --git a/include/linux/mfd/da9063/pdata.h b/include/linux/mfd/da9063/pdata.h
index 95c8742..612383b 100644
--- a/include/linux/mfd/da9063/pdata.h
+++ b/include/linux/mfd/da9063/pdata.h
@@ -103,6 +103,7 @@ struct da9063;
 struct da9063_pdata {
 	int				(*init)(struct da9063 *da9063);
 	int				irq_base;
+	bool				key_power;
 	unsigned			flags;
 	struct da9063_regulators_pdata	*regulators_pdata;
 	struct led_platform_data	*leds_pdata;
-- 
end-of-patch for PATCH V3

Re: [PATCH V3 3/3] mfd: da9063: MFD support for OnKey driver

[Geert Uytterhoeven]

Hi Steve,

On Thu, Apr 30, 2015 at 1:26 PM, S Twiss <stwiss.opensource@diasemi.com> wrote:
> Add MFD support for the DA9063 OnKey driver

Thanks  for your patch!

> --- a/drivers/mfd/da9063-core.c
> +++ b/drivers/mfd/da9063-core.c
> @@ -60,6 +60,7 @@ static struct resource da9063_rtc_resources[] = {
>
>  static struct resource da9063_onkey_resources[] = {
>         {
> +               .name   = "ONKEY",
>                 .start  = DA9063_IRQ_ONKEY,
>                 .end    = DA9063_IRQ_ONKEY,
>                 .flags  = IORESOURCE_IRQ,
> @@ -97,6 +98,7 @@ static const struct mfd_cell da9063_devs[] = {
>                 .name           = DA9063_DRVNAME_ONKEY,
>                 .num_resources  = ARRAY_SIZE(da9063_onkey_resources),
>                 .resources      = da9063_onkey_resources,
> +               .of_compatible = "dlg,da9063-onkey",
>         },
>         {
>                 .name           = DA9063_DRVNAME_RTC,
> @@ -109,12 +111,64 @@ static const struct mfd_cell da9063_devs[] = {
>         },
>  };
>
> +static int da9063_clear_fault_log(struct da9063 *da9063)
> +{
> +       int ret = 0;
> +       int fault_log = 0;
> +
> +       ret = regmap_read(da9063->regmap, DA9063_REG_FAULT_LOG, &fault_log);
> +       if (ret < 0) {
> +               dev_err(da9063->dev, "Cannot read FAULT_LOG.\n");
> +               return -EIO;
> +       }
> +
> +       if (fault_log) {
> +               if (fault_log & DA9063_TWD_ERROR)
> +                       dev_dbg(da9063->dev,
> +                               "Fault log entry detected: DA9063_TWD_ERROR\n");
> +               if (fault_log & DA9063_POR)
> +                       dev_dbg(da9063->dev,
> +                               "Fault log entry detected: DA9063_POR\n");
> +               if (fault_log & DA9063_VDD_FAULT)
> +                       dev_dbg(da9063->dev,
> +                               "Fault log entry detected: DA9063_VDD_FAULT\n");
> +               if (fault_log & DA9063_VDD_START)
> +                       dev_dbg(da9063->dev,
> +                               "Fault log entry detected: DA9063_VDD_START\n");
> +               if (fault_log & DA9063_TEMP_CRIT)
> +                       dev_dbg(da9063->dev,
> +                               "Fault log entry detected: DA9063_TEMP_CRIT\n");
> +               if (fault_log & DA9063_KEY_RESET)
> +                       dev_dbg(da9063->dev,
> +                               "Fault log entry detected: DA9063_KEY_RESET\n");
> +               if (fault_log & DA9063_NSHUTDOWN)
> +                       dev_dbg(da9063->dev,
> +                               "Fault log entry detected: DA9063_NSHUTDOWN\n");
> +               if (fault_log & DA9063_WAIT_SHUT)
> +                       dev_dbg(da9063->dev,
> +                               "Fault log entry detected: DA9063_WAIT_SHUT\n");
> +       }
> +
> +       ret = regmap_write(da9063->regmap,
> +                          DA9063_REG_FAULT_LOG,
> +                          fault_log);
> +       if (ret < 0)
> +               dev_err(da9063->dev,
> +                       "Cannot reset FAULT_LOG values %d\n", ret);
> +
> +       return ret;
> +}
> +
>  int da9063_device_init(struct da9063 *da9063, unsigned int irq)
>  {
>         struct da9063_pdata *pdata = da9063->dev->platform_data;
>         int model, variant_id, variant_code;
>         int ret;
>
> +       ret = da9063_clear_fault_log(da9063);
> +       if (ret < 0)
> +               dev_err(da9063->dev, "Cannot clear fault log\n");
> +
>         if (pdata) {
>                 da9063->flags = pdata->flags;
>                 da9063->irq_base = pdata->irq_base;

The code above doesn't seem to match the patch description.
Can you please explain why it is needed?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH V3 3/3] mfd: da9063: MFD support for OnKey driver

[Opensource [Steve Twiss]]

On 04 May 2015 08:47, Geert Uytterhoeven wrote:

> On Thu, Apr 30, 2015 at 1:26 PM, S Twiss <stwiss.opensource@diasemi.com>
> wrote:
> > Add MFD support for the DA9063 OnKey driver
> 
> Thanks  for your patch!
> 
> > +static int da9063_clear_fault_log(struct da9063 *da9063)
> > +{	
> > +       int ret = 0;
> > +       int fault_log = 0;
> > +
> > +       ret = regmap_read(da9063->regmap, DA9063_REG_FAULT_LOG,
> &fault_log);
> > +       if (ret < 0) {
> > +               dev_err(da9063->dev, "Cannot read FAULT_LOG.\n");
> > +               return -EIO;
> > +       }
> > +
> > +       if (fault_log) {
> > +               if (fault_log & DA9063_TWD_ERROR)
> > +                       dev_dbg(da9063->dev,
> > +                               "Fault log entry detected: DA9063_TWD_ERROR\n");
> > +               if (fault_log & DA9063_POR)
> > +                       dev_dbg(da9063->dev,
> > +                               "Fault log entry detected: DA9063_POR\n");
> > +               if (fault_log & DA9063_VDD_FAULT)
> > +                       dev_dbg(da9063->dev,
> > +                               "Fault log entry detected: DA9063_VDD_FAULT\n");
> > +               if (fault_log & DA9063_VDD_START)
> > +                       dev_dbg(da9063->dev,
> > +                               "Fault log entry detected: DA9063_VDD_START\n");
> > +               if (fault_log & DA9063_TEMP_CRIT)
> > +                       dev_dbg(da9063->dev,
> > +                               "Fault log entry detected: DA9063_TEMP_CRIT\n");
> > +               if (fault_log & DA9063_KEY_RESET)
> > +                       dev_dbg(da9063->dev,
> > +                               "Fault log entry detected: DA9063_KEY_RESET\n");
> > +               if (fault_log & DA9063_NSHUTDOWN)
> > +                       dev_dbg(da9063->dev,
> > +                               "Fault log entry detected: DA9063_NSHUTDOWN\n");
> > +               if (fault_log & DA9063_WAIT_SHUT)
> > +                       dev_dbg(da9063->dev,
> > +                               "Fault log entry detected: DA9063_WAIT_SHUT\n");
> > +       }
> > +
> > +       ret = regmap_write(da9063->regmap,
> > +                          DA9063_REG_FAULT_LOG,
> > +                          fault_log);
> > +       if (ret < 0)
> > +               dev_err(da9063->dev,
> > +                       "Cannot reset FAULT_LOG values %d\n", ret);
> > +
> > +       return ret;
> > +}
> > +
> >  int da9063_device_init(struct da9063 *da9063, unsigned int irq)
> >  {
> >         struct da9063_pdata *pdata = da9063->dev->platform_data;
> >         int model, variant_id, variant_code;
> >         int ret;
> >
> > +       ret = da9063_clear_fault_log(da9063);
> > +       if (ret < 0)
> > +               dev_err(da9063->dev, "Cannot clear fault log\n");
> > +
> >         if (pdata) {
> >                 da9063->flags = pdata->flags;
> >                 da9063->irq_base = pdata->irq_base;
> 
> The code above doesn't seem to match the patch description.
> Can you please explain why it is needed?
> 

Hi Geert,

Yes, at first it does seem that the fault-log clearing function is unrelated to the
"MFD support for the DA9063 OnKey driver", but there is an important connection
that makes it essential for the OnKey driver in this case.

I have made some explanation of the OnKey's operation in this thread here:
https://lkml.org/lkml/2015/4/29/406

But I only described the OnKey's point-of-view for case (4) of the "OnKey operation"
Case (4): the long-long press and no key release -- the hardware power-cut when
software is unable to respond (for whatever reason).

In this case, if the software was not responsive and a hardware shutdown of the
device was chosen, the FAULT_LOG would persist with information that would be
accessible when the device was restarted: it would be possible to take action the
next time the device was turned on again (maybe to trigger some mitigation exercise
in the bootloader -- e.g. say to complete memory checks or put the device into a
safe mode). 

However this mitigation exercise and clearance of the fault-log cannot be counted on 
outside the Linux kernel and so the clearance function da9063_clear_fault_log () has
been done here.

If this clearance function did not exist, then after a hardware shutdown (due to S/W
failure), the next time the device is restarted the FAULT_LOG would persist with values
from the previous error.

Regards,
Steve

Re: [PATCH V3 3/3] mfd: da9063: MFD support for OnKey driver

[Geert Uytterhoeven]

Hi Steve,

On Tue, May 5, 2015 at 10:36 AM, Opensource [Steve Twiss]
<stwiss.opensource@diasemi.com> wrote:
> On 04 May 2015 08:47, Geert Uytterhoeven wrote:
>> On Thu, Apr 30, 2015 at 1:26 PM, S Twiss <stwiss.opensource@diasemi.com>
>> wrote:
>> > Add MFD support for the DA9063 OnKey driver

>> > +static int da9063_clear_fault_log(struct da9063 *da9063)
>> > +{

>> > +}
>> > +
>> >  int da9063_device_init(struct da9063 *da9063, unsigned int irq)
>> >  {
>> >         struct da9063_pdata *pdata = da9063->dev->platform_data;
>> >         int model, variant_id, variant_code;
>> >         int ret;
>> >
>> > +       ret = da9063_clear_fault_log(da9063);
>> > +       if (ret < 0)
>> > +               dev_err(da9063->dev, "Cannot clear fault log\n");
>> > +
>> >         if (pdata) {
>> >                 da9063->flags = pdata->flags;
>> >                 da9063->irq_base = pdata->irq_base;
>>
>> The code above doesn't seem to match the patch description.
>> Can you please explain why it is needed?
>
> Yes, at first it does seem that the fault-log clearing function is unrelated to the
> "MFD support for the DA9063 OnKey driver", but there is an important connection
> that makes it essential for the OnKey driver in this case.
>
> I have made some explanation of the OnKey's operation in this thread here:
> https://lkml.org/lkml/2015/4/29/406
>
> But I only described the OnKey's point-of-view for case (4) of the "OnKey operation"
> Case (4): the long-long press and no key release -- the hardware power-cut when
> software is unable to respond (for whatever reason).
>
> In this case, if the software was not responsive and a hardware shutdown of the
> device was chosen, the FAULT_LOG would persist with information that would be
> accessible when the device was restarted: it would be possible to take action the
> next time the device was turned on again (maybe to trigger some mitigation exercise
> in the bootloader -- e.g. say to complete memory checks or put the device into a
> safe mode).
>
> However this mitigation exercise and clearance of the fault-log cannot be counted on
> outside the Linux kernel and so the clearance function da9063_clear_fault_log () has
> been done here.
>
> If this clearance function did not exist, then after a hardware shutdown (due to S/W
> failure), the next time the device is restarted the FAULT_LOG would persist with values
> from the previous error.

Thanks for your explanation!

Please add (some parts of it) to the patch description.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH V3 3/3] mfd: da9063: MFD support for OnKey driver

[Opensource [Steve Twiss]]

On 05 May 2015 09:52 Geert Uytterhoeven wrote:

> On Tue, May 5, 2015 at 10:36 AM, Opensource [Steve Twiss]
> <stwiss.opensource@diasemi.com> wrote:
> > On 04 May 2015 08:47, Geert Uytterhoeven wrote:
> >> On Thu, Apr 30, 2015 at 1:26 PM, S Twiss <stwiss.opensource@diasemi.com> wrote:
> >> > Add MFD support for the DA9063 OnKey driver
> >> > +static int da9063_clear_fault_log(struct da9063 *da9063)
> >> > +{
> 
> >> > +}
> >> > +
> >> >  int da9063_device_init(struct da9063 *da9063, unsigned int irq)
> >> >  {
> >> >         struct da9063_pdata *pdata = da9063->dev->platform_data;
> >> >         int model, variant_id, variant_code;
> >> >         int ret;
> >> >
> >> > +       ret = da9063_clear_fault_log(da9063);
> >> > +       if (ret < 0)
> >> > +               dev_err(da9063->dev, "Cannot clear fault log\n");
> >> > +
> >> >         if (pdata) {
> >> >                 da9063->flags = pdata->flags;
> >> >                 da9063->irq_base = pdata->irq_base;
> >>
> >> The code above doesn't seem to match the patch description.
> >> Can you please explain why it is needed?
> >
> > Yes, at first it does seem that the fault-log clearing function is unrelated to the
> > "MFD support for the DA9063 OnKey driver", but there is an important connection
> > that makes it essential for the OnKey driver in this case.
> >
> > I have made some explanation of the OnKey's operation in this thread here:
> > https://lkml.org/lkml/2015/4/29/406
> >
> > But I only described the OnKey's point-of-view for case (4) of the "OnKey operation"
> > Case (4): the long-long press and no key release -- the hardware power-cut when
> > software is unable to respond (for whatever reason).
> >
> > In this case, if the software was not responsive and a hardware shutdown of the
> > device was chosen, the FAULT_LOG would persist with information that would be
> > accessible when the device was restarted: it would be possible to take action the
> > next time the device was turned on again (maybe to trigger some mitigation exercise
> > in the bootloader -- e.g. say to complete memory checks or put the device into a
> > safe mode).
> >
> > However this mitigation exercise and clearance of the fault-log cannot be counted on
> > outside the Linux kernel and so the clearance function da9063_clear_fault_log () has
> > been done here.
> >
> > If this clearance function did not exist, then after a hardware shutdown (due to S/W
> > failure), the next time the device is restarted the FAULT_LOG would persist with values
> > from the previous error.
> 
> Thanks for your explanation!
> 
> Please add (some parts of it) to the patch description.
> 

Thanks Geert, 

I will do that.
I'll wait a while to see if there are any other responses (e.g. OnKey) and then make a new
patch set called PATCH V4.

Regards,
Steve

CC: Dmitry Torokhov for information on these MFD changes since they also relate to the
OnKey.