| Message ID | 20190723122016.30279-4-a.swigon@partner.samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Simple QoS for exynos-bus driver using interconnect | expand |
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ń <a.swigon@partner.samsung.com> > --- > drivers/devfreq/exynos-bus.c | 24 +++++++++--------------- > 1 file changed, 9 insertions(+), 15 deletions(-) Code looks much better: Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
2019년 7월 24일 (수) 오전 8:08, Artur Świgoń <a.swigon@partner.samsung.com>님이 작성: > > 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ń <a.swigon@partner.samsung.com> > --- > 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.
2019년 7월 25일 (목) 오후 9:56, Chanwoo Choi <cwchoi00@gmail.com>님이 작성: > > 2019년 7월 24일 (수) 오전 8:08, Artur Świgoń <a.swigon@partner.samsung.com>님이 작성: > > > > 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ń <a.swigon@partner.samsung.com> > > --- > > 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
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);