Re: "Generic" MMIO ADC

Re: "Generic" MMIO ADC

[Jonathan Cameron]

On Fri, 30 Jun 2017 10:44:42 +0200
Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org> wrote:

> I'm writing an IIO driver for a custom IP, which is pretty simple:
> 
> - Map the registers to memory and register IRQ routine
> - To start acquisition, write a value to the config register
> - On interrupt, read data register, push it into queue
> - To stop, write another value to config.
> 
> Would it be useful to provide this as a generic driver in Linux, so that you 
> can provide the config register and data register offsets and values in the 
> devicetree? That would make it work on a lot of similar project without 
> everyone coding the same thing.

Hi Mike,

I've brought in the devicetree bindings maintainers and list as well on this
discussion because in many ways it's more about the strategy on bindings
than the actual driver...  Also Linus Walleij for any experience handing
the gpio-mmio driver which strikes me as fairly similar in aim.

To the driver question, yes absolutely this would be a good thing to have.
The majority of ADCs out there are irritatingly more complex than what
you have.  Everyone seems to feel obliged to put in their own 'special
sauce'. 

As for bindings, there are two options, either we

1) As you suggest push all differences between different parts into the
bindings.  Note we'll still need a compatible list in the driver.
So this is something like gpio-mmio.c

2) Do something similar to the irq generic_chip (CONFIG_GENERIC_IRQ)
infrastructure and actually have very slim wrapper drivers around a
library.   That allows those elements that are constrained by the
hardware to not need specifying in device tree bindings.

> 
> And a related question: Is there a driver that's close to this?
I can't immediately think of one, but it's possible that one
of the SoC ADC drivers is simple enough to describe this way.

So Mark / Rob.  You guys have experience with how the bindings
worked out for each approach.  Which do you prefer?
Either approach is fine with me.

Mike, perhaps you have an initial cut of some bindings that might
help the discussion to move along?

Jonathan


> 
> 
> Kind regards,
> 
> Mike Looijmans
> System Expert
> 
> TOPIC Products
> Materiaalweg 4, NL-5681 RJ Best
> Postbus 440, NL-5680 AK Best
> Telefoon: +31 (0) 499 33 69 79
> E-mail: mike.looijmans-yhtFebqMsb9it5bFGTN0CAC/G2K4zDHf@public.gmane.org
> Website: www.topicproducts.com
> 
> Please consider the environment before printing this e-mail
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: "Generic" MMIO ADC

[Linus Walleij]

On Fri, Jun 30, 2017 at 3:53 PM, Jonathan Cameron
<jic23-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org> wrote:
> On Fri, 30 Jun 2017 10:44:42 +0200
> Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org> wrote:
>
>> I'm writing an IIO driver for a custom IP, which is pretty simple:
>>
>> - Map the registers to memory and register IRQ routine
>> - To start acquisition, write a value to the config register
>> - On interrupt, read data register, push it into queue
>> - To stop, write another value to config.
>>
>> Would it be useful to provide this as a generic driver in Linux, so that you
>> can provide the config register and data register offsets and values in the
>> devicetree? That would make it work on a lot of similar project without
>> everyone coding the same thing.
>
> Hi Mike,
>
> I've brought in the devicetree bindings maintainers and list as well on this
> discussion because in many ways it's more about the strategy on bindings
> than the actual driver...  Also Linus Walleij for any experience handing
> the gpio-mmio driver which strikes me as fairly similar in aim.

The gpio-mmio is for the simple case where we have one bit per
GPIO, one bit reads/writes value, one bit sets direction etc.

(I am happy we did not call it "gpio-simple" because these "simple"
things are seldom simple in practice. So MMIO ADC is a good name.)

I would ask the following architectural questions:

- Is it really that simple, no clocks, regulator etc?

- Is it advisible to have it as a child of a syscon, using "simple-mfd"
  to spawn the child device c.f.
  Documentation/devicetree/bindings/leds/register-bit-led.txt
  drivers/leds/leds-syscon.c

- drivers/power/reset/syscon-*
  also use the syscon child approach

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: "Generic" MMIO ADC

[Rob Herring]

