All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
To: "Jakub Szczudło" <jakubszczudlo40@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>,
	linux-iio@vger.kernel.org, andy@kernel.org,
	antoniu.miclaus@analog.com, conor+dt@kernel.org,
	devicetree@vger.kernel.org, dlechner@baylibre.com,
	duje@dujemihanovic.xyz, jishnu.prakash@oss.qualcomm.com,
	jorge.marques@analog.com, joshua.crofts1@gmail.com,
	krzk+dt@kernel.org, linusw@kernel.org,
	marcelo.schmitt@analog.com, mazziesaccount@gmail.com,
	mike.looijmans@topic.nl, nuno.sa@analog.com, robh@kernel.org,
	sakari.ailus@linux.intel.com, wens@kernel.org
Subject: Re: [PATCH v6 1/3] iio: adc: Fix incorrect reading when datarate changed in single mode
Date: Tue, 14 Jul 2026 17:56:45 -0700	[thread overview]
Message-ID: <20260714175645.00003812@oss.qualcomm.com> (raw)
In-Reply-To: <CA+gq5JjOV2J87Ycm6bjpa1o=+eKELt6FZBk_KSdEt1zLiQs4MQ@mail.gmail.com>

On Mon, 13 Jul 2026 12:53:49 +0200
Jakub Szczudło <jakubszczudlo40@gmail.com> wrote:

> > > +     ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));  
> >
> > In theory this can return short (generally reflects a misbehaving device).
> >  
> > > +     if (ret < 0) {
> > > +             dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
> > > +             return true;
> > > +     }  
> >
> >         if (ret < sizeof(buffer))
> >                 return -EIO;
> >  
> I have checked the source code for i2c_master_recv and it should
> return sizeof(buffer)
> or errno code. Also David and Andy in v4 told me to delete such check.

Fair point on it not being needed.  Gah, I get my i2c functions confused.
Only the multi transfer ones can return partial success.

> > > +
> > > +     return FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);
> > > +}
> > > +
> > > +static int ads1100_poll_data_ready(struct ads1100_data *data)
> > > +{
> > > +     int data_rate_Hz = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
> > > +     /* To be sure we wait 5 times more than data rate */
> > > +     unsigned long wait_time_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, 5 * data_rate_Hz);
> > > +     bool data_ready;
> > > +     u8 buffer[3];
> > > +     int ret;
> > > +
> > > +     /* To be sure that polled value will have value after config change */
> > > +     ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
> > > +     if (ret < 0) {
> > > +             dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
> > > +             return ret;
> > > +     }  
> >         if (ret < sizeof(buffer))
> >                 reutrn -EIO;  
> > > +
> > > +     return readx_poll_timeout(ads1100_new_data_not_ready, data,
> > > +                              data_ready, data_ready != 0,  
> >
> > Kind of related to David's question: sashiko doesn't like this.
> > https://sashiko.dev/#/patchset/20260711184414.1013686-1-jakubszczudlo40%40gmail.com
> > I think I agree with it.  Generally need to check for errors in the polling
> > routing as well and return those if seen (in data_ready I think at the end?)
> >  
> > > +                              wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);
> > > +}  
> >  
> sure I will update the function to return int instead of bool and
> check for errors
> 
> 


  reply	other threads:[~2026-07-15  0:56 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-11 18:44 [PATCH v6 0/3 ] iio: adc: Add support for TI ADS1110 to ti-ads1100 driver Jakub Szczudlo
2026-07-11 18:44 ` [PATCH v6 1/3] iio: adc: Fix incorrect reading when datarate changed in single mode Jakub Szczudlo
2026-07-11 18:54   ` sashiko-bot
2026-07-11 19:51   ` David Lechner
2026-07-13  1:20   ` Jonathan Cameron
2026-07-13 10:53     ` Jakub Szczudło
2026-07-15  0:56       ` Jonathan Cameron [this message]
2026-07-13 14:12     ` David Lechner
2026-07-11 18:44 ` [PATCH v6 2/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110 Jakub Szczudlo
2026-07-11 18:44 ` [PATCH v6 3/3] iio: adc: Add ti-ads1110 support to ti-ads1100 driver Jakub Szczudlo
2026-07-11 18:57   ` sashiko-bot
2026-07-11 20:01   ` David Lechner
2026-07-12 10:16     ` Jakub Szczudło
2026-07-12 16:32       ` David Lechner
2026-07-13  1:26   ` Jonathan Cameron

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=20260714175645.00003812@oss.qualcomm.com \
    --to=jonathan.cameron@oss.qualcomm.com \
    --cc=andy@kernel.org \
    --cc=antoniu.miclaus@analog.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=duje@dujemihanovic.xyz \
    --cc=jakubszczudlo40@gmail.com \
    --cc=jic23@kernel.org \
    --cc=jishnu.prakash@oss.qualcomm.com \
    --cc=jorge.marques@analog.com \
    --cc=joshua.crofts1@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=marcelo.schmitt@analog.com \
    --cc=mazziesaccount@gmail.com \
    --cc=mike.looijmans@topic.nl \
    --cc=nuno.sa@analog.com \
    --cc=robh@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=wens@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.