mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Aleksa Savic <savicaleksa83@gmail.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-hwmon@vger.kernel.org, leonard.anderweit@gmail.com,
	Jack Doan <me@jackdoan.com>, Jean Delvare <jdelvare@suse.com>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] hwmon: (aquacomputer_d5next) Add support for temperature sensor offsets
Date: Sun, 23 Oct 2022 15:15:36 +0200	[thread overview]
Message-ID: <6ef88c2c-3435-8847-0f4f-53e81cd3af5d@gmail.com> (raw)
In-Reply-To: <20221022135720.GA739650@roeck-us.net>

On 2022-10-22 15:57:20 GMT+02:00, Guenter Roeck wrote:
> Please go up to 100 columns to avoid excessive line splits.

Will fix this and other comments in v2.

> Is it really necessary to re-read the control buffer repeatedly
> to report this value ? I don't know how costly that is, but unlike
> the pwm value I would not expect the number to change.

Yes, aside from the driver userspace can also change settings on the
device using hidraw and we'd end up with stale data. Reading it is
very fast, it takes about 4ms in my testing.

> Also, is this number indeed not included in the regular reports
> sent from the controller ?

Unfortunately, it's not. The sensor report only includes final (calculated)
sensor readings.

> The driver doesn't distinguish between offsets in the control buffer
> (pwm, and now temperature sensor offset) and offsets in the report buffer,
> making it a bit difficult to determine if those are the same or not.
> Some explanation in the driver would be nice if someone finds the time
> to provide one. If the control buffer offsets are in a different number
> space, they should really be marked accordingly (for example with a
> _CTRL in the define).

I can see how it can be confusing. After this, I can send a patch to
reorder the macros & initializations and add more comments regarding
what is what.

Thanks,
Aleksa

  reply	other threads:[~2022-10-23 13:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-21 17:48 Aleksa Savic
2022-10-22 13:57 ` Guenter Roeck
2022-10-23 13:15   ` Aleksa Savic [this message]
2022-10-23 14:21     ` Guenter Roeck

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=6ef88c2c-3435-8847-0f4f-53e81cd3af5d@gmail.com \
    --to=savicaleksa83@gmail.com \
    --cc=corbet@lwn.net \
    --cc=jdelvare@suse.com \
    --cc=leonard.anderweit@gmail.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=me@jackdoan.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®