From owner-freebsd-hackers@freebsd.org Sun Mar 18 14:30:32 2018 Return-Path: Delivered-To: freebsd-hackers@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 681CDF58311 for ; Sun, 18 Mar 2018 14:30:32 +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 E2BE173A88 for ; Sun, 18 Mar 2018 14:30:31 +0000 (UTC) (envelope-from ian@freebsd.org) X-MHO-User: e5d427a1-2ab8-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 e5d427a1-2ab8-11e8-91c6-33ffc249f3e8; Sun, 18 Mar 2018 14:30:26 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.15.2/8.15.2) with ESMTP id w2IEUKLH034326; Sun, 18 Mar 2018 08:30:20 -0600 (MDT) (envelope-from ian@freebsd.org) Message-ID: <1521383420.99081.87.camel@freebsd.org> Subject: Re: Custom I2C and RTC chip drivers: where is iccbus_get_nostop() defined? From: Ian Lepore To: Lee D , FreeBSD Hackers Date: Sun, 18 Mar 2018 08:30:20 -0600 In-Reply-To: References: Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.18.5.1 FreeBSD GNOME Team Port Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Mar 2018 14:30:32 -0000 On Sun, 2018-03-18 at 00:46 -0400, Lee D wrote: > Hi Everyone, > > I am back to working on my Zynq I2C and M41T82 RTC chip drivers.  I am > still using 11.0.1. > > It turns out that the Zynq I2C hardware is buggy and it doesn't really > fit in with the FreeBSD paradigm of issuing discrete bus transactions > (start, stop, etc.) > > I am trying to work around this by writing my own version of > iicbus_transfer_gen(), copied from src/sys/dev/iicbus/iiconf.c > > My question is, where is iicbus_get_nostop() defined?  I can't seem to > find it with grep. > > "nostop" seems to get turned on at some point, and while I could just > ignore it, I'd like to know where and how it is happening. > > Thanks, > > Lee Nostop is an ivar of the child device, so iicbus_get_nostop() is formed by the IICBUS_ACCESSOR macro in iicbus.h. Now for the bad news:  don't use it.  It doesn't work.  It's 100% a bug in the code that maybe kinda-sorta seemed to work for whoever added it, because accidentally the right garbage was on the stack in the local nostop var.  The generic transfer code doesn't check that the accessor failed so it ends up using stack garbage for nostop.  The reason there's g'teed to be no such ivar is because the code is asking the wrong device, and it doesn't even have a handle to the right child device to get the info it wants. So the bottom line is, write your transfer routine to work however it has to work.  That might mean ignoring the stop/nostop flags and just doing whatever your hardware does.  Like if a write transaction is handled by the hardware by putting the slave address and the offse- within-slave values into registers and it does a write of the offset then a read from the slave and you get no control over whether it does that as two transactions or as 1 transaction with a repeat-start between the read and write, then just silently do it that way. I had forgotten about the nostop mess, which I discovered some time last year.  It really needs to be fixed the right way, which is to remove the nostop hack, remove all uses of the NOSTOP flag everywhere in the code, and make the default behavior that all back-to-back operations in the same array of cmds handed to a single transfer call have implicit repeat-start behavior when changing slave or direction.  We could add a specific STOP flag to force a stop/start between two commands, but even that's not really needed. The only example of a transfer-only driver I know of offhand is the rpi driver (arm/broadcom/bcm2835/bcm2835_bsc.c).  Unfortunately, bugs in the rpi silicon complicate the code and make it a messy example. -- Ian