| Message ID | 20240705-cleanup-h-iio-v1-6-77114c7e84c5@linaro.org (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | iio: adc: simplify with cleanup.h | expand |
On Fri, 2024-07-05 at 12:40 +0200, Krzysztof Kozlowski wrote: > The driver calls ad5755_parse_fw() only from probe() function, so > devm_kfree() during error path is not necessary and only makes code > weirder. > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- Reviewed-by: Nuno Sa <nuno.sa@analog.com>
On Fri, 05 Jul 2024 12:40:49 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > The driver calls ad5755_parse_fw() only from probe() function, so > devm_kfree() during error path is not necessary and only makes code > weirder. > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> The path this is in doesn't result in the driver failing to probe as it falls back to a const default structure. So whilst it's not a 'bug' to remove this free, we are removing data the driver is not going to use - so to my eye at least this is a deliberate design decision. Mind you it's not a particularly big allocation so maybe worth not cleaning up until driver remove in order to save on complexity. Sean, your code I think. Do you care either way? Jonathan > --- > drivers/iio/dac/ad5755.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c > index 0b24cb19ac9d..bfbfc3c1b6a5 100644 > --- a/drivers/iio/dac/ad5755.c > +++ b/drivers/iio/dac/ad5755.c > @@ -803,7 +803,6 @@ static struct ad5755_platform_data *ad5755_parse_fw(struct device *dev) > > error_out: > fwnode_handle_put(pp); > - devm_kfree(dev, pdata); > return NULL; > } > >
On 06/07/2024 12:52, Jonathan Cameron wrote: > On Fri, 05 Jul 2024 12:40:49 +0200 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >> The driver calls ad5755_parse_fw() only from probe() function, so >> devm_kfree() during error path is not necessary and only makes code >> weirder. >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > The path this is in doesn't result in the driver failing to probe as it > falls back to a const default structure. > So whilst it's not a 'bug' to remove this free, we are removing data the driver > is not going to use - so to my eye at least this is a deliberate design > decision. Ah, I missed that part - just looked at !pdata in the probe. > > Mind you it's not a particularly big allocation so maybe worth not cleaning > up until driver remove in order to save on complexity. > > Sean, your code I think. Do you care either way? I think the code was correct and my patch can be abandoned. Best regards, Krzysztof
diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c index 0b24cb19ac9d..bfbfc3c1b6a5 100644 --- a/drivers/iio/dac/ad5755.c +++ b/drivers/iio/dac/ad5755.c @@ -803,7 +803,6 @@ static struct ad5755_platform_data *ad5755_parse_fw(struct device *dev) error_out: fwnode_handle_put(pp); - devm_kfree(dev, pdata); return NULL; }
The driver calls ad5755_parse_fw() only from probe() function, so devm_kfree() during error path is not necessary and only makes code weirder. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- drivers/iio/dac/ad5755.c | 1 - 1 file changed, 1 deletion(-)