Re: [PATCH v2 edk2-platforms 03/20] Platform/Broadcom/RPi3: Add GPIO and RTC libraries

[Leif Lindholm <leif.lindholm@linaro.org>]

On Thu, Dec 13, 2018 at 10:49:05AM +0000, Pete Batard wrote:
> > > diff --git a/Platform/Broadcom/Bcm283x/Include/Utils.h b/Platform/Broadcom/Bcm283x/Include/Utils.h
> > > new file mode 100644
> > > index 000000000000..47713275de3f
> > > --- /dev/null
> > > +++ b/Platform/Broadcom/Bcm283x/Include/Utils.h
> > > @@ -0,0 +1,33 @@
> > > +/** @file
> > > + *
> > > + *  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> > > + *
> > > + *  This program and the accompanying materials
> > > + *  are licensed and made available under the terms and conditions of the BSD License
> > > + *  which accompanies this distribution.  The full text of the license may be found at
> > > + *  http://opensource.org/licenses/bsd-license.php
> > > + *
> > > + *  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > + *  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > > + *
> > > + **/
> > > +
> > > +#ifndef UTILS_H
> > > +#define UTILS_H
> > > +
> > > +#define _IX_BITS(sm, bg) ((bg) - (sm) + 1)
> > > +#define _IX_MASK(sm, bg) ((1ul << _IX_BITS((sm), (bg))) - 1)
> > > +#define _X(val, sm, bg) ((val) >> (sm)) & _IX_MASK((sm), (bg))
> > > +#define X(val, ix1, ix2) (((ix1) < (ix2)) ? _X((val), (ix1), (ix2)) :   \
> > > +                          _X((val), (ix2), (ix1)))
> > > +
> > > +#define _I(val, sm, bg)  (((val) & _IX_MASK((sm), (bg))) << (sm))
> > > +#define I(val, ix1, ix2) (((ix1) < (ix2)) ? _I((val), (ix1), (ix2)) :   \
> > > +                          _I((val), (ix2), (ix1)))
> > > +#define _M(val, sm, bg)  ((val) & (_IX_MASK((sm), (bg)) << (sm)))
> > > +#define M(val, ix1, ix2) (((ix1) < (ix2)) ? _M((val), (ix1), (ix2)) :   \
> > > +                          _M((val), (ix2), (ix1)))
> > > +
> > > +#define ELES(x) (sizeof((x)) / sizeof((x)[0]))
> > > +
> > 
> > The name of the file suggests that these are generic utility macros,
> > and they use single letter identifiers. Not great for grep, so please
> > improve the names (assuming that they are actually used anywhere)
> 
> These macros are used. But I agree their naming should be better.
> 
> Also, I'll double check if these macros are actually used in more than one
> source, or if we can remove utils.h altogether and just move these macros
> into the driver/library that requires them.

ELES (if used) can be replaced with ARRAY_SIZE from Base.h.

/
    Leif