[PATCH v2 2/2] clk: imx8mq: Add a clock driver for the imx8mq

[Marek Vasut]

On 3/11/22 19:41, Angus Ainslie wrote:
> On 2022-03-11 10:05, Marek Vasut wrote:
>> On 3/11/22 18:02, Angus Ainslie wrote:
>>> On 2022-03-11 08:57, Marek Vasut wrote:
>>>> On 3/11/22 17:35, Angus Ainslie wrote:
>>>>> All of the PLLs and clocks are initialized so the subsystems below are
>>>>> functional and tested.
>>>>>
>>>>> 1) USB host and peripheral
>>>>> 2) ECSPI
>>>>> 3) UART
>>>>> 4) I2C all busses
>>>>> 5) USDHC for eMMC support
>>>>> 6) USB storage
>>>>> 7) GPIO
>>>>> 8) DRAM
>>>>>
>>>>> The PLL rate tables are from the kernel
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=43cdaa1567ad3931fbde438853947d45238cc040 
>>>>>
>>>>
>>>> That patch is three years old.
>>>> That patch is for MX8M Mini clock, not for MX8M(Q).
>>>>
>>>> You can use the abbreviated commit ID instead:
>>>> 43cdaa1567ad3 ("clk: imx8mm: Move 1443X/1416X PLL clock structure to 
>>>> common place")
>>>> But that seems to be the wrong commit.
>>>
>>> That's the commit where the imx8m PLL frequency table is moved to a 
>>> common file for use by all of the imx8m variants. The imx8mq linux 
>>> driver does not even use the frequency tables so there is not a 
>>> specific commit for it.
>>
>> Isn't large part of this driver coming from these tables ?
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/clk/imx/clk-imx8mq.c 
>>
> 
> When I said tables I was referring to the PLL frequency tables.

Ahh, hmmm, I see that we now have three copies of those PLL tables in 
each MX8M{M,N,P} driver and Linux instead has this in common code. Can 
you deduplicate the PLL tables before we add fourth copy ?

> The driver is modelled after the u-boot imx8mm u-boot driver with 
> register and mux updates from the imx8mq reference manual. Very little 
> comes from the imx8mq kernel driver. Mainly I just verified mux naming 
> and register offsets against that driver.

Would it make sense to pick the Linux kernel tables instead then, 
instead of hand-writing them from scratch ? That seems error prone.

> That reminds me that I need to send a kernel patch for the monitor muxes.

[...]