Date: Thu, 16 Aug 2018 10:10:05 +0300 From: Toomas Soome <tsoome@me.com> To: John Baldwin <jhb@FreeBSD.org> Cc: Warner Losh <imp@bsdimp.com>, Ian Lepore <ian@freebsd.org>, Toomas Soome <tsoome@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r337878 - head/stand/i386/libi386 Message-ID: <18E7897D-9985-44CE-B648-2A839417B1BC@me.com> In-Reply-To: <05b36083-9f4d-658e-ab39-e8c317d01db7@FreeBSD.org> References: <201808152225.w7FMP5J2018006@repo.freebsd.org> <1534372134.1466.21.camel@freebsd.org> <CANCZdfpR_v5_ES=XhpKK6GD6Hd7zneObwKQwf40P0aDLxWh=WA@mail.gmail.com> <05b36083-9f4d-658e-ab39-e8c317d01db7@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> On 16 Aug 2018, at 09:59, John Baldwin <jhb@FreeBSD.org> wrote: >=20 > On 8/15/18 11:59 PM, Warner Losh wrote: >>=20 >>=20 >> On Wed, Aug 15, 2018 at 4:28 PM, Ian Lepore <ian@freebsd.org = <mailto:ian@freebsd.org>> wrote: >>=20 >> On Wed, 2018-08-15 at 22:25 +0000, Toomas Soome wrote: >>> Author: tsoome >>> Date: Wed Aug 15 22:25:05 2018 >>> New Revision: 337878 >>> URL: https://svnweb.freebsd.org/changeset/base/337878 = <https://svnweb.freebsd.org/changeset/base/337878>; >>>=20 >>> Log: >>> libi386: remove bd_read() and bd_write() wrappers >>> =20 >>> Those wroappers are nice, but do not really add much value. >>>=20 >>> Modified: >>> head/stand/i386/libi386/biosdisk.c >>>=20 >>> Modified: head/stand/i386/libi386/biosdisk.c >>> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>> =3D=3D=3D=3D=3D=3D=3D=3D=3D >>> --- head/stand/i386/libi386/biosdisk.c Wed Aug 15 21:47:03 >>> 2018 (r337877) >>> +++ head/stand/i386/libi386/biosdisk.c Wed Aug 15 22:25:05 >>> 2018 (r337878) >>> @@ -94,10 +94,7 @@ static int nbdinfo =3D 0; >>> =20 >>> static void bd_io_workaround(struct disk_devdesc *dev); >>> =20 >>> -static int bd_read(struct disk_devdesc *dev, daddr_t dblk, int = blks, >>> - caddr_t dest); >>> -static int bd_write(struct disk_devdesc *dev, daddr_t dblk, int >>> blks, >>> - caddr_t dest); >>> +static int bd_io(struct disk_devdesc *, daddr_t, int, caddr_t, = int); >>> static int bd_int13probe(struct bdinfo *bd); >>> =20 >>> static int bd_init(void); >>> @@ -506,7 +503,7 @@ bd_realstrategy(void *devdata, int rw, daddr_t >>> dblk, s >>> case F_READ: >>> DEBUG("read %d from %lld to %p", blks, dblk, buf); >>> =20 >>> - if (blks && (rc =3D bd_read(dev, dblk, blks, buf))) { >>> + if (blks && (rc =3D bd_io(dev, dblk, blks, buf, 0))) { >>> /* Filter out floppy controller errors */ >>> if (BD(dev).bd_flags !=3D BD_FLOPPY || rc !=3D >>> 0x20) { >>> printf("read %d from %lld to %p, >>> error: 0x%x\n", >>> @@ -518,7 +515,7 @@ bd_realstrategy(void *devdata, int rw, daddr_t >>> dblk, s >>> case F_WRITE : >>> DEBUG("write %d from %lld to %p", blks, dblk, buf); >>> =20 >>> - if (blks && bd_write(dev, dblk, blks, buf)) { >>> + if (blks && bd_io(dev, dblk, blks, buf, 1)) { >>> DEBUG("write error"); >>> return (EIO); >>> } >>> @@ -713,20 +710,6 @@ bd_io(struct disk_devdesc *dev, daddr_t dblk, >>> int blks >>> } >>> =20 >>> return (0); >>> -} >>> - >>> -static int >>> -bd_read(struct disk_devdesc *dev, daddr_t dblk, int blks, caddr_t >>> dest) >>> -{ >>> - >>> - return (bd_io(dev, dblk, blks, dest, 0)); >>> -} >>> - >>> -static int >>> -bd_write(struct disk_devdesc *dev, daddr_t dblk, int blks, caddr_t >>> dest) >>> -{ >>> - >>> - return (bd_io(dev, dblk, blks, dest, 1)); >>> } >>> =20 >>> /* >>>=20 >>=20 >> This would be a more satisfying change if there were something = like >>=20 >> #define BD_RD 0 >> #define BD_WR 1 >>=20 >> so that it was clear at a glance what a bd_io() call is doing. >>=20 >>=20 >> I think that's a good idea... >=20 > Arguably the bd_read/write wrappers were even clearer (and there = purpose > was readability in that case). Yes thats true, but also will leave us in mercy of inlining etc.. = anyhow, *my* purpose is to get the line of changes done (to be able to = perform IO with >512 sector size, merge with bioscd.c and split up = floppy, cd and hdd cases, so the user can distinguish the devices.=20 rgds, toomas
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?18E7897D-9985-44CE-B648-2A839417B1BC>