diff mbox series

clocksource/drivers/sp804: avoid error on multiple instances

Message ID 20220427142759.4138694-1-andre.przywara@arm.com (mailing list archive)
State New, archived
Headers show
Series clocksource/drivers/sp804: avoid error on multiple instances | expand

Commit Message

Andre Przywara April 27, 2022, 2:27 p.m. UTC
When a machine sports more than one SP804 timer instance, we only bring
up the first one, since multiple timers of the same kind are not useful
to Linux. As this is intentional behaviour, we should not return an
error message, as we do today:
===============
[    0.000800] Failed to initialize '/bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/timer@120000': -22
===============

Replace the -EINVAL return with an informative message and return 0
instead.

Also we do not reach the init function anymore if the DT node is
disabled (as this is now handled by OF_DECLARE), so remove the explicit
check for that case.

This fixes a long standing bogus error when booting ARM's fastmodels.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/clocksource/timer-sp804.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Robin Murphy April 27, 2022, 3:42 p.m. UTC | #1
On 2022-04-27 15:27, Andre Przywara wrote:
> When a machine sports more than one SP804 timer instance, we only bring
> up the first one, since multiple timers of the same kind are not useful
> to Linux. As this is intentional behaviour, we should not return an
> error message, as we do today:
> ===============
> [    0.000800] Failed to initialize '/bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/timer@120000': -22
> ===============
> 
> Replace the -EINVAL return with an informative message and return 0
> instead.
> 
> Also we do not reach the init function anymore if the DT node is
> disabled (as this is now handled by OF_DECLARE), so remove the explicit
> check for that case.
> 
> This fixes a long standing bogus error when booting ARM's fastmodels.

And on Juno too, hooray! :)

> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>   drivers/clocksource/timer-sp804.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c
> index 401d592e85f5a..30cbc580e0448 100644
> --- a/drivers/clocksource/timer-sp804.c
> +++ b/drivers/clocksource/timer-sp804.c
> @@ -259,6 +259,12 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time
>   	struct clk *clk1, *clk2;
>   	const char *name = of_get_property(np, "compatible", NULL);
>   
> +	if (initialized) {
> +		pr_info("%s: only instantiating one timer\n",
> +			of_node_full_name(np));

Nit: %pOF? However, do we really need to print anything at all? As it 
is, the message already looks ambiguously like it might be saying that 
this node *is* the one timer that we're instantiating, however we'll 
have already logged the registration of an SP804 clocksource, so further 
messages about what we *aren't* doing seem little more useful than 
pr_info("not waxing an owl") IMO.

Cheers,
Robin.

> +		return 0;
> +	}
> +
>   	base = of_iomap(np, 0);
>   	if (!base)
>   		return -ENXIO;
> @@ -270,11 +276,6 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time
>   	writel(0, timer1_base + timer->ctrl);
>   	writel(0, timer2_base + timer->ctrl);
>   
> -	if (initialized || !of_device_is_available(np)) {
> -		ret = -EINVAL;
> -		goto err;
> -	}
> -
>   	clk1 = of_clk_get(np, 0);
>   	if (IS_ERR(clk1))
>   		clk1 = NULL;
diff mbox series

Patch

diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c
index 401d592e85f5a..30cbc580e0448 100644
--- a/drivers/clocksource/timer-sp804.c
+++ b/drivers/clocksource/timer-sp804.c
@@ -259,6 +259,12 @@  static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time
 	struct clk *clk1, *clk2;
 	const char *name = of_get_property(np, "compatible", NULL);
 
+	if (initialized) {
+		pr_info("%s: only instantiating one timer\n",
+			of_node_full_name(np));
+		return 0;
+	}
+
 	base = of_iomap(np, 0);
 	if (!base)
 		return -ENXIO;
@@ -270,11 +276,6 @@  static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time
 	writel(0, timer1_base + timer->ctrl);
 	writel(0, timer2_base + timer->ctrl);
 
-	if (initialized || !of_device_is_available(np)) {
-		ret = -EINVAL;
-		goto err;
-	}
-
 	clk1 = of_clk_get(np, 0);
 	if (IS_ERR(clk1))
 		clk1 = NULL;