Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPI Engine offload support and ad7944/ad7380 mainline pre-review #2341

Closed
wants to merge 22 commits into from

Conversation

dlech
Copy link
Collaborator

@dlech dlech commented Nov 14, 2023

This is my work in progress so far on mainlining SPI Engine offload support. I have also created a new AD7944 driver for testing this. Most of this is still a work in progress, so I'm not looking for detailed review of everything quite yet.

The first 12 patches (everything before "dt-bindings: spi: adi,axi-spi-engine: add offload bindings") are just cleanups and improvements of the existing upstream SPI Engine driver. These patches should be ready to submit upstream ASAP as a series if there aren't any objections.

There are some additional non-offload related SPI Engine improvements starting with "spi: axi-spi-engine: populate xfer->effective_speed_hz", but those need more testing and cleanup before they will be ready for pre-review here and submission to the mailing lists. I plan to send these as a second series once the first has been finalized and picked up.

The offload support will be a 3rd series and will probably need to also include the ad7944 driver to show how consumers of the offload work. FYI, I have already run the offload devicetree bindings by Rob Herring on IRC, so I don't expect any major changes needed there. There are a few things to sort out with the HDL first (i.e. https://github.com/adi-ses/sea-misc/pull/56) before the details are finalized.

But, the big picture is...

  • Compared to the offload implementation in the ADI tree, a bit of the SPI Engine offload support is moved to the core SPI code as discussed in Unsafe buffer access in AXI SPI Engine offload #2300 (comment) which simplifies things a bit.
  • I wrote the AD7944 driver so that it works with any SPI controller (without any offload mechanism). The reasoning being that this would ensure that the IIO ABI is "correct" and is not being bent to match offload requirements (and who knows, maybe someday someone would want to use this with a regular SPI controller with cyclic DMA transfers).
  • I implemented the PWM and DMA that are connected to the SPI Engine offload as a sort of standalone device that is both an IIO trigger and an IIO buffer (currently, the poorly titled "iio: trigger: add PWM trigger " patch).

You will also find a bunch of // comments in the later patches. This is intentional to remind myself of things that need more work before submitting upstream. If anyone has any bright ideas for addressing any of those TODOs though, I will gladly take suggestions since I haven't thought of good solutions to a lot of them yet.

@nunojsa
Copy link
Collaborator

nunojsa commented Nov 14, 2023

The first 12 patches (everything before "dt-bindings: spi: adi,axi-spi-engine: add offload bindings") are just cleanups and improvements of the existing upstream SPI Engine driver. These patches should be ready to submit upstream ASAP as a series if there aren't any objections.

Alright, for now I will concentrate on those. I hope to have some time today or tomorrow so you can send them upstream ASAP. If there's no feedback on me till Thursday, please ping me.

Copy link
Collaborator

@threexc threexc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really minor comment - in the commit message for 8d057a8, you say "However, it did set any..." when it should be "However, it didn't set any..."

@dlech
Copy link
Collaborator Author

dlech commented Nov 16, 2023

If there's no feedback on me till Thursday, please ping me.

ping @nunojsa

@nunojsa
Copy link
Collaborator

nunojsa commented Nov 16, 2023

ping @nunojsa

Oh yeah, will do it tomorrow morning... Sorry for the delay.

But if you are really in a hurry, just send it and I'll check it upstream.

Copy link
Collaborator

@nunojsa nunojsa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM... Nice series. The one more worrying thing is the devm conversion. Other than that, just some things that likely I'm failing to understand.

So, go ahead :)

drivers/spi/spi-axi-spi-engine.c Outdated Show resolved Hide resolved
drivers/spi/spi-axi-spi-engine.c Show resolved Hide resolved
drivers/spi/spi-axi-spi-engine.c Outdated Show resolved Hide resolved
drivers/spi/spi-axi-spi-engine.c Outdated Show resolved Hide resolved
drivers/spi/spi-axi-spi-engine.c Outdated Show resolved Hide resolved
drivers/spi/spi-axi-spi-engine.c Outdated Show resolved Hide resolved
@nunojsa
Copy link
Collaborator

nunojsa commented Nov 17, 2023

I would also make a comment about the maintainers you set (me and Michael). I'm ok with that but the original developer was Lars, so maybe it would be a good idea to ask him if he's ok with and if he wants to get included in the list.

Also remembered one thing.... There's no MAINTAINERS entry for this driver AFAICT, so one patch for it would be nice (like the first one in the series :))

@dlech
Copy link
Collaborator Author

dlech commented Nov 17, 2023

Status update: the first round of patches has been sent upstream: https://lore.kernel.org/linux-spi/[email protected]/

