Page MenuHomeFreeBSD
Differential D20892 Diff 59795 sys/dev/ntb/ntb_hw/ntb_hw_amd.h

Changeset View

Standalone View

View Options

sys/dev/ntb/ntb_hw/ntb_hw_amd.h

Context not available.
* Rajesh Kumar <rajesh1.kumar@amd.com> * Rajesh Kumar <rajesh1.kumar@amd.com>
* *
* $FreeBSD$ * $FreeBSD$
*/ */
#ifndef NTB_HW_AMD_H #ifndef NTB_HW_AMD_H
#define NTB_HW_AMD_H #define NTB_HW_AMD_H
#define NTB_HW_AMD_VENDOR_ID 0x1022 #define NTB_HW_AMD_VENDOR_ID 0x1022
#define NTB_HW_AMD_DEVICE_ID 0x145B #define NTB_HW_AMD_DEVICE_ID1 0x145B
#define NTB_HW_AMD_DEVICE_ID2 0x148B
#define NTB_DEF_PEER_CNT 1 #define NTB_DEF_PEER_CNT 1
#define NTB_DEF_PEER_IDX 0 #define NTB_DEF_PEER_IDX 0
#define BIT(n) (1 << n) #define BIT(n) (1 << n)
#define AMD_LINK_HB_TIMEOUT (1 * hz) #define AMD_LINK_HB_TIMEOUT (1 * hz)
#define NTB_LIN_STA_ACTIVE_BIT 0x00000002 #define NTB_LIN_STA_ACTIVE_BIT 0x00000002
#define NTB_LNK_STA_SPEED_MASK 0x000F0000 #define NTB_LNK_STA_SPEED_MASK 0x000F0000
cemUnsubmitted
Done
Inline Actions

I don't think it makes much sense to define these as enums anymore. They're only used once, in the amd_ntb_hw_info_list table instantiation. Instead, the numbers can just be placed in the table directly.

cem: I don't think it makes much sense to define these as enums anymore. They're only used once, in…
rajesh1.kumar_amd.comAuthorUnsubmitted
Done
Inline Actions

I believe, you mentioned about AMD_MW_CNT1/2, AMD_BART_START_IDX1/2, AMD_DB_CNT and AMD_SPADS_CNT. If so, they are removed now and the numbers are used directly.

