On Fri, Nov 29, 2013 at 5:41 AM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 28 November 2013 03:31, Peter Crosthwaite
> <peter.crosthwa...@xilinx.com> wrote:
>> GIC_BASE_ADDR is not the base address of the GIC. Its clear from the
>> code that this is the base address of the MPCore. Rename to
>> MPCORE_PERIPHBASE accordingly.
>
> "MPCore" is one of those terms I dislike because it
> doesn't actually match up with the documentation's
> terminology...
>
What entity ultimately owns PERIPHBASE and whats its symbolic name? I
felt like PERIPHBASE on its own was wrong. GIC_BASE_ADDR seems very
wrong though. Open to suggestion.
>> Signed-off-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
>> ---
>>
>> hw/arm/highbank.c | 15 ++++++++-------
>> 1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
>> index cb32325..be2cc20 100644
>> --- a/hw/arm/highbank.c
>> +++ b/hw/arm/highbank.c
>> @@ -28,11 +28,11 @@
>> #include "exec/address-spaces.h"
>> #include "qemu/error-report.h"
>>
>> -#define SMP_BOOT_ADDR 0x100
>> -#define SMP_BOOT_REG 0x40
>> -#define GIC_BASE_ADDR 0xfff10000
>> +#define SMP_BOOT_ADDR 0x100
>> +#define SMP_BOOT_REG 0x40
>> +#define MPCORE_PERIPHBASE 0xfff10000
>>
>> -#define NIRQ_GIC 160
>> +#define NIRQ_GIC 160
>>
>> /* Board init. */
>>
>> @@ -55,7 +55,7 @@ static void hb_write_secondary(ARMCPU *cpu, const struct
>> arm_boot_info *info)
>> 0xe1110001, /* tst r1, r1 */
>> 0x0afffffb, /* beq <wfi> */
>> 0xe12fff11, /* bx r1 */
>> - GIC_BASE_ADDR /* privbase: gic address. */
>> + MPCORE_PERIPHBASE /* privbase: gic address. */
>
> Comment no longer matches code.
>
Will fix.
Regards,
Peter
> More seriously, this secondary-boot code has a bunch
> of hardcoded offsets for GIC registers which are
> correct for the A9 but wrong for A15. Andre, did you
> test Midway with an SMP boot config?
>
> The generic hw/arm/boot code dealt with this problem
> by feeding the boot code the GIC CPU interface base
> address rather than the generic peripheral base address;
> the offsets within the CPU i/f are the same for both
> cores.
>
>> };
>> for (n = 0; n < ARRAY_SIZE(smpboot); n++) {
>> smpboot[n] = tswap32(smpboot[n]);
>> @@ -236,7 +236,8 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum
>> cxmachines machine)
>>
>> cpu = ARM_CPU(object_new(object_class_get_name(oc)));
>>
>> - object_property_set_int(OBJECT(cpu), GIC_BASE_ADDR, "reset-cbar",
>> &err);
>> + object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE,
>> "reset-cbar",
>> + &err);
>> if (err) {
>> error_report("%s", error_get_pretty(err));
>> exit(1);
>> @@ -287,7 +288,7 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum
>> cxmachines machine)
>> qdev_prop_set_uint32(dev, "num-irq", NIRQ_GIC);
>> qdev_init_nofail(dev);
>> busdev = SYS_BUS_DEVICE(dev);
>> - sysbus_mmio_map(busdev, 0, GIC_BASE_ADDR);
>> + sysbus_mmio_map(busdev, 0, MPCORE_PERIPHBASE);
>> for (n = 0; n < smp_cpus; n++) {
>> sysbus_connect_irq(busdev, n, cpu_irq[n]);
>> }
>> --
>> 1.8.4.4
>>
>
> thanks
> -- PMM
>
