From: Jonathan Cameron <jic23@kernel.org>
To: David Lechner <dlechner@baylibre.com>
Cc: "Michael Hennerich" <Michael.Hennerich@analog.com>,
"Nuno Sá" <nuno.sa@analog.com>,
"Jonathan Corbet" <corbet@lwn.net>,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org
Subject: Re: [PATCH 1/2] iio: adc: ad7944: add support for chain mode
Date: Sun, 28 Apr 2024 17:29:23 +0100 [thread overview]
Message-ID: <20240428172923.7dfe00ff@jic23-huawei> (raw)
In-Reply-To: <20240425-iio-ad7944-chain-mode-v1-1-9d9220ff21e1@baylibre.com>
On Thu, 25 Apr 2024 09:09:59 -0500
David Lechner <dlechner@baylibre.com> wrote:
> This adds support for the chain mode of the AD7944 ADC. This mode allows
> multiple ADCs to be daisy-chained together. Data from all of the ADCs in
> is read by reading multiple words from the first ADC in the chain.
>
> Each chip in the chain adds an extra IIO input voltage channel to the
> IIO device.
>
> Only the wiring configuration where the SPI controller CS line is
> connected to the CNV pin of all of the ADCs in the chain is supported
> in this patch.
>
> Signed-off-by: David Lechner <dlechner@baylibre.com>
Looks good except for one minor tweak needed to ensure the allocated buffer
is zeroed as we don't necessarily overwrite the the whole thing.
Given that's all I found, I've just switched that to devm_kzalloc and
applied the series.
Applied to the togreg branch of iio.git and pushed out initially as testing
for 0-day to look at it.
Thanks,
Jonathan
>
> +/**
> + * ad7944_chain_mode_alloc - allocate and initialize channel specs and buffers
> + * for daisy-chained devices
> + * @dev: The device for devm_ functions
> + * @chan_template: The channel template for the devices (array of 2 channels
> + * voltage and timestamp)
> + * @n_chain_dev: The number of devices in the chain
> + * @chain_chan: Pointer to receive the allocated channel specs
> + * @chain_mode_buf: Pointer to receive the allocated rx buffer
> + * @chain_scan_masks: Pointer to receive the allocated scan masks
> + * Return: 0 on success, a negative error code on failure
> + */
> +static int ad7944_chain_mode_alloc(struct device *dev,
> + const struct iio_chan_spec *chan_template,
> + u32 n_chain_dev,
> + struct iio_chan_spec **chain_chan,
> + void **chain_mode_buf,
> + unsigned long **chain_scan_masks)
> +{
> + struct iio_chan_spec *chan;
> + size_t chain_mode_buf_size;
> + unsigned long *scan_masks;
> + void *buf;
> + int i;
> +
> + /* 1 channel for each device in chain plus 1 for soft timestamp */
> +
> + chan = devm_kcalloc(dev, n_chain_dev + 1, sizeof(*chan), GFP_KERNEL);
> + if (!chan)
> + return -ENOMEM;
> +
> + for (i = 0; i < n_chain_dev; i++) {
> + chan[i] = chan_template[0];
> +
> + if (chan_template[0].differential) {
> + chan[i].channel = 2 * i;
> + chan[i].channel2 = 2 * i + 1;
> + } else {
> + chan[i].channel = i;
> + }
> +
> + chan[i].scan_index = i;
> + }
> +
> + /* soft timestamp */
> + chan[i] = chan_template[1];
> + chan[i].scan_index = i;
> +
> + *chain_chan = chan;
> +
> + /* 1 word for each voltage channel + aligned u64 for timestamp */
> +
> + chain_mode_buf_size = ALIGN(n_chain_dev *
> + BITS_TO_BYTES(chan[0].scan_type.storagebits), sizeof(u64))
> + + sizeof(u64);
> + buf = devm_kmalloc(dev, chain_mode_buf_size, GFP_KERNEL);
Zero it - It's not a problem to leak stale ADC data or similar
into the gap between the data and the timestamp, but it is a problem
if it's general kernel data potentially leaking.
So play it safe and devm_kzalloc()
> + if (!buf)
> + return -ENOMEM;
> +
> + *chain_mode_buf = buf;
> +
> + /*
> + * Have to limit n_chain_dev due to current implementation of
> + * available_scan_masks.
> + */
> + if (n_chain_dev > BITS_PER_LONG)
> + return dev_err_probe(dev, -EINVAL,
> + "chain is limited to 32 devices\n");
> +
> + scan_masks = devm_kcalloc(dev, 2, sizeof(*scan_masks), GFP_KERNEL);
> + if (!scan_masks)
> + return -ENOMEM;
> +
> + /*
> + * Scan mask is needed since we always have to read all devices in the
> + * chain in one SPI transfer.
> + */
> + scan_masks[0] = GENMASK(n_chain_dev - 1, 0);
> +
> + *chain_scan_masks = scan_masks;
> +
> + return 0;
> +}
next prev parent reply other threads:[~2024-04-28 16:29 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-25 14:09 [PATCH 0/2] iio: adc: ad7944: implement chain mode support David Lechner
2024-04-25 14:09 ` [PATCH 1/2] iio: adc: ad7944: add support for chain mode David Lechner
2024-04-28 16:29 ` Jonathan Cameron [this message]
2024-04-25 14:10 ` [PATCH 2/2] docs: iio: ad7944: add documentation " 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=20240428172923.7dfe00ff@jic23-huawei \
--to=jic23@kernel.org \
--cc=Michael.Hennerich@analog.com \
--cc=corbet@lwn.net \
--cc=dlechner@baylibre.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nuno.sa@analog.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).