Re: [PATCH #upstream, v2] ahci: Implement SATA AHCI FIS-based switching support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello, Shane.

Huang, Shane wrote:
>>>> +		/* time to wait for DEC is not specified by AHCI spec,
>>>> +		 * add a retry loop for safety */
>>>> +		do {
>>>> +			writel(fbs | PORT_FBS_DEC, port_mmio + 
>>> PORT_FBS);
>>>> +			fbs = readl(port_mmio + PORT_FBS);
>>>> +			retries--;
>>>> +		} while ((fbs & PORT_FBS_DEC) && retries);
> 
> 
> At the 2nd thought, if (!pp->fbs_enabled) appears unfortunately
> (although it should not), replacement of if (pp->fbs_enabled) with
> BUG_ON() will lead to the execution of the followed DEC,

Nope, it will result in the executing task being stopped with a scary
stackdump.

> which
> might lead to indeterminate result, because of AHCI v1.2 3.3.16:
> 
> Device Error Clear (DEC): ....... Software shall only set this bit to
> '1' if
> PxFBS.EN is set to '1' and PxFBS.SDE is set to '1'.
> 
> So I would suggest to keep my original implementation in v2, or put
> BUG_ON to the else case, if you insist, to replace the original
> dev_printk(KERN_ERR).

Problem with the original loop is that it writes PORT_FBS_DEC multiple
times.  Assume there are two consecutive device errors, the code might
end up clearing the second one unintentionally.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux