The following patchset incorporates the first four patches from a bigger
RFC[1]. The purpose of these patches is to improve readability of the code,
with the main focus on the exynos_bus_probe() function.
The original exynos_bus_probe() function has 13 local variables, over 140
lines of code, and multiple goto statements. Patches 01 and 02 from this
series extract two mutually exclusive code paths into separate functions,
exynos_bus_profile_init[_passive](). Furthermore, patch 03 reduces the
number of goto statements by introducing an if-else construct.
The last patch adds other minor improvements, including cleaning up header
includes, variables, and return paths. This also applies to functions
introduced by patches 01 & 02 -- to avoid moving and changing code in the
same patch.
---
Changes since RFCv2[1] (patches 01..04):
* Rebase on next-20191209.
* Drop some unnecessary changes, cf. [2].
---
Artur Świgoń
Samsung R&D Institute Poland
Samsung Electronics
---
References:
[1] https://patchwork.kernel.org/cover/11152595/
[2] https://patchwork.kernel.org/patch/11152637/
Artur Świgoń (4):
devfreq: exynos-bus: Extract exynos_bus_profile_init()
devfreq: exynos-bus: Extract exynos_bus_profile_init_passive()
devfreq: exynos-bus: Change goto-based logic to if-else logic
devfreq: exynos-bus: Clean up code
drivers/devfreq/exynos-bus.c | 156 +++++++++++++++++++----------------
1 file changed, 84 insertions(+), 72 deletions(-)
--
2.17.1
This patch adds a new static function, exynos_bus_profile_init(), extracted
from exynos_bus_probe().
Signed-off-by: Artur Świgoń <[email protected]>
---
drivers/devfreq/exynos-bus.c | 106 ++++++++++++++++++++---------------
1 file changed, 60 insertions(+), 46 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index c832673273a2..b8ca6b9f4b82 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -287,12 +287,69 @@ static int exynos_bus_parse_of(struct device_node *np,
return ret;
}
+static int exynos_bus_profile_init(struct exynos_bus *bus,
+ struct devfreq_dev_profile *profile)
+{
+ struct device *dev = bus->dev;
+ struct devfreq_simple_ondemand_data *ondemand_data;
+ int ret;
+
+ /* Initialize the struct profile and governor data for parent device */
+ profile->polling_ms = 50;
+ profile->target = exynos_bus_target;
+ profile->get_dev_status = exynos_bus_get_dev_status;
+ profile->exit = exynos_bus_exit;
+
+ ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
+ if (!ondemand_data) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ ondemand_data->upthreshold = 40;
+ ondemand_data->downdifferential = 5;
+
+ /* Add devfreq device to monitor and handle the exynos bus */
+ bus->devfreq = devm_devfreq_add_device(dev, profile,
+ DEVFREQ_GOV_SIMPLE_ONDEMAND,
+ ondemand_data);
+ if (IS_ERR(bus->devfreq)) {
+ dev_err(dev, "failed to add devfreq device\n");
+ ret = PTR_ERR(bus->devfreq);
+ goto err;
+ }
+
+ /* 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;
+ }
+
+ /*
+ * Enable devfreq-event to get raw data which is used to determine
+ * current bus load.
+ */
+ ret = exynos_bus_enable_edev(bus);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable devfreq-event devices\n");
+ goto err;
+ }
+
+ ret = exynos_bus_set_event(bus);
+ if (ret < 0) {
+ dev_err(dev, "failed to set event to devfreq-event devices\n");
+ goto err;
+ }
+
+err:
+ return ret;
+}
+
static int exynos_bus_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node, *node;
struct devfreq_dev_profile *profile;
- struct devfreq_simple_ondemand_data *ondemand_data;
struct devfreq_passive_data *passive_data;
struct devfreq *parent_devfreq;
struct exynos_bus *bus;
@@ -334,52 +391,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
if (passive)
goto passive;
- /* Initialize the struct profile and governor data for parent device */
- profile->polling_ms = 50;
- profile->target = exynos_bus_target;
- profile->get_dev_status = exynos_bus_get_dev_status;
- profile->exit = exynos_bus_exit;
-
- ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
- if (!ondemand_data) {
- ret = -ENOMEM;
+ ret = exynos_bus_profile_init(bus, profile);
+ if (ret < 0)
goto err;
- }
- ondemand_data->upthreshold = 40;
- ondemand_data->downdifferential = 5;
-
- /* Add devfreq device to monitor and handle the exynos bus */
- bus->devfreq = devm_devfreq_add_device(dev, profile,
- DEVFREQ_GOV_SIMPLE_ONDEMAND,
- ondemand_data);
- if (IS_ERR(bus->devfreq)) {
- dev_err(dev, "failed to add devfreq device\n");
- ret = PTR_ERR(bus->devfreq);
- goto err;
- }
-
- /* 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;
- }
-
- /*
- * Enable devfreq-event to get raw data which is used to determine
- * current bus load.
- */
- ret = exynos_bus_enable_edev(bus);
- if (ret < 0) {
- dev_err(dev, "failed to enable devfreq-event devices\n");
- goto err;
- }
-
- ret = exynos_bus_set_event(bus);
- if (ret < 0) {
- dev_err(dev, "failed to set event to devfreq-event devices\n");
- goto err;
- }
goto out;
passive:
--
2.17.1
On 12/9/19 7:48 PM, Artur Świgoń wrote:
> This patch adds a new static function, exynos_bus_profile_init(), extracted
> from exynos_bus_probe().
>
> Signed-off-by: Artur Świgoń <[email protected]>
> ---
> drivers/devfreq/exynos-bus.c | 106 ++++++++++++++++++++---------------
> 1 file changed, 60 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index c832673273a2..b8ca6b9f4b82 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -287,12 +287,69 @@ static int exynos_bus_parse_of(struct device_node *np,
> return ret;
> }
>
> +static int exynos_bus_profile_init(struct exynos_bus *bus,
> + struct devfreq_dev_profile *profile)
> +{
> + struct device *dev = bus->dev;
> + struct devfreq_simple_ondemand_data *ondemand_data;
> + int ret;
> +
> + /* Initialize the struct profile and governor data for parent device */
> + profile->polling_ms = 50;
> + profile->target = exynos_bus_target;
> + profile->get_dev_status = exynos_bus_get_dev_status;
> + profile->exit = exynos_bus_exit;
> +
> + ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
> + if (!ondemand_data) {
> + ret = -ENOMEM;
> + goto err;
> + }
> + ondemand_data->upthreshold = 40;
> + ondemand_data->downdifferential = 5;
> +
> + /* Add devfreq device to monitor and handle the exynos bus */
> + bus->devfreq = devm_devfreq_add_device(dev, profile,
> + DEVFREQ_GOV_SIMPLE_ONDEMAND,
> + ondemand_data);
> + if (IS_ERR(bus->devfreq)) {
> + dev_err(dev, "failed to add devfreq device\n");
> + ret = PTR_ERR(bus->devfreq);
> + goto err;
> + }
> +
> + /* 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;
> + }
> +
> + /*
> + * Enable devfreq-event to get raw data which is used to determine
> + * current bus load.
> + */
> + ret = exynos_bus_enable_edev(bus);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable devfreq-event devices\n");
> + goto err;
> + }
> +
> + ret = exynos_bus_set_event(bus);
> + if (ret < 0) {
> + dev_err(dev, "failed to set event to devfreq-event devices\n");
> + goto err;
> + }
> +
> +err:
> + return ret;
> +}
> +
> static int exynos_bus_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node, *node;
> struct devfreq_dev_profile *profile;
> - struct devfreq_simple_ondemand_data *ondemand_data;
> struct devfreq_passive_data *passive_data;
> struct devfreq *parent_devfreq;
> struct exynos_bus *bus;
> @@ -334,52 +391,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
> if (passive)
> goto passive;
>
> - /* Initialize the struct profile and governor data for parent device */
> - profile->polling_ms = 50;
> - profile->target = exynos_bus_target;
> - profile->get_dev_status = exynos_bus_get_dev_status;
> - profile->exit = exynos_bus_exit;
> -
> - ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
> - if (!ondemand_data) {
> - ret = -ENOMEM;
> + ret = exynos_bus_profile_init(bus, profile);
> + if (ret < 0)
> goto err;
> - }
> - ondemand_data->upthreshold = 40;
> - ondemand_data->downdifferential = 5;
> -
> - /* Add devfreq device to monitor and handle the exynos bus */
> - bus->devfreq = devm_devfreq_add_device(dev, profile,
> - DEVFREQ_GOV_SIMPLE_ONDEMAND,
> - ondemand_data);
> - if (IS_ERR(bus->devfreq)) {
> - dev_err(dev, "failed to add devfreq device\n");
> - ret = PTR_ERR(bus->devfreq);
> - goto err;
> - }
> -
> - /* 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;
> - }
> -
> - /*
> - * Enable devfreq-event to get raw data which is used to determine
> - * current bus load.
> - */
> - ret = exynos_bus_enable_edev(bus);
> - if (ret < 0) {
> - dev_err(dev, "failed to enable devfreq-event devices\n");
> - goto err;
> - }
> -
> - ret = exynos_bus_set_event(bus);
> - if (ret < 0) {
> - dev_err(dev, "failed to set event to devfreq-event devices\n");
> - goto err;
> - }
>
> goto out;
> passive:
>
Applied it. Thanks.
--
Best Regards,
Chanwoo Choi
Samsung Electronics