[PATCH linux dev-5.4 0/2] fsi: aspeed: Divisor settings

[PATCH linux dev-5.4 0/2] fsi: aspeed: Divisor settings

[Joel Stanley]

This defaults the ASPEED FSI master to a faster bus speed, and allows a
user to override the speed using the kernel command line.

Joel Stanley (2):
  fsi: aspeed: Run the bus at maximum speed
  fsi: aspeed: Add module param for bus divisor

 drivers/fsi/fsi-master-aspeed.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH linux dev-5.4 1/2] 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.

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 fe2da6f90590..80bc9132e4f8 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,
@@ -389,9 +393,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);
@@ -450,6 +456,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.25.1

[PATCH linux dev-5.4 2/2] 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.

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 80bc9132e4f8..b44f71f1f0a8 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
 
@@ -458,9 +459,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.25.1

Re: [PATCH linux dev-5.4 1/2] fsi: aspeed: Run the bus at maximum speed

[Eddie James]

On 4/14/20 5:51 AM, Joel Stanley wrote:
> 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 fe2da6f90590..80bc9132e4f8 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,
> @@ -389,9 +393,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);
> @@ -450,6 +456,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 {

Re: [PATCH linux dev-5.4 2/2] fsi: aspeed: Add module param for bus divisor

[Eddie James]

On 4/14/20 5:51 AM, Joel Stanley wrote:
> 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.


Looks good.

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 80bc9132e4f8..b44f71f1f0a8 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
>   
> @@ -458,9 +459,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 {