Re: [PATCH] extcon: extcon-axp288: use low level P-Unit semaphore lock for axp288 register accesses

From: Hans de Goede
Date: Fri Sep 10 2021 - 03:07:00 EST

Hi,

On 9/10/21 2:26 AM, kernel test robot wrote:
> Hi Fabio,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on chanwoo-extcon/extcon-next]
> [also build test ERROR on 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/Fabio-Aiuto/extcon-extcon-axp288-use-low-level-P-Unit-semaphore-lock-for-axp288-register-accesses/20210909-232054
> base: https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git extcon-next
> config: x86_64-randconfig-a011-20210908 (attached as .config)
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 261cbe98c38f8c1ee1a482fe76511110e790f58a)
> 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
> # https://github.com/0day-ci/linux/commit/ecccd5dd3a8acfd5085a5cf9f9c97ed3d4b42a1f
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Fabio-Aiuto/extcon-extcon-axp288-use-low-level-P-Unit-semaphore-lock-for-axp288-register-accesses/20210909-232054
> git checkout ecccd5dd3a8acfd5085a5cf9f9c97ed3d4b42a1f
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
>
> All errors (new ones prefixed by >>):
>
>>> drivers/extcon/extcon-axp288.c:219:2: error: implicit declaration of function 'iosf_mbi_block_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
> iosf_mbi_block_punit_i2c_access();
> ^
>>> drivers/extcon/extcon-axp288.c:259:2: error: implicit declaration of function 'iosf_mbi_unblock_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
> iosf_mbi_unblock_punit_i2c_access();
> ^
> drivers/extcon/extcon-axp288.c:259:2: note: did you mean 'iosf_mbi_block_punit_i2c_access'?
> drivers/extcon/extcon-axp288.c:219:2: note: 'iosf_mbi_block_punit_i2c_access' declared here
> iosf_mbi_block_punit_i2c_access();
> ^
> drivers/extcon/extcon-axp288.c:317:2: error: implicit declaration of function 'iosf_mbi_block_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
> iosf_mbi_block_punit_i2c_access();
> ^
> drivers/extcon/extcon-axp288.c:324:2: error: implicit declaration of function 'iosf_mbi_unblock_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
> iosf_mbi_unblock_punit_i2c_access();
> ^
> drivers/extcon/extcon-axp288.c:397:2: error: implicit declaration of function 'iosf_mbi_block_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
> iosf_mbi_block_punit_i2c_access();
> ^
> drivers/extcon/extcon-axp288.c:403:2: error: implicit declaration of function 'iosf_mbi_unblock_punit_i2c_access' [-Werror,-Wimplicit-function-declaration]
> iosf_mbi_unblock_punit_i2c_access();
> ^
> 6 errors generated.

Ah yes, I should have caught this, we had the same issue with a similar patch to
the axp288-fuel-gauge driver.

The fix is this:

diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index c69d40ae5619..aab87c9b35c8 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -23,7 +23,7 @@ config EXTCON_ADC_JACK

config EXTCON_AXP288
tristate "X-Power AXP288 EXTCON support"
- depends on MFD_AXP20X && USB_SUPPORT && X86 && ACPI
+ depends on MFD_AXP20X && USB_SUPPORT && X86 && ACPI && IOSF_MBI
select USB_ROLE_SWITCH
help
Say Y here to enable support for USB peripheral detection

The new depends on is fine since all boards which use the AXP288 should
always have IOSF_MBI enabled anyways.

Fabio, can you send a v2 with this Kconfig change added please ?

(and please also add my earlier Reviewed-by and Tested-by to the v2)


>
>
> vim +/iosf_mbi_block_punit_i2c_access +219 drivers/extcon/extcon-axp288.c
>
> 211
> 212 static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
> 213 {
> 214 int ret, stat, cfg;
> 215 u8 chrg_type;
> 216 unsigned int cable = info->previous_cable;
> 217 bool vbus_attach = false;
> 218
> > 219 iosf_mbi_block_punit_i2c_access();
> 220
> 221 vbus_attach = axp288_get_vbus_attach(info);
> 222 if (!vbus_attach)
> 223 goto no_vbus;
> 224
> 225 /* Check charger detection completion status */
> 226 ret = regmap_read(info->regmap, AXP288_BC_GLOBAL_REG, &cfg);
> 227 if (ret < 0)
> 228 goto dev_det_ret;
> 229 if (cfg & BC_GLOBAL_DET_STAT) {
> 230 dev_dbg(info->dev, "can't complete the charger detection\n");
> 231 goto dev_det_ret;
> 232 }
> 233
> 234 ret = regmap_read(info->regmap, AXP288_BC_DET_STAT_REG, &stat);
> 235 if (ret < 0)
> 236 goto dev_det_ret;
> 237
> 238 chrg_type = (stat & DET_STAT_MASK) >> DET_STAT_SHIFT;
> 239
> 240 switch (chrg_type) {
> 241 case DET_STAT_SDP:
> 242 dev_dbg(info->dev, "sdp cable is connected\n");
> 243 cable = EXTCON_CHG_USB_SDP;
> 244 break;
> 245 case DET_STAT_CDP:
> 246 dev_dbg(info->dev, "cdp cable is connected\n");
> 247 cable = EXTCON_CHG_USB_CDP;
> 248 break;
> 249 case DET_STAT_DCP:
> 250 dev_dbg(info->dev, "dcp cable is connected\n");
> 251 cable = EXTCON_CHG_USB_DCP;
> 252 break;
> 253 default:
> 254 dev_warn(info->dev, "unknown (reserved) bc detect result\n");
> 255 cable = EXTCON_CHG_USB_SDP;
> 256 }
> 257
> 258 no_vbus:
> > 259 iosf_mbi_unblock_punit_i2c_access();
> 260
> 261 extcon_set_state_sync(info->edev, info->previous_cable, false);
> 262 if (info->previous_cable == EXTCON_CHG_USB_SDP)
> 263 extcon_set_state_sync(info->edev, EXTCON_USB, false);
> 264
> 265 if (vbus_attach) {
> 266 extcon_set_state_sync(info->edev, cable, vbus_attach);
> 267 if (cable == EXTCON_CHG_USB_SDP)
> 268 extcon_set_state_sync(info->edev, EXTCON_USB,
> 269 vbus_attach);
> 270
> 271 info->previous_cable = cable;
> 272 }
> 273
> 274 if (info->role_sw && info->vbus_attach != vbus_attach) {
> 275 info->vbus_attach = vbus_attach;
> 276 /* Setting the role can take a while */
> 277 queue_work(system_long_wq, &info->role_work);
> 278 }
> 279
> 280 return 0;
> 281
> 282 dev_det_ret:
> 283 iosf_mbi_unblock_punit_i2c_access();
> 284
> 285 if (ret < 0)
> 286 dev_err(info->dev, "failed to detect BC Mod\n");
> 287
> 288 return ret;
> 289 }
> 290
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx
>