On 1/3/23 17:54, sergiu.m...@microchip.com wrote:
On 03.01.2023 17:31, Marek Vasut wrote:
On 1/3/23 15:30, Sergiu Moga wrote:
In order to have USB functionality, drivers for SAMA7's
USB 2.0 PHY's have been added. There is one driver
for UTMI clock's SFR and RESET required functionalities and
one for its three possible subclocks of the phy's themselves.
In order for this layout to properly work in conjunction with
CCF and DT, the former driver will also act as a clock provider
for the three phy's with the help of a custom hook into the
driver's of_xlate method.

[...]

+static int sama7_utmi_probe(struct udevice *dev)
+{
+     struct clk *utmi_parent_clk, *utmi_clk;
+     struct regmap *regmap_sfr;
+     struct reset_ctl *phy_reset;
+     int i;
+     char name[16];
+
+     utmi_parent_clk = devm_clk_get(dev, "utmi_clk");
+     if (IS_ERR(utmi_parent_clk))
+             return PTR_ERR(utmi_parent_clk);
+
+     regmap_sfr = syscon_regmap_lookup_by_phandle(dev, "sfr-phandle");
+     if (IS_ERR(regmap_sfr))
+             return PTR_ERR(regmap_sfr);
+
+     for (i = 0; i < ARRAY_SIZE(sama7_utmick); i++) {
+             snprintf(name, sizeof(name), "usb%d_reset", i);
+             phy_reset = devm_reset_control_get(dev, name);
+             if (IS_ERR(phy_reset))
+                     return PTR_ERR(phy_reset);
+
+             utmi_clk = sama7_utmi_clk_register(regmap_sfr, phy_reset,
+                                                sama7_utmick[i].n,
+                                                sama7_utmick[i].p,
+                                                sama7_utmick[i].id);
+             if (IS_ERR(utmi_clk))
+                     return PTR_ERR(utmi_clk);

Isn't this going to leak memory if i>0 and a failure occurs ? I think it
will, since sama7_utmi_clk_register() calls kzalloc() .

Yes, you are right. Perhaps something like this should work better
instead, wouldn't you agree?

   static int sama7_utmi_probe(struct udevice *dev)
   {
-       struct clk *utmi_parent_clk, *utmi_clk;
+       struct clk *utmi_parent_clk, *utmi_clk[ARRAY_SIZE(sama7_utmick)];
+       struct sama7_utmi_clk *uclk;
        struct regmap *regmap_sfr;
        struct reset_ctl *phy_reset;
-       int i;
+       int i, j;
        char name[16];

        utmi_parent_clk = devm_clk_get(dev, "utmi_clk");
@@ -153,12 +154,18 @@ static int sama7_utmi_probe(struct udevice *dev)
                if (IS_ERR(phy_reset))
                        return PTR_ERR(phy_reset);

-               utmi_clk = sama7_utmi_clk_register(regmap_sfr, phy_reset,
+               utmi_clk[i] = sama7_utmi_clk_register(regmap_sfr, phy_reset,
                                                   sama7_utmick[i].n,
                                                   sama7_utmick[i].p,
                                                   sama7_utmick[i].id);
-               if (IS_ERR(utmi_clk))
+               if (IS_ERR(utmi_clk)) {
+                       for (j = 0; j < i; j++) {
+                               uclk = to_sama7_utmi_clk(utmi_clk[j]);
+                               kfree(uclk);

Do the usual error handling path using goto:

{
...
if (IS_ERR(...))
  goto error_clk_register;
...

error_clk_register:
  kfree();
error_something_else:
  clean_up_other_stuff();
  return ret;
}

Reply via email to