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

[YH Chung]

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.

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.

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.

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