openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Jeremy Kerr <jk@codeconstruct.com.au>
To: Chia-Wei Wang <chiawei_wang@aspeedtech.com>,
	robh+dt@kernel.org,  joel@jms.id.au, andrew@aj.id.au,
	linux-aspeed@lists.ozlabs.org,  openbmc@lists.ozlabs.org,
	devicetree@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Cc: ryan_chen@aspeedtech.com
Subject: Re: [PATCH v4 3/4] soc: aspeed: Add eSPI driver
Date: Thu, 02 Sep 2021 11:29:35 +0800	[thread overview]
Message-ID: <20c13b9bb023091758cac3a07fb4037b7d796578.camel@codeconstruct.com.au> (raw)
In-Reply-To: <20210901033015.910-4-chiawei_wang@aspeedtech.com>

Hi Chiawei,

> The Aspeed eSPI controller is slave device to communicate with
> the master through the Enhanced Serial Peripheral Interface (eSPI).
> All of the four eSPI channels, namely peripheral, virtual wire,
> out-of-band, and flash are supported.

I'm still not confinced this raw packet user-ABI is the right approach
for this. The four channels that you're exposing would be much more
useful to use existing kernel subsystems.

Specifically:

1) The VW channel is essentially a GPIO interface, and we have a kernel
   subsystem to expose GPIOs. We might want to add additional support
   for in-kernel system event handlers, but that's a minor addition that
   can be done separately if we don't want that handled by a gpio
   consumer in userspace.

2) The out-of-band (OOB) channel provides a way to issue SMBus
   transactions, so could well provide an i2c controller interface.
   Additionally, the eSPI specs imply that this is intended to support
   MCTP - in which case, you'll *need* a kernel i2c controller device to
   be able to use the new kernel MCTP stack.

3) The flash channel exposes read/write/erase operations, which would be
   much more useful as an actual flash-type device, perhaps using the
   existing mtd interface? Or is there additional functionality
   expected for this?

4) The peripheral channel is the only one that would seem to require
   arbitrary cycle access, but we'll still need a proper uapi definition
   to support that. At the minimum, your ioctl definitions should go
   under include/uapi/ - you shouldn't need to duplicate the header into
   each userspace repo, as you've done for the test examples.

Cheers,


Jeremy


  reply	other threads:[~2021-09-02  3:30 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 [this message]
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
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=20c13b9bb023091758cac3a07fb4037b7d796578.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=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).