Re: [PATCH 04/11] clk: ti: clkctrl: support multiple clkctrl nodes under a cm node

[Stephen Boyd <sboyd@kernel.org>]

Quoting Tero Kristo (2018-10-15 08:58:21)
> On 13/10/18 01:03, Stephen Boyd wrote:
> > Quoting Rob Herring (2018-10-12 14:45:08)
> >> On Fri, Oct 12, 2018 at 4:25 PM Stephen Boyd <sboyd@kernel.org> wrote:
> >>>
> >>> Quoting Tero Kristo (2018-08-31 08:07:00)
> >>>> @@ -492,19 +501,35 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
> >>>>
> >>>>          provider->base = of_iomap(node, 0);
> >>>>
> >>>> -       provider->clkdm_name = kmalloc(strlen(node->parent->name) + 3,
> >>>> -                                      GFP_KERNEL);
> >>>> -       if (!provider->clkdm_name) {
> >>>> -               kfree(provider);
> >>>> -               return;
> >>>> +       if (ti_clk_get_features()->flags & TI_CLK_CLKCTRL_COMPAT) {
> >>>> +               provider->clkdm_name = kmalloc(strlen(node->parent->name) + 3,
> >>>> +                                              GFP_KERNEL);
> >>>> +               if (!provider->clkdm_name) {
> >>>> +                       kfree(provider);
> >>>> +                       return;
> >>>> +               }
> >>>> +
> >>>> +               /*
> >>>> +                * Create default clkdm name, replace _cm from end of parent
> >>>> +                * node name with _clkdm
> >>>> +                */
> >>>> +               strcpy(provider->clkdm_name, node->parent->name);
> >>
> >> kasprintf can replace kmalloc, strlen and strcpy here and remove the
> >> direct access of .name in the process.
> >>
> >>>> +               provider->clkdm_name[strlen(provider->clkdm_name) - 2] = 0;
> >>>> +       } else {
> >>>> +               provider->clkdm_name = kmalloc(strlen(node->name), GFP_KERNEL);
> >>>> +               if (!provider->clkdm_name) {
> >>>> +                       kfree(provider);
> >>>> +                       return;
> >>>> +               }
> >>>> +
> >>>> +               /*
> >>>> +                * Create default clkdm name, replace _clkctrl from end of
> >>>> +                * node name with _clkdm
> >>>> +                */
> >>>> +               strcpy(provider->clkdm_name, node->name);
> >>
> >> and here.
> >>
> >>>> +               provider->clkdm_name[strlen(provider->clkdm_name) - 7] = 0;
> >>>>          }
> >>>>
> >>>
> >>> This conflicts with Rob's change so I tried to fix it up. Let me know if
> >>> something is horribly wrong with it. I suspect Rob will need to figure
> >>> out how to make node::name usage go away again, but I put it back.
> >>
> >> Then I don't have to find and fix. :)
> >>
> > 
> > Can you send a patch? Otherwise I will try and fix this up tomorrow.
> > You can use this git tree for reference:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-ti
> > 
> 
> Sorry, I've also been OoO for last week, just captured this.
> 
> Do you need help on this one? I can take a look at it tomorrow if there 
> is a specific merge conflict you want me to take a look at.

This is what I got:

----8<---
diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
index 955f2e26ab00..78deca44789d 100644
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -520,8 +520,7 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
 	provider->base = of_iomap(node, 0);
 
 	if (ti_clk_get_features()->flags & TI_CLK_CLKCTRL_COMPAT) {
-		provider->clkdm_name = kmalloc(strlen(node->parent->name) + 3,
-					       GFP_KERNEL);
+		provider->clkdm_name = kasprintf(GFP_KERNEL, "%pOFn%s", node->parent, "xxx");
 		if (!provider->clkdm_name) {
 			kfree(provider);
 			return;
@@ -531,10 +530,9 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
 		 * Create default clkdm name, replace _cm from end of parent
 		 * node name with _clkdm
 		 */
-		strcpy(provider->clkdm_name, node->parent->name);
-		provider->clkdm_name[strlen(provider->clkdm_name) - 2] = 0;
+		provider->clkdm_name[strlen(provider->clkdm_name) - 5] = 0;
 	} else {
-		provider->clkdm_name = kmalloc(strlen(node->name), GFP_KERNEL);
+		provider->clkdm_name = kasprintf(GFP_KERNEL, "%pOFn", node->name); 
 		if (!provider->clkdm_name) {
 			kfree(provider);
 			return;
@@ -544,7 +542,6 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
 		 * Create default clkdm name, replace _clkctrl from end of
 		 * node name with _clkdm
 		 */
-		strcpy(provider->clkdm_name, node->name);
 		provider->clkdm_name[strlen(provider->clkdm_name) - 7] = 0;
 	}