On Fri, Jun 30, 2017 at 8:53 AM, Jonathan Cameron
<jic23-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org> wrote:
> On Fri, 30 Jun 2017 10:44:42 +0200
> Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org> wrote:
>
>> I'm writing an IIO driver for a custom IP, which is pretty simple:
>>
>> - Map the registers to memory and register IRQ routine
>> - To start acquisition, write a value to the config register
>> - On interrupt, read data register, push it into queue
>> - To stop, write another value to config.
>>
>> Would it be useful to provide this as a generic driver in Linux, so that you
>> can provide the config register and data register offsets and values in the
>> devicetree? That would make it work on a lot of similar project without
>> everyone coding the same thing.
>
> Hi Mike,
>
> I've brought in the devicetree bindings maintainers and list as well on this
> discussion because in many ways it's more about the strategy on bindings
> than the actual driver...  Also Linus Walleij for any experience handing
> the gpio-mmio driver which strikes me as fairly similar in aim.
>
> To the driver question, yes absolutely this would be a good thing to have.
> The majority of ADCs out there are irritatingly more complex than what
> you have.  Everyone seems to feel obliged to put in their own 'special
> sauce'.
>
> As for bindings, there are two options, either we
>
> 1) As you suggest push all differences between different parts into the
> bindings.  Note we'll still need a compatible list in the driver.
> So this is something like gpio-mmio.c
>
> 2) Do something similar to the irq generic_chip (CONFIG_GENERIC_IRQ)
> infrastructure and actually have very slim wrapper drivers around a
> library.   That allows those elements that are constrained by the
> hardware to not need specifying in device tree bindings.
>
>>
>> And a related question: Is there a driver that's close to this?
> I can't immediately think of one, but it's possible that one
> of the SoC ADC drivers is simple enough to describe this way.
>
> So Mark / Rob.  You guys have experience with how the bindings
> worked out for each approach.  Which do you prefer?
> Either approach is fine with me.

The latter case. Specific bindings can map to a generic driver or
evolve to a specific driver if the generic one becomes inadequate.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: "Generic" MMIO ADC

[Mike Looijmans]

On 30-06-17 15:53, Jonathan Cameron wrote:
> On Fri, 30 Jun 2017 10:44:42 +0200
> Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org> wrote:
> 
>> I'm writing an IIO driver for a custom IP, which is pretty simple:
>>
>> - Map the registers to memory and register IRQ routine
>> - To start acquisition, write a value to the config register
>> - On interrupt, read data register, push it into queue
>> - To stop, write another value to config.
>>
>> Would it be useful to provide this as a generic driver in Linux, so that you
>> can provide the config register and data register offsets and values in the
>> devicetree? That would make it work on a lot of similar project without
>> everyone coding the same thing.
> 
> Hi Mike,
> 
> I've brought in the devicetree bindings maintainers and list as well on this
> discussion because in many ways it's more about the strategy on bindings
> than the actual driver...  Also Linus Walleij for any experience handing
> the gpio-mmio driver which strikes me as fairly similar in aim.
> 
> To the driver question, yes absolutely this would be a good thing to have.
> The majority of ADCs out there are irritatingly more complex than what
> you have.  Everyone seems to feel obliged to put in their own 'special
> sauce'.
> 
> As for bindings, there are two options, either we
> 
> 1) As you suggest push all differences between different parts into the
> bindings.  Note we'll still need a compatible list in the driver.
> So this is something like gpio-mmio.c
> 
> 2) Do something similar to the irq generic_chip (CONFIG_GENERIC_IRQ)
> infrastructure and actually have very slim wrapper drivers around a
> library.   That allows those elements that are constrained by the
> hardware to not need specifying in device tree bindings.
> 
>>
>> And a related question: Is there a driver that's close to this?
> I can't immediately think of one, but it's possible that one
> of the SoC ADC drivers is simple enough to describe this way.
> 
> So Mark / Rob.  You guys have experience with how the bindings
> worked out for each approach.  Which do you prefer?
> Either approach is fine with me.
> 
> Mike, perhaps you have an initial cut of some bindings that might
> help the discussion to move along?

I was thinking along these lines:

... {
    compatible = "iio-adc-generic-mmio";
    reg = <0x43210000 4096>;
    interrupt = <&gic 42 0>;
    iio,start-sequence = <0x0 0x3> <0x4 0xff>;
    iio,stop-sequence = <0x0 0x0>;
    iio,data-register = <0x10>;
};

Required:
   compatible: must be "iio-adc-generic-mmio";
   reg: Memory address and range of IP platform device
   interrupt: Data-ready interrupt binding

Optional:
   iio,data-register: Offset where to read data. Defaults to <0>.
   iio,start-sequence: Array of <offset value> pairs. On start of acquisition
                       the driver writes each value to each offset in order.
   iio,stop-sequence: Array of <offset value> pairs to stop acquisition.


And we could make up more properties as we go, like the number of valid data 
bits and shift, interrupt rate and fifo buffer properties...

We could add normal clock and power supply bindings as well, so it would 
enable them on starting data acquisition.


For now the bad news is that the project was put on hold, so I'm not allowed 
to spend serious development time now.


Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans-yhtFebqMsb9it5bFGTN0CAC/G2K4zDHf@public.gmane.org
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

Re: "Generic" MMIO ADC

[Jonathan Cameron]

On Mon, 3 Jul 2017 14:37:35 +0200
Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org> wrote:

> On 30-06-17 15:53, Jonathan Cameron wrote:
> > On Fri, 30 Jun 2017 10:44:42 +0200
> > Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org> wrote:
> >   
> >> I'm writing an IIO driver for a custom IP, which is pretty simple:
> >>
> >> - Map the registers to memory and register IRQ routine
> >> - To start acquisition, write a value to the config register
> >> - On interrupt, read data register, push it into queue
> >> - To stop, write another value to config.
> >>
> >> Would it be useful to provide this as a generic driver in Linux, so that you
> >> can provide the config register and data register offsets and values in the
> >> devicetree? That would make it work on a lot of similar project without
> >> everyone coding the same thing.  
> > 
> > Hi Mike,
> > 
> > I've brought in the devicetree bindings maintainers and list as well on this
> > discussion because in many ways it's more about the strategy on bindings
> > than the actual driver...  Also Linus Walleij for any experience handing
> > the gpio-mmio driver which strikes me as fairly similar in aim.
> > 
> > To the driver question, yes absolutely this would be a good thing to have.
> > The majority of ADCs out there are irritatingly more complex than what
> > you have.  Everyone seems to feel obliged to put in their own 'special
> > sauce'.
> > 
> > As for bindings, there are two options, either we
> > 
> > 1) As you suggest push all differences between different parts into the
> > bindings.  Note we'll still need a compatible list in the driver.
> > So this is something like gpio-mmio.c
> > 
> > 2) Do something similar to the irq generic_chip (CONFIG_GENERIC_IRQ)
> > infrastructure and actually have very slim wrapper drivers around a
> > library.   That allows those elements that are constrained by the
> > hardware to not need specifying in device tree bindings.
> >   
> >>
> >> And a related question: Is there a driver that's close to this?  
> > I can't immediately think of one, but it's possible that one
> > of the SoC ADC drivers is simple enough to describe this way.
> > 
> > So Mark / Rob.  You guys have experience with how the bindings
> > worked out for each approach.  Which do you prefer?
> > Either approach is fine with me.
> > 
> > Mike, perhaps you have an initial cut of some bindings that might
> > help the discussion to move along?  
> 
> I was thinking along these lines:
> 
> ... {
>     compatible = "iio-adc-generic-mmio";
>     reg = <0x43210000 4096>;
>     interrupt = <&gic 42 0>;
>     iio,start-sequence = <0x0 0x3> <0x4 0xff>;
>     iio,stop-sequence = <0x0 0x0>;
>     iio,data-register = <0x10>;
> };
> 
> Required:
>    compatible: must be "iio-adc-generic-mmio";
>    reg: Memory address and range of IP platform device
>    interrupt: Data-ready interrupt binding
> 
> Optional:
>    iio,data-register: Offset where to read data. Defaults to <0>.
>    iio,start-sequence: Array of <offset value> pairs. On start of acquisition
>                        the driver writes each value to each offset in order.
>    iio,stop-sequence: Array of <offset value> pairs to stop acquisition.
> 
> 
> And we could make up more properties as we go, like the number of valid data 
> bits and shift, interrupt rate and fifo buffer properties...
> 
> We could add normal clock and power supply bindings as well, so it would 
> enable them on starting data acquisition.
> 
> 
> For now the bad news is that the project was put on hold, so I'm not allowed 
> to spend serious development time now.
> 
Ah well, such is life unfortunately!  If it resurfaces then give this thread
a bump and we'll get back to thinking about it some more.

