From: Jeremy Kerr <[email protected]>
Signed-off-by: Jeremy Kerr <[email protected]>
Signed-off-by: Chris Bostic <[email protected]>
---
drivers/fsi/fsi-core.c | 20 ++++++++++++++++++++
drivers/fsi/fsi-master.h | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+)
create mode 100644 drivers/fsi/fsi-master.h
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 3d55bd5..ce9428d 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -17,6 +17,26 @@
#include <linux/fsi.h>
#include <linux/module.h>
+#include "fsi-master.h"
+
+static atomic_t master_idx = ATOMIC_INIT(-1);
+
+/* FSI master support */
+
+int fsi_master_register(struct fsi_master *master)
+{
+ master->idx = atomic_inc_return(&master_idx);
+ get_device(master->dev);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(fsi_master_register);
+
+void fsi_master_unregister(struct fsi_master *master)
+{
+ put_device(master->dev);
+}
+EXPORT_SYMBOL_GPL(fsi_master_unregister);
+
/* FSI core & Linux bus type definitions */
static int fsi_bus_match(struct device *dev, struct device_driver *drv)
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
new file mode 100644
index 0000000..e75a810
--- /dev/null
+++ b/drivers/fsi/fsi-master.h
@@ -0,0 +1,37 @@
+/*
+ * FSI master definitions. These comprise the core <--> master interface,
+ * to allow the core to interact with the (hardware-specific) masters.
+ *
+ * Copyright (C) IBM Corporation 2016
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef DRIVERS_FSI_MASTER_H
+#define DRIVERS_FSI_MASTER_H
+
+#include <linux/device.h>
+
+struct fsi_master {
+ struct device *dev;
+ int idx;
+ int n_links;
+ int (*read)(struct fsi_master *, int link,
+ uint8_t slave, uint32_t addr,
+ void *val, size_t size);
+ int (*write)(struct fsi_master *, int link,
+ uint8_t slave, uint32_t addr,
+ const void *val, size_t size);
+};
+
+extern int fsi_master_register(struct fsi_master *master);
+extern void fsi_master_unregister(struct fsi_master *master);
+
+#endif /* DRIVERS_FSI_MASTER_H */
--
1.8.2.2
From: Jeremy Kerr <[email protected]>
This change introduces the fsi device API: simple read, write and peek
accessors for the devices' address spaces.
Includes contributions from Chris Bostic <[email protected]>
Signed-off-by: Jeremy Kerr <[email protected]>
Signed-off-by: Chris Bostic <[email protected]>
---
drivers/fsi/fsi-core.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fsi.h | 7 ++++++-
2 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index b51ea35..80feeb8 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -33,6 +33,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;
static atomic_t master_idx = ATOMIC_INIT(-1);
@@ -46,8 +48,46 @@ struct fsi_slave {
#define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
+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);
+
/* FSI endpoint-device support */
+int fsi_device_read(struct fsi_device *dev, uint32_t addr, void *val,
+ size_t size)
+{
+ if (addr > dev->size)
+ return -EINVAL;
+
+ if (addr + size > dev->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)
+ return -EINVAL;
+
+ if (addr + size > dev->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);
@@ -100,6 +140,13 @@ static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
slave->id, addr, val, size);
}
+static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
+ const void *val, size_t size)
+{
+ return slave->master->write(slave->master, slave->link,
+ slave->id, addr, val, size);
+}
+
static int fsi_slave_scan(struct fsi_slave *slave)
{
uint32_t engine_addr;
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
From: Chris Bostic <[email protected]>
Enable each link and send a break command in preparation
for scanning each link for slaves.
Signed-off-by: Chris Bostic <[email protected]>
---
drivers/fsi/fsi-core.c | 39 ++++++++++++++++++++++++++++++++++++---
drivers/fsi/fsi-master.h | 2 ++
2 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 80feeb8..93de0f1 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -290,16 +290,49 @@ static int fsi_slave_init(struct fsi_master *master,
/* FSI master support */
+static int fsi_master_link_enable(struct fsi_master *master, int link)
+{
+ if (master->link_enable)
+ return master->link_enable(master, link);
+
+ return 0;
+}
+
+/*
+ * Issue a break command on this link
+ */
+static int fsi_master_break(struct fsi_master *master, int link)
+{
+ if (master->send_break)
+ return master->send_break(master, link);
+
+ return 0;
+}
+
static int fsi_master_scan(struct fsi_master *master)
{
- int link, slave_id;
+ int link, slave_id, rc;
+
+ for (link = 0; link < master->n_links; link++) {
+ rc = fsi_master_link_enable(master, link);
+ if (rc) {
+ dev_dbg(master->dev,
+ "enable link:%d failed with:%d\n", link, rc);
+ continue;
+ }
+ rc = fsi_master_break(master, link);
+ if (rc) {
+ dev_dbg(master->dev,
+ "Break to link:%d failed with:%d\n", link, rc);
+ continue;
+ }
- for (link = 0; link < master->n_links; link++)
for (slave_id = 0; slave_id < FSI_N_SLAVES; slave_id++)
fsi_slave_init(master, link, slave_id);
- return 0;
+ }
+ return 0;
}
int fsi_master_register(struct fsi_master *master)
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index cafb433..56aad0e 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -29,6 +29,8 @@ struct fsi_master {
int (*write)(struct fsi_master *, int link,
uint8_t slave, uint32_t addr,
const void *val, size_t size);
+ int (*send_break)(struct fsi_master *, int link);
+ int (*link_enable)(struct fsi_master *, int link);
};
extern int fsi_master_register(struct fsi_master *master);
--
1.8.2.2
From: Jeremy Kerr <[email protected]>
Create fsi_slave devices during the master scan.
Signed-off-by: Jeremy Kerr <[email protected]>
Signed-off-by: Chris Bostic <[email protected]>
---
drivers/fsi/fsi-core.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 53 insertions(+), 2 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index f0832c7..aa4330a 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -16,10 +16,14 @@
#include <linux/device.h>
#include <linux/fsi.h>
#include <linux/module.h>
+#include <linux/slab.h>
#include "fsi-master.h"
#define FSI_N_SLAVES 4
+#define FSI_SLAVE_CONF_CRC_SHIFT 4
+#define FSI_SLAVE_CONF_CRC_MASK 0x0000000f
+#define FSI_SLAVE_CONF_DATA_BITS 28
static atomic_t master_idx = ATOMIC_INIT(-1);
@@ -54,12 +58,59 @@ uint8_t fsi_crc4(uint8_t c, uint64_t x, int bits)
EXPORT_SYMBOL_GPL(fsi_crc4);
/* FSI slave support */
+
+static void fsi_slave_release(struct device *dev)
+{
+ struct fsi_slave *slave = to_fsi_slave(dev);
+
+ kfree(slave);
+}
+
static int fsi_slave_init(struct fsi_master *master,
int link, uint8_t slave_id)
{
- /* todo: initialise slave device, perform engine scan */
+ struct fsi_slave *slave;
+ uint32_t chip_id;
+ int rc;
+ uint8_t crc;
+
+ rc = master->read(master, link, slave_id, 0, &chip_id, sizeof(chip_id));
+ if (rc) {
+ dev_warn(master->dev, "can't read slave %02x:%02x: %d\n",
+ link, slave_id, rc);
+ return -ENODEV;
+ }
+ crc = fsi_crc4(0, chip_id >> FSI_SLAVE_CONF_CRC_SHIFT,
+ FSI_SLAVE_CONF_DATA_BITS);
+ if (crc != (chip_id & FSI_SLAVE_CONF_CRC_MASK)) {
+ dev_warn(master->dev, "slave %02x:%02x invalid chip id CRC!\n",
+ link, slave_id);
+ return -EIO;
+ }
+
+ pr_debug("fsi: found chip %08x at %02x:%02x:%02x\n",
+ master->idx, chip_id, link, slave_id);
+
+ /* we can communicate with a slave; create devices and scan */
+ slave = kzalloc(sizeof(*slave), GFP_KERNEL);
+ if (!slave)
+ return -ENOMEM;
+
+ slave->master = master;
+ slave->id = slave_id;
+ slave->dev.parent = master->dev;
+ slave->dev.release = fsi_slave_release;
+
+ dev_set_name(&slave->dev, "slave@%02x:%02x", link, slave_id);
+ rc = device_register(&slave->dev);
+ if (rc < 0) {
+ dev_warn(master->dev, "failed to create slave device: %d\n",
+ rc);
+ put_device(&slave->dev);
+ return rc;
+ }
- return -ENODEV;
+ return rc;
}
/* FSI master support */
--
1.8.2.2
From: Jeremy Kerr <[email protected]>
Add some helpers for the crc checks for the slave configuration table.
This works 4-bits-at-a-time, using a simple table approach.
We will need this in the FSI core code, as well as any master
implementations that need to calculate CRCs in software.
Signed-off-by: Jeremy Kerr <[email protected]>
Signed-off-by: Chris Bostic <[email protected]>
---
drivers/fsi/fsi-core.c | 21 +++++++++++++++++++++
drivers/fsi/fsi-master.h | 21 +++++++++++++++++++++
2 files changed, 42 insertions(+)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index ceaf536..f0832c7 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -32,6 +32,27 @@ struct fsi_slave {
#define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
+/* crc helpers */
+static const uint8_t crc4_tab[] = {
+ 0x0, 0x7, 0xe, 0x9, 0xb, 0xc, 0x5, 0x2,
+ 0x1, 0x6, 0xf, 0x8, 0xa, 0xd, 0x4, 0x3,
+};
+
+uint8_t fsi_crc4(uint8_t c, uint64_t x, int bits)
+{
+ int i;
+
+ /* Align to 4-bits */
+ bits = (bits + 3) & ~0x3;
+
+ /* Calculate crc4 over four-bit nibbles, starting at the MSbit */
+ for (i = bits; i >= 0; i -= 4)
+ c = crc4_tab[c ^ ((x >> i) & 0xf)];
+
+ return c;
+}
+EXPORT_SYMBOL_GPL(fsi_crc4);
+
/* FSI slave support */
static int fsi_slave_init(struct fsi_master *master,
int link, uint8_t slave_id)
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index e75a810..cafb433 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -34,4 +34,25 @@ struct fsi_master {
extern int fsi_master_register(struct fsi_master *master);
extern void fsi_master_unregister(struct fsi_master *master);
+/**
+ * crc4 helper: Given a starting crc4 state @c, calculate the crc4 vaue of @x,
+ * which is @bits in length. This may be required by master implementations
+ * that do not provide their own hardware checksums.
+ *
+ * The crc4 is performed on 4-bit chunks (which is all we need for FSI
+ * calculations). Typically, we'll want a starting state of 0:
+ *
+ * c = fsi_crc4(0, msg, len);
+ *
+ * To crc4 a message that includes a single start bit, initialise crc4 state
+ * with:
+ *
+ * c = fsi_crc4(0, 1, 1);
+ *
+ * Then update with message data:
+ *
+ * c = fsi_crc4(c, msg, len);
+ */
+uint8_t fsi_crc4(uint8_t c, uint64_t x, int bits);
+
#endif /* DRIVERS_FSI_MASTER_H */
--
1.8.2.2
On Tue, Dec 06, 2016 at 08:09:30PM -0600, Chris Bostic wrote:
> From: Jeremy Kerr <[email protected]>
>
> Signed-off-by: Jeremy Kerr <[email protected]>
> Signed-off-by: Chris Bostic <[email protected]>
> ---
> drivers/fsi/fsi-core.c | 20 ++++++++++++++++++++
> drivers/fsi/fsi-master.h | 37 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 57 insertions(+)
> create mode 100644 drivers/fsi/fsi-master.h
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 3d55bd5..ce9428d 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -17,6 +17,26 @@
> #include <linux/fsi.h>
> #include <linux/module.h>
>
> +#include "fsi-master.h"
> +
> +static atomic_t master_idx = ATOMIC_INIT(-1);
You don't really want/need an atomic variable, please use the simple ida
interface instead.
thanks,
greg k-h
On Tue, Dec 06, 2016 at 08:09:31PM -0600, Chris Bostic wrote:
> From: Jeremy Kerr <[email protected]>
>
> Add some helpers for the crc checks for the slave configuration table.
> This works 4-bits-at-a-time, using a simple table approach.
>
> We will need this in the FSI core code, as well as any master
> implementations that need to calculate CRCs in software.
>
> Signed-off-by: Jeremy Kerr <[email protected]>
> Signed-off-by: Chris Bostic <[email protected]>
> ---
> drivers/fsi/fsi-core.c | 21 +++++++++++++++++++++
> drivers/fsi/fsi-master.h | 21 +++++++++++++++++++++
> 2 files changed, 42 insertions(+)
Why not just create lib/crc4.c with these functions, like the other crc
functions in the kernel? Don't bury these in some random driver
subsystem please.
thanks,
greg k-h
On Tue, Dec 06, 2016 at 08:09:33PM -0600, Chris Bostic wrote:
> 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;
Strange whitespace change here :)
Not a real problem, I like the fact that you have broken this down into
very logical pieces making it much easier to review, thanks so much for
doing this.
greg k-h
Hi Greg,
> Why not just create lib/crc4.c with these functions, like the other crc
> functions in the kernel?
Two (bad) reasons:
- The crc4 implementation here is pretty specific to the FSI
usage (only supporting 4-bit-sized chunks), to keep the math & lookup
table simple
- I'm lazy
So yes, we should spend the effort now to make this generic enough for
a lib/crc4.c. Would we want to support different values for the
polynomial?
Chris: do you want me to to that, or will you?
Cheers,
Jeremy
On Wed, Dec 7, 2016 at 5:33 PM, Jeremy Kerr <[email protected]> wrote:
> Hi Greg,
>
>> Why not just create lib/crc4.c with these functions, like the other crc
>> functions in the kernel?
>
> Two (bad) reasons:
>
> - The crc4 implementation here is pretty specific to the FSI
> usage (only supporting 4-bit-sized chunks), to keep the math & lookup
> table simple
>
> - I'm lazy
>
> So yes, we should spend the effort now to make this generic enough for
> a lib/crc4.c. Would we want to support different values for the
> polynomial?
>
> Chris: do you want me to to that, or will you?
Hi Jeremy,
I'll take this one. Will implement as per Greg's suggestions.
Thanks,
Chris
>
> Cheers,
>
>
> Jeremy
On Wed, Dec 7, 2016 at 3:06 AM, Greg KH <[email protected]> wrote:
> On Tue, Dec 06, 2016 at 08:09:30PM -0600, Chris Bostic wrote:
>> From: Jeremy Kerr <[email protected]>
>>
>> Signed-off-by: Jeremy Kerr <[email protected]>
>> Signed-off-by: Chris Bostic <[email protected]>
>> ---
>> drivers/fsi/fsi-core.c | 20 ++++++++++++++++++++
>> drivers/fsi/fsi-master.h | 37 +++++++++++++++++++++++++++++++++++++
>> 2 files changed, 57 insertions(+)
>> create mode 100644 drivers/fsi/fsi-master.h
>>
>> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
>> index 3d55bd5..ce9428d 100644
>> --- a/drivers/fsi/fsi-core.c
>> +++ b/drivers/fsi/fsi-core.c
>> @@ -17,6 +17,26 @@
>> #include <linux/fsi.h>
>> #include <linux/module.h>
>>
>> +#include "fsi-master.h"
>> +
>> +static atomic_t master_idx = ATOMIC_INIT(-1);
>
> You don't really want/need an atomic variable, please use the simple ida
> interface instead.
Greg,
Will make the change to simple ida interface.
Thanks for your feedback,
Chris
>
> thanks,
>
> greg k-h