EPIC Vs OpenPIC Vs MontaVista

EPIC Vs OpenPIC Vs MontaVista

[Sarnath Kannan]

Guys,
  I ran into some accidental discoveries
while scanning EPIC code written by Mvista.
  As I said b4 there is a descrepancy with
the OpenPIC register layout and EPIC layout.
But the way Mvista people have programmed EPIC is
good in one sense, bad in other.
  The reason for assigning vector of 16 to PCI
interrupts is because each entry in the "Interrupt Source" is 32 bytes long, 16 * 32 = 512 = 0x200
which equals the difference in offsets between
the std openPIC layout and EPIC register layout.
This 16 has got NOTHING TO DO with NUM_8259_INTERRUPTS.
But Mvista code seems to assume that this feature
is because of NUM_8259_INTERRUPTS. ( See the
#define for SANDPOINT_SIO_IRQ ).
Thus while writing into  Sources[16], mvista code
writes into "sources[0]" in the EPIC reigster layout.
Thus IRQ0 gets initialized to vector 16.
 The "offset" field in "openpic_init" code,
is mainly used to seperate of vector spaces of
various interrupt controllers in the system.
  EPIC manual clearly says that the "Interrupt
Sources" register offset start at 0x50200. mvista
code overwrites the 0x50000-0x50200 fields ( which are
actually reserved as per EPIC manual ). Thus the
reliablity of MVISTA code is a question mark .
I have come across some "Bogus interrupts" while
runing MVISTA code in my box. The above
explan may very well be the reason of why such bogus
interrupts are being generated.

 I seek pardon if my observatons are wrong. Can
MVISTA guys give a proper expln to this ?

Sarnath


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: EPIC Vs OpenPIC Vs MontaVista

[Dag Nygren]

>
> Guys,
>   I ran into some accidental discoveries
> while scanning EPIC code written by Mvista.
>   As I said b4 there is a descrepancy with
> the OpenPIC register layout and EPIC layout.
> But the way Mvista people have programmed EPIC is
> good in one sense, bad in other.
>   The reason for assigning vector of 16 to PCI
> interrupts is because each entry in the "Interrupt Source" is 32 bytes long, 16 * 32 = 512 = 0x200
> which equals the difference in offsets between
> the std openPIC layout and EPIC register layout.
> This 16 has got NOTHING TO DO with NUM_8259_INTERRUPTS.
> But Mvista code seems to assume that this feature
> is because of NUM_8259_INTERRUPTS. ( See the
> #define for SANDPOINT_SIO_IRQ ).
> Thus while writing into  Sources[16], mvista code
> writes into "sources[0]" in the EPIC reigster layout.
> Thus IRQ0 gets initialized to vector 16.
>  The "offset" field in "openpic_init" code,
> is mainly used to seperate of vector spaces of
> various interrupt controllers in the system.
>   EPIC manual clearly says that the "Interrupt
> Sources" register offset start at 0x50200. mvista
> code overwrites the 0x50000-0x50200 fields ( which are
> actually reserved as per EPIC manual ). Thus the
> reliablity of MVISTA code is a question mark .
> I have come across some "Bogus interrupts" while
> runing MVISTA code in my box. The above
> explan may very well be the reason of why such bogus
> interrupts are being generated.

Hi,

Funny you should write about this.!
I am just trying to fix it in a reasonable way
for my EPIC-board.

I would like to suggest a global variable ie.
OpenPIC_RegOffset
which would be inited to 0 and could be changed to
16 for a MPC107 (EPIC).
This wouldn't (shouldn't) break anything existing
and will enable the port of a EPIC-board to use
all the generic openpic routines.

Even if I don't like touching "generic" code, I even
more dislike code-duplication

I could submit the patches, if your are interested,
they are noe too big, and a review from the masses
should do the code good.

BRGDS



--
Dag Nygren                               email: dag@newtech.fi
Oy Espoon NewTech Ab                     phone: +358 9 8024910
Träsktorpet 3                              fax: +358 9 8024916
02360 ESBO                              Mobile: +358 400 426312
FINLAND


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Re: EPIC Vs OpenPIC Vs MontaVista

[Sarnath Kannan]

> Hi,
>
> Funny you should write about this.!
> I am just trying to fix it in a reasonable way
> for my EPIC-board.

:-) Oh really ! Good. Since I am new to the group
I dint know its an existing bug ! but how can
one tolerate populating reserved areas ? That
sounds very dangerous to me !

