Re: [PATCH] floppy: remove unused function fdctrl_format_sector

[John Snow <jsnow@redhat.com>]

On 3/14/21 3:53 AM, Hervé Poussineau wrote:
> Le 12/03/2021 à 07:45, John Snow a écrit :
>> 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
> 
> The fdctrl_format_sector has been added in SVN revision 671 
> (baca51faff03df59386c95d9478ede18b5be5ec8), along with 
> FD_STATE_FORMAT/FD_FORMAT_CMD.
> As with current code, the only place where the FD_STATE_FORMAT flag was 
> 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.
> 
> I can however see the following comment:
>             /* Bochs BIOS is buggy and don't send format informations
>              * for each sector. So, pretend all's done right now...
>              */
>             fdctrl->data_state &= ~FD_STATE_FORMAT;
> 
> which was changed in SVN revision 2295 
> (b92090309e5ff7154e4c131438ee2d540e233955) to:
>             /* TODO: implement format using DMA expected by the Bochs BIOS
>              * and Linux fdformat (read 3 bytes per sector via DMA and fill
>              * the sector with the specified fill byte
>              */
> 
> This probably means that code may have worked without DMA (to be 
> confirmed), but was disabled since its introduction due to a problem 
> with Bochs BIOS.
> Later, fdformat was also tested and not working.
> 
> Since then, lots of work has also been done in DMA handling. I 
> especially think at bb8f32c0318cb6c6e13e09ec0f35e21eff246413, which 
> fixed a similar problem with floppy drives on IBM 40p machine.
> What happens when this flag unsetting is removed? Does fdformat now works?
> 
> I think that we should either fix the code, or remove more code 
> (everything related to fdctrl_format_sector, FD_STATE_FORMAT, 
> FD_FORMAT_CMD, maybe even fdctrl_handle_format_track).
> 
> Regards,
> 
> Hervé
> 

Alex, do you want to respin this following Hervé's suggestion for 
additional deletions?

I doubt anyone has the time or interest to actually FIX this code, so we 
may as well remove misleading code.

--js