linux-kernel - Re: [PATCH v2] drivers: counter: Add Cros EC Sync counter

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]

Message-ID: <20200425162101.21f7960e@archlinux>
Date:   Sat, 25 Apr 2020 16:21:01 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     William Breathitt Gray <vilhelm.gray@...il.com>
Cc:     Gwendal Grignou <gwendal@...omium.org>,
        Enric Balletbo i Serra <enric.balletbo@...labora.com>,
        Benson Leung <bleung@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-iio <linux-iio@...r.kernel.org>
Subject: Re: [PATCH v2] drivers: counter: Add Cros EC Sync counter

On Mon, 20 Apr 2020 15:08:10 -0400
William Breathitt Gray <vilhelm.gray@...il.com> wrote:

> On Mon, Apr 20, 2020 at 11:54:16AM -0700, Gwendal Grignou wrote:
> > On Tue, Apr 14, 2020 at 1:48 PM William Breathitt Gray
> > <vilhelm.gray@...il.com> wrote:  
> > >
> > > On Mon, Apr 13, 2020 at 12:55:14PM -0700, Gwendal Grignou wrote:  
> > > > When the camera vsync pin is connected to the embedded controller (EC) of
> > > > a chromebook, the EC reports a sensor with a counter that increases
> > > > at each GPIO rising edge.
> > > >
> > > > The sensor is presented using the counter subsystem.
> > > > In addition, it is also presented via the IIO subsystem with a timestamp,
> > > > allowing synchronisation with sensors connected to the same EC, for
> > > > image stabilisation or augmented reality applications.  
> > >
> > > Hi Gwendal,
> > >
> > > Sorry for the delay. I have some changes requested below.
> > >  
> > > > To enable the counter:
> > > > via counter ABI:
> > > > echo "rising edge" > counterX/count0/signal_action
> > > > via iio ABI
> > > > echo 1 > iio:deviceY/en
> > > >
> > > > To disable the counter:
> > > > via counter ABI:
> > > > echo "none" > counterX/count0/signal_action
> > > > via iio ABI
> > > > echo 0 > iio:deviceY/en  
> > >
> > > Although in theory a user could manually disable the actions for a
> > > Signal, this is a very roundabout way of actually disabling the Count.
> > > It's better to expose an "enable" attribute to allow the users to
> > > perform this functionality; for example:
> > >
> > > echo 0 > counterX/count0/enable
> > > echo 1 > counterX/count0/enable
> > >  
> > > >
> > > > To read the current counter value:
> > > > via counter ABI:
> > > > cat counterX/count0/count
> > > > via iio ABI
> > > > cat iio:deviceY/in_count_raw  
> > >
> > > I know we discussed this in the last review but it's still the same as
> > > before: IIO_COUNT interface is deprecated so new drivers won't be
> > > allowed to use it. You'll have to remove the IIO_COUNT code in this
> > > driver and replace it with Counter subsystem equivalents.  
> > I understand the need of a clean separation between counter and IIO subsystems.
> > I will wait for counter to offer a way to gather timestamp'ed counts.
> > Do you have a plan/proposed ABI you can share?
> > 
> > Thanks,
> > 
> > Gwendal.  
> 
> Hi Gwendal,
> 
> I'm working on a reimplementation of the internals of the Counter
> subsystem: https://gitlab.com/vilhelmgray/iio/-/tree/counter_chardev
> 
> I'm hoping to submit it to the mailing list later this week if I don't
> hit any delays; it'll include support as well for a character device
> interface for userspace application.
> 
> Once those changes are merged into IIO, I'll submit a patch to add
> timestamp support -- hopefully within a week or two after. Right now I
> haven't yet chosen any specific format for timestamps, but I will likely
> match the format IIO subsystem currently has for its timestamp support
> so that migration is easier for these drivers.
> 

Don't copy our crazy clock choosing stuff. That's legacy rubbish for
compatibility with a silly choice I made a long time ago.  Pick
a sensible clock type and stick to it.

Jonathan

> William Breathitt Gray