On 12/2/19 1:45 PM, Leonard Crestez wrote:

On 2019-12-02 3:02 AM, Chanwoo Choi wrote:

quoted

On 11/14/19 8:21 AM, Leonard Crestez wrote:

quoted

Splitting device_register into device_initialize and device_add allows
devm-based allocations to be performed before device_add.

It also simplifies error paths in devfreq_add_device: just call
put_device instead of duplicating parts of devfreq_dev_release.

Signed-off-by: Leonard Crestez <redacted>
---
  drivers/devfreq/devfreq.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 27af1b95fd23..b89a82382536 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c

@@ -689,10 +689,11 @@ struct devfreq *devfreq_add_device(struct device *dev,
  	mutex_init(&devfreq->lock);
  	mutex_lock(&devfreq->lock);
  	devfreq->dev.parent = dev;
  	devfreq->dev.class = devfreq_class;
  	devfreq->dev.release = devfreq_dev_release;
+	device_initialize(&devfreq->dev);
  	INIT_LIST_HEAD(&devfreq->node);
  	devfreq->profile = profile;
  	strncpy(devfreq->governor_name, governor_name, DEVFREQ_NAME_LEN);
  	devfreq->previous_freq = profile->initial_freq;
  	devfreq->last_status.current_frequency = profile->initial_freq;

@@ -726,15 +727,14 @@ struct devfreq *devfreq_add_device(struct device *dev,
  	devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
  	atomic_set(&devfreq->suspend_count, 0);
  
  	dev_set_name(&devfreq->dev, "devfreq%d",
  				atomic_inc_return(&devfreq_no));
-	err = device_register(&devfreq->dev);
+	err = device_add(&devfreq->dev);
  	if (err) {
  		mutex_unlock(&devfreq->lock);
-		put_device(&devfreq->dev);
-		goto err_out;
+		goto err_dev;
  	}
  
  	devfreq->trans_table = devm_kzalloc(&devfreq->dev,
  			array3_size(sizeof(unsigned int),
  				    devfreq->profile->max_state,

@@ -789,13 +789,13 @@ struct devfreq *devfreq_add_device(struct device *dev,
  
  err_init:
  	mutex_unlock(&devfreq_list_lock);
  err_devfreq:
  	devfreq_remove_device(devfreq);
-	devfreq = NULL;
+	return ERR_PTR(err);
  err_dev:
-	kfree(devfreq);
+	put_device(&devfreq->dev);
  err_out:
  	return ERR_PTR(err);
  }
  EXPORT_SYMBOL(devfreq_add_device);
  

As I previously commented, I don't prefer to split out of bodyf of device_register().
Instead, your first version is better without devm.

Very well, feel free to drop 2-5 of this series then.

Or perhaps I misunderstood and the locking cleanups would be acceptable 
in the variant that removes devm from a few allocations? There's quite a 
bunch of stuff flying around the merge window already so I'll refrain 
from posting until v5.5-rc1 anyway.

Don't need to wait the v5.5-rc1. You can send the patches.
But, This series have to be merged to v5.6-rc1.

I went a little overboard with tricky cleanups and this ended up 
delaying the functionality I wanted to push.

--
Regards,
Leonard

-- 
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