[PATCH 0/2] hwspinlock/omap - new SoC support

[PATCH 0/2] hwspinlock/omap - new SoC support

[Suman Anna]

Hi Ohad,

The following two patches allow the OMAP hwspinlock driver
to be enabled for AM33xx, AM437x and DRA7xx SoCs.

The patches are rebased onto 3.16-rc3 and are identical to those
posted in the OMAP hwspinlock DT support v5 series [1][2]. Posting
these separately to decouple from the DT series, as per the discussion
on the lists.

regards
Suman

[1] https://patchwork.kernel.org/patch/4096831/
[2] https://patchwork.kernel.org/patch/4096761/

Suman Anna (2):
  hwspinlock/omap: enable module before reading SYSSTATUS register
  hwspinlock/omap: enable build for AM33xx, AM43xx & DRA7xx

 drivers/hwspinlock/Kconfig           |  2 +-
 drivers/hwspinlock/omap_hwspinlock.c | 27 ++++++++++++++++++++-------
 2 files changed, 21 insertions(+), 8 deletions(-)

-- 
2.0.0

[PATCH 0/2] hwspinlock/omap - new SoC support

[Suman Anna]

Hi Ohad,

The following two patches allow the OMAP hwspinlock driver
to be enabled for AM33xx, AM437x and DRA7xx SoCs.

The patches are rebased onto 3.16-rc3 and are identical to those
posted in the OMAP hwspinlock DT support v5 series [1][2]. Posting
these separately to decouple from the DT series, as per the discussion
on the lists.

regards
Suman

[1] https://patchwork.kernel.org/patch/4096831/
[2] https://patchwork.kernel.org/patch/4096761/

Suman Anna (2):
  hwspinlock/omap: enable module before reading SYSSTATUS register
  hwspinlock/omap: enable build for AM33xx, AM43xx & DRA7xx

 drivers/hwspinlock/Kconfig           |  2 +-
 drivers/hwspinlock/omap_hwspinlock.c | 27 ++++++++++++++++++++-------
 2 files changed, 21 insertions(+), 8 deletions(-)

-- 
2.0.0

[PATCH 1/2] hwspinlock/omap: enable module before reading SYSSTATUS register

[Suman Anna]

The number of hwspinlocks are determined based on the value read
from the IP block's SYSSTATUS register. However, the module may
not be enabled and clocked, and the read may result in a bus error.

This particular issue is seen rather easily on AM33XX, since the
module wakeup is software controlled, and it is disabled out of
reset. Make sure the module is enabled and clocked before reading
the SYSSTATUS register.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/hwspinlock/omap_hwspinlock.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/hwspinlock/omap_hwspinlock.c b/drivers/hwspinlock/omap_hwspinlock.c
index 292869c..deb9e13 100644
--- a/drivers/hwspinlock/omap_hwspinlock.c
+++ b/drivers/hwspinlock/omap_hwspinlock.c
@@ -98,10 +98,29 @@ static int omap_hwspinlock_probe(struct platform_device *pdev)
 	if (!io_base)
 		return -ENOMEM;
 
+	/*
+	 * make sure the module is enabled and clocked before reading
+	 * the module SYSSTATUS register
+	 */
+	pm_runtime_enable(&pdev->dev);
+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&pdev->dev);
+		goto iounmap_base;
+	}
+
 	/* Determine number of locks */
 	i = readl(io_base + SYSSTATUS_OFFSET);
 	i >>= SPINLOCK_NUMLOCKS_BIT_OFFSET;
 
+	/*
+	 * runtime PM will make sure the clock of this module is
+	 * enabled again iff at least one lock is requested
+	 */
+	ret = pm_runtime_put_sync(&pdev->dev);
+	if (ret < 0)
+		goto iounmap_base;
+
 	/* one of the four lsb's must be set, and nothing else */
 	if (hweight_long(i & 0xf) != 1 || i > 8) {
 		ret = -EINVAL;
@@ -121,12 +140,6 @@ static int omap_hwspinlock_probe(struct platform_device *pdev)
 	for (i = 0, hwlock = &bank->lock[0]; i < num_locks; i++, hwlock++)
 		hwlock->priv = io_base + LOCK_BASE_OFFSET + sizeof(u32) * i;
 
-	/*
-	 * runtime PM will make sure the clock of this module is
-	 * enabled iff at least one lock is requested
-	 */
-	pm_runtime_enable(&pdev->dev);
-
 	ret = hwspin_lock_register(bank, &pdev->dev, &omap_hwspinlock_ops,
 						pdata->base_id, num_locks);
 	if (ret)
@@ -135,9 +148,9 @@ static int omap_hwspinlock_probe(struct platform_device *pdev)
 	return 0;
 
 reg_fail:
-	pm_runtime_disable(&pdev->dev);
 	kfree(bank);
 iounmap_base:
+	pm_runtime_disable(&pdev->dev);
 	iounmap(io_base);
 	return ret;
 }
-- 
2.0.0

[PATCH 1/2] hwspinlock/omap: enable module before reading SYSSTATUS register

[Suman Anna]

The number of hwspinlocks are determined based on the value read
from the IP block's SYSSTATUS register. However, the module may
not be enabled and clocked, and the read may result in a bus error.

This particular issue is seen rather easily on AM33XX, since the
module wakeup is software controlled, and it is disabled out of
reset. Make sure the module is enabled and clocked before reading
the SYSSTATUS register.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/hwspinlock/omap_hwspinlock.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/hwspinlock/omap_hwspinlock.c b/drivers/hwspinlock/omap_hwspinlock.c
index 292869c..deb9e13 100644
--- a/drivers/hwspinlock/omap_hwspinlock.c
+++ b/drivers/hwspinlock/omap_hwspinlock.c
@@ -98,10 +98,29 @@ static int omap_hwspinlock_probe(struct platform_device *pdev)
 	if (!io_base)
 		return -ENOMEM;
 
+	/*
+	 * make sure the module is enabled and clocked before reading
+	 * the module SYSSTATUS register
+	 */
+	pm_runtime_enable(&pdev->dev);
+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&pdev->dev);
+		goto iounmap_base;
+	}
+
 	/* Determine number of locks */
 	i = readl(io_base + SYSSTATUS_OFFSET);
 	i >>= SPINLOCK_NUMLOCKS_BIT_OFFSET;
 
+	/*
+	 * runtime PM will make sure the clock of this module is
+	 * enabled again iff at least one lock is requested
+	 */
+	ret = pm_runtime_put_sync(&pdev->dev);
+	if (ret < 0)
+		goto iounmap_base;
+
 	/* one of the four lsb's must be set, and nothing else */
 	if (hweight_long(i & 0xf) != 1 || i > 8) {
 		ret = -EINVAL;
@@ -121,12 +140,6 @@ static int omap_hwspinlock_probe(struct platform_device *pdev)
 	for (i = 0, hwlock = &bank->lock[0]; i < num_locks; i++, hwlock++)
 		hwlock->priv = io_base + LOCK_BASE_OFFSET + sizeof(u32) * i;
 
-	/*
-	 * runtime PM will make sure the clock of this module is
-	 * enabled iff at least one lock is requested
-	 */
-	pm_runtime_enable(&pdev->dev);
-
 	ret = hwspin_lock_register(bank, &pdev->dev, &omap_hwspinlock_ops,
 						pdata->base_id, num_locks);
 	if (ret)
@@ -135,9 +148,9 @@ static int omap_hwspinlock_probe(struct platform_device *pdev)
 	return 0;
 
 reg_fail:
-	pm_runtime_disable(&pdev->dev);
 	kfree(bank);
 iounmap_base:
+	pm_runtime_disable(&pdev->dev);
 	iounmap(io_base);
 	return ret;
 }
-- 
2.0.0

[PATCH 2/2] hwspinlock/omap: enable build for AM33xx, AM43xx & DRA7xx

[Suman Anna]

HwSpinlocks are supported on AM33xx, AM43xx and DRA7xx SoC
device families as well. The IPs are identical to that of
OMAP4/OMAP5, except for the number of locks.

Add a depends on to the above family of SoCs to enable the
build support for OMAP hwspinlock driver for any of the above
SoC configs.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/hwspinlock/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig
index 70637d2..3612cb5 100644
--- a/drivers/hwspinlock/Kconfig
+++ b/drivers/hwspinlock/Kconfig
@@ -10,7 +10,7 @@ menu "Hardware Spinlock drivers"
 
 config HWSPINLOCK_OMAP
 	tristate "OMAP Hardware Spinlock device"
-	depends on ARCH_OMAP4 || SOC_OMAP5
+	depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX || SOC_AM33XX || SOC_AM43XX
 	select HWSPINLOCK
 	help
 	  Say y here to support the OMAP Hardware Spinlock device (firstly
-- 
2.0.0

[PATCH 2/2] hwspinlock/omap: enable build for AM33xx, AM43xx & DRA7xx

[Suman Anna]

HwSpinlocks are supported on AM33xx, AM43xx and DRA7xx SoC
device families as well. The IPs are identical to that of
OMAP4/OMAP5, except for the number of locks.

Add a depends on to the above family of SoCs to enable the
build support for OMAP hwspinlock driver for any of the above
SoC configs.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/hwspinlock/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig
index 70637d2..3612cb5 100644
--- a/drivers/hwspinlock/Kconfig
+++ b/drivers/hwspinlock/Kconfig
@@ -10,7 +10,7 @@ menu "Hardware Spinlock drivers"
 
 config HWSPINLOCK_OMAP
 	tristate "OMAP Hardware Spinlock device"
-	depends on ARCH_OMAP4 || SOC_OMAP5
+	depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX || SOC_AM33XX || SOC_AM43XX
 	select HWSPINLOCK
 	help
 	  Say y here to support the OMAP Hardware Spinlock device (firstly
-- 
2.0.0

Re: [PATCH 1/2] hwspinlock/omap: enable module before reading SYSSTATUS register

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Jul 3, 2014 at 2:00 AM, Suman Anna <s-anna@ti.com> wrote:
> The number of hwspinlocks are determined based on the value read
> from the IP block's SYSSTATUS register. However, the module may
> not be enabled and clocked, and the read may result in a bus error.
>
> This particular issue is seen rather easily on AM33XX, since the
> module wakeup is software controlled, and it is disabled out of
> reset. Make sure the module is enabled and clocked before reading
> the SYSSTATUS register.
>
> Signed-off-by: Suman Anna <s-anna@ti.com>
...
> +       /*
> +        * runtime PM will make sure the clock of this module is
> +        * enabled again iff at least one lock is requested
> +        */
> +       ret = pm_runtime_put_sync(&pdev->dev);

Is there a specific reason for using the put_sync variant here? If
not, let's just use the regular put().

Let me know, and I can do the change while applying these two patches.

Thanks,
Ohad.

[PATCH 1/2] hwspinlock/omap: enable module before reading SYSSTATUS register

[Ohad Ben-Cohen]

Hi Suman,

On Thu, Jul 3, 2014 at 2:00 AM, Suman Anna <s-anna@ti.com> wrote:
> The number of hwspinlocks are determined based on the value read
> from the IP block's SYSSTATUS register. However, the module may
> not be enabled and clocked, and the read may result in a bus error.
>
> This particular issue is seen rather easily on AM33XX, since the
> module wakeup is software controlled, and it is disabled out of
> reset. Make sure the module is enabled and clocked before reading
> the SYSSTATUS register.
>
> Signed-off-by: Suman Anna <s-anna@ti.com>
...
> +       /*
> +        * runtime PM will make sure the clock of this module is
> +        * enabled again iff at least one lock is requested
> +        */
> +       ret = pm_runtime_put_sync(&pdev->dev);

Is there a specific reason for using the put_sync variant here? If
not, let's just use the regular put().

Let me know, and I can do the change while applying these two patches.

Thanks,
Ohad.

Re: [PATCH 1/2] hwspinlock/omap: enable module before reading SYSSTATUS register

[Suman Anna]

Hi Ohad,

On 07/24/2014 08:45 AM, Ohad Ben-Cohen wrote:
> Hi Suman,
> 
> On Thu, Jul 3, 2014 at 2:00 AM, Suman Anna <s-anna@ti.com> wrote:
>> The number of hwspinlocks are determined based on the value read
>> from the IP block's SYSSTATUS register. However, the module may
>> not be enabled and clocked, and the read may result in a bus error.
>>
>> This particular issue is seen rather easily on AM33XX, since the
>> module wakeup is software controlled, and it is disabled out of
>> reset. Make sure the module is enabled and clocked before reading
>> the SYSSTATUS register.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
> ...
>> +       /*
>> +        * runtime PM will make sure the clock of this module is
>> +        * enabled again iff at least one lock is requested
>> +        */
>> +       ret = pm_runtime_put_sync(&pdev->dev);
> 
> Is there a specific reason for using the put_sync variant here? If
> not, let's just use the regular put().

There was no particular reason, you can change it.

> 
> Let me know, and I can do the change while applying these two patches.

Sure, thanks.

regards
Suman

[PATCH 1/2] hwspinlock/omap: enable module before reading SYSSTATUS register

[Suman Anna]

Hi Ohad,

On 07/24/2014 08:45 AM, Ohad Ben-Cohen wrote:
> Hi Suman,
> 
> On Thu, Jul 3, 2014 at 2:00 AM, Suman Anna <s-anna@ti.com> wrote:
>> The number of hwspinlocks are determined based on the value read
>> from the IP block's SYSSTATUS register. However, the module may
>> not be enabled and clocked, and the read may result in a bus error.
>>
>> This particular issue is seen rather easily on AM33XX, since the
>> module wakeup is software controlled, and it is disabled out of
>> reset. Make sure the module is enabled and clocked before reading
>> the SYSSTATUS register.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
> ...
>> +       /*
>> +        * runtime PM will make sure the clock of this module is
>> +        * enabled again iff at least one lock is requested
>> +        */
>> +       ret = pm_runtime_put_sync(&pdev->dev);
> 
> Is there a specific reason for using the put_sync variant here? If
> not, let's just use the regular put().

There was no particular reason, you can change it.

> 
> Let me know, and I can do the change while applying these two patches.

Sure, thanks.

regards
Suman

Re: [PATCH 1/2] hwspinlock/omap: enable module before reading SYSSTATUS register

[Ohad Ben-Cohen]

On Thu, Jul 24, 2014 at 6:13 PM, Suman Anna <s-anna@ti.com> wrote:
>> Is there a specific reason for using the put_sync variant here? If
>> not, let's just use the regular put().
>
> There was no particular reason, you can change it.

Thanks for confirming. I've applied both patches to remoteproc's
for-next branch.

Thanks,
Ohad.

[PATCH 1/2] hwspinlock/omap: enable module before reading SYSSTATUS register

[Ohad Ben-Cohen]

On Thu, Jul 24, 2014 at 6:13 PM, Suman Anna <s-anna@ti.com> wrote:
>> Is there a specific reason for using the put_sync variant here? If
>> not, let's just use the regular put().
>
> There was no particular reason, you can change it.

Thanks for confirming. I've applied both patches to remoteproc's
for-next branch.

Thanks,
Ohad.