| Message ID | 20181204193416.24661-1-sboyd@kernel.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | clk: socfpga: Don't have get_parent for single parent ops | expand |
(Adding Dinh's korg email) I also wonder if this driver is even used anymore or maybe we can delete it? Quoting Stephen Boyd (2018-12-04 11:34:16) > This driver creates a gate clk with the possibility to have multiple > parents. That can cause problems if the common clk framework tries to > call the get_parent() op and gets back a number that's larger than the > number of parents the clk says it supports in > clk_init_data::num_parents. Let's duplicate the clk_ops structure each > time this function is called and drop the get/set parent ops when there > is only one parent. This allows the framework to consider a number > larger than clk_init_data::num_parents as an error condition of the > get_parent() clk op, clearing the way for proper code. > > Cc: Jerome Brunet <jbrunet@baylibre.com> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > Signed-off-by: Stephen Boyd <sboyd@kernel.org> > --- > drivers/clk/socfpga/clk-gate.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/drivers/clk/socfpga/clk-gate.c b/drivers/clk/socfpga/clk-gate.c > index aa7a6e6a15b6..73e03328d5c5 100644 > --- a/drivers/clk/socfpga/clk-gate.c > +++ b/drivers/clk/socfpga/clk-gate.c > @@ -176,8 +176,7 @@ static struct clk_ops gateclk_ops = { > .set_parent = socfpga_clk_set_parent, > }; > > -static void __init __socfpga_gate_init(struct device_node *node, > - const struct clk_ops *ops) > +void __init socfpga_gate_init(struct device_node *node) > { > u32 clk_gate[2]; > u32 div_reg[3]; > @@ -188,12 +187,17 @@ static void __init __socfpga_gate_init(struct device_node *node, > const char *clk_name = node->name; > const char *parent_name[SOCFPGA_MAX_PARENTS]; > struct clk_init_data init; > + struct clk_ops *ops; > int rc; > > socfpga_clk = kzalloc(sizeof(*socfpga_clk), GFP_KERNEL); > if (WARN_ON(!socfpga_clk)) > return; > > + ops = kmemdup(&gateclk_ops, sizeof(gateclk_ops), GFP_KERNEL); > + if (WARN_ON(!ops)) > + return; > + > rc = of_property_read_u32_array(node, "clk-gate", clk_gate, 2); > if (rc) > clk_gate[0] = 0; > @@ -202,8 +206,8 @@ static void __init __socfpga_gate_init(struct device_node *node, > socfpga_clk->hw.reg = clk_mgr_base_addr + clk_gate[0]; > socfpga_clk->hw.bit_idx = clk_gate[1]; > > - gateclk_ops.enable = clk_gate_ops.enable; > - gateclk_ops.disable = clk_gate_ops.disable; > + ops->enable = clk_gate_ops.enable; > + ops->disable = clk_gate_ops.disable; > } > > rc = of_property_read_u32(node, "fixed-divider", &fixed_div); > @@ -234,6 +238,11 @@ static void __init __socfpga_gate_init(struct device_node *node, > init.flags = 0; > > init.num_parents = of_clk_parent_fill(node, parent_name, SOCFPGA_MAX_PARENTS); > + if (init.num_parents < 2) { > + ops->get_parent = NULL; > + ops->set_parent = NULL; > + } > + > init.parent_names = parent_name; > socfpga_clk->hw.hw.init = &init; > > @@ -246,8 +255,3 @@ static void __init __socfpga_gate_init(struct device_node *node, > if (WARN_ON(rc)) > return; > } > - > -void __init socfpga_gate_init(struct device_node *node) > -{ > - __socfpga_gate_init(node, &gateclk_ops); > -}
Hi Stephen, On 12/5/18 1:17 AM, Stephen Boyd wrote: > (Adding Dinh's korg email) > > I also wonder if this driver is even used anymore or maybe we can delete > it? > The armv7 SoCFPGA platforms are using this driver. Dinh
Quoting Dinh Nguyen (2018-12-05 07:17:41) > Hi Stephen, > > On 12/5/18 1:17 AM, Stephen Boyd wrote: > > (Adding Dinh's korg email) > > > > I also wonder if this driver is even used anymore or maybe we can delete > > it? > > > > The armv7 SoCFPGA platforms are using this driver. > Ok and those platforms are booting in kernelci.org as https://kernelci.org/boot/socfpga_cyclone5_de0_sockit/ perhaps?
On 12/5/18 9:55 AM, Stephen Boyd wrote: > Quoting Dinh Nguyen (2018-12-05 07:17:41) >> Hi Stephen, >> >> On 12/5/18 1:17 AM, Stephen Boyd wrote: >>> (Adding Dinh's korg email) >>> >>> I also wonder if this driver is even used anymore or maybe we can delete >>> it? >>> >> >> The armv7 SoCFPGA platforms are using this driver. >> > > Ok and those platforms are booting in kernelci.org as > https://kernelci.org/boot/socfpga_cyclone5_de0_sockit/ perhaps? > Yes that's right. Dinh
Quoting Dinh Nguyen (2018-12-06 07:16:47) > > > On 12/5/18 9:55 AM, Stephen Boyd wrote: > > Quoting Dinh Nguyen (2018-12-05 07:17:41) > >> Hi Stephen, > >> > >> On 12/5/18 1:17 AM, Stephen Boyd wrote: > >>> (Adding Dinh's korg email) > >>> > >>> I also wonder if this driver is even used anymore or maybe we can delete > >>> it? > >>> > >> > >> The armv7 SoCFPGA platforms are using this driver. > >> > > > > Ok and those platforms are booting in kernelci.org as > > https://kernelci.org/boot/socfpga_cyclone5_de0_sockit/ perhaps? > > > > Yes that's right. > Ok. Dinh, can you review this patch? Or test it out? Or do I need to get someone in kernelci to try it out?
On 12/17/18 7:54 PM, Stephen Boyd wrote: > Quoting Dinh Nguyen (2018-12-06 07:16:47) >> >> >> On 12/5/18 9:55 AM, Stephen Boyd wrote: >>> Quoting Dinh Nguyen (2018-12-05 07:17:41) >>>> Hi Stephen, >>>> >>>> On 12/5/18 1:17 AM, Stephen Boyd wrote: >>>>> (Adding Dinh's korg email) >>>>> >>>>> I also wonder if this driver is even used anymore or maybe we can delete >>>>> it? >>>>> >>>> >>>> The armv7 SoCFPGA platforms are using this driver. >>>> >>> >>> Ok and those platforms are booting in kernelci.org as >>> https://kernelci.org/boot/socfpga_cyclone5_de0_sockit/ perhaps? >>> >> >> Yes that's right. >> > > Ok. Dinh, can you review this patch? Or test it out? Or do I need to get > someone in kernelci to try it out? > Just tested on an Arria5 board, feel free to add: Tested-by: Dinh Nguyen <dinguyen@kernel.org> Thanks, Dinh
diff --git a/drivers/clk/socfpga/clk-gate.c b/drivers/clk/socfpga/clk-gate.c index aa7a6e6a15b6..73e03328d5c5 100644 --- a/drivers/clk/socfpga/clk-gate.c +++ b/drivers/clk/socfpga/clk-gate.c @@ -176,8 +176,7 @@ static struct clk_ops gateclk_ops = { .set_parent = socfpga_clk_set_parent, }; -static void __init __socfpga_gate_init(struct device_node *node, - const struct clk_ops *ops) +void __init socfpga_gate_init(struct device_node *node) { u32 clk_gate[2]; u32 div_reg[3]; @@ -188,12 +187,17 @@ static void __init __socfpga_gate_init(struct device_node *node, const char *clk_name = node->name; const char *parent_name[SOCFPGA_MAX_PARENTS]; struct clk_init_data init; + struct clk_ops *ops; int rc; socfpga_clk = kzalloc(sizeof(*socfpga_clk), GFP_KERNEL); if (WARN_ON(!socfpga_clk)) return; + ops = kmemdup(&gateclk_ops, sizeof(gateclk_ops), GFP_KERNEL); + if (WARN_ON(!ops)) + return; + rc = of_property_read_u32_array(node, "clk-gate", clk_gate, 2); if (rc) clk_gate[0] = 0; @@ -202,8 +206,8 @@ static void __init __socfpga_gate_init(struct device_node *node, socfpga_clk->hw.reg = clk_mgr_base_addr + clk_gate[0]; socfpga_clk->hw.bit_idx = clk_gate[1]; - gateclk_ops.enable = clk_gate_ops.enable; - gateclk_ops.disable = clk_gate_ops.disable; + ops->enable = clk_gate_ops.enable; + ops->disable = clk_gate_ops.disable; } rc = of_property_read_u32(node, "fixed-divider", &fixed_div); @@ -234,6 +238,11 @@ static void __init __socfpga_gate_init(struct device_node *node, init.flags = 0; init.num_parents = of_clk_parent_fill(node, parent_name, SOCFPGA_MAX_PARENTS); + if (init.num_parents < 2) { + ops->get_parent = NULL; + ops->set_parent = NULL; + } + init.parent_names = parent_name; socfpga_clk->hw.hw.init = &init; @@ -246,8 +255,3 @@ static void __init __socfpga_gate_init(struct device_node *node, if (WARN_ON(rc)) return; } - -void __init socfpga_gate_init(struct device_node *node) -{ - __socfpga_gate_init(node, &gateclk_ops); -}
This driver creates a gate clk with the possibility to have multiple parents. That can cause problems if the common clk framework tries to call the get_parent() op and gets back a number that's larger than the number of parents the clk says it supports in clk_init_data::num_parents. Let's duplicate the clk_ops structure each time this function is called and drop the get/set parent ops when there is only one parent. This allows the framework to consider a number larger than clk_init_data::num_parents as an error condition of the get_parent() clk op, clearing the way for proper code. Cc: Jerome Brunet <jbrunet@baylibre.com> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> Cc: Dinh Nguyen <dinguyen@opensource.altera.com> Signed-off-by: Stephen Boyd <sboyd@kernel.org> --- drivers/clk/socfpga/clk-gate.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)