Re: [PATCH 1/2] hw/cxl: Add utility functions decoder interleave ways and target count.

[Jonathan Cameron]

On Tue, 5 Sep 2023 15:56:39 +0100
Jonathan Cameron via <qemu-devel@nongnu.org> wrote:

> On Mon, 4 Sep 2023 20:26:59 +0200
> Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
> > On 4/9/23 18:47, Jonathan Cameron wrote:  
> > > As an encoded version of these key configuration parameters is
> > > a register, provide functions to extract it again so as to avoid
> > > the need for duplicating the storage.
> > > 
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > ---
> > >   include/hw/cxl/cxl_component.h | 14 ++++++++++++++
> > >   hw/cxl/cxl-component-utils.c   | 17 +++++++++++++++++
> > >   2 files changed, 31 insertions(+)
> > > 
> > > diff --git a/include/hw/cxl/cxl_component.h 
> > > b/include/hw/cxl/cxl_component.h
> > > index 42c7e581a7..f0ad9cf7de 100644
> > > --- a/include/hw/cxl/cxl_component.h
> > > +++ b/include/hw/cxl/cxl_component.h
> > > @@ -238,7 +238,21 @@ static inline int cxl_decoder_count_enc(int count)
> > >       return 0;
> > >   }
> > >   
> > > +static inline int cxl_decoder_count_dec(int enc_cnt)
> > > +{
> > > +    switch (enc_cnt) {
> > > +    case 0: return 1;
> > > +    case 1: return 2;
> > > +    case 2: return 4;
> > > +    case 3: return 6;
> > > +    case 4: return 8;
> > > +    case 5: return 10;
> > > +    }
> > > +    return 0;
> > > +}    
> > 
> > Why inline?
> >   
> 
> Bad habit.
Nope. I'm being slow.  This is in a header so if I don't
mark it inline I get a bunch of defined but not used warnings.

Obviously I could move the implementation of this and the matching
encoding routines out of the header. I haven't done so for now.


> 
> 
> > Alternatively:
> > 
> >    unsigned cxl_decoder_count_dec(unsigned enc_cnt)
> >    {
> >        return enc_cnt <= 5 ? 2 * enc_cnt : 0;  
> 
> It gets a little more fiddly than the code I'm proposing implies.
> For Switches and Host Bridges larger values are defined
> (we just don't emulate them yet and may never do so) and those
> don't have a sensible mapping. 
> 
> I guess there is no harm in adding the full decode however
> which will make it more obvious why it was a switch statement.
> 
> >    }
> > 
> >   
> 
> 
>