[PATCH V1] mfd: da9053: ensure the FAULT_LOG is cleared during MFD driver probe

[PATCH V1] mfd: da9053: ensure the FAULT_LOG is cleared during MFD driver probe

[Steve Twiss]

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

The function da9052_clear_fault_log() is added to mitigate the case of
persistent data being transferred between reboots.

Clearance of any the persistent information within the DA9053 FAULT_LOG
register must be completed during start-up so the fault-log does not
continue with previous values. A clearance function has been added here in
the kernel driver because wiping the fault-log cannot be counted on outside
the Linux kernel.

Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
Reviewed-by: Adam Thomson <adam.thomson.opensource@diasemi.com>

---
This patch applies against linux-next and v4.7-rc6


Hi Lee,

This patch is similar to the requirements for DA9062 and DA9063: to ensure
the FAULT_LOG is started from a 'clean' position.

The Dialog datasheet for DA9053 suggests: "The FAULT_LOG register has to
be cleared from the host after reading, by writing a value of 11111111".

See reference:
  commit 9011e4a8a6fe57f76511609930ed00d305389089
  Author: Steve Twiss <stwiss.opensource@diasemi.com>
  Date:   Tue May 19 11:32:45 2015 +0100

Regards,
Steve


 drivers/mfd/da9052-core.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/mfd/da9052-core.c b/drivers/mfd/da9052-core.c
index c0bf68a..a88c206 100644
--- a/drivers/mfd/da9052-core.c
+++ b/drivers/mfd/da9052-core.c
@@ -167,6 +167,7 @@ static bool da9052_reg_writeable(struct device *dev, unsigned int reg)
 	case DA9052_EVENT_B_REG:
 	case DA9052_EVENT_C_REG:
 	case DA9052_EVENT_D_REG:
+	case DA9052_FAULTLOG_REG:
 	case DA9052_IRQ_MASK_A_REG:
 	case DA9052_IRQ_MASK_B_REG:
 	case DA9052_IRQ_MASK_C_REG:
@@ -541,6 +542,52 @@ const struct regmap_config da9052_regmap_config = {
 };
 EXPORT_SYMBOL_GPL(da9052_regmap_config);
 
