Re: [PATCH] floppy: remove unused function fdctrl_format_sector

[John Snow]

On 1/8/21 6:01 PM, Alexander Bulekov wrote:
fdctrl_format_sector was added in
baca51faff ("updated floppy driver: formatting code, disk geometry auto detect 
(Jocelyn Mayer)")
The single callsite is guarded by a check:
fdctrl->data_state & FD_STATE_FORMAT

However, the only place where the FD_STATE_FORMAT flag is set (in
fdctrl_handle_format_track) is closely followed by the same flag being
unset, with no possibility to call fdctrl_format_sector in between.


Hm, was this code *ever* used? It's hard to tell when we go back into 
the old SVN history.

Does this mean that fdctrl_handle_format_track is also basically an 
incomplete stub method?

I'm in favor of deleting bitrotted code, but I wonder if we should take 
a bigger bite.

--js

This removes fdctrl_format_sector and the unncessary setting/unsetting
of the FD_STATE_FORMAT flag.

Signed-off-by: Alexander Bulekov <alx...@bu.edu>
---
  hw/block/fdc.c | 68 --------------------------------------------------
  1 file changed, 68 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 3636874432..837dd819ea 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1952,67 +1952,6 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
      return retval;
  }
  
-static void fdctrl_format_sector(FDCtrl *fdctrl)
-{
-    FDrive *cur_drv;
-    uint8_t kh, kt, ks;
-
-    SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK);
-    cur_drv = get_cur_drv(fdctrl);
-    kt = fdctrl->fifo[6];
-    kh = fdctrl->fifo[7];
-    ks = fdctrl->fifo[8];
-    FLOPPY_DPRINTF("format sector at %d %d %02x %02x (%d)\n",
-                   GET_CUR_DRV(fdctrl), kh, kt, ks,
-                   fd_sector_calc(kh, kt, ks, cur_drv->last_sect,
-                                  NUM_SIDES(cur_drv)));
-    switch (fd_seek(cur_drv, kh, kt, ks, fdctrl->config & FD_CONFIG_EIS)) {
-    case 2:
-        /* sect too big */
-        fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00);
-        fdctrl->fifo[3] = kt;
-        fdctrl->fifo[4] = kh;
-        fdctrl->fifo[5] = ks;
-        return;
-    case 3:
-        /* track too big */
-        fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_EC, 0x00);
-        fdctrl->fifo[3] = kt;
-        fdctrl->fifo[4] = kh;
-        fdctrl->fifo[5] = ks;
-        return;
-    case 4:
-        /* No seek enabled */
-        fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00);
-        fdctrl->fifo[3] = kt;
-        fdctrl->fifo[4] = kh;
-        fdctrl->fifo[5] = ks;
-        return;
-    case 1:
-        fdctrl->status0 |= FD_SR0_SEEK;
-        break;
-    default:
-        break;
-    }
-    memset(fdctrl->fifo, 0, FD_SECTOR_LEN);
-    if (cur_drv->blk == NULL ||
-        blk_pwrite(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
-                   BDRV_SECTOR_SIZE, 0) < 0) {
-        FLOPPY_DPRINTF("error formatting sector %d\n", fd_sector(cur_drv));
-        fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00, 0x00);
-    } else {
-        if (cur_drv->sect == cur_drv->last_sect) {
-            fdctrl->data_state &= ~FD_STATE_FORMAT;
-            /* Last sector done */
-            fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
-        } else {
-            /* More to do */
-            fdctrl->data_pos = 0;
-            fdctrl->data_len = 4;
-        }
-    }
-}
-
  static void fdctrl_handle_lock(FDCtrl *fdctrl, int direction)
  {
      fdctrl->lock = (fdctrl->fifo[0] & 0x80) ? 1 : 0;
@@ -2126,7 +2065,6 @@ static void fdctrl_handle_format_track(FDCtrl *fdctrl, 
int direction)
  
      SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK);
      cur_drv = get_cur_drv(fdctrl);
-    fdctrl->data_state |= FD_STATE_FORMAT;
      if (fdctrl->fifo[0] & 0x80)
          fdctrl->data_state |= FD_STATE_MULTI;
      else
@@ -2144,7 +2082,6 @@ static void fdctrl_handle_format_track(FDCtrl *fdctrl, 
int direction)
       * and Linux fdformat (read 3 bytes per sector via DMA and fill
       * the sector with the specified fill byte
       */
-    fdctrl->data_state &= ~FD_STATE_FORMAT;
      fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
  }
  
@@ -2458,11 +2395,6 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
              /* We have all parameters now, execute the command */
              fdctrl->phase = FD_PHASE_EXECUTION;
  
-            if (fdctrl->data_state & FD_STATE_FORMAT) {
-                fdctrl_format_sector(fdctrl);
-                break;
-            }
-
              cmd = get_command(fdctrl->fifo[0]);
              FLOPPY_DPRINTF("Calling handler for '%s'\n", cmd->name);
              cmd->handler(fdctrl, cmd->direction);