[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1b3234ce-4526-4735-b9c0-c242e6cc3cf0@collabora.com>
Date: Tue, 18 Feb 2025 01:33:34 +0200
From: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
To: Heiko Stübner <heiko@...ech.de>,
Jianfeng Liu <liujianfeng1994@...il.com>
Cc: airlied@...il.com, andy.yan@...k-chips.com, conor+dt@...nel.org,
devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org,
hjc@...k-chips.com, kernel@...labora.com, krzk+dt@...nel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-rockchip@...ts.infradead.org, maarten.lankhorst@...ux.intel.com,
mripard@...nel.org, robh@...nel.org, simona@...ll.ch, tzimmermann@...e.de
Subject: Re: [PATCH 3/4] arm64: dts: rockchip: Add HDMI1 PHY PLL clock source
to VOP2 on RK3588
On 2/17/25 4:33 PM, Heiko Stübner wrote:
> Am Montag, 17. Februar 2025, 03:44:37 MEZ schrieb Jianfeng Liu:
>> Hi Cristian,
>>
>> On Sat, 15 Feb 2025 02:55:39 +0200, Cristian Ciocaltea wrote:
>>> The HDMI1 PHY PLL clock source cannot be added directly to vop node in
>>> rk3588-base.dtsi, along with the HDMI0 related one, because HDMI1 is an
>>> optional feature and its PHY node belongs to a separate (extra) DT file.
>>>
>>> Therefore, add the HDMI1 PHY PLL clock source to VOP2 by overwriting its
>>> clocks & clock-names properties in the extra DT file.
>>
>> There are boards that only use hdmi1 such as ROCK 5 ITX. So there are two
>> choices for this board:
>>
>> 1, Enable hdptxphy0 as dependency of vop although it is not really used.
>>
>> 2, Overwrite vop node at board dts to make it only use hdptxphy1 like:
>>
>> &vop {
>> clocks = <&cru ACLK_VOP>,
>> <&cru HCLK_VOP>,
>> <&cru DCLK_VOP0>,
>> <&cru DCLK_VOP1>,
>> <&cru DCLK_VOP2>,
>> <&cru DCLK_VOP3>,
>> <&cru PCLK_VOP_ROOT>,
>> <&hdptxphy1>;
>> clock-names = "aclk",
>> "hclk",
>> "dclk_vp0",
>> "dclk_vp1",
>> "dclk_vp2",
>> "dclk_vp3",
>> "pclk_vop",
>> "pll_hdmiphy1";
>> };
>>
>> What do you think of these two method?
>
> Going by the code in patch1 (+drm-misc) we have:
> vop2->pll_hdmiphy0 = devm_clk_get_optional(vop2->dev, "pll_hdmiphy0");
> +
> vop2->pll_hdmiphy1 = devm_clk_get_optional(vop2->dev, "pll_hdmiphy1");
>
> So the clock-reference to hdptxphy0 should just result in vop2->pll_hdmiphy0
> being NULL and thus ignored further on?
Yep, that is the intended behavior.
@Jianfeng: Did you encounter any particular issue with the current approach?
Powered by blists - more mailing lists