rajesh1.kumar_amd.com: I believe, you mentioned about AMD_MW_CNT1/2, AMD_BART_START_IDX1/2, AMD_DB_CNT and…
Context not available.
#define amd_ntb_peer_reg_read(SIZE, offset) \ #define amd_ntb_peer_reg_read(SIZE, offset) \
amd_ntb_bar_read(SIZE, NTB_CONFIG_BAR, offset + AMD_PEER_OFFSET) amd_ntb_bar_read(SIZE, NTB_CONFIG_BAR, offset + AMD_PEER_OFFSET)
#define amd_ntb_peer_reg_write(SIZE, offset, val) \ #define amd_ntb_peer_reg_write(SIZE, offset, val) \
amd_ntb_bar_write(SIZE, NTB_CONFIG_BAR, offset + AMD_PEER_OFFSET, val) amd_ntb_bar_write(SIZE, NTB_CONFIG_BAR, offset + AMD_PEER_OFFSET, val)
#define DB_MASK_LOCK(sc) mtx_lock_spin(&(sc)->db_mask_lock) #define DB_MASK_LOCK(sc) mtx_lock_spin(&(sc)->db_mask_lock)
#define DB_MASK_UNLOCK(sc) mtx_unlock_spin(&(sc)->db_mask_lock) #define DB_MASK_UNLOCK(sc) mtx_unlock_spin(&(sc)->db_mask_lock)
#define DB_MASK_ASSERT(sc, f) mtx_assert(&(sc)->db_mask_lock, (f)) #define DB_MASK_ASSERT(sc, f) mtx_assert(&(sc)->db_mask_lock, (f))
#define QUIRK_MW0_32BIT 0x01
/* amd_ntb_conn_type are hardware numbers, cannot change. */ /* amd_ntb_conn_type are hardware numbers, cannot change. */
enum amd_ntb_conn_type { enum amd_ntb_conn_type {
NTB_CONN_NONE = -1, NTB_CONN_NONE = -1,
NTB_CONN_PRI, NTB_CONN_PRI,
NTB_CONN_SEC, NTB_CONN_SEC,
}; };
enum ntb_default_port { enum ntb_default_port {
NTB_PORT_PRI_USD, NTB_PORT_PRI_USD,
cemUnsubmitted
Not Done
Inline Actions

It doesn't make a lot of sense to declare a variable (amd_ntb_hw_info_list) in a header, since it will be instantiated each time the header is included. I also don't think it makes sense to store this data separately, but referenced, from the amd_ntb_devs table below. I would just add the needed PCI VID:DID/description fields to struct amd_ntb_hw_info and use that structure for amd_ntb_devs.

amd_ntb_devs is not required to be a struct pci_device_table.

This table should be const.

cem: It doesn't make a lot of sense to declare a variable (`amd_ntb_hw_info_list`) in a header…
rajesh1.kumar_amd.comAuthorUnsubmitted
Done
Inline Actions

Moved the amd_ntb_hw_info_list definition to ntb_hw_amd.c file itself and marked it as static const. Also marked the amd_ntb_devs to be static const.

I understand, you mention to remove two different structures (amd_ntb_hw_info -> amd_ntb_hw_info_list) and (pci_device_table -> amd_ntb_devs) and make them as single structure directly with all details. If so, the reason, why I implemented the pci_device_table is to use the PCI_MATCH macro and PCI_PNP_INFO from the pci subsystem. This was one of the suggested changes when this file is submitted for the first time.

So I retain those two structures but with some of your suggested changes. If I understand wrongly, please correct me.

rajesh1.kumar_amd.com: Moved the amd_ntb_hw_info_list definition to ntb_hw_amd.c file itself and marked it as static…
cemUnsubmitted
Not Done
Inline Actions

The PCI_PNP_INFO macro could still be used on a complete table so long as the struct pci_device_table is the first member of each table entry. Yes, it breaks PCI_MATCH.

I don't think there is any real reason to prefer these matching/pnp macros.

cem: The `PCI_PNP_INFO` macro //could// still be used on a complete table so long as the `struct…
Context not available.
enum amd_ntb_bar { enum amd_ntb_bar {
NTB_CONFIG_BAR = 0, NTB_CONFIG_BAR = 0,
NTB_BAR_1, NTB_BAR_1,
NTB_BAR_2, NTB_BAR_2,
NTB_BAR_3, NTB_BAR_3,
NTB_MAX_BARS NTB_MAX_BARS
}; };
struct amd_ntb_hw_info { struct amd_ntb_hw_info {
uint32_t device_id; uint16_t device_id;
const char *desc; uint8_t mw_count;
uint8_t bar_start_idx;
uint8_t spad_count;
uint8_t db_count;
uint8_t quirks;
char *desc;
}; };
struct amd_ntb_pci_bar_info { struct amd_ntb_pci_bar_info {
bus_space_tag_t pci_bus_tag; bus_space_tag_t pci_bus_tag;
bus_space_handle_t pci_bus_handle; bus_space_handle_t pci_bus_handle;
struct resource *pci_resource; struct resource *pci_resource;
vm_paddr_t pbase; vm_paddr_t pbase;
caddr_t vbase; caddr_t vbase;
vm_size_t size; vm_size_t size;
Context not available.
struct amd_ntb_vec { struct amd_ntb_vec {
struct amd_ntb_softc *ntb; struct amd_ntb_softc *ntb;
uint32_t num; uint32_t num;
unsigned masked; unsigned masked;
}; };
enum { enum {
/* AMD NTB Capability */ /* AMD NTB Capability */
AMD_MW_CNT = 3, AMD_MW_CNT1 = 3,
AMD_MW_CNT2 = 2,
AMD_BAR_START_IDX1 = 1,
AMD_BAR_START_IDX2 = 2,
AMD_DB_CNT = 16, AMD_DB_CNT = 16,
AMD_MSIX_VECTOR_CNT = 24, AMD_MSIX_VECTOR_CNT = 24,
AMD_SPADS_CNT = 16, AMD_SPADS_CNT = 16,
/* AMD NTB Link Status Offset */ /* AMD NTB Link Status Offset */
AMD_LINK_STATUS_OFFSET = 0x68, AMD_LINK_STATUS_OFFSET = 0x68,
/* AMD NTB register offset */ /* AMD NTB register offset */
AMD_CNTL_OFFSET = 0x200, AMD_CNTL_OFFSET = 0x200,
Context not available.
AMD_SMUACK_OFFSET = 0x4A0, AMD_SMUACK_OFFSET = 0x4A0,
AMD_SINRST_OFFSET = 0x4A4, AMD_SINRST_OFFSET = 0x4A4,
AMD_RSPNUM_OFFSET = 0x4A8, AMD_RSPNUM_OFFSET = 0x4A8,
AMD_SMU_SPADMUTEX = 0x4B0, AMD_SMU_SPADMUTEX = 0x4B0,
AMD_SMU_SPADOFFSET = 0x4B4, AMD_SMU_SPADOFFSET = 0x4B4,
AMD_PEER_OFFSET = 0x400, AMD_PEER_OFFSET = 0x400,
}; };
struct amd_ntb_hw_info amd_ntb_hw_info_list[] = {
{ .device_id = NTB_HW_AMD_DEVICE_ID1,
.mw_count = AMD_MW_CNT1,
.bar_start_idx = AMD_BAR_START_IDX1,
.spad_count = AMD_SPADS_CNT,
.db_count = AMD_DB_CNT,
.quirks = QUIRK_MW0_32BIT,
.desc = "AMD Non-Transparent Bridge"},
{ .device_id = NTB_HW_AMD_DEVICE_ID2,
.mw_count = AMD_MW_CNT2,
.bar_start_idx = AMD_BAR_START_IDX2,
.spad_count = AMD_SPADS_CNT,
.db_count = AMD_DB_CNT,
.quirks = 0,
.desc = "AMD Non-Transparent Bridge"},
};
struct amd_ntb_softc { struct amd_ntb_softc {
/* ntb.c context. Do not move! Must go first! */ /* ntb.c context. Do not move! Must go first! */
void *ntb_store; void *ntb_store;
device_t device; device_t device;
enum amd_ntb_conn_type conn_type; enum amd_ntb_conn_type conn_type;
struct amd_ntb_pci_bar_info bar_info[NTB_MAX_BARS]; struct amd_ntb_pci_bar_info bar_info[NTB_MAX_BARS];
struct amd_ntb_int_info int_info[AMD_MSIX_VECTOR_CNT]; struct amd_ntb_int_info int_info[AMD_MSIX_VECTOR_CNT];
struct amd_ntb_vec *msix_vec; struct amd_ntb_vec *msix_vec;
uint16_t allocated_interrupts; uint16_t allocated_interrupts;
struct callout hb_timer; struct callout hb_timer;
uint8_t mw_count; struct amd_ntb_hw_info *hw_info;
uint8_t spad_count;
uint8_t db_count;
uint8_t msix_vec_count; uint8_t msix_vec_count;
struct mtx db_mask_lock; struct mtx db_mask_lock;
volatile uint32_t ntb_ctl; volatile uint32_t ntb_ctl;
volatile uint32_t lnk_sta; volatile uint32_t lnk_sta;
volatile uint32_t peer_sta; volatile uint32_t peer_sta;
volatile uint32_t cntl_sta; volatile uint32_t cntl_sta;
Context not available.