[PATCH 0/3] Add 'external mode' for GPIO-based FSI master

[PATCH 0/3] Add 'external mode' for GPIO-based FSI master

[Jeremy Kerr]

This series (on top of current char-misc-next) implements "external
mode" (ie, support for FSI debug devices) for the GPIO-based FSI master
driver.

We implement this control in the GPIO master driver, as it has the
mapping of raw GPIO pins to fsi control signals, and provides a
mechanism for the kernel to retain exclusive access to those GPIOs.

Cheers,


Jeremy

---
Jeremy Kerr (3):
  fsi: Add fsi_master_rescan()
  fsi/master-gpio: Add locking around gpio operations during break &
    link enable
  fsi/master-gpio: Add external mode

 .../ABI/testing/sysfs-driver-fsi-master-gpio       | 10 +++
 drivers/fsi/fsi-core.c                             |  9 ++-
 drivers/fsi/fsi-master-gpio.c                      | 85 +++++++++++++++++++++-
 drivers/fsi/fsi-master.h                           |  2 +
 4 files changed, 102 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio

-- 
2.7.4

[PATCH 1/3] fsi: Add fsi_master_rescan()

[Jeremy Kerr]

We'll want non-core fsi code to trigger a rescan, so introduce a
non-static fsi_master_rescan() function. Use this for the existing
unscan/scan behaviour too.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Christopher Bostic <clbostic@linux.vnet.ibm.com>
---
 drivers/fsi/fsi-core.c   | 9 +++++++--
 drivers/fsi/fsi-master.h | 2 ++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index a485864..28d917e 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -762,14 +762,19 @@ static void fsi_master_unscan(struct fsi_master *master)
 	device_for_each_child(&master->dev, NULL, fsi_master_remove_slave);
 }
 
+int fsi_master_rescan(struct fsi_master *master)
+{
+	fsi_master_unscan(master);
+	return fsi_master_scan(master);
+}
+
 static ssize_t master_rescan_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct fsi_master *master = to_fsi_master(dev);
 	int rc;
 
-	fsi_master_unscan(master);
-	rc = fsi_master_scan(master);
+	rc = fsi_master_rescan(master);
 	if (rc < 0)
 		return rc;
 
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index 12f7b11..18bd4ad 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -40,4 +40,6 @@ struct fsi_master {
 extern int fsi_master_register(struct fsi_master *master);
 extern void fsi_master_unregister(struct fsi_master *master);
 
+extern int fsi_master_rescan(struct fsi_master *master);
+
 #endif /* DRIVERS_FSI_MASTER_H */
-- 
2.7.4

[PATCH v2 1/3] fsi: Add fsi_master_rescan()

[Jeremy Kerr]

We'll want non-core fsi code to trigger a rescan, so introduce a
non-static fsi_master_rescan() function. Use this for the existing
unscan/scan behaviour too.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Christopher Bostic <clbostic@linux.vnet.ibm.com>

---
v2:
  - Add EXPORT_SYMBOL_GPL(fsi_master_rescan) for =m builds. Thanks
    kbuild test robot, I'll shout you your choice of coolant.
---
 drivers/fsi/fsi-core.c   | 10 ++++++++--
 drivers/fsi/fsi-master.h |  2 ++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index a485864..c3d59d9 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -762,14 +762,20 @@ static void fsi_master_unscan(struct fsi_master *master)
 	device_for_each_child(&master->dev, NULL, fsi_master_remove_slave);
 }
 
+int fsi_master_rescan(struct fsi_master *master)
+{
+	fsi_master_unscan(master);
+	return fsi_master_scan(master);
+}
+EXPORT_SYMBOL_GPL(fsi_master_rescan);
+
 static ssize_t master_rescan_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct fsi_master *master = to_fsi_master(dev);
 	int rc;
 
