* [PATCH RESEND v3 07/10] PCI: visconti: Convert to using generic resources getter [not found] <20230411033928.30397-1-Sergey.Semin@baikalelectronics.ru> @ 2023-04-11 3:39 ` Serge Semin 2023-04-11 11:19 ` Manivannan Sadhasivam 0 siblings, 1 reply; 3+ messages in thread From: Serge Semin @ 2023-04-11 3:39 UTC (permalink / raw) To: Bjorn Helgaas, Lorenzo Pieralisi, Cai Huoqing, Jingoo Han, Gustavo Pimentel, Vinod Koul, Manivannan Sadhasivam, Yoshihiro Shimoda, Rob Herring, Lorenzo Pieralisi, Krzysztof Wilczyński, Nobuhiro Iwamatsu Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko, linux-pci, dmaengine, linux-kernel, Bjorn Helgaas, linux-arm-kernel 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 <helgaas@kernel.org> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH RESEND v3 07/10] PCI: visconti: Convert to using generic resources getter 2023-04-11 3:39 ` [PATCH RESEND v3 07/10] PCI: visconti: Convert to using generic resources getter Serge Semin @ 2023-04-11 11:19 ` Manivannan Sadhasivam 2023-04-11 17:10 ` Serge Semin 0 siblings, 1 reply; 3+ messages in thread From: Manivannan Sadhasivam @ 2023-04-11 11:19 UTC (permalink / raw) To: Serge Semin, g Cc: Bjorn Helgaas, Lorenzo Pieralisi, Cai Huoqing, Jingoo Han, Gustavo Pimentel, Vinod Koul, Yoshihiro Shimoda, Rob Herring, Lorenzo Pieralisi, Krzysztof Wilczyński, Nobuhiro Iwamatsu, Serge Semin, Alexey Malahov, Pavel Parkhomenko, linux-pci, dmaengine, linux-kernel, Bjorn Helgaas, linux-arm-kernel 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 <helgaas@kernel.org> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > --- > 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 > > -- மணிவண்ணன் சதாசிவம் _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH RESEND v3 07/10] PCI: visconti: Convert to using generic resources getter 2023-04-11 11:19 ` Manivannan Sadhasivam @ 2023-04-11 17:10 ` Serge Semin 0 siblings, 0 replies; 3+ messages in thread From: Serge Semin @ 2023-04-11 17:10 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Serge Semin, g, Bjorn Helgaas, Lorenzo Pieralisi, Cai Huoqing, Jingoo Han, Gustavo Pimentel, Vinod Koul, Yoshihiro Shimoda, Rob Herring, Lorenzo Pieralisi, Krzysztof Wilczyński, Nobuhiro Iwamatsu, Alexey Malahov, Pavel Parkhomenko, linux-pci, dmaengine, linux-kernel, Bjorn Helgaas, linux-arm-kernel 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 <helgaas@kernel.org> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > 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 > > > > > > -- > மணிவண்ணன் சதாசிவம் _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-04-11 17:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20230411033928.30397-1-Sergey.Semin@baikalelectronics.ru>
2023-04-11 3:39 ` [PATCH RESEND v3 07/10] PCI: visconti: Convert to using generic resources getter Serge Semin
2023-04-11 11:19 ` Manivannan Sadhasivam
2023-04-11 17:10 ` Serge Semin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).