Re: [RFC PATCH 03/11] devfreq: exynos-bus: Change goto-based logic to if-else logic
2019년 7월 25일 (목) 오후 9:56, Chanwoo Choi 님이 작성: > > 2019년 7월 24일 (수) 오전 8:08, Artur Świgoń 님이 작성: > > > > This patch improves code readability by changing the following construct: > > > > >if (cond) > > >goto passive; > > >foo(); > > >goto out; > > >passive: > > >bar(); > > >out: > > > > into this: > > > > >if (cond) > > >bar(); > > >else > > >foo(); > > > > Signed-off-by: Artur Świgoń > > --- > > drivers/devfreq/exynos-bus.c | 24 +--- > > 1 file changed, 9 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > > index cf6f6cbd0f55..4bb83b945bf7 100644 > > --- a/drivers/devfreq/exynos-bus.c > > +++ b/drivers/devfreq/exynos-bus.c > > @@ -505,25 +505,19 @@ static int exynos_bus_probe(struct platform_device > > *pdev) > > node = of_parse_phandle(dev->of_node, "devfreq", 0); > > if (node) { > > of_node_put(node); > > - goto passive; > > + ret = exynos_bus_profile_init_passive(bus, profile); > > + if (ret < 0) > > + goto err; > > } else { > > ret = exynos_bus_parent_parse_of(np, bus); > > + if (ret < 0) > > + goto err; > > + > > + ret = exynos_bus_profile_init(bus, profile); > > + if (ret < 0) > > + goto err; > > } > > > > - if (ret < 0) > > - goto err; > > - > > - ret = exynos_bus_profile_init(bus, profile); > > - if (ret < 0) > > - goto err; > > - > > - goto out; > > -passive: > > - ret = exynos_bus_profile_init_passive(bus, profile); > > - if (ret < 0) > > - goto err; > > - > > -out: > > max_state = bus->devfreq->profile->max_state; > > min_freq = (bus->devfreq->profile->freq_table[0] / 1000); > > max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / > > 1000); > > -- > > 2.17.1 > > > > It seems more simple than before. > Instead, please merge patch1/2/3 to one patch. and drop the patch4. But, I think that you better to drop the cleanup patch from this series because the series[1] touch the exynos-bus.c driver for coupled regulator. [1] https://www.spinics.net/lists/arm-kernel/msg741971.html I recommend that you send the cleanup patch with my comment either after reviewing the Kamil's patch[1] or rebase this series base on patch[1]. > > -- > Best Regards, > Chanwoo Choi -- Best Regards, Chanwoo Choi
Re: [RFC PATCH 03/11] devfreq: exynos-bus: Change goto-based logic to if-else logic
2019년 7월 24일 (수) 오전 8:08, Artur Świgoń 님이 작성: > > This patch improves code readability by changing the following construct: > > >if (cond) > >goto passive; > >foo(); > >goto out; > >passive: > >bar(); > >out: > > into this: > > >if (cond) > >bar(); > >else > >foo(); > > Signed-off-by: Artur Świgoń > --- > drivers/devfreq/exynos-bus.c | 24 +--- > 1 file changed, 9 insertions(+), 15 deletions(-) > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > index cf6f6cbd0f55..4bb83b945bf7 100644 > --- a/drivers/devfreq/exynos-bus.c > +++ b/drivers/devfreq/exynos-bus.c > @@ -505,25 +505,19 @@ static int exynos_bus_probe(struct platform_device > *pdev) > node = of_parse_phandle(dev->of_node, "devfreq", 0); > if (node) { > of_node_put(node); > - goto passive; > + ret = exynos_bus_profile_init_passive(bus, profile); > + if (ret < 0) > + goto err; > } else { > ret = exynos_bus_parent_parse_of(np, bus); > + if (ret < 0) > + goto err; > + > + ret = exynos_bus_profile_init(bus, profile); > + if (ret < 0) > + goto err; > } > > - if (ret < 0) > - goto err; > - > - ret = exynos_bus_profile_init(bus, profile); > - if (ret < 0) > - goto err; > - > - goto out; > -passive: > - ret = exynos_bus_profile_init_passive(bus, profile); > - if (ret < 0) > - goto err; > - > -out: > max_state = bus->devfreq->profile->max_state; > min_freq = (bus->devfreq->profile->freq_table[0] / 1000); > max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / 1000); > -- > 2.17.1 > It seems more simple than before. Instead, please merge patch1/2/3 to one patch. and drop the patch4. -- Best Regards, Chanwoo Choi
Re: [RFC PATCH 03/11] devfreq: exynos-bus: Change goto-based logic to if-else logic
On Tue, Jul 23, 2019 at 02:20:08PM +0200, Artur Świgoń wrote: > This patch improves code readability by changing the following construct: > > >if (cond) > >goto passive; > >foo(); > >goto out; > >passive: > >bar(); > >out: > > into this: > > >if (cond) > >bar(); > >else > >foo(); > > Signed-off-by: Artur Świgoń > --- > drivers/devfreq/exynos-bus.c | 24 +--- > 1 file changed, 9 insertions(+), 15 deletions(-) Code looks much better: Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 03/11] devfreq: exynos-bus: Change goto-based logic to if-else logic
This patch improves code readability by changing the following construct: >if (cond) >goto passive; >foo(); >goto out; >passive: >bar(); >out: into this: >if (cond) >bar(); >else >foo(); Signed-off-by: Artur Świgoń --- drivers/devfreq/exynos-bus.c | 24 +--- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c index cf6f6cbd0f55..4bb83b945bf7 100644 --- a/drivers/devfreq/exynos-bus.c +++ b/drivers/devfreq/exynos-bus.c @@ -505,25 +505,19 @@ static int exynos_bus_probe(struct platform_device *pdev) node = of_parse_phandle(dev->of_node, "devfreq", 0); if (node) { of_node_put(node); - goto passive; + ret = exynos_bus_profile_init_passive(bus, profile); + if (ret < 0) + goto err; } else { ret = exynos_bus_parent_parse_of(np, bus); + if (ret < 0) + goto err; + + ret = exynos_bus_profile_init(bus, profile); + if (ret < 0) + goto err; } - if (ret < 0) - goto err; - - ret = exynos_bus_profile_init(bus, profile); - if (ret < 0) - goto err; - - goto out; -passive: - ret = exynos_bus_profile_init_passive(bus, profile); - if (ret < 0) - goto err; - -out: max_state = bus->devfreq->profile->max_state; min_freq = (bus->devfreq->profile->freq_table[0] / 1000); max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / 1000); -- 2.17.1