Re: [RFC PATCH 0/4] espi: introduce eSPI bus framework

"M, Krishnamoorthi" <[email protected]>
Newsgroups org.kernel.vger.linux-doc,dev.linux.lists.chrome-platform,org.kernel.vger.linux-kernel,org.kernel.vger.linux-spi
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.