Obviously don't progress from other directions though if anyone else wants to
pick this up and run with it.

Jonathan
> 
> Kind regards,
> 
> Mike Looijmans
> System Expert
> 
> TOPIC Products
> Materiaalweg 4, NL-5681 RJ Best
> Postbus 440, NL-5680 AK Best
> Telefoon: +31 (0) 499 33 69 79
> E-mail: mike.looijmans-yhtFebqMsb9it5bFGTN0CAC/G2K4zDHf@public.gmane.org
> Website: www.topicproducts.com
> 
> Please consider the environment before printing this e-mail
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: "Generic" MMIO ADC

[Linus Walleij]

On Mon, Jul 3, 2017 at 2:37 PM, Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org> wrote:

> I was thinking along these lines:
>
> ... {
>    compatible = "iio-adc-generic-mmio";
>    reg = <0x43210000 4096>;
>    interrupt = <&gic 42 0>;
>    iio,start-sequence = <0x0 0x3> <0x4 0xff>;
>    iio,stop-sequence = <0x0 0x0>;

Nope. We are not going to invent scripting languages inside device tree.
No sequences please. This is the start of what we call a jam table.

We had this discussion a long time ago when starting to convert all the ARM
platforms to device tree: sequences should be expressed in code, not in
the device tree. Because next thing that happens is that you want to encode
a for() loop or if() or while().

The reason is that device tree is already part of open firmware, which already
has a scripting language: FCode (based on forth). This is then used to create
BIOS routines along the lines of what you're suggesting. So if you insist on
writing a BIOS, by all means write a BIOS, but not inside DT, use FCode
and figure out how to bridge that.

But I think it's easier to just write a normal driver really.

Compare to ACPI, which has AML, for the very same purpose, and DSDT
for describing the hardware.

Since ACPI is now starting to share parts of the DT bindings), things would
get very weird if we did things like this. Should they also implement a
compatible interpreter, rather than call their (very much in-use) AML
snippets?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: "Generic" MMIO ADC

[Jonathan Cameron]

On Thu, 6 Jul 2017 23:37:04 +0200
Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

> On Mon, Jul 3, 2017 at 2:37 PM, Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org> wrote:
> 
> > I was thinking along these lines:
> >
> > ... {
> >    compatible = "iio-adc-generic-mmio";
> >    reg = <0x43210000 4096>;
> >    interrupt = <&gic 42 0>;
> >    iio,start-sequence = <0x0 0x3> <0x4 0xff>;
> >    iio,stop-sequence = <0x0 0x0>;  
> 
> Nope. We are not going to invent scripting languages inside device tree.
> No sequences please. This is the start of what we call a jam table.
> 
> We had this discussion a long time ago when starting to convert all the ARM
> platforms to device tree: sequences should be expressed in code, not in
> the device tree. Because next thing that happens is that you want to encode
> a for() loop or if() or while().
> 
> The reason is that device tree is already part of open firmware, which already
> has a scripting language: FCode (based on forth). This is then used to create
> BIOS routines along the lines of what you're suggesting. So if you insist on
> writing a BIOS, by all means write a BIOS, but not inside DT, use FCode
> and figure out how to bridge that.
> 
> But I think it's easier to just write a normal driver really.
> 
> Compare to ACPI, which has AML, for the very same purpose, and DSDT
> for describing the hardware.
> 
> Since ACPI is now starting to share parts of the DT bindings), things would
> get very weird if we did things like this. Should they also implement a
> compatible interpreter, rather than call their (very much in-use) AML
> snippets?
Ah that was matching up with my gut feeling on the matter.  Thanks for the detailed
response Linus.   Anyhow, doesn't mean the concept of such a driver is bad, merely that
the tables should form part of the driver code (much like any other driver that supports
a number of different parts) and be selected by the compatible string.

Jonathan
> 
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: "Generic" MMIO ADC

[Linus Walleij]

On Fri, Jul 7, 2017 at 9:44 AM, Jonathan Cameron
<Jonathan.Cameron-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:

>  Anyhow, doesn't mean the concept of such a driver is bad, merely that
> the tables should form part of the driver code (much like any other driver that supports
> a number of different parts) and be selected by the compatible string.

That works.

drivers/gpu/drm/panel/panel-simple.c

comes to mind.

Yours,
Linus Walleij