| Message ID | 20220503214638.1895-6-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: dwc: Add dma-ranges/YAML-schema/Baikal-T1 support | expand |
On Wed, May 04, 2022 at 12:46:26AM +0300, Serge Semin wrote: > Since DWC PCIe v4.70a the controller version can be read from the > PORT_LOGIC.PCIE_VERSION_OFF register. Version is represented in the FourCC > format [1]. It's standard versioning approach for the Synopsys DWC > IP-cores. Moreover some of the DWC kernel drivers already make use of it > to fixup version-dependent functionality (See DWC USB3, Stmicro STMMAC or > recent DW SPI driver). In order to preserve the standard version > representation and prevent the data conversion back and forth, we suggest > to preserve the native version representation in the DWC PCIe driver too > in the same way as it has already been done in the rest of the DWC > drivers. IP-core version reading from the CSR will be introduced in the > next commit together with a simple macro-based API to use it. > > [1] https://en.wikipedia.org/wiki/FourCC > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > --- > drivers/pci/controller/dwc/pci-keystone.c | 12 ++++++------ > drivers/pci/controller/dwc/pcie-designware.c | 8 ++++---- > drivers/pci/controller/dwc/pcie-designware.h | 10 +++++++++- > drivers/pci/controller/dwc/pcie-intel-gw.c | 4 ++-- > drivers/pci/controller/dwc/pcie-tegra194.c | 2 +- > 5 files changed, 22 insertions(+), 14 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c > index d10e5fd0f83c..c51018c68b56 100644 > --- a/drivers/pci/controller/dwc/pci-keystone.c > +++ b/drivers/pci/controller/dwc/pci-keystone.c > @@ -109,7 +109,7 @@ struct ks_pcie_of_data { > enum dw_pcie_device_mode mode; > const struct dw_pcie_host_ops *host_ops; > const struct dw_pcie_ep_ops *ep_ops; > - unsigned int version; > + u32 version; > }; > > struct keystone_pcie { > @@ -1069,19 +1069,19 @@ static int ks_pcie_am654_set_mode(struct device *dev, > > static const struct ks_pcie_of_data ks_pcie_rc_of_data = { > .host_ops = &ks_pcie_host_ops, > - .version = 0x365A, > + .version = DW_PCIE_VER_365A, > }; > > static const struct ks_pcie_of_data ks_pcie_am654_rc_of_data = { > .host_ops = &ks_pcie_am654_host_ops, > .mode = DW_PCIE_RC_TYPE, > - .version = 0x490A, > + .version = DW_PCIE_VER_490A, > }; > > static const struct ks_pcie_of_data ks_pcie_am654_ep_of_data = { > .ep_ops = &ks_pcie_am654_ep_ops, > .mode = DW_PCIE_EP_TYPE, > - .version = 0x490A, > + .version = DW_PCIE_VER_490A, > }; > > static const struct of_device_id ks_pcie_of_match[] = { > @@ -1114,12 +1114,12 @@ static int __init ks_pcie_probe(struct platform_device *pdev) > struct device_link **link; > struct gpio_desc *gpiod; > struct resource *res; > - unsigned int version; > void __iomem *base; > u32 num_viewport; > struct phy **phy; > u32 num_lanes; > char name[10]; > + u32 version; > int ret; > int irq; > int i; > @@ -1233,7 +1233,7 @@ static int __init ks_pcie_probe(struct platform_device *pdev) > goto err_get_sync; > } > > - if (pci->version >= 0x480A) > + if (pci->version >= DW_PCIE_VER_480A) > ret = ks_pcie_am654_set_mode(dev, mode); > else > ret = ks_pcie_set_mode(dev); > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > index 1682f477bf20..3ebb7bfee10f 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -289,7 +289,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no, > val = type | PCIE_ATU_FUNC_NUM(func_no); > if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr)) > val |= PCIE_ATU_INCREASE_REGION_SIZE; > - if (pci->version == 0x490A) > + if (pci->version == DW_PCIE_VER_490A) > val = dw_pcie_enable_ecrc(val); > dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val); > dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2, > @@ -336,7 +336,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, > upper_32_bits(cpu_addr)); > dw_pcie_writel_dbi(pci, PCIE_ATU_LIMIT, > lower_32_bits(limit_addr)); > - if (pci->version >= 0x460A) > + if (pci->version >= DW_PCIE_VER_460A) > dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_LIMIT, > upper_32_bits(limit_addr)); > dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_TARGET, > @@ -345,9 +345,9 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, > upper_32_bits(pci_addr)); > val = type | PCIE_ATU_FUNC_NUM(func_no); > if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) && > - pci->version >= 0x460A) > + pci->version >= DW_PCIE_VER_460A) > val |= PCIE_ATU_INCREASE_REGION_SIZE; > - if (pci->version == 0x490A) > + if (pci->version == DW_PCIE_VER_490A) > val = dw_pcie_enable_ecrc(val); > dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, val); > dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE); > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 7d6e9b7576be..5be43c662176 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -20,6 +20,14 @@ > #include <linux/pci-epc.h> > #include <linux/pci-epf.h> > > +/* 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 > + > /* Parameters for the waiting for link up routine */ > #define LINK_WAIT_MAX_RETRIES 10 > #define LINK_WAIT_USLEEP_MIN 90000 > @@ -269,7 +277,7 @@ struct dw_pcie { > struct pcie_port pp; > struct dw_pcie_ep ep; > const struct dw_pcie_ops *ops; > - unsigned int version; > + u32 version; > int num_lanes; > int link_gen; > u8 n_fts[2]; > diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c > index 5ba144924ff8..786af2ba379f 100644 > --- a/drivers/pci/controller/dwc/pcie-intel-gw.c > +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c > @@ -59,7 +59,7 @@ > #define RESET_INTERVAL_MS 100 > > struct intel_pcie_soc { > - unsigned int pcie_ver; > + u32 pcie_ver; This is not used by the Intel driver code, but just passed to the DWC core code. Given that and that the IP version is new enough, this should be removed one the detection is in place. > }; > > struct intel_pcie { > @@ -395,7 +395,7 @@ static const struct dw_pcie_host_ops intel_pcie_dw_ops = { > }; > > static const struct intel_pcie_soc pcie_data = { > - .pcie_ver = 0x520A, > + .pcie_ver = DW_PCIE_VER_520A, > }; > > static int intel_pcie_probe(struct platform_device *pdev) > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index b1b5f836a806..6f1330ed63e5 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -1981,7 +1981,7 @@ static int tegra194_pcie_probe(struct platform_device *pdev) > pci->ops = &tegra_dw_pcie_ops; > pci->n_fts[0] = N_FTS_VAL; > pci->n_fts[1] = FTS_VAL; > - pci->version = 0x490A; > + pci->version = DW_PCIE_VER_490A; > > pp = &pci->pp; > pp->num_vectors = MAX_MSI_IRQS; > -- > 2.35.1 > >
On Wed, May 04, 2022 at 12:46:26AM +0300, Serge Semin wrote: > Since DWC PCIe v4.70a the controller version can be read from the > PORT_LOGIC.PCIE_VERSION_OFF register. Version is represented in the FourCC > format [1]. It's standard versioning approach for the Synopsys DWC > IP-cores. Moreover some of the DWC kernel drivers already make use of it > to fixup version-dependent functionality (See DWC USB3, Stmicro STMMAC or > recent DW SPI driver). In order to preserve the standard version > representation and prevent the data conversion back and forth, we suggest > to preserve the native version representation in the DWC PCIe driver too > in the same way as it has already been done in the rest of the DWC > drivers. IP-core version reading from the CSR will be introduced in the > next commit together with a simple macro-based API to use it. > > [1] https://en.wikipedia.org/wiki/FourCC > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > --- > drivers/pci/controller/dwc/pci-keystone.c | 12 ++++++------ > drivers/pci/controller/dwc/pcie-designware.c | 8 ++++---- > drivers/pci/controller/dwc/pcie-designware.h | 10 +++++++++- > drivers/pci/controller/dwc/pcie-intel-gw.c | 4 ++-- > drivers/pci/controller/dwc/pcie-tegra194.c | 2 +- > 5 files changed, 22 insertions(+), 14 deletions(-) Reviewed-by: Rob Herring <robh@kernel.org>
On Mon, May 16, 2022 at 03:30:03PM -0500, Rob Herring wrote: > On Wed, May 04, 2022 at 12:46:26AM +0300, Serge Semin wrote: > > Since DWC PCIe v4.70a the controller version can be read from the > > PORT_LOGIC.PCIE_VERSION_OFF register. Version is represented in the FourCC > > format [1]. It's standard versioning approach for the Synopsys DWC > > IP-cores. Moreover some of the DWC kernel drivers already make use of it > > to fixup version-dependent functionality (See DWC USB3, Stmicro STMMAC or > > recent DW SPI driver). In order to preserve the standard version > > representation and prevent the data conversion back and forth, we suggest > > to preserve the native version representation in the DWC PCIe driver too > > in the same way as it has already been done in the rest of the DWC > > drivers. IP-core version reading from the CSR will be introduced in the > > next commit together with a simple macro-based API to use it. > > > > [1] https://en.wikipedia.org/wiki/FourCC > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > drivers/pci/controller/dwc/pci-keystone.c | 12 ++++++------ > > drivers/pci/controller/dwc/pcie-designware.c | 8 ++++---- > > drivers/pci/controller/dwc/pcie-designware.h | 10 +++++++++- > > drivers/pci/controller/dwc/pcie-intel-gw.c | 4 ++-- > > drivers/pci/controller/dwc/pcie-tegra194.c | 2 +- > > 5 files changed, 22 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c > > index d10e5fd0f83c..c51018c68b56 100644 > > --- a/drivers/pci/controller/dwc/pci-keystone.c > > +++ b/drivers/pci/controller/dwc/pci-keystone.c > > @@ -109,7 +109,7 @@ struct ks_pcie_of_data { > > enum dw_pcie_device_mode mode; > > const struct dw_pcie_host_ops *host_ops; > > const struct dw_pcie_ep_ops *ep_ops; > > - unsigned int version; > > + u32 version; > > }; > > > > struct keystone_pcie { > > @@ -1069,19 +1069,19 @@ static int ks_pcie_am654_set_mode(struct device *dev, > > > > static const struct ks_pcie_of_data ks_pcie_rc_of_data = { > > .host_ops = &ks_pcie_host_ops, > > - .version = 0x365A, > > + .version = DW_PCIE_VER_365A, > > }; > > > > static const struct ks_pcie_of_data ks_pcie_am654_rc_of_data = { > > .host_ops = &ks_pcie_am654_host_ops, > > .mode = DW_PCIE_RC_TYPE, > > - .version = 0x490A, > > + .version = DW_PCIE_VER_490A, > > }; > > > > static const struct ks_pcie_of_data ks_pcie_am654_ep_of_data = { > > .ep_ops = &ks_pcie_am654_ep_ops, > > .mode = DW_PCIE_EP_TYPE, > > - .version = 0x490A, > > + .version = DW_PCIE_VER_490A, > > }; > > > > static const struct of_device_id ks_pcie_of_match[] = { > > @@ -1114,12 +1114,12 @@ static int __init ks_pcie_probe(struct platform_device *pdev) > > struct device_link **link; > > struct gpio_desc *gpiod; > > struct resource *res; > > - unsigned int version; > > void __iomem *base; > > u32 num_viewport; > > struct phy **phy; > > u32 num_lanes; > > char name[10]; > > + u32 version; > > int ret; > > int irq; > > int i; > > @@ -1233,7 +1233,7 @@ static int __init ks_pcie_probe(struct platform_device *pdev) > > goto err_get_sync; > > } > > > > - if (pci->version >= 0x480A) > > + if (pci->version >= DW_PCIE_VER_480A) > > ret = ks_pcie_am654_set_mode(dev, mode); > > else > > ret = ks_pcie_set_mode(dev); > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > > index 1682f477bf20..3ebb7bfee10f 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.c > > +++ b/drivers/pci/controller/dwc/pcie-designware.c > > @@ -289,7 +289,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no, > > val = type | PCIE_ATU_FUNC_NUM(func_no); > > if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr)) > > val |= PCIE_ATU_INCREASE_REGION_SIZE; > > - if (pci->version == 0x490A) > > + if (pci->version == DW_PCIE_VER_490A) > > val = dw_pcie_enable_ecrc(val); > > dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val); > > dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2, > > @@ -336,7 +336,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, > > upper_32_bits(cpu_addr)); > > dw_pcie_writel_dbi(pci, PCIE_ATU_LIMIT, > > lower_32_bits(limit_addr)); > > - if (pci->version >= 0x460A) > > + if (pci->version >= DW_PCIE_VER_460A) > > dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_LIMIT, > > upper_32_bits(limit_addr)); > > dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_TARGET, > > @@ -345,9 +345,9 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, > > upper_32_bits(pci_addr)); > > val = type | PCIE_ATU_FUNC_NUM(func_no); > > if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) && > > - pci->version >= 0x460A) > > + pci->version >= DW_PCIE_VER_460A) > > val |= PCIE_ATU_INCREASE_REGION_SIZE; > > - if (pci->version == 0x490A) > > + if (pci->version == DW_PCIE_VER_490A) > > val = dw_pcie_enable_ecrc(val); > > dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, val); > > dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE); > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > > index 7d6e9b7576be..5be43c662176 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > @@ -20,6 +20,14 @@ > > #include <linux/pci-epc.h> > > #include <linux/pci-epf.h> > > > > +/* 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 > > + > > /* Parameters for the waiting for link up routine */ > > #define LINK_WAIT_MAX_RETRIES 10 > > #define LINK_WAIT_USLEEP_MIN 90000 > > @@ -269,7 +277,7 @@ struct dw_pcie { > > struct pcie_port pp; > > struct dw_pcie_ep ep; > > const struct dw_pcie_ops *ops; > > - unsigned int version; > > + u32 version; > > int num_lanes; > > int link_gen; > > u8 n_fts[2]; > > diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c > > index 5ba144924ff8..786af2ba379f 100644 > > --- a/drivers/pci/controller/dwc/pcie-intel-gw.c > > +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c > > @@ -59,7 +59,7 @@ > > #define RESET_INTERVAL_MS 100 > > > > struct intel_pcie_soc { > > - unsigned int pcie_ver; > > + u32 pcie_ver; > > This is not used by the Intel driver code, but just passed to the DWC > core code. Given that and that the IP version is new enough, this should > be removed one the detection is in place. Ok. I'll drop it in an additional patch placed after the version detection patch in the series. What about the Tegra 194 code? Shall I drop it from there too? I've got DW PCIe 4.90a reference manual here. It states there are the PCIE_VERSION_NUMBER_OFF and PCIE_VERSION_TYPE_OFF registers in the port logic reg space. So the removal shall not cause problems. -Sergey > > > }; > > > > struct intel_pcie { > > @@ -395,7 +395,7 @@ static const struct dw_pcie_host_ops intel_pcie_dw_ops = { > > }; > > > > static const struct intel_pcie_soc pcie_data = { > > - .pcie_ver = 0x520A, > > + .pcie_ver = DW_PCIE_VER_520A, > > }; > > > > static int intel_pcie_probe(struct platform_device *pdev) > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > > index b1b5f836a806..6f1330ed63e5 100644 > > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > > @@ -1981,7 +1981,7 @@ static int tegra194_pcie_probe(struct platform_device *pdev) > > pci->ops = &tegra_dw_pcie_ops; > > pci->n_fts[0] = N_FTS_VAL; > > pci->n_fts[1] = FTS_VAL; > > - pci->version = 0x490A; > > + pci->version = DW_PCIE_VER_490A; > > > > pp = &pci->pp; > > pp->num_vectors = MAX_MSI_IRQS; > > -- > > 2.35.1 > > > >
On Fri, May 20, 2022 at 12:29:54PM +0300, Serge Semin wrote: > On Mon, May 16, 2022 at 03:30:03PM -0500, Rob Herring wrote: > > On Wed, May 04, 2022 at 12:46:26AM +0300, Serge Semin wrote: > > > Since DWC PCIe v4.70a the controller version can be read from the > > > PORT_LOGIC.PCIE_VERSION_OFF register. Version is represented in the FourCC > > > format [1]. It's standard versioning approach for the Synopsys DWC > > > IP-cores. Moreover some of the DWC kernel drivers already make use of it > > > to fixup version-dependent functionality (See DWC USB3, Stmicro STMMAC or > > > recent DW SPI driver). In order to preserve the standard version > > > representation and prevent the data conversion back and forth, we suggest > > > to preserve the native version representation in the DWC PCIe driver too > > > in the same way as it has already been done in the rest of the DWC > > > drivers. IP-core version reading from the CSR will be introduced in the > > > next commit together with a simple macro-based API to use it. > > > > > > [1] https://en.wikipedia.org/wiki/FourCC > > > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > --- > > > drivers/pci/controller/dwc/pci-keystone.c | 12 ++++++------ > > > drivers/pci/controller/dwc/pcie-designware.c | 8 ++++---- > > > drivers/pci/controller/dwc/pcie-designware.h | 10 +++++++++- > > > drivers/pci/controller/dwc/pcie-intel-gw.c | 4 ++-- > > > drivers/pci/controller/dwc/pcie-tegra194.c | 2 +- > > > 5 files changed, 22 insertions(+), 14 deletions(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c > > > index d10e5fd0f83c..c51018c68b56 100644 > > > --- a/drivers/pci/controller/dwc/pci-keystone.c > > > +++ b/drivers/pci/controller/dwc/pci-keystone.c > > > @@ -109,7 +109,7 @@ struct ks_pcie_of_data { > > > enum dw_pcie_device_mode mode; > > > const struct dw_pcie_host_ops *host_ops; > > > const struct dw_pcie_ep_ops *ep_ops; > > > - unsigned int version; > > > + u32 version; > > > }; > > > > > > struct keystone_pcie { > > > @@ -1069,19 +1069,19 @@ static int ks_pcie_am654_set_mode(struct device *dev, > > > > > > static const struct ks_pcie_of_data ks_pcie_rc_of_data = { > > > .host_ops = &ks_pcie_host_ops, > > > - .version = 0x365A, > > > + .version = DW_PCIE_VER_365A, > > > }; > > > > > > static const struct ks_pcie_of_data ks_pcie_am654_rc_of_data = { > > > .host_ops = &ks_pcie_am654_host_ops, > > > .mode = DW_PCIE_RC_TYPE, > > > - .version = 0x490A, > > > + .version = DW_PCIE_VER_490A, > > > }; > > > > > > static const struct ks_pcie_of_data ks_pcie_am654_ep_of_data = { > > > .ep_ops = &ks_pcie_am654_ep_ops, > > > .mode = DW_PCIE_EP_TYPE, > > > - .version = 0x490A, > > > + .version = DW_PCIE_VER_490A, > > > }; > > > > > > static const struct of_device_id ks_pcie_of_match[] = { > > > @@ -1114,12 +1114,12 @@ static int __init ks_pcie_probe(struct platform_device *pdev) > > > struct device_link **link; > > > struct gpio_desc *gpiod; > > > struct resource *res; > > > - unsigned int version; > > > void __iomem *base; > > > u32 num_viewport; > > > struct phy **phy; > > > u32 num_lanes; > > > char name[10]; > > > + u32 version; > > > int ret; > > > int irq; > > > int i; > > > @@ -1233,7 +1233,7 @@ static int __init ks_pcie_probe(struct platform_device *pdev) > > > goto err_get_sync; > > > } > > > > > > - if (pci->version >= 0x480A) > > > + if (pci->version >= DW_PCIE_VER_480A) > > > ret = ks_pcie_am654_set_mode(dev, mode); > > > else > > > ret = ks_pcie_set_mode(dev); > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > > > index 1682f477bf20..3ebb7bfee10f 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c > > > @@ -289,7 +289,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no, > > > val = type | PCIE_ATU_FUNC_NUM(func_no); > > > if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr)) > > > val |= PCIE_ATU_INCREASE_REGION_SIZE; > > > - if (pci->version == 0x490A) > > > + if (pci->version == DW_PCIE_VER_490A) > > > val = dw_pcie_enable_ecrc(val); > > > dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val); > > > dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2, > > > @@ -336,7 +336,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, > > > upper_32_bits(cpu_addr)); > > > dw_pcie_writel_dbi(pci, PCIE_ATU_LIMIT, > > > lower_32_bits(limit_addr)); > > > - if (pci->version >= 0x460A) > > > + if (pci->version >= DW_PCIE_VER_460A) > > > dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_LIMIT, > > > upper_32_bits(limit_addr)); > > > dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_TARGET, > > > @@ -345,9 +345,9 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, > > > upper_32_bits(pci_addr)); > > > val = type | PCIE_ATU_FUNC_NUM(func_no); > > > if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) && > > > - pci->version >= 0x460A) > > > + pci->version >= DW_PCIE_VER_460A) > > > val |= PCIE_ATU_INCREASE_REGION_SIZE; > > > - if (pci->version == 0x490A) > > > + if (pci->version == DW_PCIE_VER_490A) > > > val = dw_pcie_enable_ecrc(val); > > > dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, val); > > > dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE); > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > > > index 7d6e9b7576be..5be43c662176 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > > @@ -20,6 +20,14 @@ > > > #include <linux/pci-epc.h> > > > #include <linux/pci-epf.h> > > > > > > +/* 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 > > > + > > > /* Parameters for the waiting for link up routine */ > > > #define LINK_WAIT_MAX_RETRIES 10 > > > #define LINK_WAIT_USLEEP_MIN 90000 > > > @@ -269,7 +277,7 @@ struct dw_pcie { > > > struct pcie_port pp; > > > struct dw_pcie_ep ep; > > > const struct dw_pcie_ops *ops; > > > - unsigned int version; > > > + u32 version; > > > int num_lanes; > > > int link_gen; > > > u8 n_fts[2]; > > > diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c > > > index 5ba144924ff8..786af2ba379f 100644 > > > --- a/drivers/pci/controller/dwc/pcie-intel-gw.c > > > +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c > > > @@ -59,7 +59,7 @@ > > > #define RESET_INTERVAL_MS 100 > > > > > > struct intel_pcie_soc { > > > - unsigned int pcie_ver; > > > + u32 pcie_ver; > > > > > This is not used by the Intel driver code, but just passed to the DWC > > core code. Given that and that the IP version is new enough, this should > > be removed one the detection is in place. > > Ok. I'll drop it in an additional patch placed after the version > detection patch in the series. What about the Tegra 194 code? Shall I > drop it from there too? I've got DW PCIe 4.90a reference manual here. > It states there are the PCIE_VERSION_NUMBER_OFF and > PCIE_VERSION_TYPE_OFF registers in the port logic reg space. So the > removal shall not cause problems. Yes, anywhere we can remove it would be good. Rob
On Fri, May 20, 2022 at 10:06:09AM -0500, Rob Herring wrote: > On Fri, May 20, 2022 at 12:29:54PM +0300, Serge Semin wrote: > > On Mon, May 16, 2022 at 03:30:03PM -0500, Rob Herring wrote: > > > On Wed, May 04, 2022 at 12:46:26AM +0300, Serge Semin wrote: > > > > Since DWC PCIe v4.70a the controller version can be read from the > > > > PORT_LOGIC.PCIE_VERSION_OFF register. Version is represented in the FourCC > > > > format [1]. It's standard versioning approach for the Synopsys DWC > > > > IP-cores. Moreover some of the DWC kernel drivers already make use of it > > > > to fixup version-dependent functionality (See DWC USB3, Stmicro STMMAC or > > > > recent DW SPI driver). In order to preserve the standard version > > > > representation and prevent the data conversion back and forth, we suggest > > > > to preserve the native version representation in the DWC PCIe driver too > > > > in the same way as it has already been done in the rest of the DWC > > > > drivers. IP-core version reading from the CSR will be introduced in the > > > > next commit together with a simple macro-based API to use it. > > > > > > > > [1] https://en.wikipedia.org/wiki/FourCC > > > > > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > --- > > > > drivers/pci/controller/dwc/pci-keystone.c | 12 ++++++------ > > > > drivers/pci/controller/dwc/pcie-designware.c | 8 ++++---- > > > > drivers/pci/controller/dwc/pcie-designware.h | 10 +++++++++- > > > > drivers/pci/controller/dwc/pcie-intel-gw.c | 4 ++-- > > > > drivers/pci/controller/dwc/pcie-tegra194.c | 2 +- > > > > 5 files changed, 22 insertions(+), 14 deletions(-) > > > > > > > > diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c > > > > index d10e5fd0f83c..c51018c68b56 100644 > > > > --- a/drivers/pci/controller/dwc/pci-keystone.c > > > > +++ b/drivers/pci/controller/dwc/pci-keystone.c > > > > @@ -109,7 +109,7 @@ struct ks_pcie_of_data { > > > > enum dw_pcie_device_mode mode; > > > > const struct dw_pcie_host_ops *host_ops; > > > > const struct dw_pcie_ep_ops *ep_ops; > > > > - unsigned int version; > > > > + u32 version; > > > > }; > > > > > > > > struct keystone_pcie { > > > > @@ -1069,19 +1069,19 @@ static int ks_pcie_am654_set_mode(struct device *dev, > > > > > > > > static const struct ks_pcie_of_data ks_pcie_rc_of_data = { > > > > .host_ops = &ks_pcie_host_ops, > > > > - .version = 0x365A, > > > > + .version = DW_PCIE_VER_365A, > > > > }; > > > > > > > > static const struct ks_pcie_of_data ks_pcie_am654_rc_of_data = { > > > > .host_ops = &ks_pcie_am654_host_ops, > > > > .mode = DW_PCIE_RC_TYPE, > > > > - .version = 0x490A, > > > > + .version = DW_PCIE_VER_490A, > > > > }; > > > > > > > > static const struct ks_pcie_of_data ks_pcie_am654_ep_of_data = { > > > > .ep_ops = &ks_pcie_am654_ep_ops, > > > > .mode = DW_PCIE_EP_TYPE, > > > > - .version = 0x490A, > > > > + .version = DW_PCIE_VER_490A, > > > > }; > > > > > > > > static const struct of_device_id ks_pcie_of_match[] = { > > > > @@ -1114,12 +1114,12 @@ static int __init ks_pcie_probe(struct platform_device *pdev) > > > > struct device_link **link; > > > > struct gpio_desc *gpiod; > > > > struct resource *res; > > > > - unsigned int version; > > > > void __iomem *base; > > > > u32 num_viewport; > > > > struct phy **phy; > > > > u32 num_lanes; > > > > char name[10]; > > > > + u32 version; > > > > int ret; > > > > int irq; > > > > int i; > > > > @@ -1233,7 +1233,7 @@ static int __init ks_pcie_probe(struct platform_device *pdev) > > > > goto err_get_sync; > > > > } > > > > > > > > - if (pci->version >= 0x480A) > > > > + if (pci->version >= DW_PCIE_VER_480A) > > > > ret = ks_pcie_am654_set_mode(dev, mode); > > > > else > > > > ret = ks_pcie_set_mode(dev); > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > > > > index 1682f477bf20..3ebb7bfee10f 100644 > > > > --- a/drivers/pci/controller/dwc/pcie-designware.c > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c > > > > @@ -289,7 +289,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no, > > > > val = type | PCIE_ATU_FUNC_NUM(func_no); > > > > if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr)) > > > > val |= PCIE_ATU_INCREASE_REGION_SIZE; > > > > - if (pci->version == 0x490A) > > > > + if (pci->version == DW_PCIE_VER_490A) > > > > val = dw_pcie_enable_ecrc(val); > > > > dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val); > > > > dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2, > > > > @@ -336,7 +336,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, > > > > upper_32_bits(cpu_addr)); > > > > dw_pcie_writel_dbi(pci, PCIE_ATU_LIMIT, > > > > lower_32_bits(limit_addr)); > > > > - if (pci->version >= 0x460A) > > > > + if (pci->version >= DW_PCIE_VER_460A) > > > > dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_LIMIT, > > > > upper_32_bits(limit_addr)); > > > > dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_TARGET, > > > > @@ -345,9 +345,9 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, > > > > upper_32_bits(pci_addr)); > > > > val = type | PCIE_ATU_FUNC_NUM(func_no); > > > > if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) && > > > > - pci->version >= 0x460A) > > > > + pci->version >= DW_PCIE_VER_460A) > > > > val |= PCIE_ATU_INCREASE_REGION_SIZE; > > > > - if (pci->version == 0x490A) > > > > + if (pci->version == DW_PCIE_VER_490A) > > > > val = dw_pcie_enable_ecrc(val); > > > > dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, val); > > > > dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE); > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > > > > index 7d6e9b7576be..5be43c662176 100644 > > > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > > > @@ -20,6 +20,14 @@ > > > > #include <linux/pci-epc.h> > > > > #include <linux/pci-epf.h> > > > > > > > > +/* 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 > > > > + > > > > /* Parameters for the waiting for link up routine */ > > > > #define LINK_WAIT_MAX_RETRIES 10 > > > > #define LINK_WAIT_USLEEP_MIN 90000 > > > > @@ -269,7 +277,7 @@ struct dw_pcie { > > > > struct pcie_port pp; > > > > struct dw_pcie_ep ep; > > > > const struct dw_pcie_ops *ops; > > > > - unsigned int version; > > > > + u32 version; > > > > int num_lanes; > > > > int link_gen; > > > > u8 n_fts[2]; > > > > diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c > > > > index 5ba144924ff8..786af2ba379f 100644 > > > > --- a/drivers/pci/controller/dwc/pcie-intel-gw.c > > > > +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c > > > > @@ -59,7 +59,7 @@ > > > > #define RESET_INTERVAL_MS 100 > > > > > > > > struct intel_pcie_soc { > > > > - unsigned int pcie_ver; > > > > + u32 pcie_ver; > > > > > > > > This is not used by the Intel driver code, but just passed to the DWC > > > core code. Given that and that the IP version is new enough, this should > > > be removed one the detection is in place. > > > > Ok. I'll drop it in an additional patch placed after the version > > detection patch in the series. What about the Tegra 194 code? Shall I > > drop it from there too? I've got DW PCIe 4.90a reference manual here. > > It states there are the PCIE_VERSION_NUMBER_OFF and > > PCIE_VERSION_TYPE_OFF registers in the port logic reg space. So the > > removal shall not cause problems. > > Yes, anywhere we can remove it would be good. Ok. The modification will concern both Intel GW and Tegra 194 (in two separate patches). Keystone PCIe platform driver won't be changed since it uses version ID in before the generic DW PCIe host/EP probe method invocation. -Sergey > > Rob
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c index d10e5fd0f83c..c51018c68b56 100644 --- a/drivers/pci/controller/dwc/pci-keystone.c +++ b/drivers/pci/controller/dwc/pci-keystone.c @@ -109,7 +109,7 @@ struct ks_pcie_of_data { enum dw_pcie_device_mode mode; const struct dw_pcie_host_ops *host_ops; const struct dw_pcie_ep_ops *ep_ops; - unsigned int version; + u32 version; }; struct keystone_pcie { @@ -1069,19 +1069,19 @@ static int ks_pcie_am654_set_mode(struct device *dev, static const struct ks_pcie_of_data ks_pcie_rc_of_data = { .host_ops = &ks_pcie_host_ops, - .version = 0x365A, + .version = DW_PCIE_VER_365A, }; static const struct ks_pcie_of_data ks_pcie_am654_rc_of_data = { .host_ops = &ks_pcie_am654_host_ops, .mode = DW_PCIE_RC_TYPE, - .version = 0x490A, + .version = DW_PCIE_VER_490A, }; static const struct ks_pcie_of_data ks_pcie_am654_ep_of_data = { .ep_ops = &ks_pcie_am654_ep_ops, .mode = DW_PCIE_EP_TYPE, - .version = 0x490A, + .version = DW_PCIE_VER_490A, }; static const struct of_device_id ks_pcie_of_match[] = { @@ -1114,12 +1114,12 @@ static int __init ks_pcie_probe(struct platform_device *pdev) struct device_link **link; struct gpio_desc *gpiod; struct resource *res; - unsigned int version; void __iomem *base; u32 num_viewport; struct phy **phy; u32 num_lanes; char name[10]; + u32 version; int ret; int irq; int i; @@ -1233,7 +1233,7 @@ static int __init ks_pcie_probe(struct platform_device *pdev) goto err_get_sync; } - if (pci->version >= 0x480A) + if (pci->version >= DW_PCIE_VER_480A) ret = ks_pcie_am654_set_mode(dev, mode); else ret = ks_pcie_set_mode(dev); diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c index 1682f477bf20..3ebb7bfee10f 100644 --- a/drivers/pci/controller/dwc/pcie-designware.c +++ b/drivers/pci/controller/dwc/pcie-designware.c @@ -289,7 +289,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no, val = type | PCIE_ATU_FUNC_NUM(func_no); if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr)) val |= PCIE_ATU_INCREASE_REGION_SIZE; - if (pci->version == 0x490A) + if (pci->version == DW_PCIE_VER_490A) val = dw_pcie_enable_ecrc(val); dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val); dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2, @@ -336,7 +336,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, upper_32_bits(cpu_addr)); dw_pcie_writel_dbi(pci, PCIE_ATU_LIMIT, lower_32_bits(limit_addr)); - if (pci->version >= 0x460A) + if (pci->version >= DW_PCIE_VER_460A) dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_LIMIT, upper_32_bits(limit_addr)); dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_TARGET, @@ -345,9 +345,9 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, upper_32_bits(pci_addr)); val = type | PCIE_ATU_FUNC_NUM(func_no); if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) && - pci->version >= 0x460A) + pci->version >= DW_PCIE_VER_460A) val |= PCIE_ATU_INCREASE_REGION_SIZE; - if (pci->version == 0x490A) + if (pci->version == DW_PCIE_VER_490A) val = dw_pcie_enable_ecrc(val); dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, val); dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE); diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 7d6e9b7576be..5be43c662176 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -20,6 +20,14 @@ #include <linux/pci-epc.h> #include <linux/pci-epf.h> +/* 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 + /* Parameters for the waiting for link up routine */ #define LINK_WAIT_MAX_RETRIES 10 #define LINK_WAIT_USLEEP_MIN 90000 @@ -269,7 +277,7 @@ struct dw_pcie { struct pcie_port pp; struct dw_pcie_ep ep; const struct dw_pcie_ops *ops; - unsigned int version; + u32 version; int num_lanes; int link_gen; u8 n_fts[2]; diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c index 5ba144924ff8..786af2ba379f 100644 --- a/drivers/pci/controller/dwc/pcie-intel-gw.c +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c @@ -59,7 +59,7 @@ #define RESET_INTERVAL_MS 100 struct intel_pcie_soc { - unsigned int pcie_ver; + u32 pcie_ver; }; struct intel_pcie { @@ -395,7 +395,7 @@ static const struct dw_pcie_host_ops intel_pcie_dw_ops = { }; static const struct intel_pcie_soc pcie_data = { - .pcie_ver = 0x520A, + .pcie_ver = DW_PCIE_VER_520A, }; static int intel_pcie_probe(struct platform_device *pdev) diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c index b1b5f836a806..6f1330ed63e5 100644 --- a/drivers/pci/controller/dwc/pcie-tegra194.c +++ b/drivers/pci/controller/dwc/pcie-tegra194.c @@ -1981,7 +1981,7 @@ static int tegra194_pcie_probe(struct platform_device *pdev) pci->ops = &tegra_dw_pcie_ops; pci->n_fts[0] = N_FTS_VAL; pci->n_fts[1] = FTS_VAL; - pci->version = 0x490A; + pci->version = DW_PCIE_VER_490A; pp = &pci->pp; pp->num_vectors = MAX_MSI_IRQS;
Since DWC PCIe v4.70a the controller version can be read from the PORT_LOGIC.PCIE_VERSION_OFF register. Version is represented in the FourCC format [1]. It's standard versioning approach for the Synopsys DWC IP-cores. Moreover some of the DWC kernel drivers already make use of it to fixup version-dependent functionality (See DWC USB3, Stmicro STMMAC or recent DW SPI driver). In order to preserve the standard version representation and prevent the data conversion back and forth, we suggest to preserve the native version representation in the DWC PCIe driver too in the same way as it has already been done in the rest of the DWC drivers. IP-core version reading from the CSR will be introduced in the next commit together with a simple macro-based API to use it. [1] https://en.wikipedia.org/wiki/FourCC Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> --- drivers/pci/controller/dwc/pci-keystone.c | 12 ++++++------ drivers/pci/controller/dwc/pcie-designware.c | 8 ++++---- drivers/pci/controller/dwc/pcie-designware.h | 10 +++++++++- drivers/pci/controller/dwc/pcie-intel-gw.c | 4 ++-- drivers/pci/controller/dwc/pcie-tegra194.c | 2 +- 5 files changed, 22 insertions(+), 14 deletions(-)