[PATCH 0/2] hwmon: (lm70) Couple patches

[PATCH 0/2] hwmon: (lm70) Couple patches

[Florian Fainelli]

Hi Guenter,

Here are a couple of patches against lm70. I plan on submitting
support for configurable thresholds on TI TMP122/124 once I have
resolved the lack SPI_3WIRE on the controller that connects to
this chip on my board.

Thanks!

Florian Fainelli (2):
  hwmon: (lm70) Utilize dev_warn instead of pr_warn
  hwmon: (lm70) Add support for TI TMP122/124

 Documentation/devicetree/bindings/hwmon/lm70.txt |  1 +
 Documentation/hwmon/lm70                         |  8 ++++++--
 drivers/hwmon/lm70.c                             | 12 ++++++++++--
 3 files changed, 17 insertions(+), 4 deletions(-)

-- 
2.9.3

[PATCH 1/2] hwmon: (lm70) Utilize dev_warn instead of pr_warn

[Florian Fainelli]

We have a device reference, utilize it instead of pr_warn().

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/hwmon/lm70.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
index d6ecd1a4be59..52c5cdd00448 100644
--- a/drivers/hwmon/lm70.c
+++ b/drivers/hwmon/lm70.c
@@ -72,7 +72,8 @@ static ssize_t temp1_input_show(struct device *dev,
 	 */
 	status = spi_write_then_read(spi, NULL, 0, &rxbuf[0], 2);
 	if (status < 0) {
-		pr_warn("spi_write_then_read failed with status %d\n", status);
+		dev_warn(dev, "spi_write_then_read failed with status %d\n",
+			 status);
 		goto out;
 	}
 	raw = (rxbuf[0] << 8) + rxbuf[1];
-- 
2.9.3

[PATCH 2/2] hwmon: (lm70) Add support for TI TMP122/124

[Florian Fainelli]

Add support for Texas Instruments TMP122/124 which are nearly identical to
their TMP121/123 except that they also support programmable temperature
thresholds.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/devicetree/bindings/hwmon/lm70.txt | 1 +
 Documentation/hwmon/lm70                         | 8 ++++++--
 drivers/hwmon/lm70.c                             | 9 ++++++++-
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/hwmon/lm70.txt b/Documentation/devicetree/bindings/hwmon/lm70.txt
index e7fd921aa4f1..ea417a0d32af 100644
--- a/Documentation/devicetree/bindings/hwmon/lm70.txt
+++ b/Documentation/devicetree/bindings/hwmon/lm70.txt
@@ -4,6 +4,7 @@ Required properties:
 - compatible: one of
 		"ti,lm70"
 		"ti,tmp121"
+		"ti,tmp122"
 		"ti,lm71"
 		"ti,lm74"
 
diff --git a/Documentation/hwmon/lm70 b/Documentation/hwmon/lm70
index 1bb2db440671..c3a1f2ea017d 100644
--- a/Documentation/hwmon/lm70
+++ b/Documentation/hwmon/lm70
@@ -6,6 +6,8 @@ Supported chips:
     Datasheet: http://www.national.com/pf/LM/LM70.html
   * Texas Instruments TMP121/TMP123
     Information: http://focus.ti.com/docs/prod/folders/print/tmp121.html
+  * Texas Instruments TMP122/TMP124
+    Information: http://www.ti.com/product/tmp122
   * National Semiconductor LM71
     Datasheet: http://www.ti.com/product/LM71
   * National Semiconductor LM74
@@ -35,8 +37,10 @@ As a real (in-tree) example of this "SPI protocol driver" interfacing
 with a "SPI master controller driver", see drivers/spi/spi_lm70llp.c
 and its associated documentation.
 
-The LM74 and TMP121/TMP123 are very similar; main difference is 13-bit
-temperature data (0.0625 degrees celsius resolution).
+The LM74 and TMP121/TMP122/TMP123/TMP124 are very similar; main difference is
+13-bit temperature data (0.0625 degrees celsius resolution).
+
+The TMP122/TMP124 also feature configurable temperature thresholds.
 
 The LM71 is also very similar; main difference is 14-bit temperature
 data (0.03125 degrees celsius resolution).
diff --git a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
index 52c5cdd00448..543556dc563b 100644
--- a/drivers/hwmon/lm70.c
+++ b/drivers/hwmon/lm70.c
@@ -46,6 +46,7 @@
 #define LM70_CHIP_TMP121	1	/* TI TMP121/TMP123 */
 #define LM70_CHIP_LM71		2	/* NS LM71 */
 #define LM70_CHIP_LM74		3	/* NS LM74 */
+#define LM70_CHIP_TMP122	4	/* TI TMP122/TMP124 */
 
 struct lm70 {
 	struct spi_device *spi;
@@ -92,7 +93,7 @@ static ssize_t temp1_input_show(struct device *dev,
 	 * Celsius.
 	 * So it's equivalent to multiplying by 0.25 * 1000 = 250.
 	 *
-	 * LM74 and TMP121/TMP123:
+	 * LM74 and TMP121/TMP122/TMP123/TMP124:
 	 * 13 bits of 2's complement data, discard LSB 3 bits,
 	 * resolution 0.0625 degrees celsius.
 	 *
@@ -106,6 +107,7 @@ static ssize_t temp1_input_show(struct device *dev,
 		break;
 
 	case LM70_CHIP_TMP121:
+	case LM70_CHIP_TMP122:
 	case LM70_CHIP_LM74:
 		val = ((int)raw / 8) * 625 / 10;
 		break;
@@ -143,6 +145,10 @@ static const struct of_device_id lm70_of_ids[] = {
 		.data = (void *) LM70_CHIP_TMP121,
 	},
 	{
+		.compatible = "ti,tmp122",
+		.data = (void *) LM70_CHIP_TMP122,
+	},
+	{
 		.compatible = "ti,lm71",
 		.data = (void *) LM70_CHIP_LM71,
 	},
@@ -191,6 +197,7 @@ static int lm70_probe(struct spi_device *spi)
 static const struct spi_device_id lm70_ids[] = {
 	{ "lm70",   LM70_CHIP_LM70 },
 	{ "tmp121", LM70_CHIP_TMP121 },
+	{ "tmp122", LM70_CHIP_TMP122 },
 	{ "lm71",   LM70_CHIP_LM71 },
 	{ "lm74",   LM70_CHIP_LM74 },
 	{ },
-- 
2.9.3

Re: [PATCH 1/2] hwmon: (lm70) Utilize dev_warn instead of pr_warn

[Guenter Roeck]

On 01/21/2017 11:20 AM, Florian Fainelli wrote:
> We have a device reference, utilize it instead of pr_warn().
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---

Applied to -next.

Thanks,
Guenter

Re: [PATCH 2/2] hwmon: (lm70) Add support for TI TMP122/124

[Guenter Roeck]

On 01/21/2017 11:20 AM, Florian Fainelli wrote:
> Add support for Texas Instruments TMP122/124 which are nearly identical to
> their TMP121/123 except that they also support programmable temperature
> thresholds.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Applied to -next.

Any plans to add support for the thresholds ?

Thanks,
Guenter

> ---
>  Documentation/devicetree/bindings/hwmon/lm70.txt | 1 +
>  Documentation/hwmon/lm70                         | 8 ++++++--
>  drivers/hwmon/lm70.c                             | 9 ++++++++-
>  3 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/lm70.txt b/Documentation/devicetree/bindings/hwmon/lm70.txt
> index e7fd921aa4f1..ea417a0d32af 100644
> --- a/Documentation/devicetree/bindings/hwmon/lm70.txt
> +++ b/Documentation/devicetree/bindings/hwmon/lm70.txt
> @@ -4,6 +4,7 @@ Required properties:
>  - compatible: one of
>  		"ti,lm70"
>  		"ti,tmp121"
> +		"ti,tmp122"
>  		"ti,lm71"
>  		"ti,lm74"
>
> diff --git a/Documentation/hwmon/lm70 b/Documentation/hwmon/lm70
> index 1bb2db440671..c3a1f2ea017d 100644
> --- a/Documentation/hwmon/lm70
> +++ b/Documentation/hwmon/lm70
> @@ -6,6 +6,8 @@ Supported chips:
>      Datasheet: http://www.national.com/pf/LM/LM70.html
>    * Texas Instruments TMP121/TMP123
>      Information: http://focus.ti.com/docs/prod/folders/print/tmp121.html
> +  * Texas Instruments TMP122/TMP124
> +    Information: http://www.ti.com/product/tmp122
>    * National Semiconductor LM71
>      Datasheet: http://www.ti.com/product/LM71
>    * National Semiconductor LM74
> @@ -35,8 +37,10 @@ As a real (in-tree) example of this "SPI protocol driver" interfacing
>  with a "SPI master controller driver", see drivers/spi/spi_lm70llp.c
>  and its associated documentation.
>
> -The LM74 and TMP121/TMP123 are very similar; main difference is 13-bit
> -temperature data (0.0625 degrees celsius resolution).
> +The LM74 and TMP121/TMP122/TMP123/TMP124 are very similar; main difference is
> +13-bit temperature data (0.0625 degrees celsius resolution).
> +
> +The TMP122/TMP124 also feature configurable temperature thresholds.
>
>  The LM71 is also very similar; main difference is 14-bit temperature
>  data (0.03125 degrees celsius resolution).
> diff --git a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
> index 52c5cdd00448..543556dc563b 100644
> --- a/drivers/hwmon/lm70.c
> +++ b/drivers/hwmon/lm70.c
> @@ -46,6 +46,7 @@
>  #define LM70_CHIP_TMP121	1	/* TI TMP121/TMP123 */
>  #define LM70_CHIP_LM71		2	/* NS LM71 */
>  #define LM70_CHIP_LM74		3	/* NS LM74 */
> +#define LM70_CHIP_TMP122	4	/* TI TMP122/TMP124 */
>
>  struct lm70 {
>  	struct spi_device *spi;
> @@ -92,7 +93,7 @@ static ssize_t temp1_input_show(struct device *dev,
>  	 * Celsius.
>  	 * So it's equivalent to multiplying by 0.25 * 1000 = 250.
>  	 *
> -	 * LM74 and TMP121/TMP123:
> +	 * LM74 and TMP121/TMP122/TMP123/TMP124:
>  	 * 13 bits of 2's complement data, discard LSB 3 bits,
>  	 * resolution 0.0625 degrees celsius.
>  	 *
> @@ -106,6 +107,7 @@ static ssize_t temp1_input_show(struct device *dev,
>  		break;
>
>  	case LM70_CHIP_TMP121:
> +	case LM70_CHIP_TMP122:
>  	case LM70_CHIP_LM74:
>  		val = ((int)raw / 8) * 625 / 10;
>  		break;
> @@ -143,6 +145,10 @@ static const struct of_device_id lm70_of_ids[] = {
>  		.data = (void *) LM70_CHIP_TMP121,
>  	},
>  	{
> +		.compatible = "ti,tmp122",
> +		.data = (void *) LM70_CHIP_TMP122,
> +	},
> +	{
>  		.compatible = "ti,lm71",
>  		.data = (void *) LM70_CHIP_LM71,
>  	},
> @@ -191,6 +197,7 @@ static int lm70_probe(struct spi_device *spi)
>  static const struct spi_device_id lm70_ids[] = {
>  	{ "lm70",   LM70_CHIP_LM70 },
>  	{ "tmp121", LM70_CHIP_TMP121 },
> +	{ "tmp122", LM70_CHIP_TMP122 },
>  	{ "lm71",   LM70_CHIP_LM71 },
>  	{ "lm74",   LM70_CHIP_LM74 },
>  	{ },
>

Re: [PATCH 2/2] hwmon: (lm70) Add support for TI TMP122/124

[Florian Fainelli]

On January 21, 2017 12:25:21 PM PST, Guenter Roeck <linux@roeck-us.net> wrote:
>On 01/21/2017 11:20 AM, Florian Fainelli wrote:
>> Add support for Texas Instruments TMP122/124 which are nearly
>identical to
>> their TMP121/123 except that they also support programmable
>temperature
>> thresholds.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
>Applied to -next.

Thanks was fast, thanks!

>
>Any plans to add support for the thresholds ?

Yes, as mentioned in the cover letter, need to get SPI_3WIRE working with the SPI controller (spi-ep93xx) to verify the thresholds do work.

Cheers

-- 
Florian

Re: [PATCH 2/2] hwmon: (lm70) Add support for TI TMP122/124

[Guenter Roeck]

On 01/21/2017 03:11 PM, Florian Fainelli wrote:
> On January 21, 2017 12:25:21 PM PST, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 01/21/2017 11:20 AM, Florian Fainelli wrote:
>>> Add support for Texas Instruments TMP122/124 which are nearly
>> identical to
>>> their TMP121/123 except that they also support programmable
>> temperature
>>> thresholds.
>>>
>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>
>> Applied to -next.
>
> Thanks was fast, thanks!
>

Lucky you, the patches caught up with me me while I was going through the backlog.

>>
>> Any plans to add support for the thresholds ?
>
> Yes, as mentioned in the cover letter, need to get SPI_3WIRE working with the SPI controller (spi-ep93xx) to verify the thresholds do work.
>
Guess the hint is that one should read the cover letter :-)

Guenter

Re: [PATCH 1/2] hwmon: (lm70) Utilize dev_warn instead of pr_warn

[Joe Perches]

On Sat, 2017-01-21 at 11:20 -0800, Florian Fainelli wrote:
> We have a device reference, utilize it instead of pr_warn().

There is at least one more hwmon to convert in applesmc.c

Perhaps a coccinelle script?

Two questions for Julia Lawall:

o is there a better way to do this than repeat the blocks
  one for each replacement
o can struct device * dev be made an arbitrary identifier

$ cat dev_printk.cocci
@@
identifier fn;
type T;
@@

T fn ( ..., struct device * dev, ... ) {
<...
-	pr_emerg(
+	dev_emerg(dev,
	...);
...>
}

@@
identifier fn;
type T;
@@

T fn ( ..., struct device * dev, ... ) {
<...
-	pr_crit(
+	dev_crit(dev,
	...);
...>
}

@@
identifier fn;
type T;
@@

T fn ( ..., struct device * dev, ... ) {
<...
-	pr_alert(
+	dev_alert(dev,
	...);
...>
}

@@
identifier fn;
type T;
@@

T fn ( ..., struct device * dev, ... ) {
<...
-	pr_err(
+	dev_err(dev,
	...);
...>
}

@@
identifier fn;
type T;
@@

T fn ( ..., struct device * dev, ... ) {
<...
-	pr_notice(
+	dev_notice(dev,
	...);
...>
}

@@
identifier fn;
type T;
@@

T fn ( ..., struct device * dev, ... ) {
<...
-	pr_warn(
+	dev_warn(dev,
	...);
...>
}

@@
identifier fn;
type T;
@@

T fn ( ..., struct device * dev, ... ) {
<...
-	pr_info(
+	dev_info(dev,
	...);
...>
}

@@
identifier fn;
type T;
@@

T fn ( ..., struct device * dev, ... ) {
<...
-	pr_debug(
+	dev_dbg(dev,
	...);
...>
}

Re: [PATCH 1/2] hwmon: (lm70) Utilize dev_warn instead of pr_warn

[Joe Perches]

On Sun, 2017-01-22 at 22:43 -0800, Joe Perches wrote:
> Two questions for Julia Lawall:
> 
> o is there a better way to do this than repeat the blocks
>   one for each replacement
> o can struct device * dev be made an arbitrary identifier
> 
> $ cat dev_printk.cocci
> @@
> identifier fn;
> type T;
> @@
> 
> T fn ( ..., struct device * dev, ... ) {
> <...
> -	pr_emerg(
> +	dev_emerg(dev,
> 	...);
> ...>
> }

Well, the second question is simple if I would just
think a little before asking...

@@
identifier fn;
identifier dev;
type T;
@@

T fn ( ..., struct device * dev, ... ) {
<...
-	pr_emerg(
+	dev_emerg(dev,
	...);
...>
}

etc...

Re: [PATCH 1/2] hwmon: (lm70) Utilize dev_warn instead of pr_warn

[Guenter Roeck]

On 01/22/2017 10:43 PM, Joe Perches wrote:
> On Sat, 2017-01-21 at 11:20 -0800, Florian Fainelli wrote:
>> We have a device reference, utilize it instead of pr_warn().
>
> There is at least one more hwmon to convert in applesmc.c
>
> Perhaps a coccinelle script?
>
> Two questions for Julia Lawall:
>
> o is there a better way to do this than repeat the blocks
>   one for each replacement
> o can struct device * dev be made an arbitrary identifier

Definitely yes here; otherwise you only catch the ones named 'dev'.
Did you try "identifier dev;" ?

The type of fn is irrelevant; you don't need to specify it.

There is also the case where 'struct device *dev' is a local variable

fn(...) {
...
struct device *dev = e;
<...
...>
}

or when it isn't but is still available

fn (..., struct \(platform_device\|i2c_device\|spi_device\) *pdev, ...) {
}

>
> $ cat dev_printk.cocci
> @@
> identifier fn;
> type T;
> @@
>
> T fn ( ..., struct device * dev, ... ) {
> <...
> -	pr_emerg(
> +	dev_emerg(dev,
> 	...);
> ...>
> }
>
> @@
> identifier fn;
> type T;
> @@
>
> T fn ( ..., struct device * dev, ... ) {
> <...
> -	pr_crit(
> +	dev_crit(dev,
> 	...);
> ...>
> }
>
> @@
> identifier fn;
> type T;
> @@
>
> T fn ( ..., struct device * dev, ... ) {
> <...
> -	pr_alert(
> +	dev_alert(dev,
> 	...);
> ...>
> }
>
> @@
> identifier fn;
> type T;
> @@
>
> T fn ( ..., struct device * dev, ... ) {
> <...
> -	pr_err(
> +	dev_err(dev,
> 	...);
> ...>
> }
>
> @@
> identifier fn;
> type T;
> @@
>
> T fn ( ..., struct device * dev, ... ) {
> <...
> -	pr_notice(
> +	dev_notice(dev,
> 	...);
> ...>
> }
>
> @@
> identifier fn;
> type T;
> @@
>
> T fn ( ..., struct device * dev, ... ) {
> <...
> -	pr_warn(
> +	dev_warn(dev,
> 	...);
> ...>
> }
>
> @@
> identifier fn;
> type T;
> @@
>
> T fn ( ..., struct device * dev, ... ) {
> <...
> -	pr_info(
> +	dev_info(dev,
> 	...);
> ...>
> }
>
> @@
> identifier fn;
> type T;
> @@
>
> T fn ( ..., struct device * dev, ... ) {
> <...
> -	pr_debug(
> +	dev_dbg(dev,
> 	...);
> ...>
> }
>
>

Re: [PATCH 1/2] hwmon: (lm70) Utilize dev_warn instead of pr_warn

[Julia Lawall]

On Sun, 22 Jan 2017, Guenter Roeck wrote:

> On 01/22/2017 10:43 PM, Joe Perches wrote:
> > On Sat, 2017-01-21 at 11:20 -0800, Florian Fainelli wrote:
> > > We have a device reference, utilize it instead of pr_warn().
> >
> > There is at least one more hwmon to convert in applesmc.c
> >
> > Perhaps a coccinelle script?
> >
> > Two questions for Julia Lawall:
> >
> > o is there a better way to do this than repeat the blocks
> >   one for each replacement
> > o can struct device * dev be made an arbitrary identifier
>
> Definitely yes here; otherwise you only catch the ones named 'dev'.
> Did you try "identifier dev;" ?

Definitely do that.

>
> The type of fn is irrelevant; you don't need to specify it.

Agreed.

> There is also the case where 'struct device *dev' is a local variable
>
> fn(...) {
> ...
> struct device *dev = e;
> <...
> ...>
> }

Here, you don't need the fn(...) { ... } part.  It would also be good to
say:

expression e != NULL;

dev could also be initialized:

@@
struct device *dev;
expression e != NULL;
expression e1;
@@

dev = e;
<...
-
+
...>
? dev = e1; // stop when dev is reinitialized, to avoid a double match

The rules can all be merged together with a disjunction:

(
- pr_emerg
+ dev_emerg
|
- pr_crit
+ dev_crit  // fill in all cases
)
  (
+ dev,
  ...)

julia


> or when it isn't but is still available
>
> fn (..., struct \(platform_device\|i2c_device\|spi_device\) *pdev, ...) {
> }
>
> >
> > $ cat dev_printk.cocci
> > @@
> > identifier fn;
> > type T;
> > @@
> >
> > T fn ( ..., struct device * dev, ... ) {
> > <...
> > -	pr_emerg(
> > +	dev_emerg(dev,
> > 	...);
> > ...>
> > }
> >
> > @@
> > identifier fn;
> > type T;
> > @@
> >
> > T fn ( ..., struct device * dev, ... ) {
> > <...
> > -	pr_crit(
> > +	dev_crit(dev,
> > 	...);
> > ...>
> > }
> >
> > @@
> > identifier fn;
> > type T;
> > @@
> >
> > T fn ( ..., struct device * dev, ... ) {
> > <...
> > -	pr_alert(
> > +	dev_alert(dev,
> > 	...);
> > ...>
> > }
> >
> > @@
> > identifier fn;
> > type T;
> > @@
> >
> > T fn ( ..., struct device * dev, ... ) {
> > <...
> > -	pr_err(
> > +	dev_err(dev,
> > 	...);
> > ...>
> > }
> >
> > @@
> > identifier fn;
> > type T;
> > @@
> >
> > T fn ( ..., struct device * dev, ... ) {
> > <...
> > -	pr_notice(
> > +	dev_notice(dev,
> > 	...);
> > ...>
> > }
> >
> > @@
> > identifier fn;
> > type T;
> > @@
> >
> > T fn ( ..., struct device * dev, ... ) {
> > <...
> > -	pr_warn(
> > +	dev_warn(dev,
> > 	...);
> > ...>
> > }
> >
> > @@
> > identifier fn;
> > type T;
> > @@
> >
> > T fn ( ..., struct device * dev, ... ) {
> > <...
> > -	pr_info(
> > +	dev_info(dev,
> > 	...);
> > ...>
> > }
> >
> > @@
> > identifier fn;
> > type T;
> > @@
> >
> > T fn ( ..., struct device * dev, ... ) {
> > <...
> > -	pr_debug(
> > +	dev_dbg(dev,
> > 	...);
> > ...>
> > }
> >
> >
>
>

Re: [PATCH 1/2] hwmon: (lm70) Utilize dev_warn instead of pr_warn

[Julia Lawall]

On Sun, 22 Jan 2017, Joe Perches wrote:

> On Sun, 2017-01-22 at 22:43 -0800, Joe Perches wrote:
> > Two questions for Julia Lawall:
> >
> > o is there a better way to do this than repeat the blocks
> >   one for each replacement
> > o can struct device * dev be made an arbitrary identifier
> >
> > $ cat dev_printk.cocci
> > @@
> > identifier fn;
> > type T;
> > @@
> >
> > T fn ( ..., struct device * dev, ... ) {
> > <...
> > -	pr_emerg(
> > +	dev_emerg(dev,
> > 	...);
> > ...>
> > }
>
> Well, the second question is simple if I would just
> think a little before asking...
>
> @@
> identifier fn;
> identifier dev;

Yes :)

julia

> type T;
> @@
>
> T fn ( ..., struct device * dev, ... ) {
> <...
> -	pr_emerg(
> +	dev_emerg(dev,
> 	...);
> ...>
> }
>
> etc...
>
>