-	fsi_master_unscan(master);
-	rc = fsi_master_scan(master);
+	rc = fsi_master_rescan(master);
 	if (rc < 0)
 		return rc;
 
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index 12f7b11..18bd4ad 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -40,4 +40,6 @@ struct fsi_master {
 extern int fsi_master_register(struct fsi_master *master);
 extern void fsi_master_unregister(struct fsi_master *master);
 
+extern int fsi_master_rescan(struct fsi_master *master);
+
 #endif /* DRIVERS_FSI_MASTER_H */
-- 
2.7.4

[PATCH 2/3] fsi/master-gpio: Add locking around gpio operations during break & link enable

[Jeremy Kerr]

Currently, we perform GPIO accesses in fsi_master_gpio_break and
fsi_master_link_enable, without holding cmd_lock. This change adds the
appropriate locking.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Christopher Bostic <clbostic@linux.vnet.ibm.com>
---
 drivers/fsi/fsi-master-gpio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index ae26187..a6d602e 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -461,12 +461,14 @@ static int fsi_master_gpio_term(struct fsi_master *_master,
 static int fsi_master_gpio_break(struct fsi_master *_master, int link)
 {
 	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+	unsigned long flags;
 
 	if (link != 0)
 		return -ENODEV;
 
 	trace_fsi_master_gpio_break(master);
 
+	spin_lock_irqsave(&master->cmd_lock, flags);
 	set_sda_output(master, 1);
 	sda_out(master, 1);
 	clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
@@ -475,6 +477,7 @@ static int fsi_master_gpio_break(struct fsi_master *_master, int link)
 	echo_delay(master);
 	sda_out(master, 1);
 	clock_toggle(master, FSI_POST_BREAK_CLOCKS);
+	spin_unlock_irqrestore(&master->cmd_lock, flags);
 
 	/* Wait for logic reset to take effect */
 	udelay(200);
@@ -497,10 +500,14 @@ static void fsi_master_gpio_init(struct fsi_master_gpio *master)
 static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
 {
 	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+	unsigned long flags;
 
 	if (link != 0)
 		return -ENODEV;
+
+	spin_lock_irqsave(&master->cmd_lock, flags);
 	gpiod_set_value(master->gpio_enable, 1);
+	spin_unlock_irqrestore(&master->cmd_lock, flags);
 
 	return 0;
 }
-- 
2.7.4

[PATCH 3/3] fsi/master-gpio: Add external mode

[Jeremy Kerr]

This change introduces an 'external mode' for GPIO-based FSI masters,
allowing the clock and data lines to be driven by an external source.
For example, external mode is selected by a user when an external debug
device is attached to the FSI pins.

To do this, we need to set specific states for the trans, mux and enable
gpios, and prevent access to clk & data from the FSI core code (by
returning EBUSY).

External mode is controlled by a sysfs attribute, so add the relevent
information to Documentation/ABI/

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 .../ABI/testing/sysfs-driver-fsi-master-gpio       | 10 +++
 drivers/fsi/fsi-master-gpio.c                      | 78 +++++++++++++++++++++-
 2 files changed, 86 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio

diff --git a/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
new file mode 100644
index 0000000..9667bb4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
@@ -0,0 +1,10 @@
+What:           /sys/bus/platform/devices/[..]/fsi-master-gpio/external_mode
+Date:           June 2017
+KernelVersion:  4.12
+Contact:        jk@ozlabs.org
+Description:
+                Controls access arbitration for GPIO-based FSI master. A
+		value of 0 (the default) sets normal mode, where the
+		driver performs FSI bus transactions, 1 sets external mode,
+		where the FSI bus is driven externally (for example, by
+		a debug device).
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index a6d602e..88d26fe 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -59,6 +59,7 @@ struct fsi_master_gpio {
 	struct gpio_desc	*gpio_trans;	/* Voltage translator */
 	struct gpio_desc	*gpio_enable;	/* FSI enable */
 	struct gpio_desc	*gpio_mux;	/* Mux control */
+	bool			external_mode;
 };
 
 #define CREATE_TRACE_POINTS
@@ -411,6 +412,12 @@ static int fsi_master_gpio_xfer(struct fsi_master_gpio *master, uint8_t slave,
 	int rc;
 
 	spin_lock_irqsave(&master->cmd_lock, flags);
+
+	if (master->external_mode) {
+		spin_unlock_irqrestore(&master->cmd_lock, flags);
+		return -EBUSY;
+	}
+
 	serial_out(master, cmd);
 	echo_delay(master);
 	rc = poll_for_response(master, slave, resp_len, resp);
@@ -469,6 +476,10 @@ static int fsi_master_gpio_break(struct fsi_master *_master, int link)
 	trace_fsi_master_gpio_break(master);
 
 	spin_lock_irqsave(&master->cmd_lock, flags);
+	if (master->external_mode) {
+		spin_unlock_irqrestore(&master->cmd_lock, flags);
+		return -EBUSY;
+	}
 	set_sda_output(master, 1);
 	sda_out(master, 1);
 	clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
@@ -497,25 +508,84 @@ static void fsi_master_gpio_init(struct fsi_master_gpio *master)
 	clock_zeros(master, FSI_INIT_CLOCKS);
 }
 
+static void fsi_master_gpio_init_external(struct fsi_master_gpio *master)
+{
+	gpiod_direction_output(master->gpio_mux, 0);
+	gpiod_direction_output(master->gpio_trans, 0);
+	gpiod_direction_output(master->gpio_enable, 1);
+	gpiod_direction_input(master->gpio_clk);
+	gpiod_direction_input(master->gpio_data);
+}
+
 static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
 {
 	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
 	unsigned long flags;
+	int rc = -EBUSY;
 
 	if (link != 0)
 		return -ENODEV;
 
 	spin_lock_irqsave(&master->cmd_lock, flags);
-	gpiod_set_value(master->gpio_enable, 1);
+	if (!master->external_mode) {
+		gpiod_set_value(master->gpio_enable, 1);
+		rc = 0;
+	}
 	spin_unlock_irqrestore(&master->cmd_lock, flags);
 
-	return 0;
+	return rc;
+}
+
+static ssize_t external_mode_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct fsi_master_gpio *master = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE - 1, "%u",
+			master->external_mode ? 1 : 0);
+}
+
+static ssize_t external_mode_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct fsi_master_gpio *master = dev_get_drvdata(dev);
+	unsigned long flags, val;
+	bool external_mode;
+	int err;
+
+	err = kstrtoul(buf, 0, &val);
+	if (err)
+		return err;
+
+	external_mode = !!val;
+
+	spin_lock_irqsave(&master->cmd_lock, flags);
+
+	if (external_mode == master->external_mode) {
+		spin_unlock_irqrestore(&master->cmd_lock, flags);
+		return count;
+	}
+
+	master->external_mode = external_mode;
+	if (master->external_mode)
+		fsi_master_gpio_init_external(master);
+	else
+		fsi_master_gpio_init(master);
+	spin_unlock_irqrestore(&master->cmd_lock, flags);
+
+	fsi_master_rescan(&master->master);
+
+	return count;
 }
 
