Re: [PATCH 03/17] libata-link: introduce ata_link

[Jeff Garzik <jgarzik@pobox.com>]

Tejun Heo wrote:
> @@ -496,6 +496,23 @@ struct ata_eh_context {
>  	unsigned int		did_probe_mask;
>  };
>  
> +struct ata_link {
> +	struct ata_port		*ap;
> +
> +	unsigned int		active_tag;	/* active tag on this link */
> +	u32			sactive;	/* active NCQ commands */
> +
> +	unsigned int		hw_sata_spd_limit;
> +	unsigned int		sata_spd_limit;
> +
> +	/* record runtime error info, protected by host_set lock */
> +	struct ata_eh_info	eh_info;
> +	/* EH context */
> +	struct ata_eh_context	eh_context;
> +
> +	struct ata_device	device[1];
> +};
> +
>  struct ata_port {
>  	struct Scsi_Host	*host;	/* our co-allocated scsi host */
>  	const struct ata_port_operations *ops;
> @@ -520,22 +537,13 @@ struct ata_port {
>  	unsigned int		mwdma_mask;
>  	unsigned int		udma_mask;
>  	unsigned int		cbl;	/* cable type; ATA_CBL_xxx */
> -	unsigned int		hw_sata_spd_limit;
> -	unsigned int		sata_spd_limit;	/* SATA PHY speed limit */
> -
> -	/* record runtime error info, protected by host_set lock */
> -	struct ata_eh_info	eh_info;
> -	/* EH context owned by EH */
> -	struct ata_eh_context	eh_context;
> -
> -	struct ata_device	device[ATA_MAX_DEVICES];
>  
>  	struct ata_queued_cmd	qcmd[ATA_MAX_QUEUE];
>  	unsigned long		qc_allocated;
>  	unsigned int		qc_active;
>  
> -	unsigned int		active_tag;
> -	u32			sactive;
> +	struct ata_link		link;	/* host default link */
> +	struct ata_device	__dev1;	/* storage for link.device[1] */

This storage mess for link.device[] is NAK'd.

This patch overall looks good, but it should really be entirely an 
equivalent-transformation patch, one that is easy to review and prove 
correct.

As such, this patch should move 'struct ata_device 
device[ATA_MAX_DEVICES]' into struct ata_link, rather than the strange 
arrangement you have above.  If you want to do something unusual like 
this, it's better to do it in a separate patch.

	Jeff