lkml.org 
[lkml]   [2022]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v5 9/9] tty: serial: atmel: Use FIELD_PREP/FIELD_GET
    On Thu, 22 Sep 2022, Sergiu Moga wrote:

    > Convert all open-coded instances of bitfields retrieval/setting
    > to FIELD_PREP/FIELD_GET where possible.
    >
    > Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>

    Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

    With one caveat below which I leave up to you to decide as it might be
    more a matter of taste thing.

    > @@ -82,7 +84,7 @@
    > #define ATMEL_US_INACK BIT(20) /* Inhibit Non Acknowledge */
    > #define ATMEL_US_DSNACK BIT(21) /* Disable Successive NACK */
    > #define ATMEL_US_MAX_ITER_MASK GENMASK(26, 24) /* Max Iterations */
    > -#define ATMEL_US_MAX_ITER(n) (((n) << 24) & ATMEL_US_MAX_ITER_MASK)
    > +#define ATMEL_US_MAX_ITER(n) FIELD_PREP(ATMEL_US_MAX_ITER_MASK, (n))
    > #define ATMEL_US_FILTER BIT(28) /* Infrared Receive Line Filter */
    >
    > #define ATMEL_US_IER 0x08 /* Interrupt Enable Register */
    > @@ -134,19 +136,19 @@
    >
    > #define ATMEL_US_CMPR 0x90 /* Comparaison Register */
    > #define ATMEL_US_FMR 0xa0 /* FIFO Mode Register */
    > -#define ATMEL_US_TXRDYM(data) (((data) & 0x3) << 0) /* TX Ready Mode */
    > -#define ATMEL_US_RXRDYM(data) (((data) & 0x3) << 4) /* RX Ready Mode */
    > +#define ATMEL_US_TXRDYM(data) FIELD_PREP(GENMASK(1, 0), (data)) /* TX Ready Mode */
    > +#define ATMEL_US_RXRDYM(data) FIELD_PREP(GENMASK(5, 4), (data)) /* RX Ready Mode */
    > #define ATMEL_US_ONE_DATA 0x0
    > #define ATMEL_US_TWO_DATA 0x1
    > #define ATMEL_US_FOUR_DATA 0x2
    > #define ATMEL_US_FRTSC BIT(7) /* FIFO RTS pin Control */
    > -#define ATMEL_US_TXFTHRES(thr) (((thr) & 0x3f) << 8) /* TX FIFO Threshold */
    > -#define ATMEL_US_RXFTHRES(thr) (((thr) & 0x3f) << 16) /* RX FIFO Threshold */
    > -#define ATMEL_US_RXFTHRES2(thr) (((thr) & 0x3f) << 24) /* RX FIFO Threshold2 */
    > +#define ATMEL_US_TXFTHRES(thr) FIELD_PREP(GENMASK(13, 8), (thr)) /* TX FIFO Threshold */
    > +#define ATMEL_US_RXFTHRES(thr) FIELD_PREP(GENMASK(21, 16), (thr)) /* RX FIFO Threshold */
    > +#define ATMEL_US_RXFTHRES2(thr) FIELD_PREP(GENMASK(29, 24), (thr)) /* RX FIFO Threshold2 */
    >
    > #define ATMEL_US_FLR 0xa4 /* FIFO Level Register */
    > -#define ATMEL_US_TXFL(reg) (((reg) >> 0) & 0x3f) /* TX FIFO Level */
    > -#define ATMEL_US_RXFL(reg) (((reg) >> 16) & 0x3f) /* RX FIFO Level */
    > +#define ATMEL_US_TXFL(reg) FIELD_GET(GENMASK(5, 0), (reg)) /* TX FIFO Level */
    > +#define ATMEL_US_RXFL(reg) FIELD_GET(GENMASK(21, 16), (reg)) /* RX FIFO Level */

    I don't particularly like this kind of constructs. IMHO, all these would
    be nice after removing the macro argument and moving the FIELD_PREP() to
    .c file. That is,

    .h:
    #define ATMEL_US_RXFL GENMASK(21, 16)

    .c:
    ... FIELD_PREP(ATMEL_US_RXFL, arg) ...

    But I guess there might be differing opinions on it (and both are
    correct from purely technical perspective).

    --
    i.
    \
     
     \ /
      Last update: 2022-09-22 14:13    [W:4.044 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site