| Message ID | 20241216-wip-bl-ad3552r-axi-v0-iio-testing-carlos-v1-4-856ff71fc930@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: ad3552r-hs: add support for ad3541/42r | expand |
On Mon, 2024-12-16 at 21:36 +0100, Angelo Dureghello wrote: > From: Antoniu Miclaus <antoniu.miclaus@analog.com> > > Add backend support for setting and getting the interface type > in use. > > Link: > https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > Co-developed-by: Angelo Dureghello <adureghello@baylibre.com> > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > --- > This patch has been picked up from the Antoniu patchset > still not accepted, and extended with the interface setter, > fixing also namespace names to be between quotation marks. > --- > drivers/iio/industrialio-backend.c | 42 > ++++++++++++++++++++++++++++++++++++++ > include/linux/iio/backend.h | 19 +++++++++++++++++ > 2 files changed, 61 insertions(+) > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- > backend.c > index 363281272035..6edc3e685f6a 100644 > --- a/drivers/iio/industrialio-backend.c > +++ b/drivers/iio/industrialio-backend.c > @@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev > *indio_dev, uintptr_t private, > } > EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND"); > > +/** > + * iio_backend_interface_type_get - get the interface type used. > + * @back: Backend device > + * @type: Interface type > + * > + * RETURNS: > + * 0 on success, negative error number on failure. > + */ > +int iio_backend_interface_type_get(struct iio_backend *back, > + enum iio_backend_interface_type *type) > +{ > + int ret; > + > + ret = iio_backend_op_call(back, interface_type_get, type); > + if (ret) > + return ret; > + > + if (*type >= IIO_BACKEND_INTERFACE_MAX) > + return -EINVAL; > + > + return 0; > +} > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND"); > + > +/** > + * iio_backend_interface_type_set - set the interface type used. > + * @back: Backend device > + * @type: Interface type > + * > + * RETURNS: > + * 0 on success, negative error number on failure. > + */ > +int iio_backend_interface_type_set(struct iio_backend *back, > + enum iio_backend_interface_type type) > +{ > + if (type >= IIO_BACKEND_INTERFACE_MAX) > + return -EINVAL; > + > + return iio_backend_op_call(back, interface_type_set, type); > +} > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND"); > + > /** > * iio_backend_extend_chan_spec - Extend an IIO channel > * @back: Backend device > diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h > index 10be00f3b120..2b7221099d8c 100644 > --- a/include/linux/iio/backend.h > +++ b/include/linux/iio/backend.h > @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger { > IIO_BACKEND_SAMPLE_TRIGGER_MAX > }; > > +enum iio_backend_interface_type { > + IIO_BACKEND_INTERFACE_SERIAL_LVDS, > + IIO_BACKEND_INTERFACE_SERIAL_CMOS, The above are apparently not used in the next patch so I would not add them now. > + IIO_BACKEND_INTERFACE_SERIAL_SPI, > + IIO_BACKEND_INTERFACE_SERIAL_DSPI, > + IIO_BACKEND_INTERFACE_SERIAL_QSPI, I'll throw my 2 cents but it would be nice to have more feedback on this. I'm not completely sure about the xSPI stuff in here. We treated the QSPI as a bus both for control and data in which we also add child devices. And we've been adding specific bus operations/configurations through the 'struct ad3552r_hs_platform_data' interface. So, I'm wondering if this should also not be set through that interface? LVDS/CMOS still looks slightly different to me... - Nuno Sá
On Tue, 17 Dec 2024 11:13:59 +0100 Nuno Sá <noname.nuno@gmail.com> wrote: > On Mon, 2024-12-16 at 21:36 +0100, Angelo Dureghello wrote: > > From: Antoniu Miclaus <antoniu.miclaus@analog.com> > > > > Add backend support for setting and getting the interface type > > in use. > > > > Link: > > https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > > Co-developed-by: Angelo Dureghello <adureghello@baylibre.com> > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > > --- > > This patch has been picked up from the Antoniu patchset > > still not accepted, and extended with the interface setter, > > fixing also namespace names to be between quotation marks. > > --- > > drivers/iio/industrialio-backend.c | 42 > > ++++++++++++++++++++++++++++++++++++++ > > include/linux/iio/backend.h | 19 +++++++++++++++++ > > 2 files changed, 61 insertions(+) > > > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- > > backend.c > > index 363281272035..6edc3e685f6a 100644 > > --- a/drivers/iio/industrialio-backend.c > > +++ b/drivers/iio/industrialio-backend.c > > @@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev > > *indio_dev, uintptr_t private, > > } > > EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND"); > > > > +/** > > + * iio_backend_interface_type_get - get the interface type used. > > + * @back: Backend device > > + * @type: Interface type > > + * > > + * RETURNS: > > + * 0 on success, negative error number on failure. > > + */ > > +int iio_backend_interface_type_get(struct iio_backend *back, > > + enum iio_backend_interface_type *type) > > +{ > > + int ret; > > + > > + ret = iio_backend_op_call(back, interface_type_get, type); > > + if (ret) > > + return ret; > > + > > + if (*type >= IIO_BACKEND_INTERFACE_MAX) > > + return -EINVAL; > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND"); > > + > > +/** > > + * iio_backend_interface_type_set - set the interface type used. > > + * @back: Backend device > > + * @type: Interface type > > + * > > + * RETURNS: > > + * 0 on success, negative error number on failure. > > + */ > > +int iio_backend_interface_type_set(struct iio_backend *back, > > + enum iio_backend_interface_type type) > > +{ > > + if (type >= IIO_BACKEND_INTERFACE_MAX) > > + return -EINVAL; > > + > > + return iio_backend_op_call(back, interface_type_set, type); > > +} > > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND"); > > + > > /** > > * iio_backend_extend_chan_spec - Extend an IIO channel > > * @back: Backend device > > diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h > > index 10be00f3b120..2b7221099d8c 100644 > > --- a/include/linux/iio/backend.h > > +++ b/include/linux/iio/backend.h > > @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger { > > IIO_BACKEND_SAMPLE_TRIGGER_MAX > > }; > > > > +enum iio_backend_interface_type { > > + IIO_BACKEND_INTERFACE_SERIAL_LVDS, > > + IIO_BACKEND_INTERFACE_SERIAL_CMOS, > > The above are apparently not used in the next patch so I would not add them now. > > + IIO_BACKEND_INTERFACE_SERIAL_SPI, > > + IIO_BACKEND_INTERFACE_SERIAL_DSPI, > > + IIO_BACKEND_INTERFACE_SERIAL_QSPI, > > I'll throw my 2 cents but it would be nice to have more feedback on this. I'm > not completely sure about the xSPI stuff in here. We treated the QSPI as a bus > both for control and data in which we also add child devices. And we've been > adding specific bus operations/configurations through the 'struct > ad3552r_hs_platform_data' interface. So, I'm wondering if this should also not > be set through that interface? Maybe - kind of hard to tell until we actually have code. I'd go for kicking them into the long grass for now if they aren't used and just dropping them from this patch. If we ever need them,easy to bring back and then we should have a justification for why! J > > LVDS/CMOS still looks slightly different to me... > > - Nuno Sá > > >
On Thu, 19 Dec 2024 16:42:33 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Tue, 17 Dec 2024 11:13:59 +0100 > Nuno Sá <noname.nuno@gmail.com> wrote: > > > On Mon, 2024-12-16 at 21:36 +0100, Angelo Dureghello wrote: > > > From: Antoniu Miclaus <antoniu.miclaus@analog.com> > > > > > > Add backend support for setting and getting the interface type > > > in use. > > > > > > Link: > > > https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > > > Co-developed-by: Angelo Dureghello <adureghello@baylibre.com> > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > > > --- > > > This patch has been picked up from the Antoniu patchset > > > still not accepted, and extended with the interface setter, > > > fixing also namespace names to be between quotation marks. > > > --- > > > drivers/iio/industrialio-backend.c | 42 > > > ++++++++++++++++++++++++++++++++++++++ > > > include/linux/iio/backend.h | 19 +++++++++++++++++ > > > 2 files changed, 61 insertions(+) > > > > > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- > > > backend.c > > > index 363281272035..6edc3e685f6a 100644 > > > --- a/drivers/iio/industrialio-backend.c > > > +++ b/drivers/iio/industrialio-backend.c > > > @@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev > > > *indio_dev, uintptr_t private, > > > } > > > EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND"); > > > > > > +/** > > > + * iio_backend_interface_type_get - get the interface type used. > > > + * @back: Backend device > > > + * @type: Interface type > > > + * > > > + * RETURNS: > > > + * 0 on success, negative error number on failure. > > > + */ > > > +int iio_backend_interface_type_get(struct iio_backend *back, > > > + enum iio_backend_interface_type *type) > > > +{ > > > + int ret; > > > + > > > + ret = iio_backend_op_call(back, interface_type_get, type); > > > + if (ret) > > > + return ret; > > > + > > > + if (*type >= IIO_BACKEND_INTERFACE_MAX) > > > + return -EINVAL; > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND"); > > > + > > > +/** > > > + * iio_backend_interface_type_set - set the interface type used. > > > + * @back: Backend device > > > + * @type: Interface type > > > + * > > > + * RETURNS: > > > + * 0 on success, negative error number on failure. > > > + */ > > > +int iio_backend_interface_type_set(struct iio_backend *back, > > > + enum iio_backend_interface_type type) > > > +{ > > > + if (type >= IIO_BACKEND_INTERFACE_MAX) > > > + return -EINVAL; > > > + > > > + return iio_backend_op_call(back, interface_type_set, type); > > > +} > > > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND"); > > > + > > > /** > > > * iio_backend_extend_chan_spec - Extend an IIO channel > > > * @back: Backend device > > > diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h > > > index 10be00f3b120..2b7221099d8c 100644 > > > --- a/include/linux/iio/backend.h > > > +++ b/include/linux/iio/backend.h > > > @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger { > > > IIO_BACKEND_SAMPLE_TRIGGER_MAX > > > }; > > > > > > +enum iio_backend_interface_type { > > > + IIO_BACKEND_INTERFACE_SERIAL_LVDS, > > > + IIO_BACKEND_INTERFACE_SERIAL_CMOS, > > > > The above are apparently not used in the next patch so I would not add them now. > > > + IIO_BACKEND_INTERFACE_SERIAL_SPI, > > > + IIO_BACKEND_INTERFACE_SERIAL_DSPI, > > > + IIO_BACKEND_INTERFACE_SERIAL_QSPI, > > > > I'll throw my 2 cents but it would be nice to have more feedback on this. I'm > > not completely sure about the xSPI stuff in here. We treated the QSPI as a bus > > both for control and data in which we also add child devices. And we've been > > adding specific bus operations/configurations through the 'struct > > ad3552r_hs_platform_data' interface. So, I'm wondering if this should also not > > be set through that interface? > > Maybe - kind of hard to tell until we actually have code. > I'd go for kicking them into the long grass for now if they aren't used and > just dropping them from this patch. If we ever need them,easy to bring back > and then we should have a justification for why! oops. Misread. Obviously Nuno was saying the ones above aren't used, not the SPI ones... I don't feel strongly either way on setting these via this generic interface, or via the other path. Jonathan > > J > > > > > > LVDS/CMOS still looks slightly different to me... > > > > - Nuno Sá > > > > > > > >
On 12/19/24 10:51 AM, Jonathan Cameron wrote: > On Thu, 19 Dec 2024 16:42:33 +0000 > Jonathan Cameron <jic23@kernel.org> wrote: > >> On Tue, 17 Dec 2024 11:13:59 +0100 >> Nuno Sá <noname.nuno@gmail.com> wrote: >> >>> On Mon, 2024-12-16 at 21:36 +0100, Angelo Dureghello wrote: >>>> From: Antoniu Miclaus <antoniu.miclaus@analog.com> >>>> >>>> Add backend support for setting and getting the interface type >>>> in use. >>>> >>>> Link: >>>> https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b >>>> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> >>>> Co-developed-by: Angelo Dureghello <adureghello@baylibre.com> >>>> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> >>>> --- >>>> This patch has been picked up from the Antoniu patchset >>>> still not accepted, and extended with the interface setter, >>>> fixing also namespace names to be between quotation marks. >>>> --- >>>> drivers/iio/industrialio-backend.c | 42 >>>> ++++++++++++++++++++++++++++++++++++++ >>>> include/linux/iio/backend.h | 19 +++++++++++++++++ >>>> 2 files changed, 61 insertions(+) >>>> >>>> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- >>>> backend.c >>>> index 363281272035..6edc3e685f6a 100644 >>>> --- a/drivers/iio/industrialio-backend.c >>>> +++ b/drivers/iio/industrialio-backend.c >>>> @@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev >>>> *indio_dev, uintptr_t private, >>>> } >>>> EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND"); >>>> >>>> +/** >>>> + * iio_backend_interface_type_get - get the interface type used. >>>> + * @back: Backend device >>>> + * @type: Interface type >>>> + * >>>> + * RETURNS: >>>> + * 0 on success, negative error number on failure. >>>> + */ >>>> +int iio_backend_interface_type_get(struct iio_backend *back, >>>> + enum iio_backend_interface_type *type) >>>> +{ >>>> + int ret; >>>> + >>>> + ret = iio_backend_op_call(back, interface_type_get, type); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (*type >= IIO_BACKEND_INTERFACE_MAX) >>>> + return -EINVAL; >>>> + >>>> + return 0; >>>> +} >>>> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND"); >>>> + >>>> +/** >>>> + * iio_backend_interface_type_set - set the interface type used. >>>> + * @back: Backend device >>>> + * @type: Interface type >>>> + * >>>> + * RETURNS: >>>> + * 0 on success, negative error number on failure. >>>> + */ >>>> +int iio_backend_interface_type_set(struct iio_backend *back, >>>> + enum iio_backend_interface_type type) >>>> +{ >>>> + if (type >= IIO_BACKEND_INTERFACE_MAX) >>>> + return -EINVAL; >>>> + >>>> + return iio_backend_op_call(back, interface_type_set, type); >>>> +} >>>> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND"); >>>> + >>>> /** >>>> * iio_backend_extend_chan_spec - Extend an IIO channel >>>> * @back: Backend device >>>> diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h >>>> index 10be00f3b120..2b7221099d8c 100644 >>>> --- a/include/linux/iio/backend.h >>>> +++ b/include/linux/iio/backend.h >>>> @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger { >>>> IIO_BACKEND_SAMPLE_TRIGGER_MAX >>>> }; >>>> >>>> +enum iio_backend_interface_type { >>>> + IIO_BACKEND_INTERFACE_SERIAL_LVDS, >>>> + IIO_BACKEND_INTERFACE_SERIAL_CMOS, >>> >>> The above are apparently not used in the next patch so I would not add them now. >>>> + IIO_BACKEND_INTERFACE_SERIAL_SPI, >>>> + IIO_BACKEND_INTERFACE_SERIAL_DSPI, >>>> + IIO_BACKEND_INTERFACE_SERIAL_QSPI, >>> >>> I'll throw my 2 cents but it would be nice to have more feedback on this. I'm >>> not completely sure about the xSPI stuff in here. We treated the QSPI as a bus >>> both for control and data in which we also add child devices. And we've been >>> adding specific bus operations/configurations through the 'struct >>> ad3552r_hs_platform_data' interface. So, I'm wondering if this should also not >>> be set through that interface? >> >> Maybe - kind of hard to tell until we actually have code. >> I'd go for kicking them into the long grass for now if they aren't used and >> just dropping them from this patch. If we ever need them,easy to bring back >> and then we should have a justification for why! > > oops. Misread. Obviously Nuno was saying the ones above aren't used, not the > SPI ones... I don't feel strongly either way on setting these via > this generic interface, or via the other path. > > Jonathan > >> >> J >> >> >>> >>> LVDS/CMOS still looks slightly different to me... >>> >>> - Nuno Sá >>> >>> >>> >> >> > I'm the one that suggested doing it this way to Angelo, so I'll chime in with my thinking. In Angelo's previous series where we added IIO backend support for this family of chips, in the devicetree discussion, we considered the possibility of two SPI controllers, one for register access and one for the high-speed streaming provided by the backend. Since the dual and quad SPI here is for IIO backend (the high-speed sample data interface) is what made me think we should put the control here rather than putting it in the platform data interface. Since this is new HDL, maybe we could avoid this issue altogether and tweak the implementation in the FPGA a bit. Clearly we had the AD3552R working without needing to poke this registers, so why can't we do the same for the new chip? In other words, make these things a compile time option in the HDL rather than a runtime option?
On 19.12.2024 11:01, David Lechner wrote: > On 12/19/24 10:51 AM, Jonathan Cameron wrote: > > On Thu, 19 Dec 2024 16:42:33 +0000 > > Jonathan Cameron <jic23@kernel.org> wrote: > > > >> On Tue, 17 Dec 2024 11:13:59 +0100 > >> Nuno Sá <noname.nuno@gmail.com> wrote: > >> > >>> On Mon, 2024-12-16 at 21:36 +0100, Angelo Dureghello wrote: > >>>> From: Antoniu Miclaus <antoniu.miclaus@analog.com> > >>>> > >>>> Add backend support for setting and getting the interface type > >>>> in use. > >>>> > >>>> Link: > >>>> https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b > >>>> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > >>>> Co-developed-by: Angelo Dureghello <adureghello@baylibre.com> > >>>> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > >>>> --- > >>>> This patch has been picked up from the Antoniu patchset > >>>> still not accepted, and extended with the interface setter, > >>>> fixing also namespace names to be between quotation marks. > >>>> --- > >>>> drivers/iio/industrialio-backend.c | 42 > >>>> ++++++++++++++++++++++++++++++++++++++ > >>>> include/linux/iio/backend.h | 19 +++++++++++++++++ > >>>> 2 files changed, 61 insertions(+) > >>>> > >>>> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- > >>>> backend.c > >>>> index 363281272035..6edc3e685f6a 100644 > >>>> --- a/drivers/iio/industrialio-backend.c > >>>> +++ b/drivers/iio/industrialio-backend.c > >>>> @@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev > >>>> *indio_dev, uintptr_t private, > >>>> } > >>>> EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND"); > >>>> > >>>> +/** > >>>> + * iio_backend_interface_type_get - get the interface type used. > >>>> + * @back: Backend device > >>>> + * @type: Interface type > >>>> + * > >>>> + * RETURNS: > >>>> + * 0 on success, negative error number on failure. > >>>> + */ > >>>> +int iio_backend_interface_type_get(struct iio_backend *back, > >>>> + enum iio_backend_interface_type *type) > >>>> +{ > >>>> + int ret; > >>>> + > >>>> + ret = iio_backend_op_call(back, interface_type_get, type); > >>>> + if (ret) > >>>> + return ret; > >>>> + > >>>> + if (*type >= IIO_BACKEND_INTERFACE_MAX) > >>>> + return -EINVAL; > >>>> + > >>>> + return 0; > >>>> +} > >>>> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND"); > >>>> + > >>>> +/** > >>>> + * iio_backend_interface_type_set - set the interface type used. > >>>> + * @back: Backend device > >>>> + * @type: Interface type > >>>> + * > >>>> + * RETURNS: > >>>> + * 0 on success, negative error number on failure. > >>>> + */ > >>>> +int iio_backend_interface_type_set(struct iio_backend *back, > >>>> + enum iio_backend_interface_type type) > >>>> +{ > >>>> + if (type >= IIO_BACKEND_INTERFACE_MAX) > >>>> + return -EINVAL; > >>>> + > >>>> + return iio_backend_op_call(back, interface_type_set, type); > >>>> +} > >>>> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND"); > >>>> + > >>>> /** > >>>> * iio_backend_extend_chan_spec - Extend an IIO channel > >>>> * @back: Backend device > >>>> diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h > >>>> index 10be00f3b120..2b7221099d8c 100644 > >>>> --- a/include/linux/iio/backend.h > >>>> +++ b/include/linux/iio/backend.h > >>>> @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger { > >>>> IIO_BACKEND_SAMPLE_TRIGGER_MAX > >>>> }; > >>>> > >>>> +enum iio_backend_interface_type { > >>>> + IIO_BACKEND_INTERFACE_SERIAL_LVDS, > >>>> + IIO_BACKEND_INTERFACE_SERIAL_CMOS, > >>> > >>> The above are apparently not used in the next patch so I would not add them now. > >>>> + IIO_BACKEND_INTERFACE_SERIAL_SPI, > >>>> + IIO_BACKEND_INTERFACE_SERIAL_DSPI, > >>>> + IIO_BACKEND_INTERFACE_SERIAL_QSPI, > >>> > >>> I'll throw my 2 cents but it would be nice to have more feedback on this. I'm > >>> not completely sure about the xSPI stuff in here. We treated the QSPI as a bus > >>> both for control and data in which we also add child devices. And we've been > >>> adding specific bus operations/configurations through the 'struct > >>> ad3552r_hs_platform_data' interface. So, I'm wondering if this should also not > >>> be set through that interface? > >> > >> Maybe - kind of hard to tell until we actually have code. > >> I'd go for kicking them into the long grass for now if they aren't used and > >> just dropping them from this patch. If we ever need them,easy to bring back > >> and then we should have a justification for why! > > > > oops. Misread. Obviously Nuno was saying the ones above aren't used, not the > > SPI ones... I don't feel strongly either way on setting these via > > this generic interface, or via the other path. > > > > Jonathan > > > >> > >> J > >> > >> > >>> > >>> LVDS/CMOS still looks slightly different to me... > >>> > >>> - Nuno Sá > >>> > >>> > >>> > >> > >> > > > > I'm the one that suggested doing it this way to Angelo, so I'll chime in > with my thinking. In Angelo's previous series where we added IIO backend > support for this family of chips, in the devicetree discussion, we > considered the possibility of two SPI controllers, one for register > access and one for the high-speed streaming provided by the backend. > Since the dual and quad SPI here is for IIO backend (the high-speed sample > data interface) is what made me think we should put the control here rather > than putting it in the platform data interface. > > Since this is new HDL, maybe we could avoid this issue altogether and > tweak the implementation in the FPGA a bit. Clearly we had the AD3552R > working without needing to poke this registers, so why can't we do the > same for the new chip? In other words, make these things a compile time > option in the HDL rather than a runtime option? The purpose of the new HDL is to support all the devices of the family, (DSPI/QSPI) just acting on axi MULTI_IO_MODE. Thinking to this a bit better, changing MULTI_IO_MODE is not exactly changing "interface" type so maybe is more appropriarte to use a patform data "bus_mode" function. New ad354x chips do not have QSPI pin, so no chance to access the whole primary + secondary area in DSPI without changes in configuration registers, as done before with ad355xr. Regards, angelo
diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c index 363281272035..6edc3e685f6a 100644 --- a/drivers/iio/industrialio-backend.c +++ b/drivers/iio/industrialio-backend.c @@ -636,6 +636,48 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private, } EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, "IIO_BACKEND"); +/** + * iio_backend_interface_type_get - get the interface type used. + * @back: Backend device + * @type: Interface type + * + * RETURNS: + * 0 on success, negative error number on failure. + */ +int iio_backend_interface_type_get(struct iio_backend *back, + enum iio_backend_interface_type *type) +{ + int ret; + + ret = iio_backend_op_call(back, interface_type_get, type); + if (ret) + return ret; + + if (*type >= IIO_BACKEND_INTERFACE_MAX) + return -EINVAL; + + return 0; +} +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND"); + +/** + * iio_backend_interface_type_set - set the interface type used. + * @back: Backend device + * @type: Interface type + * + * RETURNS: + * 0 on success, negative error number on failure. + */ +int iio_backend_interface_type_set(struct iio_backend *back, + enum iio_backend_interface_type type) +{ + if (type >= IIO_BACKEND_INTERFACE_MAX) + return -EINVAL; + + return iio_backend_op_call(back, interface_type_set, type); +} +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND"); + /** * iio_backend_extend_chan_spec - Extend an IIO channel * @back: Backend device diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h index 10be00f3b120..2b7221099d8c 100644 --- a/include/linux/iio/backend.h +++ b/include/linux/iio/backend.h @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger { IIO_BACKEND_SAMPLE_TRIGGER_MAX }; +enum iio_backend_interface_type { + IIO_BACKEND_INTERFACE_SERIAL_LVDS, + IIO_BACKEND_INTERFACE_SERIAL_CMOS, + IIO_BACKEND_INTERFACE_SERIAL_SPI, + IIO_BACKEND_INTERFACE_SERIAL_DSPI, + IIO_BACKEND_INTERFACE_SERIAL_QSPI, + IIO_BACKEND_INTERFACE_MAX +}; + /** * struct iio_backend_ops - operations structure for an iio_backend * @enable: Enable backend. @@ -88,6 +97,8 @@ enum iio_backend_sample_trigger { * @extend_chan_spec: Extend an IIO channel. * @ext_info_set: Extended info setter. * @ext_info_get: Extended info getter. + * @interface_type_get: Interface type. + * @interface_type_set: Interface type setter. * @read_raw: Read a channel attribute from a backend device * @debugfs_print_chan_status: Print channel status into a buffer. * @debugfs_reg_access: Read or write register value of backend. @@ -128,6 +139,10 @@ struct iio_backend_ops { const char *buf, size_t len); int (*ext_info_get)(struct iio_backend *back, uintptr_t private, const struct iio_chan_spec *chan, char *buf); + int (*interface_type_get)(struct iio_backend *back, + enum iio_backend_interface_type *type); + int (*interface_type_set)(struct iio_backend *back, + enum iio_backend_interface_type type); int (*read_raw)(struct iio_backend *back, struct iio_chan_spec const *chan, int *val, int *val2, long mask); @@ -186,6 +201,10 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private, const char *buf, size_t len); ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private, const struct iio_chan_spec *chan, char *buf); +int iio_backend_interface_type_get(struct iio_backend *back, + enum iio_backend_interface_type *type); +int iio_backend_interface_type_set(struct iio_backend *back, + enum iio_backend_interface_type type); int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const *chan, int *val, int *val2, long mask);