> I would like to suggest a global variable ie.
> OpenPIC_RegOffset
> which would be inited to 0 and could be changed to
> 16 for a MPC107 (EPIC).
> This wouldn't (shouldn't) break anything existing
> and will enable the port of a EPIC-board to use
> all the generic openpic routines.
>
 Yeah there are plenty ways u could solve. The
best one is padding 0x200 bytes to the "OpenPIC"
structure before "Source" in case of CONFIG_SANDPOINT.
That should end the story !

> Even if I don't like touching "generic" code, I even
> more dislike code-duplication
>
> I could submit the patches, if your are interested,
> they are noe too big, and a review from the masses
> should do the code good.
>
> BRGDS
 I think I can fix it myself. Thanks for the offer.
Good Luck !

Sarnath, the NATH ;-)


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: EPIC Vs OpenPIC Vs MontaVista

[Andrew Johnson]

Sarnath Kannan wrote:
>
>   The reason for assigning vector of 16 to PCI
> interrupts is because each entry in the "Interrupt Source" is 32 bytes long, 16 * 32 = 512 = 0x200
> which equals the difference in offsets between
> the std openPIC layout and EPIC register layout.
> This 16 has got NOTHING TO DO with NUM_8259_INTERRUPTS.
> But Mvista code seems to assume that this feature
> is because of NUM_8259_INTERRUPTS. ( See the
> #define for SANDPOINT_SIO_IRQ ).

... unless the register layout for the EPIC was designed to allow an
external i8259 to be inserted into the gap, which ISTR is done on some
boards.  I don't know for sure and I might be completely confused about
that, but there must have been some reason why the EPIC was designed like
that.

Just throwing that idea into the pot...

- Andrew
--
Perfection is reached, not when there is no longer anything to add,
but when there is no longer anything to take away.
- Antoine de Saint-Exupery

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: EPIC Vs OpenPIC Vs MontaVista

[Mark A. Greer]

Sarnath, Dag, Andrew,

What I think you guys are discovering is an artifact of the inadequate
interface to openpic_init()--its wider than just a 8259/107 problem.  This has
been discussed before and some ideas kick around.

Since you guys seem to have the time & motivation, why don't you go back &
check out the mail archives for that/those discussion(s).  If you like what
was talked about there, go with it or come up with a better solution and post
it here for everyone to comment on.  It would be really nice to have a more
flexible interface to openpic_init(), et. al.

Mark


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: EPIC Vs OpenPIC (RFC)

[Dag Nygren]

> Sarnath, Dag, Andrew,
>
> What I think you guys are discovering is an artifact of the inadequate
> interface to openpic_init()--its wider than just a 8259/107 problem.  This has
> been discussed before and some ideas kick around.
>
> Since you guys seem to have the time & motivation, why don't you go back &
> check out the mail archives for that/those discussion(s).  If you like what
> was talked about there, go with it or come up with a better solution and post
> it here for everyone to comment on.  It would be really nice to have a more
> flexible interface to openpic_init(), et. al.

Would something like this be sufficient/good as interface ?:



typedef enum irq_level_en
    {OP_IRQ_HIGH=1,
     OP_IRQ_LOW=0}
irq_level_type;

typedef enum irq_enable_en
    {OP_IRQ_ON=1,
    OP_IRQ_OFF=0}
irq_enable_type;

typedef struct openpic_irq_def_str {
    u_int           PICIrq;
    u_int           LinuxIrq;
    u_char          Priority;
    irq_level_type  IrqLevel;
    irq_enable_type IrqEnable;
} openpic_irq_def;

typedef struct openpic_def_str {
    struct OpenPIC  *OpenPIC_Addr;
    int             main_pic;
    u_char          RegOffset; /* 0 for "standard", 16 for MPC107 */
    char            *chrp_ack;
    int             programmer_switch_irq; /* Not really needed with the
Priority/IRQ poss. */
    openpic_irq_def *IRQdef;
} openpic_def;

extern void openpic_init(*openpic_def);


If so, I could try to implement it in the current open_pic.c

BRGDS

Dag


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: EPIC Vs OpenPIC (RFC)

[Mark A. Greer]

Dag Nygren wrote:

> > Sarnath, Dag, Andrew,
> >
> > What I think you guys are discovering is an artifact of the inadequate
> > interface to openpic_init()--its wider than just a 8259/107 problem.  This has
> > been discussed before and some ideas kick around.
> >
> > Since you guys seem to have the time & motivation, why don't you go back &
> > check out the mail archives for that/those discussion(s).  If you like what
> > was talked about there, go with it or come up with a better solution and post
> > it here for everyone to comment on.  It would be really nice to have a more
> > flexible interface to openpic_init(), et. al.
>
> Would something like this be sufficient/good as interface ?:
>
> typedef enum irq_level_en
>     {OP_IRQ_HIGH=1,
>      OP_IRQ_LOW=0}
> irq_level_type;
>
> typedef enum irq_enable_en
>     {OP_IRQ_ON=1,
>     OP_IRQ_OFF=0}
> irq_enable_type;
>
> typedef struct openpic_irq_def_str {
>     u_int           PICIrq;
>     u_int           LinuxIrq;
>     u_char          Priority;
>     irq_level_type  IrqLevel;
>     irq_enable_type IrqEnable;
> } openpic_irq_def;
>
> typedef struct openpic_def_str {
>     struct OpenPIC  *OpenPIC_Addr;
>     int             main_pic;
>     u_char          RegOffset; /* 0 for "standard", 16 for MPC107 */
>     char            *chrp_ack;
>     int             programmer_switch_irq; /* Not really needed with the
> Priority/IRQ poss. */
>     openpic_irq_def *IRQdef;
> } openpic_def;
>
> extern void openpic_init(*openpic_def);
>
> If so, I could try to implement it in the current open_pic.c

This is along the lines that I was thinking except add a struct member to specify
whether edge or level sensitive--i.e., separate members for hi/lo and edge/level.

What exactly is the "IrqEnable" for?  Is this so you can set up the IRQ but not
enable it?  Then the driver can enable it later and its set up correctly (e.g.,
I2C)??

Doesn't the new struct eliminate the need for "RegOffset"?  And as you noted,
"programmer_switch_irq" probably isn't needed either.  Right?

If you're up to it, how about implementing, testing, then posting a patch for
everyone to check out??

Mark


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: EPIC Vs OpenPIC (RFC)

[Dag Nygren]

> Mark A. Greer wrote:
>
>
> This is along the lines that I was thinking except add a struct member to specify
> whether edge or level sensitive--i.e., separate members for hi/lo and edge/level

Actually this evolved after I posted it :-), after I started
to modify open_pic.c

