Hi Nuno,
On 11/14/23 10:03, Nuno Sá wrote:
On Mon, 2023-11-13 at 18:20 +0100, Olivier MOYSAN wrote:
Ho Olivier,
Hi Nuno, Jonathan,
On 9/4/23 17:31, Jonathan Cameron wrote:
On Mon, 04 Sep 2023 16:14:17 +0200
Nuno Sá <noname.nuno@xxxxxxxxx> wrote:
On Sun, 2023-09-03 at 11:56 +0100, Jonathan Cameron wrote:
On Thu, 31 Aug 2023 11:32:54 +0200
Nuno Sá <noname.nuno@xxxxxxxxx> wrote:
On Wed, 2023-08-30 at 18:02 +0100, Jonathan Cameron wrote:
On Fri, 4 Aug 2023 16:53:39 +0200
Nuno Sa <nuno.sa@xxxxxxxxxx> wrote:
Signed-off-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
Hi Nuno,
Hi Jonathan,
Thanks for the initial review...
One general comment is that you could have stripped this back a fair
bit
for ease of understanding. At this stage we don't care about things
like debug or control of test patterns. Bring those in as extra
patches.
Agreed... As I mentioned (I think) in the cover, I made the RFC bigger
than
needed to
kind of showcase how we can properly configure the hdl core to support
things
(interface calibration) that were very hard to do with the current
implementation.
I'll make sure to add the minimum needed API to accommodate what we
have
right now.
I haven't fully gotten my head around the ordering constraints on
removal.
Are there other users of the component framework that have similar
problems?
My understanding on the component API is that one should do all the
tear
down in the
.unbind() callback. As usual, I can see some drivers not really doing
that.
Also, I don't yet understand how a multiple front end, single
backend
setup
would work. Or indeed single front end, multiple backend... Maybe
we
don't
need those cases, but if we want this to be useful beyond adi-axi we
probably at least want an outline of how they work.
Indeed we can have multiple (and we have it out of tree) backends on
one
frontend.
Think on an ADC/DAC with fairly complex data path with more than one
channel/interface (CMOS, LVDS, etc). Typically, in those case, each of
the
interface
will be connected to an instance of the hdl core (the backend).
That might work out for your case, but not the stm32 one where I think
we can
end
up with interleaved data from two front ends in the same buffer...
Not sure I'm following this one. But wouldn't that be something specific
for
each system (through devicetree)? I haven't tried but I think the same
backend
could be used in different frontend devices (using the component API).
That is
not really a usecase for me but definitely something that could be
supported (if
we need to start doing things like keep enable/disable counters and so on)
if it
is a usecase for stm32.
If we are going to support both usecases, we just need to figure out what
composite
devices with N-M backend - frontend look like and make sure that doesn't
cause problems. I'd expect the separation between backend instances might
reflect data storage on capture but then again that might end up like the
many
IIO devices for many buffers mess we had before the multiple buffer support
was added.
The stm32 dfsdm interleaved use case is not a problem as it is possible
to associate several backends to a frontend.
I did some experiments based on converter framework, and did not
identified blocking points regarding dfsdm use cases.
Some limitations where discussed in [1], about generic bindings support.
The preferred solution was to extend converter_frontend_add_matches() to
parse also child nodes. I have added converters_get_from_fwnode() API
and adapted converter_frontend_add_matches() to test this approach.
With this changes and an additional api to support channel attributes
read, the framework fulfills all the needs for dfsdm.
So, I feel comfortable to drop my previous "backend framework" proposal,
and move to the current proposal.
If we go further in converter framework adaption, I will push these updates.
I hope you didn't had too much trouble with those patches. The reason I'm saying
this is because, after some thought, I'm strongly considering in moving to
normal OF/ACPI lookup. 3 mains reasons for it:
No problem, this was an opportunity to discover the component framework.
The converter API is quite simple to use once you understand the basics
of component concepts. It does the job for stm dfsdm, but I agree that
for the long-term, a component-based solution is probably less scalable.
1) That "hack/rule" for a driver to provide a .remove() callback (in order to
devm_*) is really non obvious and might even be prune to subtle bugs (that I'm
not seeing now :)). But my main argument is that it can become hard to maintain
it (depending on how much people starts to use the framework).
2) From the discussion we had about the limitations you pointed in your link, I
started to realize that it might get harder to scale the framework. Yes, we
found a fairly easy way of doing it but still took more code to do it when
compared to a typical lookup.
3) This is the most important together with 1). You mentioned something like
cascaded designs and I just found an usercase in ADI out of tree drivers. We
have a design where we have something like:
------------------------------------------
| FPGA |
-------------- | ------------- ------------------- |
|DAC Frontend| -> | |DAC Backend| -> |DAC Interpolation| |
-------------- | ------------- ------------------- |
| |
------------------------------------------
In the above design we kind of have a cascaded thing where the DAC backend is
both a frontend and a backend and the Intrerpolation stuff just serves as
backend to the DAC core. So, ideally the DAC frontend should not have to know a
thing about the interpolation... I realized that having this with the component
framework is far from straight because we would need two components/aggregate
devices to accomplish this (DAC Front + DAC Back) and (DAC Back + DAC Interp)
and I think we would need some extra care regarding one of the components going
away (not sure though). One way to make it simple would be to not "respect" the
HW configuration and just have one aggregate device with 1 Frontend + 2 Backends
and so the frontend would need to "know" about the interpolation core. Again, I
think that with OF/ACPI this setup with be fairly straight to get and "respect".
Anyways, all the above makes me feel that component might not be the best choice
(even though I was eager to use it :))
I'll also get to work on this again and I might just use an industrialio-
backend.c file in the base dir as you had in your RFC. From a quick look on your
series, I'm no sure how much code I will reuse but we can see later if a Co-
authored-by tag makes sense or not.
The APIs in my RFC were kept minimalistic to assess the proposal's
relevance. As you pointed out, there is room for improvement,
particularly with regards to object releasing. If you are on the way to
rework your framework, feel free to re-use some parts of this RFC.
At the end, what really matters is to converge towards a solution :)
Regards
Olivier
Let me know if there's something you don't agree or if there's any concern on
your side.
- Nuno Sá
