On Wed, 2023-12-20 at 08:17 -0600, Rob Herring wrote: > On Sun, Dec 17, 2023 at 02:04:12PM +0000, Jonathan Cameron wrote: > > On Fri, 15 Dec 2023 16:18:39 +0100 > > Nuno Sá <noname.nuno@xxxxxxxxx> wrote: > > > > > On Thu, 2023-12-14 at 11:03 -0600, Rob Herring wrote: > > > > On Thu, Dec 14, 2023 at 05:05:20PM +0100, Nuno Sá wrote: > > > > > On Thu, 2023-12-14 at 08:16 -0600, Rob Herring wrote: > > > > > > On Wed, Dec 13, 2023 at 04:02:31PM +0100, Nuno Sa wrote: > > > > > > > v1: > > > > > > > > > > > > > > https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f517 > > > > > > > 5273b81dbfe40b7f0daffcdc67d6cb8ff > > > > > > > > > > > > > > v2: > > > > > > > > > > > > > > https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@xxxxxxxxx > > > > > > > m > > > > > > > > > > > > > > Changes in v3: > > > > > > > - Patch 1: > > > > > > > * Use proposed generic schema [1]. Also make it a required > > > > > > > property; > > > > > > > * Improved the commit message. > > > > > > > - Patch 2: > > > > > > > * Improved commit message. > > > > > > > - Patch 4: > > > > > > > * Namespace all IIO DMAENGINE buffer exports; > > > > > > > * Removed unrelated new line removal change. > > > > > > > - Patch 5: > > > > > > > * Namespace all IIO backend exports. > > > > > > > - Patch 6: > > > > > > > * Set backend.h in alphabetical order; > > > > > > > * Import IIO backend namespace. > > > > > > > - Patch 7: > > > > > > > * Don't depend on OF in kbuild anymore; > > > > > > > * Import IIO backend namespace. > > > > > > > > > > > > > > For the bindings patches, I tried not to enter into much details > > > > > > > about > > > > > > > the IIO framework as I think specifics of the implementation don't > > > > > > > care > > > > > > > from the bindings perspective. Hopefully the commit messages are > > > > > > > good > > > > > > > enough. > > > > > > > > > > > > > > I'm also aware that patch 1 is not backward compatible but we are > > > > > > > anyways doing it on the driver side (and on the driver the > > > > > > > property is > > > > > > > indeed required). Anyways, just let me know if making the property > > > > > > > required is not acceptable (I'm fairly confident no one was using > > > > > > > the > > > > > > > upstream version of the driver and so validating devicetrees for > > > > > > > it). > > > > > > > > > > > > > > Keeping the block diagram in v3's cover so we don't have to follow > > > > > > > links > > > > > > > to check the one of the typicals setups. > > > > > > > > > > > > > > ----------------------- > > > > > > > ----------- > > > > > > > ---- > > > > > > > ----------------- > > > > > > > ------------------ | ----------- - > > > > > > > ----------- > > > > > > > ------- FPGA | > > > > > > > | ADC |------------------------| | AXI ADC |---------| > > > > > > > DMA CORE > > > > > > > > ---- > > > > > > > --| RAM | | > > > > > > > | (Frontend/IIO) | Serial Data (eg: LVDS) | |(backend)|--------- > > > > > > > | > > > > > > > > ---- > > > > > > > --| | | > > > > > > > | |------------------------| ----------- - > > > > > > > ----------- > > > > > > > ------- | > > > > > > > ------------------ ----------------------- > > > > > > > ----------- > > > > > > > ---- > > > > > > > ----------------- > > > > > > > > > > > > Why doesn't axi-adc just have an io-channels property to adc? It's > > > > > > the > > > > > > opposite direction for the link, but it seems more logical to me > > > > > > that > > > > > > axi-adc depends on adc rather than the other way around. > > > > > > > > > > > > > > > > We are not interested on a single channel but on the complete > > > > > device. >From the > > > > > axi- > > > > > adc point of view, there's not much we could do with the adc channel. > > > > > I mean, > > > > > maybe > > > > > we could still do something but it would be far from ideal (see > > > > > below). > > > > > > > > Will this hold up for everyone? Could there be a backend device that > > > > handles multiple ADCs? IOW, do you need #io-backend-cells? It's somewhat > > > > better if we add that up front rather than later and have to treat > > > > missing as 0 cells. It is also the only way to generically identify the > > > > providers (well, there's also 'foo-controller' properties, but we've > > > > gone away from those). > > > > > > > > > > For the axi-adc backend, it's very unlikely. The way the core connects to > > > the > > > converters is through a serial data interface (LVDS, CMOS or JESD in ADI > > > usecases). > > > This interface is not really a bus so it's kind of a 1:1 connection. Now, > > > in more > > > complicated devices (like highly integrated RF transceivers) what we have > > > is that we > > > have multiple of these cores (one per RX/TX port) connected to the > > > frontend. So, > > > effectively 1 frontend could have multiple backends. So, yes, your first > > > "doubts" are > > > not that "crazy" as this is also not the "typical" provider - consumer > > > relationship. > > > However, for all of what I've said in the previous email, even in these > > > cases, > > > thinking in these cores as the provider, makes things much more easier to > > > handle. > > > > > > However, the above is just ADI usecases. In theory, yes, it can be very > > > possible for > > > another backend other than axi-adc to have multiple frontends connected so > > > I guess we > > > can make #io-backend-cells already available in the schema. > > > > I'd like ultimately to consider making this work for new instances of > > dfsdm devices (separately front end ADC that spits out a modulated signal > > that a host > > controller then turns into something useful). In those cases we might well > > see a mix > > of front ends connected to one backend (at least in theory - not sure anyone > > would > > build such thing outside of an eval board). > > > > Adding the flexibility from the start would be sensible. So I agree with Rob > > here. > > > > > > > > For the axi-adc bindings this would be 'const: 0', right? > > > > > > > > > > > > The opposite direction is exactly what we had (look at patch 2) just > > > > > with another > > > > > custom property. The problem with that is that we would then need a > > > > > bidirectional > > > > > link (we would need to callback into the provider and the provider > > > > > would need to > > > > > access the consumer) between consumers and providers and that would be > > > > > far from > > > > > optimal. The bidirectional link would exist because if we want to > > > > > support > > > > > fundamental > > > > > things like LVDS/CMOS interface tuning we need to set/get settings > > > > > from the axi- > > > > > adc > > > > > core. And as Jonathan suggested, the real data is captured or sent on > > > > > the > > > > > converters > > > > > (ADC or DACs) and that is why we have the IIO device/interface in > > > > > there and why > > > > > we > > > > > call them "frontends". In ADI usecases, backends are these FPGA cores > > > > > providing > > > > > "services" to the "real" high speed converters. To put it in another > > > > > way, the > > > > > real > > > > > converter is the one who knows how to use the axi-adc core and not the > > > > > other way > > > > > around. That's also one of the reasons why it would be way more > > > > > difficult to > > > > > handle > > > > > things with the opposite link. That's how we have done it so far and > > > > > the mess we > > > > > have > > > > > out of tree is massive :sweat_smile: We ended up doing raw writes and > > > > > reads on > > > > > the > > > > > axi-adc MMIO registers from the converter driver just because we had > > > > > to configure > > > > > or > > > > > get something from the axi-adc device but the link was backwards. > > > > > > > > The direction (for the binding) doesn't really matter. It doesn't > > > > dictate the direction in the OS. In the ad9467 driver, you can search > > > > the DT for 'adi,adc-dev' and find the node which matches your node's > > > > phandle. It's not exactly efficient, but you are doing it once. It would > > > > also prevent the DT ABI break you are introducing. > > > > > > > > > > Hmm, I think I see your idea. So you mean something like > > > devm_iio_backend_get_optional() and if not present, then we would look for > > > nodes > > > having the 'adi,adc-dev' property and look for the one pointing at us... > > > Then, we > > > would need another API in the backend to look for registered backends > > > matching that > > > fwnode. Right? > > > > > > I mean, I guess this could work but we would already have to start a fresh > > > framework > > > with API's that are not really meant to be used anymore other than the > > > ad9467 driver > > > (not devm_iio_backend_get_optional() because sooner or later I think we'll > > > need that > > > one). We are already breaking ABI in the driver and I'm still fairly > > > confident no one > > > is really using the upstream driver because it's lacking support for > > > devices and > > > important features (not even in ADI fork we're using it). > > > > > > Anyways, if you still insist on having something like this (and feel more > > > comfortable > > > in not breaking DT ABI), I can see how this would look like in the next > > > version... > > > > See how it looks. If it means removing the need to convince Rob then it's > > probably easier > > to write the code than try and talk him around ;) Can happily stick a bit > > deprecated > > note next to the binding and the code. > > I only point out ABI breaks and require they are justified in the commit > message (basically stating what you state above). Otherwise, I don't > care if I don't use the platform. > Good to know. I already worked on a new series only with your suggestion so now I'll send it anyways :). In the end it will be up to me and Jonathan to decide if we want to add an API only meant to be used for this. Thanks! - Nuno Sá > > > > >
