All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jean Delvare <khali@linux-fr.org>
To: Guenter Roeck <guenter.roeck@ericsson.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org
Subject: Re: [lm-sensors] [PATCH v2 7/7] hwmon: (lm90) Add support for
Date: Wed, 15 Sep 2010 14:34:07 +0000	[thread overview]
Message-ID: <20100915163407.47e8de54@hyperion.delvare> (raw)
In-Reply-To: <20100915142927.GB9707@ericsson.com>

On Wed, 15 Sep 2010 07:29:27 -0700, Guenter Roeck wrote:
> On Wed, Sep 15, 2010 at 07:20:29AM -0400, Jean Delvare wrote:
> > Would it be possible to simply place these functions at the right
> > location so that forward declarations are not needed? Ideally I would
> > like to get rid of all these forward declarations.
> > 
> I moved lm90_select_remote_channel() so the lm90_read_reg() declaration
> is no longer needed.
> 
> Code structure is such that sensor show/set functions come first, followed by
> "real" code. Unfortunately, that implies that local functions called from
> those functions have to be forward declarated.
> 
> We can move that around to avoid forward declarations, but I would prefer
> to do that in a separate patch if we decide to go that way.

Sure, fine with me.

> [...]
> > > +/*
> > > + * Additional attributes for devices with 3 temperature sensors
> > > + */
> > > +static SENSOR_DEVICE_ATTR_2(temp3_input, S_IRUGO, show_temp11, NULL, 0, 5);
> > > +static SENSOR_DEVICE_ATTR_2(temp3_min, S_IWUSR | S_IRUGO, show_temp11,
> > > +     set_temp11, 3, 6);
> > > +static SENSOR_DEVICE_ATTR_2(temp3_max, S_IWUSR | S_IRUGO, show_temp11,
> > > +     set_temp11, 4, 7);
> > > +static SENSOR_DEVICE_ATTR(temp3_crit, S_IWUSR | S_IRUGO, show_temp8,
> > > +     set_temp8, 6);
> > > +static SENSOR_DEVICE_ATTR(temp3_crit_hyst, S_IRUGO, show_temphyst, NULL, 4);
> > 
> > 4, really? Remote 2 critical limit is 6.
> > 
> I am impressed. Good catch.

Now you understand why it takes me so long to review patches. I
actually read them ;)

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

WARNING: multiple messages have this Message-ID (diff)
From: Jean Delvare <khali@linux-fr.org>
To: Guenter Roeck <guenter.roeck@ericsson.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 7/7] hwmon: (lm90) Add support for max6695 and  max6696
Date: Wed, 15 Sep 2010 16:34:07 +0200	[thread overview]
Message-ID: <20100915163407.47e8de54@hyperion.delvare> (raw)
In-Reply-To: <20100915142927.GB9707@ericsson.com>

On Wed, 15 Sep 2010 07:29:27 -0700, Guenter Roeck wrote:
> On Wed, Sep 15, 2010 at 07:20:29AM -0400, Jean Delvare wrote:
> > Would it be possible to simply place these functions at the right
> > location so that forward declarations are not needed? Ideally I would
> > like to get rid of all these forward declarations.
> > 
> I moved lm90_select_remote_channel() so the lm90_read_reg() declaration
> is no longer needed.
> 
> Code structure is such that sensor show/set functions come first, followed by
> "real" code. Unfortunately, that implies that local functions called from
> those functions have to be forward declarated.
> 
> We can move that around to avoid forward declarations, but I would prefer
> to do that in a separate patch if we decide to go that way.

Sure, fine with me.

