From owner-svn-src-head@freebsd.org Thu Aug 16 14:00:52 2018 Return-Path: Delivered-To: svn-src-head@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 CD0E01066F5B; Thu, 16 Aug 2018 14:00:51 +0000 (UTC) (envelope-from tsoome@me.com) Received: from st13p35im-asmtp002.me.com (st13p35im-asmtp002.me.com [17.164.199.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 7704C83BC0; Thu, 16 Aug 2018 14:00:51 +0000 (UTC) (envelope-from tsoome@me.com) Received: from process-dkim-sign-daemon.st13p35im-asmtp002.me.com by st13p35im-asmtp002.me.com (Oracle Communications Messaging Server 8.0.2.2.20180531 64bit (built May 31 2018)) id <0PDK004003Q0K100@st13p35im-asmtp002.me.com>; Thu, 16 Aug 2018 14:00:43 +0000 (GMT) Received: from icloud.com ([127.0.0.1]) by st13p35im-asmtp002.me.com (Oracle Communications Messaging Server 8.0.2.2.20180531 64bit (built May 31 2018)) with ESMTPSA id <0PDK00NZ748T3610@st13p35im-asmtp002.me.com>; Thu, 16 Aug 2018 14:00:39 +0000 (GMT) X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 clxscore=1015 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000 definitions=main-1808160147 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-08-16_03:,, signatures=0 From: Toomas Soome Message-id: <4A015D53-1336-4E7F-B53E-28ACEBA3318A@me.com> MIME-version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\)) Subject: Re: svn commit: r337878 - head/stand/i386/libi386 Date: Thu, 16 Aug 2018 17:00:29 +0300 In-reply-to: Cc: John Baldwin , Ian Lepore , Toomas Soome , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org To: Warner Losh References: <201808152225.w7FMP5J2018006@repo.freebsd.org> <1534372134.1466.21.camel@freebsd.org> <05b36083-9f4d-658e-ab39-e8c317d01db7@FreeBSD.org> <18E7897D-9985-44CE-B648-2A839417B1BC@me.com> X-Mailer: Apple Mail (2.3445.9.1) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.27 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Aug 2018 14:00:52 -0000 > On 16 Aug 2018, at 16:03, Warner Losh wrote: >=20 >=20 >=20 > On Thu, Aug 16, 2018 at 1:10 AM, Toomas Soome > wrote: >=20 >=20 > > On 16 Aug 2018, at 09:59, John Baldwin 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 >> 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 = = > > >>>=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). >=20 > 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 > OK, Now I'm confused. All this sounds like regression, apart from the = >512 sector thing. >=20 > Warner Oh sorry, it was bad wording I think - the split up in sense that = currently the BIOS loader can not distinguish floppy from disk, so the = user has to guess if diskX is floppy or disk - so I want to make the = device list for user very clear (fdX:, cdX:, diskX: as its already done = in EFI version). The code should be consolidated (biosdisk + bioscd). rgds, toomas