[PATCH] iio: accel: bma220: move set_wdt() out of bma220_core

[PATCH] iio: accel: bma220: move set_wdt() out of bma220_core

[Petre Rodan]

Move bma220_set_wdt() into bma220_i2c.c instead of using a conditional
based on i2c_verify_client() in bma220_core.c that would make core
always depend on the i2c module.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202510102117.Jqxrw1vF-lkp@intel.com/
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
This patch fixes a linking error reported by the kernel robot

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
head:   89cba586b8b4cde09c44b1896624720ea29f0205
commit: 3499375209ca839a741e775d579f8bb9b85529d5 [56/96] iio: accel: bma220: add i2c watchdog feature
config: um-randconfig-002-20251021 (https://download.01.org/0day-ci/archive/20251021/202510210604.mAtgE54g-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251021/202510210604.mAtgE54g-lkp@intel.com/reproduce)

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/202510210604.mAtgE54g-lkp@intel.com/

All errors (new ones prefixed by >>):

   /usr/bin/ld: drivers/iio/accel/bma220_core.o: in function `bma220_init':
>> bma220_core.c:(.text+0x3f8): undefined reference to `i2c_verify_client'
   collect2: error: ld returned 1 exit status

not sure how to make b4 not send out the cover letter for 1 patch series
or how to make this patch a reply to the robot's email.
---
 drivers/iio/accel/bma220.h      |  6 ++++++
 drivers/iio/accel/bma220_core.c | 19 -------------------
 drivers/iio/accel/bma220_i2c.c  | 14 +++++++++++++-
 3 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/accel/bma220.h b/drivers/iio/accel/bma220.h
index e53ca63de54b..00dfe275256b 100644
--- a/drivers/iio/accel/bma220.h
+++ b/drivers/iio/accel/bma220.h
@@ -11,6 +11,12 @@
 #include <linux/pm.h>
 #include <linux/regmap.h>
 
+#define BMA220_REG_WDT				0x17
+#define BMA220_WDT_MASK				GENMASK(2, 1)
+#define BMA220_WDT_OFF				0x0
+#define BMA220_WDT_1MS				0x2
+#define BMA220_WDT_10MS				0x3
+
 struct device;
 
 extern const struct regmap_config bma220_i2c_regmap_config;
diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 871342d21456..f32d875b994e 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -11,7 +11,6 @@
 #include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/errno.h>
-#include <linux/i2c.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -78,11 +77,6 @@
 #define BMA220_FILTER_MASK			GENMASK(3, 0)
 #define BMA220_REG_RANGE			0x11
 #define BMA220_RANGE_MASK			GENMASK(1, 0)
-#define BMA220_REG_WDT				0x17
-#define BMA220_WDT_MASK				GENMASK(2, 1)
-#define BMA220_WDT_OFF				0x0
-#define BMA220_WDT_1MS				0x2
-#define BMA220_WDT_10MS				0x3
 #define BMA220_REG_SUSPEND			0x18
 #define BMA220_REG_SOFTRESET			0x19
 
@@ -443,12 +437,6 @@ static int bma220_power(struct bma220_data *data, bool up)
 	return -EBUSY;
 }
 
-static int bma220_set_wdt(struct bma220_data *data, const u8 val)
-{
-	return regmap_update_bits(data->regmap, BMA220_REG_WDT, BMA220_WDT_MASK,
-				  FIELD_PREP(BMA220_WDT_MASK, val));
-}
-
 static int bma220_init(struct device *dev, struct bma220_data *data)
 {
 	int ret;
@@ -477,13 +465,6 @@ static int bma220_init(struct device *dev, struct bma220_data *data)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to soft reset chip\n");
 
-	if (i2c_verify_client(dev)) {
-		ret = bma220_set_wdt(data, BMA220_WDT_1MS);
-		if (ret)
-			return dev_err_probe(dev, ret,
-					     "Failed to set i2c watchdog\n");
-	}
-
 	return 0;
 }
 
diff --git a/drivers/iio/accel/bma220_i2c.c b/drivers/iio/accel/bma220_i2c.c
index 2b85d4921768..8b6f8e305c8c 100644
--- a/drivers/iio/accel/bma220_i2c.c
+++ b/drivers/iio/accel/bma220_i2c.c
@@ -8,6 +8,7 @@
  * I2C address is either 0x0b or 0x0a depending on CSB (pin 10)
  */
 
