It turns out the recent DW PCIe-related patchset was merged in with
several relatively trivial issues left unsettled (noted by Bjorn and
Manivannan). All of these lefovers have been fixed in this patchset.
Namely the series starts with two bug-fixes. The first one concerns the
improper link-mode initialization in case if the CDM-check is enabled. The
second unfortunate mistake I made in the IP-core version type helper. In
particular instead of testing the IP-core version type the macro function
referred to the just IP-core version which obviously wasn't what I
intended.
Afterwards two @Mani-noted fixes follow. Firstly the dma-ranges related warning
message is fixed to start with "DMA-ranges" word instead of "Dma-ranges".
Secondly the Baikal-T1 PCIe Host driver is converted to perform the
asynchronous probe type which saved us of about 15% of bootup time if no any
PCIe peripheral device attached to the port.
Then the patchset contains the Baikal-T1 PCIe driver fix. The
corresponding patch removes the false error message printed during the
controller probe procedure. I accidentally added the unconditional
dev_err_probe() method invocation. It was obviously wrong.
Then two trivial cleanups are introduced. The first one concerns the
duplicated fast-link-mode flag unsetting. The second one implies
dropping a redundant empty line from the dw_pcie_link_set_max_speed()
function.
The series continues with a patch inspired by the last @Bjorn note
regarding the generic resources request interface. As @Bjorn correctly
said it would be nice to have the new interface used wider in the DW PCIe
subsystem. Aside with the Baikal-T1 PCIe Host driver the Toshiba Visconti
PCIe driver can be easily converted to using the generic clock names.
That's what is done in the noted patch.
The patchset is closed with a series of MAINTAINERS-list related patches.
Firstly after getting the DW PCIe RP/EP DT-schemas refactored I forgot to
update the MAINTAINER-list with the new files added in the framework of
that procedure. All the snps,dw-pcie* schemas shall be maintained by the
DW PCIe core driver maintainers. Secondly seeing how long it took for my
patchsets to review and not having any comments from the original driver
maintainers I'd suggest to add myself as the reviewer to the DW PCIe and
eDMA drivers. Thus hopefully the new updates review process will be
performed with much less latencies. For the same reason I would also like
to suggest to add @Manivannan as the DW PCIe/eDMA drivers maintainer if
he isn't against that idea. What do you think about the last suggestion?
Link: https://lore.kernel.org/linux-pci/[email protected]/
Changelog v2:
- Rebase onto the kernel 6.3-rc2.
Link: https://lore.kernel.org/linux-pci/[email protected]/
Changelog v3:
- Drop the patch:
[PATCH v2 01/11] PCI: dwc: Fix port link CSR improper init if CDM check enabled
and rebase onto the already submitted by @Yoshihiro fix:
commit cdce67099117 ("PCI: dwc: Fix PORT_LINK_CONTROL update when CDM check enabled")
- Just resend.
Signed-off-by: Serge Semin <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Pavel Parkhomenko <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Serge Semin (10):
PCI: dwc: Fix erroneous version type test helper
PCI: dwc: Fix inbound iATU entries out-of-bounds warning message
PCI: bt1: Enable async probe type
PCI: bt1: Fix printing false error message
PCI: dwc: Drop duplicated fast-link-mode flag unsetting
PCI: dwc: Drop empty line from dw_pcie_link_set_max_speed()
PCI: visconti: Convert to using generic resources getter
MAINTAINERS: Add all generic DW PCIe RP/EP DT-schemas
MAINTAINERS: Add myself as the DW PCIe core reviewer
MAINTAINERS: Add myself as the DW eDMA driver reviewer
MAINTAINERS | 5 ++-
drivers/pci/controller/dwc/pcie-bt1.c | 5 ++-
.../pci/controller/dwc/pcie-designware-host.c | 2 +-
drivers/pci/controller/dwc/pcie-designware.c | 2 -
drivers/pci/controller/dwc/pcie-designware.h | 7 +++-
drivers/pci/controller/dwc/pcie-visconti.c | 37 +++++++++----------
6 files changed, 30 insertions(+), 28 deletions(-)
--
2.40.0
The dev_err_probe() method is supposed to be invoked only if any error is
happened. It was definitely wrong to call it unconditionally. Due to that
the DWC PCIe host initialization error-message is printed all the time the
Baikal-T1 PCIe controller is probed even if no error actually happened.
Fixes: ba6ed462dcf4 ("PCI: dwc: Add Baikal-T1 PCIe controller support")
Signed-off-by: Serge Semin <[email protected]>
---
drivers/pci/controller/dwc/pcie-bt1.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-bt1.c b/drivers/pci/controller/dwc/pcie-bt1.c
index e36a20bf82cf..6557141873ad 100644
--- a/drivers/pci/controller/dwc/pcie-bt1.c
+++ b/drivers/pci/controller/dwc/pcie-bt1.c
@@ -597,8 +597,10 @@ static int bt1_pcie_add_port(struct bt1_pcie *btpci)
dw_pcie_cap_set(&btpci->dw, REQ_RES);
ret = dw_pcie_host_init(&btpci->dw.pp);
+ if (ret)
+ dev_err_probe(dev, ret, "Failed to initialize DWC PCIe host\n");
- return dev_err_probe(dev, ret, "Failed to initialize DWC PCIe host\n");
+ return ret;
}
static void bt1_pcie_del_port(struct bt1_pcie *btpci)
--
2.40.0
The message is printed if the number of requested inbound iATU windows
exceed the device capability. In that case the message should either refer
to the "dma-ranges" DT property or to the DMA-ranges mapping. We suggest
to use the later version as a counterpart to the just CPU-ranges mapping.
In any case the current "Dma-ranges" phrase seems incorrect.
Suggested-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 9952057c8819..5718b4bb67f0 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -723,7 +723,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
}
if (pci->num_ib_windows <= i)
- dev_warn(pci->dev, "Dma-ranges exceed inbound iATU size (%u)\n",
+ dev_warn(pci->dev, "DMA-ranges exceed inbound iATU size (%u)\n",
pci->num_ib_windows);
return 0;
--
2.40.0
Due to an unfortunate mistake the macro function actually checks the
IP-core version instead of the IP-core version type which isn't what
originally implied. Fix it by introducing a new helper
__dw_pcie_ver_type_cmp() with the same semantic as the __dw_pcie_ver_cmp()
counterpart except it refers to the dw_pcie.type field in order to perform
the passed comparison operation.
Fixes: 0b0a780d52ad ("PCI: dwc: Add macros to compare Synopsys IP core versions")
Signed-off-by: Serge Semin <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 79713ce075cc..adad0ea61799 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -37,17 +37,20 @@
#define __dw_pcie_ver_cmp(_pci, _ver, _op) \
((_pci)->version _op DW_PCIE_VER_ ## _ver)
+#define __dw_pcie_ver_type_cmp(_pci, _type, _op) \
+ ((_pci)->type _op DW_PCIE_VER_TYPE_ ## _type)
+
#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, ==))
+ __dw_pcie_ver_type_cmp(_pci, _type, ==))
#define dw_pcie_ver_type_is_ge(_pci, _ver, _type) \
(__dw_pcie_ver_cmp(_pci, _ver, ==) && \
- __dw_pcie_ver_cmp(_pci, TYPE_ ## _type, >=))
+ __dw_pcie_ver_type_cmp(_pci, _type, >=))
/* DWC PCIe controller capabilities */
#define DW_PCIE_CAP_REQ_RES 0
--
2.40.0
It's safe to enable the asyncronous probe type since the PCIe peripheral
devices probing order isn't essential for booting the system. Moreover
enabling that feature saves 0.5 seconds of bootup time if no any device
attached to the PCIe root port. It's a significant performance gain seeing
the total bootup time takes about 3 seconds.
Suggested-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
---
drivers/pci/controller/dwc/pcie-bt1.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/controller/dwc/pcie-bt1.c b/drivers/pci/controller/dwc/pcie-bt1.c
index 95a723a6fd46..e36a20bf82cf 100644
--- a/drivers/pci/controller/dwc/pcie-bt1.c
+++ b/drivers/pci/controller/dwc/pcie-bt1.c
@@ -638,6 +638,7 @@ static struct platform_driver bt1_pcie_driver = {
.driver = {
.name = "bt1-pcie",
.of_match_table = bt1_pcie_of_match,
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
},
};
module_platform_driver(bt1_pcie_driver);
--
2.40.0
Most likely by mistake the PORT_LINK_FAST_LINK_MODE flag unsetting was
added twice in the commit cff9244432e8 ("PCI: dwc: Ensure FAST_LINK_MODE
is cleared"): first it is cleared right after the content of the
PCIE_PORT_LINK_CONTROL register is read, second it's cleared in the
framework of the link-mode initialization procedure. The later action is
redundant. Let's drop it.
Signed-off-by: Serge Semin <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 8e33e6e59e68..e55b7b387eb6 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -1019,7 +1019,6 @@ void dw_pcie_setup(struct dw_pcie *pci)
}
/* Set the number of lanes */
- val &= ~PORT_LINK_FAST_LINK_MODE;
val &= ~PORT_LINK_MODE_MASK;
switch (pci->num_lanes) {
case 1:
--
2.40.0
Well, just drop a redundant empty line from the tail of the denoted
function which by mistake was added in commit 39bc5006501c ("PCI: dwc:
Centralize link gen setting").
Signed-off-by: Serge Semin <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index e55b7b387eb6..ede166645289 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -729,7 +729,6 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
cap &= ~((u32)PCI_EXP_LNKCAP_SLS);
dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, cap | link_speed);
-
}
void dw_pcie_iatu_detect(struct dw_pcie *pci)
--
2.40.0
Recently the DT-schema of the DW PCIe Root Port and End-point controllers
has been refactored by detaching the common bindings into a separate
schema. The provided modification must be reflected in the MAINTAINERS
list so the patch submitters would be aware of the new files maintainers.
Let's do that by adding the maintained files wildcard pattern like
snps,dw-pcie*.yaml, which is applicable for all the old DW PCIe DT-schema
files and the new one.
Signed-off-by: Serge Semin <[email protected]>
---
MAINTAINERS | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index ec57c42ed544..489fd4b4c7ae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16063,8 +16063,7 @@ M: Jingoo Han <[email protected]>
M: Gustavo Pimentel <[email protected]>
L: [email protected]
S: Maintained
-F: Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
-F: Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
+F: Documentation/devicetree/bindings/pci/snps,dw-pcie*.yaml
F: drivers/pci/controller/dwc/*designware*
PCI DRIVER FOR TI DRA7XX/J721E
--
2.40.0
The driver maintainer has been inactive for almost two years now. It
doesn't positively affect the new patches tests and reviews process. Since
the DW eDMA engine has been embedded into the PCIe controllers in several
our SoCs we will be interested in helping with the updates review.
Signed-off-by: Serge Semin <[email protected]>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 51adcafa0f0c..fb7bada6cdd0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5862,6 +5862,7 @@ F: drivers/mtd/nand/raw/denali*
DESIGNWARE EDMA CORE IP DRIVER
M: Gustavo Pimentel <[email protected]>
+R: Serge Semin <[email protected]>
L: [email protected]
S: Maintained
F: drivers/dma/dw-edma/
--
2.40.0
No actions have been spotted from the driver maintainers for almost two
years now. It significantly delays the review process of the relatively
often incoming updates. Since that IP-core has been used in several our
SoCs adding myself to the list of reviewers will help in the evolving the
driver faster and in catching any potential problem as early as possible.
Signed-off-by: Serge Semin <[email protected]>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 489fd4b4c7ae..51adcafa0f0c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16061,6 +16061,7 @@ F: drivers/pci/controller/dwc/pci-exynos.c
PCI DRIVER FOR SYNOPSYS DESIGNWARE
M: Jingoo Han <[email protected]>
M: Gustavo Pimentel <[email protected]>
+R: Serge Semin <[email protected]>
L: [email protected]
S: Maintained
F: Documentation/devicetree/bindings/pci/snps,dw-pcie*.yaml
--
2.40.0
The generic resources request infrastructure has been recently added to
the DW PCIe core driver. Since the DT-bindings of the Toshibo Visconti
PCIe Host controller is fully compatible with the generic names set let's
convert the driver to using that infrastructure. It won't take much effort
since the low-level device driver implies the resources request only with
no additional manipulations involving them. So just drop the locally
defined clocks request procedures, activate the generic resources request
capability and make sure the mandatory resources have been requested by
the DW PCIe core driver.
Suggested-by: Bjorn Helgaas <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
---
drivers/pci/controller/dwc/pcie-visconti.c | 37 ++++++++++------------
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-visconti.c b/drivers/pci/controller/dwc/pcie-visconti.c
index 71026fefa366..ae1517b52c58 100644
--- a/drivers/pci/controller/dwc/pcie-visconti.c
+++ b/drivers/pci/controller/dwc/pcie-visconti.c
@@ -29,9 +29,6 @@ struct visconti_pcie {
void __iomem *ulreg_base;
void __iomem *smu_base;
void __iomem *mpu_base;
- struct clk *refclk;
- struct clk *coreclk;
- struct clk *auxclk;
};
#define PCIE_UL_REG_S_PCIE_MODE 0x00F4
@@ -198,6 +195,21 @@ static int visconti_pcie_host_init(struct dw_pcie_rp *pp)
int err;
u32 val;
+ if (!pcie->pci.core_clks[DW_PCIE_REF_CLK].clk) {
+ dev_err(pci->dev, "Missing ref clock source\n");
+ return -ENOENT;
+ }
+
+ if (!pcie->pci.core_clks[DW_PCIE_CORE_CLK].clk) {
+ dev_err(pci->dev, "Missing core clock source\n");
+ return -ENOENT;
+ }
+
+ if (!pcie->pci.core_clks[DW_PCIE_AUX_CLK].clk) {
+ dev_err(pci->dev, "Missing aux clock source\n");
+ return -ENOENT;
+ }
+
visconti_smu_writel(pcie,
PISMU_CKON_PCIE_AUX_CLK | PISMU_CKON_PCIE_MSTR_ACLK,
PISMU_CKON_PCIE);
@@ -242,8 +254,6 @@ static const struct dw_pcie_host_ops visconti_pcie_host_ops = {
static int visconti_get_resources(struct platform_device *pdev,
struct visconti_pcie *pcie)
{
- struct device *dev = &pdev->dev;
-
pcie->ulreg_base = devm_platform_ioremap_resource_byname(pdev, "ulreg");
if (IS_ERR(pcie->ulreg_base))
return PTR_ERR(pcie->ulreg_base);
@@ -256,21 +266,6 @@ static int visconti_get_resources(struct platform_device *pdev,
if (IS_ERR(pcie->mpu_base))
return PTR_ERR(pcie->mpu_base);
- pcie->refclk = devm_clk_get(dev, "ref");
- if (IS_ERR(pcie->refclk))
- return dev_err_probe(dev, PTR_ERR(pcie->refclk),
- "Failed to get ref clock\n");
-
- pcie->coreclk = devm_clk_get(dev, "core");
- if (IS_ERR(pcie->coreclk))
- return dev_err_probe(dev, PTR_ERR(pcie->coreclk),
- "Failed to get core clock\n");
-
- pcie->auxclk = devm_clk_get(dev, "aux");
- if (IS_ERR(pcie->auxclk))
- return dev_err_probe(dev, PTR_ERR(pcie->auxclk),
- "Failed to get aux clock\n");
-
return 0;
}
@@ -304,6 +299,8 @@ static int visconti_pcie_probe(struct platform_device *pdev)
pci->dev = dev;
pci->ops = &dw_pcie_ops;
+ dw_pcie_cap_set(pci, REQ_RES);
+
ret = visconti_get_resources(pdev, pcie);
if (ret)
return ret;
--
2.40.0
On Tue, Apr 11, 2023 at 06:39:18AM +0300, Serge Semin wrote:
> It turns out the recent DW PCIe-related patchset was merged in with
> several relatively trivial issues left unsettled (noted by Bjorn and
> Manivannan). All of these lefovers have been fixed in this patchset.
> Namely the series starts with two bug-fixes. The first one concerns the
> improper link-mode initialization in case if the CDM-check is enabled. The
> second unfortunate mistake I made in the IP-core version type helper. In
> particular instead of testing the IP-core version type the macro function
> referred to the just IP-core version which obviously wasn't what I
> intended.
>
> Afterwards two @Mani-noted fixes follow. Firstly the dma-ranges related warning
> message is fixed to start with "DMA-ranges" word instead of "Dma-ranges".
> Secondly the Baikal-T1 PCIe Host driver is converted to perform the
> asynchronous probe type which saved us of about 15% of bootup time if no any
> PCIe peripheral device attached to the port.
>
> Then the patchset contains the Baikal-T1 PCIe driver fix. The
> corresponding patch removes the false error message printed during the
> controller probe procedure. I accidentally added the unconditional
> dev_err_probe() method invocation. It was obviously wrong.
>
> Then two trivial cleanups are introduced. The first one concerns the
> duplicated fast-link-mode flag unsetting. The second one implies
> dropping a redundant empty line from the dw_pcie_link_set_max_speed()
> function.
>
> The series continues with a patch inspired by the last @Bjorn note
> regarding the generic resources request interface. As @Bjorn correctly
> said it would be nice to have the new interface used wider in the DW PCIe
> subsystem. Aside with the Baikal-T1 PCIe Host driver the Toshiba Visconti
> PCIe driver can be easily converted to using the generic clock names.
> That's what is done in the noted patch.
>
> The patchset is closed with a series of MAINTAINERS-list related patches.
> Firstly after getting the DW PCIe RP/EP DT-schemas refactored I forgot to
> update the MAINTAINER-list with the new files added in the framework of
> that procedure. All the snps,dw-pcie* schemas shall be maintained by the
> DW PCIe core driver maintainers. Secondly seeing how long it took for my
> patchsets to review and not having any comments from the original driver
> maintainers I'd suggest to add myself as the reviewer to the DW PCIe and
> eDMA drivers. Thus hopefully the new updates review process will be
> performed with much less latencies. For the same reason I would also like
> to suggest to add @Manivannan as the DW PCIe/eDMA drivers maintainer if
> he isn't against that idea. What do you think about the last suggestion?
>
I'm willing to co-maintain the drivers.
- Mani
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Changelog v2:
> - Rebase onto the kernel 6.3-rc2.
>
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Changelog v3:
> - Drop the patch:
> [PATCH v2 01/11] PCI: dwc: Fix port link CSR improper init if CDM check enabled
> and rebase onto the already submitted by @Yoshihiro fix:
> commit cdce67099117 ("PCI: dwc: Fix PORT_LINK_CONTROL update when CDM check enabled")
> - Just resend.
>
> Signed-off-by: Serge Semin <[email protected]>
> Cc: Alexey Malahov <[email protected]>
> Cc: Pavel Parkhomenko <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
> Serge Semin (10):
> PCI: dwc: Fix erroneous version type test helper
> PCI: dwc: Fix inbound iATU entries out-of-bounds warning message
> PCI: bt1: Enable async probe type
> PCI: bt1: Fix printing false error message
> PCI: dwc: Drop duplicated fast-link-mode flag unsetting
> PCI: dwc: Drop empty line from dw_pcie_link_set_max_speed()
> PCI: visconti: Convert to using generic resources getter
> MAINTAINERS: Add all generic DW PCIe RP/EP DT-schemas
> MAINTAINERS: Add myself as the DW PCIe core reviewer
> MAINTAINERS: Add myself as the DW eDMA driver reviewer
>
> MAINTAINERS | 5 ++-
> drivers/pci/controller/dwc/pcie-bt1.c | 5 ++-
> .../pci/controller/dwc/pcie-designware-host.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.c | 2 -
> drivers/pci/controller/dwc/pcie-designware.h | 7 +++-
> drivers/pci/controller/dwc/pcie-visconti.c | 37 +++++++++----------
> 6 files changed, 30 insertions(+), 28 deletions(-)
>
> --
> 2.40.0
>
>
--
மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 06:39:19AM +0300, Serge Semin wrote:
> Due to an unfortunate mistake the macro function actually checks the
> IP-core version instead of the IP-core version type which isn't what
> originally implied. Fix it by introducing a new helper
> __dw_pcie_ver_type_cmp() with the same semantic as the __dw_pcie_ver_cmp()
> counterpart except it refers to the dw_pcie.type field in order to perform
> the passed comparison operation.
>
> Fixes: 0b0a780d52ad ("PCI: dwc: Add macros to compare Synopsys IP core versions")
> Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
- Mani
> ---
> drivers/pci/controller/dwc/pcie-designware.h | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 79713ce075cc..adad0ea61799 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -37,17 +37,20 @@
> #define __dw_pcie_ver_cmp(_pci, _ver, _op) \
> ((_pci)->version _op DW_PCIE_VER_ ## _ver)
>
> +#define __dw_pcie_ver_type_cmp(_pci, _type, _op) \
> + ((_pci)->type _op DW_PCIE_VER_TYPE_ ## _type)
> +
> #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, ==))
> + __dw_pcie_ver_type_cmp(_pci, _type, ==))
>
> #define dw_pcie_ver_type_is_ge(_pci, _ver, _type) \
> (__dw_pcie_ver_cmp(_pci, _ver, ==) && \
> - __dw_pcie_ver_cmp(_pci, TYPE_ ## _type, >=))
> + __dw_pcie_ver_type_cmp(_pci, _type, >=))
>
> /* DWC PCIe controller capabilities */
> #define DW_PCIE_CAP_REQ_RES 0
> --
> 2.40.0
>
>
--
மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 06:39:20AM +0300, Serge Semin wrote:
> The message is printed if the number of requested inbound iATU windows
> exceed the device capability. In that case the message should either refer
> to the "dma-ranges" DT property or to the DMA-ranges mapping. We suggest
> to use the later version as a counterpart to the just CPU-ranges mapping.
> In any case the current "Dma-ranges" phrase seems incorrect.
>
> Suggested-by: Manivannan Sadhasivam <[email protected]>
> Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
- Mani
> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 9952057c8819..5718b4bb67f0 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -723,7 +723,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> }
>
> if (pci->num_ib_windows <= i)
> - dev_warn(pci->dev, "Dma-ranges exceed inbound iATU size (%u)\n",
> + dev_warn(pci->dev, "DMA-ranges exceed inbound iATU size (%u)\n",
> pci->num_ib_windows);
>
> return 0;
> --
> 2.40.0
>
>
--
மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 06:39:22AM +0300, Serge Semin wrote:
> The dev_err_probe() method is supposed to be invoked only if any error is
> happened. It was definitely wrong to call it unconditionally. Due to that
> the DWC PCIe host initialization error-message is printed all the time the
> Baikal-T1 PCIe controller is probed even if no error actually happened.
>
> Fixes: ba6ed462dcf4 ("PCI: dwc: Add Baikal-T1 PCIe controller support")
> Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
- Mani
> ---
> drivers/pci/controller/dwc/pcie-bt1.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-bt1.c b/drivers/pci/controller/dwc/pcie-bt1.c
> index e36a20bf82cf..6557141873ad 100644
> --- a/drivers/pci/controller/dwc/pcie-bt1.c
> +++ b/drivers/pci/controller/dwc/pcie-bt1.c
> @@ -597,8 +597,10 @@ static int bt1_pcie_add_port(struct bt1_pcie *btpci)
> dw_pcie_cap_set(&btpci->dw, REQ_RES);
>
> ret = dw_pcie_host_init(&btpci->dw.pp);
> + if (ret)
> + dev_err_probe(dev, ret, "Failed to initialize DWC PCIe host\n");
>
> - return dev_err_probe(dev, ret, "Failed to initialize DWC PCIe host\n");
> + return ret;
> }
>
> static void bt1_pcie_del_port(struct bt1_pcie *btpci)
> --
> 2.40.0
>
>
--
மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 06:39:21AM +0300, Serge Semin wrote:
> It's safe to enable the asyncronous probe type since the PCIe peripheral
> devices probing order isn't essential for booting the system. Moreover
> enabling that feature saves 0.5 seconds of bootup time if no any device
> attached to the PCIe root port. It's a significant performance gain seeing
> the total bootup time takes about 3 seconds.
>
> Suggested-by: Manivannan Sadhasivam <[email protected]>
> Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
- Mani
> ---
> drivers/pci/controller/dwc/pcie-bt1.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-bt1.c b/drivers/pci/controller/dwc/pcie-bt1.c
> index 95a723a6fd46..e36a20bf82cf 100644
> --- a/drivers/pci/controller/dwc/pcie-bt1.c
> +++ b/drivers/pci/controller/dwc/pcie-bt1.c
> @@ -638,6 +638,7 @@ static struct platform_driver bt1_pcie_driver = {
> .driver = {
> .name = "bt1-pcie",
> .of_match_table = bt1_pcie_of_match,
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> },
> };
> module_platform_driver(bt1_pcie_driver);
> --
> 2.40.0
>
>
--
மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 06:39:24AM +0300, Serge Semin wrote:
> Well, just drop a redundant empty line from the tail of the denoted
> function which by mistake was added in commit 39bc5006501c ("PCI: dwc:
> Centralize link gen setting").
>
> Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
- Mani
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index e55b7b387eb6..ede166645289 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -729,7 +729,6 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
>
> cap &= ~((u32)PCI_EXP_LNKCAP_SLS);
> dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, cap | link_speed);
> -
> }
>
> void dw_pcie_iatu_detect(struct dw_pcie *pci)
> --
> 2.40.0
>
>
--
மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 06:39:23AM +0300, Serge Semin wrote:
> Most likely by mistake the PORT_LINK_FAST_LINK_MODE flag unsetting was
> added twice in the commit cff9244432e8 ("PCI: dwc: Ensure FAST_LINK_MODE
> is cleared"): first it is cleared right after the content of the
> PCIE_PORT_LINK_CONTROL register is read, second it's cleared in the
> framework of the link-mode initialization procedure. The later action is
> redundant. Let's drop it.
>
> Signed-off-by: Serge Semin <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
- Mani
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 8e33e6e59e68..e55b7b387eb6 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -1019,7 +1019,6 @@ void dw_pcie_setup(struct dw_pcie *pci)
> }
>
> /* Set the number of lanes */
> - val &= ~PORT_LINK_FAST_LINK_MODE;
> val &= ~PORT_LINK_MODE_MASK;
> switch (pci->num_lanes) {
> case 1:
> --
> 2.40.0
>
>
--
மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 06:39:25AM +0300, Serge Semin wrote:
> The generic resources request infrastructure has been recently added to
> the DW PCIe core driver. Since the DT-bindings of the Toshibo Visconti
> PCIe Host controller is fully compatible with the generic names set let's
> convert the driver to using that infrastructure. It won't take much effort
> since the low-level device driver implies the resources request only with
> no additional manipulations involving them. So just drop the locally
> defined clocks request procedures, activate the generic resources request
> capability and make sure the mandatory resources have been requested by
> the DW PCIe core driver.
>
> Suggested-by: Bjorn Helgaas <[email protected]>
> Signed-off-by: Serge Semin <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-visconti.c | 37 ++++++++++------------
> 1 file changed, 17 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-visconti.c b/drivers/pci/controller/dwc/pcie-visconti.c
> index 71026fefa366..ae1517b52c58 100644
> --- a/drivers/pci/controller/dwc/pcie-visconti.c
> +++ b/drivers/pci/controller/dwc/pcie-visconti.c
> @@ -29,9 +29,6 @@ struct visconti_pcie {
> void __iomem *ulreg_base;
> void __iomem *smu_base;
> void __iomem *mpu_base;
> - struct clk *refclk;
> - struct clk *coreclk;
> - struct clk *auxclk;
> };
>
> #define PCIE_UL_REG_S_PCIE_MODE 0x00F4
> @@ -198,6 +195,21 @@ static int visconti_pcie_host_init(struct dw_pcie_rp *pp)
> int err;
> u32 val;
>
> + if (!pcie->pci.core_clks[DW_PCIE_REF_CLK].clk) {
> + dev_err(pci->dev, "Missing ref clock source\n");
> + return -ENOENT;
> + }
> +
> + if (!pcie->pci.core_clks[DW_PCIE_CORE_CLK].clk) {
> + dev_err(pci->dev, "Missing core clock source\n");
> + return -ENOENT;
> + }
> +
> + if (!pcie->pci.core_clks[DW_PCIE_AUX_CLK].clk) {
> + dev_err(pci->dev, "Missing aux clock source\n");
> + return -ENOENT;
> + }
> +
Looking at the driver, I could see no call to clk_prepare_enable() for these
clocks. So from kernel's PoV these are not used at all. So either these clocks
are not required (unlikely) or enabled by the bootloader so kernel just uses it.
In that case, the driver should handle these clocks properly.
@Nobuhiro-San, can you please comment?
- Mani
> visconti_smu_writel(pcie,
> PISMU_CKON_PCIE_AUX_CLK | PISMU_CKON_PCIE_MSTR_ACLK,
> PISMU_CKON_PCIE);
> @@ -242,8 +254,6 @@ static const struct dw_pcie_host_ops visconti_pcie_host_ops = {
> static int visconti_get_resources(struct platform_device *pdev,
> struct visconti_pcie *pcie)
> {
> - struct device *dev = &pdev->dev;
> -
> pcie->ulreg_base = devm_platform_ioremap_resource_byname(pdev, "ulreg");
> if (IS_ERR(pcie->ulreg_base))
> return PTR_ERR(pcie->ulreg_base);
> @@ -256,21 +266,6 @@ static int visconti_get_resources(struct platform_device *pdev,
> if (IS_ERR(pcie->mpu_base))
> return PTR_ERR(pcie->mpu_base);
>
> - pcie->refclk = devm_clk_get(dev, "ref");
> - if (IS_ERR(pcie->refclk))
> - return dev_err_probe(dev, PTR_ERR(pcie->refclk),
> - "Failed to get ref clock\n");
> -
> - pcie->coreclk = devm_clk_get(dev, "core");
> - if (IS_ERR(pcie->coreclk))
> - return dev_err_probe(dev, PTR_ERR(pcie->coreclk),
> - "Failed to get core clock\n");
> -
> - pcie->auxclk = devm_clk_get(dev, "aux");
> - if (IS_ERR(pcie->auxclk))
> - return dev_err_probe(dev, PTR_ERR(pcie->auxclk),
> - "Failed to get aux clock\n");
> -
> return 0;
> }
>
> @@ -304,6 +299,8 @@ static int visconti_pcie_probe(struct platform_device *pdev)
> pci->dev = dev;
> pci->ops = &dw_pcie_ops;
>
> + dw_pcie_cap_set(pci, REQ_RES);
> +
> ret = visconti_get_resources(pdev, pcie);
> if (ret)
> return ret;
> --
> 2.40.0
>
>
--
மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 06:39:26AM +0300, Serge Semin wrote:
> Recently the DT-schema of the DW PCIe Root Port and End-point controllers
> has been refactored by detaching the common bindings into a separate
> schema. The provided modification must be reflected in the MAINTAINERS
> list so the patch submitters would be aware of the new files maintainers.
> Let's do that by adding the maintained files wildcard pattern like
> snps,dw-pcie*.yaml, which is applicable for all the old DW PCIe DT-schema
> files and the new one.
>
> Signed-off-by: Serge Semin <[email protected]>
Acked-by: Manivannan Sadhasivam <[email protected]>
- Mani
> ---
> MAINTAINERS | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ec57c42ed544..489fd4b4c7ae 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16063,8 +16063,7 @@ M: Jingoo Han <[email protected]>
> M: Gustavo Pimentel <[email protected]>
> L: [email protected]
> S: Maintained
> -F: Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
> -F: Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
> +F: Documentation/devicetree/bindings/pci/snps,dw-pcie*.yaml
> F: drivers/pci/controller/dwc/*designware*
>
> PCI DRIVER FOR TI DRA7XX/J721E
> --
> 2.40.0
>
>
--
மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 06:39:28AM +0300, Serge Semin wrote:
> The driver maintainer has been inactive for almost two years now. It
> doesn't positively affect the new patches tests and reviews process. Since
> the DW eDMA engine has been embedded into the PCIe controllers in several
> our SoCs we will be interested in helping with the updates review.
>
> Signed-off-by: Serge Semin <[email protected]>
Same comment as 9/10. I'll defer the decision to Vinod.
Acked-by: Manivannan Sadhasivam <[email protected]>
- Mani
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 51adcafa0f0c..fb7bada6cdd0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5862,6 +5862,7 @@ F: drivers/mtd/nand/raw/denali*
>
> DESIGNWARE EDMA CORE IP DRIVER
> M: Gustavo Pimentel <[email protected]>
> +R: Serge Semin <[email protected]>
> L: [email protected]
> S: Maintained
> F: drivers/dma/dw-edma/
> --
> 2.40.0
>
>
--
மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 06:39:27AM +0300, Serge Semin wrote:
> No actions have been spotted from the driver maintainers for almost two
> years now. It significantly delays the review process of the relatively
> often incoming updates. Since that IP-core has been used in several our
> SoCs adding myself to the list of reviewers will help in the evolving the
> driver faster and in catching any potential problem as early as possible.
>
> Signed-off-by: Serge Semin <[email protected]>
If the Maintainers were not responding for these long years, then it implies
that they are not maintaining at all. So I'm inclined to remove their entries
too but I'll defer that decision to Lorenzo.
For this patch,
Acked-by: Manivannan Sadhasivam <[email protected]>
- Mani
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 489fd4b4c7ae..51adcafa0f0c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16061,6 +16061,7 @@ F: drivers/pci/controller/dwc/pci-exynos.c
> PCI DRIVER FOR SYNOPSYS DESIGNWARE
> M: Jingoo Han <[email protected]>
> M: Gustavo Pimentel <[email protected]>
> +R: Serge Semin <[email protected]>
> L: [email protected]
> S: Maintained
> F: Documentation/devicetree/bindings/pci/snps,dw-pcie*.yaml
> --
> 2.40.0
>
>
--
மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 04:32:40PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Apr 11, 2023 at 06:39:18AM +0300, Serge Semin wrote:
> > It turns out the recent DW PCIe-related patchset was merged in with
> > several relatively trivial issues left unsettled (noted by Bjorn and
> > Manivannan). All of these lefovers have been fixed in this patchset.
> > Namely the series starts with two bug-fixes. The first one concerns the
> > improper link-mode initialization in case if the CDM-check is enabled. The
> > second unfortunate mistake I made in the IP-core version type helper. In
> > particular instead of testing the IP-core version type the macro function
> > referred to the just IP-core version which obviously wasn't what I
> > intended.
> >
> > Afterwards two @Mani-noted fixes follow. Firstly the dma-ranges related warning
> > message is fixed to start with "DMA-ranges" word instead of "Dma-ranges".
> > Secondly the Baikal-T1 PCIe Host driver is converted to perform the
> > asynchronous probe type which saved us of about 15% of bootup time if no any
> > PCIe peripheral device attached to the port.
> >
> > Then the patchset contains the Baikal-T1 PCIe driver fix. The
> > corresponding patch removes the false error message printed during the
> > controller probe procedure. I accidentally added the unconditional
> > dev_err_probe() method invocation. It was obviously wrong.
> >
> > Then two trivial cleanups are introduced. The first one concerns the
> > duplicated fast-link-mode flag unsetting. The second one implies
> > dropping a redundant empty line from the dw_pcie_link_set_max_speed()
> > function.
> >
> > The series continues with a patch inspired by the last @Bjorn note
> > regarding the generic resources request interface. As @Bjorn correctly
> > said it would be nice to have the new interface used wider in the DW PCIe
> > subsystem. Aside with the Baikal-T1 PCIe Host driver the Toshiba Visconti
> > PCIe driver can be easily converted to using the generic clock names.
> > That's what is done in the noted patch.
> >
> > The patchset is closed with a series of MAINTAINERS-list related patches.
> > Firstly after getting the DW PCIe RP/EP DT-schemas refactored I forgot to
> > update the MAINTAINER-list with the new files added in the framework of
> > that procedure. All the snps,dw-pcie* schemas shall be maintained by the
> > DW PCIe core driver maintainers. Secondly seeing how long it took for my
> > patchsets to review and not having any comments from the original driver
> > maintainers I'd suggest to add myself as the reviewer to the DW PCIe and
> > eDMA drivers. Thus hopefully the new updates review process will be
> > performed with much less latencies. For the same reason I would also like
> > to suggest to add @Manivannan as the DW PCIe/eDMA drivers maintainer if
> > he isn't against that idea. What do you think about the last suggestion?
> >
>
> I'm willing to co-maintain the drivers.
Awesome! @Bjorn, @Lorenzo, @Vinod what do you think about this? If you
are ok with that shall I resubmit the series with @Mani added to the
DW PCIe/eDMA maintainers list or will you create the respective
patches yourself?
-Serge(y)
>
> - Mani
>
> > Link: https://lore.kernel.org/linux-pci/[email protected]/
> > Changelog v2:
> > - Rebase onto the kernel 6.3-rc2.
> >
> > Link: https://lore.kernel.org/linux-pci/[email protected]/
> > Changelog v3:
> > - Drop the patch:
> > [PATCH v2 01/11] PCI: dwc: Fix port link CSR improper init if CDM check enabled
> > and rebase onto the already submitted by @Yoshihiro fix:
> > commit cdce67099117 ("PCI: dwc: Fix PORT_LINK_CONTROL update when CDM check enabled")
> > - Just resend.
> >
> > Signed-off-by: Serge Semin <[email protected]>
> > Cc: Alexey Malahov <[email protected]>
> > Cc: Pavel Parkhomenko <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> >
> > Serge Semin (10):
> > PCI: dwc: Fix erroneous version type test helper
> > PCI: dwc: Fix inbound iATU entries out-of-bounds warning message
> > PCI: bt1: Enable async probe type
> > PCI: bt1: Fix printing false error message
> > PCI: dwc: Drop duplicated fast-link-mode flag unsetting
> > PCI: dwc: Drop empty line from dw_pcie_link_set_max_speed()
> > PCI: visconti: Convert to using generic resources getter
> > MAINTAINERS: Add all generic DW PCIe RP/EP DT-schemas
> > MAINTAINERS: Add myself as the DW PCIe core reviewer
> > MAINTAINERS: Add myself as the DW eDMA driver reviewer
> >
> > MAINTAINERS | 5 ++-
> > drivers/pci/controller/dwc/pcie-bt1.c | 5 ++-
> > .../pci/controller/dwc/pcie-designware-host.c | 2 +-
> > drivers/pci/controller/dwc/pcie-designware.c | 2 -
> > drivers/pci/controller/dwc/pcie-designware.h | 7 +++-
> > drivers/pci/controller/dwc/pcie-visconti.c | 37 +++++++++----------
> > 6 files changed, 30 insertions(+), 28 deletions(-)
> >
> > --
> > 2.40.0
> >
> >
>
> --
> மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 04:49:47PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Apr 11, 2023 at 06:39:25AM +0300, Serge Semin wrote:
> > The generic resources request infrastructure has been recently added to
> > the DW PCIe core driver. Since the DT-bindings of the Toshibo Visconti
> > PCIe Host controller is fully compatible with the generic names set let's
> > convert the driver to using that infrastructure. It won't take much effort
> > since the low-level device driver implies the resources request only with
> > no additional manipulations involving them. So just drop the locally
> > defined clocks request procedures, activate the generic resources request
> > capability and make sure the mandatory resources have been requested by
> > the DW PCIe core driver.
> >
> > Suggested-by: Bjorn Helgaas <[email protected]>
> > Signed-off-by: Serge Semin <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pcie-visconti.c | 37 ++++++++++------------
> > 1 file changed, 17 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-visconti.c b/drivers/pci/controller/dwc/pcie-visconti.c
> > index 71026fefa366..ae1517b52c58 100644
> > --- a/drivers/pci/controller/dwc/pcie-visconti.c
> > +++ b/drivers/pci/controller/dwc/pcie-visconti.c
> > @@ -29,9 +29,6 @@ struct visconti_pcie {
> > void __iomem *ulreg_base;
> > void __iomem *smu_base;
> > void __iomem *mpu_base;
> > - struct clk *refclk;
> > - struct clk *coreclk;
> > - struct clk *auxclk;
> > };
> >
> > #define PCIE_UL_REG_S_PCIE_MODE 0x00F4
> > @@ -198,6 +195,21 @@ static int visconti_pcie_host_init(struct dw_pcie_rp *pp)
> > int err;
> > u32 val;
> >
> > + if (!pcie->pci.core_clks[DW_PCIE_REF_CLK].clk) {
> > + dev_err(pci->dev, "Missing ref clock source\n");
> > + return -ENOENT;
> > + }
> > +
> > + if (!pcie->pci.core_clks[DW_PCIE_CORE_CLK].clk) {
> > + dev_err(pci->dev, "Missing core clock source\n");
> > + return -ENOENT;
> > + }
> > +
> > + if (!pcie->pci.core_clks[DW_PCIE_AUX_CLK].clk) {
> > + dev_err(pci->dev, "Missing aux clock source\n");
> > + return -ENOENT;
> > + }
> > +
>
> Looking at the driver, I could see no call to clk_prepare_enable() for these
> clocks. So from kernel's PoV these are not used at all. So either these clocks
> are not required (unlikely) or enabled by the bootloader so kernel just uses it.
>
> In that case, the driver should handle these clocks properly.
>
> @Nobuhiro-San, can you please comment?
I will easily update the patch with a code enabling these clocks in
the visconti_pcie_host_init() method if @Nobuhiro-San are ok with
that.
-Serge(y)
>
> - Mani
>
> > visconti_smu_writel(pcie,
> > PISMU_CKON_PCIE_AUX_CLK | PISMU_CKON_PCIE_MSTR_ACLK,
> > PISMU_CKON_PCIE);
> > @@ -242,8 +254,6 @@ static const struct dw_pcie_host_ops visconti_pcie_host_ops = {
> > static int visconti_get_resources(struct platform_device *pdev,
> > struct visconti_pcie *pcie)
> > {
> > - struct device *dev = &pdev->dev;
> > -
> > pcie->ulreg_base = devm_platform_ioremap_resource_byname(pdev, "ulreg");
> > if (IS_ERR(pcie->ulreg_base))
> > return PTR_ERR(pcie->ulreg_base);
> > @@ -256,21 +266,6 @@ static int visconti_get_resources(struct platform_device *pdev,
> > if (IS_ERR(pcie->mpu_base))
> > return PTR_ERR(pcie->mpu_base);
> >
> > - pcie->refclk = devm_clk_get(dev, "ref");
> > - if (IS_ERR(pcie->refclk))
> > - return dev_err_probe(dev, PTR_ERR(pcie->refclk),
> > - "Failed to get ref clock\n");
> > -
> > - pcie->coreclk = devm_clk_get(dev, "core");
> > - if (IS_ERR(pcie->coreclk))
> > - return dev_err_probe(dev, PTR_ERR(pcie->coreclk),
> > - "Failed to get core clock\n");
> > -
> > - pcie->auxclk = devm_clk_get(dev, "aux");
> > - if (IS_ERR(pcie->auxclk))
> > - return dev_err_probe(dev, PTR_ERR(pcie->auxclk),
> > - "Failed to get aux clock\n");
> > -
> > return 0;
> > }
> >
> > @@ -304,6 +299,8 @@ static int visconti_pcie_probe(struct platform_device *pdev)
> > pci->dev = dev;
> > pci->ops = &dw_pcie_ops;
> >
> > + dw_pcie_cap_set(pci, REQ_RES);
> > +
> > ret = visconti_get_resources(pdev, pcie);
> > if (ret)
> > return ret;
> > --
> > 2.40.0
> >
> >
>
> --
> மணிவண்ணன் சதாசிவம்
On Tue, Apr 11, 2023 at 04:54:16PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Apr 11, 2023 at 06:39:27AM +0300, Serge Semin wrote:
> > No actions have been spotted from the driver maintainers for almost two
> > years now. It significantly delays the review process of the relatively
> > often incoming updates. Since that IP-core has been used in several our
> > SoCs adding myself to the list of reviewers will help in the evolving the
> > driver faster and in catching any potential problem as early as possible.
> >
> > Signed-off-by: Serge Semin <[email protected]>
>
> If the Maintainers were not responding for these long years, then it implies
> that they are not maintaining at all. So I'm inclined to remove their entries
> too but I'll defer that decision to Lorenzo.
To be completely honest @Jingoo responded to a few patches with minor
comments lately meanwhile @Gustavo has been completely silent for
about two years now.
>
> For this patch,
>
> Acked-by: Manivannan Sadhasivam <[email protected]>
Thanks.
-Serge(y)
>
> - Mani
>
> > ---
> > MAINTAINERS | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 489fd4b4c7ae..51adcafa0f0c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -16061,6 +16061,7 @@ F: drivers/pci/controller/dwc/pci-exynos.c
> > PCI DRIVER FOR SYNOPSYS DESIGNWARE
> > M: Jingoo Han <[email protected]>
> > M: Gustavo Pimentel <[email protected]>
> > +R: Serge Semin <[email protected]>
> > L: [email protected]
> > S: Maintained
> > F: Documentation/devicetree/bindings/pci/snps,dw-pcie*.yaml
> > --
> > 2.40.0
> >
> >
>
> --
> மணிவண்ணன் சதாசிவம்
On 11-04-23, 19:59, Serge Semin wrote:
> On Tue, Apr 11, 2023 at 04:32:40PM +0530, Manivannan Sadhasivam wrote:
> > On Tue, Apr 11, 2023 at 06:39:18AM +0300, Serge Semin wrote:
> > > It turns out the recent DW PCIe-related patchset was merged in with
> > > several relatively trivial issues left unsettled (noted by Bjorn and
> > > Manivannan). All of these lefovers have been fixed in this patchset.
> > > Namely the series starts with two bug-fixes. The first one concerns the
> > > improper link-mode initialization in case if the CDM-check is enabled. The
> > > second unfortunate mistake I made in the IP-core version type helper. In
> > > particular instead of testing the IP-core version type the macro function
> > > referred to the just IP-core version which obviously wasn't what I
> > > intended.
> > >
> > > Afterwards two @Mani-noted fixes follow. Firstly the dma-ranges related warning
> > > message is fixed to start with "DMA-ranges" word instead of "Dma-ranges".
> > > Secondly the Baikal-T1 PCIe Host driver is converted to perform the
> > > asynchronous probe type which saved us of about 15% of bootup time if no any
> > > PCIe peripheral device attached to the port.
> > >
> > > Then the patchset contains the Baikal-T1 PCIe driver fix. The
> > > corresponding patch removes the false error message printed during the
> > > controller probe procedure. I accidentally added the unconditional
> > > dev_err_probe() method invocation. It was obviously wrong.
> > >
> > > Then two trivial cleanups are introduced. The first one concerns the
> > > duplicated fast-link-mode flag unsetting. The second one implies
> > > dropping a redundant empty line from the dw_pcie_link_set_max_speed()
> > > function.
> > >
> > > The series continues with a patch inspired by the last @Bjorn note
> > > regarding the generic resources request interface. As @Bjorn correctly
> > > said it would be nice to have the new interface used wider in the DW PCIe
> > > subsystem. Aside with the Baikal-T1 PCIe Host driver the Toshiba Visconti
> > > PCIe driver can be easily converted to using the generic clock names.
> > > That's what is done in the noted patch.
> > >
> > > The patchset is closed with a series of MAINTAINERS-list related patches.
> > > Firstly after getting the DW PCIe RP/EP DT-schemas refactored I forgot to
> > > update the MAINTAINER-list with the new files added in the framework of
> > > that procedure. All the snps,dw-pcie* schemas shall be maintained by the
> > > DW PCIe core driver maintainers. Secondly seeing how long it took for my
> > > patchsets to review and not having any comments from the original driver
> > > maintainers I'd suggest to add myself as the reviewer to the DW PCIe and
> > > eDMA drivers. Thus hopefully the new updates review process will be
> > > performed with much less latencies. For the same reason I would also like
> > > to suggest to add @Manivannan as the DW PCIe/eDMA drivers maintainer if
> > > he isn't against that idea. What do you think about the last suggestion?
> > >
> >
> > I'm willing to co-maintain the drivers.
>
> Awesome! @Bjorn, @Lorenzo, @Vinod what do you think about this? If you
> are ok with that shall I resubmit the series with @Mani added to the
> DW PCIe/eDMA maintainers list or will you create the respective
> patches yourself?
Pls send the patch, that is preferred.
--
~Vinod
On Wed, Apr 12, 2023 at 10:28:05PM +0530, Vinod Koul wrote:
> On 11-04-23, 19:59, Serge Semin wrote:
> > On Tue, Apr 11, 2023 at 04:32:40PM +0530, Manivannan Sadhasivam wrote:
> > > On Tue, Apr 11, 2023 at 06:39:18AM +0300, Serge Semin wrote:
> > > > It turns out the recent DW PCIe-related patchset was merged in with
> > > > several relatively trivial issues left unsettled (noted by Bjorn and
> > > > Manivannan). All of these lefovers have been fixed in this patchset.
> > > > Namely the series starts with two bug-fixes. The first one concerns the
> > > > improper link-mode initialization in case if the CDM-check is enabled. The
> > > > second unfortunate mistake I made in the IP-core version type helper. In
> > > > particular instead of testing the IP-core version type the macro function
> > > > referred to the just IP-core version which obviously wasn't what I
> > > > intended.
> > > >
> > > > Afterwards two @Mani-noted fixes follow. Firstly the dma-ranges related warning
> > > > message is fixed to start with "DMA-ranges" word instead of "Dma-ranges".
> > > > Secondly the Baikal-T1 PCIe Host driver is converted to perform the
> > > > asynchronous probe type which saved us of about 15% of bootup time if no any
> > > > PCIe peripheral device attached to the port.
> > > >
> > > > Then the patchset contains the Baikal-T1 PCIe driver fix. The
> > > > corresponding patch removes the false error message printed during the
> > > > controller probe procedure. I accidentally added the unconditional
> > > > dev_err_probe() method invocation. It was obviously wrong.
> > > >
> > > > Then two trivial cleanups are introduced. The first one concerns the
> > > > duplicated fast-link-mode flag unsetting. The second one implies
> > > > dropping a redundant empty line from the dw_pcie_link_set_max_speed()
> > > > function.
> > > >
> > > > The series continues with a patch inspired by the last @Bjorn note
> > > > regarding the generic resources request interface. As @Bjorn correctly
> > > > said it would be nice to have the new interface used wider in the DW PCIe
> > > > subsystem. Aside with the Baikal-T1 PCIe Host driver the Toshiba Visconti
> > > > PCIe driver can be easily converted to using the generic clock names.
> > > > That's what is done in the noted patch.
> > > >
> > > > The patchset is closed with a series of MAINTAINERS-list related patches.
> > > > Firstly after getting the DW PCIe RP/EP DT-schemas refactored I forgot to
> > > > update the MAINTAINER-list with the new files added in the framework of
> > > > that procedure. All the snps,dw-pcie* schemas shall be maintained by the
> > > > DW PCIe core driver maintainers. Secondly seeing how long it took for my
> > > > patchsets to review and not having any comments from the original driver
> > > > maintainers I'd suggest to add myself as the reviewer to the DW PCIe and
> > > > eDMA drivers. Thus hopefully the new updates review process will be
> > > > performed with much less latencies. For the same reason I would also like
> > > > to suggest to add @Manivannan as the DW PCIe/eDMA drivers maintainer if
> > > > he isn't against that idea. What do you think about the last suggestion?
> > > >
> > >
> > > I'm willing to co-maintain the drivers.
> >
> > Awesome! @Bjorn, @Lorenzo, @Vinod what do you think about this? If you
> > are ok with that shall I resubmit the series with @Mani added to the
> > DW PCIe/eDMA maintainers list or will you create the respective
> > patches yourself?
>
> Pls send the patch, that is preferred.
Ok. I'll resubmit the series with the new patches replacing @Gustavo with
@Mani as the DW PCIe/eDMA drivers maintainer.
-Serge(y)
>
> --
> ~Vinod
On Thu, Apr 13, 2023 at 04:34:54PM +0300, Serge Semin wrote:
> On Wed, Apr 12, 2023 at 10:28:05PM +0530, Vinod Koul wrote:
> > On 11-04-23, 19:59, Serge Semin wrote:
> > > On Tue, Apr 11, 2023 at 04:32:40PM +0530, Manivannan Sadhasivam wrote:
> > > > On Tue, Apr 11, 2023 at 06:39:18AM +0300, Serge Semin wrote:
> > > > > It turns out the recent DW PCIe-related patchset was merged in with
> > > > > several relatively trivial issues left unsettled (noted by Bjorn and
> > > > > Manivannan). All of these lefovers have been fixed in this patchset.
> > > > > Namely the series starts with two bug-fixes. The first one concerns the
> > > > > improper link-mode initialization in case if the CDM-check is enabled. The
> > > > > second unfortunate mistake I made in the IP-core version type helper. In
> > > > > particular instead of testing the IP-core version type the macro function
> > > > > referred to the just IP-core version which obviously wasn't what I
> > > > > intended.
> > > > >
> > > > > Afterwards two @Mani-noted fixes follow. Firstly the dma-ranges related warning
> > > > > message is fixed to start with "DMA-ranges" word instead of "Dma-ranges".
> > > > > Secondly the Baikal-T1 PCIe Host driver is converted to perform the
> > > > > asynchronous probe type which saved us of about 15% of bootup time if no any
> > > > > PCIe peripheral device attached to the port.
> > > > >
> > > > > Then the patchset contains the Baikal-T1 PCIe driver fix. The
> > > > > corresponding patch removes the false error message printed during the
> > > > > controller probe procedure. I accidentally added the unconditional
> > > > > dev_err_probe() method invocation. It was obviously wrong.
> > > > >
> > > > > Then two trivial cleanups are introduced. The first one concerns the
> > > > > duplicated fast-link-mode flag unsetting. The second one implies
> > > > > dropping a redundant empty line from the dw_pcie_link_set_max_speed()
> > > > > function.
> > > > >
> > > > > The series continues with a patch inspired by the last @Bjorn note
> > > > > regarding the generic resources request interface. As @Bjorn correctly
> > > > > said it would be nice to have the new interface used wider in the DW PCIe
> > > > > subsystem. Aside with the Baikal-T1 PCIe Host driver the Toshiba Visconti
> > > > > PCIe driver can be easily converted to using the generic clock names.
> > > > > That's what is done in the noted patch.
> > > > >
> > > > > The patchset is closed with a series of MAINTAINERS-list related patches.
> > > > > Firstly after getting the DW PCIe RP/EP DT-schemas refactored I forgot to
> > > > > update the MAINTAINER-list with the new files added in the framework of
> > > > > that procedure. All the snps,dw-pcie* schemas shall be maintained by the
> > > > > DW PCIe core driver maintainers. Secondly seeing how long it took for my
> > > > > patchsets to review and not having any comments from the original driver
> > > > > maintainers I'd suggest to add myself as the reviewer to the DW PCIe and
> > > > > eDMA drivers. Thus hopefully the new updates review process will be
> > > > > performed with much less latencies. For the same reason I would also like
> > > > > to suggest to add @Manivannan as the DW PCIe/eDMA drivers maintainer if
> > > > > he isn't against that idea. What do you think about the last suggestion?
> > > > >
> > > >
> > > > I'm willing to co-maintain the drivers.
> > >
> > > Awesome! @Bjorn, @Lorenzo, @Vinod what do you think about this? If you
> > > are ok with that shall I resubmit the series with @Mani added to the
> > > DW PCIe/eDMA maintainers list or will you create the respective
> > > patches yourself?
> >
>
> > Pls send the patch, that is preferred.
>
> Ok. I'll resubmit the series with the new patches replacing @Gustavo with
> @Mani as the DW PCIe/eDMA drivers maintainer.
>
I talked to Vinod about the non-responsive maintainers and he suggested first
demoting them as Reviewers instead of dropping altogether. So you can move
Gustavo as a Reviewer.
- Mani
> -Serge(y)
>
> >
> > --
> > ~Vinod
--
மணிவண்ணன் சதாசிவம்
On Thu, Apr 13, 2023 at 07:30:24PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Apr 13, 2023 at 04:34:54PM +0300, Serge Semin wrote:
> > On Wed, Apr 12, 2023 at 10:28:05PM +0530, Vinod Koul wrote:
> > > On 11-04-23, 19:59, Serge Semin wrote:
> > > > On Tue, Apr 11, 2023 at 04:32:40PM +0530, Manivannan Sadhasivam wrote:
> > > > > On Tue, Apr 11, 2023 at 06:39:18AM +0300, Serge Semin wrote:
> > > > > > It turns out the recent DW PCIe-related patchset was merged in with
> > > > > > several relatively trivial issues left unsettled (noted by Bjorn and
> > > > > > Manivannan). All of these lefovers have been fixed in this patchset.
> > > > > > Namely the series starts with two bug-fixes. The first one concerns the
> > > > > > improper link-mode initialization in case if the CDM-check is enabled. The
> > > > > > second unfortunate mistake I made in the IP-core version type helper. In
> > > > > > particular instead of testing the IP-core version type the macro function
> > > > > > referred to the just IP-core version which obviously wasn't what I
> > > > > > intended.
> > > > > >
> > > > > > Afterwards two @Mani-noted fixes follow. Firstly the dma-ranges related warning
> > > > > > message is fixed to start with "DMA-ranges" word instead of "Dma-ranges".
> > > > > > Secondly the Baikal-T1 PCIe Host driver is converted to perform the
> > > > > > asynchronous probe type which saved us of about 15% of bootup time if no any
> > > > > > PCIe peripheral device attached to the port.
> > > > > >
> > > > > > Then the patchset contains the Baikal-T1 PCIe driver fix. The
> > > > > > corresponding patch removes the false error message printed during the
> > > > > > controller probe procedure. I accidentally added the unconditional
> > > > > > dev_err_probe() method invocation. It was obviously wrong.
> > > > > >
> > > > > > Then two trivial cleanups are introduced. The first one concerns the
> > > > > > duplicated fast-link-mode flag unsetting. The second one implies
> > > > > > dropping a redundant empty line from the dw_pcie_link_set_max_speed()
> > > > > > function.
> > > > > >
> > > > > > The series continues with a patch inspired by the last @Bjorn note
> > > > > > regarding the generic resources request interface. As @Bjorn correctly
> > > > > > said it would be nice to have the new interface used wider in the DW PCIe
> > > > > > subsystem. Aside with the Baikal-T1 PCIe Host driver the Toshiba Visconti
> > > > > > PCIe driver can be easily converted to using the generic clock names.
> > > > > > That's what is done in the noted patch.
> > > > > >
> > > > > > The patchset is closed with a series of MAINTAINERS-list related patches.
> > > > > > Firstly after getting the DW PCIe RP/EP DT-schemas refactored I forgot to
> > > > > > update the MAINTAINER-list with the new files added in the framework of
> > > > > > that procedure. All the snps,dw-pcie* schemas shall be maintained by the
> > > > > > DW PCIe core driver maintainers. Secondly seeing how long it took for my
> > > > > > patchsets to review and not having any comments from the original driver
> > > > > > maintainers I'd suggest to add myself as the reviewer to the DW PCIe and
> > > > > > eDMA drivers. Thus hopefully the new updates review process will be
> > > > > > performed with much less latencies. For the same reason I would also like
> > > > > > to suggest to add @Manivannan as the DW PCIe/eDMA drivers maintainer if
> > > > > > he isn't against that idea. What do you think about the last suggestion?
> > > > > >
> > > > >
> > > > > I'm willing to co-maintain the drivers.
> > > >
> > > > Awesome! @Bjorn, @Lorenzo, @Vinod what do you think about this? If you
> > > > are ok with that shall I resubmit the series with @Mani added to the
> > > > DW PCIe/eDMA maintainers list or will you create the respective
> > > > patches yourself?
> > >
> >
> > > Pls send the patch, that is preferred.
> >
> > Ok. I'll resubmit the series with the new patches replacing @Gustavo with
> > @Mani as the DW PCIe/eDMA drivers maintainer.
> >
>
> I talked to Vinod about the non-responsive maintainers and he suggested first
> demoting them as Reviewers instead of dropping altogether. So you can move
> Gustavo as a Reviewer.
Got it. Thanks for the note.
-Serge(y)
>
> - Mani
>
> > -Serge(y)
> >
> > >
> > > --
> > > ~Vinod
>
> --
> மணிவண்ணன் சதாசிவம்