From owner-svn-src-head@freebsd.org Thu Aug 16 14:28:41 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 3C8FF1067C25 for ; Thu, 16 Aug 2018 14:28:41 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-io0-x244.google.com (mail-io0-x244.google.com [IPv6:2607:f8b0:4001:c06::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id BB94E84F51 for ; Thu, 16 Aug 2018 14:28:40 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-io0-x244.google.com with SMTP id z19-v6so4016973ioh.4 for ; Thu, 16 Aug 2018 07:28:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=jWzBgGpO9DbC25LpzoN0zuA8eIiodIkjAYSyBdgrAjY=; b=kUErp9tMuwolBaoDVZPdeYN1xkI1Ea/qyviShBr02OlI+CPNJWgU6k9T/8GkBuXwWm AIeS0OhAoOo+Sno2A3LffzD6Wp3cb3qVwra4Xcm5Wpau9aSufpPVKsC5lxVZ/ZVyNVIy UGRp/+O2wD+nNoDEx8nFKwpkv+AduPx4FkmDn5Tq587YSEEohfyFbJl77shGf4/2fByd mxtGmcl3HgySqx/OWbUi/Rb5p3qHdrHcsAZtlITtSiZsFG2wNF/VhSLZM7OTejxBhbWf G/q06Oulz8ZXfI/DThVrYUBH0/3SjQijA0yzPZFTWtVi2NDekkdsT8ySeOaYNsoMuXeD sbaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=jWzBgGpO9DbC25LpzoN0zuA8eIiodIkjAYSyBdgrAjY=; b=hdk9QS4/n7Fmf0wc2DcSZrV4SEMwoCzswTqUpjSEso8HyEv6hRSa3GHQ+qAGm6QChb aaE8v8w3qjEBww7SsxCA06DALnAX6MLbtBTSOxWRGGJFnE/FP7FrB4m6WShFqmw8k7K+ WGyGoL7FZFU1JNLFRKui/gmugZt04n2MTrPad40ywKRcQ5N3jigbLCeioyDtF2C5Kir1 3B+91tJBwVA/7LwDDTZrrQOtCBXBQpbEWY3P6sAZACOiDnBlO23U82n6cwXNeRPiStvB Xa57YXolSQ7xoo/k3EpJSfPWIJr+/XJG71EUeFfL1mA+3mqmSLKKkKjiyDI5tHhFT0L2 jstQ== X-Gm-Message-State: AOUpUlGkWw678GCwwk+RULOjtDN/GAd72ILaDzpTBfXEgbCd2vFKT/XU gA38DSNovZmh4tZhxl7ZpZOvHobwl57Y8BbB0X1YYQ== X-Google-Smtp-Source: AA+uWPyXPwcb+j3wGbTYAUkWmLrIPsiYs//BBmj281fWOiDc0yBCQuXcPBjIpPi17e6uh3sQb81pg/J5a+KfyMq0XfY= X-Received: by 2002:a6b:d004:: with SMTP id x4-v6mr25460565ioa.299.1534429720002; Thu, 16 Aug 2018 07:28:40 -0700 (PDT) MIME-Version: 1.0 Sender: wlosh@bsdimp.com Received: by 2002:a4f:257:0:0:0:0:0 with HTTP; Thu, 16 Aug 2018 07:28:39 -0700 (PDT) X-Originating-IP: [2603:300b:6:5100:1052:acc7:f9de:2b6d] In-Reply-To: <4A015D53-1336-4E7F-B53E-28ACEBA3318A@me.com> 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> <4A015D53-1336-4E7F-B53E-28ACEBA3318A@me.com> From: Warner Losh Date: Thu, 16 Aug 2018 08:28:39 -0600 X-Google-Sender-Auth: -irifzsMUHjhEN5YlqxjAh06wpM Message-ID: Subject: Re: svn commit: r337878 - head/stand/i386/libi386 To: Toomas Soome Cc: John Baldwin , Ian Lepore , Toomas Soome , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" 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:28:41 -0000 On Thu, Aug 16, 2018 at 8:00 AM, Toomas Soome wrote: > > > On 16 Aug 2018, at 16:03, Warner Losh wrote: > > > > On Thu, Aug 16, 2018 at 1:10 AM, Toomas Soome wrote: > >> >> >> > On 16 Aug 2018, at 09:59, John Baldwin wrote: >> > >> > On 8/15/18 11:59 PM, Warner Losh wrote: >> >> >> >> >> >> On Wed, Aug 15, 2018 at 4:28 PM, Ian Lepore > ian@freebsd.org>> wrote: >> >> >> >> 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 > ps://svnweb.freebsd.org/changeset/base/337878> >> >>> >> >>> Log: >> >>> libi386: remove bd_read() and bd_write() wrappers >> >>> >> >>> Those wroappers are nice, but do not really add much value. >> >>> >> >>> Modified: >> >>> head/stand/i386/libi386/biosdisk.c >> >>> >> >>> Modified: head/stand/i386/libi386/biosdisk.c >> >>> ===================================================================== >> >>> ========= >> >>> --- 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 = 0; >> >>> >> >>> static void bd_io_workaround(struct disk_devdesc *dev); >> >>> >> >>> -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); >> >>> >> >>> 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); >> >>> >> >>> - if (blks && (rc = bd_read(dev, dblk, blks, buf))) { >> >>> + if (blks && (rc = bd_io(dev, dblk, blks, buf, 0))) { >> >>> /* Filter out floppy controller errors */ >> >>> if (BD(dev).bd_flags != BD_FLOPPY || rc != >> >>> 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); >> >>> >> >>> - 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 >> >>> } >> >>> >> >>> 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)); >> >>> } >> >>> >> >>> /* >> >>> >> >> >> >> This would be a more satisfying change if there were something like >> >> >> >> #define BD_RD 0 >> >> #define BD_WR 1 >> >> >> >> so that it was clear at a glance what a bd_io() call is doing. >> >> >> >> >> >> I think that's a good idea... >> > >> > 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. >> > > OK, Now I'm confused. All this sounds like regression, apart from the >512 > sector thing. > > 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). > OK. I have concerns, but be sure to put me on the code review so we can talk about it there when there's a concrete example to talk about. Warner