I will make another comment when the next round of patches after that is ready to look at again.

@dlech dlech force-pushed the dlech/ad7944-mainline branch 2 times, most recently from e9ed1bd to 9eb53ae Compare November 27, 2023 17:04
@dlech
Copy link
Collaborator Author

dlech commented Nov 27, 2023

I have updated this with the latest work in progress to facilitate the discussion in https://github.com/adi-ses/sea-misc/issues/55.

Also rebased on current spi/for-6.8 where some of these patches have already been applied upstream.

@dlech dlech force-pushed the dlech/ad7944-mainline branch 6 times, most recently from 3ba28ba to 0eb5548 Compare November 29, 2023 22:50
@dlech
Copy link
Collaborator Author

dlech commented Nov 29, 2023

The second round of patches is ready for review (ping @nunojsa).

Since the last review, many of these patches are new or have significantly changed. Once we've had a few more eyes on these, I will send this as a series on the mailing lists.

098e802 spi: axi-spi-engine: fix CPHA
7bf6634 spi: axi-spi-engine: change return of spi_engine_compile_message() to void
8965c13 spi: axi-spi-engine: populate xfer->effective_speed_hz
34cd0a3 spi: axi-spi-engine: remove spi_engine_get_clk_div()
cb1d761 spi: axi-spi-engine: fix sleep ticks calculation
a1cc53b spi: axi-spi-engine: remove xfer arg from spi_engine_gen_sleep()
d874af4 spi: axi-spi-engine: implement xfer->cs_change_delay
dfcf742 spi: axi-spi-engine: restore clkdiv at end of message
85d2751 spi: axi-spi-engine: remove delay from CS assertion
27b9c56 spi: axi-spi-engine: add watchdog timer

Copy link
Collaborator

@nunojsa nunojsa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM... nice series.

drivers/spi/spi-axi-spi-engine.c Outdated Show resolved Hide resolved
drivers/spi/spi-axi-spi-engine.c Outdated Show resolved Hide resolved
drivers/spi/spi-axi-spi-engine.c Outdated Show resolved Hide resolved
drivers/spi/spi-axi-spi-engine.c Outdated Show resolved Hide resolved
drivers/spi/spi-axi-spi-engine.c Outdated Show resolved Hide resolved
@dlech dlech force-pushed the dlech/ad7944-mainline branch 2 times, most recently from 152b52a to 7c341c0 Compare December 6, 2023 21:49
@dlech dlech changed the title SPI Engine offload support and ad7944 mainline pre-review SPI Engine offload support and ad7944/ad7380 mainline pre-review Dec 7, 2023
@dlech dlech force-pushed the dlech/ad7944-mainline branch 6 times, most recently from 488d51c to 66a2402 Compare December 15, 2023 14:55
This replaces use of individual buffer mode flags with
INDIO_ALL_BUFFER_MODES in the iio_buffer_enabled() function.

This simplifies the code and makes it robust in case of the addition of
new buffer modes.

Signed-off-by: David Lechner <[email protected]>
This is the culmination of the previous AXI SPI Engine improvement
series [1] and [2] where we made fixes and improvements to prepare
for adding offload support.

Simply put, "offload" support is defined as a capability of advanced
SPI controllers to record a series of SPI transactions and then play
them back using a hardware trigger. This allows operations to be
performed, usually repeating many times, without any CPU intervention.

This series includes core SPI support along with the first SPI
controller (AXI SPI Engine) and SPI device (AD7380 ADC) that use them.
This enables capturing analog data at 2 million samples per second with
virtually no jitter.

I did some research while working on this to see if there are any other
SPI controllers in the wild that provide similar offload capabilities.
I found that Freescale QuadSPI and NXP FlexSPI have programable sequence
engines similar to the AXI SPI engine. These SPI controllers are
designed primarily for use with flash memory, so it looks like this
feature could potentially be useful for more than just this first use
case with ADCs.

The series is organized as follows:

- SPI core changes to support the new offload features:
    [PATCH 1] spi: add core support for controllers with offload
        capabilities
- A couple of changes needed to support the bindings introduced in
  PATCH 4:
    [PATCH 2] scripts: dtc: checks: don't warn on SPI non-peripheral
        child nodes
    [PATCH 3] spi: do not attempt to register DT nodes without @ in name
- Add offload support to the AXI SPI Engine controller driver:
    [PATCH 4] dt-bindings: spi: adi,axi-spi-engine: add offload bindings
    [PATCH 5] spi: axi-spi-engine: add offload support