> What exactly is the "IrqEnable" for?  Is this so you can set up the IRQ but not
> enable it?  Then the driver can enable it later and its set up correctly (e.g.,
> I2C)??

Not really, it was more or less a mistake in the first draft. Would there be a
need
for that ?

> Doesn't the new struct eliminate the need for "RegOffset"?

Have to think about that when implementing the stuff.

>  And as you noted,
> "programmer_switch_irq" probably isn't needed either.  Right?

Yep, and I also came up with another scheme for the cascading as the
special handling for chrp is actually.....drum roll.... a cascade!
And it would remove the i8259 refs from the openpic stuff.

> If you're up to it, how about implementing, testing, then posting a patch for
> everyone to check out??

I'm trying, but I am a bit scared of destroying too much of the open_pic
code...

Here is the newest suggestion for that interface:

typedef enum irq_polarity_en {
    OP_IRQ_POS = 1,
    OP_IRQ_NEG = 0
} irq_polarity_type;

typedef enum irq_sense_en {
    OP_IRQ_LEVEL = 1,
    OP_IRQ_EDGE  = 0
} irq_sense_type;

typedef struct openpic_irq_def_str {
    u_int               PICIrq;
    u_int               Vector;
    u_char              Priority;
    irq_sense_type      IrqSense;
    irq_polarity_type   IrqPolarity;
    irq_polarity_type   IrqPolarity;
    int                 (*CascadeHandler)();
} openpic_irq_def;

typedef struct openpic_def_str {
    struct OpenPIC  *OpenPIC_Addr;
    int             main_pic;
    u_char          RegOffset; /* 0 for "standard", 16 for MPC107 */
    int             programmer_switch_irq;
    openpic_irq_def *IRQdef;    /* The lovest vector first in the table !!! */
} openpic_def;

Comments?


Dag


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/