Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Ondrej Zary <linux@zary.sk>
To:     Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH] pata_parport: add driver (PARIDE replacement)
Date:   Mon, 14 Mar 2022 21:25:40 +0100
User-Agent: KMail/1.9.10
Cc:     Damien Le Moal <damien.lemoal@opensource.wdc.com>,
        Christoph Hellwig <hch@lst.de>, Tim Waugh <tim@cyberelk.net>,
        linux-block@vger.kernel.org, linux-parport@lists.infradead.org,
        linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220312144415.20010-1-linux@zary.sk> <202203132015.18183.linux@zary.sk> <5161ed17-5f55-e851-c2e2-5340cc62fa3b@kernel.dk>
In-Reply-To: <5161ed17-5f55-e851-c2e2-5340cc62fa3b@kernel.dk>
MIME-Version: 1.0
Content-Type: Text/Plain;
  charset="utf-8"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Message-Id: <202203142125.40532.linux@zary.sk>
Precedence: bulk

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?
 
> - 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.
> 


-- 
Ondrej Zary