Port-x68k archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: libsa/sdcd: correct blocksize

> The first one corrects a condition expression.
> - start is LBA but dblk is relative from this partition.
> - Numerical comparison makes it easy.
> There would have been no impact.

This one seems ok for me. Maybe the original one intended to generate
smaller code as asm code, but I guess nowadays compiler can handle it
better with -Os.

> The second one clarifies confusing definitions of sector size.
> There are three kind of sector size in this code.  DEV_BSIZE
> from xxstrategy, media's sector size, and Human68k's sector size.
> By this patch,
> - For 512 bytes/sector HDD, no changes are intended.
> - For CD, corrects the blocksize (%d5) passes to SCSIIOCS.
>   It has worked previously though the blocksize was incorrect.
>   Now it works with correct blocksize.
> - As a secondary effect, 256 or 1024 bytes/sector media may work but
>   not well tested.

I guess essencial part is "shift op" vs "log2 calculation" for blocksize.
The logic seems ok but there are several comments on implementation:

- maybe it's better to have function prototypes, even if they are static
- is it worth to explicitly specify static inline (or gcc already does?)
- I wonder if human2blk(), human2bsd(), and bsd2blk() should be unsigned
- if now cdstrategy() is identical with sdstrategy(), maybe it's better
  to rename current sdstrategy() to sdcdstrategy() and use it both
  sd and cd in devsw[] in stand/boot/conf.c?

Izumi Tsutsui

Home | Main Index | Thread Index | Old Index