linux-kernel - Re: [PATCH v6 1/1] iio/scmi: Adding support for IIO SCMI Based Sensors

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Tue, 9 Mar 2021 10:36:49 +0000
From:   Cristian Marussi <cristian.marussi@....com>
To:     Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc:     Sudeep Holla <sudeep.holla@....com>,
        Jonathan Cameron <jic23@...nel.org>,
        Jyoti Bhayana <jbhayana@...gle.com>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Rob Herring <robh@...nel.org>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
        Enrico Granata <egranata@...gle.com>,
        Mikhail Golubev <mikhail.golubev@...nsynergy.com>,
        Igor Skalkin <Igor.Skalkin@...nsynergy.com>,
        Peter Hilber <Peter.hilber@...nsynergy.com>,
        Ankit Arora <ankitarora@...gle.com>,
        kernel test robot <lkp@...el.com>, cristian.marussi@....com
Subject: Re: [PATCH v6 1/1] iio/scmi: Adding support for IIO SCMI Based
 Sensors

Hi Jonathan,

On Tue, Mar 09, 2021 at 10:27:01AM +0000, Jonathan Cameron wrote:
> On Tue, 9 Mar 2021 07:38:13 +0000
> Cristian Marussi <cristian.marussi@....com> wrote:
> 
> > Hi
> > 
> > On Tue, Mar 09, 2021 at 06:37:27AM +0000, Sudeep Holla wrote:
> > > On Mon, Mar 08, 2021 at 07:48:41PM +0000, Jonathan Cameron wrote:  
> > > > On Mon, 8 Mar 2021 04:28:42 +0000
> > > > Sudeep Holla <sudeep.holla@....com> wrote:
> > > >  
> > > > > Hi Jonathan,
> > > > >
> > > > > On Tue, Feb 23, 2021 at 10:30:37AM -0800, Jyoti Bhayana wrote:  
> > > > > > Hi Jonathan,
> > > > > >
> > > > > > Thanks for the detailed and careful review of this patch. Good to hear
> > > > > > that v7 is not required.   Please find below answers to your
> > > > > > questions. Looking forward to seeing this patch merged in the next
> > > > > > cycle. Thanks for your help in making this happen.
> > > > > >  
> > > > >
> > > > > Any update on this ? Please share the branch with is patch so that I
> > > > > can pull and ask Cristian to make his changes on top.  
> > > > Running a bit behind at the moment.
> > > >  
> > > 
> > > No worries.
> > >   
> > > > Anyhow, there should now be an ib-iio-scmi-5.12-rc1 branch
> > > > on https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
> > > >  
> > > 
> > > Thanks
> > >   
> > > > Includes making the various long long local variables explicitly
> > > > s64 and u64 as relevant.
> > > >
> > > > Based on the rc1 that eats babies so handle with care :)
> > > >  
> > > 
> > > 😁 
> > >   
> > > > I've also merge this into the togreg branch of iio.git.
> > > > As explained above that wasn't entirely trivial so Jyoti
> > > > please take a quick look and check that changes are fine.
> > > > Pushed out as testing to let the autobuilders poke at it.
> > > > Assuming they don't find anything, it should be fine
> > > > for Sudeep to merge that ib and everything will unwind
> > > > nicely in Linus' tree next cycle.
> > > >  
> > > 
> > > Hope so.
> > >   
> > > > There is a bit of an ongoing discussion of an earlier patch
> > > > in the IIO tree, so I might end up redoing this merge
> > > > if I need to rebase to sort that out, but I'll make sure
> > > > the diff is the same (git ID might change).
> > > >  
> > > 
> > > I can wait for a week or 2 if you think things will settle down by then.
> > > We can avoid 2 different git IDs if possible. The main intention was to
> > > give some reference to Cristian to rebase/post his series. I am all
> > > fine to wait for a week or 2 for final branch.
> > >  
> > 
> > In the meantime, I've anyway started reworking just based on -rc2 and barely
> > cherry-picked Jyoti v6 on top of it just to able to start porting early the
> > iiodev to the new SCMI API and be able to fully test it out (no problems
> > so far); I will finally rebase on whatever final base branch Sudeep will
> > pick but as Sudeep said I can wait, since I'm not expecting so much work
> > still to do in that final rebase to -rc1-smh-final (...last famous words o_O)
> 
> Side note, there was a build bug on 32 bit that needs fixing so we'll be
> tweaking the original patch.
> 

Sure, thanks, I'll rework it anyway, it was just preliminary work.

> > 
> > Side question for Jyoti/Jonathan, for basic testing of this IIO SCMI
> > driver (given that I'm not really familiar with IIO and I have not a full
> > Android CTS/VTS suite to use it for testing), I'm doing something like:
> > 
> > (debian-arm64)root@...arm64:~# cd /sys/bus/iio/devices/iio\:device0
> > (debian-arm64)root@...arm64:/sys/bus/iio/devices/iio:device0# echo 1 > scan_elements/in_accel_x_en 
> > (debian-arm64)root@...arm64:/sys/bus/iio/devices/iio:device0# echo 1 > scan_elements/in_accel_y_en 
> > (debian-arm64)root@...arm64:/sys/bus/iio/devices/iio:device0# echo 1 > scan_elements/in_accel_z_en 
> > (debian-arm64)root@...arm64:/sys/bus/iio/devices/iio:device0# echo 1 > scan_elements/in_timestamp_en 
> > (debian-arm64)root@...arm64:/sys/bus/iio/devices/iio:device0# echo 1 > buffer/enable && cat /dev/iio\:device0 | xxd
> > 00000000: 08da ffff ffff ffff 10da ffff ffff ffff  ................
> > 00000010: 18da ffff ffff ffff 00b8 d3a0 c09b 6a16  ..............j.
> > 00000020: 08da ffff ffff ffff 10da ffff ffff ffff  ................
> > 00000030: 18da ffff ffff ffff 0082 6edc c09b 6a16  ..........n...j.
> > 00000040: 08da ffff ffff ffff 10da ffff ffff ffff  ................
> > 00000050: 18da ffff ffff ffff 004c 0918 c19b 6a16  .........L....j.
> > 00000060: 08da ffff ffff ffff 10da ffff ffff ffff  ................
> > 00000070: 18da ffff ffff ffff 0016 a453 c19b 6a16  ...........S..j.
> > 00000080: 08da ffff ffff ffff 10da ffff ffff ffff  ................
> > 00000090: 18da ffff ffff ffff 00aa d9ca c19b 6a16  ..............j.
> > 000000a0: 08da ffff ffff ffff 10da ffff ffff ffff  ................
> > 000000b0: 18da ffff ffff ffff 0074 7406 c29b 6a16  .........tt...j.
> > ^C
> > (debian-arm64)root@...arm64:/sys/bus/iio/devices/iio:device0# echo 0 > buffer/enable 
> That looks superficially plausible. 
> 

Yes, definitely not an in depth testing, I was just trying to double
check not to break the general mechanism when porting to the new API.

> There is a test tool in 
> tools/iio/iio_generic_buffer.c which is fairly simple to use and will pretty print values for
> you.  That'll sanity check all the type descriptions etc are correct as well.
> 

Cool, thanks a lot I'll give it a go.

Thanks

Cristian

> Jonathan
> 
> > 
> > (plus a bunch of DEBUG in my series to track notifs flows...)
> > 
> > Is this any good ?
> > 
> > Thanks
> > 
> > Cristian
> > 
> > > --
> > > Regards,
> > > Sudeep  
>