diff mbox series

[v2,4/4] pinctrl: stm32: make pinctrl use hwspinlock

Message ID 20181114090114.7727-5-benjamin.gaignard@st.com
State Superseded
Headers show
Series Add Hardware Spinlock class | expand

Commit Message

Benjamin Gaignard Nov. 14, 2018, 9:01 a.m. UTC 
From: Benjamin Gaignard <benjamin.gaignard@linaro.org>

Protect configuration registers with a hardware spinlock.

If a hwspinlock is defined in the device-tree node used it
to be sure that none of the others processors on the SoC could
change the configuration at the same time.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
version 2:
- be more verbose in commit message
- log the error after hwspinlock_get_by_index()

 arch/arm/dts/stm32mp157c-ed1.dts |  4 ++++
 drivers/pinctrl/pinctrl_stm32.c  | 27 +++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

Comments

Simon Glass Nov. 16, 2018, 12:07 a.m. UTC | #1 
On 14 November 2018 at 01:01, Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
> From: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>
> Protect configuration registers with a hardware spinlock.
>
> If a hwspinlock is defined in the device-tree node used it
> to be sure that none of the others processors on the SoC could
> change the configuration at the same time.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> ---
> version 2:
> - be more verbose in commit message
> - log the error after hwspinlock_get_by_index()
>
>  arch/arm/dts/stm32mp157c-ed1.dts |  4 ++++
>  drivers/pinctrl/pinctrl_stm32.c  | 27 +++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

with nit below

