| Message ID | 20230613130011.305589-9-linux@rasmusvillemoes.dk |
|---|---|
| State | Accepted |
| Commit | d57d12db774820819d0e591548a56b5cfc95f82a |
| Headers | show |
| Series | rtc: isl12022: battery backup voltage and clock support | expand |
Hi Rasmus,
kernel test robot noticed the following build errors:
[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on robh/for-next linus/master v6.4-rc6 next-20230613]
[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/Rasmus-Villemoes/rtc-isl12022-remove-wrong-warning-for-low-battery-level/20230613-210308
base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
patch link: https://lore.kernel.org/r/20230613130011.305589-9-linux%40rasmusvillemoes.dk
patch subject: [PATCH v2 8/8] rtc: isl12022: implement support for the #clock-cells DT property
config: i386-randconfig-i012-20230612 (https://download.01.org/0day-ci/archive/20230614/202306141318.xPzubJXo-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build):
git remote add abelloni https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git
git fetch abelloni rtc-next
git checkout abelloni/rtc-next
b4 shazam https://lore.kernel.org/r/20230613130011.305589-9-linux@rasmusvillemoes.dk
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
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>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306141318.xPzubJXo-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "__clk_hw_register_fixed_rate" [drivers/rtc/rtc-isl12022.ko] undefined!
>> ERROR: modpost: "of_clk_hw_simple_get" [drivers/rtc/rtc-isl12022.ko] undefined!
>> ERROR: modpost: "devm_of_clk_add_hw_provider" [drivers/rtc/rtc-isl12022.ko] undefined!
On 13/06/2023 17.25, Andy Shevchenko wrote: > On Tue, Jun 13, 2023 at 03:00:10PM +0200, Rasmus Villemoes wrote: >> If device tree implies that the chip's IRQ/F_OUT pin is used as a >> clock, expose that in the driver. For now, pretend it is a >> fixed-rate (32kHz) clock; if other use cases appear the driver can be >> updated to provide its own clk_ops etc. >> >> When the clock output is not used on a given board, one can prolong >> the battery life by ensuring that the FOx bits are 0. For the hardware >> I'm currently working on, the RTC draws 1.2uA with the FOx bits at >> their default 0001 value, dropping to 0.88uA when those bits are >> cleared. > > ... > >> +#define ISL12022_INT_FO_MASK GENMASK(3, 0) >> +#define ISL12022_INT_FO_OFF 0x0 >> +#define ISL12022_INT_FO_32K 0x1 > > A nit-pick. Are they decimal or bit fields? -ENOPARSE. A number is a number. Its representation in C code may be decimal or hexadecimal (or...). And sure, 0 and 0x0 are different spellings of the same thing. The data sheet lists the possible values in terms of individual bits, so I suppose I could even do 0b0000 and 0b0001, but that's too unusual (even if perfectly acceptable by gcc). > To me seems like the 0x can be dropped. Can, but won't, a single hex digit is more natural way to represent a four-bit field. >> + ret = regmap_update_bits(regmap, ISL12022_REG_INT, ISL12022_INT_FO_MASK, ISL12022_INT_FO_32K); > > Seems too long even for 100 limit. > Maybe: > > ret = regmap_update_bits(regmap, ISL12022_REG_INT, > ISL12022_INT_FO_MASK, ISL12022_INT_FO_32K); Sure. Rasmus
diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c index 44603169e575..3e69f1a5dc12 100644 --- a/drivers/rtc/rtc-isl12022.c +++ b/drivers/rtc/rtc-isl12022.c @@ -10,6 +10,7 @@ #include <linux/bcd.h> #include <linux/bitfield.h> +#include <linux/clk-provider.h> #include <linux/err.h> #include <linux/hwmon.h> #include <linux/i2c.h> @@ -44,6 +45,9 @@ #define ISL12022_SR_LBAT75 (1 << 1) #define ISL12022_INT_WRTC (1 << 6) +#define ISL12022_INT_FO_MASK GENMASK(3, 0) +#define ISL12022_INT_FO_OFF 0x0 +#define ISL12022_INT_FO_32K 0x1 #define ISL12022_REG_VB85_MASK GENMASK(5, 3) #define ISL12022_REG_VB75_MASK GENMASK(2, 0) @@ -241,6 +245,37 @@ static const struct regmap_config regmap_config = { .use_single_write = true, }; +static int isl12022_register_clock(struct device *dev) +{ + struct regmap *regmap = dev_get_drvdata(dev); + struct clk_hw *hw; + int ret; + + if (!device_property_present(dev, "#clock-cells")) { + /* + * Disabling the F_OUT pin reduces the power + * consumption in battery mode by ~25%. + */ + regmap_update_bits(regmap, ISL12022_REG_INT, ISL12022_INT_FO_MASK, + ISL12022_INT_FO_OFF); + + return 0; + } + + /* + * For now, only support a fixed clock of 32768Hz (the reset default). + */ + ret = regmap_update_bits(regmap, ISL12022_REG_INT, ISL12022_INT_FO_MASK, ISL12022_INT_FO_32K); + if (ret) + return ret; + + hw = devm_clk_hw_register_fixed_rate(dev, "isl12022", NULL, 0, 32768); + if (IS_ERR(hw)) + return PTR_ERR(hw); + + return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, hw); +} + static const u32 trip_level85[] = { 2125000, 2295000, 2550000, 2805000, 3060000, 4250000, 4675000 }; static const u32 trip_level75[] = { 1875000, 2025000, 2250000, 2475000, 2700000, 3750000, 4125000 }; @@ -284,6 +319,7 @@ static int isl12022_probe(struct i2c_client *client) { struct rtc_device *rtc; struct regmap *regmap; + int ret; if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) return -ENODEV; @@ -296,6 +332,10 @@ static int isl12022_probe(struct i2c_client *client) dev_set_drvdata(&client->dev, regmap); + ret = isl12022_register_clock(&client->dev); + if (ret) + return ret; + isl12022_set_trip_levels(&client->dev); isl12022_hwmon_register(&client->dev);
If device tree implies that the chip's IRQ/F_OUT pin is used as a clock, expose that in the driver. For now, pretend it is a fixed-rate (32kHz) clock; if other use cases appear the driver can be updated to provide its own clk_ops etc. When the clock output is not used on a given board, one can prolong the battery life by ensuring that the FOx bits are 0. For the hardware I'm currently working on, the RTC draws 1.2uA with the FOx bits at their default 0001 value, dropping to 0.88uA when those bits are cleared. Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- drivers/rtc/rtc-isl12022.c | 40 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)