+#include <linux/bitfield.h>
 #include <linux/i2c.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
@@ -16,16 +17,27 @@
 
 #include "bma220.h"
 
+static int bma220_set_wdt(struct regmap *regmap, const u8 val)
+{
+	return regmap_update_bits(regmap, BMA220_REG_WDT, BMA220_WDT_MASK,
+				  FIELD_PREP(BMA220_WDT_MASK, val));
+}
+
 static int bma220_i2c_probe(struct i2c_client *client)
 {
 	struct regmap *regmap;
+	int ret;
 
 	regmap = devm_regmap_init_i2c(client, &bma220_i2c_regmap_config);
 	if (IS_ERR(regmap))
 		return dev_err_probe(&client->dev, PTR_ERR(regmap),
 				     "failed to create regmap\n");
 
-	return bma220_common_probe(&client->dev, regmap, client->irq);
+	ret = bma220_common_probe(&client->dev, regmap, client->irq);
+	if (ret)
+		return ret;
+
+	return bma220_set_wdt(regmap, BMA220_WDT_1MS);
 }
 
 static const struct of_device_id bma220_i2c_match[] = {

---
base-commit: 89cba586b8b4cde09c44b1896624720ea29f0205
change-id: 20251021-bma220_set_wdt_move-06556f561ac8

Best regards,
-- 
Petre Rodan <petre.rodan@subdimension.ro>

Re: [PATCH] iio: accel: bma220: move set_wdt() out of bma220_core

[Andy Shevchenko]

On Tue, Oct 21, 2025 at 01:31:49PM +0300, Petre Rodan wrote:
> Move bma220_set_wdt() into bma220_i2c.c instead of using a conditional
> based on i2c_verify_client() in bma220_core.c that would make core
> always depend on the i2c module.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

But Kconfig for this driver is a bit strange. Usually we do other way around,
i.e. make user visible selection of the glue drivers, while core is selected if
at least one of the leaf driver selected by the user.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: accel: bma220: move set_wdt() out of bma220_core

[Jonathan Cameron]

On Wed, 22 Oct 2025 14:50:18 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Tue, Oct 21, 2025 at 01:31:49PM +0300, Petre Rodan wrote:
> > Move bma220_set_wdt() into bma220_i2c.c instead of using a conditional
> > based on i2c_verify_client() in bma220_core.c that would make core
> > always depend on the i2c module.  
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> But Kconfig for this driver is a bit strange. Usually we do other way around,
> i.e. make user visible selection of the glue drivers, while core is selected if
> at least one of the leaf driver selected by the user.
> 
This comes up from time to time.  There kind of isn't a right answer
to my mind in the trade off between complexity of configuration 
and desire for minimum useful set of Kconfig symbols and people wanting
to build only exactly what they want.  So we've ended up with a mix.

I don't mind setting a policy on this for new code going forwards, but
that means we need to decide which approach we prefer and document
it somewhere.

Jonathan

Re: [PATCH] iio: accel: bma220: move set_wdt() out of bma220_core

[Andy Shevchenko]

On Thu, Oct 23, 2025 at 06:23:18PM +0100, Jonathan Cameron wrote:
> On Wed, 22 Oct 2025 14:50:18 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Oct 21, 2025 at 01:31:49PM +0300, Petre Rodan wrote:
> > > Move bma220_set_wdt() into bma220_i2c.c instead of using a conditional
> > > based on i2c_verify_client() in bma220_core.c that would make core
> > > always depend on the i2c module.  
> > 
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > But Kconfig for this driver is a bit strange. Usually we do other way around,
> > i.e. make user visible selection of the glue drivers, while core is selected if
> > at least one of the leaf driver selected by the user.
> > 
> This comes up from time to time.  There kind of isn't a right answer
> to my mind in the trade off between complexity of configuration 
> and desire for minimum useful set of Kconfig symbols and people wanting
> to build only exactly what they want.  So we've ended up with a mix.
> 
> I don't mind setting a policy on this for new code going forwards, but
> that means we need to decide which approach we prefer and document
> it somewhere.

One immediate thing from top of my mind why current approach worse than
proposed by me.

- when you have an SPI or I²C enabled the glue driver will be silently build.
As a used on the embedded system with smaller resources (flash/ROM) I prefer
the opposite behaviour, i.e. when _I_ control what SPI/I²C component driver is
needed even if the subsystem is present.

But let's give a time to gather other opinions and justifications.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: accel: bma220: move set_wdt() out of bma220_core

[Petre Rodan]

Hello Jonathan.

On Thu, Oct 23, 2025 at 06:23:18PM +0100, Jonathan Cameron wrote:
> On Wed, 22 Oct 2025 14:50:18 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > On Tue, Oct 21, 2025 at 01:31:49PM +0300, Petre Rodan wrote:
> > > Move bma220_set_wdt() into bma220_i2c.c instead of using a conditional
> > > based on i2c_verify_client() in bma220_core.c that would make core
> > > always depend on the i2c module.  
> > 
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > But Kconfig for this driver is a bit strange. Usually we do other way around,
> > i.e. make user visible selection of the glue drivers, while core is selected if
> > at least one of the leaf driver selected by the user.
> > 
> This comes up from time to time.  There kind of isn't a right answer
> to my mind in the trade off between complexity of configuration 
> and desire for minimum useful set of Kconfig symbols and people wanting
> to build only exactly what they want.  So we've ended up with a mix.
> 
> I don't mind setting a policy on this for new code going forwards, but
> that means we need to decide which approach we prefer and document
> it somewhere.

I will come back with a new patch to Kconfig once you decide what is the best way to handle dependecies, but in the meantime can you please accept this current patch?

I keep getting automated errors that would be fixed by it:

https://lore.kernel.org/oe-kbuild-all/202510210604.mAtgE54g-lkp@intel.com/
https://lore.kernel.org/oe-kbuild-all/202510222324.SxYlIaLW-lkp@intel.com/
https://lore.kernel.org/oe-kbuild-all/202510271347.115BMnsC-lkp@intel.com/

If the current patch does not correctly reference the automated 0day-ci reports please tell me what I should change within my b4 workflow.

thank you,
peter

Re: [PATCH] iio: accel: bma220: move set_wdt() out of bma220_core

[Jonathan Cameron]

On Tue, 21 Oct 2025 13:31:49 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Move bma220_set_wdt() into bma220_i2c.c instead of using a conditional
> based on i2c_verify_client() in bma220_core.c that would make core
> always depend on the i2c module.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202510102117.Jqxrw1vF-lkp@intel.com/
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
Applied to the togreg branch of iio.git.

Thanks,
J

Re: [PATCH] iio: accel: bma220: move set_wdt() out of bma220_core

[Jonathan Cameron]

On Mon, 27 Oct 2025 07:27:35 +0200
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Hello Jonathan.
> 
> On Thu, Oct 23, 2025 at 06:23:18PM +0100, Jonathan Cameron wrote:
> > On Wed, 22 Oct 2025 14:50:18 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >   
> > > On Tue, Oct 21, 2025 at 01:31:49PM +0300, Petre Rodan wrote:  
> > > > Move bma220_set_wdt() into bma220_i2c.c instead of using a conditional
> > > > based on i2c_verify_client() in bma220_core.c that would make core
> > > > always depend on the i2c module.    
> > > 
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > 
> > > But Kconfig for this driver is a bit strange. Usually we do other way around,
> > > i.e. make user visible selection of the glue drivers, while core is selected if
> > > at least one of the leaf driver selected by the user.
> > >   
> > This comes up from time to time.  There kind of isn't a right answer
> > to my mind in the trade off between complexity of configuration 
> > and desire for minimum useful set of Kconfig symbols and people wanting
> > to build only exactly what they want.  So we've ended up with a mix.
> > 
> > I don't mind setting a policy on this for new code going forwards, but
> > that means we need to decide which approach we prefer and document
> > it somewhere.  
> 
> I will come back with a new patch to Kconfig once you decide what is the best way to handle dependecies, but in the meantime can you please accept this current patch?
> 
> I keep getting automated errors that would be fixed by it:
> 
> https://lore.kernel.org/oe-kbuild-all/202510210604.mAtgE54g-lkp@intel.com/
> https://lore.kernel.org/oe-kbuild-all/202510222324.SxYlIaLW-lkp@intel.com/
> https://lore.kernel.org/oe-kbuild-all/202510271347.115BMnsC-lkp@intel.com/

Done. Was travelling (and on wrong computer).

Should be resolved now.
> 
> If the current patch does not correctly reference the automated 0day-ci reports please tell me what I should change within my b4 workflow.
> 
> thank you,
> peter