openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: ChiaWei Wang <chiawei_wang@aspeedtech.com>
To: Jeremy Kerr <jk@codeconstruct.com.au>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"joel@jms.id.au" <joel@jms.id.au>,
	"andrew@aj.id.au" <andrew@aj.id.au>,
	"linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
	"openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: Morris Mao <morris_mao@aspeedtech.com>,
	Ryan Chen <ryan_chen@aspeedtech.com>
Subject: RE: [PATCH v4 3/4] soc: aspeed: Add eSPI driver
Date: Fri, 10 Sep 2021 03:23:26 +0000	[thread overview]
Message-ID: <HK0PR06MB37793C268FEB0E33A5547D0791D69@HK0PR06MB3779.apcprd06.prod.outlook.com> (raw)
In-Reply-To: <9fa4ae962c185e0e4f07f0299356969c17ae5ea5.camel@codeconstruct.com.au>

Hi Jeremy,

> From: Jeremy Kerr <jk@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.

  reply	other threads:[~2021-09-10  3:24 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-01  3:30 [PATCH v4 0/4] arm: aspeed: Add eSPI support Chia-Wei Wang
2021-09-01  3:30 ` [PATCH v4 1/4] dt-bindings: aspeed: Add eSPI controller Chia-Wei Wang
2021-09-01  3:30 ` [PATCH v4 2/4] MAINTAINER: Add ASPEED eSPI driver entry Chia-Wei Wang
2021-09-01  3:30 ` [PATCH v4 3/4] soc: aspeed: Add eSPI driver Chia-Wei Wang
2021-09-02  3:29   ` Jeremy Kerr
2021-09-02  6:44     ` ChiaWei Wang
2021-09-02  7:04       ` Jeremy Kerr
2021-09-06  1:19         ` ChiaWei Wang
2021-09-06  3:16           ` Jeremy Kerr
2021-09-08  9:16             ` ChiaWei Wang
2021-09-09  1:52               ` Jeremy Kerr
2021-09-10  3:23                 ` ChiaWei Wang [this message]
2021-09-01  3:30 ` [PATCH v4 4/4] ARM: dts: aspeed: Add eSPI node Chia-Wei Wang
2021-11-10 11:21   ` Andrei Kartashev
2021-11-11  1:55     ` ChiaWei Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=HK0PR06MB37793C268FEB0E33A5547D0791D69@HK0PR06MB3779.apcprd06.prod.outlook.com \
    --to=chiawei_wang@aspeedtech.com \
    --cc=andrew@aj.id.au \
    --cc=devicetree@vger.kernel.org \
    --cc=jk@codeconstruct.com.au \
    --cc=joel@jms.id.au \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=morris_mao@aspeedtech.com \
    --cc=openbmc@lists.ozlabs.org \
    --cc=robh+dt@kernel.org \
    --cc=ryan_chen@aspeedtech.com \
    --subject='RE: [PATCH v4 3/4] soc: aspeed: Add eSPI driver' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).