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:
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.
Let me know if there's something you don't agree or if there's any concern on
your side.
- Nuno Sá
> > >
