Re: [PATCH RFC 4/8] dt-bindings: iio: dac: add adi axi-dac bus property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06/09/24 1:32 PM, Nuno Sá wrote:
On Fri, 2024-09-06 at 10:04 +0100, Conor Dooley wrote:
On Mon, Sep 02, 2024 at 11:32:37AM +0200, Angelo Dureghello wrote:
Hi Conor,


On 30/08/24 5:33 PM, Conor Dooley wrote:
On Fri, Aug 30, 2024 at 10:19:49AM +0200, Angelo Dureghello wrote:
Hi Conor,

On 29/08/24 5:46 PM, Conor Dooley wrote:
On Thu, Aug 29, 2024 at 02:32:02PM +0200, Angelo Dureghello wrote:
From: Angelo Dureghello <adureghello@xxxxxxxxxxxx>

Add bus property.
RFC it may be, but you do need to explain what this bus-type actually
describes for commenting on the suitability of the method to be
meaningful.
thanks for the feedbacks,

a "bus" is intended as a generic interface connected to the target,
may be used from a custom IP (fpga) to communicate with the target
device (by read/write(reg and value)) using a special custom interface.

The bus could also be physically the same of some well-known existing
interfaces (as parallel, lvds or other uncommon interfaces), but using
an uncommon/custom protocol over it.

In concrete, actually bus-type is added to the backend since the
ad3552r DAC chip can be connected (for maximum speed) by a 5 lanes DDR
parallel bus (interface that i named QSPI, but it's not exactly a QSPI
as a protocol), so it's a device-specific interface.

With additions in this patchset, other frontends, of course not only
DACs, will be able to add specific busses and read/wrtie to the bus
as needed.

Signed-off-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
---
    Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml | 9
+++++++++
    1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
b/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
index a55e9bfc66d7..a7ce72e1cd81 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
@@ -38,6 +38,15 @@ properties:
      clocks:
        maxItems: 1
You mentioned about new compatible strings, does the one currently
listed in this binding support both bus types?
You didn't answer this, and there's insufficient explanation of the
"hardware" in this RFC, but I found this which is supposedly the
backend:
https://github.com/analogdevicesinc/hdl/tree/main/library/axi_ad3552r
adi,axi-dac.yaml has a single compatible, and that compatible has
nothing to do with "axi_ad3552r" as it is "adi,axi-dac-9.1.b". I would
expect either justification for reuse of the compatible, or a brand new
compatible for this backend, even if the driver can mostly be reused.

Could you please link to whatever ADI wiki has detailed information on
how this stuff works so that I can look at it to better understand the
axes of configuration here?
https://analogdevicesinc.github.io/hdl/library/axi_ad3552r/index.html

that has same structure and register set of the generic ADI AXI-DAC IP:
https://wiki.analog.com/resources/fpga/docs/axi_dac_ip


Making the bus type decision based on compatible only really makes sense
if they're different versions of the IP, but not if they're different
configuration options for a given version.

+  bus-type:
DAC IP on fpga actually respects same structure and register set, except
for a named "custom" register that may use specific bitfields depending
on the application of the IP.
To paraphrase:
"The register map is the same, except for the bit that is different".
If ADI is shipping several different configurations of this IP for
different DACs, I'd be expecting different compatibles for each backend
to be honest
i am still quite new to this fpga-based implementations, at least for how
such IPs are actually interfacing to the linux subsystem, so i may miss
some point.

About the "adi,axi-dac-9.1.b" compatible, the generic DAC IP register set
is mostly the same structure of this ad3552r IP (links above), except for
bitfields in the DAC_CUSTOM_CTRL register.

My choice for now was to add a bus-type property.

Not an HDL expert, but i think a different bus means, from an hardware point
of
view, a different IP in terms of internal fpga circuitry, even if not as a
register-set.
Depending on whether or not the unmodified driver can be used with this
IP (so the QSPI bus stuff would need to be optional) then a fallback
should be used given the degree of similarity. It, however, seems likely
that is not the case, and without the QSPI bus there'd be no way to
communicate with the device. Is there any reason to use this IP as a
backend, without connecting the QSPI bus at all, leaving the ADC/DAC on
a regular SPI bus?

Somewhere in my replies, I'm doing the exact same question to myself. We probably
need to speak with the FPGA folks but I guess (hope) they had a good reason for this.

- Nuno Sá

to clarify a bit the custom (fpga-based) QSPI need, i did some checks in the
datasheets:

1. ADI is actually supporting ad3552r by eval-ad3552r-fmcx eval boards,
with specific fmc connector for the ZedBoard (zynq7000). This is the
current focused hardware for this job.

2. Zynq7000 std non-fpga controller is designed to control flash memories,
but can operate in "raw I/O" mode, so it may work with ad3552r, but is not
supporting DDR, even if it may reach 100Mhz clock.

3. ad3552r accepts a maximum clock of 66Mhz. So for ZedBoard maximum speed
of 33MUPS cannot be reached without DDR.

4. ad3552r requires DDR only in the data part, and in DDR mode we
may also send some "non-loop" reg read/write, so requiring also the
address to be sent in SDR. Not sure how many QSPI controllers in the market are working this way, even if it seems quite standard, looks like not many are actually supporting DDR. There may be, but not actually in the priority of my customer
right now. And in that case, we could extend the generic spi ad3552r.c.

Regards,

--
 ,,,      Angelo Dureghello
:: :.     BayLibre -runtime team- Developer
:`___:
 `____:





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux