xiaoxiang781216 commented on code in PR #16772: URL: https://github.com/apache/nuttx/pull/16772#discussion_r2230617135
########## include/nuttx/pci/pci.h: ########## @@ -234,6 +234,56 @@ * Public Types ****************************************************************************/ +/* Values from Link Status register, PCIe r3.1, sec 7.8.8 */ +enum pcie_link_width { + PCIE_LNK_WIDTH_RESRV = 0x00, + PCIE_LNK_X1 = 0x01, + PCIE_LNK_X2 = 0x02, + PCIE_LNK_X4 = 0x04, + PCIE_LNK_X8 = 0x08, + PCIE_LNK_X12 = 0x0c, + PCIE_LNK_X16 = 0x10, + PCIE_LNK_X32 = 0x20, Review Comment: let's move the common code change to new patch ########## include/nuttx/pci/pci.h: ########## @@ -234,6 +234,56 @@ * Public Types ****************************************************************************/ +/* Values from Link Status register, PCIe r3.1, sec 7.8.8 */ +enum pcie_link_width { Review Comment: move { to new line, run tools/checkpatch.sh to fix all style issue before upstreaming ########## include/nuttx/pci/pci_regs.h: ########## @@ -69,6 +72,7 @@ #define PCI_CACHE_LINE_SIZE 0x0c /* 8 bits */ #define PCI_LATENCY_TIMER 0x0d /* 8 bits */ #define PCI_HEADER_TYPE 0x0e /* 8 bits */ +#define PCI_HEADER_TYPE_MASK 0x7f Review Comment: align ########## include/nuttx/pci/pci.h: ########## @@ -234,6 +234,56 @@ * Public Types ****************************************************************************/ +/* Values from Link Status register, PCIe r3.1, sec 7.8.8 */ +enum pcie_link_width { + PCIE_LNK_WIDTH_RESRV = 0x00, + PCIE_LNK_X1 = 0x01, + PCIE_LNK_X2 = 0x02, + PCIE_LNK_X4 = 0x04, + PCIE_LNK_X8 = 0x08, + PCIE_LNK_X12 = 0x0c, + PCIE_LNK_X16 = 0x10, + PCIE_LNK_X32 = 0x20, + PCIE_LNK_WIDTH_UNKNOWN = 0xff, +}; + +/* See matching string table in pci_speed_string() */ +enum pci_bus_speed { Review Comment: why not use macro, but enum for all const ########## include/nuttx/pci/pcie_designware.h: ########## @@ -0,0 +1,567 @@ +/**************************************************************************** + * include/nuttx/pci/pcie_designware.h Review Comment: let's unify the term to pcie_dw and PCIE_DW in ALL places ########## include/nuttx/pci/pcie_designware.h: ########## @@ -0,0 +1,567 @@ +/**************************************************************************** + * include/nuttx/pci/pcie_designware.h + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +#ifndef _PCIE_DESIGNWARE_H +#define _PCIE_DESIGNWARE_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <debug.h> + +#include <nuttx/bits.h> +#include <nuttx/kmalloc.h> +#include <nuttx/pci/pci.h> +#include <nuttx/pci/pci_epc.h> +#include <nuttx/pci/pci_epf.h> +#include <nuttx/spinlock.h> +#include <nuttx/list.h> +#include <nuttx/nuttx.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define SZ_1K 0x00000400 +#define SZ_4K 0x00001000 +#define SZ_1G 0x40000000 +#define SZ_2G 0x80000000 +#define SZ_4G 0x100000000 + +#define upper_32_bits(n) ((uint32_t)(((n) >> 16) >> 16)) +#define lower_32_bits(n) ((uint32_t)(n)) +#define readb(a) (*(FAR volatile uint8_t *)(a)) +#define writeb(v,a) (*(FAR volatile uint8_t *)(a) = (v)) +#define readw(a) (*(FAR volatile uint16_t *)(a)) +#define writew(v,a) (*(FAR volatile uint16_t *)(a) = (v)) +#define readl(a) (*(FAR volatile uint32_t *)(a)) +#define writel(v,a) (*(FAR volatile uint32_t *)(a) = (v)) + +#define __bf_shf(x) (ffsll(x) - 1) +#define FIELD_PREP(_mask, _val) (((_val) << __bf_shf(_mask)) & (_mask)) +#define FIELD_GET(_mask, _reg) (((_reg) & (_mask)) >> __bf_shf(_mask)) + +/* DWC PCIe IP-core versions (native support since v4.70a) */ +#define DW_PCIE_VER_365A 0x3336352a +#define DW_PCIE_VER_460A 0x3436302a +#define DW_PCIE_VER_470A 0x3437302a +#define DW_PCIE_VER_480A 0x3438302a +#define DW_PCIE_VER_490A 0x3439302a +#define DW_PCIE_VER_520A 0x3532302a +#define DW_PCIE_VER_540A 0x3534302a + +#define __dw_pcie_ver_cmp(_pci, _ver, _op) \ + ((_pci)->version _op DW_PCIE_VER_ ## _ver) + +#define dw_pcie_ver_is(_pci, _ver) __dw_pcie_ver_cmp(_pci, _ver, ==) + +#define dw_pcie_ver_is_ge(_pci, _ver) __dw_pcie_ver_cmp(_pci, _ver, >=) + +#define dw_pcie_ver_type_is(_pci, _ver, _type) \ + (__dw_pcie_ver_cmp(_pci, _ver, ==) && \ + __dw_pcie_ver_cmp(_pci, TYPE_ ## _type, ==)) + +#define dw_pcie_ver_type_is_ge(_pci, _ver, _type) \ + (__dw_pcie_ver_cmp(_pci, _ver, ==) && \ + __dw_pcie_ver_cmp(_pci, TYPE_ ## _type, >=)) + +/* DWC PCIe controller capabilities */ +#define DW_PCIE_CAP_REQ_RES 0 +#define DW_PCIE_CAP_IATU_UNROLL 1 +#define DW_PCIE_CAP_CDM_CHECK 2 + +#define dw_pcie_cap_is(_pci, _cap) \ + test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps) + +#define dw_pcie_cap_set(_pci, _cap) \ + set_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps) + +/* Parameters for the waiting for link up routine */ +#define LINK_WAIT_MAX_RETRIES 10 +#define LINK_WAIT_SLEEP_MS 90 + +/* Parameters for the waiting for iATU enabled routine */ +#define LINK_WAIT_MAX_IATU_RETRIES 5 +#define LINK_WAIT_IATU 9 + +/* Synopsys-specific PCIe configuration registers */ +#define PCIE_PORT_FORCE 0x708 +#define PORT_FORCE_DO_DESKEW_FOR_SRIS BIT(23) + +#define PCIE_PORT_AFR 0x70C +#define PORT_AFR_N_FTS_MASK GENMASK(15, 8) +#define PORT_AFR_N_FTS(n) FIELD_PREP(PORT_AFR_N_FTS_MASK, n) +#define PORT_AFR_CC_N_FTS_MASK GENMASK(23, 16) +#define PORT_AFR_CC_N_FTS(n) FIELD_PREP(PORT_AFR_CC_N_FTS_MASK, n) +#define PORT_AFR_ENTER_ASPM BIT(30) +#define PORT_AFR_L0S_ENTRANCE_LAT_SHIFT 24 +#define PORT_AFR_L0S_ENTRANCE_LAT_MASK GENMASK(26, 24) +#define PORT_AFR_L1_ENTRANCE_LAT_SHIFT 27 +#define PORT_AFR_L1_ENTRANCE_LAT_MASK GENMASK(29, 27) + +#define PCIE_PORT_LINK_CONTROL 0x710 +#define PORT_LINK_DLL_LINK_EN BIT(5) +#define PORT_LINK_FAST_LINK_MODE BIT(7) +#define PORT_LINK_MODE_MASK GENMASK(21, 16) +#define PORT_LINK_MODE(n) FIELD_PREP(PORT_LINK_MODE_MASK, n) +#define PORT_LINK_MODE_1_LANES PORT_LINK_MODE(0x1) +#define PORT_LINK_MODE_2_LANES PORT_LINK_MODE(0x3) +#define PORT_LINK_MODE_4_LANES PORT_LINK_MODE(0x7) +#define PORT_LINK_MODE_8_LANES PORT_LINK_MODE(0xf) + +#define PCIE_PORT_LANE_SKEW 0x714 +#define PORT_LANE_SKEW_INSERT_MASK GENMASK(23, 0) + +#define PCIE_PORT_DEBUG0 0x728 +#define PORT_LOGIC_LTSSM_STATE_MASK 0x1f +#define PORT_LOGIC_LTSSM_STATE_L0 0x11 +#define PCIE_PORT_DEBUG1 0x72C +#define PCIE_PORT_DEBUG1_LINK_UP BIT(4) +#define PCIE_PORT_DEBUG1_LINK_IN_TRAINING BIT(29) + +#define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C +#define PORT_LOGIC_N_FTS_MASK GENMASK(7, 0) +#define PORT_LOGIC_SPEED_CHANGE BIT(17) +#define PORT_LOGIC_LINK_WIDTH_MASK GENMASK(12, 8) +#define PORT_LOGIC_LINK_WIDTH(n) FIELD_PREP(PORT_LOGIC_LINK_WIDTH_MASK, n) +#define PORT_LOGIC_LINK_WIDTH_1_LANES PORT_LOGIC_LINK_WIDTH(0x1) +#define PORT_LOGIC_LINK_WIDTH_2_LANES PORT_LOGIC_LINK_WIDTH(0x2) +#define PORT_LOGIC_LINK_WIDTH_4_LANES PORT_LOGIC_LINK_WIDTH(0x4) +#define PORT_LOGIC_LINK_WIDTH_8_LANES PORT_LOGIC_LINK_WIDTH(0x8) + +#define PCIE_MSI_ADDR_LO 0x820 +#define PCIE_MSI_ADDR_HI 0x824 +#define PCIE_MSI_INTR0_ENABLE 0x828 +#define PCIE_MSI_INTR0_MASK 0x82C +#define PCIE_MSI_INTR0_STATUS 0x830 + +#define GEN3_RELATED_OFF 0x890 Review Comment: let's align the value to the same column, let's move the macro which doesn't need for chip driver to drivers/pci/pcie_dw.h ########## include/nuttx/pci/pci.h: ########## @@ -234,6 +234,56 @@ * Public Types ****************************************************************************/ +/* Values from Link Status register, PCIe r3.1, sec 7.8.8 */ +enum pcie_link_width { + PCIE_LNK_WIDTH_RESRV = 0x00, Review Comment: why not move the register bit definition to pci_regs.h and follow the register field macro -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org