Hi Chris,

On 30/01/2017 20:19, Chris Brandt wrote:
Hi Jacopo,

On Wednesday, January 25, 2017, Jacopo Mondi wrote:
+
+       return 0;
+
+free_map:
+       devm_kfree(rz_pinctrl->dev, *map);
+free_fngrps:
+       devm_kfree(rz_pinctrl->dev, fngrps);
+free_pins:
+       devm_kfree(rz_pinctrl->dev, mux_modes);
+       devm_kfree(rz_pinctrl->dev, grpins);
+       return ret;
+}

Since one of the benefits of using devm_kzalloc is that if
the probe fails and returns an error, all the memory associated with
that device will automatically get freed, you 'might' not
need this code to free memory.

I say might because I'm not sure if returning an error here will
kill the driver or not. But, might be interesting to look into.


No, returning an error here would not kill the driver BUT if dt_node_to_map fails, dt_free_map is called immediately later [1]

So here we maybe should not use device managed memory as it does not bring anything, do not free *map as it is freed later in dt_free_map, but release fngrps mux_modes and grpins, as we lose reference to them outside the scope of this function.

Do you agree?



+#define RZ_PIN_NAME(bank, pin)                                         \
+       PIN_##bank##_##pin
+
+#define RZ_PIN_DESC(b, p)                                              \
+       { .number = RZ_PIN_NAME(b, p),                                  \
+         .name = __stringify(RZ_PIN_NAME(b, p)),                       \

The hardware manual uses the names "P1_0" for ports, so it might
be better to match that format for consistency.



Noted

Thanks
   j


[1] http://lxr.free-electrons.com/source/drivers/pinctrl/devicetree.c#L236


Reply via email to