Messages in this thread | | | Subject | RE: [PATCH 3/7] RapidIO: Add Port-Write handling for EM | Date | Fri, 26 Feb 2010 12:20:30 -0800 | From | "Bounine, Alexandre" <> |
| |
Micha Nelissen wrote: > Bounine, Alexandre wrote: > > Micha Nelissen wrote: > >> Alexandre Bounine wrote: > >>> /** > >>> + * rio_em_set_ops- Sets Error Managment operations for a particular > > vendor switch > >>> + * @rdev: RIO device > >>> + * > >>> + * Searches the RIO EM ops table for known switch types. If the vid > >>> + * and did match a switch table entry, then set the em_init() and > >>> + * em_handle() ops to the table entry values. > >> Shouldn't any RIO device be able to support error management, not just > >> switches? > > > > Only if a device reports this capability by having Error Management > > Extended Features block. > > Ideally, we have to provide default handler for every such device (I am > > planning it for some future updates). It should be the same as for > > routing operations - if the standard feature exists, it has to be used > > unless something else takes over. > > Yes, therefore I thought that: or the EM_OPS are per driver, or they can > be integrated in the switch hooks list.
Or combination of both methods as I implemented it. I want to have it flexible enough to be adopted to further development of RapidIO ecosystem.
> > For now I keep all port-write messages from end-points serviced by their > > individual drivers. One of reasons for this: the EM PW message format > > Maybe have a generic rio function that can be called by any driver that > knows a particular port-write was due to error management causes? This > function would read the standard defined EF block registers. Then the > driver part can be quite small.
I think this echoes what I mentioned above as default handler.
> > >>> + if (port->ops->pwenable) > >>> + port->ops->pwenable(port, enable); > >>> +} > >>> + > >> Maybe this can be done by switch->init function? > > > > This is not per-switch function. This function enables mport to receive > > incoming PW messages. Per-switch PW enable is done in switch->init as > > for Tsi57x. > > Oops, I meant this comment for the em_init function call. > > >>> + rio_mport_write_config_32(mport, destid, > > hopcount, > >>> + rdev->phys_efptr + > >>> + RIO_PORT_N_ACK_STS_CSR(portnum), > >>> + RIO_PORT_N_ACK_CLEAR); > >> This doesn't work for the 568; but the 568 has no special handling? > > > > Tsi568 will not send EM PW message. Tsi568 PWs are disabled in its > > em_init(). > > Why?
Because Tsi568 requires special handling which is not implemented (yet?).
> > >>> +DECLARE_RIO_EM_OPS(RIO_VID_TUNDRA, RIO_DID_TSI578, tsi57x_em_init, > > tsi57x_em_handler); > >> Why not declare these along with the other ops? > > > > Because the EM extensions is a separate capability. It is not guaranteed > > to be in every switch. > > They might initialize them with NULL to indicate they don't support it?
My preference is to keep unrelated capabilities separately. Otherwise, these long ops lists may become unmanageable later when new extensions will be added into RIO spec.
Cheers,
Alex.
| |