[PATCH v2 03/10] RapidIO: Use stored ingress port number instead of register read

[Andrew Morton]

On Mon, 20 Sep 2010 07:31:22 -0700
"Bounine, Alexandre" <Alexandre.Bounine at idt.com> wrote:

> Andrew Morton <akpm at linux-foundation.org> wrote:
> > 
> > > @@ -219,6 +219,7 @@ struct rio_net {
> > >  /**
> > >   * struct rio_switch - RIO switch info
> > >   * @node: Node in global list of switches
> > > + * @rdev: Associated RIO device structure
> > >   * @switchid: Switch ID that is unique across a network
> > >   * @hopcount: Hopcount to this switch
> > >   * @destid: Associated destid in the path
> > > @@ -234,6 +235,7 @@ struct rio_net {
> > >   */
> > >  struct rio_switch {
> > >  	struct list_head node;
> > > +	struct rio_dev *rdev;
> > >  	u16 switchid;
> > >  	u16 hopcount;
> > >  	u16 destid;
> > 
> > What is the locking for rdev?
> 
> This question prompted me consider the following change:
> 
> Because the rio_switch structure (in current implementation) is a
> dynamically allocated part of rio_dev, I think it may be simpler to
> combine them into single allocation by using zero length array
> declaration:
> 
> struct rio_dev {
>     struct list_head global_list;
>     struct list_head net_list;
>     .....
>     ..... rest of rio_dev
>     .....
>     struct rio_switch switch[0];
> }
> 
> This will remove extra memory allocation, remove overlapping structure
> members and clean code sections like one shown below:
> 
> 	u8 hopcount = 0xff;
> 	u16 destid = rdev->destid;
> 
> 	if (rdev->rswitch) {
> 		destid = rdev->rswitch->destid;
> 		hopcount = rdev->rswitch->hopcount;
> 	}    
> 
> And this looks better aligned with RapidIO definitions - both: endpoints
> and switches are RIO devices. The current implementation of rio_dev
> somewhat separates rio_switch from its common part (this is why I have
> added that link into rio_switch). We may keep using the pointer to
> associated rio_dev, but reworking the rio_dev structure seems better way
> to me.
> 
> Please, let me know what do you think about this conversion. And if
> there are no objections I will make a patch.
> 

If you say so ;)

The "variable length array at the end of the struct" thing is pretty
commonly used and works well.  As long as we never want to change the
number of switches on the fly (hotplug?).