Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] watchdog: da9062: make restart handler atomic safe
@ 2020-01-13  9:15 Marco Felsch
  2020-01-13 20:28 ` Guenter Roeck
  2020-01-14 20:00 ` Stefan Lengfeld
  0 siblings, 2 replies; 5+ messages in thread
From: Marco Felsch @ 2020-01-13  9:15 UTC (permalink / raw)
  To: support.opensource, linux, contact, Adam.Thomson.Opensource
  Cc: linux-watchdog, linux-kernel, kernel

The restart handler is executed during the shutdown phase which is
atomic/irq-less. The i2c framework supports atomic transfers since
commit 63b96983a5dd ("i2c: core: introduce callbacks for atomic
transfers") but unfortunately the regmap framework doesn't support it
yet. Hard coding the i2c stuff can be done without worries since the
DA9062 is an i2c-only device.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
Hi,

This patch is based on Stefan Lengfeld's RFC Patch [1].

[1] https://patchwork.ozlabs.org/patch/1085942/
---
 drivers/watchdog/da9062_wdt.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c
index c9b9d6394525..84c5a0a455b2 100644
--- a/drivers/watchdog/da9062_wdt.c
+++ b/drivers/watchdog/da9062_wdt.c
@@ -11,6 +11,7 @@
 #include <linux/platform_device.h>
 #include <linux/uaccess.h>
 #include <linux/slab.h>
+#include <linux/i2c.h>
 #include <linux/delay.h>
 #include <linux/jiffies.h>
 #include <linux/mfd/da9062/registers.h>
@@ -149,12 +150,18 @@ static int da9062_wdt_restart(struct watchdog_device *wdd, unsigned long action,
 			      void *data)
 {
 	struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd);
+	struct i2c_client *client = to_i2c_client(wdt->hw->dev);
+	u8 buf[] = {DA9062AA_CONTROL_F, DA9062AA_SHUTDOWN_MASK};
+	struct i2c_msg msg = {
+		.addr = client->addr,
+		.flags = 0,
+		.len = sizeof(buf),
+		.buf = buf,
+	};
 	int ret;
 
-	ret = regmap_write(wdt->hw->regmap,
-			   DA9062AA_CONTROL_F,
-			   DA9062AA_SHUTDOWN_MASK);
-	if (ret)
+	ret = i2c_transfer(client->adapter, &msg, 1);
+	if (ret < 0)
 		dev_alert(wdt->hw->dev, "Failed to shutdown (err = %d)\n",
 			  ret);
 
-- 
2.20.1


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

* Re: [PATCH] watchdog: da9062: make restart handler atomic safe
  2020-01-13  9:15 [PATCH] watchdog: da9062: make restart handler atomic safe Marco Felsch
@ 2020-01-13 20:28 ` Guenter Roeck
  2020-01-14 17:30   ` Marco Felsch
  2020-01-14 20:00 ` Stefan Lengfeld
  1 sibling, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2020-01-13 20:28 UTC (permalink / raw)
  To: Marco Felsch
  Cc: support.opensource, contact, Adam.Thomson.Opensource,
	linux-watchdog, linux-kernel, kernel

On Mon, Jan 13, 2020 at 10:15:21AM +0100, Marco Felsch wrote:
> The restart handler is executed during the shutdown phase which is
> atomic/irq-less. The i2c framework supports atomic transfers since
> commit 63b96983a5dd ("i2c: core: introduce callbacks for atomic
> transfers") but unfortunately the regmap framework doesn't support it
> yet. Hard coding the i2c stuff can be done without worries since the
> DA9062 is an i2c-only device.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> Hi,
> 
> This patch is based on Stefan Lengfeld's RFC Patch [1].
> 
> [1] https://patchwork.ozlabs.org/patch/1085942/
> ---
>  drivers/watchdog/da9062_wdt.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c
> index c9b9d6394525..84c5a0a455b2 100644
> --- a/drivers/watchdog/da9062_wdt.c
> +++ b/drivers/watchdog/da9062_wdt.c
> @@ -11,6 +11,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/uaccess.h>
>  #include <linux/slab.h>
> +#include <linux/i2c.h>
>  #include <linux/delay.h>
>  #include <linux/jiffies.h>
>  #include <linux/mfd/da9062/registers.h>
> @@ -149,12 +150,18 @@ static int da9062_wdt_restart(struct watchdog_device *wdd, unsigned long action,
>  			      void *data)
>  {
>  	struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd);
> +	struct i2c_client *client = to_i2c_client(wdt->hw->dev);
> +	u8 buf[] = {DA9062AA_CONTROL_F, DA9062AA_SHUTDOWN_MASK};
> +	struct i2c_msg msg = {
> +		.addr = client->addr,
> +		.flags = 0,
> +		.len = sizeof(buf),
> +		.buf = buf,
> +	};
>  	int ret;
>  
> -	ret = regmap_write(wdt->hw->regmap,
> -			   DA9062AA_CONTROL_F,
> -			   DA9062AA_SHUTDOWN_MASK);
> -	if (ret)
> +	ret = i2c_transfer(client->adapter, &msg, 1);

Why not i2c_smbus_write_byte_data() ? I don't immediately see the difference.

Guenter

> +	if (ret < 0)
>  		dev_alert(wdt->hw->dev, "Failed to shutdown (err = %d)\n",
>  			  ret);
>  
> -- 
> 2.20.1
> 

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

* Re: [PATCH] watchdog: da9062: make restart handler atomic safe
  2020-01-13 20:28 ` Guenter Roeck
@ 2020-01-14 17:30   ` Marco Felsch
  0 siblings, 0 replies; 5+ messages in thread
From: Marco Felsch @ 2020-01-14 17:30 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: support.opensource, contact, Adam.Thomson.Opensource,
	linux-watchdog, linux-kernel, kernel

Hi,

On 20-01-13 12:28, Guenter Roeck wrote:
> On Mon, Jan 13, 2020 at 10:15:21AM +0100, Marco Felsch wrote:
> > The restart handler is executed during the shutdown phase which is
> > atomic/irq-less. The i2c framework supports atomic transfers since
> > commit 63b96983a5dd ("i2c: core: introduce callbacks for atomic
> > transfers") but unfortunately the regmap framework doesn't support it
> > yet. Hard coding the i2c stuff can be done without worries since the
> > DA9062 is an i2c-only device.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> > Hi,
> > 
> > This patch is based on Stefan Lengfeld's RFC Patch [1].
> > 
> > [1] https://patchwork.ozlabs.org/patch/1085942/
> > ---
> >  drivers/watchdog/da9062_wdt.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c
> > index c9b9d6394525..84c5a0a455b2 100644
> > --- a/drivers/watchdog/da9062_wdt.c
> > +++ b/drivers/watchdog/da9062_wdt.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/uaccess.h>
> >  #include <linux/slab.h>
> > +#include <linux/i2c.h>
> >  #include <linux/delay.h>
> >  #include <linux/jiffies.h>
> >  #include <linux/mfd/da9062/registers.h>
> > @@ -149,12 +150,18 @@ static int da9062_wdt_restart(struct watchdog_device *wdd, unsigned long action,
> >  			      void *data)
> >  {
> >  	struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd);
> > +	struct i2c_client *client = to_i2c_client(wdt->hw->dev);
> > +	u8 buf[] = {DA9062AA_CONTROL_F, DA9062AA_SHUTDOWN_MASK};
> > +	struct i2c_msg msg = {
> > +		.addr = client->addr,
> > +		.flags = 0,
> > +		.len = sizeof(buf),
> > +		.buf = buf,
> > +	};
> >  	int ret;
> >  
> > -	ret = regmap_write(wdt->hw->regmap,
> > -			   DA9062AA_CONTROL_F,
> > -			   DA9062AA_SHUTDOWN_MASK);
> > -	if (ret)
> > +	ret = i2c_transfer(client->adapter, &msg, 1);
> 
> Why not i2c_smbus_write_byte_data() ? I don't immediately see the difference.

Because I didn't noticed it, sorry. I changed it and notice no
differences. Thanks for the review =)

Regards,
  Marco

> Guenter
> 
> > +	if (ret < 0)
> >  		dev_alert(wdt->hw->dev, "Failed to shutdown (err = %d)\n",
> >  			  ret);
> >  
> > -- 
> > 2.20.1
> > 

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

* Re: [PATCH] watchdog: da9062: make restart handler atomic safe
  2020-01-13  9:15 [PATCH] watchdog: da9062: make restart handler atomic safe Marco Felsch
  2020-01-13 20:28 ` Guenter Roeck
@ 2020-01-14 20:00 ` Stefan Lengfeld
  2020-01-15  7:19   ` Marco Felsch
  1 sibling, 1 reply; 5+ messages in thread
From: Stefan Lengfeld @ 2020-01-14 20:00 UTC (permalink / raw)
  To: Marco Felsch
  Cc: support.opensource, linux, Adam.Thomson.Opensource,
	linux-watchdog, linux-kernel, kernel

Hi Marco,

On Mon, Jan 13, 2020 at 10:15:21AM +0100, Marco Felsch wrote:
> The restart handler is executed during the shutdown phase which is
> atomic/irq-less. The i2c framework supports atomic transfers since
> commit 63b96983a5dd ("i2c: core: introduce callbacks for atomic
> transfers") but unfortunately the regmap framework doesn't support it
> yet. Hard coding the i2c stuff can be done without worries since the
> DA9062 is an i2c-only device.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Reviewed-by: Stefan Lengfeld <contact@stefanchrist.eu>

Thanks for picking up the patch and submitting it.

Kind regards,
    Stefan

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

* Re: [PATCH] watchdog: da9062: make restart handler atomic safe
  2020-01-14 20:00 ` Stefan Lengfeld
@ 2020-01-15  7:19   ` Marco Felsch
  0 siblings, 0 replies; 5+ messages in thread
From: Marco Felsch @ 2020-01-15  7:19 UTC (permalink / raw)
  To: Stefan Lengfeld
  Cc: support.opensource, linux, Adam.Thomson.Opensource,
	linux-watchdog, linux-kernel, kernel

Hi Stefan,

On 20-01-14 21:00, Stefan Lengfeld wrote:
> Hi Marco,
> 
> On Mon, Jan 13, 2020 at 10:15:21AM +0100, Marco Felsch wrote:
> > The restart handler is executed during the shutdown phase which is
> > atomic/irq-less. The i2c framework supports atomic transfers since
> > commit 63b96983a5dd ("i2c: core: introduce callbacks for atomic
> > transfers") but unfortunately the regmap framework doesn't support it
> > yet. Hard coding the i2c stuff can be done without worries since the
> > DA9062 is an i2c-only device.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> 
> Reviewed-by: Stefan Lengfeld <contact@stefanchrist.eu>
> 
> Thanks for picking up the patch and submitting it.

I will send a v2 to cover Guenter's suggestion. Can I keep your reviewed
by tag?

Regards,
  Marco

> Kind regards,
>     Stefan

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-13  9:15 [PATCH] watchdog: da9062: make restart handler atomic safe Marco Felsch
2020-01-13 20:28 ` Guenter Roeck
2020-01-14 17:30   ` Marco Felsch
2020-01-14 20:00 ` Stefan Lengfeld
2020-01-15  7:19   ` Marco Felsch

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org
	public-inbox-index linux-watchdog

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git