Re: [PATCH v2 2/2] ata: pata_cswarp: Add Amiga cslab ata support

[Damien Le Moal <dlemoal@kernel.org>]

On 2025/03/24 5:24, Paolo Pisati wrote:
> Driver for the on-board IDE interface on CS-Lab Warp Expansion Card
> (NOTE that idemode=native has to be set in Warp Configuration)
> 
> Signed-off-by: Paolo Pisati <p.pisati@gmail.com>

2 nits below.

[...]

> +static int pata_cswarp_probe(struct zorro_dev *z,
> +			     const struct zorro_device_id *ent)
> +{
> +	static const char board_name[] = "csWarp";
> +	struct ata_host *host;
> +	struct ata_port *ap;
> +	void __iomem *cswarp_ctrl_board, *base;
> +	unsigned long board = z->resource.start;
> +
> +	dev_info(&z->dev, "%s IDE controller (board: 0x%lx)\n", board_name,
> +		 board);
> +
> +	if (!devm_request_mem_region(&z->dev, board + WARP_OFFSET_ATA, 0x1800,
> +				     DRV_NAME))
> +		return -ENXIO;
> +
> +	/* allocate host */

Not a very useful comment :)
You can drop this as the function name is obvious.

> +	host = ata_host_alloc(&z->dev, 1);
> +	if (!host)
> +		return -ENXIO;
> +
> +	cswarp_ctrl_board = (void *)board;
> +
> +	ap = host->ports[0];
> +	base = cswarp_ctrl_board + WARP_OFFSET_ATA;
> +
> +	ap->ops = &pata_cswarp_ops;
> +
> +	ap->pio_mask = ATA_PIO4;
> +	ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY |
> +		ATA_FLAG_PIO_POLLING;
> +
> +	ap->ioaddr.data_addr		= base;
> +	ap->ioaddr.error_addr		= base + 1 * 4;
> +	ap->ioaddr.feature_addr		= base + 1 * 4;
> +	ap->ioaddr.nsect_addr		= base + 2 * 4;
> +	ap->ioaddr.lbal_addr		= base + 3 * 4;
> +	ap->ioaddr.lbam_addr		= base + 4 * 4;
> +	ap->ioaddr.lbah_addr		= base + 5 * 4;
> +	ap->ioaddr.device_addr		= base + 6 * 4;
> +	ap->ioaddr.status_addr		= base + 7 * 4;
> +	ap->ioaddr.command_addr		= base + 7 * 4;
> +
> +	ap->ioaddr.altstatus_addr	= base + (0x1000 | (6UL << 2));
> +	ap->ioaddr.ctl_addr		= base + (0x1000 | (6UL << 2));
> +
> +	ata_port_desc(ap, "  cmd 0x%lx ctl 0x%lx", (unsigned long)base,
> +		      (unsigned long)ap->ioaddr.ctl_addr);
> +
> +	ata_host_activate(host, 0, NULL,
> +			  IRQF_SHARED, &pata_cswarp_sht);

	return ata_host_activate(host, 0, NULL,
				 IRQF_SHARED, &pata_cswarp_sht);

so that errors are properly reported.

> +
> +	return 0;
> +}
> +


-- 
Damien Le Moal
Western Digital Research