On 3/15/22 05:29, Jens Axboe wrote: > On 3/14/22 2:25 PM, Ondrej Zary wrote: >> On Monday 14 March 2022 00:19:30 Jens Axboe wrote: >>> 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. >> >> My knowledge about blk-mq is exactly zero. After grepping the code, I >> guess that BLK_MQ_F_BLOCKING should be used by the block device >> drivers - sd and sr? > > The controller would set > > ->needs_blocking_queue_rq = true; > > or something, and we'd default to false. And if that is set, when the > blk-mq queue is created, then we'd set BLK_MQ_F_BLOCKING upon creation > if that flag is true. > > That's the block layer side. Then in libata you'd need to ensure that > you check that same setting and invoke ata_qc_issue() appropriately. > > Very top level stuff, there might be more things lurking below. But > you'll probably find them as you test this stuff... Yes, the ata_port spinlock being held when calling ata_qc_issue() is mandatory. But since I am assuming that all the IDE devices connected to this adapter are QD=1 maximum, there can only be only one command in flight. So it may be OK to release that lock before calling pi_connect() and retake it right after it. libsas actually does something similar (for no good reasons in that case though). Jens point remain though that since pi_connect() can sleep, marking the device queue with BLK_MQ_F_BLOCKING is mandatory. -- Damien Le Moal Western Digital Research
