[PATCH v4 3/4] soc: aspeed: Add eSPI driver

ChiaWei Wang chiawei_wang at aspeedtech.com
Fri Sep 10 13:23:26 AEST 2021


Hi Jeremy,

> From: Jeremy Kerr <jk at codeconstruct.com.au>
> Sent: Thursday, September 9, 2021 9:53 AM
>  
> > Yes, there is security concern using HW mode.
> > Our designer is considering to remove the HW mode support in the next
> > generation of Aspeed SoCs.
> > So far we haven't encountered a scenario demanding HW mode.
> 
> Great to hear :) can we unconditionally set ESPI000[9] in the driver then?

Yes for the v5 revision. As HW mode is going to be eliminated in the future.
A brief explanation will also be added.

> 
> > > With than in mind, if we're disabling hardware mode - what does the
> > > direction control setting effect when we're in software mode
> > > (ESPICTRL[9] == 1)? Does it even matter?
> >
> > Yes, the direction matters even in SW mode.
> > When the direction is 'master-to-slave' and the GPIO value is updated
> > by the Host through PUT_VW, a VW interrupt is trigger to notify BMC.
> > For the 'slave-to-master' GPIO, a alert is generated to notify the
> > Host to issue GET_VW for the GPIO value updated by the BMC by ESPI09C.
> 
> OK, but the datasheet mentions that ESPICFG804 is only applicable when
> ESPI000[9] = 0, or is that not the case?

Yes, ESPICFG804 is applicable only on HW mode (ESPI000[9]=0).
When the HW mode is selected, the eSPI slave forwards GPIO update in PUT_VW packet sent by the Host to the physical GPIO based on the ESPICFG804 mapping.
This is purely done by HW. No interrupts will be generated to notify SW.

> 
> But based on what you've said: yes, it sounds like the generic gpiodev parts
> won't be useful for this.
> 
> > > Separate from this: I'm also proposing that we represent the system
> > > event VWs as gpiodevs as well.
> > >
> > > > A raw packet, primitive interface should have better flexibility
> > > > to manage MCTP packets over the OOB channel.
> > >
> > > OK, let me phrase this differently: can the OOB channel be used for
> > > anything other than SMBus messaging? Is it useful to provide an
> > > interface that isn't a standard SMBus/i2c device?
> >
> > Yes, the PCH spec. also defines two additional packet format for an
> > eSPI slave to retrieve PCH Temperature Data and RTC time.
> > It should be trivial to prepare a byte buffer in that format and send
> > it through the raw packet interface.
> 
> OK, understood.
> 
> > > If you need custom uapi definitions for this driver, that might be
> > > okay, but it's going to be more work for you (to define an interface
> > > that can be supported long-term), rather than using standard
> > > infrastructure that already exists.
> >
> > Thus I suggested that we can refer to the IPMI KCS BMC driver, which
> > supports the selection of different user interfaces, RAW or IPMI.
> 
> Yep, but the KCS "raw" register set is standardised as part of the IPMI spec too,
> which helps to define a stable user API. We don't have that in this case.
> 
> Overall though, if you want to start with the "low-level" API, then introduce
> "enhanced" functionality - like an actual SMBus driver - alongside that, then
> that sounds like an OK approach.
> 
> > If IOCTL is considered to be not user friendly or magic code
> > polluting, file-based read/write on the miscdevice node is also an
> > option.
> 
> It's not really my decision to make, but a read/write event interface would
> seem to be more consistent to me. Is there an obvious event format that would
> be common across all channels, perhaps? We'd probably also need a poll too -
> to make use of incoming events, like master-to-slave VW changes, perhaps?

A file-based read/write/poll interface is OK to me as well. The main concern is about the peripheral and the VW channels.
For the peripheral channel, it takes two miscdevice to export TX/RX interfaces for posted and non-posted packets, respectively.
And for the VW channel, the settings GPIO direction is RO and that of GPIO value is RW. And these two should be exported individually.


More information about the openbmc mailing list