Re: [RFC 1/5] pinctrl: rz-pfc: Add Renesas RZ pinctrl core module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux