Date:	Fri, 26 Feb 2010 12:20:30 -0800
From:	"Bounine, Alexandre" <Alexandre.Bounine@....com>
To:	"Micha Nelissen" <micha@...i.hopto.org>
Cc:	<mporter@...nel.crashing.org>, <linuxppc-dev@...ts.ozlabs.org>,
	<linux-kernel@...r.kernel.org>, <thomas.moll.ext@....com>,
	<thomas.moll@...go.com>, "Alexandre Bounine" <alexb@...dra.com>
Subject: RE: [PATCH 3/7] RapidIO: Add Port-Write handling for EM

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists