Re: [RFC PATCH 03/25] hw/cxl/component: Introduce CXL components (8.1.x, 8.2.5)

[Ben Widawsky]

On 20-11-17 12:29:40, Jonathan Cameron wrote:

[snip]

> > >   
> > > > +
> > > > +/* 8.2.5.10 - CXL Security Capability Structure */
> > > > +#define CXL_SEC_REGISTERS_OFFSET (CXL_RAS_REGISTERS_OFFSET + 
> > > > CXL_RAS_REGISTERS_SIZE)
> > > > +#define CXL_SEC_REGISTERS_SIZE   0 /* We don't implement 1.1 
> > > > downstream ports */
> > > > +
> > > > +/* 8.2.5.11 - CXL Link Capability Structure */
> > > > +#define CXL_LINK_REGISTERS_OFFSET (CXL_SEC_REGISTERS_OFFSET + 
> > > > CXL_SEC_REGISTERS_SIZE)
> > > > +#define CXL_LINK_REGISTERS_SIZE   0x38  
> > > 
> > > Bit odd to introduce this but not define anything... Can't we bring these
> > > in when we need them later?  
> > 
> > Repeating my comment from 00/25...
> > 
> > For this specific patch series I liked providing #defines in bulk so that 
> > it's
> > easy enough to just bring up the spec and review them. Not sure if there is 
> > a
> > preference from others in the community on this.
> 
> Personally I'd prefer to see the lot if you are going to do that, on basis
> should only need reviewing against the spec once.
> Not sure others will agree though as "the lot" is an awful lot.
> 

I took a shot at stripping some of this out, but it turns out I already use all
of it for the cxl-component-utils. While some of them aren't directly used, the
space reservations for the various caps make sense here IMO.

So for v2, I'm going to leave this as is, and if there is a desire to do things
differently, I think I'd need a suggestion of how to do so.

[snip]


> 
> Thanks,
> 
> Jonathan
>