diff mbox

[1/3] m68k: implmenent more ColdFire 5208 interrupt controller functionality

Message ID 1408426627-12071-2-git-send-email-gerg@uclinux.org
State New
Headers show

Commit Message

Greg Ungerer Aug. 19, 2014, 5:37 a.m. UTC
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(+)

Comments

Peter Crosthwaite June 19, 2015, 5:24 a.m. UTC | #1
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>

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.

Regards,
Peter

> +        } 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
>
>
Greg Ungerer June 19, 2015, 6:04 a.m. UTC | #2
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
>>
>>
>
diff mbox

Patch

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;
+        } 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;