diff mbox

[linux,14/15] drivers/fsi: Rename slave to cfam

Message ID 1475700868-75737-15-git-send-email-christopher.lee.bostic@gmail.com
State Superseded, archived
Headers show

Commit Message

christopher.lee.bostic@gmail.com Oct. 5, 2016, 8:54 p.m. UTC 
From: Chris Bostic <cbostic@us.ibm.com>

Suggested changes to the patch set so far as provided by Jeremy Kerr.
Rename slave to cfam to distinguish between the slave engine and its
containing cfam.  A few formatting changes to make checkpatch accept
patches 0001-0013.

Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
---
 drivers/fsi/fsi-cfam.h        |  71 +++++++++++++++++++++
 drivers/fsi/fsi-core.c        | 144 ++++++++++++++++++------------------------
 drivers/fsi/fsi-master-fake.c |   4 +-
 drivers/fsi/fsi-master-gpio.c |   4 +-
 drivers/fsi/fsi-master.h      |   4 +-
 include/linux/fsi.h           |   2 +-
 6 files changed, 140 insertions(+), 89 deletions(-)
 create mode 100644 drivers/fsi/fsi-cfam.h

Comments

Jeremy Kerr Oct. 6, 2016, 6:58 a.m. UTC | #1 
Hi Chris,

> Suggested changes to the patch set so far as provided by Jeremy Kerr.
> Rename slave to cfam to distinguish between the slave engine and its
> containing cfam.

