From: Artur Świgoń <[email protected]>
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 | 92 +++++++++++++++++++++---------------
1 file changed, 53 insertions(+), 39 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 29f422469960..78f38b7fb596 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -287,12 +287,62 @@ 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;
+ }
+
+ /*
+ * 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,45 +384,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;
- }
-
- /*
- * 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
Hi,
As I already replied on v1, patch1/2/3 clean-up code
for readability without any behavior changes.
I think that you better to merge patch1/2/3 to one patch.
On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
> From: Artur Świgoń <[email protected]>
>
> 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 | 92 +++++++++++++++++++++---------------
> 1 file changed, 53 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index 29f422469960..78f38b7fb596 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -287,12 +287,62 @@ 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;
> + }
> +
> + /*
> + * 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,45 +384,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;
> - }
> -
> - /*
> - * 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:
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Hi,
On Fri, 2019-09-20 at 11:15 +0900, Chanwoo Choi wrote:
> Hi,
>
> As I already replied on v1, patch1/2/3 clean-up code
> for readability without any behavior changes.
>
> I think that you better to merge patch1/2/3 to one patch.
Yes, when writing the cover letter I think I forgot to explain why I decided not
to merge these patches. Basically, none of the diff algorithms available in git
(I've got v2.17.1) is able to produce a readable patch with these changes
combined together into a single patch (functions are intermixed together in the
output, git thinks that 'exynos_bus_probe' is a new function).
Please take a look at the diff at the bottom of this message to see how patches
01..03 look when combined. If such patch looks acceptable to you, I can merge.
> On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
> > From: Artur Świgoń <[email protected]>
> >
> > 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 | 92 +++++++++++++++++++++---------------
> > 1 file changed, 53 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > index 29f422469960..78f38b7fb596 100644
> > --- a/drivers/devfreq/exynos-bus.c
> > +++ b/drivers/devfreq/exynos-bus.c
> > @@ -287,12 +287,62 @@ 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;
> > + }
> > +
> > + /*
> > + * 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,45 +384,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;
> > - }
> > -
> > - /*
> > - * 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:
commit cacf8e4ea0e111908d11779977c81e29d6418801
Author: Artur Świgoń <[email protected]>
Date: Tue Aug 27 13:17:28 2019 +0200
tmp: merge patches 01-03
Signed-off-by: Artur Świgoń <[email protected]>
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 29f422469960..60ad4319fd80 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -287,52 +287,12 @@ static int exynos_bus_parse_of(struct device_node *np,
return ret;
}
-static int exynos_bus_probe(struct platform_device *pdev)
+static int exynos_bus_profile_init(struct exynos_bus *bus,
+ struct devfreq_dev_profile *profile)
{
- struct device *dev = &pdev->dev;
- struct device_node *np = dev->of_node, *node;
- struct devfreq_dev_profile *profile;
+ struct device *dev = bus->dev;
struct devfreq_simple_ondemand_data *ondemand_data;
- struct devfreq_passive_data *passive_data;
- struct devfreq *parent_devfreq;
- struct exynos_bus *bus;
- int ret, max_state;
- unsigned long min_freq, max_freq;
- bool passive = false;
-
- if (!np) {
- dev_err(dev, "failed to find devicetree node\n");
- return -EINVAL;
- }
-
- bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
- if (!bus)
- return -ENOMEM;
- mutex_init(&bus->lock);
- bus->dev = &pdev->dev;
- platform_set_drvdata(pdev, bus);
-
- profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
- if (!profile)
- return -ENOMEM;
-
- node = of_parse_phandle(dev->of_node, "devfreq", 0);
- if (node) {
- of_node_put(node);
- passive = true;
- } else {
- ret = exynos_bus_parent_parse_of(np, bus);
- if (ret < 0)
- return ret;
- }
-
- /* Parse the device-tree to get the resource information */
- ret = exynos_bus_parse_of(np, bus);
- if (ret < 0)
- goto err_reg;
-
- if (passive)
- goto passive;
+ int ret;
/* Initialize the struct profile and governor data for parent device */
profile->polling_ms = 50;
@@ -374,8 +334,18 @@ static int exynos_bus_probe(struct platform_device *pdev)
goto err;
}
- goto out;
-passive:
+err:
+ return ret;
+}
+
+static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
+ struct devfreq_dev_profile *profile)
+{
+ 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;
profile->exit = exynos_bus_passive_exit;
@@ -404,7 +374,59 @@ static int exynos_bus_probe(struct platform_device *pdev)
goto err;
}
-out:
+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 exynos_bus *bus;
+ int ret, max_state;
+ unsigned long min_freq, max_freq;
+ bool passive = false;
+
+ if (!np) {
+ dev_err(dev, "failed to find devicetree node\n");
+ return -EINVAL;
+ }
+
+ bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
+ if (!bus)
+ return -ENOMEM;
+ mutex_init(&bus->lock);
+ bus->dev = &pdev->dev;
+ platform_set_drvdata(pdev, bus);
+
+ profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
+ if (!profile)
+ return -ENOMEM;
+
+ node = of_parse_phandle(dev->of_node, "devfreq", 0);
+ if (node) {
+ of_node_put(node);
+ passive = true;
+ } else {
+ ret = exynos_bus_parent_parse_of(np, bus);
+ if (ret < 0)
+ return ret;
+ }
+
+ /* Parse the device-tree to get the resource information */
+ ret = exynos_bus_parse_of(np, bus);
+ if (ret < 0)
+ goto err_reg;
+
+ if (passive)
+ ret = exynos_bus_profile_init_passive(bus, profile);
+ else
+ ret = exynos_bus_profile_init(bus, profile);
+
+ if (ret < 0)
+ goto err;
+
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);
On Wed, 2019-09-25 at 15:37 +0900, Chanwoo Choi wrote:
> Hi,
>
> On 19. 9. 25. 오후 2:44, Artur Świgoń wrote:
> > Hi,
> >
> > On Fri, 2019-09-20 at 11:15 +0900, Chanwoo Choi wrote:
> > > Hi,
> > >
> > > As I already replied on v1, patch1/2/3 clean-up code
> > > for readability without any behavior changes.
> > >
> > > I think that you better to merge patch1/2/3 to one patch.
> >
> > Yes, when writing the cover letter I think I forgot to explain why I decided not
> > to merge these patches. Basically, none of the diff algorithms available in git
> > (I've got v2.17.1) is able to produce a readable patch with these changes
> > combined together into a single patch (functions are intermixed together in the
> > output, git thinks that 'exynos_bus_probe' is a new function).
>
> After merged three patches, as you commented, looks like that 'exynos_bus_probe'
> is new function. Your patch style(three patches) is better than one merged patch.
> Keep your original patches. Thanks.
I know that having three separate patches is suboptimal, but they are more readable
this way. I am glad you agree. I will keep them separate. Thank you for your
comments.
> >
> > Please take a look at the diff at the bottom of this message to see how patches
> > 01..03 look when combined. If such patch looks acceptable to you, I can merge.
> >
> > > On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
> > > > From: Artur Świgoń <[email protected]>
> > > >
> > > > 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 | 92 +++++++++++++++++++++---------------
> > > > 1 file changed, 53 insertions(+), 39 deletions(-)
> > > >
> > > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > > > index 29f422469960..78f38b7fb596 100644
> > > > --- a/drivers/devfreq/exynos-bus.c
> > > > +++ b/drivers/devfreq/exynos-bus.c
> > > > @@ -287,12 +287,62 @@ 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;
> > > > + }
> > > > +
> > > > + /*
> > > > + * 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,45 +384,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;
> > > > - }
> > > > -
> > > > - /*
> > > > - * 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:
> >
> > commit cacf8e4ea0e111908d11779977c81e29d6418801
> > Author: Artur Świgoń <[email protected]>
> > Date: Tue Aug 27 13:17:28 2019 +0200
> >
> > tmp: merge patches 01-03
> >
> > Signed-off-by: Artur Świgoń <[email protected]>
> >
> > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > index 29f422469960..60ad4319fd80 100644
> > --- a/drivers/devfreq/exynos-bus.c
> > +++ b/drivers/devfreq/exynos-bus.c
> > @@ -287,52 +287,12 @@ static int exynos_bus_parse_of(struct device_node *np,
> > return ret;
> > }
> >
> > -static int exynos_bus_probe(struct platform_device *pdev)
> > +static int exynos_bus_profile_init(struct exynos_bus *bus,
> > + struct devfreq_dev_profile *profile)
> > {
> > - struct device *dev = &pdev->dev;
> > - struct device_node *np = dev->of_node, *node;
> > - struct devfreq_dev_profile *profile;
> > + struct device *dev = bus->dev;
> > struct devfreq_simple_ondemand_data *ondemand_data;
> > - struct devfreq_passive_data *passive_data;
> > - struct devfreq *parent_devfreq;
> > - struct exynos_bus *bus;
> > - int ret, max_state;
> > - unsigned long min_freq, max_freq;
> > - bool passive = false;
> > -
> > - if (!np) {
> > - dev_err(dev, "failed to find devicetree node\n");
> > - return -EINVAL;
> > - }
> > -
> > - bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> > - if (!bus)
> > - return -ENOMEM;
> > - mutex_init(&bus->lock);
> > - bus->dev = &pdev->dev;
> > - platform_set_drvdata(pdev, bus);
> > -
> > - profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
> > - if (!profile)
> > - return -ENOMEM;
> > -
> > - node = of_parse_phandle(dev->of_node, "devfreq", 0);
> > - if (node) {
> > - of_node_put(node);
> > - passive = true;
> > - } else {
> > - ret = exynos_bus_parent_parse_of(np, bus);
> > - if (ret < 0)
> > - return ret;
> > - }
> > -
> > - /* Parse the device-tree to get the resource information */
> > - ret = exynos_bus_parse_of(np, bus);
> > - if (ret < 0)
> > - goto err_reg;
> > -
> > - if (passive)
> > - goto passive;
> > + int ret;
> >
> > /* Initialize the struct profile and governor data for parent device */
> > profile->polling_ms = 50;
> > @@ -374,8 +334,18 @@ static int exynos_bus_probe(struct platform_device *pdev)
> > goto err;
> > }
> >
> > - goto out;
> > -passive:
> > +err:
> > + return ret;
> > +}
> > +
> > +static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
> > + struct devfreq_dev_profile *profile)
> > +{
> > + 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;
> > profile->exit = exynos_bus_passive_exit;
> > @@ -404,7 +374,59 @@ static int exynos_bus_probe(struct platform_device *pdev)
> > goto err;
> > }
> >
> > -out:
> > +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 exynos_bus *bus;
> > + int ret, max_state;
> > + unsigned long min_freq, max_freq;
> > + bool passive = false;
> > +
> > + if (!np) {
> > + dev_err(dev, "failed to find devicetree node\n");
> > + return -EINVAL;
> > + }
> > +
> > + bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> > + if (!bus)
> > + return -ENOMEM;
> > + mutex_init(&bus->lock);
> > + bus->dev = &pdev->dev;
> > + platform_set_drvdata(pdev, bus);
> > +
> > + profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
> > + if (!profile)
> > + return -ENOMEM;
> > +
> > + node = of_parse_phandle(dev->of_node, "devfreq", 0);
> > + if (node) {
> > + of_node_put(node);
> > + passive = true;
> > + } else {
> > + ret = exynos_bus_parent_parse_of(np, bus);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + /* Parse the device-tree to get the resource information */
> > + ret = exynos_bus_parse_of(np, bus);
> > + if (ret < 0)
> > + goto err_reg;
> > +
> > + if (passive)
> > + ret = exynos_bus_profile_init_passive(bus, profile);
> > + else
> > + ret = exynos_bus_profile_init(bus, profile);
> > +
> > + if (ret < 0)
> > + goto err;
> > +
> > 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);
> >
> >
> >
> >
>
>
--
Artur Świgoń
Samsung R&D Institute Poland
Samsung Electronics
Hi,
On 19. 9. 25. 오후 2:44, Artur Świgoń wrote:
> Hi,
>
> On Fri, 2019-09-20 at 11:15 +0900, Chanwoo Choi wrote:
>> Hi,
>>
>> As I already replied on v1, patch1/2/3 clean-up code
>> for readability without any behavior changes.
>>
>> I think that you better to merge patch1/2/3 to one patch.
>
> Yes, when writing the cover letter I think I forgot to explain why I decided not
> to merge these patches. Basically, none of the diff algorithms available in git
> (I've got v2.17.1) is able to produce a readable patch with these changes
> combined together into a single patch (functions are intermixed together in the
> output, git thinks that 'exynos_bus_probe' is a new function).
After merged three patches, as you commented, looks like that 'exynos_bus_probe'
is new function. Your patch style(three patches) is better than one merged patch.
Keep your original patches. Thanks.
>
> Please take a look at the diff at the bottom of this message to see how patches
> 01..03 look when combined. If such patch looks acceptable to you, I can merge.
>
>> On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
>>> From: Artur Świgoń <[email protected]>
>>>
>>> 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 | 92 +++++++++++++++++++++---------------
>>> 1 file changed, 53 insertions(+), 39 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>>> index 29f422469960..78f38b7fb596 100644
>>> --- a/drivers/devfreq/exynos-bus.c
>>> +++ b/drivers/devfreq/exynos-bus.c
>>> @@ -287,12 +287,62 @@ 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;
>>> + }
>>> +
>>> + /*
>>> + * 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,45 +384,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;
>>> - }
>>> -
>>> - /*
>>> - * 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:
>
> commit cacf8e4ea0e111908d11779977c81e29d6418801
> Author: Artur Świgoń <[email protected]>
> Date: Tue Aug 27 13:17:28 2019 +0200
>
> tmp: merge patches 01-03
>
> Signed-off-by: Artur Świgoń <[email protected]>
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index 29f422469960..60ad4319fd80 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -287,52 +287,12 @@ static int exynos_bus_parse_of(struct device_node *np,
> return ret;
> }
>
> -static int exynos_bus_probe(struct platform_device *pdev)
> +static int exynos_bus_profile_init(struct exynos_bus *bus,
> + struct devfreq_dev_profile *profile)
> {
> - struct device *dev = &pdev->dev;
> - struct device_node *np = dev->of_node, *node;
> - struct devfreq_dev_profile *profile;
> + struct device *dev = bus->dev;
> struct devfreq_simple_ondemand_data *ondemand_data;
> - struct devfreq_passive_data *passive_data;
> - struct devfreq *parent_devfreq;
> - struct exynos_bus *bus;
> - int ret, max_state;
> - unsigned long min_freq, max_freq;
> - bool passive = false;
> -
> - if (!np) {
> - dev_err(dev, "failed to find devicetree node\n");
> - return -EINVAL;
> - }
> -
> - bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> - if (!bus)
> - return -ENOMEM;
> - mutex_init(&bus->lock);
> - bus->dev = &pdev->dev;
> - platform_set_drvdata(pdev, bus);
> -
> - profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
> - if (!profile)
> - return -ENOMEM;
> -
> - node = of_parse_phandle(dev->of_node, "devfreq", 0);
> - if (node) {
> - of_node_put(node);
> - passive = true;
> - } else {
> - ret = exynos_bus_parent_parse_of(np, bus);
> - if (ret < 0)
> - return ret;
> - }
> -
> - /* Parse the device-tree to get the resource information */
> - ret = exynos_bus_parse_of(np, bus);
> - if (ret < 0)
> - goto err_reg;
> -
> - if (passive)
> - goto passive;
> + int ret;
>
> /* Initialize the struct profile and governor data for parent device */
> profile->polling_ms = 50;
> @@ -374,8 +334,18 @@ static int exynos_bus_probe(struct platform_device *pdev)
> goto err;
> }
>
> - goto out;
> -passive:
> +err:
> + return ret;
> +}
> +
> +static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
> + struct devfreq_dev_profile *profile)
> +{
> + 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;
> profile->exit = exynos_bus_passive_exit;
> @@ -404,7 +374,59 @@ static int exynos_bus_probe(struct platform_device *pdev)
> goto err;
> }
>
> -out:
> +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 exynos_bus *bus;
> + int ret, max_state;
> + unsigned long min_freq, max_freq;
> + bool passive = false;
> +
> + if (!np) {
> + dev_err(dev, "failed to find devicetree node\n");
> + return -EINVAL;
> + }
> +
> + bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> + if (!bus)
> + return -ENOMEM;
> + mutex_init(&bus->lock);
> + bus->dev = &pdev->dev;
> + platform_set_drvdata(pdev, bus);
> +
> + profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
> + if (!profile)
> + return -ENOMEM;
> +
> + node = of_parse_phandle(dev->of_node, "devfreq", 0);
> + if (node) {
> + of_node_put(node);
> + passive = true;
> + } else {
> + ret = exynos_bus_parent_parse_of(np, bus);
> + if (ret < 0)
> + return ret;
> + }
> +
> + /* Parse the device-tree to get the resource information */
> + ret = exynos_bus_parse_of(np, bus);
> + if (ret < 0)
> + goto err_reg;
> +
> + if (passive)
> + ret = exynos_bus_profile_init_passive(bus, profile);
> + else
> + ret = exynos_bus_profile_init(bus, profile);
> +
> + if (ret < 0)
> + goto err;
> +
> 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