+static DEVICE_ATTR(external_mode, 0664,
+		external_mode_show, external_mode_store);
+
 static int fsi_master_gpio_probe(struct platform_device *pdev)
 {
 	struct fsi_master_gpio *master;
 	struct gpio_desc *gpio;
+	int rc;
 
 	master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
 	if (!master)
@@ -572,6 +642,10 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
 
 	fsi_master_gpio_init(master);
 
+	rc = device_create_file(&pdev->dev, &dev_attr_external_mode);
+	if (rc)
+		return rc;
+
 	return fsi_master_register(&master->master);
 }
 
-- 
2.7.4

Re: [PATCH 3/3] fsi/master-gpio: Add external mode

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]

Hi Jeremy,

[auto build test ERROR on next-20170619]
[cannot apply to linus/master linux/master v4.12-rc5 v4.12-rc4 v4.12-rc3 v4.12-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jeremy-Kerr/Add-external-mode-for-GPIO-based-FSI-master/20170619-204217
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

All errors (new ones prefixed by >>):

   ERROR: "ia64_delay_loop" [drivers/spi/spi-thunderx.ko] undefined!
   ERROR: "ia64_delay_loop" [drivers/net/phy/mdio-cavium.ko] undefined!
>> ERROR: "fsi_master_rescan" [drivers/fsi/fsi-master-gpio.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48166 bytes --]

Re: [PATCH 3/3] fsi/master-gpio: Add external mode

[Joel Stanley]

Hi Jeremy,

On Mon, Jun 19, 2017 at 6:56 PM, Jeremy Kerr <jk@ozlabs.org> wrote:
> This change introduces an 'external mode' for GPIO-based FSI masters,
> +static ssize_t external_mode_show(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +       struct fsi_master_gpio *master = dev_get_drvdata(dev);
> +
> +       return snprintf(buf, PAGE_SIZE - 1, "%u",

I gave this a spin on a machine today and noticed we're missing a
newline there. Should this be "%u\n"?

Cheers,

Joel

> +                       master->external_mode ? 1 : 0);
> +}
> +

Re: [PATCH 3/3] fsi/master-gpio: Add external mode

[Jeremy Kerr]

Hi Joel,

>> +static ssize_t external_mode_show(struct device *dev,
>> +               struct device_attribute *attr, char *buf)
>> +{
>> +       struct fsi_master_gpio *master = dev_get_drvdata(dev);
>> +
>> +       return snprintf(buf, PAGE_SIZE - 1, "%u",
> 
> I gave this a spin on a machine today and noticed we're missing a
> newline there. Should this be "%u\n"?

Yep, that'd make things easier to read. v2 coming.

Cheers,


Jeremy

[PATCH v2 3/3] fsi/master-gpio: Add external mode

[Jeremy Kerr]

This change introduces an 'external mode' for GPIO-based FSI masters,
allowing the clock and data lines to be driven by an external source.
For example, external mode is selected by a user when an external debug
device is attached to the FSI pins.

To do this, we need to set specific states for the trans, mux and enable
gpios, and prevent access to clk & data from the FSI core code (by
returning EBUSY).

External mode is controlled by a sysfs attribute, so add the relevent
information to Documentation/ABI/

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 .../ABI/testing/sysfs-driver-fsi-master-gpio       | 10 +++
 drivers/fsi/fsi-master-gpio.c                      | 78 +++++++++++++++++++++-
 2 files changed, 86 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio

diff --git a/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
new file mode 100644
index 0000000..9667bb4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
@@ -0,0 +1,10 @@
+What:           /sys/bus/platform/devices/[..]/fsi-master-gpio/external_mode
+Date:           June 2017
+KernelVersion:  4.12
+Contact:        jk@ozlabs.org
+Description:
+                Controls access arbitration for GPIO-based FSI master. A
+		value of 0 (the default) sets normal mode, where the
+		driver performs FSI bus transactions, 1 sets external mode,
+		where the FSI bus is driven externally (for example, by
+		a debug device).
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index a6d602e..b54c213 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -59,6 +59,7 @@ struct fsi_master_gpio {
 	struct gpio_desc	*gpio_trans;	/* Voltage translator */
 	struct gpio_desc	*gpio_enable;	/* FSI enable */
 	struct gpio_desc	*gpio_mux;	/* Mux control */
+	bool			external_mode;
 };
 
 #define CREATE_TRACE_POINTS
@@ -411,6 +412,12 @@ static int fsi_master_gpio_xfer(struct fsi_master_gpio *master, uint8_t slave,
 	int rc;
 
 	spin_lock_irqsave(&master->cmd_lock, flags);
+
+	if (master->external_mode) {
+		spin_unlock_irqrestore(&master->cmd_lock, flags);
+		return -EBUSY;
+	}
+
 	serial_out(master, cmd);
 	echo_delay(master);
 	rc = poll_for_response(master, slave, resp_len, resp);
@@ -469,6 +476,10 @@ static int fsi_master_gpio_break(struct fsi_master *_master, int link)
 	trace_fsi_master_gpio_break(master);
 
 	spin_lock_irqsave(&master->cmd_lock, flags);
+	if (master->external_mode) {
+		spin_unlock_irqrestore(&master->cmd_lock, flags);
+		return -EBUSY;
+	}
 	set_sda_output(master, 1);
 	sda_out(master, 1);
 	clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
@@ -497,25 +508,84 @@ static void fsi_master_gpio_init(struct fsi_master_gpio *master)
 	clock_zeros(master, FSI_INIT_CLOCKS);
 }
 
+static void fsi_master_gpio_init_external(struct fsi_master_gpio *master)
+{
+	gpiod_direction_output(master->gpio_mux, 0);
+	gpiod_direction_output(master->gpio_trans, 0);
+	gpiod_direction_output(master->gpio_enable, 1);
+	gpiod_direction_input(master->gpio_clk);
+	gpiod_direction_input(master->gpio_data);
+}
+
 static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
 {
 	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
 	unsigned long flags;
+	int rc = -EBUSY;
 
 	if (link != 0)
 		return -ENODEV;
 
 	spin_lock_irqsave(&master->cmd_lock, flags);
-	gpiod_set_value(master->gpio_enable, 1);
+	if (!master->external_mode) {
+		gpiod_set_value(master->gpio_enable, 1);
+		rc = 0;
+	}
 	spin_unlock_irqrestore(&master->cmd_lock, flags);
 
-	return 0;
+	return rc;
+}
+
+static ssize_t external_mode_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct fsi_master_gpio *master = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE - 1, "%u\n",
+			master->external_mode ? 1 : 0);
+}
+
+static ssize_t external_mode_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct fsi_master_gpio *master = dev_get_drvdata(dev);
+	unsigned long flags, val;
+	bool external_mode;
+	int err;
+
+	err = kstrtoul(buf, 0, &val);
+	if (err)
+		return err;
+
+	external_mode = !!val;
+
+	spin_lock_irqsave(&master->cmd_lock, flags);
+
+	if (external_mode == master->external_mode) {
+		spin_unlock_irqrestore(&master->cmd_lock, flags);
+		return count;
+	}
+
+	master->external_mode = external_mode;
+	if (master->external_mode)
+		fsi_master_gpio_init_external(master);
+	else
+		fsi_master_gpio_init(master);
+	spin_unlock_irqrestore(&master->cmd_lock, flags);
+
+	fsi_master_rescan(&master->master);
+
+	return count;
 }
 
+static DEVICE_ATTR(external_mode, 0664,
+		external_mode_show, external_mode_store);
+
 static int fsi_master_gpio_probe(struct platform_device *pdev)
 {
 	struct fsi_master_gpio *master;
 	struct gpio_desc *gpio;
+	int rc;
 
 	master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
 	if (!master)
@@ -572,6 +642,10 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
 
 	fsi_master_gpio_init(master);
 
+	rc = device_create_file(&pdev->dev, &dev_attr_external_mode);
+	if (rc)
+		return rc;
+
 	return fsi_master_register(&master->master);
 }
 
-- 
2.7.4

Re: [PATCH v2 3/3] fsi/master-gpio: Add external mode

[Joel Stanley]

Hi Jeremy,

On Thu, Jun 22, 2017 at 7:40 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> This change introduces an 'external mode' for GPIO-based FSI masters,
> allowing the clock and data lines to be driven by an external source.
> For example, external mode is selected by a user when an external debug
> device is attached to the FSI pins.
>
> To do this, we need to set specific states for the trans, mux and enable
> gpios, and prevent access to clk & data from the FSI core code (by
> returning EBUSY).
>
> External mode is controlled by a sysfs attribute, so add the relevent
> information to Documentation/ABI/
>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> Reviewed-by: Joel Stanley <joel@jms.id.au>

