* [PATCH] devfreq: exynos-bus: Clean up code [not found] <CGME20191216102003eucas1p280d2bb32bc439a52353536dca87246f0@eucas1p2.samsung.com> @ 2019-12-16 10:19 ` Artur Świgoń 0 siblings, 0 replies; 4+ messages in thread From: Artur Świgoń @ 2019-12-16 10:19 UTC (permalink / raw) To: linux-pm, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: Artur Świgoń, cw00.choi, myungjoo.ham, kyungmin.park, kgene, krzk, b.zolnierkie, m.szyprowski, inki.dae, sw0312.kim 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(); as well as eliminating a few more goto statements and fixing header includes. Signed-off-by: Artur Świgoń <a.swigon@samsung.com> --- drivers/devfreq/exynos-bus.c | 54 +++++++++++++----------------------- 1 file changed, 19 insertions(+), 35 deletions(-) diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c index 19d9f9f8ced2..7f5917d59072 100644 --- a/drivers/devfreq/exynos-bus.c +++ b/drivers/devfreq/exynos-bus.c @@ -15,11 +15,10 @@ #include <linux/device.h> #include <linux/export.h> #include <linux/module.h> -#include <linux/of_device.h> +#include <linux/of.h> #include <linux/pm_opp.h> #include <linux/platform_device.h> #include <linux/regulator/consumer.h> -#include <linux/slab.h> #define DEFAULT_SATURATION_RATIO 40 @@ -301,10 +300,9 @@ static int exynos_bus_profile_init(struct exynos_bus *bus, profile->exit = exynos_bus_exit; ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL); - if (!ondemand_data) { - ret = -ENOMEM; - goto err; - } + if (!ondemand_data) + return -ENOMEM; + ondemand_data->upthreshold = 40; ondemand_data->downdifferential = 5; @@ -314,15 +312,14 @@ static int exynos_bus_profile_init(struct exynos_bus *bus, ondemand_data); if (IS_ERR(bus->devfreq)) { dev_err(dev, "failed to add devfreq device\n"); - ret = PTR_ERR(bus->devfreq); - goto err; + return PTR_ERR(bus->devfreq); } /* Register opp_notifier to catch the change of OPP */ ret = devm_devfreq_register_opp_notifier(dev, bus->devfreq); if (ret < 0) { dev_err(dev, "failed to register opp notifier\n"); - goto err; + return ret; } /* @@ -332,17 +329,16 @@ static int exynos_bus_profile_init(struct exynos_bus *bus, ret = exynos_bus_enable_edev(bus); if (ret < 0) { dev_err(dev, "failed to enable devfreq-event devices\n"); - goto err; + return ret; } ret = exynos_bus_set_event(bus); if (ret < 0) { dev_err(dev, "failed to set event to devfreq-event devices\n"); - goto err; + return ret; } -err: - return ret; + return 0; } static int exynos_bus_profile_init_passive(struct exynos_bus *bus, @@ -351,7 +347,6 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, struct device *dev = bus->dev; struct devfreq_passive_data *passive_data; struct devfreq *parent_devfreq; - int ret = 0; /* Initialize the struct profile and governor data for passive device */ profile->target = exynos_bus_target; @@ -359,16 +354,13 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, /* Get the instance of parent devfreq device */ parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0); - if (IS_ERR(parent_devfreq)) { - ret = -EPROBE_DEFER; - goto err; - } + if (IS_ERR(parent_devfreq)) + return -EPROBE_DEFER; passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL); - if (!passive_data) { - ret = -ENOMEM; - goto err; - } + if (!passive_data) + return -ENOMEM; + passive_data->parent = parent_devfreq; /* Add devfreq device for exynos bus with passive governor */ @@ -377,12 +369,10 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, if (IS_ERR(bus->devfreq)) { dev_err(dev, "failed to add devfreq dev with passive governor\n"); - ret = PTR_ERR(bus->devfreq); - goto err; + return PTR_ERR(bus->devfreq); } -err: - return ret; + return 0; } static int exynos_bus_probe(struct platform_device *pdev) @@ -427,19 +417,13 @@ static int exynos_bus_probe(struct platform_device *pdev) goto err_reg; if (passive) - goto passive; - - ret = exynos_bus_profile_init(bus, profile); - if (ret < 0) - goto err; + ret = exynos_bus_profile_init_passive(bus, profile); + else + ret = exynos_bus_profile_init(bus, profile); - 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] devfreq: exynos-bus: Clean up code @ 2019-12-16 10:19 ` Artur Świgoń 0 siblings, 0 replies; 4+ messages in thread From: Artur Świgoń @ 2019-12-16 10:19 UTC (permalink / raw) To: linux-pm, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: b.zolnierkie, sw0312.kim, Artur Świgoń, krzk, inki.dae, cw00.choi, kyungmin.park, kgene, myungjoo.ham, m.szyprowski 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(); as well as eliminating a few more goto statements and fixing header includes. Signed-off-by: Artur Świgoń <a.swigon@samsung.com> --- drivers/devfreq/exynos-bus.c | 54 +++++++++++++----------------------- 1 file changed, 19 insertions(+), 35 deletions(-) diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c index 19d9f9f8ced2..7f5917d59072 100644 --- a/drivers/devfreq/exynos-bus.c +++ b/drivers/devfreq/exynos-bus.c @@ -15,11 +15,10 @@ #include <linux/device.h> #include <linux/export.h> #include <linux/module.h> -#include <linux/of_device.h> +#include <linux/of.h> #include <linux/pm_opp.h> #include <linux/platform_device.h> #include <linux/regulator/consumer.h> -#include <linux/slab.h> #define DEFAULT_SATURATION_RATIO 40 @@ -301,10 +300,9 @@ static int exynos_bus_profile_init(struct exynos_bus *bus, profile->exit = exynos_bus_exit; ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL); - if (!ondemand_data) { - ret = -ENOMEM; - goto err; - } + if (!ondemand_data) + return -ENOMEM; + ondemand_data->upthreshold = 40; ondemand_data->downdifferential = 5; @@ -314,15 +312,14 @@ static int exynos_bus_profile_init(struct exynos_bus *bus, ondemand_data); if (IS_ERR(bus->devfreq)) { dev_err(dev, "failed to add devfreq device\n"); - ret = PTR_ERR(bus->devfreq); - goto err; + return PTR_ERR(bus->devfreq); } /* Register opp_notifier to catch the change of OPP */ ret = devm_devfreq_register_opp_notifier(dev, bus->devfreq); if (ret < 0) { dev_err(dev, "failed to register opp notifier\n"); - goto err; + return ret; } /* @@ -332,17 +329,16 @@ static int exynos_bus_profile_init(struct exynos_bus *bus, ret = exynos_bus_enable_edev(bus); if (ret < 0) { dev_err(dev, "failed to enable devfreq-event devices\n"); - goto err; + return ret; } ret = exynos_bus_set_event(bus); if (ret < 0) { dev_err(dev, "failed to set event to devfreq-event devices\n"); - goto err; + return ret; } -err: - return ret; + return 0; } static int exynos_bus_profile_init_passive(struct exynos_bus *bus, @@ -351,7 +347,6 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, struct device *dev = bus->dev; struct devfreq_passive_data *passive_data; struct devfreq *parent_devfreq; - int ret = 0; /* Initialize the struct profile and governor data for passive device */ profile->target = exynos_bus_target; @@ -359,16 +354,13 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, /* Get the instance of parent devfreq device */ parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0); - if (IS_ERR(parent_devfreq)) { - ret = -EPROBE_DEFER; - goto err; - } + if (IS_ERR(parent_devfreq)) + return -EPROBE_DEFER; passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL); - if (!passive_data) { - ret = -ENOMEM; - goto err; - } + if (!passive_data) + return -ENOMEM; + passive_data->parent = parent_devfreq; /* Add devfreq device for exynos bus with passive governor */ @@ -377,12 +369,10 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, if (IS_ERR(bus->devfreq)) { dev_err(dev, "failed to add devfreq dev with passive governor\n"); - ret = PTR_ERR(bus->devfreq); - goto err; + return PTR_ERR(bus->devfreq); } -err: - return ret; + return 0; } static int exynos_bus_probe(struct platform_device *pdev) @@ -427,19 +417,13 @@ static int exynos_bus_probe(struct platform_device *pdev) goto err_reg; if (passive) - goto passive; - - ret = exynos_bus_profile_init(bus, profile); - if (ret < 0) - goto err; + ret = exynos_bus_profile_init_passive(bus, profile); + else + ret = exynos_bus_profile_init(bus, profile); - 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] devfreq: exynos-bus: Clean up code 2019-12-16 10:19 ` Artur Świgoń @ 2019-12-16 10:36 ` Chanwoo Choi -1 siblings, 0 replies; 4+ messages in thread From: Chanwoo Choi @ 2019-12-16 10:36 UTC (permalink / raw) To: Artur Świgoń, linux-pm, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: myungjoo.ham, kyungmin.park, kgene, krzk, b.zolnierkie, m.szyprowski, inki.dae, sw0312.kim Hi, If possible, you better to write the more correct patch title because 'Clean up code' is too comprehensive. And please add 'PM / devfreq: ...' prefix for the devfreq patch. When you check the merged patches of driver/devfreq/, you can know the this prefix for patch title was used. On 12/16/19 7:19 PM, 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(); When you add the example, please remove '>' char. > > as well as eliminating a few more goto statements and fixing header > includes. And better to write what to fix the header including. > > Signed-off-by: Artur Świgoń <a.swigon@samsung.com> > --- > drivers/devfreq/exynos-bus.c | 54 +++++++++++++----------------------- > 1 file changed, 19 insertions(+), 35 deletions(-) > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > index 19d9f9f8ced2..7f5917d59072 100644 > --- a/drivers/devfreq/exynos-bus.c > +++ b/drivers/devfreq/exynos-bus.c > @@ -15,11 +15,10 @@ > #include <linux/device.h> > #include <linux/export.h> > #include <linux/module.h> > -#include <linux/of_device.h> > +#include <linux/of.h> > #include <linux/pm_opp.h> > #include <linux/platform_device.h> > #include <linux/regulator/consumer.h> > -#include <linux/slab.h> > > #define DEFAULT_SATURATION_RATIO 40 > > @@ -301,10 +300,9 @@ static int exynos_bus_profile_init(struct exynos_bus *bus, > profile->exit = exynos_bus_exit; > > ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL); > - if (!ondemand_data) { > - ret = -ENOMEM; > - goto err; > - } > + if (!ondemand_data) > + return -ENOMEM; > + > ondemand_data->upthreshold = 40; > ondemand_data->downdifferential = 5; > > @@ -314,15 +312,14 @@ static int exynos_bus_profile_init(struct exynos_bus *bus, > ondemand_data); > if (IS_ERR(bus->devfreq)) { > dev_err(dev, "failed to add devfreq device\n"); > - ret = PTR_ERR(bus->devfreq); > - goto err; > + return PTR_ERR(bus->devfreq); > } > > /* Register opp_notifier to catch the change of OPP */ > ret = devm_devfreq_register_opp_notifier(dev, bus->devfreq); > if (ret < 0) { > dev_err(dev, "failed to register opp notifier\n"); > - goto err; > + return ret; > } > > /* > @@ -332,17 +329,16 @@ static int exynos_bus_profile_init(struct exynos_bus *bus, > ret = exynos_bus_enable_edev(bus); > if (ret < 0) { > dev_err(dev, "failed to enable devfreq-event devices\n"); > - goto err; > + return ret; > } > > ret = exynos_bus_set_event(bus); > if (ret < 0) { > dev_err(dev, "failed to set event to devfreq-event devices\n"); > - goto err; > + return ret; > } > > -err: > - return ret; > + return 0; > } > > static int exynos_bus_profile_init_passive(struct exynos_bus *bus, > @@ -351,7 +347,6 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, > struct device *dev = bus->dev; > struct devfreq_passive_data *passive_data; > struct devfreq *parent_devfreq; > - int ret = 0; > > /* Initialize the struct profile and governor data for passive device */ > profile->target = exynos_bus_target; > @@ -359,16 +354,13 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, > > /* Get the instance of parent devfreq device */ > parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0); > - if (IS_ERR(parent_devfreq)) { > - ret = -EPROBE_DEFER; > - goto err; > - } > + if (IS_ERR(parent_devfreq)) > + return -EPROBE_DEFER; > > passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL); > - if (!passive_data) { > - ret = -ENOMEM; > - goto err; > - } > + if (!passive_data) > + return -ENOMEM; > + > passive_data->parent = parent_devfreq; > > /* Add devfreq device for exynos bus with passive governor */ > @@ -377,12 +369,10 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, > if (IS_ERR(bus->devfreq)) { > dev_err(dev, > "failed to add devfreq dev with passive governor\n"); > - ret = PTR_ERR(bus->devfreq); > - goto err; > + return PTR_ERR(bus->devfreq); > } > > -err: > - return ret; > + return 0; > } > > static int exynos_bus_probe(struct platform_device *pdev) > @@ -427,19 +417,13 @@ static int exynos_bus_probe(struct platform_device *pdev) > goto err_reg; > > if (passive) > - goto passive; > - > - ret = exynos_bus_profile_init(bus, profile); > - if (ret < 0) > - goto err; > + ret = exynos_bus_profile_init_passive(bus, profile); > + else > + ret = exynos_bus_profile_init(bus, profile); > > - 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); > -- Best Regards, Chanwoo Choi Samsung Electronics ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] devfreq: exynos-bus: Clean up code @ 2019-12-16 10:36 ` Chanwoo Choi 0 siblings, 0 replies; 4+ messages in thread From: Chanwoo Choi @ 2019-12-16 10:36 UTC (permalink / raw) To: Artur Świgoń, linux-pm, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: b.zolnierkie, sw0312.kim, krzk, inki.dae, kyungmin.park, myungjoo.ham, kgene, m.szyprowski Hi, If possible, you better to write the more correct patch title because 'Clean up code' is too comprehensive. And please add 'PM / devfreq: ...' prefix for the devfreq patch. When you check the merged patches of driver/devfreq/, you can know the this prefix for patch title was used. On 12/16/19 7:19 PM, 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(); When you add the example, please remove '>' char. > > as well as eliminating a few more goto statements and fixing header > includes. And better to write what to fix the header including. > > Signed-off-by: Artur Świgoń <a.swigon@samsung.com> > --- > drivers/devfreq/exynos-bus.c | 54 +++++++++++++----------------------- > 1 file changed, 19 insertions(+), 35 deletions(-) > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > index 19d9f9f8ced2..7f5917d59072 100644 > --- a/drivers/devfreq/exynos-bus.c > +++ b/drivers/devfreq/exynos-bus.c > @@ -15,11 +15,10 @@ > #include <linux/device.h> > #include <linux/export.h> > #include <linux/module.h> > -#include <linux/of_device.h> > +#include <linux/of.h> > #include <linux/pm_opp.h> > #include <linux/platform_device.h> > #include <linux/regulator/consumer.h> > -#include <linux/slab.h> > > #define DEFAULT_SATURATION_RATIO 40 > > @@ -301,10 +300,9 @@ static int exynos_bus_profile_init(struct exynos_bus *bus, > profile->exit = exynos_bus_exit; > > ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL); > - if (!ondemand_data) { > - ret = -ENOMEM; > - goto err; > - } > + if (!ondemand_data) > + return -ENOMEM; > + > ondemand_data->upthreshold = 40; > ondemand_data->downdifferential = 5; > > @@ -314,15 +312,14 @@ static int exynos_bus_profile_init(struct exynos_bus *bus, > ondemand_data); > if (IS_ERR(bus->devfreq)) { > dev_err(dev, "failed to add devfreq device\n"); > - ret = PTR_ERR(bus->devfreq); > - goto err; > + return PTR_ERR(bus->devfreq); > } > > /* Register opp_notifier to catch the change of OPP */ > ret = devm_devfreq_register_opp_notifier(dev, bus->devfreq); > if (ret < 0) { > dev_err(dev, "failed to register opp notifier\n"); > - goto err; > + return ret; > } > > /* > @@ -332,17 +329,16 @@ static int exynos_bus_profile_init(struct exynos_bus *bus, > ret = exynos_bus_enable_edev(bus); > if (ret < 0) { > dev_err(dev, "failed to enable devfreq-event devices\n"); > - goto err; > + return ret; > } > > ret = exynos_bus_set_event(bus); > if (ret < 0) { > dev_err(dev, "failed to set event to devfreq-event devices\n"); > - goto err; > + return ret; > } > > -err: > - return ret; > + return 0; > } > > static int exynos_bus_profile_init_passive(struct exynos_bus *bus, > @@ -351,7 +347,6 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, > struct device *dev = bus->dev; > struct devfreq_passive_data *passive_data; > struct devfreq *parent_devfreq; > - int ret = 0; > > /* Initialize the struct profile and governor data for passive device */ > profile->target = exynos_bus_target; > @@ -359,16 +354,13 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, > > /* Get the instance of parent devfreq device */ > parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0); > - if (IS_ERR(parent_devfreq)) { > - ret = -EPROBE_DEFER; > - goto err; > - } > + if (IS_ERR(parent_devfreq)) > + return -EPROBE_DEFER; > > passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL); > - if (!passive_data) { > - ret = -ENOMEM; > - goto err; > - } > + if (!passive_data) > + return -ENOMEM; > + > passive_data->parent = parent_devfreq; > > /* Add devfreq device for exynos bus with passive governor */ > @@ -377,12 +369,10 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, > if (IS_ERR(bus->devfreq)) { > dev_err(dev, > "failed to add devfreq dev with passive governor\n"); > - ret = PTR_ERR(bus->devfreq); > - goto err; > + return PTR_ERR(bus->devfreq); > } > > -err: > - return ret; > + return 0; > } > > static int exynos_bus_probe(struct platform_device *pdev) > @@ -427,19 +417,13 @@ static int exynos_bus_probe(struct platform_device *pdev) > goto err_reg; > > if (passive) > - goto passive; > - > - ret = exynos_bus_profile_init(bus, profile); > - if (ret < 0) > - goto err; > + ret = exynos_bus_profile_init_passive(bus, profile); > + else > + ret = exynos_bus_profile_init(bus, profile); > > - 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); > -- Best Regards, Chanwoo Choi Samsung Electronics _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-12-16 10:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20191216102003eucas1p280d2bb32bc439a52353536dca87246f0@eucas1p2.samsung.com>
2019-12-16 10:19 ` [PATCH] devfreq: exynos-bus: Clean up code Artur Świgoń
2019-12-16 10:19 ` Artur Świgoń
2019-12-16 10:36 ` Chanwoo Choi
2019-12-16 10:36 ` Chanwoo Choi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.