| Message ID | 20240812-cleanup-h-of-node-put-memory-v1-3-5065a8f361d2@linaro.org |
|---|---|
| State | Accepted |
| Commit | e2cc3ddaec0218f227cc53e3f784144640cd1765 |
| Headers | show |
| Series | memory: simplify with scoped/cleanup.h for device nodes | expand |
On Mon, 12 Aug 2024 15:33:57 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > Obtain the device node reference with scoped/cleanup.h to reduce error > handling and make the code a bit simpler. > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Trivial comments inline Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/memory/samsung/exynos5422-dmc.c | 31 +++++++++++-------------------- > 1 file changed, 11 insertions(+), 20 deletions(-) > > diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c > index da7ecd921c72..d3ae4d95a3ba 100644 > --- a/drivers/memory/samsung/exynos5422-dmc.c > +++ b/drivers/memory/samsung/exynos5422-dmc.c > @@ -4,6 +4,7 @@ > * Author: Lukasz Luba <l.luba@partner.samsung.com> > */ > > +#include <linux/cleanup.h> > #include <linux/clk.h> > #include <linux/devfreq.h> > #include <linux/devfreq-event.h> > @@ -1176,10 +1177,10 @@ static int of_get_dram_timings(struct exynos5_dmc *dmc) > { > int ret = 0; > int idx; > - struct device_node *np_ddr; This would definitely benefit from a struct device *dev = dmc->dev; > u32 freq_mhz, clk_period_ps; > > - np_ddr = of_parse_phandle(dmc->dev->of_node, "device-handle", 0); > + struct device_node *np_ddr __free(device_node) = of_parse_phandle(dmc->dev->of_node, > + "device-handle", 0); Trivial. Maybe consider the wrap suggested in patch 1. > + struct device_node *np_ddr __free(device_node) = of_parse_phandle(dmc->dev->of_node, "device-handle", 0); > if (!np_ddr) { > dev_warn(dmc->dev, "could not find 'device-handle' in DT\n"); > return -EINVAL; > @@ -1187,39 +1188,31 @@ static int of_get_dram_timings(struct exynos5_dmc *dmc) > > dmc->timing_row = devm_kmalloc_array(dmc->dev, TIMING_COUNT, > sizeof(u32), GFP_KERNEL); > - if (!dmc->timing_row) { > - ret = -ENOMEM; > - goto put_node; > - } > + if (!dmc->timing_row) > + return -ENOMEM; > > dmc->timing_data = devm_kmalloc_array(dmc->dev, TIMING_COUNT, > sizeof(u32), GFP_KERNEL); > - if (!dmc->timing_data) { > - ret = -ENOMEM; > - goto put_node; > - } > + if (!dmc->timing_data) > + return -ENOMEM; > > dmc->timing_power = devm_kmalloc_array(dmc->dev, TIMING_COUNT, > sizeof(u32), GFP_KERNEL); > - if (!dmc->timing_power) { > - ret = -ENOMEM; > - goto put_node; > - } > + if (!dmc->timing_power) > + return -ENOMEM; > > dmc->timings = of_lpddr3_get_ddr_timings(np_ddr, dmc->dev, > DDR_TYPE_LPDDR3, > &dmc->timings_arr_size); > if (!dmc->timings) { > dev_warn(dmc->dev, "could not get timings from DT\n"); > - ret = -EINVAL; > - goto put_node; > + return -EINVAL; > } > > dmc->min_tck = of_lpddr3_get_min_tck(np_ddr, dmc->dev); > if (!dmc->min_tck) { > dev_warn(dmc->dev, "could not get tck from DT\n"); > - ret = -EINVAL; > - goto put_node; > + return -EINVAL; > } > > /* Sorted array of OPPs with frequency ascending */ > @@ -1239,8 +1232,6 @@ static int of_get_dram_timings(struct exynos5_dmc *dmc) > dmc->bypass_timing_data = dmc->timing_data[idx - 1]; > dmc->bypass_timing_power = dmc->timing_power[idx - 1]; > > -put_node: > - of_node_put(np_ddr); > return ret; > } > >
On 14/08/2024 18:42, Jonathan Cameron wrote: > On Mon, 12 Aug 2024 15:33:57 +0200 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >> Obtain the device node reference with scoped/cleanup.h to reduce error >> handling and make the code a bit simpler. >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Trivial comments inline > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > >> --- >> drivers/memory/samsung/exynos5422-dmc.c | 31 +++++++++++-------------------- >> 1 file changed, 11 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c >> index da7ecd921c72..d3ae4d95a3ba 100644 >> --- a/drivers/memory/samsung/exynos5422-dmc.c >> +++ b/drivers/memory/samsung/exynos5422-dmc.c >> @@ -4,6 +4,7 @@ >> * Author: Lukasz Luba <l.luba@partner.samsung.com> >> */ >> >> +#include <linux/cleanup.h> >> #include <linux/clk.h> >> #include <linux/devfreq.h> >> #include <linux/devfreq-event.h> >> @@ -1176,10 +1177,10 @@ static int of_get_dram_timings(struct exynos5_dmc *dmc) >> { >> int ret = 0; >> int idx; >> - struct device_node *np_ddr; > > This would definitely benefit from a > struct device *dev = dmc->dev; True, I'll do it in separate patch. > >> u32 freq_mhz, clk_period_ps; >> >> - np_ddr = of_parse_phandle(dmc->dev->of_node, "device-handle", 0); >> + struct device_node *np_ddr __free(device_node) = of_parse_phandle(dmc->dev->of_node, >> + "device-handle", 0); > Trivial. Maybe consider the wrap suggested in patch 1. >> + struct device_node *np_ddr __free(device_node) = > of_parse_phandle(dmc->dev->of_node, "device-handle", 0); Ack. Best regards, Krzysztof
diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c index da7ecd921c72..d3ae4d95a3ba 100644 --- a/drivers/memory/samsung/exynos5422-dmc.c +++ b/drivers/memory/samsung/exynos5422-dmc.c @@ -4,6 +4,7 @@ * Author: Lukasz Luba <l.luba@partner.samsung.com> */ +#include <linux/cleanup.h> #include <linux/clk.h> #include <linux/devfreq.h> #include <linux/devfreq-event.h> @@ -1176,10 +1177,10 @@ static int of_get_dram_timings(struct exynos5_dmc *dmc) { int ret = 0; int idx; - struct device_node *np_ddr; u32 freq_mhz, clk_period_ps; - np_ddr = of_parse_phandle(dmc->dev->of_node, "device-handle", 0); + struct device_node *np_ddr __free(device_node) = of_parse_phandle(dmc->dev->of_node, + "device-handle", 0); if (!np_ddr) { dev_warn(dmc->dev, "could not find 'device-handle' in DT\n"); return -EINVAL; @@ -1187,39 +1188,31 @@ static int of_get_dram_timings(struct exynos5_dmc *dmc) dmc->timing_row = devm_kmalloc_array(dmc->dev, TIMING_COUNT, sizeof(u32), GFP_KERNEL); - if (!dmc->timing_row) { - ret = -ENOMEM; - goto put_node; - } + if (!dmc->timing_row) + return -ENOMEM; dmc->timing_data = devm_kmalloc_array(dmc->dev, TIMING_COUNT, sizeof(u32), GFP_KERNEL); - if (!dmc->timing_data) { - ret = -ENOMEM; - goto put_node; - } + if (!dmc->timing_data) + return -ENOMEM; dmc->timing_power = devm_kmalloc_array(dmc->dev, TIMING_COUNT, sizeof(u32), GFP_KERNEL); - if (!dmc->timing_power) { - ret = -ENOMEM; - goto put_node; - } + if (!dmc->timing_power) + return -ENOMEM; dmc->timings = of_lpddr3_get_ddr_timings(np_ddr, dmc->dev, DDR_TYPE_LPDDR3, &dmc->timings_arr_size); if (!dmc->timings) { dev_warn(dmc->dev, "could not get timings from DT\n"); - ret = -EINVAL; - goto put_node; + return -EINVAL; } dmc->min_tck = of_lpddr3_get_min_tck(np_ddr, dmc->dev); if (!dmc->min_tck) { dev_warn(dmc->dev, "could not get tck from DT\n"); - ret = -EINVAL; - goto put_node; + return -EINVAL; } /* Sorted array of OPPs with frequency ascending */ @@ -1239,8 +1232,6 @@ static int of_get_dram_timings(struct exynos5_dmc *dmc) dmc->bypass_timing_data = dmc->timing_data[idx - 1]; dmc->bypass_timing_power = dmc->timing_power[idx - 1]; -put_node: - of_node_put(np_ddr); return ret; }
Obtain the device node reference with scoped/cleanup.h to reduce error handling and make the code a bit simpler. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- drivers/memory/samsung/exynos5422-dmc.c | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-)