LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Oskar Senft <osk@google.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Jean Delvare <jdelvare@suse.com>,
	Rob Herring <robh+dt@kernel.org>
Subject: Re: [PATCH] dt-bindings: hwmon: Add nct7802 bindings
Date: Tue, 14 Sep 2021 13:11:07 -0400	[thread overview]
Message-ID: <CABoTLcQfS5-UL92NR9vbc2YrGJv3oQPYCqAm-diNoq-tkHP_hQ@mail.gmail.com> (raw)
In-Reply-To: <20210914150859.GB3457579@roeck-us.net>

Hi Guenter

> > Following the example from tmp421, this could then be like this:
>
> Something like that, only we'll need something to distinguish
> temperature sensors from other sensor types, eg voltage or current.
> Maybe a "type" property. I'd suggest "sensor-type", but we have
> non-sensor attributes such as fan count and pwm values which should
> be covered as well. But it looks like a good start for a set of
> generic sensor properties.
Would it be acceptable to simply number the sensors and document which
sensor has which number?

Something like this:
0 = LTD
1 = RTD1
2 = RTD2
3 = RTD3
4 = FAN1
5 = FAN2
6 = FAN3

Would we also want to be able to define PWMs? From what I can tell the
driver does not support running individual pins in GPIO mode, right?
So I'm not quite clear what "disabling PWM" would actually mean.

Anyway, if we simply go by "sensor number", that would mean that we'd
have different attributes depending on the sensor number. Would that
be ok?

Also, I'm sorry, I think I just realized that in "voltage mode" we
don't seem to get a temperature reading. I hadn't actually looked
through more of the datasheet except for the single MODE register
before. But I don't think this makes a difference for what I proposed
so far?

> >         /* LTD */
> >         input@0 {
> >             reg = <0x0>;
> >             status = "okay";
>
> Not sure what the default is here ('okay' or 'disabled').
> We'd also need to define what to do if there is no data
> for a given sensor.
I think I'd like to keep previous behavior unmodified. From what I can
tell previous behavior was:
- xTDs enabled by default
- RTD modes unmodified, i.e. defaulting to whatever the HW comes up with

The NCT7802Y can self-program from an EEPROM, so I assume we should
honor the "power-up configuration" obtained from there? I.e. if no
configuration is provided in the device tree, the driver should use
whatever configuration the chip has when the driver is loaded.

> >             label = "voltage mode";
>
> That isn't the idea for "label", as "label" would be expected to
> show up as tempX_label (and a label of "voltage mode" would be odd).
> The label should indicate where the sensor is located on a board,
> such as "inlet" or "outlet".
Yes, absolutely. This was a bad example on my part. In my
understanding "label" is just a string that we pass through.

Oskar.

  reply	other threads:[~2021-09-14 17:11 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-10 13:03 Oskar Senft
2021-09-10 16:10 ` Guenter Roeck
2021-09-10 20:44   ` Oskar Senft
2021-09-10 21:28     ` Guenter Roeck
2021-09-14 12:41       ` Oskar Senft
2021-09-14 15:08         ` Guenter Roeck
2021-09-14 17:11           ` Oskar Senft [this message]
2021-09-14 17:33             ` Guenter Roeck
2021-09-16 19:19               ` Oskar Senft
2021-09-16 19:34                 ` Guenter Roeck
2021-09-16 19:53                   ` Oskar Senft
2021-09-16 20:07                     ` Guenter Roeck
2021-09-16 20:10                       ` Oskar Senft
2021-09-17  3:09                         ` Oskar Senft
2021-09-17  3:29                           ` Guenter Roeck
2021-09-12  4:03     ` Guenter Roeck
2021-09-10 21:03 ` Rob Herring

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=CABoTLcQfS5-UL92NR9vbc2YrGJv3oQPYCqAm-diNoq-tkHP_hQ@mail.gmail.com \
    --to=osk@google.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jdelvare@suse.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=robh+dt@kernel.org \
    --subject='Re: [PATCH] dt-bindings: hwmon: Add nct7802 bindings' \
    /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

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).