| Message ID | 20210518175153.3176764-1-robimarko@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | clk: qcom: ipq8074: fix PCI-E clock oops | expand |
Quoting Robert Marko (2021-05-18 10:51:53) > Fix PCI-E clock related kernel oops that are causes by missing > parent_names. > > Without the use of parent_names kernel will panic on > clk_core_get_parent_by_index() due to a NULL pointer. > > Without this earlycon is needed to even catch the OOPS as it will reset > the board before serial is initialized. Can you share the oops message here in the commit text? > > Fixes: f0cfcf1ade20 ("clk: qcom: ipq8074: Add missing clocks for pcie") > Signed-off-by: Robert Marko <robimarko@gmail.com> > --- > drivers/clk/qcom/gcc-ipq8074.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/clk/qcom/gcc-ipq8074.c b/drivers/clk/qcom/gcc-ipq8074.c > index 0c619ed35c82..8d8b1717a203 100644 > --- a/drivers/clk/qcom/gcc-ipq8074.c > +++ b/drivers/clk/qcom/gcc-ipq8074.c > @@ -4357,8 +4357,7 @@ static struct clk_rcg2 pcie0_rchng_clk_src = { > .parent_map = gcc_xo_gpll0_map, > .clkr.hw.init = &(struct clk_init_data){ > .name = "pcie0_rchng_clk_src", > - .parent_hws = (const struct clk_hw *[]) { > - &gpll0.clkr.hw }, > + .parent_names = gcc_xo_gpll0, This seems to imply that we need to have two parents but we didn't realize that was the case. Ouch! Please use a struct clk_parent_data array and then use the firmware name for XO and the clk_hw pointer for gpll0. > .num_parents = 2, > .ops = &clk_rcg2_ops, > }, > @@ -4372,8 +4371,8 @@ static struct clk_branch gcc_pcie0_rchng_clk = { > .enable_mask = BIT(1), > .hw.init = &(struct clk_init_data){ > .name = "gcc_pcie0_rchng_clk", > - .parent_hws = (const struct clk_hw *[]){ > - &pcie0_rchng_clk_src.clkr.hw, > + .parent_names = (const char *[]){ > + "pcie0_rchng_clk_src", > }, > .num_parents = 1, > .flags = CLK_SET_RATE_PARENT, > @@ -4390,8 +4389,8 @@ static struct clk_branch gcc_pcie0_axi_s_bridge_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_pcie0_axi_s_bridge_clk", > - .parent_hws = (const struct clk_hw *[]){ > - &pcie0_axi_clk_src.clkr.hw, > + .parent_names = (const char *[]){ > + "pcie0_axi_clk_src" These two hunks can be dropped.
On Mon, 28 Jun 2021 at 02:07, Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Robert Marko (2021-05-18 10:51:53) > > Fix PCI-E clock related kernel oops that are causes by missing > > parent_names. > > > > Without the use of parent_names kernel will panic on > > clk_core_get_parent_by_index() due to a NULL pointer. > > > > Without this earlycon is needed to even catch the OOPS as it will reset > > the board before serial is initialized. > > Can you share the oops message here in the commit text? > > > > > Fixes: f0cfcf1ade20 ("clk: qcom: ipq8074: Add missing clocks for pcie") > > Signed-off-by: Robert Marko <robimarko@gmail.com> > > --- > > drivers/clk/qcom/gcc-ipq8074.c | 11 +++++------ > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/clk/qcom/gcc-ipq8074.c b/drivers/clk/qcom/gcc-ipq8074.c > > index 0c619ed35c82..8d8b1717a203 100644 > > --- a/drivers/clk/qcom/gcc-ipq8074.c > > +++ b/drivers/clk/qcom/gcc-ipq8074.c > > @@ -4357,8 +4357,7 @@ static struct clk_rcg2 pcie0_rchng_clk_src = { > > .parent_map = gcc_xo_gpll0_map, > > .clkr.hw.init = &(struct clk_init_data){ > > .name = "pcie0_rchng_clk_src", > > - .parent_hws = (const struct clk_hw *[]) { > > - &gpll0.clkr.hw }, > > + .parent_names = gcc_xo_gpll0, > > This seems to imply that we need to have two parents but we didn't > realize that was the case. Ouch! Please use a struct clk_parent_data > array and then use the firmware name for XO and the clk_hw pointer for > gpll0. > > > .num_parents = 2, > > .ops = &clk_rcg2_ops, > > }, > > @@ -4372,8 +4371,8 @@ static struct clk_branch gcc_pcie0_rchng_clk = { > > .enable_mask = BIT(1), > > .hw.init = &(struct clk_init_data){ > > .name = "gcc_pcie0_rchng_clk", > > - .parent_hws = (const struct clk_hw *[]){ > > - &pcie0_rchng_clk_src.clkr.hw, > > + .parent_names = (const char *[]){ > > + "pcie0_rchng_clk_src", > > }, > > .num_parents = 1, > > .flags = CLK_SET_RATE_PARENT, > > @@ -4390,8 +4389,8 @@ static struct clk_branch gcc_pcie0_axi_s_bridge_clk = { > > .enable_mask = BIT(0), > > .hw.init = &(struct clk_init_data){ > > .name = "gcc_pcie0_axi_s_bridge_clk", > > - .parent_hws = (const struct clk_hw *[]){ > > - &pcie0_axi_clk_src.clkr.hw, > > + .parent_names = (const char *[]){ > > + "pcie0_axi_clk_src" > > These two hunks can be dropped. Hi Stephen, sorry for the really late reply. Essentially it looks this is not necessary in the newer kernels, I tested again with 5.14 and newer and I don't have a panic anymore. I was hitting the issue with 5.10 kernel. Regards, Robert
diff --git a/drivers/clk/qcom/gcc-ipq8074.c b/drivers/clk/qcom/gcc-ipq8074.c index 0c619ed35c82..8d8b1717a203 100644 --- a/drivers/clk/qcom/gcc-ipq8074.c +++ b/drivers/clk/qcom/gcc-ipq8074.c @@ -4357,8 +4357,7 @@ static struct clk_rcg2 pcie0_rchng_clk_src = { .parent_map = gcc_xo_gpll0_map, .clkr.hw.init = &(struct clk_init_data){ .name = "pcie0_rchng_clk_src", - .parent_hws = (const struct clk_hw *[]) { - &gpll0.clkr.hw }, + .parent_names = gcc_xo_gpll0, .num_parents = 2, .ops = &clk_rcg2_ops, }, @@ -4372,8 +4371,8 @@ static struct clk_branch gcc_pcie0_rchng_clk = { .enable_mask = BIT(1), .hw.init = &(struct clk_init_data){ .name = "gcc_pcie0_rchng_clk", - .parent_hws = (const struct clk_hw *[]){ - &pcie0_rchng_clk_src.clkr.hw, + .parent_names = (const char *[]){ + "pcie0_rchng_clk_src", }, .num_parents = 1, .flags = CLK_SET_RATE_PARENT, @@ -4390,8 +4389,8 @@ static struct clk_branch gcc_pcie0_axi_s_bridge_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_pcie0_axi_s_bridge_clk", - .parent_hws = (const struct clk_hw *[]){ - &pcie0_axi_clk_src.clkr.hw, + .parent_names = (const char *[]){ + "pcie0_axi_clk_src" }, .num_parents = 1, .flags = CLK_SET_RATE_PARENT,
Fix PCI-E clock related kernel oops that are causes by missing parent_names. Without the use of parent_names kernel will panic on clk_core_get_parent_by_index() due to a NULL pointer. Without this earlycon is needed to even catch the OOPS as it will reset the board before serial is initialized. Fixes: f0cfcf1ade20 ("clk: qcom: ipq8074: Add missing clocks for pcie") Signed-off-by: Robert Marko <robimarko@gmail.com> --- drivers/clk/qcom/gcc-ipq8074.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)