Hi,
On 07.01.25 15:37, Jules Maselbas wrote:
> Adds a "one-size fit all" clock driver to be used for complex
> "composite" clock, composed of: a gate, a mux and severals dividers.
> This clk driver could be used to make SoC specific drivers easier
> to port from linux sources, but this is not the case yet.
Just curious: Here you say tha this driver can make porting kernel
drivers easier, but at the end you say this one-size fits all is
barebox-specific?
> The pll-cpux is set to 816MHz and pll-periph0-2x is set to 1.2GHz.
>
> Signed-off-by: Jules Maselbas <jmaselbas@xxxxxxxx>
Reviewed-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
As I read through the code, some nitpicks below, but nothing critical.
> ---
> +#define MHZ (1000UL * 1000UL)
There is linux/units.h with HZ_PER_MHZ.
> +static struct clk_div_table div_apb1[] = {
> + { .val = 0, .div = 2 },
> + { .val = 1, .div = 2 },
> + { .val = 2, .div = 4 },
> + { .val = 3, .div = 8 },
> + { /* Sentinel */ },
If comma is removed, it's less likely to add a member after it by mistake.
> +static void sun50i_a64_resets_init(void __iomem *regs)
> +{
> + u32 rst;
> +
> + rst = 0 |
> + /* RST_USB_PHY0 */ BIT(0) |
> + /* RST_USB_PHY1 */ BIT(1) |
> + /* RST_USB_HSIC */ BIT(2);
Why not name this ${REG}_RST_USB_PHY0 and so on instead of the comments?
> +static inline void nop_delay(u32 cnt)
> +{
> + while (cnt--)
> + barrier();
While we use GCC by default, first clang support was recently added and
clang is known to do optimizations that would reorder cnt over a memory
clobber if cnt is held in a register. Better make cnt volatile here.
> +}
> +
> +/* a "one size fit all" clk for barebox */
Cheers,
Ahmad
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
