Re: [Qemu-block] [Qemu-devel] [RFC 06/10] fdc: add physical disk sizes

[Markus Armbruster]

John Snow <address@hidden> writes:

> 2.88MB capable drives can accept 1.44MB floppies,
> for instance. To rework the pick_geometry function,
> we need to know if our current drive can even accept
> the type of disks we're considering.
>
> NB: This allows us to distinguish between all of the
> "total sectors" collisions between 1.20MB and 1.44MB
> diskette types.
>
> Signed-off-by: John Snow <address@hidden>
> ---
>  hw/block/fdc.c | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 4004b98..6e5f87b 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -59,6 +59,11 @@ typedef enum FDriveRate {
>      FDRIVE_RATE_1M   = 0x03,  /*   1 Mbps */
>  } FDriveRate;
>  
> +typedef enum FDriveSize {
> +    FDRIVE_SIZE_350,
> +    FDRIVE_SIZE_525
> +} FDriveSize;
> +
>  typedef struct FDFormat {
>      FDriveType drive;
>      uint8_t last_sect;
> @@ -95,15 +100,15 @@ static const FDFormat fd_formats[] = {
>      { FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, },
>      /* 1.2 MB 5"1/4 floppy disks */
>      { FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, },
> -    { FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, },
> +    { FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, }, /* conflicts w/ #0 */
>      { FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, },
>      { FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, },
> -    { FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, },
> +    { FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, }, /* conflicts w/ #1 */
>      /* 720 kB 5"1/4 floppy disks */
> -    { FDRIVE_DRV_120,  9, 80, 1, FDRIVE_RATE_250K, },
> +    { FDRIVE_DRV_120,  9, 80, 1, FDRIVE_RATE_250K, }, /* conflicts w/ #13 */
>      { FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, },
>      /* 360 kB 5"1/4 floppy disks */
> -    { FDRIVE_DRV_120,  9, 40, 1, FDRIVE_RATE_300K, },
> +    { FDRIVE_DRV_120,  9, 40, 1, FDRIVE_RATE_300K, }, /* conflicts w/ #32 */
>      { FDRIVE_DRV_120,  9, 40, 0, FDRIVE_RATE_300K, },
>      { FDRIVE_DRV_120, 10, 41, 1, FDRIVE_RATE_300K, },
>      { FDRIVE_DRV_120, 10, 42, 1, FDRIVE_RATE_300K, },

Can you elaborate on what "conflicts w/" means here?

> @@ -116,6 +121,20 @@ static const FDFormat fd_formats[] = {
>      { FDRIVE_DRV_NONE, -1, -1, 0, 0, },
>  };
>  
> +__attribute__((__unused__))
> +static FDriveSize drive_size(FDriveType drive)
> +{
> +    switch (drive) {
> +    case FDRIVE_DRV_120:
> +        return FDRIVE_SIZE_525;
> +    case FDRIVE_DRV_144:
> +    case FDRIVE_DRV_288:
> +        return FDRIVE_SIZE_350;
> +    default:
> +        return -1;

Works, but isn't so nice.  When I see a function returning an enum type,
I expect the return value to be one of its enum constants.

You can either return int, or add the error value to the enum type.

> +    }
> +}
> +
>  #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv)
>  #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive))