* [PATCH] extcon: extcon-axp288: use low level P-Unit semaphore lock for axp288 register accesses
@ 2021-09-09 15:18 Fabio Aiuto
2021-09-09 16:20 ` Hans de Goede
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Fabio Aiuto @ 2021-09-09 15:18 UTC (permalink / raw)
To: Chanwoo Choi; +Cc: MyungJoo Ham, Hans de Goede, linux-kernel, Fabio Aiuto
use low level P-Unit semaphore lock for axp288 register
accesses directly and for more than one access a time,
to reduce the number of times this semaphore is locked
and released which is an expensive operation.
i2c-bus to the XPower is shared between the kernel and the
SoCs P-Unit. The P-Unit has a semaphore wich the kernel must
lock for axp288 register accesses. When the P-Unit semaphore
is locked CPU and GPU power states cannot change or the system
will freeze.
The P-Unit semaphore lock is already managed inside the regmap
access logic, but for each access the semaphore is locked and
released. So use directly iosf_mbi_(un)block_punit_i2c_access(),
we are safe in doing so because nested calls to the same
semaphore are turned to nops.
Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Fabio Aiuto <fabioaiuto83@gmail.com>
---
drivers/extcon/extcon-axp288.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
index fdb31954cf2b..460402b14ef2 100644
--- a/drivers/extcon/extcon-axp288.c
+++ b/drivers/extcon/extcon-axp288.c
@@ -24,6 +24,7 @@
#include <asm/cpu_device_id.h>
#include <asm/intel-family.h>
+#include <asm/iosf_mbi.h>
/* Power source status register */
#define PS_STAT_VBUS_TRIGGER BIT(0)
@@ -215,6 +216,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
unsigned int cable = info->previous_cable;
bool vbus_attach = false;
+ iosf_mbi_block_punit_i2c_access();
+
vbus_attach = axp288_get_vbus_attach(info);
if (!vbus_attach)
goto no_vbus;
@@ -253,6 +256,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
}
no_vbus:
+ iosf_mbi_unblock_punit_i2c_access();
+
extcon_set_state_sync(info->edev, info->previous_cable, false);
if (info->previous_cable == EXTCON_CHG_USB_SDP)
extcon_set_state_sync(info->edev, EXTCON_USB, false);
@@ -275,6 +280,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
return 0;
dev_det_ret:
+ iosf_mbi_unblock_punit_i2c_access();
+
if (ret < 0)
dev_err(info->dev, "failed to detect BC Mod\n");
@@ -307,11 +314,14 @@ static irqreturn_t axp288_extcon_isr(int irq, void *data)
static void axp288_extcon_enable(struct axp288_extcon_info *info)
{
+ iosf_mbi_block_punit_i2c_access();
regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
BC_GLOBAL_RUN, 0);
/* Enable the charger detection logic */
regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
BC_GLOBAL_RUN, BC_GLOBAL_RUN);
+
+ iosf_mbi_unblock_punit_i2c_access();
}
static void axp288_put_role_sw(void *data)
@@ -384,10 +394,14 @@ static int axp288_extcon_probe(struct platform_device *pdev)
}
}
+ iosf_mbi_block_punit_i2c_access();
+
info->vbus_attach = axp288_get_vbus_attach(info);
axp288_extcon_log_rsi(info);
+ iosf_mbi_unblock_punit_i2c_access();
+
/* Initialize extcon device */
info->edev = devm_extcon_dev_allocate(&pdev->dev,
axp288_extcon_cables);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] extcon: extcon-axp288: use low level P-Unit semaphore lock for axp288 register accesses
2021-09-09 15:18 [PATCH] extcon: extcon-axp288: use low level P-Unit semaphore lock for axp288 register accesses Fabio Aiuto
@ 2021-09-09 16:20 ` Hans de Goede
2021-09-10 0:26 ` kernel test robot
2021-09-10 1:00 ` kernel test robot
2 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2021-09-09 16:20 UTC (permalink / raw)
To: Fabio Aiuto, Chanwoo Choi; +Cc: MyungJoo Ham, linux-kernel
Hi All,
On 9/9/21 5:18 PM, Fabio Aiuto wrote:
> use low level P-Unit semaphore lock for axp288 register
> accesses directly and for more than one access a time,
> to reduce the number of times this semaphore is locked
> and released which is an expensive operation.
>
> i2c-bus to the XPower is shared between the kernel and the
> SoCs P-Unit. The P-Unit has a semaphore wich the kernel must
> lock for axp288 register accesses. When the P-Unit semaphore
> is locked CPU and GPU power states cannot change or the system
> will freeze.
>
> The P-Unit semaphore lock is already managed inside the regmap
> access logic, but for each access the semaphore is locked and
> released. So use directly iosf_mbi_(un)block_punit_i2c_access(),
> we are safe in doing so because nested calls to the same
> semaphore are turned to nops.
>
> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Fabio Aiuto <fabioaiuto83@gmail.com>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
I've also given it a test run on a Chuwi Hibook which uses this PMIC:
Tested-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> drivers/extcon/extcon-axp288.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
> index fdb31954cf2b..460402b14ef2 100644
> --- a/drivers/extcon/extcon-axp288.c
> +++ b/drivers/extcon/extcon-axp288.c
> @@ -24,6 +24,7 @@
>
> #include <asm/cpu_device_id.h>
> #include <asm/intel-family.h>
> +#include <asm/iosf_mbi.h>
>
> /* Power source status register */
> #define PS_STAT_VBUS_TRIGGER BIT(0)
> @@ -215,6 +216,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
> unsigned int cable = info->previous_cable;
> bool vbus_attach = false;
>
> + iosf_mbi_block_punit_i2c_access();
> +
> vbus_attach = axp288_get_vbus_attach(info);
> if (!vbus_attach)
> goto no_vbus;
> @@ -253,6 +256,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
> }
>
> no_vbus:
> + iosf_mbi_unblock_punit_i2c_access();
> +
> extcon_set_state_sync(info->edev, info->previous_cable, false);
> if (info->previous_cable == EXTCON_CHG_USB_SDP)
> extcon_set_state_sync(info->edev, EXTCON_USB, false);
> @@ -275,6 +280,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
> return 0;
>
> dev_det_ret:
> + iosf_mbi_unblock_punit_i2c_access();
> +
> if (ret < 0)
> dev_err(info->dev, "failed to detect BC Mod\n");
>
> @@ -307,11 +314,14 @@ static irqreturn_t axp288_extcon_isr(int irq, void *data)
>
> static void axp288_extcon_enable(struct axp288_extcon_info *info)
> {
> + iosf_mbi_block_punit_i2c_access();
> regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
> BC_GLOBAL_RUN, 0);
> /* Enable the charger detection logic */
> regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
> BC_GLOBAL_RUN, BC_GLOBAL_RUN);
> +
> + iosf_mbi_unblock_punit_i2c_access();
> }
>
> static void axp288_put_role_sw(void *data)
> @@ -384,10 +394,14 @@ static int axp288_extcon_probe(struct platform_device *pdev)
> }
> }
>
> + iosf_mbi_block_punit_i2c_access();
> +
> info->vbus_attach = axp288_get_vbus_attach(info);
>
> axp288_extcon_log_rsi(info);
>
> + iosf_mbi_unblock_punit_i2c_access();
> +
> /* Initialize extcon device */
> info->edev = devm_extcon_dev_allocate(&pdev->dev,
> axp288_extcon_cables);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] extcon: extcon-axp288: use low level P-Unit semaphore lock for axp288 register accesses
2021-09-09 15:18 [PATCH] extcon: extcon-axp288: use low level P-Unit semaphore lock for axp288 register accesses Fabio Aiuto
2021-09-09 16:20 ` Hans de Goede
@ 2021-09-10 0:26 ` kernel test robot
2021-09-10 7:03 ` Hans de Goede
2021-09-10 1:00 ` kernel test robot
2 siblings, 1 reply; 6+ messages in thread
From: kernel test robot @ 2021-09-10 0:26 UTC (permalink / raw)
To: Fabio Aiuto, Chanwoo Choi
Cc: llvm, kbuild-all, MyungJoo Ham, Hans de Goede, linux-kernel, Fabio Aiuto
[-- Attachment #1: Type: text/plain, Size: 5892 bytes --]
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@intel.com>
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.
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@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35282 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] extcon: extcon-axp288: use low level P-Unit semaphore lock for axp288 register accesses
2021-09-09 15:18 [PATCH] extcon: extcon-axp288: use low level P-Unit semaphore lock for axp288 register accesses Fabio Aiuto
2021-09-09 16:20 ` Hans de Goede
2021-09-10 0:26 ` kernel test robot
@ 2021-09-10 1:00 ` kernel test robot
2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2021-09-10 1:00 UTC (permalink / raw)
To: Fabio Aiuto, Chanwoo Choi
Cc: kbuild-all, MyungJoo Ham, Hans de Goede, linux-kernel, Fabio Aiuto
[-- Attachment #1: Type: text/plain, Size: 4674 bytes --]
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-r033-20210908 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# 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
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/extcon/extcon-axp288.c: In function 'axp288_handle_chrg_det_event':
>> drivers/extcon/extcon-axp288.c:219:2: error: implicit declaration of function 'iosf_mbi_block_punit_i2c_access' [-Werror=implicit-function-declaration]
219 | 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=implicit-function-declaration]
259 | iosf_mbi_unblock_punit_i2c_access();
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
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@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35346 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] extcon: extcon-axp288: use low level P-Unit semaphore lock for axp288 register accesses
2021-09-10 0:26 ` kernel test robot
@ 2021-09-10 7:03 ` Hans de Goede
2021-09-10 7:13 ` Fabio Aiuto
0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2021-09-10 7:03 UTC (permalink / raw)
To: kernel test robot, Fabio Aiuto, Chanwoo Choi
Cc: llvm, kbuild-all, MyungJoo Ham, linux-kernel
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@intel.com>
>
> 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@lists.01.org
>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] extcon: extcon-axp288: use low level P-Unit semaphore lock for axp288 register accesses
2021-09-10 7:03 ` Hans de Goede
@ 2021-09-10 7:13 ` Fabio Aiuto
0 siblings, 0 replies; 6+ messages in thread
From: Fabio Aiuto @ 2021-09-10 7:13 UTC (permalink / raw)
To: Hans de Goede
Cc: kernel test robot, Chanwoo Choi, llvm, kbuild-all, MyungJoo Ham,
linux-kernel
Hi Hans,
On Fri, Sep 10, 2021 at 09:03:42AM +0200, Hans de Goede wrote:
> Hi,
<snip>
> > ^
> > 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)
sure will fix and resend
>
>
> >
> >
thank you,
fabio
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-10 7:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-09 15:18 [PATCH] extcon: extcon-axp288: use low level P-Unit semaphore lock for axp288 register accesses Fabio Aiuto
2021-09-09 16:20 ` Hans de Goede
2021-09-10 0:26 ` kernel test robot
2021-09-10 7:03 ` Hans de Goede
2021-09-10 7:13 ` Fabio Aiuto
2021-09-10 1:00 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).