>
> diff --git a/arch/arm/dts/stm32mp157c-ed1.dts b/arch/arm/dts/stm32mp157c-ed1.dts
> index fc277dd7d2..7a9b742d36 100644
> --- a/arch/arm/dts/stm32mp157c-ed1.dts
> +++ b/arch/arm/dts/stm32mp157c-ed1.dts
> @@ -369,6 +369,10 @@
>         status = "okay";
>  };
>
> +&pinctrl {
> +       hwlocks = <&hwspinlock 0>;
> +};
> +
>  &usbphyc_port0 {
>         phy-supply = <&vdd_usb>;
>         vdda1v1-supply = <&reg11>;
> diff --git a/drivers/pinctrl/pinctrl_stm32.c b/drivers/pinctrl/pinctrl_stm32.c
> index 31285cdd57..5b63a2de15 100644
> --- a/drivers/pinctrl/pinctrl_stm32.c
> +++ b/drivers/pinctrl/pinctrl_stm32.c
> @@ -1,6 +1,7 @@
>  #include <common.h>
>  #include <dm.h>
>  #include <dm/pinctrl.h>
> +#include <hwspinlock.h>
>  #include <asm/arch/gpio.h>
>  #include <asm/gpio.h>
>  #include <asm/io.h>
> @@ -19,12 +20,20 @@ static int stm32_gpio_config(struct gpio_desc *desc,
>  {
>         struct stm32_gpio_priv *priv = dev_get_priv(desc->dev);
>         struct stm32_gpio_regs *regs = priv->regs;
> +       struct hwspinlock *hws = dev_get_priv(desc->dev->parent);

dev_get_parent()

>         u32 index;
> +       int ret;
>
>         if (!ctl || ctl->af > 15 || ctl->mode > 3 || ctl->otype > 1 ||
>             ctl->pupd > 2 || ctl->speed > 3)
>                 return -EINVAL;
>
> +       ret = hwspinlock_lock_timeout(hws, 1);
> +       if (ret == -ETIME) {
> +               dev_err(desc->dev, "HWSpinlock timeout\n");
> +               return ret;
> +       }
> +
>         index = (desc->offset & 0x07) * 4;
>         clrsetbits_le32(&regs->afr[desc->offset >> 3], AFR_MASK << index,
>                         ctl->af << index);
> @@ -39,6 +48,8 @@ static int stm32_gpio_config(struct gpio_desc *desc,
>         index = desc->offset;
>         clrsetbits_le32(&regs->otyper, OTYPE_MSK << index, ctl->otype << index);
>
> +       hwspinlock_unlock(hws);
> +
>         return 0;
>  }
>
> @@ -176,6 +187,20 @@ static int stm32_pinctrl_set_state_simple(struct udevice *dev,
>  }
>  #endif /* PINCTRL_FULL */
>
> +static int stm32_pinctrl_probe(struct udevice *dev)
> +{
> +       struct hwspinlock *hws = dev_get_priv(dev);
> +       int err;
> +
> +       /* hwspinlock property is optional, just log the error */
> +       err = hwspinlock_get_by_index(dev, 0, hws);
> +       if (err)
> +               debug("%s: hwspinlock_get_by_index may have failed (%d)\n",
> +                     __func__, err);
> +
> +       return 0;
> +}
> +
>  static struct pinctrl_ops stm32_pinctrl_ops = {
>  #if CONFIG_IS_ENABLED(PINCTRL_FULL)
>         .set_state              = stm32_pinctrl_set_state,
> @@ -200,4 +225,6 @@ U_BOOT_DRIVER(pinctrl_stm32) = {
>         .of_match       = stm32_pinctrl_ids,
>         .ops            = &stm32_pinctrl_ops,
>         .bind           = dm_scan_fdt_dev,
> +       .probe          = stm32_pinctrl_probe,
> +       .priv_auto_alloc_size = sizeof(struct hwspinlock),
>  };
> --
> 2.15.0
>
Patrice CHOTARD Nov. 16, 2018, 8:21 a.m. UTC | #2 
Hi Benjamin

On 11/14/18 10:01 AM, Benjamin Gaignard wrote:
> From: Benjamin Gaignard <benjamin.gaignard@linaro.org>

> 

> Protect configuration registers with a hardware spinlock.

> 

> If a hwspinlock is defined in the device-tree node used it

> to be sure that none of the others processors on the SoC could

> change the configuration at the same time.

> 

> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

> ---

> version 2:

> - be more verbose in commit message

> - log the error after hwspinlock_get_by_index()

> 

>  arch/arm/dts/stm32mp157c-ed1.dts |  4 ++++

>  drivers/pinctrl/pinctrl_stm32.c  | 27 +++++++++++++++++++++++++++

>  2 files changed, 31 insertions(+)

> 

> diff --git a/arch/arm/dts/stm32mp157c-ed1.dts b/arch/arm/dts/stm32mp157c-ed1.dts

> index fc277dd7d2..7a9b742d36 100644

> --- a/arch/arm/dts/stm32mp157c-ed1.dts

> +++ b/arch/arm/dts/stm32mp157c-ed1.dts

> @@ -369,6 +369,10 @@

>  	status = "okay";

>  };

>  

> +&pinctrl {

> +	hwlocks = <&hwspinlock 0>;

> +};

> +

>  &usbphyc_port0 {

>  	phy-supply = <&vdd_usb>;

>  	vdda1v1-supply = <&reg11>;

> diff --git a/drivers/pinctrl/pinctrl_stm32.c b/drivers/pinctrl/pinctrl_stm32.c

> index 31285cdd57..5b63a2de15 100644

> --- a/drivers/pinctrl/pinctrl_stm32.c

> +++ b/drivers/pinctrl/pinctrl_stm32.c

> @@ -1,6 +1,7 @@

>  #include <common.h>

>  #include <dm.h>

>  #include <dm/pinctrl.h>

> +#include <hwspinlock.h>

>  #include <asm/arch/gpio.h>

>  #include <asm/gpio.h>

>  #include <asm/io.h>

> @@ -19,12 +20,20 @@ static int stm32_gpio_config(struct gpio_desc *desc,

>  {

>  	struct stm32_gpio_priv *priv = dev_get_priv(desc->dev);

>  	struct stm32_gpio_regs *regs = priv->regs;

> +	struct hwspinlock *hws = dev_get_priv(desc->dev->parent);

>  	u32 index;

> +	int ret;

>  

>  	if (!ctl || ctl->af > 15 || ctl->mode > 3 || ctl->otype > 1 ||

>  	    ctl->pupd > 2 || ctl->speed > 3)

>  		return -EINVAL;

>  

> +	ret = hwspinlock_lock_timeout(hws, 1);

> +	if (ret == -ETIME) {

> +		dev_err(desc->dev, "HWSpinlock timeout\n");

> +		return ret;

> +	}

> +

>  	index = (desc->offset & 0x07) * 4;

>  	clrsetbits_le32(&regs->afr[desc->offset >> 3], AFR_MASK << index,

>  			ctl->af << index);

> @@ -39,6 +48,8 @@ static int stm32_gpio_config(struct gpio_desc *desc,

>  	index = desc->offset;

>  	clrsetbits_le32(&regs->otyper, OTYPE_MSK << index, ctl->otype << index);

>  

> +	hwspinlock_unlock(hws);

> +

>  	return 0;

>  }

>  

> @@ -176,6 +187,20 @@ static int stm32_pinctrl_set_state_simple(struct udevice *dev,

>  }

>  #endif /* PINCTRL_FULL */

>  

> +static int stm32_pinctrl_probe(struct udevice *dev)

> +{

> +	struct hwspinlock *hws = dev_get_priv(dev);

> +	int err;

> +

> +	/* hwspinlock property is optional, just log the error */

> +	err = hwspinlock_get_by_index(dev, 0, hws);

> +	if (err)

> +		debug("%s: hwspinlock_get_by_index may have failed (%d)\n",

> +		      __func__, err);

> +

> +	return 0;

> +}

> +

>  static struct pinctrl_ops stm32_pinctrl_ops = {

>  #if CONFIG_IS_ENABLED(PINCTRL_FULL)

>  	.set_state		= stm32_pinctrl_set_state,

> @@ -200,4 +225,6 @@ U_BOOT_DRIVER(pinctrl_stm32) = {

>  	.of_match	= stm32_pinctrl_ids,

>  	.ops		= &stm32_pinctrl_ops,

>  	.bind		= dm_scan_fdt_dev,

> +	.probe		= stm32_pinctrl_probe,

> +	.priv_auto_alloc_size = sizeof(struct hwspinlock),

>  };

> 



Reviewed-by: Patrice Chotard <patrice.chotard@st.com>


Thanks
diff mbox series

Patch

diff --git a/arch/arm/dts/stm32mp157c-ed1.dts b/arch/arm/dts/stm32mp157c-ed1.dts
index fc277dd7d2..7a9b742d36 100644
--- a/arch/arm/dts/stm32mp157c-ed1.dts
+++ b/arch/arm/dts/stm32mp157c-ed1.dts
@@ -369,6 +369,10 @@ 
 	status = "okay";
 };
 
+&pinctrl {
+	hwlocks = <&hwspinlock 0>;
+};
+
 &usbphyc_port0 {
 	phy-supply = <&vdd_usb>;
 	vdda1v1-supply = <&reg11>;
diff --git a/drivers/pinctrl/pinctrl_stm32.c b/drivers/pinctrl/pinctrl_stm32.c
index 31285cdd57..5b63a2de15 100644
--- a/drivers/pinctrl/pinctrl_stm32.c
+++ b/drivers/pinctrl/pinctrl_stm32.c
@@ -1,6 +1,7 @@ 
 #include <common.h>
 #include <dm.h>
 #include <dm/pinctrl.h>
+#include <hwspinlock.h>
 #include <asm/arch/gpio.h>
 #include <asm/gpio.h>
 #include <asm/io.h>
@@ -19,12 +20,20 @@  static int stm32_gpio_config(struct gpio_desc *desc,
 {
 	struct stm32_gpio_priv *priv = dev_get_priv(desc->dev);
 	struct stm32_gpio_regs *regs = priv->regs;
+	struct hwspinlock *hws = dev_get_priv(desc->dev->parent);
 	u32 index;
+	int ret;
 
 	if (!ctl || ctl->af > 15 || ctl->mode > 3 || ctl->otype > 1 ||
 	    ctl->pupd > 2 || ctl->speed > 3)
 		return -EINVAL;
 
+	ret = hwspinlock_lock_timeout(hws, 1);
+	if (ret == -ETIME) {
+		dev_err(desc->dev, "HWSpinlock timeout\n");
+		return ret;
+	}
+
 	index = (desc->offset & 0x07) * 4;
 	clrsetbits_le32(&regs->afr[desc->offset >> 3], AFR_MASK << index,
 			ctl->af << index);
@@ -39,6 +48,8 @@  static int stm32_gpio_config(struct gpio_desc *desc,
 	index = desc->offset;
 	clrsetbits_le32(&regs->otyper, OTYPE_MSK << index, ctl->otype << index);
 
+	hwspinlock_unlock(hws);
+
 	return 0;
 }
 
@@ -176,6 +187,20 @@  static int stm32_pinctrl_set_state_simple(struct udevice *dev,
 }
 #endif /* PINCTRL_FULL */
 
+static int stm32_pinctrl_probe(struct udevice *dev)
+{
+	struct hwspinlock *hws = dev_get_priv(dev);
+	int err;
+
+	/* hwspinlock property is optional, just log the error */
+	err = hwspinlock_get_by_index(dev, 0, hws);
+	if (err)
+		debug("%s: hwspinlock_get_by_index may have failed (%d)\n",
+		      __func__, err);
+
+	return 0;
+}
+
 static struct pinctrl_ops stm32_pinctrl_ops = {
 #if CONFIG_IS_ENABLED(PINCTRL_FULL)
 	.set_state		= stm32_pinctrl_set_state,
@@ -200,4 +225,6 @@  U_BOOT_DRIVER(pinctrl_stm32) = {
 	.of_match	= stm32_pinctrl_ids,
 	.ops		= &stm32_pinctrl_ops,
 	.bind		= dm_scan_fdt_dev,
+	.probe		= stm32_pinctrl_probe,
+	.priv_auto_alloc_size = sizeof(struct hwspinlock),
 };