Date: Sat, 22 Sep 2007 18:27:17 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Eugene Grosbein <eugen@grosbein.pp.ru> 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 Message-ID: <20070922172534.R46046@delplex.bde.org> In-Reply-To: <200709220513.l8M5DlQw002006@grosbein.pp.ru> References: <200709220513.l8M5DlQw002006@grosbein.pp.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070922172534.R46046>