Re: [PATCH 05/15] fsi/fsi-master-gpio: Add "no-gpio-delays" option
On Tue, 2018-05-29 at 11:30 +1000, Benjamin Herrenschmidt wrote:
> This adds support for an optional device-tree property that
> makes the driver skip all the delays around clocking the
> GPIOs and set it in the device-tree of common POWER9 based
> OpenPower platforms.
>
> This useful on chips like the AST2500 where the GPIO block is
> running at a fairly low clock frequency (25Mhz typically). In
> this case, the delays are unnecessary and due to the low
> precision of the timers, actually quite harmful in terms of
> performance.
Forgot to add the DT binding documentation, I'll send it as a separate
patch.
> Signed-off-by: Benjamin Herrenschmidt
> Reviewed-by: Christopher Bostic
> ---
> arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 1 +
> .../boot/dts/aspeed-bmc-opp-witherspoon.dts | 1 +
> arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts| 1 +
> drivers/fsi/fsi-master-gpio.c | 20 +++
> 4 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> index 3a93ae77cde3..71cb9317d624 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> @@ -53,6 +53,7 @@
> compatible = "fsi-master-gpio", "fsi-master";
> #address-cells = <2>;
> #size-cells = <0>;
> + no-gpio-delays;
>
> clock-gpios = < ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
> data-gpios = < ASPEED_GPIO(AA, 2) GPIO_ACTIVE_HIGH>;
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> index dab467463c99..97cf819309b5 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -126,6 +126,7 @@
> compatible = "fsi-master-gpio", "fsi-master";
> #address-cells = <2>;
> #size-cells = <0>;
> + no-gpio-delays;
>
> clock-gpios = < ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
> data-gpios = < ASPEED_GPIO(E, 0) GPIO_ACTIVE_HIGH>;
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> index f3bc89a4262f..71a64730f080 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -86,6 +86,7 @@
> compatible = "fsi-master-gpio", "fsi-master";
> #address-cells = <2>;
> #size-cells = <0>;
> + no-gpio-delays;
>
> trans-gpios = < ASPEED_GPIO(O, 6) GPIO_ACTIVE_HIGH>;
> enable-gpios = < ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index d6508bbad1fb..c82bbd35276e 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -62,6 +62,7 @@ struct fsi_master_gpio {
> struct gpio_desc*gpio_enable; /* FSI enable */
> struct gpio_desc*gpio_mux; /* Mux control */
> boolexternal_mode;
> + boolno_delays;
> };
>
> #define CREATE_TRACE_POINTS
> @@ -79,9 +80,11 @@ static void clock_toggle(struct fsi_master_gpio *master,
> int count)
> int i;
>
> for (i = 0; i < count; i++) {
> - ndelay(FSI_GPIO_STD_DLY);
> + if (!master->no_delays)
> + ndelay(FSI_GPIO_STD_DLY);
> gpiod_set_value(master->gpio_clk, 0);
> - ndelay(FSI_GPIO_STD_DLY);
> + if (!master->no_delays)
> + ndelay(FSI_GPIO_STD_DLY);
> gpiod_set_value(master->gpio_clk, 1);
> }
> }
> @@ -90,10 +93,12 @@ static int sda_clock_in(struct fsi_master_gpio *master)
> {
> int in;
>
> - ndelay(FSI_GPIO_STD_DLY);
> + if (!master->no_delays)
> + ndelay(FSI_GPIO_STD_DLY);
> gpiod_set_value(master->gpio_clk, 0);
> in = gpiod_get_value(master->gpio_data);
> - ndelay(FSI_GPIO_STD_DLY);
> + if (!master->no_delays)
> + ndelay(FSI_GPIO_STD_DLY);
> gpiod_set_value(master->gpio_clk, 1);
> return in ? 1 : 0;
> }
> @@ -677,6 +682,13 @@ static int fsi_master_gpio_probe(struct platform_device
> *pdev)
> }
> master->gpio_mux = gpio;
>
> + /*
> + * Check if GPIO block is slow enought that no extra delays
> + * are necessary. This improves performance on ast2500 by
> + * an order of magnitude.
> + */
> + master->no_delays = device_property_present(>dev,
> "no-gpio-delays");
> +
> master->master.n_links = 1;
> master->master.flags = FSI_MASTER_FLAG_SWCLOCK;
> master->master.read = fsi_master_gpio_read;
Re: [PATCH 05/15] fsi/fsi-master-gpio: Add "no-gpio-delays" option
On Tue, 2018-05-29 at 11:30 +1000, Benjamin Herrenschmidt wrote:
> This adds support for an optional device-tree property that
> makes the driver skip all the delays around clocking the
> GPIOs and set it in the device-tree of common POWER9 based
> OpenPower platforms.
>
> This useful on chips like the AST2500 where the GPIO block is
> running at a fairly low clock frequency (25Mhz typically). In
> this case, the delays are unnecessary and due to the low
> precision of the timers, actually quite harmful in terms of
> performance.
Forgot to add the DT binding documentation, I'll send it as a separate
patch.
> Signed-off-by: Benjamin Herrenschmidt
> Reviewed-by: Christopher Bostic
> ---
> arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 1 +
> .../boot/dts/aspeed-bmc-opp-witherspoon.dts | 1 +
> arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts| 1 +
> drivers/fsi/fsi-master-gpio.c | 20 +++
> 4 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> index 3a93ae77cde3..71cb9317d624 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> @@ -53,6 +53,7 @@
> compatible = "fsi-master-gpio", "fsi-master";
> #address-cells = <2>;
> #size-cells = <0>;
> + no-gpio-delays;
>
> clock-gpios = < ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
> data-gpios = < ASPEED_GPIO(AA, 2) GPIO_ACTIVE_HIGH>;
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> index dab467463c99..97cf819309b5 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -126,6 +126,7 @@
> compatible = "fsi-master-gpio", "fsi-master";
> #address-cells = <2>;
> #size-cells = <0>;
> + no-gpio-delays;
>
> clock-gpios = < ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
> data-gpios = < ASPEED_GPIO(E, 0) GPIO_ACTIVE_HIGH>;
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> index f3bc89a4262f..71a64730f080 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -86,6 +86,7 @@
> compatible = "fsi-master-gpio", "fsi-master";
> #address-cells = <2>;
> #size-cells = <0>;
> + no-gpio-delays;
>
> trans-gpios = < ASPEED_GPIO(O, 6) GPIO_ACTIVE_HIGH>;
> enable-gpios = < ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index d6508bbad1fb..c82bbd35276e 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -62,6 +62,7 @@ struct fsi_master_gpio {
> struct gpio_desc*gpio_enable; /* FSI enable */
> struct gpio_desc*gpio_mux; /* Mux control */
> boolexternal_mode;
> + boolno_delays;
> };
>
> #define CREATE_TRACE_POINTS
> @@ -79,9 +80,11 @@ static void clock_toggle(struct fsi_master_gpio *master,
> int count)
> int i;
>
> for (i = 0; i < count; i++) {
> - ndelay(FSI_GPIO_STD_DLY);
> + if (!master->no_delays)
> + ndelay(FSI_GPIO_STD_DLY);
> gpiod_set_value(master->gpio_clk, 0);
> - ndelay(FSI_GPIO_STD_DLY);
> + if (!master->no_delays)
> + ndelay(FSI_GPIO_STD_DLY);
> gpiod_set_value(master->gpio_clk, 1);
> }
> }
> @@ -90,10 +93,12 @@ static int sda_clock_in(struct fsi_master_gpio *master)
> {
> int in;
>
> - ndelay(FSI_GPIO_STD_DLY);
> + if (!master->no_delays)
> + ndelay(FSI_GPIO_STD_DLY);
> gpiod_set_value(master->gpio_clk, 0);
> in = gpiod_get_value(master->gpio_data);
> - ndelay(FSI_GPIO_STD_DLY);
> + if (!master->no_delays)
> + ndelay(FSI_GPIO_STD_DLY);
> gpiod_set_value(master->gpio_clk, 1);
> return in ? 1 : 0;
> }
> @@ -677,6 +682,13 @@ static int fsi_master_gpio_probe(struct platform_device
> *pdev)
> }
> master->gpio_mux = gpio;
>
> + /*
> + * Check if GPIO block is slow enought that no extra delays
> + * are necessary. This improves performance on ast2500 by
> + * an order of magnitude.
> + */
> + master->no_delays = device_property_present(>dev,
> "no-gpio-delays");
> +
> master->master.n_links = 1;
> master->master.flags = FSI_MASTER_FLAG_SWCLOCK;
> master->master.read = fsi_master_gpio_read;
[PATCH 05/15] fsi/fsi-master-gpio: Add "no-gpio-delays" option
This adds support for an optional device-tree property that
makes the driver skip all the delays around clocking the
GPIOs and set it in the device-tree of common POWER9 based
OpenPower platforms.
This useful on chips like the AST2500 where the GPIO block is
running at a fairly low clock frequency (25Mhz typically). In
this case, the delays are unnecessary and due to the low
precision of the timers, actually quite harmful in terms of
performance.
Signed-off-by: Benjamin Herrenschmidt
Reviewed-by: Christopher Bostic
---
arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 1 +
.../boot/dts/aspeed-bmc-opp-witherspoon.dts | 1 +
arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts| 1 +
drivers/fsi/fsi-master-gpio.c | 20 +++
4 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
index 3a93ae77cde3..71cb9317d624 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
@@ -53,6 +53,7 @@
compatible = "fsi-master-gpio", "fsi-master";
#address-cells = <2>;
#size-cells = <0>;
+ no-gpio-delays;
clock-gpios = < ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
data-gpios = < ASPEED_GPIO(AA, 2) GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index dab467463c99..97cf819309b5 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -126,6 +126,7 @@
compatible = "fsi-master-gpio", "fsi-master";
#address-cells = <2>;
#size-cells = <0>;
+ no-gpio-delays;
clock-gpios = < ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
data-gpios = < ASPEED_GPIO(E, 0) GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
index f3bc89a4262f..71a64730f080 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
@@ -86,6 +86,7 @@
compatible = "fsi-master-gpio", "fsi-master";
#address-cells = <2>;
#size-cells = <0>;
+ no-gpio-delays;
trans-gpios = < ASPEED_GPIO(O, 6) GPIO_ACTIVE_HIGH>;
enable-gpios = < ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index d6508bbad1fb..c82bbd35276e 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -62,6 +62,7 @@ struct fsi_master_gpio {
struct gpio_desc*gpio_enable; /* FSI enable */
struct gpio_desc*gpio_mux; /* Mux control */
boolexternal_mode;
+ boolno_delays;
};
#define CREATE_TRACE_POINTS
@@ -79,9 +80,11 @@ static void clock_toggle(struct fsi_master_gpio *master, int
count)
int i;
for (i = 0; i < count; i++) {
- ndelay(FSI_GPIO_STD_DLY);
+ if (!master->no_delays)
+ ndelay(FSI_GPIO_STD_DLY);
gpiod_set_value(master->gpio_clk, 0);
- ndelay(FSI_GPIO_STD_DLY);
+ if (!master->no_delays)
+ ndelay(FSI_GPIO_STD_DLY);
gpiod_set_value(master->gpio_clk, 1);
}
}
@@ -90,10 +93,12 @@ static int sda_clock_in(struct fsi_master_gpio *master)
{
int in;
- ndelay(FSI_GPIO_STD_DLY);
+ if (!master->no_delays)
+ ndelay(FSI_GPIO_STD_DLY);
gpiod_set_value(master->gpio_clk, 0);
in = gpiod_get_value(master->gpio_data);
- ndelay(FSI_GPIO_STD_DLY);
+ if (!master->no_delays)
+ ndelay(FSI_GPIO_STD_DLY);
gpiod_set_value(master->gpio_clk, 1);
return in ? 1 : 0;
}
@@ -677,6 +682,13 @@ static int fsi_master_gpio_probe(struct platform_device
*pdev)
}
master->gpio_mux = gpio;
+ /*
+* Check if GPIO block is slow enought that no extra delays
+* are necessary. This improves performance on ast2500 by
+* an order of magnitude.
+*/
+ master->no_delays = device_property_present(>dev,
"no-gpio-delays");
+
master->master.n_links = 1;
master->master.flags = FSI_MASTER_FLAG_SWCLOCK;
master->master.read = fsi_master_gpio_read;
--
2.17.0
[PATCH 05/15] fsi/fsi-master-gpio: Add "no-gpio-delays" option
This adds support for an optional device-tree property that
makes the driver skip all the delays around clocking the
GPIOs and set it in the device-tree of common POWER9 based
OpenPower platforms.
This useful on chips like the AST2500 where the GPIO block is
running at a fairly low clock frequency (25Mhz typically). In
this case, the delays are unnecessary and due to the low
precision of the timers, actually quite harmful in terms of
performance.
Signed-off-by: Benjamin Herrenschmidt
Reviewed-by: Christopher Bostic
---
arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 1 +
.../boot/dts/aspeed-bmc-opp-witherspoon.dts | 1 +
arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts| 1 +
drivers/fsi/fsi-master-gpio.c | 20 +++
4 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
index 3a93ae77cde3..71cb9317d624 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
@@ -53,6 +53,7 @@
compatible = "fsi-master-gpio", "fsi-master";
#address-cells = <2>;
#size-cells = <0>;
+ no-gpio-delays;
clock-gpios = < ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
data-gpios = < ASPEED_GPIO(AA, 2) GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index dab467463c99..97cf819309b5 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -126,6 +126,7 @@
compatible = "fsi-master-gpio", "fsi-master";
#address-cells = <2>;
#size-cells = <0>;
+ no-gpio-delays;
clock-gpios = < ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
data-gpios = < ASPEED_GPIO(E, 0) GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
index f3bc89a4262f..71a64730f080 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
@@ -86,6 +86,7 @@
compatible = "fsi-master-gpio", "fsi-master";
#address-cells = <2>;
#size-cells = <0>;
+ no-gpio-delays;
trans-gpios = < ASPEED_GPIO(O, 6) GPIO_ACTIVE_HIGH>;
enable-gpios = < ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index d6508bbad1fb..c82bbd35276e 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -62,6 +62,7 @@ struct fsi_master_gpio {
struct gpio_desc*gpio_enable; /* FSI enable */
struct gpio_desc*gpio_mux; /* Mux control */
boolexternal_mode;
+ boolno_delays;
};
#define CREATE_TRACE_POINTS
@@ -79,9 +80,11 @@ static void clock_toggle(struct fsi_master_gpio *master, int
count)
int i;
for (i = 0; i < count; i++) {
- ndelay(FSI_GPIO_STD_DLY);
+ if (!master->no_delays)
+ ndelay(FSI_GPIO_STD_DLY);
gpiod_set_value(master->gpio_clk, 0);
- ndelay(FSI_GPIO_STD_DLY);
+ if (!master->no_delays)
+ ndelay(FSI_GPIO_STD_DLY);
gpiod_set_value(master->gpio_clk, 1);
}
}
@@ -90,10 +93,12 @@ static int sda_clock_in(struct fsi_master_gpio *master)
{
int in;
- ndelay(FSI_GPIO_STD_DLY);
+ if (!master->no_delays)
+ ndelay(FSI_GPIO_STD_DLY);
gpiod_set_value(master->gpio_clk, 0);
in = gpiod_get_value(master->gpio_data);
- ndelay(FSI_GPIO_STD_DLY);
+ if (!master->no_delays)
+ ndelay(FSI_GPIO_STD_DLY);
gpiod_set_value(master->gpio_clk, 1);
return in ? 1 : 0;
}
@@ -677,6 +682,13 @@ static int fsi_master_gpio_probe(struct platform_device
*pdev)
}
master->gpio_mux = gpio;
+ /*
+* Check if GPIO block is slow enought that no extra delays
+* are necessary. This improves performance on ast2500 by
+* an order of magnitude.
+*/
+ master->no_delays = device_property_present(>dev,
"no-gpio-delays");
+
master->master.n_links = 1;
master->master.flags = FSI_MASTER_FLAG_SWCLOCK;
master->master.read = fsi_master_gpio_read;
--
2.17.0
