lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 10 May 2017 13:38:21 -0500 From: Christopher Bostic <cbostic@...ux.vnet.ibm.com> To: Joel Stanley <joel@....id.au> Cc: Rob Herring <robh+dt@...nel.org>, Mark Rutland <mark.rutland@....com>, Russell King <linux@...linux.org.uk>, rostedt@...dmis.org, mingo@...hat.com, Greg KH <gregkh@...uxfoundation.org>, devicetree <devicetree@...r.kernel.org>, linux-arm-kernel@...ts.infradead.org, Jeremy Kerr <jk@...abs.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Andrew Jeffery <andrew@...id.au>, Alistair Popple <alistair@...ple.id.au>, Benjamin Herrenschmidt <benh@...nel.crashing.org>, "Edward A . James" <eajames@...ibm.com> Subject: Re: [PATCH v6 10/23] drivers/fsi: Add device read/write/peek API On 5/10/17 3:13 AM, Joel Stanley wrote: > On Tue, Apr 11, 2017 at 5:16 AM, Christopher Bostic > <cbostic@...ux.vnet.ibm.com> wrote: >> From: Jeremy Kerr <jk@...abs.org> >> >> This change introduces the fsi device API: simple read, write and peek >> accessors for the devices' address spaces. >> >> Includes contributions from Chris Bostic <cbostic@...ux.vnet.ibm.com> >> and Edward A. James <eajames@...ibm.com>. >> >> Signed-off-by: Edward A. James <eajames@...ibm.com> >> Signed-off-by: Jeremy Kerr <jk@...abs.org> >> Signed-off-by: Chris Bostic <cbostic@...ux.vnet.ibm.com> >> Signed-off-by: Joel Stanley <joel@....id.au> >> --- >> drivers/fsi/fsi-core.c | 33 +++++++++++++++++++++++++++++++++ >> include/linux/fsi.h | 7 ++++++- >> 2 files changed, 39 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c >> index a8faa89..4da0b030 100644 >> --- a/drivers/fsi/fsi-core.c >> +++ b/drivers/fsi/fsi-core.c >> @@ -32,6 +32,8 @@ >> #define FSI_SLAVE_CONF_CRC_MASK 0x0000000f >> #define FSI_SLAVE_CONF_DATA_BITS 28 >> >> +#define FSI_PEEK_BASE 0x410 >> + >> static const int engine_page_size = 0x400; >> >> #define FSI_SLAVE_BASE 0x800 >> @@ -73,9 +75,40 @@ static int fsi_master_read(struct fsi_master *master, int link, >> uint8_t slave_id, uint32_t addr, void *val, size_t size); >> static int fsi_master_write(struct fsi_master *master, int link, >> uint8_t slave_id, uint32_t addr, const void *val, size_t size); >> +static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr, >> + void *val, size_t size); >> +static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr, >> + const void *val, size_t size); > I don't think these >> /* FSI endpoint-device support */ >> >> +int fsi_device_read(struct fsi_device *dev, uint32_t addr, void *val, >> + size_t size) > I suggest adding some comments about the use of fsi_device_read, > fsi_device_write and fsi_device_peek APIs for driver authors. > > One important note is that the data in val must be FSI bus endian (big endian). Hi Joel, OK will add some comments here. > It would be nice to have the sparse notation (eg. __be32) as well. > That doesn't make sense for void *, but we could add some helper > functions like: > > int fsi_device_read32(struct fsi_device *dev, uint32_t addr, __be32 *val) > > int fsi_device_write32(struct fsi_device *dev, uint32_t addr, __be32 val) > > We probably only need a 32 bit version, as all of the reads/writes > being done in users of this function are 32 bit. > What do you think? These functions will need to support reads/writes of 8 bit and 16 bit values. Thanks -Chris >> +{ >> + if (addr > dev->size || size > dev->size || addr > dev->size - size) >> + return -EINVAL; >> + >> + return fsi_slave_read(dev->slave, dev->addr + addr, val, size); >> +} >> +EXPORT_SYMBOL_GPL(fsi_device_read); >> + >> +int fsi_device_write(struct fsi_device *dev, uint32_t addr, const void *val, >> + size_t size) >> +{ >> + if (addr > dev->size || size > dev->size || addr > dev->size - size) >> + return -EINVAL; >> + >> + return fsi_slave_write(dev->slave, dev->addr + addr, val, size); >> +} >> +EXPORT_SYMBOL_GPL(fsi_device_write); >> + >> +int fsi_device_peek(struct fsi_device *dev, void *val) >> +{ >> + uint32_t addr = FSI_PEEK_BASE + ((dev->unit - 2) * sizeof(uint32_t)); >> + >> + return fsi_slave_read(dev->slave, addr, val, sizeof(uint32_t)); >> +} >> + >> static void fsi_device_release(struct device *_device) >> { >> struct fsi_device *device = to_fsi_dev(_device); >> diff --git a/include/linux/fsi.h b/include/linux/fsi.h >> index efa55ba..66bce48 100644 >> --- a/include/linux/fsi.h >> +++ b/include/linux/fsi.h >> @@ -27,6 +27,12 @@ struct fsi_device { >> uint32_t size; >> }; >> >> +extern int fsi_device_read(struct fsi_device *dev, uint32_t addr, >> + void *val, size_t size); >> +extern int fsi_device_write(struct fsi_device *dev, uint32_t addr, >> + const void *val, size_t size); >> +extern int fsi_device_peek(struct fsi_device *dev, void *val); >> + >> struct fsi_device_id { >> u8 engine_type; >> u8 version; >> @@ -40,7 +46,6 @@ struct fsi_device_id { >> #define FSI_DEVICE_VERSIONED(t, v) \ >> .engine_type = (t), .version = (v), >> >> - >> struct fsi_driver { >> struct device_driver drv; >> const struct fsi_device_id *id_table; >> -- >> 1.8.2.2 >>
Powered by blists - more mailing lists