Linux IIO development
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Fabio Estevam <festevam@gmail.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	mazziesaccount@gmail.com, linux-iio@vger.kernel.org,
	Fabio Estevam <festevam@denx.de>,
	stable@vger.kernel.org
Subject: Re: [PATCH 1/2] iio: Fix scan mask subset check logic
Date: Sun, 4 May 2025 18:04:20 +0100	[thread overview]
Message-ID: <20250504180420.73b96437@jic23-huawei> (raw)
In-Reply-To: <CAOMZO5BCLWFJ=83r0saT=NxVP0f9G-P-2QosDNGArYAtX6v5Lw@mail.gmail.com>

On Wed, 30 Apr 2025 16:37:30 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> Hi Jonathan,
> 
> On Wed, Apr 30, 2025 at 2:25 PM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> 
> > Should be:
> > #define MAX1363_4X_CHANS(bits, ev_spec, num_ev_spec) {                  \
> >         MAX1363_CHAN_U(0, _s0, 0, bits, ev_spec, num_ev_spec),          \
> >         MAX1363_CHAN_U(1, _s1, 1, bits, ev_spec, num_ev_spec),          \
> >         MAX1363_CHAN_U(2, _s2, 2, bits, ev_spec, num_ev_spec),          \
> >         MAX1363_CHAN_U(3, _s3, 3, bits, ev_spec, num_ev_spec),          \
> >         MAX1363_CHAN_B(0, 1, d0m1, 12, bits, ev_spec, num_ev_spec),     \
> >         MAX1363_CHAN_B(2, 3, d2m3, 13, bits, ev_spec, num_ev_spec),     \
> >         MAX1363_CHAN_B(1, 0, d1m0, 14, bits, ev_spec, num_ev_spec),     \
> >         MAX1363_CHAN_B(3, 2, d3m2, 15, bits, ev_spec, num_ev_spec),     \
> >         IIO_CHAN_SOFT_TIMESTAMP(8)                                      \
> >         }  
> 
> I have applied your suggestion.
> 
> Please take a look and let me know your thoughts.
> 
> --- a/drivers/iio/adc/max1363.c
> +++ b/drivers/iio/adc/max1363.c
> @@ -504,10 +504,10 @@ static const struct iio_event_spec max1363_events[] = {
>         MAX1363_CHAN_U(1, _s1, 1, bits, ev_spec, num_ev_spec),          \
>         MAX1363_CHAN_U(2, _s2, 2, bits, ev_spec, num_ev_spec),          \
>         MAX1363_CHAN_U(3, _s3, 3, bits, ev_spec, num_ev_spec),          \
> -       MAX1363_CHAN_B(0, 1, d0m1, 4, bits, ev_spec, num_ev_spec),      \
> -       MAX1363_CHAN_B(2, 3, d2m3, 5, bits, ev_spec, num_ev_spec),      \
> -       MAX1363_CHAN_B(1, 0, d1m0, 6, bits, ev_spec, num_ev_spec),      \
> -       MAX1363_CHAN_B(3, 2, d3m2, 7, bits, ev_spec, num_ev_spec),      \
> +       MAX1363_CHAN_B(0, 1, d0m1, 12, bits, ev_spec, num_ev_spec),     \
> +       MAX1363_CHAN_B(2, 3, d2m3, 13, bits, ev_spec, num_ev_spec),     \
> +       MAX1363_CHAN_B(1, 0, d1m0, 14, bits, ev_spec, num_ev_spec),     \
> +       MAX1363_CHAN_B(3, 2, d3m2, 15, bits, ev_spec, num_ev_spec),     \

Ah. I was wrong for last two channels above. The scan masks for single channel for
differential channels also index a bunch we aren't using in this device.

> +       MAX1363_CHAN_B(0, 1, d0m1, 12, bits, ev_spec, num_ev_spec),     \
> +       MAX1363_CHAN_B(2, 3, d2m3, 13, bits, ev_spec, num_ev_spec),     \
//gap here as we aren't using 4,5  6,7 8,9, 10,11

> +       MAX1363_CHAN_B(1, 0, d1m0, 14, bits, ev_spec, num_ev_spec),     \
Should be 18
https://elixir.bootlin.com/linux/v6.14.5/source/drivers/iio/adc/max1363.c#L262
> +       MAX1363_CHAN_B(3, 2, d3m2, 15, bits, ev_spec, num_ev_spec),     \
and 19 I believe.

Maybe we can simplify this and make it less error prone. These scan indices
are always the same as the enum max1363_mode entries in the 2nd or 3rd parameter.
We can just reuse those I think.  Only the single channel ones apply here
but those are the first set of entries in that enum.

Bonus points for just dropping the parameter and using the existing addr
parameter for the macro as si as well.

That is:
#define MAX1363_CHAN_U(num, addr, bits, ev_spec, num_ev_spec)	\
	{								\
		.type = IIO_VOLTAGE,					\
		.indexed = 1,						\
		.channel = num,						\
		.address = addr,					\
		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
		.datasheet_name = "AIN"#num,				\
		.scan_type = {						\
			.sign = 'u',					\
			.realbits = bits,				\
			.storagebits = (bits > 8) ? 16 : 8,		\
			.endianness = IIO_BE,				\
		},							\
		.scan_index = (addr),					\
The above changed from si to addr
		.event_spec = ev_spec,					\
		.num_event_specs = num_ev_spec,				\
	}


>         IIO_CHAN_SOFT_TIMESTAMP(8)                                      \
>         }
> 
> @@ -577,8 +577,8 @@ static const struct iio_chan_spec
> max1238_channels[] = MAX1363_12X_CHANS(12);
> 
>  static const enum max1363_modes max11607_mode_list[] = {
>         _s0, _s1, _s2, _s3,
> -       s0to1, s0to2, s0to3,
> -       s2to3,
> +       s0to1, s0to2, s2to3,
> +       s0to3,
>         d0m1, d2m3, d1m0, d3m2,
>         d0m1to2m3, d1m0to3m2,
>  };
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 6a6568d4a2cb..8c9330a9f615 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1947,15 +1947,21 @@ static void
> iio_sanity_check_avail_scan_masks(struct iio_dev *indio_dev)
>          * available masks in the order of preference (presumably the least
>          * costy to access masks first).
>          */
> +       pr_err("******** masklength is %d", masklength);
> +       pr_err("******** longs_per_mask is %d", longs_per_mask);
> +
> +
>         for (i = 0; i < num_masks - 1; i++) {
>                 const unsigned long *mask1;
>                 int j;
> 
>                 mask1 = av_masks + i * longs_per_mask;
> +               pr_err("******** mask1[%d] is 0x%lx\n", i, mask1[0]);
>                 for (j = i + 1; j < num_masks; j++) {
>                         const unsigned long *mask2;
> 
>                         mask2 = av_masks + j * longs_per_mask;
> +                       pr_err("******** mask2[%d] is 0x%lx\n", i, mask2[0]);
>                         if (bitmap_subset(mask2, mask1, masklength))
>                                 dev_warn(indio_dev->dev.parent,
>                                          "available_scan_mask %d
> subset of %d. Never used\n",
> 
> [    1.569250] iio-core: ******** masklength is 16
> [    1.569261] iio-core: ******** longs_per_mask is 1
> [    1.573834] iio-core: ******** mask1[0] is 0x1
> [    1.583173] iio-core: ******** mask2[0] is 0x2
> [    1.587629] iio-core: ******** mask2[0] is 0x4
> [    1.592091] iio-core: ******** mask2[0] is 0x8
> [    1.596569] iio-core: ******** mask2[0] is 0x3
> [    1.601027] iio-core: ******** mask2[0] is 0x7
> [    1.605482] iio-core: ******** mask2[0] is 0xc
> [    1.609942] iio-core: ******** mask2[0] is 0xf
> [    1.614400] iio-core: ******** mask2[0] is 0x1000
> [    1.619118] iio-core: ******** mask2[0] is 0x2000
> [    1.623832] iio-core: ******** mask2[0] is 0x40000
> [    1.628642] max1363 1-0064: available_scan_mask 10 subset of 0. Never used
> [    1.635543] iio-core: ******** mask2[0] is 0x80000
> [    1.640343] max1363 1-0064: available_scan_mask 11 subset of 0. Never used
> [    1.647229] iio-core: ******** mask2[0] is 0x3000
> [    1.651943] iio-core: ******** mask2[0] is 0xc0000
> [    1.656745] max1363 1-0064: available_scan_mask 13 subset of 0. Never used
> [    1.663631] iio-core: ******** mask1[1] is 0x2
> [    1.668088] iio-core: ******** mask2[1] is 0x4
> [    1.672544] iio-core: ******** mask2[1] is 0x8
> [    1.676997] iio-core: ******** mask2[1] is 0x3
> [    1.681452] iio-core: ******** mask2[1] is 0x7
> [    1.685905] iio-core: ******** mask2[1] is 0xc
> [    1.690365] iio-core: ******** mask2[1] is 0xf
> [    1.694820] iio-core: ******** mask2[1] is 0x1000
> [    1.699535] iio-core: ******** mask2[1] is 0x2000
> [    1.704251] iio-core: ******** mask2[1] is 0x40000
> [    1.709059] max1363 1-0064: available_scan_mask 10 subset of 1. Never used
> [    1.715945] iio-core: ******** mask2[1] is 0x80000
> [    1.720747] max1363 1-0064: available_scan_mask 11 subset of 1. Never used
> [    1.727632] iio-core: ******** mask2[1] is 0x3000
> [    1.732349] iio-core: ******** mask2[1] is 0xc0000
> [    1.737150] max1363 1-0064: available_scan_mask 13 subset of 1. Never used
> [    1.744038] iio-core: ******** mask1[2] is 0x4
> [    1.748488] iio-core: ******** mask2[2] is 0x8
> [    1.752941] iio-core: ******** mask2[2] is 0x3
> [    1.757393] iio-core: ******** mask2[2] is 0x7
> [    1.761846] iio-core: ******** mask2[2] is 0xc
> [    1.766300] iio-core: ******** mask2[2] is 0xf
> [    1.770756] iio-core: ******** mask2[2] is 0x1000
> [    1.775469] iio-core: ******** mask2[2] is 0x2000
> [    1.780184] iio-core: ******** mask2[2] is 0x40000
> [    1.784986] max1363 1-0064: available_scan_mask 10 subset of 2. Never used
> [    1.791871] iio-core: ******** mask2[2] is 0x80000
> [    1.796673] max1363 1-0064: available_scan_mask 11 subset of 2. Never used
> [    1.803558] iio-core: ******** mask2[2] is 0x3000
> [    1.808271] iio-core: ******** mask2[2] is 0xc0000
> [    1.813072] max1363 1-0064: available_scan_mask 13 subset of 2. Never used
> [    1.819961] iio-core: ******** mask1[3] is 0x8
> [    1.824415] iio-core: ******** mask2[3] is 0x3
> [    1.828865] iio-core: ******** mask2[3] is 0x7
> [    1.833317] iio-core: ******** mask2[3] is 0xc
> [    1.837771] iio-core: ******** mask2[3] is 0xf
> [    1.842224] iio-core: ******** mask2[3] is 0x1000
> [    1.846937] iio-core: ******** mask2[3] is 0x2000
> [    1.851649] iio-core: ******** mask2[3] is 0x40000
> [    1.856448] max1363 1-0064: available_scan_mask 10 subset of 3. Never used
> [    1.863334] iio-core: ******** mask2[3] is 0x80000
> [    1.868135] max1363 1-0064: available_scan_mask 11 subset of 3. Never used
> [    1.875026] iio-core: ******** mask2[3] is 0x3000
> [    1.879739] iio-core: ******** mask2[3] is 0xc0000
> [    1.884540] max1363 1-0064: available_scan_mask 13 subset of 3. Never used
> [    1.891427] iio-core: ******** mask1[4] is 0x3
> [    1.895882] iio-core: ******** mask2[4] is 0x7
> [    1.900334] iio-core: ******** mask2[4] is 0xc
> [    1.904787] iio-core: ******** mask2[4] is 0xf
> [    1.909239] iio-core: ******** mask2[4] is 0x1000
> [    1.913952] iio-core: ******** mask2[4] is 0x2000
> [    1.918666] iio-core: ******** mask2[4] is 0x40000
> [    1.923467] max1363 1-0064: available_scan_mask 10 subset of 4. Never used
> [    1.930353] iio-core: ******** mask2[4] is 0x80000
> [    1.935154] max1363 1-0064: available_scan_mask 11 subset of 4. Never used
> [    1.942045] iio-core: ******** mask2[4] is 0x3000
> [    1.946759] iio-core: ******** mask2[4] is 0xc0000
> [    1.951560] max1363 1-0064: available_scan_mask 13 subset of 4. Never used
> [    1.958445] iio-core: ******** mask1[5] is 0x7
> [    1.962898] iio-core: ******** mask2[5] is 0xc
> [    1.967351] iio-core: ******** mask2[5] is 0xf
> [    1.971804] iio-core: ******** mask2[5] is 0x1000
> [    1.976515] iio-core: ******** mask2[5] is 0x2000
> [    1.981233] iio-core: ******** mask2[5] is 0x40000
> [    1.986034] max1363 1-0064: available_scan_mask 10 subset of 5. Never used
> [    1.992920] iio-core: ******** mask2[5] is 0x80000
> [    1.997722] max1363 1-0064: available_scan_mask 11 subset of 5. Never used
> [    2.004612] iio-core: ******** mask2[5] is 0x3000
> [    2.009327] iio-core: ******** mask2[5] is 0xc0000
> [    2.014125] max1363 1-0064: available_scan_mask 13 subset of 5. Never used
> [    2.021011] iio-core: ******** mask1[6] is 0xc
> [    2.025463] iio-core: ******** mask2[6] is 0xf
> [    2.029916] iio-core: ******** mask2[6] is 0x1000
> [    2.034629] iio-core: ******** mask2[6] is 0x2000
> [    2.039343] iio-core: ******** mask2[6] is 0x40000
> [    2.044146] max1363 1-0064: available_scan_mask 10 subset of 6. Never used
> [    2.051031] iio-core: ******** mask2[6] is 0x80000
> [    2.055832] max1363 1-0064: available_scan_mask 11 subset of 6. Never used
> [    2.062717] iio-core: ******** mask2[6] is 0x3000
> [    2.067435] iio-core: ******** mask2[6] is 0xc0000
> [    2.072237] max1363 1-0064: available_scan_mask 13 subset of 6. Never used
> [    2.079123] iio-core: ******** mask1[7] is 0xf
> [    2.083579] iio-core: ******** mask2[7] is 0x1000
> [    2.088293] iio-core: ******** mask2[7] is 0x2000
> [    2.093006] iio-core: ******** mask2[7] is 0x40000
> [    2.097808] max1363 1-0064: available_scan_mask 10 subset of 7. Never used
> [    2.104694] iio-core: ******** mask2[7] is 0x80000
> [    2.109493] max1363 1-0064: available_scan_mask 11 subset of 7. Never used
> [    2.116378] iio-core: ******** mask2[7] is 0x3000
> [    2.121091] iio-core: ******** mask2[7] is 0xc0000
> [    2.125892] max1363 1-0064: available_scan_mask 13 subset of 7. Never used
> [    2.132777] iio-core: ******** mask1[8] is 0x1000
> [    2.137490] iio-core: ******** mask2[8] is 0x2000
> [    2.142203] iio-core: ******** mask2[8] is 0x40000
> [    2.147009] max1363 1-0064: available_scan_mask 10 subset of 8. Never used
> [    2.153896] iio-core: ******** mask2[8] is 0x80000
> [    2.158698] max1363 1-0064: available_scan_mask 11 subset of 8. Never used
> [    2.165583] iio-core: ******** mask2[8] is 0x3000
> [    2.170297] iio-core: ******** mask2[8] is 0xc0000
> [    2.175098] max1363 1-0064: available_scan_mask 13 subset of 8. Never used
> [    2.181983] iio-core: ******** mask1[9] is 0x2000
> [    2.186698] iio-core: ******** mask2[9] is 0x40000
> [    2.191504] max1363 1-0064: available_scan_mask 10 subset of 9. Never used
> [    2.198390] iio-core: ******** mask2[9] is 0x80000
> [    2.203191] max1363 1-0064: available_scan_mask 11 subset of 9. Never used
> [    2.210077] iio-core: ******** mask2[9] is 0x3000
> [    2.214788] iio-core: ******** mask2[9] is 0xc0000
> [    2.219591] max1363 1-0064: available_scan_mask 13 subset of 9. Never used
> [    2.226476] iio-core: ******** mask1[10] is 0x40000
> [    2.231363] iio-core: ******** mask2[10] is 0x80000
> [    2.236250] max1363 1-0064: available_scan_mask 11 subset of 10. Never used
> [    2.243222] iio-core: ******** mask2[10] is 0x3000
> [    2.248022] iio-core: ******** mask2[10] is 0xc0000
> [    2.252910] max1363 1-0064: available_scan_mask 13 subset of 10. Never used
> [    2.259882] iio-core: ******** mask1[11] is 0x80000
> [    2.264770] iio-core: ******** mask2[11] is 0x3000
> [    2.269570] iio-core: ******** mask2[11] is 0xc0000
> [    2.274458] max1363 1-0064: available_scan_mask 13 subset of 11. Never used
> [    2.281430] iio-core: ******** mask1[12] is 0x3000
> [    2.286229] iio-core: ******** mask2[12] is 0xc0000
> [    2.291120] max1363 1-0064: available_scan_mask 13 subset of 12. Never used
> 


  parent reply	other threads:[~2025-05-04 17:04 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-29 15:02 [PATCH 1/2] iio: Fix scan mask subset check logic Fabio Estevam
2025-04-29 15:02 ` [PATCH 2/2] iio: adc: max1363: Reorder max11607_mode_list[] Fabio Estevam
2025-04-29 17:35   ` Jonathan Cameron
2025-04-29 18:37     ` Fabio Estevam
2025-04-29 17:33 ` [PATCH 1/2] iio: Fix scan mask subset check logic Jonathan Cameron
2025-04-29 18:33   ` Fabio Estevam
2025-04-29 19:05     ` Fabio Estevam
2025-04-30 13:11       ` Jonathan Cameron
2025-04-30 13:26         ` Fabio Estevam
2025-04-30 17:25           ` Jonathan Cameron
2025-04-30 19:37             ` Fabio Estevam
2025-05-03 12:02               ` Fabio Estevam
2025-05-04 17:06                 ` Jonathan Cameron
2025-05-04 17:04               ` Jonathan Cameron [this message]
2025-05-05 14:24                 ` Fabio Estevam
2025-05-05 15:16                   ` Jonathan Cameron
2025-05-08 13:29                     ` Fabio Estevam
2025-05-08 19:05                       ` 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=20250504180420.73b96437@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=festevam@denx.de \
    --cc=festevam@gmail.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=mazziesaccount@gmail.com \
    --cc=stable@vger.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