Re: [patch 2/2] libata: _SDD support

[Pavel Machek <pavel@ucw.cz>]

Hi!

> _SDD (Set Device Data) is an ACPI method that is used to tell the 
> firmware what the identify data is of the device that is attached to
> the port.  It is an optional method, and it's ok for it to be missing. 
> Because of this, we always return success from the routine that calls
> this method, even if the execution fails.
> 
> Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> 
> +	if (noacpi)
> +		return 0;

That variable needs better name, too.

> +	/* Don't continue if not a SATA device. */
> +	if (!(ap->cbl == ATA_CBL_SATA)) {

Can we just use != ?

> +	err = ACPI_FAILURE(status) ? -EIO : 0;
> +	if (err < 0) {
> +		if (ata_msg_probe(ap))
> +			ata_dev_printk(atadev, KERN_DEBUG,
> +				"ata%u(%u): %s _SDD error: status = 0x%x\n",
> +				ap->id, ap->device->devno,
> +				__FUNCTION__, status);
> +	}

err is unused, you always return 0; can we do if (ACPI_FAILURE()) instead?

> +	/* always return success */
> +out:
> +	return 0;
> +}
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html