All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <j.w.r.degoede@hhs.nl>
To: lm-sensors@vger.kernel.org
Subject: Re: [lm-sensors] f71882fg suggestions
Date: Sun, 08 Jul 2007 09:25:07 +0000	[thread overview]
Message-ID: <4690AD73.1000902@hhs.nl> (raw)

Krzysztof Helt wrote:
> Hi,
> 
> Here are 3 suggestions to your f71882fg driver (I have no code to 
> comment directly, but they should be easy to locate).
> 
> 1. You should be an author in the MODULE_AUTHOR() macro.

Good point!

> 2. You should not set data.addr in f71882fg_find()
Wrong, I must do that, because after that I call f71882fg_read8(data, ...)
and 71882fg_read8 needs addr to be set, notice that I'm operating on a 
temporary data struct on the stack here, which is used only to call 
f71882fg_read8, to read the config register and see if the hwmon is enabled.

> 3. I wonder why you add all these sysfs entries one by one (in 
> loops) instead of creating a group and add them with one function 
> call (see the asb100 driver). You need to create 3 groups, one 
> for each loop.

I do it this way because I have an array of attributes, not an array of 
attribute addresses as the group register function expects. Walking my array 
with a loop is easier then then first creating an array of addresses (with a 
loop, or handfilling it, which is even worse) and then calling the group 
register function.

Now if there would be a group register function which I can pass the address of 
a set of attr, the offsets between all the attr (one needs to specify the 
offset ebcause we use sensor_attr not sysfs_attr which is larger) and the no of 
attr, then that would be great.

Regards,

Hans

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

                 reply	other threads:[~2007-07-08  9:25 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=4690AD73.1000902@hhs.nl \
    --to=j.w.r.degoede@hhs.nl \
    --cc=lm-sensors@vger.kernel.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.