I'm still not 100% on that. As is my understanding, a (hardware) CFAM
contains multiple slaves, and that not what `struct fsi_slave`
represents (it's a single slave).

> A few formatting changes to make checkpatch accept
> patches 0001-0013.

Those would be better split out. If you like, I can then take that
change and re-roll my original skeleton series with that incorporated.

Cheers,


Jeremy
christopher.lee.bostic@gmail.com Oct. 6, 2016, 1:41 p.m. UTC | #2 
On Thu, Oct 6, 2016 at 1:58 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>> Suggested changes to the patch set so far as provided by Jeremy Kerr.
>> Rename slave to cfam to distinguish between the slave engine and its
>> containing cfam.
>
> I'm still not 100% on that. As is my understanding, a (hardware) CFAM
> contains multiple slaves, and that not what `struct fsi_slave`
> represents (it's a single slave).

In redundant configurations yes there are two slaves.  However, any
controlling device driver would only ever use one of them.  A redundant
system with its own fsi device driver would be controlling the other slave.

I still maintain that there is bound to be confusion with slave->read( )
etc... which are meant for accessing more than just the slave engine.
cfam->read makes it more clear I think that you are accessing some
range within the cfam, not just the slave engine.  A cfam acts as a slave
in a general sense but it doesn't distinguish itself from the slave engine.

If this is a big sticking point I'll rename it to slave and deal with the
potential ambiguity that may come up later. I'd like to move ahead
with the rest of the implementation.

>
>> A few formatting changes to make checkpatch accept
>> patches 0001-0013.
>
> Those would be better split out. If you like, I can then take that
> change and re-roll my original skeleton series with that incorporated.
>

I'm not sure what split out means in this case.  Git am applies each patch
in sequence so those warnings will be hit before any patches I add to reverse
something are applied.

I know we had discussed last week about making it clear what changes
you had made versus mine.  As per discussion I left your changes as
is in the patch series, then made my recommended changes as the
next in the series.   A bit confusing on how to deal with checkpatch,
git am, ... in those circumstances.

> Cheers,
>
>
> Jeremy
christopher.lee.bostic@gmail.com Oct. 6, 2016, 6:50 p.m. UTC | #3 
On Thu, Oct 6, 2016 at 1:58 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>> Suggested changes to the patch set so far as provided by Jeremy Kerr.
>> Rename slave to cfam to distinguish between the slave engine and its
>> containing cfam.
>
> I'm still not 100% on that. As is my understanding, a (hardware) CFAM
> contains multiple slaves, and that not what `struct fsi_slave`
> represents (it's a single slave).

One other thing that would make a cfam struct that is separate from the
slave engine useful is the fact that there are several different hardware
quirks for which the only way for firmware to know what to do is to check
the cfam type.

P8, P9, zNEW cfam types

* Controlling master's MATRB bit encoding is different versus other cfams.
This is not a function of the master type, only cfam type.

   I'd propose something like:



   uint32_t cfam->decode_matrb(master->matrb)   possibly.

* Any master that has cascaded masters downstream must set its
MSIEP registers differently than if there was no cascaded master.
This is to work around a known hardware bug in interrupt signaling.
The only way to determine this is to see if downstream cfam is a type
that has a cascaded master.  Again independent of a controlling
master's type.

   bool cfam->has_cascaded_master(master->cfam);

Several cfam engine device drivers have to work around known
hardware bugs on specific hardware.  There are cases where
the same hardware engine (id and version same) behaves differently
on two different cfam types:

* IOU mailbox engine has same engine id, version number but
has different behavior on a CFAM-S type versus an IOU type.
This would require some form of fsi client query of cfam type
or initialization of some op.

* The P9 iic boe engine did not have its version ID incremented
from P8 even though there are differences in the command
register set.  There is no way for the iic driver to know which
engine its on unless it knows the cfam type.

The IMD iic engine with the same engine ID and version will
have differences in iic clocks due to differences in cfam
type.  The iic bus clock is derived from the parent fsi master
clock which is divided down differently on different cfam types.
In order for iic driver to know what frequency its buses are
operating at has to know the cfam type.







>
>
> Jeremy
Jeremy Kerr Oct. 7, 2016, 1:22 a.m. UTC | #4 
Hi Chris,

>>> Suggested changes to the patch set so far as provided by Jeremy Kerr.
>>> Rename slave to cfam to distinguish between the slave engine and its
>>> containing cfam.
>>
>> I'm still not 100% on that. As is my understanding, a (hardware) CFAM
>> contains multiple slaves, and that not what `struct fsi_slave`
>> represents (it's a single slave).
> 
> In redundant configurations yes there are two slaves.  However, any
> controlling device driver would only ever use one of them.  A redundant
> system with its own fsi device driver would be controlling the other slave.
> 
> I still maintain that there is bound to be confusion with slave->read( )
> etc... which are meant for accessing more than just the slave engine.
> cfam->read makes it more clear I think that you are accessing some
> range within the cfam, not just the slave engine.
>
> A cfam acts as a slave
> in a general sense but it doesn't distinguish itself from the slave engine.

Right, and that's where our confusion is coming from there.

In my initial series, I've used `struct fsi_slave` to represent *just*
the slave on the FSI bus, which is logically separated from the *slave
engines*. In this case, a "slave" is a different logical unit from a
"slave engine"; the latter being one of the actual functional units made
available behind the slave.

This separation is important, as each of the engines is what will be
bound to a FSI device driver, whereas the slave itself will be managed
by the core.

So we have:

  struct fsi_slave -> core slave logic, knows little about engines

  struct fsi_device -> engine logic, to be bound to a driver

(whereas a CFAM is slave + engines, right?)

Nothing outside of the core will be doing slave->read; FSI engine
drivers will be using the fsi_device API.

> I'm not sure what split out means in this case.  Git am applies each patch
> in sequence so those warnings will be hit before any patches I add to reverse
> something are applied.
> 
> I know we had discussed last week about making it clear what changes
> you had made versus mine.  As per discussion I left your changes as
> is in the patch series, then made my recommended changes as the
> next in the series.   A bit confusing on how to deal with checkpatch,
> git am, ... in those circumstances.

Yep, and if there are fixups to those patches, keep them separate from
other changes, as I'll apply them directly to my tree.

From your follow-up email, it sounds like we want some identification of
the slave HW implementation, to allow the core and drivers to work
around any HW issues.

Cheers,


Jeremy
christopher.lee.bostic@gmail.com Oct. 7, 2016, 6:33 p.m. UTC | #5 
On Thu, Oct 6, 2016 at 8:22 PM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Chris,
>
>>>> Suggested changes to the patch set so far as provided by Jeremy Kerr.
>>>> Rename slave to cfam to distinguish between the slave engine and its
>>>> containing cfam.
>>>
>>> I'm still not 100% on that. As is my understanding, a (hardware) CFAM
>>> contains multiple slaves, and that not what `struct fsi_slave`
>>> represents (it's a single slave).
>>
>> In redundant configurations yes there are two slaves.  However, any
>> controlling device driver would only ever use one of them.  A redundant
>> system with its own fsi device driver would be controlling the other slave.
>>
>> I still maintain that there is bound to be confusion with slave->read( )
>> etc... which are meant for accessing more than just the slave engine.
>> cfam->read makes it more clear I think that you are accessing some
>> range within the cfam, not just the slave engine.
>>
>> A cfam acts as a slave
>> in a general sense but it doesn't distinguish itself from the slave engine.
>
> Right, and that's where our confusion is coming from there.
>
> In my initial series, I've used `struct fsi_slave` to represent *just*
> the slave on the FSI bus, which is logically separated from the *slave
> engines*. In this case, a "slave" is a different logical unit from a
> "slave engine"; the latter being one of the actual functional units made
> available behind the slave.
>
> This separation is important, as each of the engines is what will be
> bound to a FSI device driver, whereas the slave itself will be managed
> by the core.
> So we have:
>
>   struct fsi_slave -> core slave logic, knows little about engines
>
>   struct fsi_device -> engine logic, to be bound to a driver
>
> (whereas a CFAM is slave + engines, right?)
>
> Nothing outside of the core will be doing slave->read; FSI engine
> drivers will be using the fsi_device API.

Hi Jeremy,

That makes sense.  I wasn't getting what you were thinking.  I'll
keep your 'struct fsi_slave' as is.   The previous note regarding
hardware workarounds, though, will possibly require some concept
of a separate 'struct cfam'.   That or some other construct to
deal in a common way with all the quirks related to a particular
cfam.  Can you make Chris Austen's call Tuesday morning
your time?  We can discuss that further.

>
>> I'm not sure what split out means in this case.  Git am applies each patch
>> in sequence so those warnings will be hit before any patches I add to reverse
>> something are applied.
>>
>> I know we had discussed last week about making it clear what changes
>> you had made versus mine.  As per discussion I left your changes as
>> is in the patch series, then made my recommended changes as the
>> next in the series.   A bit confusing on how to deal with checkpatch,
>> git am, ... in those circumstances.
>
> Yep, and if there are fixups to those patches, keep them separate from
> other changes, as I'll apply them directly to my tree.

So the plan is,  patches 01-12 your set.  patch 13 are my fixups to that.
Patches 14+ my content.

>
> From your follow-up email, it sounds like we want some identification of
> the slave HW implementation, to allow the core and drivers to work
> around any HW issues.
>
> Cheers,
>
>
> Jeremy
diff mbox

Patch

diff --git a/drivers/fsi/fsi-cfam.h b/drivers/fsi/fsi-cfam.h
new file mode 100644
index 0000000..a542f60
--- /dev/null
+++ b/drivers/fsi/fsi-cfam.h
@@ -0,0 +1,71 @@ 
+/*
+ * FSI CFAM definitions
+ *
+ * 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_CFAM_H
+#define DRIVERS_FSI_CFAM_H
+
+#include <linux/device.h>
+
+#include "fsi-master.h"
+
+#define FSI_SLAVE_ENG_ID	2
+
+#define FSI_MAX_CFAMS_PER_LINK	4
+#define FSI_CFAM_SIZE		(FSI_LINK_SIZE / FSI_MAX_CFAMS_PER_LINK)
+#define FSI_PEEK_BASE		0x410
+
+/* Config space decoding */
+#define FSI_CFG_NEXT_MASK	0x80000000
+#define FSI_CFG_SLOT_MASK	0x00ff0000
+#define FSI_CFG_SLOT_SHIFT	16
+#define FSI_CFG_VERS_MASK	0x0000f000
+#define FSI_CFG_VERS_SHIFT	12
+#define FSI_CFG_TYPE_MASK	0x00000ff0
+#define FSI_CFG_TYPE_SHIFT	4
+
+/*
+ * Return number of slots in the configuration word
+ */
+static inline uint8_t fsi_cfg_slot(uint32_t x)
+{
+	return (x & FSI_CFG_SLOT_MASK) >> FSI_CFG_SLOT_SHIFT;
+}
+
+/*
+ * Return version in the configuration word
+ */
+static inline uint8_t fsi_cfg_version(uint32_t x)
+{
+	return (x & FSI_CFG_VERS_MASK) >> FSI_CFG_VERS_SHIFT;
+}
+
+/*
+ * Return type field in the configuration word
+ */
+static inline uint8_t fsi_cfg_type(uint32_t x)
+{
+	return (x & FSI_CFG_TYPE_MASK) >> FSI_CFG_TYPE_SHIFT;
+}
+
+struct fsi_cfam {
+	struct device		dev;
+	struct fsi_master	*master;
+	uint8_t			link;
+	uint8_t			id;
+};
+
+#define to_fsi_cfam(d) container_of(d, struct fsi_cfam, dev)
+
+#endif /* DRIVERS_FSI_MASTER_H */
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 10bf817..a7f23a5 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -21,16 +21,7 @@ 
 #include <linux/slab.h>
 
 #include "fsi-master.h"
-
-#define FSI_N_SLAVES	4
-
-#define FSI_SLAVE_CONF_NEXT_MASK	0x80000000
-#define FSI_SLAVE_CONF_SLOTS_MASK	0x00ff0000
-#define FSI_SLAVE_CONF_SLOTS_SHIFT	16
-#define FSI_SLAVE_CONF_VERSION_MASK	0x0000f000
-#define FSI_SLAVE_CONF_VERSION_SHIFT	12
-#define FSI_SLAVE_CONF_TYPE_MASK	0x00000ff0
-#define FSI_SLAVE_CONF_TYPE_SHIFT	4
+#include "fsi-cfam.h"
 
 #define FSI_PEEK_BASE			0x410
 
@@ -38,18 +29,9 @@  static const int engine_page_size = 0x400;
 
 static atomic_t master_idx = ATOMIC_INIT(-1);
 
-struct fsi_slave {
-	struct device		dev;
-	struct fsi_master	*master;
-	int			link;
-	uint8_t			id;
-};
-
-#define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
-
-static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
+static int fsi_cfam_read(struct fsi_cfam *cfam, uint32_t addr,
 		void *val, size_t size);
-static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
+static int fsi_cfam_write(struct fsi_cfam *cfam, uint32_t addr,
 		const void *val, size_t size);
 
 /* FSI endpoint-device support */
@@ -62,7 +44,7 @@  int fsi_device_read(struct fsi_device *dev, uint32_t addr, void *val,
 	if (addr + size > dev->size)
 		return -EINVAL;
 
-	return fsi_slave_read(dev->slave, dev->addr + addr, val, size);
+	return fsi_cfam_read(dev->cfam, dev->addr + addr, val, size);
 }
 
 int fsi_device_write(struct fsi_device *dev, uint32_t addr, const void *val,
@@ -74,14 +56,14 @@  int fsi_device_write(struct fsi_device *dev, uint32_t addr, const void *val,
 	if (addr + size > dev->size)
 		return -EINVAL;
 
-	return fsi_slave_write(dev->slave, dev->addr + addr, val, size);
+	return fsi_cfam_write(dev->cfam, dev->addr + addr, val, size);
 }
 
 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));
+	return fsi_cfam_read(dev->cfam, addr, val, sizeof(uint32_t));
 }
 
 static void fsi_device_release(struct device *_device)
@@ -90,7 +72,7 @@  static void fsi_device_release(struct device *_device)
 	kfree(device);
 }
 
-static struct fsi_device *fsi_create_device(struct fsi_slave *slave)
+static struct fsi_device *fsi_create_device(struct fsi_cfam *cfam)
 {
 	struct fsi_device *dev;
 
@@ -98,7 +80,7 @@  static struct fsi_device *fsi_create_device(struct fsi_slave *slave)
 	if (!dev)
 		return NULL;
 
-	dev->dev.parent = &slave->dev;
+	dev->dev.parent = &cfam->dev;
 	dev->dev.bus = &fsi_bus_type;
 	dev->dev.release = fsi_device_release;
 
@@ -128,22 +110,22 @@  static bool check_crc4(uint32_t x)
 	return crc4(x) == 0;
 }
 
-/* FSI slave support */
-static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
+/* FSI CFAM support */
+static int fsi_cfam_read(struct fsi_cfam *cfam, uint32_t addr,
 		void *val, size_t size)
 {
-	return slave->master->read(slave->master, slave->link,
-			slave->id, addr, val, size);
+	return cfam->master->read(cfam->master, cfam->link,
+			cfam->id, addr, val, size);
 }
 
-static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
+static int fsi_cfam_write(struct fsi_cfam *cfam, uint32_t addr,
 		const void *val, size_t size)
 {
-	return slave->master->write(slave->master, slave->link,
-			slave->id, addr, val, size);
+	return cfam->master->write(cfam->master, cfam->link,
+			cfam->id, addr, val, size);
 }
 
-static int fsi_slave_scan(struct fsi_slave *slave)
+static int fsi_cfam_scan(struct fsi_cfam *cfam)
 {
 	uint32_t engine_addr;
 	uint32_t conf;
@@ -157,40 +139,37 @@  static int fsi_slave_scan(struct fsi_slave *slave)
 	 * skip the chip ID entry at the start of the address space.
 	 */
 	engine_addr = engine_page_size * 3;
-	for (i = 2; i < engine_page_size / sizeof(uint32_t); i++) {
+	for (i = FSI_SLAVE_ENG_ID; i < engine_page_size / sizeof(uint32_t);
+							i++) {
 		uint8_t slots, version, type;
 		struct fsi_device *dev;
 
-		rc = fsi_slave_read(slave, (i + 1) * sizeof(conf),
+		rc = fsi_cfam_read(cfam, (i + 1) * sizeof(conf),
 				&conf, sizeof(conf));
 		if (rc) {
-			dev_warn(&slave->dev,
-					"error reading slave registers\n");
+			dev_warn(&cfam->dev,
+					"error reading cfam registers\n");
 			return -1;
 		}
 
 		conf = be32_to_cpu(conf);
 
 		if (!check_crc4(conf)) {
-			dev_warn(&slave->dev,
-				"crc error in slave register at 0x%04x\n",
+			dev_warn(&cfam->dev,
+				"crc error in cfam register at 0x%04x\n",
 					i);
 			return -1;
 		}
+		slots = fsi_cfg_slot(conf);
+		version = fsi_cfg_version(conf);
+		type = fsi_cfg_type(conf);
 
-		slots = (conf & FSI_SLAVE_CONF_SLOTS_MASK)
-			>> FSI_SLAVE_CONF_SLOTS_SHIFT;
-		version = (conf & FSI_SLAVE_CONF_VERSION_MASK)
-			>> FSI_SLAVE_CONF_VERSION_SHIFT;
-		type = (conf & FSI_SLAVE_CONF_TYPE_MASK)
-			>> FSI_SLAVE_CONF_TYPE_SHIFT;
-
-		/* Unused address areas are marked by a zero type value; this
-		 * skips the defined address areas */
+		/* Unused address areas are marked by a zero type value; */
+		/* This skips the defined address areas */
 		if (type != 0) {
 
 			/* create device */
-			dev = fsi_create_device(slave);
+			dev = fsi_create_device(cfam);
 			if (!dev)
 				return -ENOMEM;
 
@@ -200,82 +179,83 @@  static int fsi_slave_scan(struct fsi_slave *slave)
 			dev->addr = engine_addr;
 			dev->size = slots * engine_page_size;
 
-			dev_info(&slave->dev,
+			dev_info(&cfam->dev,
 			"engine[%i]: type %x, version %x, addr %x size %x\n",
 					dev->unit, dev->engine_type, version,
 					dev->addr, dev->size);
 
 			device_initialize(&dev->dev);
 			dev_set_name(&dev->dev, "%02x:%02x:%02x:%02x",
-					slave->master->idx, slave->link,
-					slave->id, i - 2);
+					cfam->master->idx, cfam->link,
+					cfam->id, i - 2);
 
 			rc = device_add(&dev->dev);
 			if (rc) {
-				dev_warn(&slave->dev, "add failed: %d\n", rc);
+				dev_warn(&cfam->dev, "add failed: %d\n", rc);
 				put_device(&dev->dev);
 			}
 		}
 
 		engine_addr += slots * engine_page_size;
 
-		if (!(conf & FSI_SLAVE_CONF_NEXT_MASK))
+		if (!(conf & FSI_CFG_NEXT_MASK))
 			break;
 	}
 
 	return 0;
 }
 
-static void fsi_slave_release(struct device *dev)
+static void fsi_cfam_release(struct device *dev)
 {
-	struct fsi_slave *slave = to_fsi_slave(dev);
-	kfree(slave);
+	struct fsi_cfam *cfam = to_fsi_cfam(dev);
+
+	kfree(cfam);
 }
 
-static int fsi_slave_init(struct fsi_master *master,
-		int link, uint8_t slave_id)
+static int fsi_cfam_init(struct fsi_master *master,
+		int link, uint8_t cfam_id)
 {
-	struct fsi_slave *slave;
+	struct fsi_cfam *cfam = NULL;
 	uint32_t chip_id;
 	int rc;
 
-	rc = master->read(master, link, slave_id, 0, &chip_id, sizeof(chip_id));
+	rc = master->read(master, link, cfam_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);
+		dev_warn(master->dev, "can't read cfam %02x:%02x: %d\n",
+				link, cfam_id, rc);
 		return -ENODEV;
 	}
 
 	chip_id = be32_to_cpu(chip_id);
 	if (!check_crc4(chip_id)) {
-		dev_warn(master->dev, "slave %02x:%02x: invalid chip id CRC!\n",
-				link, slave_id);
+		dev_warn(master->dev, "cfam %02x:%02x: invalid chip id CRC!\n",
+				link, cfam_id);
 		return -EIO;
 	}
 
 	pr_debug("fsi: found chip %08x at %02x:%02x:%02x\n",
-			master->idx, chip_id, link, slave_id);
+			master->idx, chip_id, link, cfam_id);
 
-	/* we can communicate with a slave; create devices and scan */
-	slave = kzalloc(sizeof(*slave), GFP_KERNEL);
-	if (!slave)
+	/* we can communicate with a cfam; create devices and scan */
+	cfam = kzalloc(sizeof(*cfam), GFP_KERNEL);
+	if (!cfam)
 		return -ENOMEM;
 
-	slave->master = master;
-	slave->id = slave_id;
-	slave->dev.parent = master->dev;
-	slave->dev.release = fsi_slave_release;
+	cfam->master = master;
+	cfam->id = cfam_id;
+	cfam->dev.parent = master->dev;
+	cfam->dev.release = fsi_cfam_release;
 
-	dev_set_name(&slave->dev, "slave@%02x:%02x", link, slave_id);
-	rc = device_register(&slave->dev);
+	dev_set_name(&cfam->dev, "cfam@%02x:%02x", link, cfam_id);
+	rc = device_register(&cfam->dev);
 	if (rc < 0) {
-		dev_warn(master->dev, "failed to create slave device: %d\n",
+		dev_warn(master->dev, "failed to create cfam device: %d\n",
 				rc);
-		put_device(&slave->dev);
+		put_device(&cfam->dev);
 		return rc;
 	}
 
-	fsi_slave_scan(slave);
+	fsi_cfam_scan(cfam);
 	return 0;
 }
 
@@ -283,11 +263,11 @@  static int fsi_slave_init(struct fsi_master *master,
 
 static int fsi_master_scan(struct fsi_master *master)
 {
-	int link, slave_id;
+	int link, cfam_id;
 
 	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);
+		for (cfam_id = 0; cfam_id < FSI_MAX_CFAMS_PER_LINK; cfam_id++)
+			fsi_cfam_init(master, link, cfam_id);
 
 	return 0;
 
diff --git a/drivers/fsi/fsi-master-fake.c b/drivers/fsi/fsi-master-fake.c
index 50aac0b..658a5f6 100644
--- a/drivers/fsi/fsi-master-fake.c
+++ b/drivers/fsi/fsi-master-fake.c
@@ -29,7 +29,7 @@  const uint8_t data[] = {
 
 
 static int fsi_master_fake_read(struct fsi_master *_master, int link,
-		uint8_t slave, uint32_t addr, void *val, size_t size)
+		uint8_t cfam, uint32_t addr, void *val, size_t size)
 {
 	if (link != 0)
 		return -ENODEV;
@@ -43,7 +43,7 @@  static int fsi_master_fake_read(struct fsi_master *_master, int link,
 }
 
 static int fsi_master_fake_write(struct fsi_master *_master, int link,
-		uint8_t slave, uint32_t addr, const void *val, size_t size)
+		uint8_t cfam, uint32_t addr, const void *val, size_t size)
 {
 	if (link != 0)
 		return -ENODEV;
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index 3855829..3e1aeb6 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -22,7 +22,7 @@  struct fsi_gpio_msg {
 };
 
 static int fsi_master_gpio_read(struct fsi_master *_master, int link,
-		uint8_t slave, uint32_t addr, void *val, size_t size)
+		uint8_t cfam, uint32_t addr, void *val, size_t size)
 {
 	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
 
@@ -37,7 +37,7 @@  static int fsi_master_gpio_read(struct fsi_master *_master, int link,
 }
 
 static int fsi_master_gpio_write(struct fsi_master *_master, int link,
-		uint8_t slave, uint32_t addr, const void *val, size_t size)
+		uint8_t cfam, uint32_t addr, const void *val, size_t size)
 {
 	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
 
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index e75a810..4cdc4b4 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -24,10 +24,10 @@  struct fsi_master {
 	int		idx;
 	int		n_links;
 	int		(*read)(struct fsi_master *, int link,
-				uint8_t slave, uint32_t addr,
+				uint8_t cfam, uint32_t addr,
 				void *val, size_t size);
 	int		(*write)(struct fsi_master *, int link,
-				uint8_t slave, uint32_t addr,
+				uint8_t cfam, uint32_t addr,
 				const void *val, size_t size);
 };
 
diff --git a/include/linux/fsi.h b/include/linux/fsi.h
index 47af0af..c919058 100644
--- a/include/linux/fsi.h
+++ b/include/linux/fsi.h
@@ -23,7 +23,7 @@  struct fsi_device {
 	u8			engine_type;
 	u8			version;
 	u8			unit;
-	struct fsi_slave	*slave;
+	struct fsi_cfam		*cfam;
 	uint32_t		addr;
 	uint32_t		size;
 };