| Message ID | e5d7afd3-2e1a-ad5a-a455-8882fae9cc47@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | clk: add devm_add_clk_disable_unprepare | expand |
Hi Heiner,
I love your patch! Perhaps something to improve:
[auto build test WARNING on clk/clk-next]
[also build test WARNING on linus/master v6.2-rc8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Heiner-Kallweit/clk-add-devm_add_clk_disable_unprepare/20230219-070000
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link: https://lore.kernel.org/r/e5d7afd3-2e1a-ad5a-a455-8882fae9cc47%40gmail.com
patch subject: [PATCH] clk: add devm_add_clk_disable_unprepare
config: arm-buildonly-randconfig-r001-20230219 (https://download.01.org/0day-ci/archive/20230219/202302191007.tgr4xPrA-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project db89896bbbd2251fff457699635acbbedeead27f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/intel-lab-lkp/linux/commit/c4da42247f42760cecce516094595706f88f4bc9
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Heiner-Kallweit/clk-add-devm_add_clk_disable_unprepare/20230219-070000
git checkout c4da42247f42760cecce516094595706f88f4bc9
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/clk/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302191007.tgr4xPrA-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/clk/clk-devres.c:228:39: warning: cast from 'void (*)(struct clk *)' to 'void (*)(void *)' converts to incompatible function type [-Wcast-function-type-strict]
return devm_add_action_or_reset(dev, (void (*)(void *))clk_disable_unprepare, clk);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
vim +228 drivers/clk/clk-devres.c
225
226 int devm_add_clk_disable_unprepare(struct device *dev, struct clk *clk)
227 {
> 228 return devm_add_action_or_reset(dev, (void (*)(void *))clk_disable_unprepare, clk);
Quoting Heiner Kallweit (2023-02-18 14:58:54) > I'm aware that there have been numerous rejected attempts to add a > devm_clk_prepare_enable(). The advice I've seen was: See devm_clk_get_optional_enabled() > If you need this functionality, use devm_add_action_or_reset(). > As a result lots of callers implemented their own action wrapper for > clk_disable_unprepare(). Just one example: dw8250_clk_disable_unprepare > This can be avoided with a little bit of cast magic, as used by few > drivers already. > Helper devm_add_clk_disable_unprepare() is supposed to reduce the > code duplication caused by all these individual wrappers. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c index 4fb4fd4b0..97e7c33b6 100644 --- a/drivers/clk/clk-devres.c +++ b/drivers/clk/clk-devres.c @@ -222,3 +222,9 @@ struct clk *devm_get_clk_from_child(struct device *dev, return clk; } EXPORT_SYMBOL(devm_get_clk_from_child); + +int devm_add_clk_disable_unprepare(struct device *dev, struct clk *clk) +{ + return devm_add_action_or_reset(dev, (void (*)(void *))clk_disable_unprepare, clk); +} +EXPORT_SYMBOL_GPL(devm_add_clk_disable_unprepare); diff --git a/include/linux/clk.h b/include/linux/clk.h index 1ef013324..c4d3b1cc0 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -732,6 +732,17 @@ void clk_bulk_put_all(int num_clks, struct clk_bulk_data *clks); */ void devm_clk_put(struct device *dev, struct clk *clk); +/** + * devm_add_clk_disable_unprepare - device-managed call to clk_disable_unprepare + * @dev: device used to acquire the clock + * @clk: clock source acquired with devm_clk_get() et al + * + * Notes: + * - should only be called if clock was acquired device-managed + * - should not be called from within interrupt context + */ +int devm_add_clk_disable_unprepare(struct device *dev, struct clk *clk); + /* * The remaining APIs are optional for machine class support. */
I'm aware that there have been numerous rejected attempts to add a devm_clk_prepare_enable(). The advice I've seen was: If you need this functionality, use devm_add_action_or_reset(). As a result lots of callers implemented their own action wrapper for clk_disable_unprepare(). Just one example: dw8250_clk_disable_unprepare This can be avoided with a little bit of cast magic, as used by few drivers already. Helper devm_add_clk_disable_unprepare() is supposed to reduce the code duplication caused by all these individual wrappers. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/clk/clk-devres.c | 6 ++++++ include/linux/clk.h | 11 +++++++++++ 2 files changed, 17 insertions(+)