Date:   Sat, 24 Jun 2023 18:59:31 +0300
From:   George Stark <stark.georgy@...il.com>
To:     Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        George Stark <gnstark@...rdevices.ru>
Cc:     jic23@...nel.org, lars@...afoo.de, neil.armstrong@...aro.org,
        khilman@...libre.com, jbrunet@...libre.com,
        andriy.shevchenko@...ux.intel.com, nuno.sa@...log.com,
        linux-iio@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-amlogic@...ts.infradead.org,
        kernel@...rdevices.ru
Subject: Re: [PATCH v2 6/6] meson saradc: support reading from channel7 mux
 inputs

Hello Martin

Thanks for review

On 6/23/23 09:16, Martin Blumenstingl wrote:
> Hi George,
>
> On Fri, Jun 23, 2023 at 4:23 AM George Stark <gnstark@...rdevices.ru> wrote:
> [...]
>> Meson saradc channel 7 is connected to muxer that can switch channel
> I think that this should read: ... is connected to a mux that ...
>
> [...]
>>   static const struct iio_chan_spec meson_sar_adc_iio_channels[] = {
>> @@ -245,6 +280,11 @@ static const struct iio_chan_spec meson_sar_adc_iio_channels[] = {
>>          MESON_SAR_ADC_CHAN(INDEX_CHAN_6),
>>          MESON_SAR_ADC_CHAN(INDEX_CHAN_7),
>>          IIO_CHAN_SOFT_TIMESTAMP(INDEX_CHAN_SOFT_TIMESTAMP),
>> +       MESON_SAR_ADC_MUX(INDEX_MUX_0_VSS, 0),
>> +       MESON_SAR_ADC_MUX(INDEX_MUX_1_VDD_DIV4, 1),
>> +       MESON_SAR_ADC_MUX(INDEX_MUX_2_VDD_DIV2, 2),
>> +       MESON_SAR_ADC_MUX(INDEX_MUX_3_VDD_MUL3_DIV4, 3),
>> +       MESON_SAR_ADC_MUX(INDEX_MUX_4_VDD, 4),
>>          MESON_SAR_ADC_TEMP_CHAN(), /* must be the last item */
> I haven't had the chance to run these patches yet but: I think they
> are breaking the temperature sensor readings on Meson8/8b/8m2 boards.
> See arch/arm/boot/dts/meson.dtsi where the temperature channel is
> being referenced:
>    io-channels = <&saradc 8>
>
> With this series (this patch and I think also patch 3/6 "meson saradc:
> unite iio channel array definitions") the numbering of the temperature
> sensor channel changes.
>
> To make things worse: in theory we can use meson_saradc to read the
> SoC temperature sensor on GXBB, GXL and GXM boards (possibly on AXG as
> well but I can't recall from the top of my head) instead of going
> through SCPI.
> I have experimented with this in the past but never got it to work.
> Doing so in the future could lead to another channel index change,
> depending on how we decide to go forward now.
>
> There's two that I can think of:
> - update meson.dtsi to use the new channel numbering (I don't expect
> many 32-bit SoC users out there using new kernel + old .dtbs, but it's
> impossible to say for sure)
> - or keep the driver backwards compatible (that involves re-adding the
> channel tables)
>
> What do you think?
Actually we'd have to make 2 patches to meson.dtsi, the first change 
8->9, than 9 ->14.
And if that index exposed externally (ABI like) I'd not change it 
without good reason at all.
So I think to return to double definition of meson_sar_adc_iio_channels 
and keep the driver backwards compatible.

I've just realized another moment with channels defined after 
MESON_SAR_ADC_TEMP_CHAN in channel array.
In dts by default channels are referenced by channel array index not 
even by channel number.
So channel e.g MUX_0_VSS will have the same number (due to enum patch) 
but different index on meson8 and gxbb.
As alternative we can implement fwnode_xlate method in meson adc driver 
and use channel numbers in dts
(probably not in the current patchset).

Best regards,
George

>
> Best regards,
> Martin
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

Powered by blists - more mailing lists