Re: [PATCH v6 10/23] drivers/fsi: Add device read/write/peek API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






On 5/10/17 3:13 AM, Joel Stanley wrote:
On Tue, Apr 11, 2017 at 5:16 AM, Christopher Bostic
<cbostic@xxxxxxxxxxxxxxxxxx> wrote:
From: Jeremy Kerr <jk@xxxxxxxxxx>

This change introduces the fsi device API: simple read, write and peek
accessors for the devices' address spaces.

Includes contributions from Chris Bostic <cbostic@xxxxxxxxxxxxxxxxxx>
and Edward A. James <eajames@xxxxxxxxxx>.

Signed-off-by: Edward A. James <eajames@xxxxxxxxxx>
Signed-off-by: Jeremy Kerr <jk@xxxxxxxxxx>
Signed-off-by: Chris Bostic <cbostic@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Joel Stanley <joel@xxxxxxxxx>
---
  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


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux