[PATCH] bus: ti-sysc: Don't reserve the system timers on AM33xx

[PATCH] bus: ti-sysc: Don't reserve the system timers on AM33xx

[Markus Schneider-Pargmann (TI)]

Commit
    3f6178392050 ("ti-sysc: allow OMAP2 and OMAP4 timers to be reserved on AM33xx")
introduced handling of am33 SoC timers that are already active as
reserved. During probe this means the clocks are enabled and a reference
is kept.

On AM33xx deep sleep is entered by the PM coprocessor which
requires the PER power domain to be idle. The system timer driver
disables the clock during suspend but the additional reference held by
ti-sysc leads to the PER power domain not being idle:

    pm33xx pm33xx: PM: Could not transition all powerdomains to target state

The reserved status is only needed where the
gpt12 system timer must be kept from idling, see commit
    3ff340e24c9d ("bus: ti-sysc: Fix gpt12 system timer issue with reserved status")

The timer configuration in the devicetree is the same on all SoCs, so
there is no reason to reserve the timers on AM33xx.

Return -ENODEV for the system timers on all other SoCs. Like -EBUSY it
stops the probe but the driver core doesn't report it as failure.

Fixes: 3f6178392050 ("ti-sysc: allow OMAP2 and OMAP4 timers to be reserved on AM33xx")
Cc: stable@vger.kernel.org
Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
---
 drivers/bus/ti-sysc.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index e118b900c9ac..922a90d4c5a3 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -48,7 +48,6 @@ enum sysc_soc {
 	SOC_UNKNOWN,
 	SOC_2420,
 	SOC_2430,
-	SOC_AM33,
 	SOC_3430,
 	SOC_AM35,
 	SOC_3630,
@@ -2917,7 +2916,6 @@ static void ti_sysc_idle(struct work_struct *work)
 static const struct soc_device_attribute sysc_soc_match[] = {
 	SOC_FLAG("OMAP242*", SOC_2420),
 	SOC_FLAG("OMAP243*", SOC_2430),
-	SOC_FLAG("AM33*", SOC_AM33),
 	SOC_FLAG("AM35*", SOC_AM35),
 	SOC_FLAG("OMAP3[45]*", SOC_3430),
 	SOC_FLAG("OMAP3[67]*", SOC_3630),
@@ -3124,13 +3122,12 @@ static int sysc_check_active_timer(struct sysc *ddata)
 	 * A to B4 at some point.
 	 */
 	switch (sysc_soc->soc) {
-	case SOC_AM33:
 	case SOC_3430:
 	case SOC_AM35:
 		error = -ENXIO;
 		break;
 	default:
-		error = -EBUSY;
+		error = -ENODEV;
 	}
 
 	if ((ddata->cfg.quirks & SYSC_QUIRK_NO_RESET_ON_INIT) &&

---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261030-v7-3-topic-am335lpm-ti-sysc-13f0219c9d67

Best regards,
--  
Markus Schneider-Pargmann (TI) <msp@baylibre.com>

Re: [PATCH] bus: ti-sysc: Don't reserve the system timers on AM33xx

[Judith Mendez]

Hi Markus

On 11/3/26 7:24 AM, Markus Schneider-Pargmann (TI) wrote:
> Commit
>      3f6178392050 ("ti-sysc: allow OMAP2 and OMAP4 timers to be reserved on AM33xx")
> introduced handling of am33 SoC timers that are already active as
> reserved. During probe this means the clocks are enabled and a reference
> is kept.
> 
> On AM33xx deep sleep is entered by the PM coprocessor which
> requires the PER power domain to be idle. The system timer driver
> disables the clock during suspend but the additional reference held by
> ti-sysc leads to the PER power domain not being idle:
> 
>      pm33xx pm33xx: PM: Could not transition all powerdomains to target state
> 
> The reserved status is only needed where the
> gpt12 system timer must be kept from idling, see commit
>      3ff340e24c9d ("bus: ti-sysc: Fix gpt12 system timer issue with reserved status")
> 
> The timer configuration in the devicetree is the same on all SoCs, so
> there is no reason to reserve the timers on AM33xx.
> 
> Return -ENODEV for the system timers on all other SoCs. Like -EBUSY it
> stops the probe but the driver core doesn't report it as failure.
> 
> Fixes: 3f6178392050 ("ti-sysc: allow OMAP2 and OMAP4 timers to be reserved on AM33xx")
> Cc: stable@vger.kernel.org
> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>

Thanks for the patch, tested on various AM3* boards.

Tested-by: Judith Mendez <jm@ti.com>

> ---
>   drivers/bus/ti-sysc.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> index e118b900c9ac..922a90d4c5a3 100644
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -48,7 +48,6 @@ enum sysc_soc {
>   	SOC_UNKNOWN,
>   	SOC_2420,
>   	SOC_2430,
> -	SOC_AM33,
>   	SOC_3430,
>   	SOC_AM35,
>   	SOC_3630,
> @@ -2917,7 +2916,6 @@ static void ti_sysc_idle(struct work_struct *work)
>   static const struct soc_device_attribute sysc_soc_match[] = {
>   	SOC_FLAG("OMAP242*", SOC_2420),
>   	SOC_FLAG("OMAP243*", SOC_2430),
> -	SOC_FLAG("AM33*", SOC_AM33),
>   	SOC_FLAG("AM35*", SOC_AM35),
>   	SOC_FLAG("OMAP3[45]*", SOC_3430),
>   	SOC_FLAG("OMAP3[67]*", SOC_3630),
> @@ -3124,13 +3122,12 @@ static int sysc_check_active_timer(struct sysc *ddata)
>   	 * A to B4 at some point.
>   	 */
>   	switch (sysc_soc->soc) {
> -	case SOC_AM33:
>   	case SOC_3430:
>   	case SOC_AM35:
>   		error = -ENXIO;
>   		break;
>   	default:
> -		error = -EBUSY;
> +		error = -ENODEV;
>   	}
>   
>   	if ((ddata->cfg.quirks & SYSC_QUIRK_NO_RESET_ON_INIT) &&
> 
> ---
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> change-id: 20261030-v7-3-topic-am335lpm-ti-sysc-13f0219c9d67
> 
> Best regards,
> --
> Markus Schneider-Pargmann (TI) <msp@baylibre.com>
> 

Re: [PATCH] bus: ti-sysc: Don't reserve the system timers on AM33xx

[Kory Maincent]

On 11/4/26 16:53, Judith Mendez wrote:
> Hi Markus
> 
> On 11/3/26 7:24 AM, Markus Schneider-Pargmann (TI) wrote:
>> Commit
>>      3f6178392050 ("ti-sysc: allow OMAP2 and OMAP4 timers to be 
>> reserved on AM33xx")
>> introduced handling of am33 SoC timers that are already active as
>> reserved. During probe this means the clocks are enabled and a reference
>> is kept.
>>
>> On AM33xx deep sleep is entered by the PM coprocessor which
>> requires the PER power domain to be idle. The system timer driver
>> disables the clock during suspend but the additional reference held by
>> ti-sysc leads to the PER power domain not being idle:
>>
>>      pm33xx pm33xx: PM: Could not transition all powerdomains to 
>> target state
>>
>> The reserved status is only needed where the
>> gpt12 system timer must be kept from idling, see commit
>>      3ff340e24c9d ("bus: ti-sysc: Fix gpt12 system timer issue with 
>> reserved status")
>>
>> The timer configuration in the devicetree is the same on all SoCs, so
>> there is no reason to reserve the timers on AM33xx.
>>
>> Return -ENODEV for the system timers on all other SoCs. Like -EBUSY it
>> stops the probe but the driver core doesn't report it as failure.
>>
>> Fixes: 3f6178392050 ("ti-sysc: allow OMAP2 and OMAP4 timers to be 
>> reserved on AM33xx")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
> 
> Thanks for the patch, tested on various AM3* boards.
> 
> Tested-by: Judith Mendez <jm@ti.com>

Tested-by: Kory Maincent <kory.maincent@bootlin.com>
On BeagleBone Green Eco.

Thank you!
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com