From mboxrd@z Thu Jan 1 00:00:00 1970 From: Roger Quadros Subject: Re: Bug with dwc3 id detect and regulators Date: Mon, 11 Jun 2018 11:53:32 +0300 Message-ID: <941bfd38-e975-e460-81aa-ce8d90c7a3f5@ti.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: Content-Language: en-GB Sender: linux-kernel-owner@vger.kernel.org To: "H. Nikolaus Schaller" , Tony Lindgren , Kishon Vijay Abraham I , Chanwoo Choi Cc: Discussions about the Letux Kernel , kernel@pyra-handheld.com, linux-omap , "linux-kernel@vger.kernel.org" List-Id: linux-omap@vger.kernel.org Chanwoo, On 11/06/18 11:33, H. Nikolaus Schaller wrote: > Hi Tony, > another bug... > > [ 174.540313] BUG: scheduling while atomic: kworker/0:4/1327/0x00000002 > [ 174.547353] Modules linked in: omapdrm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm drm_panel_orientation_quirks bnep bluetooth ecdh_generic usb_f_ecm g_ether usb_f_rndis u_ether libcomposite configfs ipv6 arc4 wl18xx wlcore mac80211 panel_boe_w677l snd_soc_omap_hdmi_audio snd_soc_dmic cfg80211 dwc3 snd_soc_omap_abe_twl6040 snd_soc_twl6040 leds_gpio wwan_on_off connector_hdmi omapdss encoder_tpd12s015 cec pwm_omap_dmtimer omapdss_base pwm_bl generic_adc_battery ehci_omap wlcore_sdio dwc3_omap bmp280_spi snd_soc_ts3a227e crtouch_mt leds_is31fl319x tsc2007 bq2429x_charger bq27xxx_battery_i2c bq27xxx_battery ina2xx as5013 tca8418_keypad twl6040_vibra palmas_pwrbutton gpio_twl6040 palmas_gpadc usb3503 bmp280_i2c bmc150_accel_i2c w2cbw003_bluetooth bmc150_magn_i2c bmp28 0 bmc150_accel_core > [ 174.624601] bmc150_magn bno055 industrialio_triggered_buffer kfifo_buf industrialio snd_soc_omap_mcbsp snd_soc_omap_mcpdm snd_soc_omap snd_pcm_dmaengine [last unloaded: syscopyarea] > [ 174.642327] CPU: 0 PID: 1327 Comm: kworker/0:4 Tainted: G W 4.17.0-letux+ #2408 > [ 174.651541] Hardware name: Generic OMAP5 (Flattened Device Tree) > [ 174.658004] Workqueue: events_power_efficient palmas_gpio_id_detect > [ 174.664780] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > [ 174.673109] [] (show_stack) from [] (dump_stack+0x7c/0x9c) > [ 174.680881] [] (dump_stack) from [] (__schedule_bug+0x60/0x84) > [ 174.689006] [] (__schedule_bug) from [] (__schedule+0x50/0x694) > [ 174.697217] [] (__schedule) from [] (schedule+0xb0/0xcc) > [ 174.704790] [] (schedule) from [] (schedule_timeout+0x354/0x3d4) > [ 174.713100] [] (schedule_timeout) from [] (wait_for_common+0x118/0x158) > [ 174.722064] [] (wait_for_common) from [] (omap_i2c_xfer+0x354/0x48c) > [ 174.730749] [] (omap_i2c_xfer) from [] (__i2c_transfer+0x238/0x550) > [ 174.739350] [] (__i2c_transfer) from [] (i2c_transfer+0x84/0xb4) > [ 174.747685] [] (i2c_transfer) from [] (bq24296_i2c_reg8_read.constprop.8+0x54/0x64 [bq2429x_charger]) > [ 174.759465] [] (bq24296_i2c_reg8_read.constprop.8 [bq2429x_charger]) from [] (bq24296_update_reg+0x28/0xf8 [bq2429x_charger]) > [ 174.773437] [] (bq24296_update_reg [bq2429x_charger]) from [] (_regulator_do_disable+0x100/0x238) > [ 174.784804] [] (_regulator_do_disable) from [] (_regulator_disable+0x88/0x120) > [ 174.794404] [] (_regulator_disable) from [] (regulator_disable+0x30/0x60) > [ 174.803556] [] (regulator_disable) from [] (dwc3_omap_set_mailbox+0x84/0xf8 [dwc3_omap]) > [ 174.814124] [] (dwc3_omap_set_mailbox [dwc3_omap]) from [] (dwc3_omap_id_notifier+0x14/0x1c [dwc3_omap]) > [ 174.826149] [] (dwc3_omap_id_notifier [dwc3_omap]) from [] (notifier_call_chain+0x40/0x68) > [ 174.836867] [] (notifier_call_chain) from [] (raw_notifier_call_chain+0x14/0x1c) > [ 174.846661] [] (raw_notifier_call_chain) from [] (extcon_sync+0x54/0x19c) > [ 174.855804] [] (extcon_sync) from [] (process_one_work+0x244/0x464) > [ 174.864386] [] (process_one_work) from [] (worker_thread+0x2c0/0x3ec) > [ 174.873156] [] (worker_thread) from [] (kthread+0x134/0x150) > [ 174.881087] [] (kthread) from [] (ret_from_fork+0x14/0x2c) > > It turns out that extcon_sync() holds a spinlock while sending its notifiers and this > is ending up in our regulator.en/disable() which wants to use blocking i2c. > > Do you see similar things on the OMAP5EVM when using OTG mode? > The Palmas SMPS10 is also handled through i2c. Or is this magically hidden by regmap? > > Well, as a workaround, I can make the regulator.en/disable() in the bq2429x driver > just trigger a worker, but IMHO it is not expected for regulator ops to be spinlock safe. > > So I think extcon should not spinlock (which might be against the extcon design) or I think this something that should be addressed in the Extcon layer. Do you really need to call the raw_notifier_call_chain() function with spinlock held in extcon_sync()? if yes why? I think we don't want to call all notifiers in atomic context as this would keep interrupts disabled for quite some time and is suboptimal. > dwc3_omap_set_mailbox should move dis/enabling regulator to some worker thread so > that they can block. > > The best would be to make dwc3_omap_set_mailbox call regulator_enable_deferred(omap->vbus_reg, 0) > but that function does not exist. > > Any ideas? > > BR, > Nikolaus > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- cheers, -roger Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki