From owner-freebsd-arm@freebsd.org Thu Aug 20 22:39:29 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 407193AB156 for ; Thu, 20 Aug 2020 22:39:29 +0000 (UTC) (envelope-from jmg@gold.funkthat.com) Received: from gold.funkthat.com (gate2.funkthat.com [208.87.223.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "gate2.funkthat.com", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BXfhw23lpz466Y for ; Thu, 20 Aug 2020 22:39:27 +0000 (UTC) (envelope-from jmg@gold.funkthat.com) Received: from gold.funkthat.com (localhost [127.0.0.1]) by gold.funkthat.com (8.15.2/8.15.2) with ESMTPS id 07KMdJxL021343 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 20 Aug 2020 15:39:19 -0700 (PDT) (envelope-from jmg@gold.funkthat.com) Received: (from jmg@localhost) by gold.funkthat.com (8.15.2/8.15.2/Submit) id 07KMdIRI021342; Thu, 20 Aug 2020 15:39:18 -0700 (PDT) (envelope-from jmg) Date: Thu, 20 Aug 2020 15:39:18 -0700 From: John-Mark Gurney To: Alexander Mishin Cc: freebsd-arm@freebsd.org Subject: Re: Kmod driver at iicbus. attach() and config_intrhook(9) Message-ID: <20200820223918.GC4213@funkthat.com> Mail-Followup-To: Alexander Mishin , freebsd-arm@freebsd.org References: <7fabb65d99aaa74775c1daa91bffb873@mh.net.ru> <3249fa7e-554a-83ef-57b2-7c38aa0b4591@FreeBSD.org> <20200819072409.GA59949@bluezbox.com> <05145b71692af74b103bb226a2e93a15e1e851cb.camel@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: FreeBSD 11.3-STABLE amd64 X-PGP-Fingerprint: D87A 235F FB71 1F3F 55B7 ED9B D5FF 5A51 C0AC 3D65 X-Files: The truth is out there X-URL: https://www.funkthat.com/ X-Resume: https://www.funkthat.com/~jmg/resume.html X-TipJar: bitcoin:13Qmb6AeTgQecazTWph4XasEsP7nGRbAPE X-to-the-FBI-CIA-and-NSA: HI! HOW YA DOIN? can i haz chizburger? User-Agent: Mutt/1.6.1 (2016-04-27) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (gold.funkthat.com [127.0.0.1]); Thu, 20 Aug 2020 15:39:19 -0700 (PDT) X-Rspamd-Queue-Id: 4BXfhw23lpz466Y X-Spamd-Bar: ++ Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=none (mx1.freebsd.org: domain of jmg@gold.funkthat.com has no SPF policy when checking 208.87.223.18) smtp.mailfrom=jmg@gold.funkthat.com X-Spamd-Result: default: False [2.03 / 15.00]; MID_RHS_MATCH_FROM(0.00)[]; MAILMAN_DEST(0.00)[freebsd-arm]; FREEFALL_USER(0.00)[jmg]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; ARC_NA(0.00)[]; NEURAL_SPAM_SHORT(0.06)[0.064]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[funkthat.com]; AUTH_NA(1.00)[]; NEURAL_SPAM_MEDIUM(0.76)[0.764]; TO_MATCH_ENVRCPT_SOME(0.00)[]; RCPT_COUNT_TWO(0.00)[2]; R_SPF_NA(0.00)[no SPF record]; FORGED_SENDER(0.30)[jmg@funkthat.com,jmg@gold.funkthat.com]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; FROM_NEQ_ENVFROM(0.00)[jmg@funkthat.com,jmg@gold.funkthat.com]; ASN(0.00)[asn:32354, ipnet:208.87.216.0/21, country:US] 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: Thu, 20 Aug 2020 22:39:29 -0000 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... -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."