Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

> On Mar 25, 2018, at 9:54 AM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> 
> On Sun, 25 Mar 2018 01:29:41 -0700
> John Syne <john3909@xxxxxxxxx> wrote:
> 
>> Hi Jonathan,
> Hi John,
> 
> Please wrap your normal emails (excepting tables) to 80 chars.
Yeah, Iâm trying to do that, but I use a Mac and Apple Mail doesnât
have a feature to wrap at 80 automatically so I have to do
this manually.
> 
>> 
>> I was speaking with Rodrigo and here is what I think must be done to move
>> ADE7878 out of staging
>> 
>> Here are the steps as I see them:
>> 
>> 1) Define the IIO Attributes so they are consistent with the IIO ABI. This
>>   should be pretty simple given agreement on the naming convention.
> Please don't go just changing attribute names - the driver needs to
> use the standard approach of iio_chan_spec and create the vast
> majority of attributes automatically via that.  There 'may' be
> a few corner cases wee don't want to make generic and they 'might'
> be exposed as attributes.
> 
> I suspect this change is the 'big' job.  The core support necessary
> for RMS and MAV (computedtype or whatever we call it) will need adding
> as well.  That is a separate patch set with support for some examples
> in the dummy driver.
Great that you mentioned this, because I though we only needed to
modify the IIO_ATTR naming. Iâll take a look at iio_chan_spec and
the examples in the dummy driver.
> 
>> 2) Map the ADE7854 interrupt status to IIO events. This requires an
>>   interrupt processing section.
>> 3) Add DeviceTree support.
>> 4) Create DeviceTree overlay for the ADE7854.
> More a case of bindings for now.  If those are used via an overlay
> fine but given we don't have any boards with one one in mainline,
> this is an implementation detail for the user rather than part
> of moving this driver out of staging.
I was thinking more along the lines of documentation to show a
developer what was needed to get the driver working.
> 
>> 5) Update ADE7854 probe to read in the DeviceTree register settings.
>> 6) Add support for power modes (PM1, PM2).
> This isn't necessary for a move out of staging (nice to have though).
> 
>> 7) Not sure if we will support measurement streaming on the ADE7854.
>>   The problem is ADE7854 is designed as an SPI master, which means
>>   it controls the SPI clock, so the driver must support SPI slave
>>   mode. However, the Linux Kernel does not currently support SPI
>>   slave mode. We have three choices to make this work and they
>>   are all a lot of work: 1) Add support for SPI Slave mode to the
>>   kernel,  2) Use hardware to convert SPI signals to I2S signals
>>   and with the use of a custom codec, use the ALSA framework to
>>   stream the samples (this is an approach I used, but I donât like
>>   it), 3) Move the I2S driver out of the sound subsystem and use it
>>   together with DMA to stream samples directly into the ADE7854 driver
>>   (my preferred solutions). Perhaps Mark Brown has some ideas on how
>>    to make this work. 
> I'll be honest, this is an end of line part and frankly more than
> a little crazy. I would go with simply not supporting the measurement
> streaming at all for this part.  If you really need it we can then
> move onto the how part, but from what you have said I'm guessing you
> don't care except in an abstract 'it would be nice' sort of a way?
Yeah, Iâm moving to the ADE9000 so Iâll drop this.
> 
>> 
>> The ADE9000 will be much easier because it uses an SPI Slave interface. 
>> 
>> I hope I have captured everything, but let me know if I have missed anything.
>> 
> 
> That will do for now ;)  I'm sure there will be details that need
> tidying up once we have the above done, but that's true for any new
> driver (and this will be nearly a new driver before things are done).
Thank you again for all the detailed feedback.
> 
> Jonathan