- A couple of changes needed to support the trigger driver in PATCH 8:
    [PATCH 6] iio: buffer: dmaengine: export
        devm_iio_dmaengine_buffer_alloc()
    [PATCH 7] iio: trigger: add iio_trigger_acquire_by_parent()
- Add a new driver for an offload trigger:
    [PATCH 8] dt-bindings: iio: trigger: add binding for SPI Engine
        Offload PWM trigger
    [PATCH 9] iio: trigger: add AXI SPI Engine offload PWM trigger
- Add offload support to the ADC driver from [3]:
    [PATCH 10] iio: adc: ad7380: add AXI SPI Engine offload support

Note: these changes also depend on the series below. The first 2 have
been applied to spi/for-6.8 and the last is expected to be applied to
iio/testing soon.

[1]: https://lore.kernel.org/linux-spi/[email protected]
[2]: https://lore.kernel.org/linux-spi/[email protected]
[3]: https://lore.kernel.org/linux-iio/[email protected]

# Describe the purpose of this series. The information you put here
# will be used by the project maintainer to make a decision whether
# your patches should be reviewed, and in what priority order. Please be
# very detailed and link to any relevant discussions or sites that the
# maintainer can review to better understand your proposed changes. If you
# only have a single patch in your series, the contents of the cover
# letter will be appended to the "under-the-cut" portion of the patch.

# Lines starting with # will be removed from the cover letter. You can
# use them to add notes or reminders to yourself. If you want to use
# markdown headers in your cover letter, start the line with ">#".

# You can add trailers to the cover letter. Any email addresses found in
# these trailers will be added to the addresses specified/generated
# during the b4 send stage. You can also run "b4 prep --auto-to-cc" to
# auto-populate the To: and Cc: trailers based on the code being
# modified.

To: Mark Brown <[email protected]>
To: Rob Herring <[email protected]>
To: Frank Rowand <[email protected]>
To: Krzysztof Kozlowski <[email protected]>
To: Conor Dooley <[email protected]>
To: Michael Hennerich <[email protected]>
To: Nuno Sá <[email protected]>
To: David Lechner <[email protected]>
To: Jonathan Corbet <[email protected]>
To: Jonathan Cameron <[email protected]>
To: Lars-Peter Clausen <[email protected]>
To: Thierry Reding <[email protected]>
To: Uwe Kleine-König <[email protected]>
Cc:  <[email protected]>
Cc:  <[email protected]>
Cc:  <[email protected]>
Cc:  <[email protected]>
Cc:  <[email protected]>
Cc:  <[email protected]>

--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
  "series": {
    "revision": 1,
    "change-id": "20231215-axi-spi-engine-series-3-1c6a584d408d",
    "prefixes": []
  }
}
This adds a feature for specialized SPI controllers that can record
a series of SPI transfers, including tx data, cs assertions, delays,
etc. and then play them back using a hardware trigger without CPU
intervention.

The intended use case for this is with the AXI SPI Engine to capture
data from ADCs at high rates (MSPS) with a stable sample period.
Additional potential future use cases include SPI controllers with a
lookup table (LUT) like Freescale QuadSPI and NXP FlexSPI. These could
make use of these new offload APIs to enable use of the full LUT (i.e.
offload index would correspond to a LUT index for those controllers).

Most of the implementation is controller-specific and will be handled by
drivers that implement the offload_ops callbacks. The API follows a
prepare/enable pattern that should be familiar to users of the clk
subsystem.

Consumers of this API will make calls similar to this:

    /* in probe() */
    offload = spi_offload_get(spi, index /* e.g. from fwnode */);
    ...

    /* in some setup function */
    ret = spi_offload_prepare(offload, xfers, ARRAY_SIZE(xfers));
    ...

    /* in some enable function */
    ret = spi_offload_enable(offload);
    ...

    /* in corresponding disable function */
    spi_offload_disable();
    ...

    /* in corresponding teardown function */
    spi_offload_unprepare();
    ...

Signed-off-by: David Lechner <[email protected]>
According to the spi-controller.yaml bindings, SPI peripheral child
nodes match the pattern "^.*@[0-9a-f]+$".

A SPI controller binding may require a child object node that is not a
peripheral. For example, the adi,axi-spi-engine binding requires an
"offloads" child node that is not a peripheral but rather a part of the
controller itself.

By checking for '@' in the node name, we can avoids a warnings like:

    Warning (spi_bus_reg): /example-0/spi@44a00000/offloads: missing or empty reg property

for a binding like:

    spi {
        ...

        offloads {
            offload@0 {
                ...
            };
            ...
        };

        peripheral@0 {
            ...
        };
    };

Signed-off-by: David Lechner <[email protected]>
In the DT bindings for SPI devices, it is specified that peripheral
nodes have the @ character in the node name. A SPI controller may need
to create bindings with child nodes that are not peripherals. For
example, the AXI SPI Engine bindings will use an "offloads" child node
to describe what is connected to the offload interfaces of the SPI
controller.

Without this change, the SPI controller would attempt to register all
child nodes as SPI devices. After this change, only nodes with '@' in
the name will be registered as SPI devices.

Signed-off-by: David Lechner <[email protected]>
The ADI AXI SPI Engine driver supports offloading SPI transfers to
hardware. This is essentially a feature that allows recording an
arbitrary sequence of SPI transfers and then playing them back with
no CPU intervention via a hardware trigger.

This adds the bindings for this feature. Each SPI Engine instance
can have from 0 to 32 offload instances. Each offload instance has a
trigger input and a data stream output. Typically, the trigger is
connected to a PWM to determine the sampling rate and the output stream
is connected to a DMA channel to pipe the received data to memory as
shown in the example.

SPI peripherals act as consumers of the offload instances. These could
be assigned in a one-to-one, one-to-many, many-to-one or many-to-many
relationship.

Signed-off-by: David Lechner <[email protected]>
This adds an implementation of the SPI offload_ops to the AXI SPI Engine
driver to provide offload support.

Offload lookup is done by phandle lookup, e.g. from a devicetree. SPI
Engine commands and tx data recorded by writing to offload-specific
FIFOs in the SPI Engine hardware.

Co-developed-by: Lars-Peter Clausen <[email protected]>
Signed-off-by: Lars-Peter Clausen <[email protected]>
Signed-off-by: David Lechner <[email protected]>
This changes devm_iio_dmaengine_buffer_alloc() to an exported symbol.
This will be used by drivers that need to allocate a DMA buffer without
attaching it to an IIO device.

Signed-off-by: David Lechner <[email protected]>
The auxiliary bus uses names in the form of "module_name.device_name"
for matching devices to driver. Since the module name is the actual
module name, it can be quite long which doesn't leave enough room for
the device name.

This patch increases the size AUXILIARY_NAME_SIZE to 64 to allow for
both a ~32 character module name and a ~32 character device name.

Signed-off-by: David Lechner <[email protected]>
This adds a new mode INDIO_HW_BUFFER_TRIGGERED to the IIO subsystem.

This mode is essentially the hardware version of INDIO_BUFFER_TRIGGERED
where the trigger has the semantics of INDIO_HARDWARE_TRIGGERED and the
buffer has the semantics of INDIO_BUFFER_HARDWARE.

So basically INDIO_HW_BUFFER_TRIGGERED is the same as
INDIO_BUFFER_HARDWARE except that it also enables the trigger when the
buffer is enabled.

Signed-off-by: David Lechner <[email protected]>
This adds the new INDIO_HW_BUFFER_TRIGGERED flag to the available modes
of the dmaengine buffer. This allows it to be used as the buffer of
devices that use the INDIO_HW_BUFFER_TRIGGERED flag.

Signed-off-by: David Lechner <[email protected]>
This adds a new hardware triggered buffer driver for the IIO subsystem.
This driver is intended to be used by IIO device drivers that have
a hardware buffer that is triggered by a hardware signal.

It is expected that components such as those providing a backend via the
IIO backend framework will provide the actual implementation of this
functionality by registering a matching device on the auxillary bus.
The auxillary bus was chosen since it allows us to make use of existing
kernel infrastructure instead of implementing our own registration and
lookup system.

Signed-off-by: David Lechner <[email protected]>
This adds a new driver for handling SPI offloading using a PWM as the
trigger and DMA for the received data. This will be used by ADCs in
conjunction with SPI controllers with offloading support to be able
to sample at high rates without CPU intervention.

Signed-off-by: David Lechner <[email protected]>
…gger

This adds a new binding for a PWM trigger and DMA data output connected
to an AXI SPI Engine offload instance.

Signed-off-by: David Lechner <[email protected]>
This extends the ad7380 ADC driver to use the offload capabilities of
the AXI SPI Engine. This adds a hardware trigger and a hardware buffer
to allow sampling a high rates without CPU intervention.

To keep things simple, when this feature is present in hardware we
disable the usual IIO triggered buffer and software timestamp rather
than trying to support multiple buffers.

Signed-off-by: David Lechner <[email protected]>
cherry-picked from ADI tree
@nunojsa
Copy link
Collaborator

nunojsa commented Feb 16, 2024

@dlech can we close this one?

@dlech
Copy link
Collaborator Author

dlech commented Feb 16, 2024

Yes, let's close.

@dlech dlech closed this Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants