Re: [PATCH] pata_parport: add driver (PARIDE replacement)

[Jens Axboe]

On 3/13/22 1:15 PM, Ondrej Zary wrote:
> On Saturday 12 March 2022 15:44:15 Ondrej Zary wrote:
>> The pata_parport is a libata-based replacement of the old PARIDE
>> subsystem - driver for parallel port IDE devices.
>> It uses the original paride low-level protocol drivers but does not
>> need the high-level drivers (pd, pcd, pf, pt, pg). The IDE devices
>> behind parallel port adapters are handled by the ATA layer.
>>
>> This will allow paride and its high-level drivers to be removed.
>>
>> paride and pata_parport are mutually exclusive because the compiled
>> protocol drivers are incompatible.
>>
>> Tested with Imation SuperDisk LS-120 and HP C4381A (both use EPAT
>> chip).
>>
>> Note: EPP-32 mode is buggy in EPAT - and also in all other protocol
>> drivers - they don't handle non-multiple-of-4 block transfers
>> correctly. This causes problems with LS-120 drive.
>> There is also another bug in EPAT: EPP modes don't work unless a 4-bit
>> or 8-bit mode is used first (probably some initialization missing?).
>> Once the device is initialized, EPP works until power cycle.
>>
>> So after device power on, you have to:
>> echo "parport0 epat 0" >/sys/bus/pata_parport/new_device
>> echo pata_parport.0 >/sys/bus/pata_parport/delete_device
>> echo "parport0 epat 4" >/sys/bus/pata_parport/new_device
>> (autoprobe will initialize correctly as it tries the slowest modes
>> first but you'll get the broken EPP-32 mode)
> 
> Found a bug - the same device can be registered multiple times. Fix
> will be in v2. But this revealed a bigger problem: pi_connect can
> sleep (uses parport_claim_or_block) and libata does not like that. Any
> ideas how to fix this?

I think you'd need two things here:

- The blk-mq queue should be registered with BLK_MQ_F_BLOCKING, which
  will allow blocking off the queue_rq path.

- You need to look at making libata safe wrt calling ata_qc_issue()
  outside the lock. Should probably be fine if you just gate that on
  whether or not the queue was setup in blocking mode, as that doesn't
  currently exist in libata.

-- 
Jens Axboe