This one never made it into Greg's tree (I assume because we didn't cc him).

Can you resend with Greg on cc please?

Cheers,

Joel

> ---
>  .../ABI/testing/sysfs-driver-fsi-master-gpio       | 10 +++
>  drivers/fsi/fsi-master-gpio.c                      | 78 +++++++++++++++++++++-
>  2 files changed, 86 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
> new file mode 100644
> index 0000000..9667bb4
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
> @@ -0,0 +1,10 @@
> +What:           /sys/bus/platform/devices/[..]/fsi-master-gpio/external_mode
> +Date:           June 2017
> +KernelVersion:  4.12
> +Contact:        jk@ozlabs.org
> +Description:
> +                Controls access arbitration for GPIO-based FSI master. A
> +               value of 0 (the default) sets normal mode, where the
> +               driver performs FSI bus transactions, 1 sets external mode,
> +               where the FSI bus is driven externally (for example, by
> +               a debug device).
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index a6d602e..b54c213 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -59,6 +59,7 @@ struct fsi_master_gpio {
>         struct gpio_desc        *gpio_trans;    /* Voltage translator */
>         struct gpio_desc        *gpio_enable;   /* FSI enable */
>         struct gpio_desc        *gpio_mux;      /* Mux control */
> +       bool                    external_mode;
>  };
>
>  #define CREATE_TRACE_POINTS
> @@ -411,6 +412,12 @@ static int fsi_master_gpio_xfer(struct fsi_master_gpio *master, uint8_t slave,
>         int rc;
>
>         spin_lock_irqsave(&master->cmd_lock, flags);
> +
> +       if (master->external_mode) {
> +               spin_unlock_irqrestore(&master->cmd_lock, flags);
> +               return -EBUSY;
> +       }
> +
>         serial_out(master, cmd);
>         echo_delay(master);
>         rc = poll_for_response(master, slave, resp_len, resp);
> @@ -469,6 +476,10 @@ static int fsi_master_gpio_break(struct fsi_master *_master, int link)
>         trace_fsi_master_gpio_break(master);
>
>         spin_lock_irqsave(&master->cmd_lock, flags);
> +       if (master->external_mode) {
> +               spin_unlock_irqrestore(&master->cmd_lock, flags);
> +               return -EBUSY;
> +       }
>         set_sda_output(master, 1);
>         sda_out(master, 1);
>         clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
> @@ -497,25 +508,84 @@ static void fsi_master_gpio_init(struct fsi_master_gpio *master)
>         clock_zeros(master, FSI_INIT_CLOCKS);
>  }
>
> +static void fsi_master_gpio_init_external(struct fsi_master_gpio *master)
> +{
> +       gpiod_direction_output(master->gpio_mux, 0);
> +       gpiod_direction_output(master->gpio_trans, 0);
> +       gpiod_direction_output(master->gpio_enable, 1);
> +       gpiod_direction_input(master->gpio_clk);
> +       gpiod_direction_input(master->gpio_data);
> +}
> +
>  static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
>  {
>         struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
>         unsigned long flags;
> +       int rc = -EBUSY;
>
>         if (link != 0)
>                 return -ENODEV;
>
>         spin_lock_irqsave(&master->cmd_lock, flags);
> -       gpiod_set_value(master->gpio_enable, 1);
> +       if (!master->external_mode) {
> +               gpiod_set_value(master->gpio_enable, 1);
> +               rc = 0;
> +       }
>         spin_unlock_irqrestore(&master->cmd_lock, flags);
>
> -       return 0;
> +       return rc;
> +}
> +
> +static ssize_t external_mode_show(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +       struct fsi_master_gpio *master = dev_get_drvdata(dev);
> +
> +       return snprintf(buf, PAGE_SIZE - 1, "%u\n",
> +                       master->external_mode ? 1 : 0);
> +}
> +
> +static ssize_t external_mode_store(struct device *dev,
> +               struct device_attribute *attr, const char *buf, size_t count)
> +{
> +       struct fsi_master_gpio *master = dev_get_drvdata(dev);
> +       unsigned long flags, val;
> +       bool external_mode;
> +       int err;
> +
> +       err = kstrtoul(buf, 0, &val);
> +       if (err)
> +               return err;
> +
> +       external_mode = !!val;
> +
> +       spin_lock_irqsave(&master->cmd_lock, flags);
> +
> +       if (external_mode == master->external_mode) {
> +               spin_unlock_irqrestore(&master->cmd_lock, flags);
> +               return count;
> +       }
> +
> +       master->external_mode = external_mode;
> +       if (master->external_mode)
> +               fsi_master_gpio_init_external(master);
> +       else
> +               fsi_master_gpio_init(master);
> +       spin_unlock_irqrestore(&master->cmd_lock, flags);
> +
> +       fsi_master_rescan(&master->master);
> +
> +       return count;
>  }
>
> +static DEVICE_ATTR(external_mode, 0664,
> +               external_mode_show, external_mode_store);
> +
>  static int fsi_master_gpio_probe(struct platform_device *pdev)
>  {
>         struct fsi_master_gpio *master;
>         struct gpio_desc *gpio;
> +       int rc;
>
>         master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
>         if (!master)
> @@ -572,6 +642,10 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
>
>         fsi_master_gpio_init(master);
>
> +       rc = device_create_file(&pdev->dev, &dev_attr_external_mode);
> +       if (rc)
> +               return rc;
> +
>         return fsi_master_register(&master->master);
>  }
>
> --
> 2.7.4
>

