linux-kernel - Re: [PATCH v2 6/6] meson saradc: support reading from channel7 mux inputs

Open Source and information security mailing list archives

Message-ID: <20230625121916.32578914@jic23-huawei>
Date:   Sun, 25 Jun 2023 12:19:16 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     George Stark <stark.georgy@...il.com>
Cc:     Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        George Stark <gnstark@...rdevices.ru>, 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

On Sat, 24 Jun 2023 18:59:31 +0300
George Stark <stark.georgy@...il.com> wrote:

> 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)

Agreed. Put the table back.  It might make the code slightly more complex
but we need to avoid shifting channel numbers around where possible.

Jonathan

> >
> > 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  
> 
> 

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives