Re: [RFC PATCH 0/4] espi: introduce eSPI bus framework
"M, Krishnamoorthi" <[email protected]>
| Newsgroups | org.ozlabs.lists.openbmc,dev.linux.lists.chrome-platform,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kernel.vger.linux-spi,org.ozlabs.lists.linux-aspeed |
|---|---|
| Message-ID | <[email protected]> |
Hi Chung,
On 8/10/2026 11:43 AM, YH Chung wrote:
> Hi Krishnamoorthi,
>
>> YH Chung, would you be open to collaborating on the slave-side
>> interfaces of the new eSPI framework? Happy to discuss further on the
>> list or off-list to align on the design before the next revision.
>
> Thanks for reaching out. I am glad to share some design considerations
> from a target-side point of view.
>
> After reading the series, I think it might be useful to align on the
> layering between the eSPI core, channel implementations, and hardware
> drivers as part of defining the target-side interfaces.
>
> 1. Reuse existing kernel subsystems for the individual channels
>
> I agree that we should reuse existing kernel subsystems where their
> semantics match, for example GPIO for general-purpose Virtual Wire
> groups, MCTP for MCTP-over-OOB, and MTD for flash access. Other Virtual
> Wire groups and OOB protocols may need different consumers.
>
> The eSPI subsystem could provide common adapters between those
> subsystems and the corresponding channel instead of requiring each
> controller or target hardware driver to implement the integration
> independently.
>
> For example, Controller Attached Flash Sharing (CAFS) and Target
> Attached Flash Sharing (TAFS) have different ownership and transaction
> directions, but share the eSPI Flash packet and request/completion
> semantics. A common Flash layer could provide that protocol handling,
> with a requester-side MTD frontend that turns MTD operations into eSPI
> requests and a provider-side backend that services requests using
> locally attached flash.
Reusing GPIO for VWire, MCTP for OOB, and MTD for Flash where semantics
match is the right direction.
For CAFS/TAFS, the same espi-flash channel type is used on both sides: a
requester role exposing an MTD frontend (controller owns flash), and a
provider role servicing requests from local flash (target owns flash).
It's the same channel/protocol layer and packet/completion semantics on
each side; on any given endpoint, only one role is active.
>
> 2. Consider the boundary between channel semantics and hardware transport
>
> The current struct espi_controller_ops exposes high-level operations
> such as periph_io_read(), oob_send(), and flash_read(). These may be
> useful as channel-consumer APIs, but I wonder whether they are too
> high-level for the hardware-driver interface itself.
>
> Would it make sense to keep high-level behavior in the channel layers
> while defining the hardware-facing boundary in terms of per-channel
> transmit and receive primitives?
>
> Conceptually:
>
> TX: core/channel -> *_tx() -> hardware
> RX: hardware IRQ -> espi_*_rx() -> core/channel
>
> The packet or request structure could remain channel-specific. The main
> idea is to keep register, FIFO, and DMA handling below this boundary and
> eSPI channel semantics above it.
>
> For example, a controller read from target-attached flash could use the
> same transport interface on both sides:
>
> Controller (requester) Target (flash owner)
> ---------------------- --------------------
>
> MTD frontend
> |
> Flash layer
> |
> build READ request
> |
> flash_tx() ------- READ -------> espi_flash_rx()
> |
> Flash provider
> |
> local MTD read
> |
> espi_flash_rx() <-- COMPLETION --- flash_tx()
> |
> match request
> |
> complete MTD read
>
> One option would be for controller and target drivers to use the same
> low-level endpoint interface, including the same per-channel *_tx()
> callbacks and espi_*_rx() entry points. A common endpoint object could
> carry the role, for example:
>
> enum espi_role {
> ESPI_ROLE_CONTROLLER,
> ESPI_ROLE_TARGET,
> };
>
> The endpoint role and capabilities would determine which transaction
> types are valid and which optional operations are implemented. Object
> lifetime, capabilities, packet definitions, and request state could
> also be shared. Linux SPI's shared spi_controller infrastructure for
> host and target roles may be a useful reference, although eSPI's
> role-specific protocol behavior is more asymmetric.
>
> The benefit of this boundary is that common packet and channel protocol
> handling can be implemented once while each hardware driver remains
> focused on its registers, FIFOs, DMA, and interrupts. It should reduce
> duplication and role-specific divergence, make support for additional
> controller or target hardware easier to add, and allow both roles to be
> tested against the same transport contract.
>
> Channel-independent commands such as GET/SET_CONFIGURATION and
> GET_STATUS may likewise need role-specific callbacks within this common
> endpoint interface: command submission/completion on the controller
> side and configuration-provider callbacks on the target side. These
> callbacks could be optional to support hardware-assisted
> implementations.
The TX/RX boundary is a clean separation and we agree hardware drivers
should focus purely on registers, FIFOs and DMA while channel semantics
live above. That means the current high-level ops (flash_read(),
oob_send(), ...) move up into the channel layers, and the hardware
boundary becomes your per-channel *_tx() / espi_*_rx() primitives.
As an alternative to a shared espi_role endpoint, we are considering one
device per channel under the target (CS#), single target shown for
clarity:
espi0 (controller)
`-- espi0-cs0 (target at Chip Select 0)
|-- espi0-cs0-periph -> I/O + memory
|-- espi0-cs0-vwire -> gpiochip (GP VWire groups; system
| VWires handled separately)
|-- espi0-cs0-oob -> MCTP
`-- espi0-cs0-flash -> MTD
Both controller and target drivers populate the same per-channel ops and
the role (requester/provider) stays local to each channel driver and
decides direction and which optional callbacks exist. The
channel-independent commands (GET/SET_CONFIGURATION, GET_STATUS) are
link-level, so they stay on the controller device with role-specific
callbacks. This gives the same separation as your endpoint model.
>
> 3. Leave room for asynchronous deferred transaction handling
>
> This does not necessarily need to be implemented in the initial
> framework. A synchronous API may be a practical first step, provided
> the hardware-facing interface does not prevent asynchronous handling
> from being added later.
>
> As a future improvement, the common channel layer could track
> outstanding non-posted Peripheral and Flash requests and complete them
> when the corresponding completion packets arrive. For tagged requests,
> this state would be scoped by endpoint/Chip Select#, channel, and tag.
> A complete request object would also need to handle split completions,
> timeouts, errors, and cancellation during channel or link reset.
>
Agreed on synchronous first. We will keep the boundary open for later
async tracking of non-posted requests, scoped by CS#/channel/tag.
Thanks for the detailed design considerations. Do you have any
comments/suggestions about the per channel approach as a starting point?
Happy to continue on or off the list.
Thanks,
Krishna
> The exact model could remain channel-specific because OOB is
> message-oriented and Virtual Wire is event/state-oriented. Keeping this
> possibility open would allow asynchronous handling to be added later
> without changing the hardware-driver interface.
>
> These are my initial thoughts from the target-side implementation
> perspective. I hope they are useful when considering the layering and
> public interfaces for the next revision, and I would be interested in
> your thoughts on the proposed transport boundary and common endpoint
> model.
>
> Regards,
> Yun-Hsuan Chung