Re: [MiNT] [PATCH] Make EHCI driver interrupt handler follow PCI-BIOS specification.

[David Gálvez <dgalvez75@gmail.com>]

2014-10-08 17:56 GMT+02:00 Alan Hourihane <alanh@fairlite.co.uk>:
>
> On 08/10/14 16:31, David Gálvez wrote:
>> 2014-10-08 14:08 GMT+02:00 Vincent Rivière <vincent.riviere@freesbee.fr>:
>>> On 08/10/2014 11:44, David Gálvez wrote:
>>>> +       asm volatile("move.l d1,-(SP)\n\t" : : : );
>>>
>>> I'm really not sure that this block does what you think (and I'm pretty sure
>>> it does not).
>>> By doing so, you modify SP, but GCC does not know that. Afterwards, it may
>>> access local variables or parameters through SP. If you change SP, those
>>> references will be wrong.
>>>
>>> The only safe way to preserve registers in a callback is to implement it as
>>> a full assembler stub (in a .S file). So it can save/restore the registers,
>>> get the parameters and call a standard C function containing the main
>>> callback code.
>>>
>>
>> Thanks Vincent for taking a look to the patch and also for the advice,
>> I'll review it.
>> The code has been tested and it's working but I understand that could
>> be just luck.
>>
>>
>
> I think you're just being lucky, because you restore the registers later.
>
> But essentially, I think this step is a no-op, because GCC will save/restore
> it's accessed registers anyway.
>
> Also, you save/restore D1/A1 but the comment says D0/A0.
>

What I mean with this comment is that PCI-BIOS expects A0 and D0 to be
modified in the interrupt handler so we don't need to backup them. A1
and D1 if I'm not wrong are used by GCC as scratch registers (together
with D0 and A0), so we only need to backup D1 and A1.