Re: [PATCH RFC] ata: Intel IDE-R support

[Sergei Shtylyov <sshtylyov@mvista.com>]

Hello.

Alan Cox wrote:

> Intel IDE-R devices are part of the Intel AMT management setup. They don't
> have any special configuration registers or settings so the ata_generic
> driver will support them fully.

> Rather than add a huge table of IDs for each chipset and keep sending in
> new ones this patch autodetects them.

> (And yes Jeff I'll resurrect the delay patches in a couple of weeks)

    Doesn't have to be in the changelog...

> Signed-off-by: Alan Cox <alan@linux.intel.com>

[...]

> diff --git a/drivers/ata/ata_generic.c b/drivers/ata/ata_generic.c
> index cc5f772..476f194 100644
> --- a/drivers/ata/ata_generic.c
> +++ b/drivers/ata/ata_generic.c
[...]
> @@ -109,6 +110,44 @@ static struct ata_port_operations generic_port_ops = {
>  static int all_generic_ide;		/* Set to claim all devices */
>  
>  /**
> + *	is_intel_ider		-	identify intel IDE-R devices
> + *	@dev: PCI device
> + *
> + *	Distinguish Intel IDE-R controller devices from other Intel IDE
> + *	devices. IDE-R devices have no timing registers and are in
> + *	most respects virtual. They should be driven by the ata_generic
> + *	driver.
> + */
> +
> +static int is_intel_ider(struct pci_dev *dev)
> +{
> +	/* For Intel IDE the value at 0xF8 is only zero on IDE-R
> +	   interfaces */

    Preferred style for multi-line comments is:

/*
  * bla
  * bla
  */

> +	u32 r;
> +	u16 t;
> +
> +	pci_read_config_dword(dev, 0xF8, &r);
> +	/* Not IDE-R: punt so that ata_(old)piix gets it */
> +	if (r != 0)
> +		return 0;
> +	/* 0xF8 is also be zero on some early Intel IDE devices

    Grammar.

> +	   but they will have a sane timing register */

    The same remark about the comment style...

> +	pci_read_config_word(dev, 0x40, &t);
> +	if (t != 0)
> +		return 0;
> +	/* Finally check if the timing register is writable so that
> +	   we eliminate any early devices hot-docked in a docking
> +	   station */

    ... and here too.

> @@ -134,6 +173,10 @@ static int ata_generic_init_one(struct pci_dev *dev, const struct pci_device_id
>  	if ((id->driver_data & ATA_GEN_CLASS_MATCH) && all_generic_ide == 0)
>  		return -ENODEV;
>  
> +	if (id->driver_data && ATA_GEN_INTEL_IDER)

    I think you mean & here, rather than &&...

MBR, Sergei