> [...]
> > > +/*
> > > + * Additional attributes for devices with 3 temperature sensors
> > > + */
> > > +static SENSOR_DEVICE_ATTR_2(temp3_input, S_IRUGO, show_temp11, NULL, 0, 5);
> > > +static SENSOR_DEVICE_ATTR_2(temp3_min, S_IWUSR | S_IRUGO, show_temp11,
> > > +     set_temp11, 3, 6);
> > > +static SENSOR_DEVICE_ATTR_2(temp3_max, S_IWUSR | S_IRUGO, show_temp11,
> > > +     set_temp11, 4, 7);
> > > +static SENSOR_DEVICE_ATTR(temp3_crit, S_IWUSR | S_IRUGO, show_temp8,
> > > +     set_temp8, 6);
> > > +static SENSOR_DEVICE_ATTR(temp3_crit_hyst, S_IRUGO, show_temphyst, NULL, 4);
> > 
> > 4, really? Remote 2 critical limit is 6.
> > 
> I am impressed. Good catch.

Now you understand why it takes me so long to review patches. I
actually read them ;)

-- 
Jean Delvare

  reply	other threads:[~2010-09-15 14:34 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-09 13:25 [lm-sensors] [PATCH v2 0/7] hwmon: Add support for MAX6695/6696 to Guenter Roeck
2010-09-09 13:25 ` [PATCH v2 0/7] hwmon: Add support for MAX6695/6696 to lm90 driver Guenter Roeck
2010-09-09 13:25 ` [lm-sensors] [PATCH v2 1/7] hwmon: Add tempX_emergency_alarm Guenter Roeck
2010-09-09 13:25   ` [PATCH v2 1/7] hwmon: Add tempX_emergency_alarm attribute to sysfs ABI Guenter Roeck
2010-09-13 16:17   ` [lm-sensors] [PATCH v2 1/7] hwmon: Add tempX_emergency_alarm Jean Delvare
2010-09-13 16:17     ` [PATCH v2 1/7] hwmon: Add tempX_emergency_alarm attribute to sysfs ABI Jean Delvare
2010-09-13 16:54     ` [lm-sensors] [PATCH v2 1/7] hwmon: Add tempX_emergency_alarm Guenter Roeck
2010-09-13 16:54       ` [PATCH v2 1/7] hwmon: Add tempX_emergency_alarm attribute to sysfs ABI Guenter Roeck
2010-09-16  1:52   ` [lm-sensors] [PATCH v2 1/7] hwmon: Add tempX_emergency_alarm Guenter Roeck
2010-09-09 13:25 ` [lm-sensors] [PATCH v2 2/7] hwmon: (lm90) Introduce device feature Guenter Roeck
2010-09-09 13:25   ` [PATCH v2 2/7] hwmon: (lm90) Introduce device feature bits Guenter Roeck
2010-09-13 19:30   ` [lm-sensors] [PATCH v2 2/7] hwmon: (lm90) Introduce device Jean Delvare
2010-09-13 19:30     ` [PATCH v2 2/7] hwmon: (lm90) Introduce device feature bits Jean Delvare
2010-09-13 20:32     ` [lm-sensors] [PATCH v2 2/7] hwmon: (lm90) Introduce device Guenter Roeck
2010-09-13 20:32       ` [PATCH v2 2/7] hwmon: (lm90) Introduce device feature bits Guenter Roeck
2010-09-09 13:25 ` [lm-sensors] [PATCH v2 3/7] hwmon: (lm90) Introduce function to Guenter Roeck
2010-09-09 13:25   ` [PATCH v2 3/7] hwmon: (lm90) Introduce function to delete sysfs files Guenter Roeck
2010-09-13 19:48   ` [lm-sensors] [PATCH v2 3/7] hwmon: (lm90) Introduce function to Jean Delvare
2010-09-13 19:48     ` [PATCH v2 3/7] hwmon: (lm90) Introduce function to delete sysfs files Jean Delvare
2010-09-16  1:52   ` [lm-sensors] [PATCH v2 3/7] hwmon: (lm90) Introduce function to Guenter Roeck
2010-09-09 13:25 ` [lm-sensors] [PATCH v2 4/7] hwmon: (lm90) Simplify set_temp11 Guenter Roeck
2010-09-09 13:25   ` [PATCH v2 4/7] hwmon: (lm90) Simplify set_temp11 register calculations Guenter Roeck
2010-09-14  9:18   ` [lm-sensors] [PATCH v2 4/7] hwmon: (lm90) Simplify set_temp11 Jean Delvare
2010-09-14  9:18     ` [PATCH v2 4/7] hwmon: (lm90) Simplify set_temp11 register calculations Jean Delvare
2010-09-09 13:25 ` [lm-sensors] [PATCH v2 5/7] hwmon: (lm90) Introduce 3rd set of Guenter Roeck
2010-09-09 13:25   ` [PATCH v2 5/7] hwmon: (lm90) Introduce 3rd set of upper temperature limits Guenter Roeck
2010-09-14 10:51   ` [lm-sensors] [PATCH v2 5/7] hwmon: (lm90) Introduce 3rd set of Jean Delvare
2010-09-14 10:51     ` [PATCH v2 5/7] hwmon: (lm90) Introduce 3rd set of upper temperature limits Jean Delvare
2010-09-14 11:46     ` [lm-sensors] [PATCH v2 5/7] hwmon: (lm90) Introduce 3rd set of Guenter Roeck
2010-09-14 11:46       ` [PATCH v2 5/7] hwmon: (lm90) Introduce 3rd set of upper temperature limits Guenter Roeck
2010-09-14 11:46     ` [lm-sensors] [PATCH v2 5/7] hwmon: (lm90) Introduce 3rd set of Jean Delvare
2010-09-14 11:46       ` [lm-sensors] [PATCH v2 5/7] hwmon: (lm90) Introduce 3rd set of upper temperature limits Jean Delvare
2010-09-14 12:38       ` [lm-sensors] [PATCH v2 5/7] hwmon: (lm90) Introduce 3rd set of Guenter Roeck
2010-09-14 12:38         ` [lm-sensors] [PATCH v2 5/7] hwmon: (lm90) Introduce 3rd set of upper temperature limits Guenter Roeck
2010-09-09 13:25 ` [lm-sensors] [PATCH v2 6/7] hwmon: (lm90) Add support for Guenter Roeck
2010-09-09 13:25   ` [PATCH v2 6/7] hwmon: (lm90) Add support for additional features of max6659 Guenter Roeck
2010-09-14 11:52   ` [lm-sensors] [PATCH v2 6/7] hwmon: (lm90) Add support for Jean Delvare
2010-09-14 11:52     ` [PATCH v2 6/7] hwmon: (lm90) Add support for additional features of max6659 Jean Delvare
2010-09-14 16:08     ` [lm-sensors] [PATCH v2 6/7] hwmon: (lm90) Add support for Guenter Roeck
2010-09-14 16:08       ` [PATCH v2 6/7] hwmon: (lm90) Add support for additional features of max6659 Guenter Roeck
2010-09-09 13:25 ` [lm-sensors] [PATCH v2 7/7] hwmon: (lm90) Add support for max6695 Guenter Roeck
2010-09-09 13:25   ` [PATCH v2 7/7] hwmon: (lm90) Add support for max6695 and max6696 Guenter Roeck
2010-09-15 11:20   ` [lm-sensors] [PATCH v2 7/7] hwmon: (lm90) Add support for Jean Delvare
2010-09-15 11:20     ` [PATCH v2 7/7] hwmon: (lm90) Add support for max6695 and max6696 Jean Delvare
2010-09-15 14:29     ` [lm-sensors] [PATCH v2 7/7] hwmon: (lm90) Add support for Guenter Roeck
2010-09-15 14:29       ` [PATCH v2 7/7] hwmon: (lm90) Add support for max6695 and max6696 Guenter Roeck
2010-09-15 14:34       ` Jean Delvare [this message]
2010-09-15 14:34         ` Jean Delvare

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100915163407.47e8de54@hyperion.delvare \
    --to=khali@linux-fr.org \
    --cc=akpm@linux-foundation.org \
    --cc=guenter.roeck@ericsson.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lm-sensors@lm-sensors.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.