[PATCH V2] clk: introduce u-boot,ignore-clk-defaults
Sean Anderson
seanga2 at gmail.com
Fri Nov 26 19:17:10 CET 2021
On 11/24/21 9:10 AM, Tom Rini wrote:
> On Tue, Nov 23, 2021 at 09:16:14PM -0500, Sean Anderson wrote:
>> On 11/22/21 10:02 PM, Peng Fan (OSS) wrote:
>>>> Subject: Re: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults
>>>>
>>>> On Mon, Nov 22, 2021 at 11:33:27AM +0800, Peng Fan (OSS) wrote:
>>>>> + Rob
>>>>>
>>>>> On 2021/11/20 20:57, Tom Rini wrote:
>>>>>> On Sat, Nov 20, 2021 at 12:10:54PM +0000, Peng Fan (OSS) wrote:
>>>>>>>> Subject: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults
>>>>>>>>
>>>>>>>> From: Peng Fan <peng.fan at nxp.com>
>>>>>>>>
>>>>>>>> Current code has a force clk_set_defaults in multiple stages,
>>>>>>>> U-Boot reuse the same device tree and Linux Kernel device tree,
>>>>>>>> but we not register all the clks as Linux Kernel, so
>>>>>>>> clk_set_defaults will fail and cause the clk provider registeration fail.
>>>>>>>>
>>>>>>>> So introduce a new property to ignore the default settings which
>>>>>>>> could be used by any node that wanna ignore default settings.
>>>>>>>>
>>>>>>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>>>>>> Signed-off-by: Peng Fan <peng.fan at nxp.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> V2:
>>>>>>>> Add R-b tag
>>>>>>>> Tom, Simon
>>>>>>>> After a thought, I think still put it as a u-boot thing.
>>>> assigned-clock-x is
>>>>>>>> actually Linux specific, however I could not add the new property
>>>> to Linux,
>>>>>>>> because we are supporting SystemReady-IR, we need the
>>>>>>>> assigned-clock-x property
>>>>>>>> in linux working and ignore it in U-Boot.
>>>>>>>
>>>>>>> Any more thoughts?
>>>>>>
>>>>>> Just my continued request that you treat this as generic and submit
>>>>>> the binding upstream so it can be in the device tree for the platform.
>>>>>>
>>>>>
>>>>> As Sean said, this is to serve cast that linux and U-Boot use the same
>>>>> device tree, I mean U-Boot runtime export device tree to linux for
>>>>> SR-IR (system-ready IR) booting.
>>>>>
>>>>> Linux needs assigned-clocks to some reason, but U-Boot not need that
>>>>> because the driver not added the support or not a must to have that.
>>>>>
>>>>> Because assigned-clocks failure in U-Boot will cause probe fail now,
>>>>> the device driver will report failure.
>>>>>
>>>>> You mean rename this to "ignore-clk-defaults" or keep
>>>>> "u-boot,ignore-clk-defauls" or "firmware,ignore-clk-defaults" to linux
>>>>> device tree binding?
>>>>>
>>>>> I could try to send to linux kernel with "firmware" as a prefix.
>>>>
>>>> What I mean is that first I'm not seeing the description of the property as
>>>> being clear enough, either in commit message or the binding itself.
>>>> That's why in my mind I keep seeing this as "we set the properties
>>>> linux,assigned-clocks and u-boot,ignore-clk-defaults" and I don't know why
>>>> we need both. Is there not something we can do based on seeing
>>>> linux,assigned-clocks ? Showing something that makes use of the property
>>>> you're wishing to add would be helpful. In fact, some specific dts snippets
>>>> would be helpful to understand what's going on here.
>>>
>>> clk: clock-controller at 30380000 {
>>> compatible = "fsl,imx8mp-ccm";
>>> reg = <0x30380000 0x10000>;
>>> #clock-cells = <1>;
>>> clocks = <&osc_32k>, <&osc_24m>, <&clk_ext1>, <&clk_ext2>,
>>> <&clk_ext3>, <&clk_ext4>;
>>> clock-names = "osc_32k", "osc_24m", "clk_ext1", "clk_ext2",
>>> "clk_ext3", "clk_ext4";
>>> assigned-clocks = <&clk IMX8MP_CLK_A53_SRC>,
>>> <&clk IMX8MP_CLK_A53_CORE>,
>>> <&clk IMX8MP_CLK_NOC>,
>>> <&clk IMX8MP_CLK_NOC_IO>,
>>> <&clk IMX8MP_CLK_GIC>,
>>> <&clk IMX8MP_CLK_AUDIO_AHB>,
>>> <&clk IMX8MP_CLK_AUDIO_AXI_SRC>,
>>> <&clk IMX8MP_CLK_IPG_AUDIO_ROOT>,
>>> <&clk IMX8MP_AUDIO_PLL1>,
>>> <&clk IMX8MP_AUDIO_PLL2>;
>>> assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>,
>>> <&clk IMX8MP_ARM_PLL_OUT>,
>>> <&clk IMX8MP_SYS_PLL2_1000M>,
>>> <&clk IMX8MP_SYS_PLL1_800M>,
>>> <&clk IMX8MP_SYS_PLL2_500M>,
>>> <&clk IMX8MP_SYS_PLL1_800M>,
>>> <&clk IMX8MP_SYS_PLL1_800M>;
>>> assigned-clock-rates = <0>, <0>,
>>> <1000000000>,
>>> <800000000>,
>>> <500000000>,
>>> <400000000>,
>>> <800000000>,
>>> <400000000>,
>>> <393216000>,
>>> <361267200>;
>>> u-boot,ignore-clk-defaults;
>>> };
>>>
>>> The above node is that I wanna have in U-Boot device tree.
>>> This node is same as the one used in linux device tree except the new
>>> property introduced here.
>>>
>>> In i.MX U-Boot, we not support the clks here in the clk driver, but linux needs the assigned-clocks property, so I could not delete it
>>> because U-Boot runtime export the device tree to Linux.
>>>
>>> So add a new property here for U-Boot specific usage, if the property
>>> is there, U-Boot ignore the assigned-clock settings for a node.
>>>
>>> I hope this is clear and please suggest what to do next.
>>
>> Hm. Well perhaps we can designate a specific return code to indicate
>> that we have a valid clock ID but it is just unimplemented. So in your
>> driver you would do
>>
>> ulong my_get_rate(struct clk *clk)
>> {
>> switch (clk->id) {
>> case MY_CLK:
>> ...
>> default:
>> if (clk->id < MY_CLK_MAX)
>> return -ENOSYS;
>> else
>> return -EINVAL;
>> }
>> }
>>
>> and then the clock rate assignment would see -ENOSYS and go on its merry
>> way.
>
> Or even just have the driver that matches on "fsl,imx8mp-ccm" have a
> log_debug("Leaving clocks as-is, OS will handle them\n");
> and returning success?
>
assigned-clocks is handled by the uclass, so that seems a bit invasive for me.
--Sean
More information about the U-Boot
mailing list