From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754139AbcIKACp (ORCPT ); Sat, 10 Sep 2016 20:02:45 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:35110 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752258AbcIKACn (ORCPT ); Sat, 10 Sep 2016 20:02:43 -0400 MIME-Version: 1.0 In-Reply-To: <1470054233-4309-3-git-send-email-amir.jer.levy@intel.com> References: <1470054233-4309-1-git-send-email-amir.jer.levy@intel.com> <1470054233-4309-3-git-send-email-amir.jer.levy@intel.com> From: Andreas Noever Date: Sun, 11 Sep 2016 02:02:02 +0200 Message-ID: Subject: Re: [PATCH v6 2/8] thunderbolt: Updating the register definitions To: Amir Levy Cc: Greg KH , Bjorn Helgaas , Jonathan Corbet , "linux-kernel@vger.kernel.org" , "linux-pci@vger.kernel.org" , netdev@vger.kernel.org, linux-doc@vger.kernel.org, thunderbolt-linux , "Westerberg, Mika" , "Winkler, Tomas" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Aug 1, 2016 at 2:23 PM, Amir Levy 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 > --- > 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 > > +#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 >