public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Mike Looijmans <mike.looijmans@topic.nl>
Cc: devicetree@vger.kernel.org, linux-iio@vger.kernel.org,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Arnd Bergmann <arnd@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Liam Beguin <liambeguin@gmail.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Maksim Kiselev <bigunclemax@gmail.com>,
	Marcus Folkesson <marcus.folkesson@gmail.com>,
	Marius Cristea <marius.cristea@microchip.com>,
	Mark Brown <broonie@kernel.org>,
	Niklas Schnelle <schnelle@linux.ibm.com>,
	Okan Sahin <okan.sahin@analog.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 2/2] iio: adc: ti-ads1298: Add driver
Date: Fri, 16 Feb 2024 17:01:49 +0000	[thread overview]
Message-ID: <20240216170149.040ff86b@jic23-huawei> (raw)
In-Reply-To: <20240216153020.485201-2-mike.looijmans@topic.nl>

On Fri, 16 Feb 2024 16:30:20 +0100
Mike Looijmans <mike.looijmans@topic.nl> wrote:

> Skeleton driver for the TI ADS1298 medical ADC. This device is
> typically used for ECG and similar measurements. Supports data
> acquisition at configurable scale and sampling frequency.
> 
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> 
> ---
> 
> Changes in v5:
> Derive the name from the chip ID register
> Fail probe if the ID is unknown

This is a fun corner when it comes to DT bindings where we may
have fallback compatibles for parts introduced in the future.
For those we are supposed to ignore ID registers and just
assume they are correct (in IIO I tends to suggest we print
a message to say we are carrying on despite not recognising the
ID)  We could go half way here and assume the channel count
can always be gotten from the register, but relax the family bit
to allow families that aren't yet 'born'.

If we don't assume channel count, we would need to add explicit
entries to the ID tables for each of the device supported.

Anyhow, I have no problem leaving that as a problem for another day.

> Interpret the number of channels (only tested the "8")

The way you've done this will not work if you want to add a timestamp
(which is tricky in this device anyway) but for now it is fine.

I tweaked the order in kconfig and Makefile whilst applying.
The relevant sections aren't in alphanumeric order as currently
the TI parts with longer names come later. Having said that this
fits better after ADS1100 than where you have it so I've moved
it up to there.

However...

  CHECK   drivers/iio/adc/ti-ads1298.c                                                                                                             
drivers/iio/adc/ti-ads1298.c:424:13: warning: context imbalance in 'ads1298_rdata_unmark_busy' - wrong count at exit
drivers/iio/adc/ti-ads1298.c:465:9: warning: context imbalance in 'ads1298_rdata_release_busy_or_restart' - wrong count at exit
drivers/iio/adc/ti-ads1298.c:531:9: warning: context imbalance in 'ads1298_interrupt' - wrong count at exit
  MODPOST Module.symvers                                                 
sparse isn't happy (and I upgraded it to make sure I had anything new for guard() etc)

I think it is the missing context tracking referred to here:
https://lore.kernel.org/linux-sparse/Zag2fYsyJDtDR7a6@google.com/

Anyhow, looks like false positives so applied to the togreg branch of iio.git
but first pushed out as testing to let 0-day take a look.

Thanks,

Jonathan

  reply	other threads:[~2024-02-16 17:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1b153bce-a66a-45ee-a5c6-963ea6fb1c82.949ef384-8293-46b8-903f-40a477c056ae.70c52742-e0f9-4db1-8f1b-864f11c71a24@emailsignatures365.codetwo.com>
2024-02-16 15:30 ` [PATCH v5 1/2] dt-bindings: iio: adc: ti-ads1298: Add bindings Mike Looijmans
2024-02-16 15:30   ` [PATCH v5 2/2] iio: adc: ti-ads1298: Add driver Mike Looijmans
2024-02-16 17:01     ` Jonathan Cameron [this message]
2024-02-16 15:53 Andy Shevchenko
     [not found] ` <fb7d41fc-328a-4ce1-88ad-5ce22ee158e4@topic.nl>
2024-02-16 17:19   ` Jonathan Cameron
2024-02-19  6:56     ` Mike Looijmans

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=20240216170149.040ff86b@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=arnd@kernel.org \
    --cc=bigunclemax@gmail.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lars@metafoo.de \
    --cc=lgirdwood@gmail.com \
    --cc=liambeguin@gmail.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcus.folkesson@gmail.com \
    --cc=marius.cristea@microchip.com \
    --cc=mike.looijmans@topic.nl \
    --cc=okan.sahin@analog.com \
    --cc=schnelle@linux.ibm.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