Devicetree
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Joshua Crofts <joshua.crofts1@gmail.com>
Cc: "Kyle Hsieh" <kylehsieh1995@gmail.com>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Liam Girdwood" <lgirdwood@gmail.com>,
	"Mark Brown" <broonie@kernel.org>,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04
Date: Sat, 1 Aug 2026 23:59:20 +0100	[thread overview]
Message-ID: <20260801235920.434c7cf9@jic23-huawei> (raw)
In-Reply-To: <20260731112750.00002c37@gmail.com>

On Fri, 31 Jul 2026 11:27:50 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Fri, 31 Jul 2026 10:58:25 +0800
> Kyle Hsieh <kylehsieh1995@gmail.com> wrote:
> > diff --git a/drivers/iio/adc/ti-ads112c04.c b/drivers/iio/adc/ti-ads112c04.c
> > new file mode 100644
> > index 000000000000..28d3be81934f
> > --- /dev/null
> > +++ b/drivers/iio/adc/ti-ads112c04.c  
> 
> Hi Kyle, quick review from me, comments inline. Additionally, please
> check Sashiko's review as there are some move severe issues (mostly
> I2C stuff), see it here:
> https://sashiko.dev/#/patchset/20260731-ti-ads112c04-driver-v2-0-aab0168c3c01%40gmail.com
Be careful with these.   Some may be misleading or the correct
response may be in a very different place to sashiko suggests.
For example we wouldn't typically bother to defend against nonsense
interrupt types from DT, so if level isn't a plausible type then
state what is as a comment in the DT.

Also, multi master doesn't seems like something we should worry
too much about.  That's not to say there isn't a better way to handle that
transaction.

> > +static int ads112c04_read_reg(struct i2c_client *client, u8 reg, u8 *val)
> > +{
> > +	u8 cmd = ADS112C04_CMD_RREG(reg);
> > +	int ret;
> > +
> > +	ret = i2c_master_send(client, &cmd, 1);  
> 
> i2c_master_send returns either the amount of bytes sent or an error
> code. If the device NACKs, the function will return 0 (zero bytes sent)
> but this will be interpreted as success.

This one is fun and not necessarily that simple. (I used to assume it was!)
Mostly nacks will result in error codes, but there are other obscure
reasons it might return 0.  

Anyhow, whatever the reason, agreed we annoyingly have to check these to see if
they are not 0.

Or, better, as per David's comment use the smbus command if that is possible.
> 
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = i2c_master_recv(client, val, 1);
> > +	return ret < 0 ? ret : 0;
> > +}

  reply	other threads:[~2026-08-01 22:59 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-31  2:58 [PATCH v2 0/2] iio: adc: Add support for Texas Instruments ADS112C04 Kyle Hsieh
2026-07-31  2:58 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads112c04: Add binding for ADS112C04 Kyle Hsieh
2026-07-31  4:32   ` Rob Herring (Arm)
2026-07-31 13:52   ` David Lechner
2026-07-31 14:23     ` David Lechner
2026-07-31  2:58 ` [PATCH v2 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04 Kyle Hsieh
2026-07-31  3:13   ` sashiko-bot
2026-07-31  9:27   ` Joshua Crofts
2026-08-01 22:59     ` Jonathan Cameron [this message]
2026-07-31 14:54   ` David Lechner
2026-07-31 15:09     ` David Lechner

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=20260801235920.434c7cf9@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=andy@kernel.org \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=joshua.crofts1@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=kylehsieh1995@gmail.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=robh@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox