[PATCH 0/5] fsi: Support extra functions on ASPEED master

[PATCH 0/5] fsi: Support extra functions on ASPEED master

[Joel Stanley]

These patches are reviewed and tested. I'm after an ack on the bindings
before submitting.

Joel Stanley (5):
  dt-bindings: fsi: Document gpios
  fsi: aspeed: Support cabled FSI
  fsi: aspeed: Run the bus at maximum speed
  fsi: aspeed: Add module param for bus divisor
  fsi: aspeed: Support CFAM reset GPIO

 .../bindings/fsi/fsi-master-aspeed.txt        |  12 ++
 drivers/fsi/fsi-master-aspeed.c               | 110 +++++++++++++++++-
 2 files changed, 119 insertions(+), 3 deletions(-)

-- 
2.27.0

[PATCH 1/5] dt-bindings: fsi: Document gpios

[Joel Stanley]

The FSI master has some associated GPIOs that may be present on some
hardware configurations.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 .../devicetree/bindings/fsi/fsi-master-aspeed.txt    | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt b/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
index b758f91914f7..a513e65ec0c9 100644
--- a/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
+++ b/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
@@ -12,6 +12,13 @@ Required properties:
  - pinctrl-0: phandle to pinctrl node
  - pinctrl-names: pinctrl state
 
+Optional properties:
+ - cfam-reset-gpios: GPIO for CFAM reset
+
+ - fsi-routing-gpios: GPIO for setting the FSI mux (internal or cabled)
+ - fsi-mux-gpios: GPIO for detecting the desired FSI mux state
+
+
 Examples:
 
     fsi-master {
@@ -21,4 +28,9 @@ Examples:
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_fsi1_default>;
 	clocks = <&syscon ASPEED_CLK_GATE_FSICLK>;
+
+	fsi-routing-gpios = <&gpio0 ASPEED_GPIO(Q, 7) GPIO_ACTIVE_HIGH>;
+	fsi-mux-gpios = <&gpio0 ASPEED_GPIO(B, 0) GPIO_ACTIVE_HIGH>;
+
+	cfam-reset-gpios = <&gpio0 ASPEED_GPIO(Q, 0) GPIO_ACTIVE_LOW>;
     };
-- 
2.27.0

Re: [PATCH 1/5] dt-bindings: fsi: Document gpios

[Andrew Jeffery]

On Tue, 28 Jul 2020, at 12:25, Joel Stanley wrote:
> The FSI master has some associated GPIOs that may be present on some
> hardware configurations.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  .../devicetree/bindings/fsi/fsi-master-aspeed.txt    | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git 
> a/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt 
> b/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
> index b758f91914f7..a513e65ec0c9 100644
> --- a/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
> +++ b/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
> @@ -12,6 +12,13 @@ Required properties:
>   - pinctrl-0: phandle to pinctrl node
>   - pinctrl-names: pinctrl state
>  
> +Optional properties:
> + - cfam-reset-gpios: GPIO for CFAM reset
> +
> + - fsi-routing-gpios: GPIO for setting the FSI mux (internal or cabled)
> + - fsi-mux-gpios: GPIO for detecting the desired FSI mux state
> +
> +

Is the double-linebreak intentional? Anyway:

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

>  Examples:
>  
>      fsi-master {
> @@ -21,4 +28,9 @@ Examples:
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_fsi1_default>;
>  	clocks = <&syscon ASPEED_CLK_GATE_FSICLK>;
> +
> +	fsi-routing-gpios = <&gpio0 ASPEED_GPIO(Q, 7) GPIO_ACTIVE_HIGH>;
> +	fsi-mux-gpios = <&gpio0 ASPEED_GPIO(B, 0) GPIO_ACTIVE_HIGH>;
> +
> +	cfam-reset-gpios = <&gpio0 ASPEED_GPIO(Q, 0) GPIO_ACTIVE_LOW>;
>      };
> -- 
> 2.27.0
> 
>

[PATCH 2/5] fsi: aspeed: Support cabled FSI

[Joel Stanley]

Some FSI capable systems have internal FSI signals, and some have
external cabled FSI. Software can detect which machine this is by
reading a jumper GPIO, and also control which pins the signals are
routed to through a mux GPIO.

This attempts to find the GPIOs at probe time. If they are not present
in the device tree the driver will not error and continue as before.

The mux GPIO is owned by the FSI driver to ensure it is not modified at
runtime. The routing jumper obtained as non-exclusive to allow other
software to inspect it's state.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/fsi/fsi-master-aspeed.c | 46 +++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index 2b8ca72baeb5..c282de76e6e7 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -13,6 +13,7 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/iopoll.h>
+#include <linux/gpio/consumer.h>
 
 #include "fsi-master.h"
 
@@ -417,6 +418,45 @@ static int aspeed_master_init(struct fsi_master_aspeed *aspeed)
 	return 0;
 }
 
+static int tacoma_cabled_fsi_fixup(struct device *dev)
+{
+	struct gpio_desc *routing_gpio, *mux_gpio;
+	int gpio;
+
+	/*
+	 * The routing GPIO is a jumper indicating we should mux for the
+	 * externally connected FSI cable.
+	 */
+	routing_gpio = devm_gpiod_get_optional(dev, "fsi-routing",
+			GPIOD_IN | GPIOD_FLAGS_BIT_NONEXCLUSIVE);
+	if (IS_ERR(routing_gpio))
+		return PTR_ERR(routing_gpio);
+	if (!routing_gpio)
+		return 0;
+
+	mux_gpio = devm_gpiod_get_optional(dev, "fsi-mux", GPIOD_ASIS);
+	if (IS_ERR(mux_gpio))
+		return PTR_ERR(mux_gpio);
+	if (!mux_gpio)
+		return 0;
+
+	gpio = gpiod_get_value(routing_gpio);
+	if (gpio < 0)
+		return gpio;
+
+	/* If the routing GPIO is high we should set the mux to low. */
+	if (gpio) {
+		gpiod_direction_output(mux_gpio, 0);
+		dev_info(dev, "FSI configured for external cable\n");
+	} else {
+		gpiod_direction_output(mux_gpio, 1);
+	}
+
+	devm_gpiod_put(dev, routing_gpio);
+
+	return 0;
+}
+
 static int fsi_master_aspeed_probe(struct platform_device *pdev)
 {
 	struct fsi_master_aspeed *aspeed;
@@ -424,6 +464,12 @@ static int fsi_master_aspeed_probe(struct platform_device *pdev)
 	int rc, links, reg;
 	__be32 raw;
 
+	rc = tacoma_cabled_fsi_fixup(&pdev->dev);
+	if (rc) {
+		dev_err(&pdev->dev, "Tacoma FSI cable fixup failed\n");
+		return rc;
+	}
+
 	aspeed = devm_kzalloc(&pdev->dev, sizeof(*aspeed), GFP_KERNEL);
 	if (!aspeed)
 		return -ENOMEM;
-- 
2.27.0

Re: [PATCH 2/5] fsi: aspeed: Support cabled FSI

[Andrew Jeffery]

On Tue, 28 Jul 2020, at 12:25, Joel Stanley wrote:
> Some FSI capable systems have internal FSI signals, and some have
> external cabled FSI. Software can detect which machine this is by
> reading a jumper GPIO, and also control which pins the signals are
> routed to through a mux GPIO.
> 
> This attempts to find the GPIOs at probe time. If they are not present
> in the device tree the driver will not error and continue as before.
> 
> The mux GPIO is owned by the FSI driver to ensure it is not modified at
> runtime. The routing jumper obtained as non-exclusive to allow other
> software to inspect it's state.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  drivers/fsi/fsi-master-aspeed.c | 46 +++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
> index 2b8ca72baeb5..c282de76e6e7 100644
> --- a/drivers/fsi/fsi-master-aspeed.c
> +++ b/drivers/fsi/fsi-master-aspeed.c
> @@ -13,6 +13,7 @@
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/iopoll.h>
> +#include <linux/gpio/consumer.h>
>  
>  #include "fsi-master.h"
>  
> @@ -417,6 +418,45 @@ static int aspeed_master_init(struct 
> fsi_master_aspeed *aspeed)
>  	return 0;
>  }
>  
> +static int tacoma_cabled_fsi_fixup(struct device *dev)
> +{
> +	struct gpio_desc *routing_gpio, *mux_gpio;
> +	int gpio;
> +
> +	/*
> +	 * The routing GPIO is a jumper indicating we should mux for the
> +	 * externally connected FSI cable.
> +	 */
> +	routing_gpio = devm_gpiod_get_optional(dev, "fsi-routing",
> +			GPIOD_IN | GPIOD_FLAGS_BIT_NONEXCLUSIVE);
> +	if (IS_ERR(routing_gpio))
> +		return PTR_ERR(routing_gpio);
> +	if (!routing_gpio)
> +		return 0;
> +
> +	mux_gpio = devm_gpiod_get_optional(dev, "fsi-mux", GPIOD_ASIS);
> +	if (IS_ERR(mux_gpio))
> +		return PTR_ERR(mux_gpio);
> +	if (!mux_gpio)
> +		return 0;
> +
> +	gpio = gpiod_get_value(routing_gpio);
> +	if (gpio < 0)
> +		return gpio;
> +
> +	/* If the routing GPIO is high we should set the mux to low. */
> +	if (gpio) {
> +		gpiod_direction_output(mux_gpio, 0);
> +		dev_info(dev, "FSI configured for external cable\n");
> +	} else {
> +		gpiod_direction_output(mux_gpio, 1);
> +	}

I haven't checked this logic against the hardware but the rest of the patch 
looks okay.

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

[PATCH 3/5] fsi: aspeed: Run the bus at maximum speed

[Joel Stanley]

Testing of Tacoma has shown that the ASPEED master can be run at maximum
speed.

The exception is when wired externally with a cable, in which case we
use a divisor of two to ensure reliable operation.

Reviewed-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/fsi/fsi-master-aspeed.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index c282de76e6e7..f503f64ab32b 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -83,7 +83,11 @@ static const u32 fsi_base = 0xa0000000;
 
 #define FSI_LINK_ENABLE_SETUP_TIME	10	/* in mS */
 
-#define DEFAULT_DIVISOR			14
+/* Run the bus at maximum speed by default */
+#define FSI_DIVISOR_DEFAULT            1
+#define FSI_DIVISOR_CABLED             2
+static u16 aspeed_fsi_divisor = FSI_DIVISOR_DEFAULT;
+
 #define OPB_POLL_TIMEOUT		10000
 
 static int __opb_write(struct fsi_master_aspeed *aspeed, u32 addr,
@@ -385,9 +389,11 @@ static int aspeed_master_init(struct fsi_master_aspeed *aspeed)
 	opb_writel(aspeed, ctrl_base + FSI_MECTRL, reg);
 
 	reg = cpu_to_be32(FSI_MMODE_ECRC | FSI_MMODE_EPC | FSI_MMODE_RELA
-			| fsi_mmode_crs0(DEFAULT_DIVISOR)
-			| fsi_mmode_crs1(DEFAULT_DIVISOR)
+			| fsi_mmode_crs0(aspeed_fsi_divisor)
+			| fsi_mmode_crs1(aspeed_fsi_divisor)
 			| FSI_MMODE_P8_TO_LSB);
+	dev_info(aspeed->dev, "mmode set to %08x (divisor %d)\n",
+			be32_to_cpu(reg), aspeed_fsi_divisor);
 	opb_writel(aspeed, ctrl_base + FSI_MMODE, reg);
 
 	reg = cpu_to_be32(0xffff0000);
@@ -446,6 +452,11 @@ static int tacoma_cabled_fsi_fixup(struct device *dev)
 
 	/* If the routing GPIO is high we should set the mux to low. */
 	if (gpio) {
+		/*
+		 * Cable signal integrity means we should run the bus
+		 * slightly slower
+		 */
+		aspeed_fsi_divisor = FSI_DIVISOR_CABLED;
 		gpiod_direction_output(mux_gpio, 0);
 		dev_info(dev, "FSI configured for external cable\n");
 	} else {
-- 
2.27.0

[PATCH 4/5] fsi: aspeed: Add module param for bus divisor

[Joel Stanley]

For testing and hardware debugging a user may wish to override the
divisor at runtime. By setting fsi_master_aspeed.bus_div=N, the divisor
will be set to N, if 0 < N <= 0x3ff.

This is a module parameter and not a device tree option as it will only
need to be set when testing or debugging.

Reviewed-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/fsi/fsi-master-aspeed.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index f503f64ab32b..2531e826ba8b 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -87,6 +87,7 @@ static const u32 fsi_base = 0xa0000000;
 #define FSI_DIVISOR_DEFAULT            1
 #define FSI_DIVISOR_CABLED             2
 static u16 aspeed_fsi_divisor = FSI_DIVISOR_DEFAULT;
+module_param_named(bus_div,aspeed_fsi_divisor, ushort, 0);
 
 #define OPB_POLL_TIMEOUT		10000
 
@@ -454,9 +455,12 @@ static int tacoma_cabled_fsi_fixup(struct device *dev)
 	if (gpio) {
 		/*
 		 * Cable signal integrity means we should run the bus
-		 * slightly slower
+		 * slightly slower. Do not override if a kernel param
+		 * has already overridden.
 		 */
-		aspeed_fsi_divisor = FSI_DIVISOR_CABLED;
+		if (aspeed_fsi_divisor == FSI_DIVISOR_DEFAULT)
+			aspeed_fsi_divisor = FSI_DIVISOR_CABLED;
+
 		gpiod_direction_output(mux_gpio, 0);
 		dev_info(dev, "FSI configured for external cable\n");
 	} else {
-- 
2.27.0

[PATCH 5/5] fsi: aspeed: Support CFAM reset GPIO

[Joel Stanley]

Systems have a line for restting the remote CFAM. This is not part of
the FSI master, but is associated with it, so it makes sense to include
it in the master driver.

This exposes a sysfs interface to reset the cfam, abstracting away the
direction and polarity of the GPIO, as well as the timing of the reset
pulse. Userspace will be blocked until the reset pulse is finished.

The reset is hard coded to be in the range of (900, 1000) us. It was
observed with a scope to regularly be just over 1ms.

If the device tree property is not preset the driver will silently
continue.

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/fsi/fsi-master-aspeed.c | 43 +++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index 2531e826ba8b..c006ec008a1a 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -22,6 +22,7 @@ struct fsi_master_aspeed {
 	struct device		*dev;
 	void __iomem		*base;
 	struct clk		*clk;
+	struct gpio_desc	*cfam_reset_gpio;
 };
 
 #define to_fsi_master_aspeed(m) \
@@ -425,6 +426,43 @@ static int aspeed_master_init(struct fsi_master_aspeed *aspeed)
 	return 0;
 }
 
+static ssize_t cfam_reset_store(struct device *dev, struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct fsi_master_aspeed *aspeed = dev_get_drvdata(dev);
+
+	gpiod_set_value(aspeed->cfam_reset_gpio, 1);
+	usleep_range(900, 1000);
+	gpiod_set_value(aspeed->cfam_reset_gpio, 0);
+
+	return count;
+}
+
+static DEVICE_ATTR(cfam_reset, 0200, NULL, cfam_reset_store);
+
+static int setup_cfam_reset(struct fsi_master_aspeed *aspeed)
+{
+	struct device *dev = aspeed->dev;
+	struct gpio_desc *gpio;
+	int rc;
+
+	gpio = devm_gpiod_get_optional(dev, "cfam-reset", GPIOD_OUT_LOW);
+	if (IS_ERR(gpio))
+		return PTR_ERR(gpio);
+	if (!gpio)
+		return 0;
+
+	aspeed->cfam_reset_gpio = gpio;
+
+	rc = device_create_file(dev, &dev_attr_cfam_reset);
+	if (rc) {
+		devm_gpiod_put(dev, gpio);
+		return rc;
+	}
+
+	return 0;
+}
+
 static int tacoma_cabled_fsi_fixup(struct device *dev)
 {
 	struct gpio_desc *routing_gpio, *mux_gpio;
@@ -507,6 +545,11 @@ static int fsi_master_aspeed_probe(struct platform_device *pdev)
 		return rc;
 	}
 
+	rc = setup_cfam_reset(aspeed);
+	if (rc) {
+		dev_err(&pdev->dev, "CFAM reset GPIO setup failed\n");
+	}
+
 	writel(0x1, aspeed->base + OPB_CLK_SYNC);
 	writel(OPB1_XFER_ACK_EN | OPB0_XFER_ACK_EN,
 			aspeed->base + OPB_IRQ_MASK);
-- 
2.27.0