Re: [PATCH v6 2/8] thunderbolt: Updating the register definitions

[Andreas Noever <andreas.noever@gmail.com>]

On Mon, Aug 1, 2016 at 2:23 PM, Amir Levy <amir.jer.levy@intel.com> wrote:
> Adding more Thunderbolt(TM) register definitions
> and some helper macros.

Thinking about this again I would prefer it if you would put your
definitions into a separate file under icm/ (even if there is some
duplication). The style (bitfields vs. genmask) is different between
the drivers and for a reader it is difficult to find out what is
actually supposed to be used by the two drivers (ring_desc vs
tbt_buf_desc or the ring RING_INT_EN/DISABLE macros in the header file
vs. ring_interrupt_active in nhi.c).

This would also completely separate the two drivers.

Andreas


> Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
> ---
>  drivers/thunderbolt/nhi_regs.h | 109 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 109 insertions(+)
>
> diff --git a/drivers/thunderbolt/nhi_regs.h b/drivers/thunderbolt/nhi_regs.h
> index 75cf069..b8e961f 100644
> --- a/drivers/thunderbolt/nhi_regs.h
> +++ b/drivers/thunderbolt/nhi_regs.h
> @@ -9,6 +9,11 @@
>
>  #include <linux/types.h>
>
> +#define NHI_MMIO_BAR 0
> +
> +#define TBT_RING_MIN_NUM_BUFFERS       2
> +#define TBT_RING_MAX_FRAME_SIZE                (4 * 1024)
> +
>  enum ring_flags {
>         RING_FLAG_ISOCH_ENABLE = 1 << 27, /* TX only? */
>         RING_FLAG_E2E_FLOW_CONTROL = 1 << 28,
> @@ -39,6 +44,33 @@ struct ring_desc {
>         u32 time; /* write zero */
>  } __packed;
>
> +/**
> + * struct tbt_buf_desc - TX/RX ring buffer descriptor.
> + * This is same as struct ring_desc, but without the use of bitfields and
> + * with explicit endianity.
> + */
> +struct tbt_buf_desc {
> +       __le64 phys;
> +       __le32 attributes;
> +       __le32 time;
> +};
> +
> +#define DESC_ATTR_LEN_SHIFT            0
> +#define DESC_ATTR_LEN_MASK             GENMASK(11, DESC_ATTR_LEN_SHIFT)
> +#define DESC_ATTR_EOF_SHIFT            12
> +#define DESC_ATTR_EOF_MASK             GENMASK(15, DESC_ATTR_EOF_SHIFT)
> +#define DESC_ATTR_SOF_SHIFT            16
> +#define DESC_ATTR_SOF_MASK             GENMASK(19, DESC_ATTR_SOF_SHIFT)
> +#define DESC_ATTR_TX_ISOCH_DMA_EN      BIT(20) /* TX */
> +#define DESC_ATTR_RX_CRC_ERR           BIT(20) /* RX after use */
> +#define DESC_ATTR_DESC_DONE            BIT(21)
> +#define DESC_ATTR_REQ_STS              BIT(22) /* TX and RX before use */
> +#define DESC_ATTR_RX_BUF_OVRN_ERR      BIT(22) /* RX after use */
> +#define DESC_ATTR_INT_EN               BIT(23)
> +#define DESC_ATTR_OFFSET_SHIFT         24
> +#define DESC_ATTR_OFFSET_MASK          GENMASK(31, DESC_ATTR_OFFSET_SHIFT)
> +
> +
>  /* NHI registers in bar 0 */
>
>  /*
> @@ -60,6 +92,30 @@ struct ring_desc {
>   */
>  #define REG_RX_RING_BASE       0x08000
>
> +#define REG_RING_STEP                  16
> +#define REG_RING_PHYS_LO_OFFSET                0
> +#define REG_RING_PHYS_HI_OFFSET                4
> +#define REG_RING_CONS_PROD_OFFSET      8       /* cons - RO, prod - RW */
> +#define REG_RING_CONS_SHIFT            0
> +#define REG_RING_CONS_MASK             GENMASK(15, REG_RING_CONS_SHIFT)
> +#define REG_RING_PROD_SHIFT            16
> +#define REG_RING_PROD_MASK             GENMASK(31, REG_RING_PROD_SHIFT)
> +#define REG_RING_SIZE_OFFSET           12
> +#define REG_RING_SIZE_SHIFT            0
> +#define REG_RING_SIZE_MASK             GENMASK(15, REG_RING_SIZE_SHIFT)
> +#define REG_RING_BUF_SIZE_SHIFT                16
> +#define REG_RING_BUF_SIZE_MASK         GENMASK(27, REG_RING_BUF_SIZE_SHIFT)
> +
> +#define TBT_RING_CONS_PROD_REG(iobase, ringbase, ringnumber) \
> +                             ((iobase) + (ringbase) + \
> +                             ((ringnumber) * REG_RING_STEP) + \
> +                             REG_RING_CONS_PROD_OFFSET)
> +
> +#define TBT_REG_RING_PROD_EXTRACT(val) (((val) & REG_RING_PROD_MASK) >> \
> +                                      REG_RING_PROD_SHIFT)
> +
> +#define TBT_REG_RING_CONS_EXTRACT(val) (((val) & REG_RING_CONS_MASK) >> \
> +                                      REG_RING_CONS_SHIFT)
>  /*
>   * 32 bytes per entry, one entry for every hop (REG_HOP_COUNT)
>   * 00: enum_ring_flags
> @@ -77,6 +133,19 @@ struct ring_desc {
>   * ..: unknown
>   */
>  #define REG_RX_OPTIONS_BASE    0x29800
> +#define REG_RX_OPTS_TX_E2E_HOP_ID_SHIFT        12
> +#define REG_RX_OPTS_TX_E2E_HOP_ID_MASK \
> +                               GENMASK(22, REG_RX_OPTS_TX_E2E_HOP_ID_SHIFT)
> +#define REG_RX_OPTS_MASK_OFFSET                4
> +#define REG_RX_OPTS_MASK_EOF_SHIFT     0
> +#define REG_RX_OPTS_MASK_EOF_MASK      GENMASK(15, REG_RX_OPTS_MASK_EOF_SHIFT)
> +#define REG_RX_OPTS_MASK_SOF_SHIFT     16
> +#define REG_RX_OPTS_MASK_SOF_MASK      GENMASK(31, REG_RX_OPTS_MASK_SOF_SHIFT)
> +
> +#define REG_OPTS_STEP                  32
> +#define REG_OPTS_E2E_EN                        BIT(28)
> +#define REG_OPTS_RAW                   BIT(30)
> +#define REG_OPTS_VALID                 BIT(31)
>
>  /*
>   * three bitfields: tx, rx, rx overflow
> @@ -86,6 +155,7 @@ struct ring_desc {
>   */
>  #define REG_RING_NOTIFY_BASE   0x37800
>  #define RING_NOTIFY_REG_COUNT(nhi) ((31 + 3 * nhi->hop_count) / 32)
> +#define REG_RING_NOTIFY_STEP   4
>
>  /*
>   * two bitfields: rx, tx
> @@ -94,8 +164,47 @@ struct ring_desc {
>   */
>  #define REG_RING_INTERRUPT_BASE        0x38200
>  #define RING_INTERRUPT_REG_COUNT(nhi) ((31 + 2 * nhi->hop_count) / 32)
> +#define REG_RING_INT_TX_PROCESSED(ring_num)            BIT(ring_num)
> +#define REG_RING_INT_RX_PROCESSED(ring_num, num_paths) BIT((ring_num) + \
> +                                                           (num_paths))
> +#define RING_INT_DISABLE(base, val) iowrite32( \
> +                       ioread32((base) + REG_RING_INTERRUPT_BASE) & ~(val), \
> +                       (base) + REG_RING_INTERRUPT_BASE)
> +#define RING_INT_ENABLE(base, val) iowrite32( \
> +                       ioread32((base) + REG_RING_INTERRUPT_BASE) | (val), \
> +                       (base) + REG_RING_INTERRUPT_BASE)
> +#define RING_INT_DISABLE_TX(base, ring_num) \
> +       RING_INT_DISABLE(base, REG_RING_INT_TX_PROCESSED(ring_num))
> +#define RING_INT_DISABLE_RX(base, ring_num, num_paths) \
> +       RING_INT_DISABLE(base, REG_RING_INT_RX_PROCESSED(ring_num, num_paths))
> +#define RING_INT_ENABLE_TX(base, ring_num) \
> +       RING_INT_ENABLE(base, REG_RING_INT_TX_PROCESSED(ring_num))
> +#define RING_INT_ENABLE_RX(base, ring_num, num_paths) \
> +       RING_INT_ENABLE(base, REG_RING_INT_RX_PROCESSED(ring_num, num_paths))
> +#define RING_INT_DISABLE_TX_RX(base, ring_num, num_paths) \
> +       RING_INT_DISABLE(base, REG_RING_INT_TX_PROCESSED(ring_num) | \
> +                              REG_RING_INT_RX_PROCESSED(ring_num, num_paths))
> +
> +#define REG_RING_INTERRUPT_STEP        4
> +
> +#define REG_INT_THROTTLING_RATE        0x38c00
> +#define REG_INT_THROTTLING_RATE_STEP   4
> +#define NUM_INT_VECTORS                        16
> +
> +#define REG_INT_VEC_ALLOC_BASE 0x38c40
> +#define REG_INT_VEC_ALLOC_STEP         4
> +#define REG_INT_VEC_ALLOC_FIELD_BITS   4
> +#define REG_INT_VEC_ALLOC_FIELD_MASK   (BIT(REG_INT_VEC_ALLOC_FIELD_BITS) - 1)
> +#define REG_INT_VEC_ALLOC_PER_REG      ((BITS_PER_BYTE * sizeof(u32)) / \
> +                                        REG_INT_VEC_ALLOC_FIELD_BITS)
>
>  /* The last 11 bits contain the number of hops supported by the NHI port. */
>  #define REG_HOP_COUNT          0x39640
> +#define REG_HOP_COUNT_TOTAL_PATHS_MASK GENMASK(10, 0)
> +
> +#define REG_HOST_INTERFACE_RST 0x39858
> +
> +#define REG_DMA_MISC           0x39864
> +#define REG_DMA_MISC_INT_AUTO_CLEAR    BIT(2)
>
>  #endif
> --
> 2.7.4
>