linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chris Packham <Chris.Packham@alliedtelesis.co.nz>
To: "linux@roeck-us.net" <linux@roeck-us.net>,
	Logan Shaw <Logan.Shaw@alliedtelesis.co.nz>,
	"jdelvare@suse.com" <jdelvare@suse.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>
Cc: Joshua Scott <Joshua.Scott@alliedtelesis.co.nz>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH v2 2/2] hwmon: (adt7475) Added attenuator bypass support
Date: Mon, 13 Jan 2020 00:08:19 +0000	[thread overview]
Message-ID: <c5e9118cf36735602cfd01a64076b98ab0ad3802.camel@alliedtelesis.co.nz> (raw)
In-Reply-To: <e88b029b-dbb3-e423-5c37-0917d7716b66@roeck-us.net>

(CC Rob and devicetree)

On Wed, 2019-12-18 at 19:53 -0800, Guenter Roeck wrote:
> On 12/18/19 7:32 PM, Logan Shaw wrote:
> > Added a new file documenting the adt7475 devicetree and added the five
> > new properties to it.
> > 
> > Signed-off-by: Logan Shaw <logan.shaw@alliedtelesis.co.nz>
> > ---
> > ---
> >   .../devicetree/bindings/hwmon/adt7475.txt     | 35 +++++++++++++++++++
> >   1 file changed, 35 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/hwmon/adt7475.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.txt b/Documentation/devicetree/bindings/hwmon/adt7475.txt
> > new file mode 100644
> > index 000000000000..388dd718a246
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/adt7475.txt

There's an effort underway to convert the device tree bindings to yaml
so it'd be better for new bindings to start off that way. It should be
relatively straight forward, there are a couple of existing examples
in  Documentation/devicetree/bindings/hwmon/ to follow.

Also there is an existing entry in
Documentation/devicetree/bindings/trivial-devices.yaml for the adt7475
and friends that should be removed as part of the patch that adds the
new binding.

> > @@ -0,0 +1,35 @@
> > +*ADT7475 hwmon sensor.
> > +
> > +Required properties:
> > +- compatible: One of
> > +	"adi,adt7473"
> > +	"adi,adt7475"
> > +	"adi,adt7476"
> > +	"adi,adt7490"
> > +
> > +- reg: I2C address
> > +
> > +optional properties:
> > +
> > +- bypass-attenuator-all: Configures bypassing all voltage input attenuators.
> > +	This is only supported on the ADT7476 and ADT7490, this property does
> > +	nothing on other chips.

I don't know that there's any point in supporting bypass-attenuator-all 
even though the adt7475 can support it configuring per VIN seems more
useful.

> 
> Both adt7473 and adt7475 do support configuring an attenuator on VCCP
> 
> > +		property present: Bit set to bypass all voltage input attenuators.
> > +		property absent: Registers left unchanged.
> > +
> > +- bypass-attenuator-inx: Configures bypassing individual voltage input
> > +	attenuators, where x is an integer from the set {0, 1, 3, 4}. This
> > +	is only supported on the ADT7476 and ADT7490, this property does nothing
> > +	on other chips.
> > +		property present: Bit set to bypass specific voltage input attenuator
> > +			for voltage input x.
> > +		property absent: Registers left unchanged.
> > +
> 
> This is interesting. It essentially means "retain configuration from ROM
> Monitor", but leaves no means to _disable_ the bypass.
> 

For our systems Linux is generally the ROM monitor, at least as far as
the hwmon devices are concerned. Overriding the HW default makes sense
for that case.

Do we want the ability to override the configuration from the ROM? It
should be easily doable by using an integer property instead of a
boolean.

> > +Example:
> > +
> > +hwmon@2e {
> > +	compatible = "adi,adt7475";
> > +	reg = <0x2e>;
> > +	bypass-attenuator-all;
> > +	bypass-attenuator-in1;
> 
> What would be the purpose of specifying both all and in1 ?
> 
> > +};
> > \ No newline at end of file
> > 
> 
> 

      parent reply	other threads:[~2020-01-13  0:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-19  3:32 [PATCH v2 0/2] " Logan Shaw
2019-12-19  3:32 ` [PATCH v2 1/2] " Logan Shaw
2019-12-19  3:32 ` [PATCH v2 2/2] " Logan Shaw
2019-12-19  3:53   ` Guenter Roeck
2019-12-27  2:53     ` Logan Shaw
2020-01-09  3:07       ` Logan Shaw
2020-01-09 23:03       ` Guenter Roeck
2020-01-13  0:08     ` Chris Packham [this message]

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=c5e9118cf36735602cfd01a64076b98ab0ad3802.camel@alliedtelesis.co.nz \
    --to=chris.packham@alliedtelesis.co.nz \
    --cc=Joshua.Scott@alliedtelesis.co.nz \
    --cc=Logan.Shaw@alliedtelesis.co.nz \
    --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 v2 2/2] hwmon: (adt7475) Added attenuator bypass support' \
    /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).