From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759559AbYHUTlL (ORCPT ); Thu, 21 Aug 2008 15:41:11 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751594AbYHUTk5 (ORCPT ); Thu, 21 Aug 2008 15:40:57 -0400 Received: from smtp1.linux-foundation.org ([140.211.169.13]:48753 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752113AbYHUTkz (ORCPT ); Thu, 21 Aug 2008 15:40:55 -0400 Date: Thu, 21 Aug 2008 12:39:49 -0700 From: Andrew Morton To: Keith Wansbrough Cc: axboe@kernel.dk, alain@linux.lu, linux-kernel@vger.kernel.org, gene.heskett@gmail.com, Michael Kerrisk Subject: Re: [PATCH] floppy: support arbitrary first-sector numbers Message-Id: <20080821123949.fc1af4ba.akpm@linux-foundation.org> In-Reply-To: <20080809184214.GA13495@ayla.home.lochan.org> References: <20080809184214.GA13495@ayla.home.lochan.org> X-Mailer: Sylpheed version 2.2.4 (GTK+ 2.8.20; i486-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 9 Aug 2008 19:42:14 +0100 Keith Wansbrough wrote: > The current floppy_struct allows floppies to number sectors starting > from 0 or 1. This patch allows arbitrary first-sector numbers - for > example, 0xC1 for Amstrad CPC disks. > > This extends the existing 1-bit field (FD_ZEROBASED, bit 2 of stretch) > to 8 bits (FD_SECTMASK, bits 2 to 9). > > Currently 0x00 denotes a first sector number of 1, and 0x01 denotes a > first sector number of 0. We extend this by interpreting FD_SECTMASK > as the first sector number with the LSB flipped. > > Signed-off-by: Keith Wansbrough > --- > > I've tested formatting, writing, and reading disks with a variety of > first sector numbers (including 0 and 1), and all works as expected. > > I have made corresponding changes to fdutils, which I will submit > separately to the fdutils maintainer. > > There's no floppy maintainer listed in MAINTAINERS, so this is sent to > the block subsystem maintainer and to the fdutils maintainer. I'm cc:ing > Gene because I know he's been working in this area recently. > > --KW 8-) > > Index: linux-2.6.27-rc1-git4/drivers/block/floppy.c > =================================================================== > --- linux-2.6.27-rc1-git4.orig/drivers/block/floppy.c 2008-08-02 23:36:32.000000000 +0100 > +++ linux-2.6.27-rc1-git4/drivers/block/floppy.c 2008-08-02 23:36:37.000000000 +0100 > @@ -423,8 +423,15 @@ static struct floppy_raw_cmd *raw_cmd, d > * 1581's logical side 0 is on physical side 1, whereas the Sharp's logical > * side 0 is on physical side 0 (but with the misnamed sector IDs). > * 'stretch' should probably be renamed to something more general, like > - * 'options'. Other parameters should be self-explanatory (see also > - * setfdprm(8)). > + * 'options'. > + * > + * Bits 2 through 9 of 'stretch' tell the number of the first sector. > + * The LSB (bit 2) is flipped. For most disks, the first sector > + * is 1 (represented by 0x00<<2). For some CP/M and music sampler > + * disks (such as Ensoniq EPS 16plus) it is 0 (represented as 0x01<<2). > + * For Amstrad CPC disks it is 0xC1 (represented as 0xC0<<2). > + * > + * Other parameters should be self-explanatory (see also setfdprm(8)). > */ > /* > Size > @@ -2236,9 +2243,9 @@ static void setup_format_params(int trac > } > } > } > - if (_floppy->stretch & FD_ZEROBASED) { > + if (_floppy->stretch & FD_SECTBASEMASK) { > for (count = 0; count < F_SECT_PER_TRACK; count++) > - here[count].sect--; > + here[count].sect += FD_SECTBASE(_floppy) - 1; > } > } > > @@ -2649,7 +2656,7 @@ static int make_raw_rw_request(void) > } > HEAD = fsector_t / _floppy->sect; > > - if (((_floppy->stretch & (FD_SWAPSIDES | FD_ZEROBASED)) || > + if (((_floppy->stretch & (FD_SWAPSIDES | FD_SECTBASEMASK)) || > TESTF(FD_NEED_TWADDLE)) && fsector_t < _floppy->sect) > max_sector = _floppy->sect; > > @@ -2679,7 +2686,7 @@ static int make_raw_rw_request(void) > CODE2SIZE; > SECT_PER_TRACK = _floppy->sect << 2 >> SIZECODE; > SECTOR = ((fsector_t % _floppy->sect) << 2 >> SIZECODE) + > - ((_floppy->stretch & FD_ZEROBASED) ? 0 : 1); > + FD_SECTBASE(_floppy); > > /* tracksize describes the size which can be filled up with sectors > * of size ssize. > @@ -3311,7 +3318,7 @@ static inline int set_geometry(unsigned > g->head <= 0 || > g->track <= 0 || g->track > UDP->tracks >> STRETCH(g) || > /* check if reserved bits are set */ > - (g->stretch & ~(FD_STRETCH | FD_SWAPSIDES | FD_ZEROBASED)) != 0) > + (g->stretch & ~(FD_STRETCH | FD_SWAPSIDES | FD_SECTBASEMASK)) != 0) > return -EINVAL; > if (type) { > if (!capable(CAP_SYS_ADMIN)) > @@ -3356,7 +3363,7 @@ static inline int set_geometry(unsigned > if (DRS->maxblock > user_params[drive].sect || > DRS->maxtrack || > ((user_params[drive].sect ^ oldStretch) & > - (FD_SWAPSIDES | FD_ZEROBASED))) > + (FD_SWAPSIDES | FD_SECTBASEMASK))) > invalidate_drive(bdev); > else > process_fd_request(); > Index: linux-2.6.27-rc1-git4/include/linux/fd.h > =================================================================== > --- linux-2.6.27-rc1-git4.orig/include/linux/fd.h 2008-08-02 23:36:32.000000000 +0100 > +++ linux-2.6.27-rc1-git4/include/linux/fd.h 2008-08-02 23:36:37.000000000 +0100 > @@ -15,10 +15,16 @@ struct floppy_struct { > sect, /* sectors per track */ > head, /* nr of heads */ > track, /* nr of tracks */ > - stretch; /* !=0 means double track steps */ > + stretch; /* bit 0 !=0 means double track steps */ > + /* bit 1 != 0 means swap sides */ > + /* bits 2..9 give the first sector */ > + /* number (the LSB is flipped) */ > #define FD_STRETCH 1 > #define FD_SWAPSIDES 2 > #define FD_ZEROBASED 4 > +#define FD_SECTBASEMASK 0x3FC > +#define FD_MKSECTBASE(s) (((s) ^ 1) << 2) > +#define FD_SECTBASE(floppy) ((((floppy)->stretch & FD_SECTBASEMASK) >> 2) ^ 1) > > unsigned char gap, /* gap1 size */ > So this is a backward-compatible enhancement to the floppy FDSETPRM and FDDEFPRM ioctl. If some userspace wasn't setting those bits to zero, it loses, and probably deserved to. I'd suggest an update to the FDDEFPRM documentation, only it isn't documented. Who is the fdutils maintainer?