lkml.org 
[lkml]   [2023]   [Aug]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 3/3] clk: qcom: add clock controller driver for qca8386/qca8084
From


On 8/10/2023 8:59 PM, Konrad Dybcio wrote:
> On 10.08.2023 13:54, Luo Jie wrote:
>> Add clock & reset controller driver for qca8386/qca8084.
>>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
>
>> +struct qcom_cc {
>> + struct qcom_reset_controller reset;
>> + struct clk_regmap **rclks;
>> + size_t num_rclks;
>> +};
> This all, including the probe func, is required because of the MDIO dance,
> I assume?
>
> Commonizing that would make more sense should more clocks like this appear
> in the future.
>
Hi Konrad,
Thanks for the review.
the structure qcom_cc is not because of MDIO dance, this is the common
structure used by other qcom clock drivers such as gcc-ipq9574 in the
probe function.

seems that we can't make any more commonization on this, please let me
know if there is any idea on this.

> [...]
>
>> +static struct clk_branch nss_cc_switch_core_clk = {
>> + .halt_reg = 0x8,
>> + .clkr = {
>> + .enable_reg = 0x8,
>> + .enable_mask = BIT(0),
>> + .hw.init = &(const struct clk_init_data) {
>> + .name = "nss_cc_switch_core_clk",
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &nss_cc_switch_core_clk_src.clkr.hw,
>> + },
>> + .num_parents = 1,
>> + /* Can be disabled in PHY mode for power saving */
> Well it clearly cannot be disabled if it has the CLK_IS_CRITICAL flag :D
>
> What's the "PHY mode" you're talking about?
Yes, the clock with flag CLK_IS_CRITICAL needs to be enabled, the
hardware register needs to be configured to enable it, it is disabled by
default.

this clock branch is necessary for the switch mode that has switch
features such routing and fdb forwarding, i will remove the
CLK_IS_CRITICAL from the clock that is not needed for the PHY mode, we
can enable it later when configuring it as switch mode.

As for PHY mode, the device works as only PHY device, that includes 4
PHY ports, no switch functions.

>
>> + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>> + .ops = &clk_branch2_mdio_ops,
>> + },
>> + },
>> +};
> I see a whole bunch of CRITICAL clocks.. please make sure these clocks
> are actually necessary for Linux to know about (i.e. if we don't need
> to call any operations on them, we might just skip registering them
> with the driver).
>
> Konrad
>
yes, Konrad, the clocks with CLK_IS_CRITICAL need to be enabled
manually, all clocks are disabled after reset(by default).

\
 
 \ /
  Last update: 2023-08-11 13:51    [W:0.061 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site