From owner-freebsd-bugs@FreeBSD.ORG Sat Sep 22 08:27:25 2007 Return-Path: Delivered-To: freebsd-bugs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 46ACB16A418; Sat, 22 Sep 2007 08:27:25 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail10.syd.optusnet.com.au (mail10.syd.optusnet.com.au [211.29.132.191]) by mx1.freebsd.org (Postfix) with ESMTP id D99D713C447; Sat, 22 Sep 2007 08:27:24 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c220-239-235-248.carlnfd3.nsw.optusnet.com.au (c220-239-235-248.carlnfd3.nsw.optusnet.com.au [220.239.235.248]) by mail10.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id l8M8RISk014519 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 22 Sep 2007 18:27:21 +1000 Date: Sat, 22 Sep 2007 18:27:17 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Eugene Grosbein In-Reply-To: <200709220513.l8M5DlQw002006@grosbein.pp.ru> Message-ID: <20070922172534.R46046@delplex.bde.org> References: <200709220513.l8M5DlQw002006@grosbein.pp.ru> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-bugs@freebsd.org, FreeBSD-gnats-submit@freebsd.org Subject: Re: kern/116536: [fdc] [patch] fdc(4) does not respect hint.fd.0.flags from device.hints X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 22 Sep 2007 08:27:25 -0000 On Sat, 22 Sep 2007, Eugene Grosbein wrote: >> Description: > Changing hint.fd.0.flags has no effect on fdc(4) > ... >> Fix: > > --- sys/dev/fdc/fdc.c.orig 2007-09-15 16:53:23.000000000 +0800 > +++ sys/dev/fdc/fdc.c 2007-09-16 16:26:07.000000000 +0800 > @@ -1848,6 +1864,7 @@ > fd->dev = dev; > fd->fdc = fdc; > fd->fdsu = fdsu; > + fd->flags = flags; > unit = device_get_unit(dev); > > /* Auto-probe if fdinfo is present, but always allow override. */ fd->flags has nothing to do with the device flags. This copy of the device flags gets clobbered later when fd->flags is used for more-dynamic flags. Note that this copy is for fd.x.flags. There is also fdc.x.flags. Unbroken versions of fd call device_get_flags() in several places in order to get the device (fd or fdc) flags. One of the places is just before the above in fdprobe() to get the fd flags, and the flags variable is used later in fdprobe(), so the fd flags still work in fdprobe(). This seems give correct behaviour for all currently supported fd flags. However, device_get_flags() is never called to get the fdc flags, so fdc flags don't work. > @@ -1857,7 +1874,7 @@ > goto done; > } else { > /* make sure fdautoselect() will be called */ > - fd->flags = FD_EMPTY; > + fd->flags |= FD_EMPTY; > fd->type = type; > } > In particular, FD_EMPTY is (1<<3), so this setting of a dynamic flag clobbers the fd flag for the top bit in FD_TYPEMASK. This may be harmless, but setting other low bits in FD_TYPEMASK from the fd flags probably breaks them. The only fd flag other than FD_TYPEMASK is 0x20 (FD_NO_PROBE). This is only used in fdprobe(), so it still seems to work correctly. 0x20 in fd->flags is FD_ISA_DMA. This is unrelated. The hole at 0x10 is for the changeline flag which you are unbreaking in another PR. This used to be read in Fdopen(). Confusion was increased by spelling the variable that holds it dflags while the variable that holds it in the above is spelled flag. Old confusion starts with not using the fd_ prefix for the flags struct member. This bug has been fixed for many of the struct members associated with the fd, but not for flags. (Older code generally uses a prefix for fdc members but no prefix for fd members.) The only documented fdc flag is 0x1 (no fifo). This apparently never worked, but only the man page was broken. The flag was actually (1 << 2) (FDC_NO_FIFO). Now the man page still says 0x1 but FDC_NO_FIFO is not defined and the fdc flags are never read. Bruce