From: Jonathan Cameron <jic23@kernel.org>
To: Olivier MOYSAN <olivier.moysan@foss.st.com>
Cc: <robh+dt@kernel.org>, <mark.rutland@arm.com>, <knaack.h@gmx.de>,
<lars@metafoo.de>, <devicetree@vger.kernel.org>,
<linux-iio@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<pmeerw@pmeerw.net>, <linux-stm32@st-md-mailman.stormreply.com>,
<linux-arm-kernel@lists.infradead.org>,
Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>,
Fabrice GASNIER <fabrice.gasnier@foss.st.com>
Subject: Re: [PATCH 4/4] iio: adc: stm32-dfsdm: add scale and offset support
Date: Thu, 30 Sep 2021 16:21:19 +0100 [thread overview]
Message-ID: <20210930162119.6b8c8455@jic23-huawei> (raw)
In-Reply-To: <a7467649-e949-9d1d-eed6-93830bf83bb4@foss.st.com>
On Wed, 29 Sep 2021 18:44:30 +0200
Olivier MOYSAN <olivier.moysan@foss.st.com> wrote:
> Hi Jonathan,
>
> >>>>
> >>>> If 'backend' option turns out to be the most appropriated to match DFSDM
> >>>> constraints, I can prepare some patches to support it.
> >>>> Would you have some guidelines or requirements for the implementation of
> >>>> such feature, in this case ?
> >>>
> >>> Closest example is that rcar-gyroadc but in this case we'd want to define
> >>> something standard to support the modulators so that if we have other filters
> >>> in future we can reuse them.
> >>>
> >>> That means implementing them as child devices of the filter - probably put
> >>> the on the IIO bus, but as different device type. Take a look at how
> >>> triggers are done in industrialio-trigger.c
> >>> You need struct device_type sd_modulator
> >>> and a suitable device struct (burred in an iio_sd_modulator struct probably).
> >>>
> >>> Also needed would be a bunch of standard callbacks to allow you to query things
> >>> like scaling. Keep that interface simple. Until we have a lot of modulator
> >>> drivers it will be hard to know exactly what is needed. Also whilst we don't
> >>> have many it is easy to modify the interface.
> >>>
> >>> Then have your filter driver walk it's own dt children and instantiate
> >>> appropriate new elements and register them on the iio_bus. They will have
> >>> the filter as their parent.
> >>>
> >>> There are various examples of this sort of thing in tree.
> >>> If you want a good one, drivers/cxl does a lot of this sort magic to manage
> >>> a fairly complex graph of devices including some nice registration stuff to
> >>> cause the correct device drivers to load automatically.
> >>>
> >>> Hmm. Thinking more on this, there is an ordering issue for driver load.
> >>> Instead of making the modulator nodes children of the modulator, you may need
> >>> to give them their own existence and use a phandle to reference them.
> >>> That will let you defer probe in the filter driver until those
> >>> modulator drivers are ready.
> >>>
> >>> This isn't going to be particularly simple, so you may want to have a look
> >>> at how various other subsystems do similar things and mock up the dependencies
> >>> to make sure you have something that doesn't end up with a loop of dependencies.
> >>> In some ways the modulators are on a bus below the filter, but the filter driver
> >>> needs them to be in place to do the rest.
> >>> You may end up with some sort of delayed load.
> >>> 1. Initial filter driver load + parsing of the modulator dt children (if done that way).
> >>> 2. Filter driver goes to sleep until...
> >>> 3. Modulator drivers call something on the filter driver to say they are ready.
> >>> 4. Filter driver finishes loading and create the IIO device etc.
> >>> You'll need some reference counting etc in there to make removal safe etc but it
> >>> shouldn't be 'too bad'.
> >>>
> >>> Good luck!
> >>>
> >>> Jonathan
> >>>
> I'am on the way to prototype this proposal for DFSDM.
> Looking at your advices, I see that the current topolgy based on
> hardware consumer, already meets most of the requirements.
>
> - SD modulators are described in DT with their own nodes and are
> referred in DFSDM nodes through their phandle.
> - Dependencies at probe are managed (defer probe through
> devm_iio_hw_consumer_alloc())
> - SD modulator scaling is retrieved through iio_read_channel_scale() ABI.
>
> So, it seems that the current implementation is not so far from this
> solution.
> It remains the unwanted sysfs interface for SD modulator. Or more than
> that, if I missed something ?
> Instead of introducing a new device type for SD modulator, could the
> mode field be used to identify devices not requesting an IIO sysfs ?
> (A dedicated mode may be used to skip sysfs register in device registration)
> Otherwise let's go for a new type.
I'd rather see them as a new device type than overload the IIO device.
We want to be able to control what can 'connect' to the DFSDM afterall
and device type is a convenient route to doing this.
Obviously if there is infrastructure that can be factored out and used
for both this and a normal IIO device we can do that to save on duplication.
Thanks,
Jonathan
>
> Regards
> Olivier
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
prev parent reply other threads:[~2021-09-30 15:19 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-04 10:10 [PATCH 0/4] iio: adc: stm32-dfsdm: add scale and offset support Olivier Moysan
2020-02-04 10:10 ` [PATCH 1/4] dt-bindings: iio: adc: sd modulator: add vref support Olivier Moysan
2020-02-06 19:08 ` Rob Herring
2020-02-08 16:04 ` Jonathan Cameron
2020-02-11 15:24 ` Olivier MOYSAN
2020-02-04 10:10 ` [PATCH 2/4] iio: adc: sd modulator: add scale support Olivier Moysan
2020-02-08 16:03 ` Jonathan Cameron
2020-02-11 15:22 ` Olivier MOYSAN
2020-02-04 10:10 ` [PATCH 3/4] iio: adc: stm32-dfsdm: use resolution define Olivier Moysan
2020-02-04 10:10 ` [PATCH 4/4] iio: adc: stm32-dfsdm: add scale and offset support Olivier Moysan
2020-02-08 16:18 ` Jonathan Cameron
2020-02-11 15:19 ` Olivier MOYSAN
2020-02-14 13:11 ` Jonathan Cameron
2020-02-14 14:49 ` Olivier MOYSAN
2020-02-14 15:10 ` Jonathan Cameron
[not found] ` <AM9PR10MB43558CEB8DAE7F373E9E7A5DF9D69@AM9PR10MB4355.EURPRD10.PROD.OUTLOOK.COM>
2021-09-10 15:56 ` Olivier MOYSAN
2021-09-12 17:26 ` Jonathan Cameron
2021-09-14 15:43 ` Olivier MOYSAN
2021-09-19 18:14 ` Jonathan Cameron
2021-09-24 13:14 ` Olivier MOYSAN
2021-09-26 14:56 ` Jonathan Cameron
2021-09-29 16:44 ` Olivier MOYSAN
2021-09-30 15:21 ` Jonathan Cameron [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210930162119.6b8c8455@jic23-huawei \
--to=jic23@kernel.org \
--cc=arnaud.pouliquen@foss.st.com \
--cc=devicetree@vger.kernel.org \
--cc=fabrice.gasnier@foss.st.com \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=mark.rutland@arm.com \
--cc=olivier.moysan@foss.st.com \
--cc=pmeerw@pmeerw.net \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox