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

[Judith Mendez <jm@ti.com>]

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>
>