<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;"><pre>Hi Chia-Wei,</pre><pre><br></pre><pre>> The Aspeed eSPI controller is slave device to communicate with</pre><pre>> the master through the Enhanced Serial Peripheral Interface (eSPI).</pre><pre>> All of the four eSPI channels, namely peripheral, virtual wire,</pre><pre>> out-of-band, and flash are supported.</pre><pre><br></pre><pre>Great to have this added submitted upstream! A few comments though:</pre><pre><br></pre><pre>> ---</pre><pre>> drivers/soc/aspeed/Kconfig | 11 +</pre><pre>> drivers/soc/aspeed/Makefile | 1 +</pre><pre>> drivers/soc/aspeed/aspeed-espi-ctrl.c | 205 +++++++++</pre><pre>> drivers/soc/aspeed/aspeed-espi-ctrl.h | 304 ++++++++++++</pre><pre>> drivers/soc/aspeed/aspeed-espi-flash.h | 380 +++++++++++++++</pre><pre>> drivers/soc/aspeed/aspeed-espi-ioc.h | 153 +++++++</pre><pre>> drivers/soc/aspeed/aspeed-espi-oob.h | 611 +++++++++++++++++++++++++</pre><pre>> drivers/soc/aspeed/aspeed-espi-perif.h | 539 ++++++++++++++++++++++</pre><pre>> drivers/soc/aspeed/aspeed-espi-vw.h | 142 ++++++</pre><pre><br></pre><pre>This structure is a bit odd - you have the one -crtl.c file, which</pre><pre>defines the actual driver, but then a bunch of headers that contain more</pre><pre>code than header-type definitions.</pre><pre><br></pre><pre>Is there any reason that -flash, -ioc, -oob, -perif and -vw components</pre><pre>can't be standard .c files?</pre><pre><br></pre><pre>Then, for the userspace ABI: it looks like you're exposing everything</pre><pre>through new device-specific ioctls. Would it not make more sense to use</pre><pre>existing interfaces? For example, the virtual wire bits could be regular</pre><pre>GPIOs; the flash interface could be a mtd or block device.</pre><pre><br></pre><pre>I understand that we'll likely still need some level of custom device</pre><pre>control, but the more we can use generic interfaces for, the less custom</pre><pre>code (and interfaces) we'll need on the userspace side.</pre><pre><br></pre><pre>Cheers,</pre><pre><br></pre><pre><br></pre><pre>Jeremy</pre></body></html>