Re: [PATCH 1/2] ata: pata_artop: read the ATP865 mode register through pci_iomap

[Florian Fuchs <fuchsfl@gmail.com>]

Hi Daniel,

On 17 Oct 13:14, Daniel Palmer wrote:
> artop_init_one() probes the 6280's "fast" bit with inb() on the raw
> start address of BAR 4.  That address is a PCI I/O address, which is
> only a usable port number on platforms whose I/O port base maps PCI I/O
> space one to one.  On an SH7751 LANDISK (identity io_port_base, PCI I/O
> window at 0xfe240000) the resource starts at 0x1400 and inb(0x1400)
> dereferences that address:
> 
>   BUG: unable to handle kernel paging request at 00001400
>   PC is at artop_init_one+0x10e/0x170
> 
> Map the BAR with pci_iomap() and use ioread8(), as libata-sff does for
> the BMDMA registers later on.
> 
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>
> ---
>  drivers/ata/pata_artop.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/pata_artop.c b/drivers/ata/pata_artop.c
> index 9f8d41639a62..351ff4c8e48e 100644
> --- a/drivers/ata/pata_artop.c
> +++ b/drivers/ata/pata_artop.c
> @@ -405,13 +405,21 @@ static int artop_init_one (struct pci_dev *pdev, const struct pci_device_id *id)
>  	case 1:		/* 6260 */
>  		ppi[0] = &info_626x;
>  		break;
> -	case 2:		/* 6280 or 6280 + fast */
> -		if (inb(pci_resource_start(pdev, 4)) & 0x10)
> +	case 2: {	/* 6280 or 6280 + fast */
> +		void __iomem *bmdma = pci_iomap(pdev, 4, 1);
> +		u8 cfg;
> +
> +		if (!bmdma)
> +			return -ENOMEM;
> +		cfg = ioread8(bmdma);
> +		pci_iounmap(pdev, bmdma);
> +		if (cfg & 0x10)
>  			ppi[0] = &info_628x_fast;
>  		else
>  			ppi[0] = &info_628x;
>  		break;
>  	}
> +	}
>  
>  	BUG_ON(ppi[0] == NULL);
>  
> -- 
> 2.55.0
> 

There is a small logic change, where -ENOMEM is returned, when BAR 4 is
unassigned, where libata would fallback to PIO [0], so maybe it
shouldn't fail when BAR 4 is unassigned?

Tested-by: Florian Fuchs <fuchsfl@gmail.com> # on PX-EH40L (LANDISK HDL-U, ATP865 1191:0009 rev 10)

I also tested forced PIO4 mode via libata.force=pio4, which also works
on the device. (ata1.00: FORCE: xfer_mask set to pio4)

[0] https://github.com/torvalds/linux/blob/master/drivers/ata/libata-sff.c#L3088

Regards
Florian