Re: [PATCH 2/5] clocksource/drivers/spacemit: Add K1 timer support

[Troy Mitchell]

On Wed, 4 Nov 2026 19:28:00 +0100, Iker Pedrosa wrote:
> > +       irq = platform_get_irq(pdev, 0);
> > +       if (irq < 0)
> > +               return irq;
>
> Consider using dev_err_probe() here for consistency with the rest of
> the function.
platform_get_irq() already prints an error through dev_err_probe(),
so I'd rather not print it twice.

> 32768 looks like a magic number. I'd recommend you to add a comment
> above explaining the clock rate bounds.

> 3 also looks line a magic number that could have an explanation. I'd
> suggest you to add a macro with a proper name to have it properly
> documented.
Agreed on both, will use named constants with comments in v2.

> The device tree bindings support both spacemit,k1-timer and
> spacemit,k3-timer, but the driver's of_match_table only lists K1. You
> should add K3 to the list.
K3 is described as "spacemit,k3-timer", "spacemit,k1-timer", so it
binds through the K1 fallback. I'll add K3 to the match table only if
it ever needs different handling.

Thanks,

-- 
Troy Mitchell

Attachment: signature.asc
Description: PGP signature