openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Jeremy Kerr <jk@codeconstruct.com.au>
To: ChiaWei Wang <chiawei_wang@aspeedtech.com>,
	"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: Mon, 06 Sep 2021 11:16:44 +0800	[thread overview]
Message-ID: <6593206c0bc90186f255c6ea86339576576f70dc.camel@codeconstruct.com.au> (raw)
In-Reply-To: <HK0PR06MB37799C48533B084CF864E49D91D29@HK0PR06MB3779.apcprd06.prod.outlook.com>

Hi Chiawei,

> > If that model doesn't fit though, that's OK, but I think we need
> > some rationale
> > there.
> 
> After an internal discussion, we found that our eSPI VW device may
> not fit into existing GPIO model.
> 
> The reason is that GPIO direction changes through VW channel has no
> interrupts triggered.
> And the direction is controlled by the Host as aforementioned.

This piqued my curiosity, so I had a look through the 2500 datasheet. It
appears that the host has full control of both the direction *and*
hardware GPIO assignment though the platform-specific eSPI configuration
register set.

So, with VW GPIOs in hardware mode (ESPICTRL[9] = 0, the default), the
host has arbitrary control of all hardware GPIO lines (except for
the GPIOAC bank, I guess?).

There's a huge security implication there - for example, GPIOs that
assert physical presence can now be set by the host, possibly remotely -
so I'd *strongly* recommend that we always get ESPICTRL[9] to 1, to set
software-only mode.

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?

For example, what happens when the host goes a GET_VW cycle for a GPIO
that is marked as 'master-to-slave' mode? Is the state of the GPIO in
ESPI09C still reported?

If that's the case, then we can just ignore the direction settings from
ESPICFG800 completely, and have the BMC assign directions to standard
gpiodevs as appropriate.

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?

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.

Cheers,


Jeremy


  reply	other threads:[~2021-09-06  3:17 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 [this message]
2021-09-08  9:16             ` ChiaWei Wang
2021-09-09  1:52               ` Jeremy Kerr
2021-09-10  3:23                 ` ChiaWei Wang
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=6593206c0bc90186f255c6ea86339576576f70dc.camel@codeconstruct.com.au \
    --to=jk@codeconstruct.com.au \
    --cc=andrew@aj.id.au \
    --cc=chiawei_wang@aspeedtech.com \
    --cc=devicetree@vger.kernel.org \
    --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).