Re: [PATCH v13 4/5] clk: sophgo: Add SG2042 clock driver

[Chen Wang <unicorn_wang@outlook.com>]

On 2024/4/11 12:11, Stephen Boyd wrote:
> Quoting Chen Wang (2024-03-28 23:21:40)
[......]
>> +/*
>> + * Note: regarding names for mux clock, "0/1" or "div0/div1" means the
>> + * first/second parent input source, not the register value.
>> + * For example:
>> + * "clk_div_ddr01_0" is the name of Clock divider 0 control of DDR01, and
>> + * "clk_gate_ddr01_div0" is the gate clock in front of the "clk_div_ddr01_0",
>> + * they are both controlled by register CLKDIVREG27;
>> + * "clk_div_ddr01_1" is the name of Clock divider 1 control of DDR01, and
>> + * "clk_gate_ddr01_div1" is the gate clock in front of the "clk_div_ddr01_1",
>> + * they are both controlled by register CLKDIVREG28;
>> + * While for register value of mux selection, use Clock Select for DDR01’s clock
>> + * as example, see CLKSELREG0, bit[2].
>> + * 1: Select in_dpll0_clk as clock source, correspondng to the parent input
>> + *    source from "clk_div_ddr01_0".
>> + * 0: Select in_fpll_clk as clock source, corresponding to the parent input
>> + *    source from "clk_div_ddr01_1".
>> + * So we need a table to define the array of register values corresponding to
>> + * the parent index and tell CCF about this when registering mux clock.
>> + */
>> +static const u32 sg2042_mux_table[] = {1, 0};
>> +
>> +static const char *const clk_mux_ddr01_p[] = {
>> +                       "clk_div_ddr01_0", "clk_div_ddr01_1"};
>> +static const char *const clk_mux_ddr23_p[] = {
>> +                       "clk_div_ddr23_0", "clk_div_ddr23_1"};
>> +static const char *const clk_mux_rp_cpu_normal_p[] = {
>> +                       "clk_div_rp_cpu_normal_0", "clk_div_rp_cpu_normal_1"};
>> +static const char *const clk_mux_axi_ddr_p[] = {
>> +                       "clk_div_axi_ddr_0", "clk_div_axi_ddr_1"};
>> +
>> +static struct sg2042_mux_clock sg2042_mux_clks[] = {
>> +       SG2042_MUX(MUX_CLK_DDR01, "clk_mux_ddr01", clk_mux_ddr01_p,
> Please use struct clk_parent_data or struct clk_hw directly instead of
> string names.

Hi, Stephen,

I understand that for clk_init_data, parent_names/parent_data/parent_hws 
are all acceptable. Why do you only suggest me to use 
parent_data/parent_hws here? Can you please explain?

Thank you again for your careful review

Chen

[......]