* [PATCH] hwmon: (lm75) Add a basic interrupt handler
@ 2017-07-12 2:52 Tony O'Brien
2017-07-16 0:42 ` Guenter Roeck
0 siblings, 1 reply; 4+ messages in thread
From: Tony O'Brien @ 2017-07-12 2:52 UTC (permalink / raw)
To: jdelvare, linux, linux-hwmon, linux-kernel
The LM75 interrupt cannot be masked in the device so an over-temperature
event can cause an interrupt that cannot be cleared.
Add an interrupt handler if an interrupt node exists in the DTS for an
LM75. The handler simply reads a device register to clear the interrupt
and returns.
Signed-off-by: Tony O'Brien <tony.obrien@alliedtelesis.co.nz>
Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
drivers/hwmon/lm75.c | 28 ++++++++++++++++++++++++++++
drivers/hwmon/lm75.h | 1 +
2 files changed, 29 insertions(+)
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 005ffb5ffa92..08a833e960aa 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -22,6 +22,7 @@
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/jiffies.h>
+#include <linux/interrupt.h>
#include <linux/i2c.h>
#include <linux/hwmon.h>
#include <linux/hwmon-sysfs.h>
@@ -265,6 +266,17 @@ static void lm75_remove(void *data)
i2c_smbus_write_byte_data(client, LM75_REG_CONF, lm75->orig_conf);
}
+static irqreturn_t lm75_process_interrupt(int irq, void *data)
+{
+ struct i2c_client *client = (struct i2c_client *)data;
+ int val;
+
+ /* Do a read to clear the interrupt */
+ val = i2c_smbus_read_byte_data(client, LM75_REG_CONF);
+
+ return IRQ_HANDLED;
+}
+
static int
lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
{
@@ -275,6 +287,7 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
u8 set_mask, clr_mask;
int new;
enum lm75_type kind;
+ int ret;
if (client->dev.of_node)
kind = (enum lm75_type)of_device_get_match_data(&client->dev);
@@ -375,6 +388,21 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
break;
}
+ if (client->irq) {
+ /* If requesting an interrupt then set to interrupt mode */
+ set_mask |= LM75_MODE_INTERRUPT;
+ ret = devm_request_threaded_irq(dev, client->irq, NULL,
+ lm75_process_interrupt,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT |
+ IRQF_SHARED,
+ dev_name(dev),
+ client);
+ if (ret) {
+ dev_err(dev, "Error requesting irq\n");
+ return ret;
+ }
+ }
+
/* configure as specified */
status = i2c_smbus_read_byte_data(client, LM75_REG_CONF);
if (status < 0) {
diff --git a/drivers/hwmon/lm75.h b/drivers/hwmon/lm75.h
index 5cde94e56f17..82a71d870331 100644
--- a/drivers/hwmon/lm75.h
+++ b/drivers/hwmon/lm75.h
@@ -31,6 +31,7 @@
#define LM75_TEMP_MIN (-55000)
#define LM75_TEMP_MAX 125000
#define LM75_SHUTDOWN 0x01
+#define LM75_MODE_INTERRUPT 0x02
/* TEMP: 0.001C/bit (-55C to +125C)
REG: (0.5C/bit, two's complement) << 7 */
--
2.13.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] hwmon: (lm75) Add a basic interrupt handler
2017-07-12 2:52 [PATCH] hwmon: (lm75) Add a basic interrupt handler Tony O'Brien
@ 2017-07-16 0:42 ` Guenter Roeck
2017-07-16 21:51 ` Tony O'Brien
0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2017-07-16 0:42 UTC (permalink / raw)
To: Tony O'Brien, jdelvare, linux-hwmon, linux-kernel
On 07/11/2017 07:52 PM, Tony O'Brien wrote:
> The LM75 interrupt cannot be masked in the device so an over-temperature
> event can cause an interrupt that cannot be cleared.
> Add an interrupt handler if an interrupt node exists in the DTS for an
> LM75. The handler simply reads a device register to clear the interrupt
> and returns.
>
Is this an actual problem ? If no interrupt handler is registered, the interrupt
should be disabled by the interrupt controller, and it should not matter if
the interrupt line from the chip is active or not.
Thanks,
Guenter
> Signed-off-by: Tony O'Brien <tony.obrien@alliedtelesis.co.nz>
> Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> drivers/hwmon/lm75.c | 28 ++++++++++++++++++++++++++++
> drivers/hwmon/lm75.h | 1 +
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index 005ffb5ffa92..08a833e960aa 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -22,6 +22,7 @@
> #include <linux/init.h>
> #include <linux/slab.h>
> #include <linux/jiffies.h>
> +#include <linux/interrupt.h>
> #include <linux/i2c.h>
> #include <linux/hwmon.h>
> #include <linux/hwmon-sysfs.h>
> @@ -265,6 +266,17 @@ static void lm75_remove(void *data)
> i2c_smbus_write_byte_data(client, LM75_REG_CONF, lm75->orig_conf);
> }
>
> +static irqreturn_t lm75_process_interrupt(int irq, void *data)
> +{
> + struct i2c_client *client = (struct i2c_client *)data;
> + int val;
> +
> + /* Do a read to clear the interrupt */
> + val = i2c_smbus_read_byte_data(client, LM75_REG_CONF);
> +
> + return IRQ_HANDLED;
> +}
> +
> static int
> lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
> {
> @@ -275,6 +287,7 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
> u8 set_mask, clr_mask;
> int new;
> enum lm75_type kind;
> + int ret;
>
> if (client->dev.of_node)
> kind = (enum lm75_type)of_device_get_match_data(&client->dev);
> @@ -375,6 +388,21 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
> break;
> }
>
> + if (client->irq) {
> + /* If requesting an interrupt then set to interrupt mode */
> + set_mask |= LM75_MODE_INTERRUPT;
> + ret = devm_request_threaded_irq(dev, client->irq, NULL,
> + lm75_process_interrupt,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT |
> + IRQF_SHARED,
> + dev_name(dev),
> + client);
> + if (ret) {
> + dev_err(dev, "Error requesting irq\n");
> + return ret;
> + }
> + }
> +
> /* configure as specified */
> status = i2c_smbus_read_byte_data(client, LM75_REG_CONF);
> if (status < 0) {
> diff --git a/drivers/hwmon/lm75.h b/drivers/hwmon/lm75.h
> index 5cde94e56f17..82a71d870331 100644
> --- a/drivers/hwmon/lm75.h
> +++ b/drivers/hwmon/lm75.h
> @@ -31,6 +31,7 @@
> #define LM75_TEMP_MIN (-55000)
> #define LM75_TEMP_MAX 125000
> #define LM75_SHUTDOWN 0x01
> +#define LM75_MODE_INTERRUPT 0x02
>
> /* TEMP: 0.001C/bit (-55C to +125C)
> REG: (0.5C/bit, two's complement) << 7 */
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] hwmon: (lm75) Add a basic interrupt handler
2017-07-16 0:42 ` Guenter Roeck
@ 2017-07-16 21:51 ` Tony O'Brien
2017-07-18 4:53 ` Guenter Roeck
0 siblings, 1 reply; 4+ messages in thread
From: Tony O'Brien @ 2017-07-16 21:51 UTC (permalink / raw)
To: Guenter Roeck, jdelvare, linux-hwmon, linux-kernel
Hi Guenter -
The interrupt line the LM75 sits on is shared by other devices and therefore cannot be disabled. It was added in hardware as a possible feature can cannot be disconnected.
Cheers,
Tony O'Brien
Senior Software/Hardware Engineer
Allied Telesis Labs
27 Nazareth Avenue
Christchurch 8024
New Zealand
Ph: +64-3-339 3000
DDI: +64-3-339 9210
Web: http://AlliedTelesis.com
________________________________________
From: Guenter Roeck <linux@roeck-us.net>
Sent: Sunday, 16 July 2017 12:42 p.m.
To: Tony O'Brien; jdelvare@suse.com; linux-hwmon@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] hwmon: (lm75) Add a basic interrupt handler
On 07/11/2017 07:52 PM, Tony O'Brien wrote:
> The LM75 interrupt cannot be masked in the device so an over-temperature
> event can cause an interrupt that cannot be cleared.
> Add an interrupt handler if an interrupt node exists in the DTS for an
> LM75. The handler simply reads a device register to clear the interrupt
> and returns.
>
Is this an actual problem ? If no interrupt handler is registered, the interrupt
should be disabled by the interrupt controller, and it should not matter if
the interrupt line from the chip is active or not.
Thanks,
Guenter
> Signed-off-by: Tony O'Brien <tony.obrien@alliedtelesis.co.nz>
> Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> drivers/hwmon/lm75.c | 28 ++++++++++++++++++++++++++++
> drivers/hwmon/lm75.h | 1 +
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index 005ffb5ffa92..08a833e960aa 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -22,6 +22,7 @@
> #include <linux/init.h>
> #include <linux/slab.h>
> #include <linux/jiffies.h>
> +#include <linux/interrupt.h>
> #include <linux/i2c.h>
> #include <linux/hwmon.h>
> #include <linux/hwmon-sysfs.h>
> @@ -265,6 +266,17 @@ static void lm75_remove(void *data)
> i2c_smbus_write_byte_data(client, LM75_REG_CONF, lm75->orig_conf);
> }
>
> +static irqreturn_t lm75_process_interrupt(int irq, void *data)
> +{
> + struct i2c_client *client = (struct i2c_client *)data;
> + int val;
> +
> + /* Do a read to clear the interrupt */
> + val = i2c_smbus_read_byte_data(client, LM75_REG_CONF);
> +
> + return IRQ_HANDLED;
> +}
> +
> static int
> lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
> {
> @@ -275,6 +287,7 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
> u8 set_mask, clr_mask;
> int new;
> enum lm75_type kind;
> + int ret;
>
> if (client->dev.of_node)
> kind = (enum lm75_type)of_device_get_match_data(&client->dev);
> @@ -375,6 +388,21 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
> break;
> }
>
> + if (client->irq) {
> + /* If requesting an interrupt then set to interrupt mode */
> + set_mask |= LM75_MODE_INTERRUPT;
> + ret = devm_request_threaded_irq(dev, client->irq, NULL,
> + lm75_process_interrupt,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT |
> + IRQF_SHARED,
> + dev_name(dev),
> + client);
> + if (ret) {
> + dev_err(dev, "Error requesting irq\n");
> + return ret;
> + }
> + }
> +
> /* configure as specified */
> status = i2c_smbus_read_byte_data(client, LM75_REG_CONF);
> if (status < 0) {
> diff --git a/drivers/hwmon/lm75.h b/drivers/hwmon/lm75.h
> index 5cde94e56f17..82a71d870331 100644
> --- a/drivers/hwmon/lm75.h
> +++ b/drivers/hwmon/lm75.h
> @@ -31,6 +31,7 @@
> #define LM75_TEMP_MIN (-55000)
> #define LM75_TEMP_MAX 125000
> #define LM75_SHUTDOWN 0x01
> +#define LM75_MODE_INTERRUPT 0x02
>
> /* TEMP: 0.001C/bit (-55C to +125C)
> REG: (0.5C/bit, two's complement) << 7 */
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] hwmon: (lm75) Add a basic interrupt handler
2017-07-16 21:51 ` Tony O'Brien
@ 2017-07-18 4:53 ` Guenter Roeck
0 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2017-07-18 4:53 UTC (permalink / raw)
To: Tony O'Brien; +Cc: jdelvare, linux-hwmon, linux-kernel
On Sun, Jul 16, 2017 at 09:51:24PM +0000, Tony O'Brien wrote:
> Hi Guenter -
>
> The interrupt line the LM75 sits on is shared by other devices and therefore cannot be disabled. It was added in hardware as a possible feature can cannot be disconnected.
>
Ah yes, it is declared as shared, which makes me slightly uncomfortable.
Any chance that this (and TRIGGER_LOW) can be specified in devicetree ?
Sorry if I am annoying, but this is a generic driver, and I would like
to prevent bad surprises if possible.
Thanks,
Guenter
> Cheers,
>
> Tony O'Brien
> Senior Software/Hardware Engineer
> Allied Telesis Labs
> 27 Nazareth Avenue
> Christchurch 8024
> New Zealand
> Ph: +64-3-339 3000
> DDI: +64-3-339 9210
> Web: http://AlliedTelesis.com
>
>
>
> ________________________________________
> From: Guenter Roeck <linux@roeck-us.net>
> Sent: Sunday, 16 July 2017 12:42 p.m.
> To: Tony O'Brien; jdelvare@suse.com; linux-hwmon@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] hwmon: (lm75) Add a basic interrupt handler
>
> On 07/11/2017 07:52 PM, Tony O'Brien wrote:
> > The LM75 interrupt cannot be masked in the device so an over-temperature
> > event can cause an interrupt that cannot be cleared.
> > Add an interrupt handler if an interrupt node exists in the DTS for an
> > LM75. The handler simply reads a device register to clear the interrupt
> > and returns.
> >
>
> Is this an actual problem ? If no interrupt handler is registered, the interrupt
> should be disabled by the interrupt controller, and it should not matter if
> the interrupt line from the chip is active or not.
>
> Thanks,
> Guenter
>
> > Signed-off-by: Tony O'Brien <tony.obrien@alliedtelesis.co.nz>
> > Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> > ---
> > drivers/hwmon/lm75.c | 28 ++++++++++++++++++++++++++++
> > drivers/hwmon/lm75.h | 1 +
> > 2 files changed, 29 insertions(+)
> >
> > diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> > index 005ffb5ffa92..08a833e960aa 100644
> > --- a/drivers/hwmon/lm75.c
> > +++ b/drivers/hwmon/lm75.c
> > @@ -22,6 +22,7 @@
> > #include <linux/init.h>
> > #include <linux/slab.h>
> > #include <linux/jiffies.h>
> > +#include <linux/interrupt.h>
> > #include <linux/i2c.h>
> > #include <linux/hwmon.h>
> > #include <linux/hwmon-sysfs.h>
> > @@ -265,6 +266,17 @@ static void lm75_remove(void *data)
> > i2c_smbus_write_byte_data(client, LM75_REG_CONF, lm75->orig_conf);
> > }
> >
> > +static irqreturn_t lm75_process_interrupt(int irq, void *data)
> > +{
> > + struct i2c_client *client = (struct i2c_client *)data;
> > + int val;
> > +
> > + /* Do a read to clear the interrupt */
> > + val = i2c_smbus_read_byte_data(client, LM75_REG_CONF);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > static int
> > lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
> > {
> > @@ -275,6 +287,7 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
> > u8 set_mask, clr_mask;
> > int new;
> > enum lm75_type kind;
> > + int ret;
> >
> > if (client->dev.of_node)
> > kind = (enum lm75_type)of_device_get_match_data(&client->dev);
> > @@ -375,6 +388,21 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
> > break;
> > }
> >
> > + if (client->irq) {
> > + /* If requesting an interrupt then set to interrupt mode */
> > + set_mask |= LM75_MODE_INTERRUPT;
> > + ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > + lm75_process_interrupt,
> > + IRQF_TRIGGER_LOW | IRQF_ONESHOT |
> > + IRQF_SHARED,
> > + dev_name(dev),
> > + client);
> > + if (ret) {
> > + dev_err(dev, "Error requesting irq\n");
> > + return ret;
> > + }
> > + }
> > +
> > /* configure as specified */
> > status = i2c_smbus_read_byte_data(client, LM75_REG_CONF);
> > if (status < 0) {
> > diff --git a/drivers/hwmon/lm75.h b/drivers/hwmon/lm75.h
> > index 5cde94e56f17..82a71d870331 100644
> > --- a/drivers/hwmon/lm75.h
> > +++ b/drivers/hwmon/lm75.h
> > @@ -31,6 +31,7 @@
> > #define LM75_TEMP_MIN (-55000)
> > #define LM75_TEMP_MAX 125000
> > #define LM75_SHUTDOWN 0x01
> > +#define LM75_MODE_INTERRUPT 0x02
> >
> > /* TEMP: 0.001C/bit (-55C to +125C)
> > REG: (0.5C/bit, two's complement) << 7 */
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-07-18 4:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-12 2:52 [PATCH] hwmon: (lm75) Add a basic interrupt handler Tony O'Brien
2017-07-16 0:42 ` Guenter Roeck
2017-07-16 21:51 ` Tony O'Brien
2017-07-18 4:53 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).