Re: [PATCH] ahci.c: fix ati sb600 sata IRQ_TF_ERR

Tejun Heo
2007-03-15T12:14:22Z

Conke Hu wrote:
>> E Internal error: The host bus adapter experienced an internal error
>> that caused the operation to fail and may have put the host bus adapter
>> into an error state. Host software should reset the interface before
>> re-trying the operation. If the condition persists, the host bus adapter
>> may suffer from a design issue rendering it incompatible with the
>> attached device.
>>
>
> Yes, I saw this too :) and I am contacting the hardware engineers to
> check if there is any hardware bug.
> But, even though this were a hardware bug and could be fixed, we would
> still need this patch since many SB600 boards have already come into
> the market and those ASICs can never be fixed :(

Yeap, we certainly need the workaround. I was just having a little fun.
:-)

>> 4381 isn't affected while 4380 is?
>
> I never see such an ID, and plan to remove 0x4381.
> The patch which added the PCI IDs was not sent out by myself. I
> checked all SB600 boards, and not found any 0x4381 controller, only
> 0x4380 instead. In fact, SB600 RAID and Non-RAID share the same PCI
> device ID, only with class code different.

I see.

>> Anyways, Conke Hu, can you please take a look at my patch from a month
>> ago? It's almost identical but SERR_INTERNAL is always ignored on both
>> SB600 PCI IDs, which I think is safer. Does this fix what you're seeing?
>>
>
> I just read your patch. Another difference is that my patch ignores
> SERR_INTERNAL only when the command is ATAPI and IRQ_TF_ERR occurs. In
> other cases, I think, we'd better not ignore the SERR_INTERNEL. Right?

Yeah, I noticed the difference. I don't really care but I was thinking
that SERR_INTERNAL might be set in other similar situations too. e.g.
TF error from ATA device or what not, so I thought it would be safer to
ignore the bit altogether. You probably need to consult your hardware
people about when exactly the bit misbehaves but unless proven
otherwise, I'd prefer to always ignore the bit. Also, please rename the
enum constant and flag name.

Thanks.

--
tejun