Skip site navigation (1)Skip section navigation (2) 
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>
index | next in thread | previous in thread | raw e-mail 



> On 16 Aug 2018, at 09:59, John Baldwin <jhb@FreeBSD.org> 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 <mailto: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 <https://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. 

rgds,
toomas
help

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?18E7897D-9985-44CE-B648-2A839417B1BC>