Message ID | d0a1be24701dcf19a2f7501a9bc7fddf2b739792.1631021349.git.krzysztof.adamski@nokia.com |
---|---|
State | New |
Headers | show |
Series | Add per channel properies support in tmp421 | expand |
Hi Krzysztof, I love your patch! Perhaps something to improve: [auto build test WARNING on hwmon/hwmon-next] [also build test WARNING on robh/for-next v5.14 next-20210907] [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] url: https://github.com/0day-ci/linux/commits/Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20210907-214724 base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next config: mips-randconfig-c004-20210907 (attached as .config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 9c476172b93367d2cb88d7d3f4b1b5b456fa6020) 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 mips cross compiling tool for clang build # apt-get install binutils-mips-linux-gnu # https://github.com/0day-ci/linux/commit/56fe3c49ca67e9ea9c4f8a40438b5adc58c417f1 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20210907-214724 git checkout 56fe3c49ca67e9ea9c4f8a40438b5adc58c417f1 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): drivers/hwmon/tmp421.c:301:6: warning: no previous prototype for function 'tmp421_probe_child_from_dt' [-Wmissing-prototypes] void tmp421_probe_child_from_dt(struct i2c_client *client, ^ drivers/hwmon/tmp421.c:301:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void tmp421_probe_child_from_dt(struct i2c_client *client, ^ static drivers/hwmon/tmp421.c:345:6: warning: no previous prototype for function 'tmp421_probe_from_dt' [-Wmissing-prototypes] void tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *data) ^ drivers/hwmon/tmp421.c:345:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *data) ^ static >> drivers/hwmon/tmp421.c:356:6: warning: no previous prototype for function 'tmp421_disable_channels' [-Wmissing-prototypes] void tmp421_disable_channels(struct i2c_client *client, uint8_t mask) ^ drivers/hwmon/tmp421.c:356:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void tmp421_disable_channels(struct i2c_client *client, uint8_t mask) ^ static 3 warnings generated. vim +/tmp421_disable_channels +356 drivers/hwmon/tmp421.c 355 > 356 void tmp421_disable_channels(struct i2c_client *client, uint8_t mask) 357 { 358 int err; 359 int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2); 360 361 if (cfg < 0) { 362 dev_err(&client->dev, 363 "error reading register, can't disable channels\n"); 364 return; 365 } 366 367 cfg &= ~mask; 368 369 err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg); 370 if (err < 0) 371 dev_err(&client->dev, 372 "error writing register, can't disable channels\n"); 373 } 374 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Krzysztof,
I love your patch! Perhaps something to improve:
[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on robh/for-next v5.14 next-20210909]
[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]
url: https://github.com/0day-ci/linux/commits/Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20210907-214724
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-randconfig-s001-20210908 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/56fe3c49ca67e9ea9c4f8a40438b5adc58c417f1
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20210907-214724
git checkout 56fe3c49ca67e9ea9c4f8a40438b5adc58c417f1
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/hwmon/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
drivers/hwmon/tmp421.c:301:6: sparse: sparse: symbol 'tmp421_probe_child_from_dt' was not declared. Should it be static?
drivers/hwmon/tmp421.c:345:6: sparse: sparse: symbol 'tmp421_probe_from_dt' was not declared. Should it be static?
>> drivers/hwmon/tmp421.c:356:6: sparse: sparse: symbol 'tmp421_disable_channels' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c index 90c6b094785e..cec25fb1c771 100644 --- a/drivers/hwmon/tmp421.c +++ b/drivers/hwmon/tmp421.c @@ -33,6 +33,8 @@ enum chips { tmp421, tmp422, tmp423, tmp441, tmp442 }; /* The TMP421 registers */ #define TMP421_STATUS_REG 0x08 #define TMP421_CONFIG_REG_1 0x09 +#define TMP421_CONFIG_REG_2 0x0A +#define TMP421_CONFIG_REG_REN(x) (BIT(3 + (x))) #define TMP421_CONVERSION_RATE_REG 0x0B #define TMP421_N_FACTOR_REG_1 0x21 #define TMP421_MANUFACTURER_ID_REG 0xFE @@ -351,6 +353,25 @@ void tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *data) } } +void tmp421_disable_channels(struct i2c_client *client, uint8_t mask) +{ + int err; + int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2); + + if (cfg < 0) { + dev_err(&client->dev, + "error reading register, can't disable channels\n"); + return; + } + + cfg &= ~mask; + + err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg); + if (err < 0) + dev_err(&client->dev, + "error writing register, can't disable channels\n"); +} + static const struct hwmon_ops tmp421_ops = { .is_visible = tmp421_is_visible, .read = tmp421_read, @@ -363,6 +384,7 @@ static int tmp421_probe(struct i2c_client *client) struct device *hwmon_dev; struct tmp421_data *data; int i, err; + u8 disable = 0; data = devm_kzalloc(dev, sizeof(struct tmp421_data), GFP_KERNEL); if (!data) @@ -380,11 +402,18 @@ static int tmp421_probe(struct i2c_client *client) if (err) return err; - for (i = 0; i < data->channels; i++) - data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT; - tmp421_probe_from_dt(client, data); + for (i = 0; i < data->channels; i++) { + data->temp_config[i] |= HWMON_T_INPUT | HWMON_T_FAULT; + if (data->channel[i].disabled) + disable |= TMP421_CONFIG_REG_REN(i); + + } + + if (disable) + tmp421_disable_channels(client, disable); + data->chip.ops = &tmp421_ops; data->chip.info = data->info;
Recent patch added possibility to disable selected channels. That would only make sure that the ENODATA is returned for those channels but would not configure the actual hardware. With this patch, the config register is written to make sure the channels are disabled also at hardware level. Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com> --- drivers/hwmon/tmp421.c | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-)