From owner-freebsd-arm@freebsd.org Fri Aug 21 20:59:38 2020 Return-Path: Delivered-To: freebsd-arm@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 549DA3C8CED for ; Fri, 21 Aug 2020 20:59:38 +0000 (UTC) (envelope-from mishin@mh.net.ru) Received: from frog.mh.net.ru (mh.balakovo.san.ru [88.147.158.22]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4BYDRD5vywz4T7r for ; Fri, 21 Aug 2020 20:59:35 +0000 (UTC) (envelope-from mishin@mh.net.ru) Received: from webmail.mh.net.ru (mouse.home [192.168.5.6]) by frog.mh.net.ru (Postfix) with ESMTPSA id CF509AD679 for ; Sat, 22 Aug 2020 00:59:25 +0400 (+04) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Sat, 22 Aug 2020 00:59:25 +0400 From: Alexander Mishin To: freebsd-arm@freebsd.org Subject: Re: Kmod driver at iicbus. attach() and config_intrhook(9) In-Reply-To: References: <7fabb65d99aaa74775c1daa91bffb873@mh.net.ru> <3249fa7e-554a-83ef-57b2-7c38aa0b4591@FreeBSD.org> <20200819072409.GA59949@bluezbox.com> <05145b71692af74b103bb226a2e93a15e1e851cb.camel@freebsd.org> <20200820223918.GC4213@funkthat.com> User-Agent: Roundcube Webmail/1.4.2 Message-ID: <496ed13afb2327d6416664b4dacfc346@mh.net.ru> X-Sender: mishin@mh.net.ru X-Rspamd-Queue-Id: 4BYDRD5vywz4T7r X-Spamd-Bar: + Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=fail reason="No valid SPF, No valid DKIM" header.from=mh.net.ru (policy=none); spf=fail (mx1.freebsd.org: domain of mishin@mh.net.ru does not designate 88.147.158.22 as permitted sender) smtp.mailfrom=mishin@mh.net.ru X-Spamd-Result: default: False [1.00 / 15.00]; RCVD_TLS_ALL(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; R_SPF_FAIL(1.00)[-all]; MID_RHS_MATCH_FROM(0.00)[]; FROM_HAS_DN(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; PREVIOUSLY_DELIVERED(0.00)[freebsd-arm@freebsd.org]; TO_DN_NONE(0.00)[]; RCPT_COUNT_ONE(0.00)[1]; ARC_NA(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:12389, ipnet:88.147.128.0/17, country:RU]; RCVD_COUNT_TWO(0.00)[2]; MAILMAN_DEST(0.00)[freebsd-arm]; DMARC_POLICY_SOFTFAIL(0.10)[mh.net.ru : No valid SPF, No valid DKIM,none] X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Aug 2020 20:59:38 -0000 Ian Lepore писал 2020-08-21 02:51: > On Thu, 2020-08-20 at 15:39 -0700, John-Mark Gurney wrote: >> Alexander Mishin wrote this message on Thu, Aug 20, 2020 at 10:07 >> +0400: >> > Ian Lepore ?????????? 2020-08-19 19:39: >> > > On Wed, 2020-08-19 at 00:24 -0700, Oleksandr Tymoshenko wrote: >> > > > Andriy Gapon (avg@FreeBSD.org) wrote: >> > > > > On 18/08/2020 22:05, Alexander Mishin wrote: >> > > > > > Hi >> > > > > > ... >> > > > > > But I see that some other devices (from /usr/src/sys/dev) >> > > > > > uses >> > > > > > CONFIG_INTRHOOK(9) >> > > > > > on attach() for initialize themselfs. >> > > > > > I wonder if I need this too? ... >> > > > > >> > > > > This is usually needed when a driver needs to talk to its >> > > > > device >> > > > > while >> > > > > attaching. E.g., to set some initial configuration or to >> > > > > confirm >> > > > > device's >> > > > > identity, etc. >> > > > >> > > > To extend Andriy's explanation a bit: all these operations may >> > > > perform >> > > > I2C transfers and most I2C controllers use interrupts to get >> > > > notified >> > > > about tranfer status change (finished, error, etc...). There is >> > > > no >> > > > guarantee that when driver's attach method is called interrupts >> > > > are >> > > > globally enabled. What would happen in this case is: I2C >> > > > controller >> > > > is going to initiate I2C operation and wait for an interrupt >> > > > that's >> > > > never going to be delivered. CONFIG_INTRHOOK is a solution for >> > > > this >> > > > problem, if your attach method requires interrupts - just split >> > > > it >> > > > in two parts and postpone running interrupt-dependent part >> > > > until >> > > > after >> > > > interrupts are globally enabled. >> > > > >> > > >> > > A note about all this: It should never be necessary for an i2c >> > > slave >> > > device driver to do this. The reason it's needed is because many >> > > i2c >> > > controller drivers attach the iicbus from their attach() routine >> > > even >> > > though they can't actually do i2c IO until interrupts are >> > > available. >> > > It is these controller drivers that should have the intrhook >> > > logic to >> > > not call bus_generic_attach() until interrupts are available if >> > > they >> > > can't do IO until interrupts are available. >> > > >> > > It has long been my goal to fix all our i2c controller drivers to >> > > behave correctly, so that i2c slave device drivers don't all need >> > > the >> > > intrhook logic. But somehow I never get around to it. >> > >> > I think, it would be helpful, as it would be possible to return an >> > error >> > on early stage, from attach(), if there is no connection with the >> > configured device. >> >> Looks like there's a function bus_delayed_attach_children designed >> exactly for this: >> * Many buses can't run transactions on the bus which children need >> to probe and >> * attach until after interrupts and/or timers are running. This >> function >> * delays their attach until interrupts and timers are enabled. >> >> and it looks like a couple controllers are already using it, imx_i2c >> and ti_i2c... >> >> It looks like maybe a simple replace of bus_generic_attach w/ >> bus_delayed_attach_children should be enough on those w/ >> interrupts... >> >> Is there any argument for doing it for ALL controllers instead of >> just >> some? >> >> Poking around some, and it looks like some (one) drivers "pretend" to >> use interrupts, but just busy wait instead, e.g. exynos5... >> > > Hmm, yeah, it looks like more has been done along these lines than I > remembered. In fact, the work may be done. > > Some i2c controllers have to work properly before interrupts are > available, to control things like PMIC chips that are required very > early in device configuration. Typically they have some sort of > polling mechanism that's used early, and revert to using interrupts > once they're available. The allwinner and rockchip drivers work that > way. > > -- Ian Just to dot the i's, my SoC is allwinner (Orange PI PC). And the issue at boot time really showed itself up until config_intrhook was used. FreeBSD myhost.mh.net.ru 13.0-CURRENT FreeBSD 13.0-CURRENT #3 r364004