On 07.01.2013, at 18:42, Scott Wood wrote:
> On 01/04/2013 05:36:42 PM, Alexander Graf wrote:
>> @@ -667,6 +671,20 @@ static void openpic_gbl_write(void *opaque, hwaddr
>> addr, uint64_t val,
>> case 0x1020: /* GCR */
>> if (val & GCR_RESET) {
>> openpic_reset(&opp->busdev.qdev);
>> + } else if (opp->mpic_mode_mask) {
>> + CPUArchState *env;
>> + int mpic_proxy = 0;
>> +
>> + opp->gcr &= ~opp->mpic_mode_mask;
>> + opp->gcr |= val & opp->mpic_mode_mask;
>> +
>> + /* Set external proxy mode */
>> + if ((val & opp->mpic_mode_mask) == GCR_MODE_PROXY) {
>> + mpic_proxy = 1;
>> + }
>> + for (env = first_cpu; env != NULL; env = env->next_cpu) {
>> + env->mpic_proxy = mpic_proxy;
>> + }
>> }
>> break;
>> case 0x1080: /* VIR */
>
> Why is "if (opp->mpic_mode_mask)" needed? The code should already be a
> no-op if mpic_mode_mask is zero.
It is today, but it wouldn't be for pass-through mode which we don't implement
today. I wasn't sure whether raven implements pass-through / mixed mode, so I
wanted to maintain the status quo as much as possible.
> Can get rid of the "else" by having the reset code do a "break".
Yes. Not sure it's worth another patch though :).
>
>> @@ -1407,6 +1425,9 @@ static int openpic_init(SysBusDevice *dev)
>> opp->irq_tim0 = FSL_MPIC_20_TMR_IRQ;
>> opp->irq_msi = FSL_MPIC_20_MSI_IRQ;
>> opp->brr1 = FSL_BRR1_IPID | FSL_BRR1_IPMJ | FSL_BRR1_IPMN;
>> + /* XXX really only available as of MPIC 4.0 */
>> + opp->mpic_mode_mask = GCR_MODE_PROXY;
>> +
>
> Shouldn't mpic_mode_mask be set to GCR_MODE_MIXED for Raven?
Does Raven support the MIXED bit? Do you have a pointer to the spec?
Alex
