Re: [Qemu-ppc] [PATCH v2] openpic: Added BRR1 register

[Bhushan Bharat-R65777]

> -----Original Message-----
> From: Alexander Graf [mailto:address@hidden
> Sent: Tuesday, July 17, 2012 3:01 PM
> To: Bhushan Bharat-R65777
> Cc: address@hidden; address@hidden
> Subject: Re: [PATCH v2] openpic: Added BRR1 register
> 
> 
> On 17.07.2012, at 11:27, Bhushan Bharat-R65777 wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: Alexander Graf [mailto:address@hidden
> >> Sent: Tuesday, July 17, 2012 2:48 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: address@hidden; address@hidden
> >> Subject: Re: [PATCH v2] openpic: Added BRR1 register
> >>
> >>
> >> On 17.07.2012, at 11:10, Bhushan Bharat-R65777 wrote:
> >>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Alexander Graf [mailto:address@hidden
> >>>> Sent: Tuesday, July 17, 2012 2:06 PM
> >>>> To: Bhushan Bharat-R65777
> >>>> Cc: address@hidden; address@hidden; Bhushan
> >>>> Bharat-R65777
> >>>> Subject: Re: [PATCH v2] openpic: Added BRR1 register
> >>>>
> >>>>
> >>>> On 17.07.2012, at 08:30, Bharat Bhushan wrote:
> >>>>
> >>>>> Linux mpic driver uses (changes may be in pipeline to get
> >>>>> upstreamed
> >>>>> soon) BRR1. This patch adds the support to emulate readonly BRR1.
> >>>>>
> >>>>> Currently QEMU does not fully emulate any version on MPIC, so the
> >>>>> MPIC Major number and Minor number are set to 0.
> >>>>
> >>>> Hrm, I can't seem to find any mentioning of this register in the
> >>>> CPC945 spec for example.
> >>>
> >>> This means that BRR1 register is FSL specific. Any suggestion on how
> >>> to add
> >> FSL specific in openpic?
> >>
> >> I'd say for now just declare it as such in the comment. We seriously
> >> need to refactor the whole code to adapt to different MPIC variants.
> >
> > Different MPIC variants or different OPENPIC variants?
> 
> Both :)

:)

> 
> >
> >>
> >>>
> >>>>
> >>>>>
> >>>>> Signed-off-by: Bharat Bhushan <address@hidden>
> >>>>> ---
> >>>>> hw/openpic.c |   16 ++++++++++++++++
> >>>>> 1 files changed, 16 insertions(+), 0 deletions(-)
> >>>>>
> >>>>> diff --git a/hw/openpic.c b/hw/openpic.c index 58ef871..aad2ee9
> >>>>> 100644
> >>>>> --- a/hw/openpic.c
> >>>>> +++ b/hw/openpic.c
> >>>>> @@ -130,6 +130,16 @@ enum {
> >>>>> #define MPIC_CPU_REG_START        0x20000
> >>>>> #define MPIC_CPU_REG_SIZE         0x100 + ((MAX_CPU - 1) * 0x1000)
> >>>>>
> >>>>> +/*
> >>>>> + * Block Revision Register1 (BRR1): QEMU does not fully emulate
> >>>>> + * any version on MPIC. So to start with, set the IP version to 0.
> >>>>> + */
> >>>>> +#define BRR1_IPID 0x00400000 /* IP-block ID */
> >>>>
> >>>> Does this mean "FSL"?
> >>>
> >>> Yes, the value is FSL specific.
> >>
> >> Well, IP-block ID sounds like it's trying to tell me something. I
> >> couldn't find the semantics of what 0x400000 means. I assume it means
> >> "Freescale", which would indicate that the register isn't FSL specific. 
> >> But I
> couldn't find it anywhere.
> >
> > IP-Block ID: it is 16 bit numeric values assigned to Hardware IPs in FSL. A
> specific ID means a specific IP (0x0040 - MPIC).
> > Probably I could have better defined this is (0x0040 << 16) with a comment.
> > 8 bit is IP major number, 8 bit IP minor number. So this makes a 32 bit BRR1
> register.
> 
> Ah, ok. Then it does seem pretty FSL specific, right.

Yes.

-Bharat

> 
> > So for now adding a comment that BRR1 is FSL specific register and leave the
> value as is ok?
> 
> Yup.
> 
> 
> Alex
>