On 07/09/2013 02:15 PM, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Thu, Jul 04, 2013 at 03:57:49PM +0800, Wei Ni wrote: >> Add support to handle irq. When the temperature touch > > Nit: This first sentence can be dropped. It merely repeats the subject. > And another nit: "irq" -> "IRQ" Ok, I will update in my next version. > >> the limit value, the driver can handle the interrupt. >> >> Signed-off-by: Wei Ni <wni@xxxxxxxxxx> >> --- >> drivers/hwmon/lm90.c | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c >> index 2cb7f8e..fce9dfa 100644 >> --- a/drivers/hwmon/lm90.c >> +++ b/drivers/hwmon/lm90.c >> @@ -89,6 +89,7 @@ >> #include <linux/err.h> >> #include <linux/mutex.h> >> #include <linux/sysfs.h> >> +#include <linux/interrupt.h> >> >> /* >> * Addresses to scan >> @@ -1423,6 +1424,18 @@ static void lm90_init_client(struct i2c_client *client) >> i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, config); >> } >> >> +static void lm90_alert(struct i2c_client *client, unsigned int flag); > > Any reason why the implementation of lm90_alert() can't be moved here. > Granted, it'll make the diff larger but at least it'll allow us to get > rid of the forward declaration. Ok, you are right, I will move it. > >> +static irqreturn_t lm90_irq_thread(int irq, void *dev_id) >> +{ >> + struct lm90_data *data = dev_id; >> + struct i2c_client *client = to_i2c_client(data->hwmon_dev->parent); >> + >> + lm90_alert(client, 0); >> + >> + return IRQ_HANDLED; >> +} > > Isn't this potentially dangerous, since if the IRQ was shared, IRQ > processing will always stop after this handler. Shouldn't you check > whether the device actually triggered an interrupt (by reading the > interrupt status register)? Yes, it has potential dangerous, I didn't consider it carefully. I will check the interrupt status. > >> + >> static int lm90_probe(struct i2c_client *client, >> const struct i2c_device_id *id) >> { >> @@ -1499,6 +1512,18 @@ static int lm90_probe(struct i2c_client *client, >> goto exit_remove_files; >> } >> >> + if (client->irq >= 0) { >> + dev_dbg(dev, "lm90 irq: %d\n", client->irq); > > Nit: "irq" -> "IRQ" Ok, got it. > > Thierry > > * Unknown Key > * 0x7F3EB3A1 > -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html