+static int da9052_clear_fault_log(struct da9052 *da9052)
+{
+	int ret = 0;
+	int fault_log = 0;
+
+	fault_log = da9052_reg_read(da9052, DA9052_FAULTLOG_REG);
+	if (fault_log < 0) {
+		dev_err(da9052->dev,
+			"Cannot read FAULT_LOG %d\n", fault_log);
+		return fault_log;
+	}
+
+	if (fault_log) {
+		if (fault_log & DA9052_FAULTLOG_TWDERROR)
+			dev_dbg(da9052->dev,
+				"Fault log entry detected: TWD_ERROR\n");
+		if (fault_log & DA9052_FAULTLOG_VDDFAULT)
+			dev_dbg(da9052->dev,
+				"Fault log entry detected: VDD_FAULT\n");
+		if (fault_log & DA9052_FAULTLOG_VDDSTART)
+			dev_dbg(da9052->dev,
+				"Fault log entry detected: VDD_START\n");
+		if (fault_log & DA9052_FAULTLOG_TEMPOVER)
+			dev_dbg(da9052->dev,
+				"Fault log entry detected: TEMP_OVER\n");
+		if (fault_log & DA9052_FAULTLOG_KEYSHUT)
+			dev_dbg(da9052->dev,
+				"Fault log entry detected: KEY_SHUT\n");
+		if (fault_log & DA9052_FAULTLOG_NSDSET)
+			dev_dbg(da9052->dev,
+				"Fault log entry detected: nSD_SHUT\n");
+		if (fault_log & DA9052_FAULTLOG_WAITSET)
+			dev_dbg(da9052->dev,
+				"Fault log entry detected: WAIT_SHUT\n");
+
+		ret = da9052_reg_write(da9052,
+					DA9052_FAULTLOG_REG,
+					0xFF);
+		if (ret < 0)
+			dev_err(da9052->dev,
+				"Cannot reset FAULT_LOG values %d\n", ret);
+	}
+
+	return ret;
+}
+
 int da9052_device_init(struct da9052 *da9052, u8 chip_id)
 {
 	struct da9052_pdata *pdata = dev_get_platdata(da9052->dev);
@@ -549,6 +596,10 @@ int da9052_device_init(struct da9052 *da9052, u8 chip_id)
 	mutex_init(&da9052->auxadc_lock);
 	init_completion(&da9052->done);
 
+	ret = da9052_clear_fault_log(da9052);
+	if (ret < 0)
+		dev_warn(da9052->dev, "Cannot clear FAULT_LOG\n");
+
 	if (pdata && pdata->init != NULL)
 		pdata->init(da9052);
 
-- 
end-of-patch for PATCH V1

Re: [PATCH V1] mfd: da9053: ensure the FAULT_LOG is cleared during MFD driver probe

[Lee Jones]

On Wed, 06 Jul 2016, Steve Twiss wrote:

> From: Steve Twiss <stwiss.opensource@diasemi.com>
> 
> The function da9052_clear_fault_log() is added to mitigate the case of
> persistent data being transferred between reboots.
> 
> Clearance of any the persistent information within the DA9053 FAULT_LOG
> register must be completed during start-up so the fault-log does not
> continue with previous values. A clearance function has been added here in
> the kernel driver because wiping the fault-log cannot be counted on outside
> the Linux kernel.
> 
> Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
> Reviewed-by: Adam Thomson <adam.thomson.opensource@diasemi.com>
> 
> ---
> This patch applies against linux-next and v4.7-rc6
> 
> 
> Hi Lee,
> 
> This patch is similar to the requirements for DA9062 and DA9063: to ensure
> the FAULT_LOG is started from a 'clean' position.
> 
> The Dialog datasheet for DA9053 suggests: "The FAULT_LOG register has to
> be cleared from the host after reading, by writing a value of 11111111".
> 
> See reference:
>   commit 9011e4a8a6fe57f76511609930ed00d305389089
>   Author: Steve Twiss <stwiss.opensource@diasemi.com>
>   Date:   Tue May 19 11:32:45 2015 +0100

Looks like much of the same code.  Can you have a pop at generifying
this to avoid any unnecessary duplication?

>  drivers/mfd/da9052-core.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/mfd/da9052-core.c b/drivers/mfd/da9052-core.c
> index c0bf68a..a88c206 100644
> --- a/drivers/mfd/da9052-core.c
> +++ b/drivers/mfd/da9052-core.c
> @@ -167,6 +167,7 @@ static bool da9052_reg_writeable(struct device *dev, unsigned int reg)
>  	case DA9052_EVENT_B_REG:
>  	case DA9052_EVENT_C_REG:
>  	case DA9052_EVENT_D_REG:
> +	case DA9052_FAULTLOG_REG:
>  	case DA9052_IRQ_MASK_A_REG:
>  	case DA9052_IRQ_MASK_B_REG:
>  	case DA9052_IRQ_MASK_C_REG:
> @@ -541,6 +542,52 @@ const struct regmap_config da9052_regmap_config = {
>  };
>  EXPORT_SYMBOL_GPL(da9052_regmap_config);
>  
> +static int da9052_clear_fault_log(struct da9052 *da9052)
> +{
> +	int ret = 0;
> +	int fault_log = 0;
> +
> +	fault_log = da9052_reg_read(da9052, DA9052_FAULTLOG_REG);
> +	if (fault_log < 0) {
> +		dev_err(da9052->dev,
> +			"Cannot read FAULT_LOG %d\n", fault_log);
> +		return fault_log;
> +	}
> +
> +	if (fault_log) {
> +		if (fault_log & DA9052_FAULTLOG_TWDERROR)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: TWD_ERROR\n");
> +		if (fault_log & DA9052_FAULTLOG_VDDFAULT)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: VDD_FAULT\n");
> +		if (fault_log & DA9052_FAULTLOG_VDDSTART)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: VDD_START\n");
> +		if (fault_log & DA9052_FAULTLOG_TEMPOVER)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: TEMP_OVER\n");
> +		if (fault_log & DA9052_FAULTLOG_KEYSHUT)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: KEY_SHUT\n");
> +		if (fault_log & DA9052_FAULTLOG_NSDSET)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: nSD_SHUT\n");
> +		if (fault_log & DA9052_FAULTLOG_WAITSET)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: WAIT_SHUT\n");
> +
> +		ret = da9052_reg_write(da9052,
> +					DA9052_FAULTLOG_REG,
> +					0xFF);
> +		if (ret < 0)
> +			dev_err(da9052->dev,
> +				"Cannot reset FAULT_LOG values %d\n", ret);
> +	}
> +
> +	return ret;
> +}
> +
>  int da9052_device_init(struct da9052 *da9052, u8 chip_id)
>  {
>  	struct da9052_pdata *pdata = dev_get_platdata(da9052->dev);
> @@ -549,6 +596,10 @@ int da9052_device_init(struct da9052 *da9052, u8 chip_id)
>  	mutex_init(&da9052->auxadc_lock);
>  	init_completion(&da9052->done);
>  
> +	ret = da9052_clear_fault_log(da9052);
> +	if (ret < 0)
> +		dev_warn(da9052->dev, "Cannot clear FAULT_LOG\n");
> +
>  	if (pdata && pdata->init != NULL)
>  		pdata->init(da9052);
>  

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

RE: [PATCH V1] mfd: da9053: ensure the FAULT_LOG is cleared during MFD driver probe

[Steve Twiss]

On 05 August 2016 10:05, Lee Jones wrote:

> To: Steve Twiss
> Subject: Re: [PATCH V1] mfd: da9053: ensure the FAULT_LOG is cleared during MFD driver probe
> 
> On Wed, 06 Jul 2016, Steve Twiss wrote:
> > From: Steve Twiss <stwiss.opensource@diasemi.com>
> >
> > Clearance of any the persistent information within the DA9053 FAULT_LOG
> > register must be completed during start-up so the fault-log does not
> > continue with previous values. A clearance function has been added here in
> > the kernel driver because wiping the fault-log cannot be counted on outside
> > the Linux kernel.

[...]

> > This patch is similar to the requirements for DA9062 and DA9063: to ensure
> > the FAULT_LOG is started from a 'clean' position.
> >
> > The Dialog datasheet for DA9053 suggests: "The FAULT_LOG register has to
> > be cleared from the host after reading, by writing a value of 11111111".
> >
> > See reference:
> >   commit 9011e4a8a6fe57f76511609930ed00d305389089
> >   Author: Steve Twiss <stwiss.opensource@diasemi.com>
> >   Date:   Tue May 19 11:32:45 2015 +0100
> 
> Looks like much of the same code.  Can you have a pop at generifying
> this to avoid any unnecessary duplication?

Hi Lee, 

Although the function looks like those found in both the DA9062 and DA9063 drivers, I was just
intending to highlight the "requirement similarities" to clear the fault register on start-up. This
requirement for the Dialog PMIC chips extends across those devices, but the function is not
really general enough to be merged into the kernel core or reused across other Dialog devices.

I don't think there is unnecessary duplication in this particular case for the DA9052/53 driver.

This da9052_clear_fault_log() function will apply to both the Dialog DA9052 and DA9053 PMICs
and so I am making good use of code re-use in this place. But, this does not extend to the
DA9062 or DA9063.

I don't think this FAULT_LOG clearance function in this case is really open to generalisation past
the DA9052/53 driver.

Regards,
Steve

> >  drivers/mfd/da9052-core.c | 51
> +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> >
> > diff --git a/drivers/mfd/da9052-core.c b/drivers/mfd/da9052-core.c
> > index c0bf68a..a88c206 100644
> > --- a/drivers/mfd/da9052-core.c
> > +++ b/drivers/mfd/da9052-core.c
> > @@ -167,6 +167,7 @@ static bool da9052_reg_writeable(struct device
> *dev, unsigned int reg)
> >  	case DA9052_EVENT_B_REG:
> >  	case DA9052_EVENT_C_REG:
> >  	case DA9052_EVENT_D_REG:
> > +	case DA9052_FAULTLOG_REG:
> >  	case DA9052_IRQ_MASK_A_REG:
> >  	case DA9052_IRQ_MASK_B_REG:
> >  	case DA9052_IRQ_MASK_C_REG:
> > @@ -541,6 +542,52 @@ const struct regmap_config
> da9052_regmap_config = {
> >  };
> >  EXPORT_SYMBOL_GPL(da9052_regmap_config);
> >
> > +static int da9052_clear_fault_log(struct da9052 *da9052)
> > +{
> > +	int ret = 0;
> > +	int fault_log = 0;
> > +
> > +	fault_log = da9052_reg_read(da9052, DA9052_FAULTLOG_REG);
> > +	if (fault_log < 0) {
> > +		dev_err(da9052->dev,
> > +			"Cannot read FAULT_LOG %d\n", fault_log);
> > +		return fault_log;
> > +	}
> > +
> > +	if (fault_log) {
> > +		if (fault_log & DA9052_FAULTLOG_TWDERROR)
> > +			dev_dbg(da9052->dev,
> > +				"Fault log entry detected: TWD_ERROR\n");
> > +		if (fault_log & DA9052_FAULTLOG_VDDFAULT)
> > +			dev_dbg(da9052->dev,
> > +				"Fault log entry detected: VDD_FAULT\n");
> > +		if (fault_log & DA9052_FAULTLOG_VDDSTART)
> > +			dev_dbg(da9052->dev,
> > +				"Fault log entry detected: VDD_START\n");
> > +		if (fault_log & DA9052_FAULTLOG_TEMPOVER)
> > +			dev_dbg(da9052->dev,
> > +				"Fault log entry detected: TEMP_OVER\n");
> > +		if (fault_log & DA9052_FAULTLOG_KEYSHUT)
> > +			dev_dbg(da9052->dev,
> > +				"Fault log entry detected: KEY_SHUT\n");
> > +		if (fault_log & DA9052_FAULTLOG_NSDSET)
> > +			dev_dbg(da9052->dev,
> > +				"Fault log entry detected: nSD_SHUT\n");
> > +		if (fault_log & DA9052_FAULTLOG_WAITSET)
> > +			dev_dbg(da9052->dev,
> > +				"Fault log entry detected: WAIT_SHUT\n");
> > +
> > +		ret = da9052_reg_write(da9052,
> > +					DA9052_FAULTLOG_REG,
> > +					0xFF);
> > +		if (ret < 0)
> > +			dev_err(da9052->dev,
> > +				"Cannot reset FAULT_LOG values %d\n", ret);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> >  int da9052_device_init(struct da9052 *da9052, u8 chip_id)
> >  {
> >  	struct da9052_pdata *pdata = dev_get_platdata(da9052->dev);
> > @@ -549,6 +596,10 @@ int da9052_device_init(struct da9052 *da9052, u8 chip_id)
> >  	mutex_init(&da9052->auxadc_lock);
> >  	init_completion(&da9052->done);
> >
> > +	ret = da9052_clear_fault_log(da9052);
> > +	if (ret < 0)
> > +		dev_warn(da9052->dev, "Cannot clear FAULT_LOG\n");
> > +
> >  	if (pdata && pdata->init != NULL)
> >  		pdata->init(da9052);
> >

RE: [PATCH V1] mfd: da9053: ensure the FAULT_LOG is cleared during MFD driver probe

[Steve Twiss]

On 08 August 2016 12:06, Steve Twiss wrote:

> On 05 August 2016 10:05, Lee Jones wrote:
> > On Wed, 06 Jul 2016, Steve Twiss wrote:
> > > From: Steve Twiss <stwiss.opensource@diasemi.com>
> > >
> > > Clearance of any the persistent information within the DA9053 FAULT_LOG
> > > register must be completed during start-up so the fault-log does not
> > > continue with previous values. A clearance function has been added here in
> > > the kernel driver because wiping the fault-log cannot be counted on outside
> > > the Linux kernel.
> 
> [...]
> 
> > > This patch is similar to the requirements for DA9062 and DA9063: to ensure
> > > the FAULT_LOG is started from a 'clean' position.
> > >
> > > The Dialog datasheet for DA9053 suggests: "The FAULT_LOG register has to
> > > be cleared from the host after reading, by writing a value of 11111111".
> > >
> > > See reference:
> > >   commit 9011e4a8a6fe57f76511609930ed00d305389089
> > >   Author: Steve Twiss <stwiss.opensource@diasemi.com>
> > >   Date:   Tue May 19 11:32:45 2015 +0100
> >
> > Looks like much of the same code.  Can you have a pop at generifying
> > this to avoid any unnecessary duplication?
> 
> Hi Lee,
> 
> Although the function looks like those found in both the DA9062 and DA9063 drivers, I was just
> intending to highlight the "requirement similarities" to clear the fault register on start-up. This
> requirement for the Dialog PMIC chips extends across those devices, but the function is not
> really general enough to be merged into the kernel core or reused across other Dialog devices.
> 
> I don't think there is unnecessary duplication in this particular case for the DA9052/53 driver.
> 
> This da9052_clear_fault_log() function will apply to both the Dialog DA9052 and DA9053 PMICs
> and so I am making good use of code re-use in this place. But, this does not extend to the
> DA9062 or DA9063.
> 
> I don't think this FAULT_LOG clearance function in this case is really open to generalisation past
> the DA9052/53 driver.

Hi Lee,

There is another point I missed out for this.
The fault log function is individual for each chip and would be customisable to meet the needs
of the customer's target platform.

The function is meant to be individual. This allows for multiple chips in the same platform -- each
fault log function could be cleared slightly differently in that case.

To start with, I just added the fault log function in my last patch and highlighted its similarity
to DA9062 and DA9063 so that previous commits of the same type could be used to back-up my
request to add this new one to the DA9052/53 driver. I did not intended to cause confusion or
and it was not intended to suggest a possible point to generalise functionality across a range 
of Dialog PMICs.

Regards,
Steve

> > >  drivers/mfd/da9052-core.c | 51
> > +++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 51 insertions(+)
> > >
> > > diff --git a/drivers/mfd/da9052-core.c b/drivers/mfd/da9052-core.c
> > > index c0bf68a..a88c206 100644
> > > --- a/drivers/mfd/da9052-core.c
> > > +++ b/drivers/mfd/da9052-core.c
> > > @@ -167,6 +167,7 @@ static bool da9052_reg_writeable(struct device *dev, unsigned int reg)
> > >  	case DA9052_EVENT_B_REG:
> > >  	case DA9052_EVENT_C_REG:
> > >  	case DA9052_EVENT_D_REG:
> > > +	case DA9052_FAULTLOG_REG:
> > >  	case DA9052_IRQ_MASK_A_REG:
> > >  	case DA9052_IRQ_MASK_B_REG:
> > >  	case DA9052_IRQ_MASK_C_REG:
> > > @@ -541,6 +542,52 @@ const struct regmap_config da9052_regmap_config = {
> > >  };
> > >  EXPORT_SYMBOL_GPL(da9052_regmap_config);
> > >
> > > +static int da9052_clear_fault_log(struct da9052 *da9052)
> > > +{
> > > +	int ret = 0;
> > > +	int fault_log = 0;
> > > +
> > > +	fault_log = da9052_reg_read(da9052, DA9052_FAULTLOG_REG);
> > > +	if (fault_log < 0) {
> > > +		dev_err(da9052->dev,
> > > +			"Cannot read FAULT_LOG %d\n", fault_log);
> > > +		return fault_log;
> > > +	}
> > > +
> > > +	if (fault_log) {
> > > +		if (fault_log & DA9052_FAULTLOG_TWDERROR)
> > > +			dev_dbg(da9052->dev,
> > > +				"Fault log entry detected: TWD_ERROR\n");
> > > +		if (fault_log & DA9052_FAULTLOG_VDDFAULT)
> > > +			dev_dbg(da9052->dev,
> > > +				"Fault log entry detected: VDD_FAULT\n");
> > > +		if (fault_log & DA9052_FAULTLOG_VDDSTART)
> > > +			dev_dbg(da9052->dev,
> > > +				"Fault log entry detected: VDD_START\n");
> > > +		if (fault_log & DA9052_FAULTLOG_TEMPOVER)
> > > +			dev_dbg(da9052->dev,
> > > +				"Fault log entry detected: TEMP_OVER\n");
> > > +		if (fault_log & DA9052_FAULTLOG_KEYSHUT)
> > > +			dev_dbg(da9052->dev,
> > > +				"Fault log entry detected: KEY_SHUT\n");
> > > +		if (fault_log & DA9052_FAULTLOG_NSDSET)
> > > +			dev_dbg(da9052->dev,
> > > +				"Fault log entry detected: nSD_SHUT\n");
> > > +		if (fault_log & DA9052_FAULTLOG_WAITSET)
> > > +			dev_dbg(da9052->dev,
> > > +				"Fault log entry detected: WAIT_SHUT\n");
> > > +
> > > +		ret = da9052_reg_write(da9052,
> > > +					DA9052_FAULTLOG_REG,
> > > +					0xFF);
> > > +		if (ret < 0)
> > > +			dev_err(da9052->dev,
> > > +				"Cannot reset FAULT_LOG values %d\n", ret);
> > > +	}
> > > +
> > > +	return ret;
> > > +}
> > > +
> > >  int da9052_device_init(struct da9052 *da9052, u8 chip_id)
> > >  {
> > >  	struct da9052_pdata *pdata = dev_get_platdata(da9052->dev);
> > > @@ -549,6 +596,10 @@ int da9052_device_init(struct da9052 *da9052, u8 chip_id)
> > >  	mutex_init(&da9052->auxadc_lock);
> > >  	init_completion(&da9052->done);
> > >
> > > +	ret = da9052_clear_fault_log(da9052);
> > > +	if (ret < 0)
> > > +		dev_warn(da9052->dev, "Cannot clear FAULT_LOG\n");
> > > +
> > >  	if (pdata && pdata->init != NULL)
> > >  		pdata->init(da9052);
> > >

RE: [PATCH V1] mfd: da9053: ensure the FAULT_LOG is cleared during MFD driver probe

[Steve Twiss]

> On 08 August 2016 12:06, Steve Twiss wrote:
> > On 05 August 2016 10:05, Lee Jones wrote:
> > > On Wed, 06 Jul 2016, Steve Twiss wrote:
> > > > From: Steve Twiss <stwiss.opensource@diasemi.com>
> > > >
> > > > Clearance of any the persistent information within the DA9053 FAULT_LOG
> > > > register must be completed during start-up so the fault-log does not
> > > > continue with previous values. A clearance function has been added here in
> > > > the kernel driver because wiping the fault-log cannot be counted on outside
> > > > the Linux kernel.
> >
> > [...]
> >
> > >
> > > Looks like much of the same code.  Can you have a pop at generifying
> > > this to avoid any unnecessary duplication?
> >
> > Hi Lee,
> >
> > Although the function looks like those found in both the DA9062 and DA9063 drivers, I was just
> > intending to highlight the "requirement similarities" to clear the fault register on start-up. This
> > requirement for the Dialog PMIC chips extends across those devices, but the function is not
> > really general enough to be merged into the kernel core or reused across other Dialog devices.
> >
> > I don't think there is unnecessary duplication in this particular case for the DA9052/53 driver.
> >
> > This da9052_clear_fault_log() function will apply to both the Dialog DA9052 and DA9053 PMICs
> > and so I am making good use of code re-use in this place. But, this does not extend to the
> > DA9062 or DA9063.
> >
> > I don't think this FAULT_LOG clearance function in this case is really open to generalisation past
> > the DA9052/53 driver.
> 
> Hi Lee,
> 
[...]
> The fault log function is individual for each chip and would be customisable to meet the needs
> of the customer's target platform.
> 
> The function is meant to be individual. This allows for multiple chips in the same platform -- each
> fault log function could be cleared slightly differently in that case.
[...]

Hi Lee,
Do you have any further thoughts on this topic?
Regards,
Steve

Re: [PATCH V1] mfd: da9053: ensure the FAULT_LOG is cleared during MFD driver probe

[Lee Jones]

On Mon, 15 Aug 2016, Steve Twiss wrote:

> > On 08 August 2016 12:06, Steve Twiss wrote:
> > > On 05 August 2016 10:05, Lee Jones wrote:
> > > > On Wed, 06 Jul 2016, Steve Twiss wrote:
> > > > > From: Steve Twiss <stwiss.opensource@diasemi.com>
> > > > >
> > > > > Clearance of any the persistent information within the DA9053 FAULT_LOG
> > > > > register must be completed during start-up so the fault-log does not
> > > > > continue with previous values. A clearance function has been added here in
> > > > > the kernel driver because wiping the fault-log cannot be counted on outside
> > > > > the Linux kernel.
> > >
> > > [...]
> > >
> > > >
> > > > Looks like much of the same code.  Can you have a pop at generifying
> > > > this to avoid any unnecessary duplication?
> > >
> > > Hi Lee,
> > >
> > > Although the function looks like those found in both the DA9062 and DA9063 drivers, I was just
> > > intending to highlight the "requirement similarities" to clear the fault register on start-up. This
> > > requirement for the Dialog PMIC chips extends across those devices, but the function is not
> > > really general enough to be merged into the kernel core or reused across other Dialog devices.
> > >
> > > I don't think there is unnecessary duplication in this particular case for the DA9052/53 driver.
> > >
> > > This da9052_clear_fault_log() function will apply to both the Dialog DA9052 and DA9053 PMICs
> > > and so I am making good use of code re-use in this place. But, this does not extend to the
> > > DA9062 or DA9063.
> > >
> > > I don't think this FAULT_LOG clearance function in this case is really open to generalisation past
> > > the DA9052/53 driver.
> > 
> > Hi Lee,
> > 
> [...]
> > The fault log function is individual for each chip and would be customisable to meet the needs
> > of the customer's target platform.
> > 
> > The function is meant to be individual. This allows for multiple chips in the same platform -- each
> > fault log function could be cleared slightly differently in that case.
> [...]
> 
> Hi Lee,
> Do you have any further thoughts on this topic?

I have it marked as "TODO" in my Inbox.

The plan is to have a `brain diff` when I can find some free time.

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

Re: [PATCH V1] mfd: da9053: ensure the FAULT_LOG is cleared during MFD driver probe

[Lee Jones]

On Wed, 06 Jul 2016, Steve Twiss wrote:

> From: Steve Twiss <stwiss.opensource@diasemi.com>
> 
> The function da9052_clear_fault_log() is added to mitigate the case of
> persistent data being transferred between reboots.
> 
> Clearance of any the persistent information within the DA9053 FAULT_LOG
> register must be completed during start-up so the fault-log does not
> continue with previous values. A clearance function has been added here in
> the kernel driver because wiping the fault-log cannot be counted on outside
> the Linux kernel.
> 
> Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
> Reviewed-by: Adam Thomson <adam.thomson.opensource@diasemi.com>

Applied, thanks.
> ---
> This patch applies against linux-next and v4.7-rc6
> 
> 
> Hi Lee,
> 
> This patch is similar to the requirements for DA9062 and DA9063: to ensure
> the FAULT_LOG is started from a 'clean' position.
> 
> The Dialog datasheet for DA9053 suggests: "The FAULT_LOG register has to
> be cleared from the host after reading, by writing a value of 11111111".
> 
> See reference:
>   commit 9011e4a8a6fe57f76511609930ed00d305389089
>   Author: Steve Twiss <stwiss.opensource@diasemi.com>
>   Date:   Tue May 19 11:32:45 2015 +0100
> 
> Regards,
> Steve
> 
> 
>  drivers/mfd/da9052-core.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/mfd/da9052-core.c b/drivers/mfd/da9052-core.c
> index c0bf68a..a88c206 100644
> --- a/drivers/mfd/da9052-core.c
> +++ b/drivers/mfd/da9052-core.c
> @@ -167,6 +167,7 @@ static bool da9052_reg_writeable(struct device *dev, unsigned int reg)
>  	case DA9052_EVENT_B_REG:
>  	case DA9052_EVENT_C_REG:
>  	case DA9052_EVENT_D_REG:
> +	case DA9052_FAULTLOG_REG:
>  	case DA9052_IRQ_MASK_A_REG:
>  	case DA9052_IRQ_MASK_B_REG:
>  	case DA9052_IRQ_MASK_C_REG:
> @@ -541,6 +542,52 @@ const struct regmap_config da9052_regmap_config = {
>  };
>  EXPORT_SYMBOL_GPL(da9052_regmap_config);
>  
> +static int da9052_clear_fault_log(struct da9052 *da9052)
> +{
> +	int ret = 0;
> +	int fault_log = 0;
> +
> +	fault_log = da9052_reg_read(da9052, DA9052_FAULTLOG_REG);
> +	if (fault_log < 0) {
> +		dev_err(da9052->dev,
> +			"Cannot read FAULT_LOG %d\n", fault_log);
> +		return fault_log;
> +	}
> +
> +	if (fault_log) {
> +		if (fault_log & DA9052_FAULTLOG_TWDERROR)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: TWD_ERROR\n");
> +		if (fault_log & DA9052_FAULTLOG_VDDFAULT)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: VDD_FAULT\n");
> +		if (fault_log & DA9052_FAULTLOG_VDDSTART)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: VDD_START\n");
> +		if (fault_log & DA9052_FAULTLOG_TEMPOVER)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: TEMP_OVER\n");
> +		if (fault_log & DA9052_FAULTLOG_KEYSHUT)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: KEY_SHUT\n");
> +		if (fault_log & DA9052_FAULTLOG_NSDSET)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: nSD_SHUT\n");
> +		if (fault_log & DA9052_FAULTLOG_WAITSET)
> +			dev_dbg(da9052->dev,
> +				"Fault log entry detected: WAIT_SHUT\n");
> +
> +		ret = da9052_reg_write(da9052,
> +					DA9052_FAULTLOG_REG,
> +					0xFF);
> +		if (ret < 0)
> +			dev_err(da9052->dev,
> +				"Cannot reset FAULT_LOG values %d\n", ret);
> +	}
> +
> +	return ret;
> +}
> +
>  int da9052_device_init(struct da9052 *da9052, u8 chip_id)
>  {
>  	struct da9052_pdata *pdata = dev_get_platdata(da9052->dev);
> @@ -549,6 +596,10 @@ int da9052_device_init(struct da9052 *da9052, u8 chip_id)
>  	mutex_init(&da9052->auxadc_lock);
>  	init_completion(&da9052->done);
>  
> +	ret = da9052_clear_fault_log(da9052);
> +	if (ret < 0)
> +		dev_warn(da9052->dev, "Cannot clear FAULT_LOG\n");
> +
>  	if (pdata && pdata->init != NULL)
>  		pdata->init(da9052);
>  

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