Re: [PATCH 0/3] Add 'external mode' for GPIO-based FSI master

[Joel Stanley]

Hi Jeremy,

On Mon, Jun 19, 2017 at 6:56 PM, Jeremy Kerr <jk@ozlabs.org> wrote:
> This series (on top of current char-misc-next) implements "external
> mode" (ie, support for FSI debug devices) for the GPIO-based FSI master
> driver.
>
> We implement this control in the GPIO master driver, as it has the
> mapping of raw GPIO pins to fsi control signals, and provides a
> mechanism for the kernel to retain exclusive access to those GPIOs.

I just noticed that this didn't get applied. I assume it's because we
forgot to cc Greg.

I have a v2 of some of the patches in my inbox; we should re-send that
v2. It may make sense to merge in some of the latter changes we have
in the openbmc tree first.

Cheers,

Joel

> ---
> Jeremy Kerr (3):
>   fsi: Add fsi_master_rescan()
>   fsi/master-gpio: Add locking around gpio operations during break &
>     link enable
>   fsi/master-gpio: Add external mode
>
>  .../ABI/testing/sysfs-driver-fsi-master-gpio       | 10 +++
>  drivers/fsi/fsi-core.c                             |  9 ++-
>  drivers/fsi/fsi-master-gpio.c                      | 85 +++++++++++++++++++++-
>  drivers/fsi/fsi-master.h                           |  2 +
>  4 files changed, 102 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
>
> --
> 2.7.4
>