From owner-freebsd-arm@freebsd.org Mon Mar 5 02:24:00 2018 Return-Path: Delivered-To: freebsd-arm@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E0B08F247D1 for ; Mon, 5 Mar 2018 02:23:59 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from outbound1a.eu.mailhop.org (outbound1a.eu.mailhop.org [52.58.109.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 6F5267D795 for ; Mon, 5 Mar 2018 02:23:59 +0000 (UTC) (envelope-from ian@freebsd.org) X-MHO-User: 41983f24-201c-11e8-91c6-33ffc249f3e8 X-Report-Abuse-To: https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information X-Originating-IP: 67.177.211.60 X-Mail-Handler: DuoCircle Outbound SMTP Received: from ilsoft.org (unknown [67.177.211.60]) by outbound1.eu.mailhop.org (Halon) with ESMTPSA id 41983f24-201c-11e8-91c6-33ffc249f3e8; Mon, 05 Mar 2018 02:23:57 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.15.2/8.15.2) with ESMTP id w252Nr1w094455; Sun, 4 Mar 2018 19:23:53 -0700 (MST) (envelope-from ian@freebsd.org) Message-ID: <1520216633.38056.12.camel@freebsd.org> Subject: Re: Panic in spi driver From: Ian Lepore To: Thomas Skibo , freebsd-arm@freebsd.org Date: Sun, 04 Mar 2018 19:23:53 -0700 In-Reply-To: <838BFE5B-61CD-4EC4-BB4F-8124B5B3AF9F@yahoo.com> References: <838BFE5B-61CD-4EC4-BB4F-8124B5B3AF9F@yahoo.com> Content-Type: text/plain; charset="windows-1251" X-Mailer: Evolution 3.18.5.1 FreeBSD GNOME Team Port Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Mar 2018 02:24:00 -0000 On Sun, 2018-03-04 at 11:38 -0800, Thomas Skibo via freebsd-arm wrote: > I’m developing a qspi driver for Zynq/Zedboard and running into a > problem that might affect other spi drivers.  When my driver does its > attach, it triggers the attach of the flash driver, > dev/flash/mx25l.c, through spibus.  In turn, the flash driver > attempts to read the flash device ident by calling back to my > driver’s transfer function.  My driver initiates the transfer and > then sleeps on its lock with a timeout (mtx_sleep(…, 2 * hz)).  That > panics the system: “panic: timed sleep before timers are > working”.  I’ve attached the stack backtrace. > > I loosely based my driver on bcm2835_spi.c which also sleeps with a > timeout in its transfer function.  I tried greping through a few > other spi drivers and noticed dev/intel/spi.c does it too.   > > Okay, before I hit send, I noticed that the other flash driver, > at45d.c, uses some hook to do a delayed attach with the comment > “We’ll see what kind of flash we have later…”.  Maybe mx25l.c needs > something like this. > > My other idea is to create a “fast transfer” function that doesn’t > use interrupts to be used for trivial transfers.  That would take > care of the IDENT and READ_STATUS commands that happen in the flash > driver’s attach routine.  It might be a nice optimization too. > > —Thomas Ooops, I just realized I had the same problem in the new imx6 spi driver I committed a couple days ago.  I did all my testing using loadable modules, forgot to ever test compiling everything in.  As soon as I did, panic. So in the i2c world this problem was historically fixed by every individual slave device driver using a config_intrhook_establish() in its attach routine to do the real attach work later, after interrupts are working.  We should avoid the same mistake in the spi world. The way I see it, if a driver that performs IO for its children requires interrupts to function, it should not attach the child devices until interrupts are working.  In other words, put the config_intrhook in the single driver that really needs it, not in every child driver. If there's a situation where the spi driver has to do IO before interrupts can be enabled (maybe to make a PMIC or other hardware- control device work early in boot), then it has to have a transfer implementation that uses some kind of polling loop until interrupts are available (basically, have two implementations of transfer()). I just committed the imx_spi fix in r330438.  The basic fix is instead of ending attach() with    return (bus_generic_attach(sc->dev)); do    config_intrhook_oneshot((ich_func_t)bus_generic_attach, dev);   return (0); -- Ian