Hi Jonathan,
On Sun, May 4, 2025 at 2:04 PM Jonathan Cameron jic23@kernel.org wrote:
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#L2...
MAX1363_CHAN_B(3, 2, d3m2, 15, bits, ev_spec, num_ev_spec), \and 19 I believe.
This works fine for me, thanks:
--- 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, 18, bits, ev_spec, num_ev_spec), \ + MAX1363_CHAN_B(3, 2, d3m2, 19, bits, ev_spec, num_ev_spec), \ IIO_CHAN_SOFT_TIMESTAMP(8) \ }
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
I tried this change on top of the previous one, and it still works.
How do you think we should proceed? Can you please submit formal patches?
You can add:
Reported-by: Fabio Estevam festevam@denx.de Tested-by: Fabio Estevam festevam@denx.de
Thanks