Re: [Qemu-devel] [PATCH 1/3] m68k: implmenent more ColdFire 5208 interrupt controller functionality

[Greg Ungerer <gerg@uclinux.org>]

Hi Peter,

On 19/06/15 15:24, Peter Crosthwaite wrote:
> On Mon, Aug 18, 2014 at 10:37 PM,  <gerg@uclinux.org> wrote:
>> From: Greg Ungerer <gerg@uclinux.org>
>>
>> Implement the SIMR and CIMR registers of the 5208 interrupt controller.
>> These are used by modern versions of Linux running on ColdFire (not sure
>> of the exact version they were introduced, but they have been in for quite
>> a while now).
>>
>> Without this change when attempting to run a linux-3.5 kernel you will
>> see:
>>
>>   qemu: hardware error: mcf_intc_write: Bad write offset 28
>>
>> and execution will stop and dump out.
>>
>> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
>> ---
>>  hw/m68k/mcf_intc.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/hw/m68k/mcf_intc.c b/hw/m68k/mcf_intc.c
>> index 621423c..37a9de0 100644
>> --- a/hw/m68k/mcf_intc.c
>> +++ b/hw/m68k/mcf_intc.c
>> @@ -102,6 +102,20 @@ static void mcf_intc_write(void *opaque, hwaddr addr,
>>      case 0x0c:
>>          s->imr = (s->imr & 0xffffffff00000000ull) | (uint32_t)val;
>>          break;
>> +    case 0x1c:
>> +        if (val & 0x40) {
>> +            s->imr = 0xffffffffffffffffull;
> 
> ~0ull.
>
> Otherwise,
> 
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Thanks, I'll change that and add the reviewed-by.


> This introduces magic numbers which is generally discouraged, by this
> device has no macrofication at all so I guess it should be cleaned up
> at some stage.

Agreed. I stuck to the existing style in this case.

Regards
Greg


>> +        } else {
>> +            s->imr |= (0x1ull << (val & 0x3f));
>> +        }
>> +        break;
>> +    case 0x1d:
>> +        if (val & 0x40) {
>> +            s->imr = 0ull;
>> +        } else {
>> +            s->imr &= ~(0x1ull << (val & 0x3f));
>> +        }
>> +        break;
>>      default:
>>          hw_error("mcf_intc_write: Bad write offset %d\n", offset);
>>          break;
>> --
>> 1.9.1
>>
>>
>