[PATCH v2 6/7] clk: clk-imx8mp-audiomix: use the auxiliary device creation helper

Jerome Brunet posted 7 patches 10 months, 1 week ago
There is a newer version of this series
[PATCH v2 6/7] clk: clk-imx8mp-audiomix: use the auxiliary device creation helper
Posted by Jerome Brunet 10 months, 1 week ago
The auxiliary device creation of this driver is simple enough to
use the available auxiliary device creation helper.

Use it and remove some boilerplate code.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/imx/clk-imx8mp-audiomix.c | 57 +++++------------------------------
 1 file changed, 7 insertions(+), 50 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mp-audiomix.c b/drivers/clk/imx/clk-imx8mp-audiomix.c
index c409fc7e061869988f83c7df3ef7860500426323..90e69e253ad5a30cd2a3cb3b63ebe85be1e6b059 100644
--- a/drivers/clk/imx/clk-imx8mp-audiomix.c
+++ b/drivers/clk/imx/clk-imx8mp-audiomix.c
@@ -228,64 +228,21 @@ struct clk_imx8mp_audiomix_priv {
 	struct clk_hw_onecell_data clk_data;
 };
 
-#if IS_ENABLED(CONFIG_RESET_CONTROLLER)
-
-static void clk_imx8mp_audiomix_reset_unregister_adev(void *_adev)
-{
-	struct auxiliary_device *adev = _adev;
-
-	auxiliary_device_delete(adev);
-	auxiliary_device_uninit(adev);
-}
-
-static void clk_imx8mp_audiomix_reset_adev_release(struct device *dev)
+static int clk_imx8mp_audiomix_reset_controller_register(struct device *dev)
 {
-	struct auxiliary_device *adev = to_auxiliary_dev(dev);
-
-	kfree(adev);
-}
-
-static int clk_imx8mp_audiomix_reset_controller_register(struct device *dev,
-							 struct clk_imx8mp_audiomix_priv *priv)
-{
-	struct auxiliary_device *adev __free(kfree) = NULL;
-	int ret;
+	struct auxiliary_device *adev;
 
 	if (!of_property_present(dev->of_node, "#reset-cells"))
 		return 0;
 
-	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
-	if (!adev)
-		return -ENOMEM;
-
-	adev->name = "reset";
-	adev->dev.parent = dev;
-	adev->dev.release = clk_imx8mp_audiomix_reset_adev_release;
-
-	ret = auxiliary_device_init(adev);
-	if (ret)
-		return ret;
-
-	ret = auxiliary_device_add(adev);
-	if (ret) {
-		auxiliary_device_uninit(adev);
-		return ret;
-	}
-
-	return devm_add_action_or_reset(dev, clk_imx8mp_audiomix_reset_unregister_adev,
-					no_free_ptr(adev));
-}
-
-#else /* !CONFIG_RESET_CONTROLLER */
+	adev = devm_auxiliary_device_create(dev, KBUILD_MODNAME,
+					    "reset", NULL, 0);
+	if (IS_ERR_OR_NULL(adev))
+		return PTR_ERR(adev);
 
-static int clk_imx8mp_audiomix_reset_controller_register(struct device *dev,
-							 struct clk_imx8mp_audiomix_priv *priv)
-{
 	return 0;
 }
 
-#endif /* !CONFIG_RESET_CONTROLLER */
-
 static void clk_imx8mp_audiomix_save_restore(struct device *dev, bool save)
 {
 	struct clk_imx8mp_audiomix_priv *priv = dev_get_drvdata(dev);
@@ -408,7 +365,7 @@ static int clk_imx8mp_audiomix_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_clk_register;
 
-	ret = clk_imx8mp_audiomix_reset_controller_register(dev, priv);
+	ret = clk_imx8mp_audiomix_reset_controller_register(dev);
 	if (ret)
 		goto err_clk_register;
 

-- 
2.45.2
Re: [PATCH v2 6/7] clk: clk-imx8mp-audiomix: use the auxiliary device creation helper
Posted by Dan Carpenter 10 months ago
Hi Jerome,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/Jerome-Brunet/driver-core-auxiliary-bus-add-device-creation-helper/20250207-023433
base:   2014c95afecee3e76ca4a56956a936e23283f05b
patch link:    https://lore.kernel.org/r/20250206-aux-device-create-helper-v2-6-fa6a0f326527%40baylibre.com
patch subject: [PATCH v2 6/7] clk: clk-imx8mp-audiomix: use the auxiliary device creation helper
config: xtensa-randconfig-r071-20250208 (https://download.01.org/0day-ci/archive/20250208/202502081655.FlCrxpYN-lkp@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 14.2.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202502081655.FlCrxpYN-lkp@intel.com/

smatch warnings:
drivers/clk/imx/clk-imx8mp-audiomix.c:241 clk_imx8mp_audiomix_reset_controller_register() warn: passing zero to 'PTR_ERR'

vim +/PTR_ERR +241 drivers/clk/imx/clk-imx8mp-audiomix.c

c350f4c434316c Jerome Brunet 2025-02-06  231  static int clk_imx8mp_audiomix_reset_controller_register(struct device *dev)
6f0e817175c5b2 Shengjiu Wang 2024-06-14  232  {
c350f4c434316c Jerome Brunet 2025-02-06  233  	struct auxiliary_device *adev;
6f0e817175c5b2 Shengjiu Wang 2024-06-14  234  
6f0e817175c5b2 Shengjiu Wang 2024-06-14  235  	if (!of_property_present(dev->of_node, "#reset-cells"))
6f0e817175c5b2 Shengjiu Wang 2024-06-14  236  		return 0;
6f0e817175c5b2 Shengjiu Wang 2024-06-14  237  
c350f4c434316c Jerome Brunet 2025-02-06  238  	adev = devm_auxiliary_device_create(dev, KBUILD_MODNAME,
c350f4c434316c Jerome Brunet 2025-02-06  239  					    "reset", NULL, 0);
c350f4c434316c Jerome Brunet 2025-02-06  240  	if (IS_ERR_OR_NULL(adev))
c350f4c434316c Jerome Brunet 2025-02-06 @241  		return PTR_ERR(adev);

If devm_auxiliary_device_create() could return NULL then that would count
as success.  But devm_auxiliary_device_create() can't return NULL.  It
only makes sense to return NULL if the auxiliary device is optional.

https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/

6f0e817175c5b2 Shengjiu Wang 2024-06-14  242  
6f0e817175c5b2 Shengjiu Wang 2024-06-14  243  	return 0;
6f0e817175c5b2 Shengjiu Wang 2024-06-14  244  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v2 6/7] clk: clk-imx8mp-audiomix: use the auxiliary device creation helper
Posted by Jerome Brunet 10 months ago
On Wed 12 Feb 2025 at 10:26, Dan Carpenter <dan.carpenter@linaro.org> wrote:

> Hi Jerome,
>
> kernel test robot noticed the following build warnings:
>
> url:
> https://github.com/intel-lab-lkp/linux/commits/Jerome-Brunet/driver-core-auxiliary-bus-add-device-creation-helper/20250207-023433
> base:   2014c95afecee3e76ca4a56956a936e23283f05b
> patch link:    https://lore.kernel.org/r/20250206-aux-device-create-helper-v2-6-fa6a0f326527%40baylibre.com
> patch subject: [PATCH v2 6/7] clk: clk-imx8mp-audiomix: use the auxiliary device creation helper
> config: xtensa-randconfig-r071-20250208
> (https://download.01.org/0day-ci/archive/20250208/202502081655.FlCrxpYN-lkp@intel.com/config)
> compiler: xtensa-linux-gcc (GCC) 14.2.0
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202502081655.FlCrxpYN-lkp@intel.com/
>
> smatch warnings:
> drivers/clk/imx/clk-imx8mp-audiomix.c:241
> clk_imx8mp_audiomix_reset_controller_register() warn: passing zero to
> 'PTR_ERR'
>
> vim +/PTR_ERR +241 drivers/clk/imx/clk-imx8mp-audiomix.c
>
> c350f4c434316c Jerome Brunet 2025-02-06 231 static int
> clk_imx8mp_audiomix_reset_controller_register(struct device *dev)
> 6f0e817175c5b2 Shengjiu Wang 2024-06-14  232  {
> c350f4c434316c Jerome Brunet 2025-02-06  233  	struct auxiliary_device *adev;
> 6f0e817175c5b2 Shengjiu Wang 2024-06-14  234  
> 6f0e817175c5b2 Shengjiu Wang 2024-06-14  235  	if (!of_property_present(dev->of_node, "#reset-cells"))
> 6f0e817175c5b2 Shengjiu Wang 2024-06-14  236  		return 0;
> 6f0e817175c5b2 Shengjiu Wang 2024-06-14  237  
> c350f4c434316c Jerome Brunet 2025-02-06  238  	adev = devm_auxiliary_device_create(dev, KBUILD_MODNAME,
> c350f4c434316c Jerome Brunet 2025-02-06  239  					    "reset", NULL, 0);
> c350f4c434316c Jerome Brunet 2025-02-06  240  	if (IS_ERR_OR_NULL(adev))
> c350f4c434316c Jerome Brunet 2025-02-06 @241  		return PTR_ERR(adev);
>
> If devm_auxiliary_device_create() could return NULL then that would count
> as success.  But devm_auxiliary_device_create() can't return NULL.  It
> only makes sense to return NULL if the auxiliary device is optional.

Hi Dan,

It should have been IS_ERR() there. It is something I've noticed and
fixed in the other changes before submitting the v2 but, somehow, this
one slipped through. Thanks for catching this. This is obviously still
present in the v3 I've sent yesterday but will be fixed on the next
version. I'm waiting for feedback on the core part before making another
one.

Cheers

>
> https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/
>
> 6f0e817175c5b2 Shengjiu Wang 2024-06-14  242  
> 6f0e817175c5b2 Shengjiu Wang 2024-06-14  243  	return 0;
> 6f0e817175c5b2 Shengjiu Wang 2024-06-14  244  }

-- 
Jerome