| Message ID | 20231213-dev-iio-backend-v3-0-bb9f12a5c6dc@analog.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | iio: add new backend framework | expand |
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/#m222f5175273b81dbfe40b7f0daffcdc67d6cb8ff > > v2: > https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@analog.com > > 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. And if there's another consumer in the chain, then a node could certainly be both an io-channels consumer and producer. The architecture of the drivers seems odd to me. It looks similar to making a phy driver handle all the state and protocol with the host controller being a backend. Rob
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@analog.com > > > > 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). 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. > And if there's another consumer in the chain, then a node could > certainly be both an io-channels consumer and producer. > This should also be possible with this architecture. A node can be both a backend (provider) and a consumer and we have an out of tree design that fits this (that I surely want to upstream after the foundations are done). > The architecture of the drivers seems odd to me. It looks similar to > making a phy driver handle all the state and protocol with the host > controller being a backend. In this case, it's not really a controller. It's more like an extension of the device because we need a way to handle the high sample rates this ADCs can do. Then we can also do test tones with the backend which is useful for interface tuning (as mentioned above). To give you a bit more context, I'm adding the generic property because we will have more users for it (from ADI - the next should be the axi-dac core) but STM is also interested in this (likely the next user). Hope this makes it a bit more clear... - Nuno Sá
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@analog.com > > > > > > 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). > 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. > > And if there's another consumer in the chain, then a node could > > certainly be both an io-channels consumer and producer. > > > > This should also be possible with this architecture. A node can be both a backend > (provider) and a consumer and we have an out of tree design that fits this (that I > surely want to upstream after the foundations are done). > > > The architecture of the drivers seems odd to me. It looks similar to > > making a phy driver handle all the state and protocol with the host > > controller being a backend. > > In this case, it's not really a controller. It's more like an extension of the device > because we need a way to handle the high sample rates this ADCs can do. Then we can > also do test tones with the backend which is useful for interface tuning (as > mentioned above). > > To give you a bit more context, I'm adding the generic property because we will have > more users for it (from ADI - the next should be the axi-dac core) but STM is also > interested in this (likely the next user). > > Hope this makes it a bit more clear... Yes, thanks. I generally ask for 2 users on new common bindings. I've accepted too many only to have the 2nd user come in a month later and need additions. An ack on the binding from the STM folks would be nice here. And Jonathan too. Rob
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@analog.co > > > > 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. 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... > > > > And if there's another consumer in the chain, then a node could > > > certainly be both an io-channels consumer and producer. > > > > > > > This should also be possible with this architecture. A node can be both a backend > > (provider) and a consumer and we have an out of tree design that fits this (that > > I > > surely want to upstream after the foundations are done). > > > > > The architecture of the drivers seems odd to me. It looks similar to > > > making a phy driver handle all the state and protocol with the host > > > controller being a backend. > > > > In this case, it's not really a controller. It's more like an extension of the > > device > > because we need a way to handle the high sample rates this ADCs can do. Then we > > can > > also do test tones with the backend which is useful for interface tuning (as > > mentioned above). > > > > To give you a bit more context, I'm adding the generic property because we will > > have > > more users for it (from ADI - the next should be the axi-dac core) but STM is > > also > > interested in this (likely the next user). > > > > Hope this makes it a bit more clear... > > Yes, thanks. > > I generally ask for 2 users on new common bindings. I've accepted too > many only to have the 2nd user come in a month later and need additions. > An ack on the binding from the STM folks would be nice here. And > Jonathan too. > Olivier, could we get an ack on the bindings patch? Do you also have any idea about how long it would take for you to send patches so we have another user of the schema? On my side, it might very well take a month or so (given we have holidays nearby) as the axi-dac core is more complex than the axi-adc. Bah it might take less than a month to have the first version of it posted in the lists but I can't make any promises. - Nuno Sá
On Fri, 15 Dec 2023 16:18:39 +0100 Nuno Sá <noname.nuno@gmail.com> 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@analog.co > > > > > 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. > > > > > > > And if there's another consumer in the chain, then a node could > > > > certainly be both an io-channels consumer and producer. > > > > > > > > > > This should also be possible with this architecture. A node can be both a backend > > > (provider) and a consumer and we have an out of tree design that fits this (that > > > I > > > surely want to upstream after the foundations are done). > > > > > > > The architecture of the drivers seems odd to me. It looks similar to > > > > making a phy driver handle all the state and protocol with the host > > > > controller being a backend. > > > > > > In this case, it's not really a controller. It's more like an extension of the > > > device > > > because we need a way to handle the high sample rates this ADCs can do. Then we > > > can > > > also do test tones with the backend which is useful for interface tuning (as > > > mentioned above). > > > > > > To give you a bit more context, I'm adding the generic property because we will > > > have > > > more users for it (from ADI - the next should be the axi-dac core) but STM is > > > also > > > interested in this (likely the next user). > > > > > > Hope this makes it a bit more clear... > > > > Yes, thanks. > > > > I generally ask for 2 users on new common bindings. I've accepted too > > many only to have the 2nd user come in a month later and need additions. > > An ack on the binding from the STM folks would be nice here. And > > Jonathan too. > > > > Olivier, could we get an ack on the bindings patch? Do you also have any idea about > how long it would take for you to send patches so we have another user of the schema? > > On my side, it might very well take a month or so (given we have holidays nearby) as > the axi-dac core is more complex than the axi-adc. Bah it might take less than a > month to have the first version of it posted in the lists but I can't make any > promises. For the driver side of things I'd like at least 2, preferably 3 users before merging. We have more flexibility to rework things as any issues will probably be internal interfaces, but I'd rather wait if we are going to have 3 users within another month or 2. Jonathan > > - Nuno Sá >
On Sun, 2023-12-17 at 14:04 +0000, Jonathan Cameron wrote: > On Fri, 15 Dec 2023 16:18:39 +0100 > Nuno Sá <noname.nuno@gmail.com> 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@analog.co > > > > > > 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. > Agreed... > > > > 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. > Will do... > > > > > > > > > > And if there's another consumer in the chain, then a node could > > > > > certainly be both an io-channels consumer and producer. > > > > > > > > > > > > > This should also be possible with this architecture. A node can be both > > > > a backend > > > > (provider) and a consumer and we have an out of tree design that fits > > > > this (that > > > > I > > > > surely want to upstream after the foundations are done). > > > > > > > > > The architecture of the drivers seems odd to me. It looks similar to > > > > > making a phy driver handle all the state and protocol with the host > > > > > controller being a backend. > > > > > > > > In this case, it's not really a controller. It's more like an extension > > > > of the > > > > device > > > > because we need a way to handle the high sample rates this ADCs can do. > > > > Then we > > > > can > > > > also do test tones with the backend which is useful for interface tuning > > > > (as > > > > mentioned above). > > > > > > > > To give you a bit more context, I'm adding the generic property because > > > > we will > > > > have > > > > more users for it (from ADI - the next should be the axi-dac core) but > > > > STM is > > > > also > > > > interested in this (likely the next user). > > > > > > > > Hope this makes it a bit more clear... > > > > > > Yes, thanks. > > > > > > I generally ask for 2 users on new common bindings. I've accepted too > > > many only to have the 2nd user come in a month later and need additions. > > > An ack on the binding from the STM folks would be nice here. And > > > Jonathan too. > > > > > > > Olivier, could we get an ack on the bindings patch? Do you also have any > > idea about > > how long it would take for you to send patches so we have another user of > > the schema? > > > > On my side, it might very well take a month or so (given we have holidays > > nearby) as > > the axi-dac core is more complex than the axi-adc. Bah it might take less > > than a > > month to have the first version of it posted in the lists but I can't make > > any > > promises. > > For the driver side of things I'd like at least 2, preferably 3 users before > merging. > We have more flexibility to rework things as any issues will probably be > internal > interfaces, but I'd rather wait if we are going to have 3 users within another > month > or 2. > Totally fine by me. But how would this look like? Could we have an immutable branch where we can send patches about this? Or maybe staging? I'm asking because adding more stuff into these series might make it harder to review (the axi-dac might have some fun ABI discussion :)). Ideally, we would have this merged somewhere and then add users on top of it. - Nuno Sá
> > > > > > > > > > > > > And if there's another consumer in the chain, then a node could > > > > > > certainly be both an io-channels consumer and producer. > > > > > > > > > > > > > > > > This should also be possible with this architecture. A node can be both > > > > > a backend > > > > > (provider) and a consumer and we have an out of tree design that fits > > > > > this (that > > > > > I > > > > > surely want to upstream after the foundations are done). > > > > > > > > > > > The architecture of the drivers seems odd to me. It looks similar to > > > > > > making a phy driver handle all the state and protocol with the host > > > > > > controller being a backend. > > > > > > > > > > In this case, it's not really a controller. It's more like an extension > > > > > of the > > > > > device > > > > > because we need a way to handle the high sample rates this ADCs can do. > > > > > Then we > > > > > can > > > > > also do test tones with the backend which is useful for interface tuning > > > > > (as > > > > > mentioned above). > > > > > > > > > > To give you a bit more context, I'm adding the generic property because > > > > > we will > > > > > have > > > > > more users for it (from ADI - the next should be the axi-dac core) but > > > > > STM is > > > > > also > > > > > interested in this (likely the next user). > > > > > > > > > > Hope this makes it a bit more clear... > > > > > > > > Yes, thanks. > > > > > > > > I generally ask for 2 users on new common bindings. I've accepted too > > > > many only to have the 2nd user come in a month later and need additions. > > > > An ack on the binding from the STM folks would be nice here. And > > > > Jonathan too. > > > > > > > > > > Olivier, could we get an ack on the bindings patch? Do you also have any > > > idea about > > > how long it would take for you to send patches so we have another user of > > > the schema? > > > > > > On my side, it might very well take a month or so (given we have holidays > > > nearby) as > > > the axi-dac core is more complex than the axi-adc. Bah it might take less > > > than a > > > month to have the first version of it posted in the lists but I can't make > > > any > > > promises. > > > > For the driver side of things I'd like at least 2, preferably 3 users before > > merging. > > We have more flexibility to rework things as any issues will probably be > > internal > > interfaces, but I'd rather wait if we are going to have 3 users within another > > month > > or 2. > > > > Totally fine by me. But how would this look like? Could we have an immutable > branch where we can send patches about this? Or maybe staging? I'm asking > because adding more stuff into these series might make it harder to review (the > axi-dac might have some fun ABI discussion :)). Ideally, we would have this > merged somewhere and then add users on top of it. It's fine to post a bunch of series with stated dependencies (I've gotten 5 series + deep in the past :) Obviously useful to have a git tree with them all on somewhere though but if you host that it would be ideal given you are driving this work in general. Jonathan > > - Nuno Sá
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@gmail.com> 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@analog.co > > > > > > 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. > > > > > And if there's another consumer in the chain, then a node could > > > > > certainly be both an io-channels consumer and producer. > > > > > > > > > > > > > This should also be possible with this architecture. A node can be both a backend > > > > (provider) and a consumer and we have an out of tree design that fits this (that > > > > I > > > > surely want to upstream after the foundations are done). > > > > > > > > > The architecture of the drivers seems odd to me. It looks similar to > > > > > making a phy driver handle all the state and protocol with the host > > > > > controller being a backend. > > > > > > > > In this case, it's not really a controller. It's more like an extension of the > > > > device > > > > because we need a way to handle the high sample rates this ADCs can do. Then we > > > > can > > > > also do test tones with the backend which is useful for interface tuning (as > > > > mentioned above). > > > > > > > > To give you a bit more context, I'm adding the generic property because we will > > > > have > > > > more users for it (from ADI - the next should be the axi-dac core) but STM is > > > > also > > > > interested in this (likely the next user). > > > > > > > > Hope this makes it a bit more clear... > > > > > > Yes, thanks. > > > > > > I generally ask for 2 users on new common bindings. I've accepted too > > > many only to have the 2nd user come in a month later and need additions. > > > An ack on the binding from the STM folks would be nice here. And > > > Jonathan too. > > > > > > > Olivier, could we get an ack on the bindings patch? Do you also have any idea about > > how long it would take for you to send patches so we have another user of the schema? > > > > On my side, it might very well take a month or so (given we have holidays nearby) as > > the axi-dac core is more complex than the axi-adc. Bah it might take less than a > > month to have the first version of it posted in the lists but I can't make any > > promises. > > For the driver side of things I'd like at least 2, preferably 3 users before merging. > We have more flexibility to rework things as any issues will probably be internal > interfaces, but I'd rather wait if we are going to have 3 users within another month > or 2. Really, for the binding, I'm fine with participation in the upstreaming of this (i.e. reviewing and/or testing on another platform) and a 'this works for us' on the binding. Rob
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@gmail.com> 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@analog.co > > > > > > > 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á > > > > >
Hi Nuno, On 12/15/23 16:18, Nuno Sá 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@analog.co >>>>> 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. > > 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... > >> >>>> And if there's another consumer in the chain, then a node could >>>> certainly be both an io-channels consumer and producer. >>>> >>> >>> This should also be possible with this architecture. A node can be both a backend >>> (provider) and a consumer and we have an out of tree design that fits this (that >>> I >>> surely want to upstream after the foundations are done). >>> >>>> The architecture of the drivers seems odd to me. It looks similar to >>>> making a phy driver handle all the state and protocol with the host >>>> controller being a backend. >>> >>> In this case, it's not really a controller. It's more like an extension of the >>> device >>> because we need a way to handle the high sample rates this ADCs can do. Then we >>> can >>> also do test tones with the backend which is useful for interface tuning (as >>> mentioned above). >>> >>> To give you a bit more context, I'm adding the generic property because we will >>> have >>> more users for it (from ADI - the next should be the axi-dac core) but STM is >>> also >>> interested in this (likely the next user). >>> >>> Hope this makes it a bit more clear... >> >> Yes, thanks. >> >> I generally ask for 2 users on new common bindings. I've accepted too >> many only to have the 2nd user come in a month later and need additions. >> An ack on the binding from the STM folks would be nice here. And >> Jonathan too. >> > > Olivier, could we get an ack on the bindings patch? Do you also have any idea about > how long it would take for you to send patches so we have another user of the schema? > > On my side, it might very well take a month or so (given we have holidays nearby) as > the axi-dac core is more complex than the axi-adc. Bah it might take less than a > month to have the first version of it posted in the lists but I can't make any > promises. > Sorry for late answer. Regarding bindings patch, I assume you refer to [1]. After adding the #io-backend-cells property in v5 you can add my Ack-by Olivier Moysan <olivier.moysan@foss.st.com> I will prepare a serie based on these bindings. Hopefully it should be possible this month. BRs Olivier [1] https://github.com/devicetree-org/dt-schema/pull/120 > - Nuno Sá >
v1: https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f5175273b81dbfe40b7f0daffcdc67d6cb8ff v2: https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@analog.com 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)|---------| |------| | | | |------------------------| ----------- ------------ ------- | ------------------ ------------------------------------------------------- Jonathan, I was also tempted in including the diagram in the source file. Would that be a good idea? [1]: https://github.com/devicetree-org/dt-schema/pull/120 --- Nuno Sa (7): dt-bindings: adc: ad9467: add new io-backend property dt-bindings: adc: axi-adc: deprecate 'adi,adc-dev' driver: core: allow modifying device_links flags iio: buffer-dmaengine: export buffer alloc and free functions iio: add the IIO backend framework iio: adc: ad9467: convert to backend framework iio: adc: adi-axi-adc: move to backend framework Olivier Moysan (1): of: property: add device link support for io-backends .../devicetree/bindings/iio/adc/adi,ad9467.yaml | 5 + .../devicetree/bindings/iio/adc/adi,axi-adc.yaml | 4 +- MAINTAINERS | 8 + drivers/base/core.c | 14 +- drivers/iio/Kconfig | 5 + drivers/iio/Makefile | 1 + drivers/iio/adc/Kconfig | 4 +- drivers/iio/adc/ad9467.c | 243 +++++++------ drivers/iio/adc/adi-axi-adc.c | 381 +++++--------------- drivers/iio/buffer/industrialio-buffer-dmaengine.c | 8 +- drivers/iio/industrialio-backend.c | 386 +++++++++++++++++++++ drivers/of/property.c | 2 + include/linux/iio/adc/adi-axi-adc.h | 68 ---- include/linux/iio/backend.h | 68 ++++ include/linux/iio/buffer-dmaengine.h | 3 + 15 files changed, 730 insertions(+), 470 deletions(-) --- base-commit: 3cde3cdb0b23151dbaac476b17eb1868335993aa change-id: 20231120-dev-iio-backend-d14b473a1d9f -- Thanks! - Nuno Sá