Hi Jacopo, On Tuesday, January 31, 2017, Jacopo Mondi wrote: > > 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? Like you said, there is no benefit one way of the other. But, doing a grep of the pinctrl directory, devm_kzalloc is used more by the other drivers than kzalloc, so maybe we just stick with devm_kzalloc (baah goes the sheep) Chris > -----Original Message----- > From: jacopo mondi [mailto:jacopo@xxxxxxxxxx] > Sent: Tuesday, January 31, 2017 4:01 AM > To: Chris Brandt <Chris.Brandt@xxxxxxxxxxx>; Jacopo Mondi > <jacopo+renesas@xxxxxxxxxx>; laurent.pinchart@xxxxxxxxxxxxxxxx; > geert+renesas@xxxxxxxxx; linus.walleij@xxxxxxxxxx > Cc: linux-renesas-soc@xxxxxxxxxxxxxxx; linux-gpio@xxxxxxxxxxxxxxx > Subject: Re: [RFC 1/5] pinctrl: rz-pfc: Add Renesas RZ pinctrl core module > > 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 >
