From owner-freebsd-arm@freebsd.org Tue Sep 1 22:18:43 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 CE0A537F998 for ; Tue, 1 Sep 2020 22:18:43 +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 4Bh1gR0Zpbz3VPJ for ; Tue, 1 Sep 2020 22:18:42 +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 081MIeJS032308 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 1 Sep 2020 15:18:40 -0700 (PDT) (envelope-from jmg@gold.funkthat.com) Received: (from jmg@localhost) by gold.funkthat.com (8.15.2/8.15.2/Submit) id 081MIdYx032307; Tue, 1 Sep 2020 15:18:39 -0700 (PDT) (envelope-from jmg) Date: Tue, 1 Sep 2020 15:18:39 -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: <20200901221839.GY4213@funkthat.com> Mail-Followup-To: Alexander Mishin , freebsd-arm@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0496ebc5628a015c005c150549e1e70a@mh.net.ru> <496ed13afb2327d6416664b4dacfc346@mh.net.ru> 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]); Tue, 01 Sep 2020 15:18:40 -0700 (PDT) X-Rspamd-Queue-Id: 4Bh1gR0Zpbz3VPJ 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 [0.07 / 15.00]; RCVD_TLS_ALL(0.00)[]; ARC_NA(0.00)[]; FREEFALL_USER(0.00)[jmg]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; NEURAL_HAM_MEDIUM(-0.60)[-0.595]; MID_RHS_MATCH_FROM(0.00)[]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[funkthat.com]; AUTH_NA(1.00)[]; NEURAL_SPAM_SHORT(0.43)[0.429]; NEURAL_HAM_LONG(-0.97)[-0.968]; 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]; ASN(0.00)[asn:32354, ipnet:208.87.216.0/21, country:US]; FROM_NEQ_ENVFROM(0.00)[jmg@funkthat.com,jmg@gold.funkthat.com]; MAILMAN_DEST(0.00)[freebsd-arm] 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: Tue, 01 Sep 2020 22:18:43 -0000 Alexander Mishin wrote this message on Sat, Aug 22, 2020 at 00:59 +0400: > 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. > > > 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 Maybe I misread this thread, but this implied to me that the device had to do the config_intrhook work in order to work on the twsi i2c bus driver, is this not the case? Alexander Mishin wrote this message on Tue, Sep 01, 2020 at 19:21 +0400: > Andriy Gapon ?????????? 2020-09-01 09:51: > > On 01/09/2020 03:26, John-Mark Gurney wrote: > >> Ok, then the OP shouldn't be having a problem then, maybe you can help > >> then figure it out then? > > > > Was there actually any problem? > > I thought that OP was just inquiring about the pattern commonly seen > > in the code. > > > > Alexander Mishin wrote: > >> 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? ...or maybe... when I might need it? > > Yes, exactly. And I quite understood about using config_intrhook(9), > thank you all! > ...but still interesting to read if the need for this would be removed > for the twsi controller. So my reading of the twsi driver is that it handles things correctly allowing devices pre-interrupts to attach, and using interrupts for transfers post-cold. So, your driver does not need to do any config_intrhook magic to make it work. If you run across any other i2c bus controllers than need it, then that controller should be fixed. -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."