| Message ID | 20211208171442.1327689-5-dmitry.baryshkov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | qcom: add support for PCIe0 on SM8450 platform | expand |
On Wed, Dec 08, 2021 at 08:14:36PM +0300, Dmitry Baryshkov wrote: > In preparation to adding more flags to configuration data, use struct > qcom_pcie_cfg directly inside struct qcom_pcie, rather than duplicating > all its fields. This would save us from the boilerplate code that just > copies flags values from one sruct to another one. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/pci/controller/dwc/pcie-qcom.c | 39 +++++++++++--------------- > 1 file changed, 17 insertions(+), 22 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 1c3d1116bb60..51a0475173fb 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -204,8 +204,7 @@ struct qcom_pcie { > union qcom_pcie_resources res; > struct phy *phy; > struct gpio_desc *reset; > - const struct qcom_pcie_ops *ops; > - unsigned int pipe_clk_need_muxing:1; > + const struct qcom_pcie_cfg *cfg; There is no change in this patch that adds "pipe_clk_need_muxing" to qcom_pcie_cfg. Thanks, Mani > }; > > #define to_qcom_pcie(x) dev_get_drvdata((x)->dev) > @@ -229,8 +228,8 @@ static int qcom_pcie_start_link(struct dw_pcie *pci) > struct qcom_pcie *pcie = to_qcom_pcie(pci); > > /* Enable Link Training state machine */ > - if (pcie->ops->ltssm_enable) > - pcie->ops->ltssm_enable(pcie); > + if (pcie->cfg->ops->ltssm_enable) > + pcie->cfg->ops->ltssm_enable(pcie); > > return 0; > } > @@ -1176,7 +1175,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) > if (ret < 0) > return ret; > > - if (pcie->pipe_clk_need_muxing) { > + if (pcie->cfg->pipe_clk_need_muxing) { > res->pipe_clk_src = devm_clk_get(dev, "pipe_mux"); > if (IS_ERR(res->pipe_clk_src)) > return PTR_ERR(res->pipe_clk_src); > @@ -1209,7 +1208,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) > } > > /* Set TCXO as clock source for pcie_pipe_clk_src */ > - if (pcie->pipe_clk_need_muxing) > + if (pcie->cfg->pipe_clk_need_muxing) > clk_set_parent(res->pipe_clk_src, res->ref_clk_src); > > ret = clk_bulk_prepare_enable(res->num_clks, res->clks); > @@ -1284,7 +1283,7 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie) > struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0; > > /* Set pipe clock as clock source for pcie_pipe_clk_src */ > - if (pcie->pipe_clk_need_muxing) > + if (pcie->cfg->pipe_clk_need_muxing) > clk_set_parent(res->pipe_clk_src, res->phy_pipe_clk); > > return clk_prepare_enable(res->pipe_clk); > @@ -1384,7 +1383,7 @@ static int qcom_pcie_host_init(struct pcie_port *pp) > > qcom_ep_reset_assert(pcie); > > - ret = pcie->ops->init(pcie); > + ret = pcie->cfg->ops->init(pcie); > if (ret) > return ret; > > @@ -1392,16 +1391,16 @@ static int qcom_pcie_host_init(struct pcie_port *pp) > if (ret) > goto err_deinit; > > - if (pcie->ops->post_init) { > - ret = pcie->ops->post_init(pcie); > + if (pcie->cfg->ops->post_init) { > + ret = pcie->cfg->ops->post_init(pcie); > if (ret) > goto err_disable_phy; > } > > qcom_ep_reset_deassert(pcie); > > - if (pcie->ops->config_sid) { > - ret = pcie->ops->config_sid(pcie); > + if (pcie->cfg->ops->config_sid) { > + ret = pcie->cfg->ops->config_sid(pcie); > if (ret) > goto err; > } > @@ -1410,12 +1409,12 @@ static int qcom_pcie_host_init(struct pcie_port *pp) > > err: > qcom_ep_reset_assert(pcie); > - if (pcie->ops->post_deinit) > - pcie->ops->post_deinit(pcie); > + if (pcie->cfg->ops->post_deinit) > + pcie->cfg->ops->post_deinit(pcie); > err_disable_phy: > phy_power_off(pcie->phy); > err_deinit: > - pcie->ops->deinit(pcie); > + pcie->cfg->ops->deinit(pcie); > > return ret; > } > @@ -1531,7 +1530,6 @@ static int qcom_pcie_probe(struct platform_device *pdev) > struct pcie_port *pp; > struct dw_pcie *pci; > struct qcom_pcie *pcie; > - const struct qcom_pcie_cfg *pcie_cfg; > int ret; > > pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); > @@ -1553,15 +1551,12 @@ static int qcom_pcie_probe(struct platform_device *pdev) > > pcie->pci = pci; > > - pcie_cfg = of_device_get_match_data(dev); > - if (!pcie_cfg || !pcie_cfg->ops) { > + pcie->cfg = of_device_get_match_data(dev); > + if (!pcie->cfg || !pcie->cfg->ops) { > dev_err(dev, "Invalid platform data\n"); > return -EINVAL; > } > > - pcie->ops = pcie_cfg->ops; > - pcie->pipe_clk_need_muxing = pcie_cfg->pipe_clk_need_muxing; > - > pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH); > if (IS_ERR(pcie->reset)) { > ret = PTR_ERR(pcie->reset); > @@ -1586,7 +1581,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) > goto err_pm_runtime_put; > } > > - ret = pcie->ops->get_resources(pcie); > + ret = pcie->cfg->ops->get_resources(pcie); > if (ret) > goto err_pm_runtime_put; > > -- > 2.33.0 >
On 10/12/2021 14:15, Manivannan Sadhasivam wrote: > On Wed, Dec 08, 2021 at 08:14:36PM +0300, Dmitry Baryshkov wrote: >> In preparation to adding more flags to configuration data, use struct >> qcom_pcie_cfg directly inside struct qcom_pcie, rather than duplicating >> all its fields. This would save us from the boilerplate code that just >> copies flags values from one sruct to another one. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> drivers/pci/controller/dwc/pcie-qcom.c | 39 +++++++++++--------------- >> 1 file changed, 17 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c >> index 1c3d1116bb60..51a0475173fb 100644 >> --- a/drivers/pci/controller/dwc/pcie-qcom.c >> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >> @@ -204,8 +204,7 @@ struct qcom_pcie { >> union qcom_pcie_resources res; >> struct phy *phy; >> struct gpio_desc *reset; >> - const struct qcom_pcie_ops *ops; >> - unsigned int pipe_clk_need_muxing:1; >> + const struct qcom_pcie_cfg *cfg; > > There is no change in this patch that adds "pipe_clk_need_muxing" to > qcom_pcie_cfg. pipe_clk_need_muxing is already a part of the qcom_pcie_cfg structure. > Thanks, > Mani > >> }; >> >> #define to_qcom_pcie(x) dev_get_drvdata((x)->dev) >> @@ -229,8 +228,8 @@ static int qcom_pcie_start_link(struct dw_pcie *pci) >> struct qcom_pcie *pcie = to_qcom_pcie(pci); >> >> /* Enable Link Training state machine */ >> - if (pcie->ops->ltssm_enable) >> - pcie->ops->ltssm_enable(pcie); >> + if (pcie->cfg->ops->ltssm_enable) >> + pcie->cfg->ops->ltssm_enable(pcie); >> >> return 0; >> } >> @@ -1176,7 +1175,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) >> if (ret < 0) >> return ret; >> >> - if (pcie->pipe_clk_need_muxing) { >> + if (pcie->cfg->pipe_clk_need_muxing) { >> res->pipe_clk_src = devm_clk_get(dev, "pipe_mux"); >> if (IS_ERR(res->pipe_clk_src)) >> return PTR_ERR(res->pipe_clk_src); >> @@ -1209,7 +1208,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) >> } >> >> /* Set TCXO as clock source for pcie_pipe_clk_src */ >> - if (pcie->pipe_clk_need_muxing) >> + if (pcie->cfg->pipe_clk_need_muxing) >> clk_set_parent(res->pipe_clk_src, res->ref_clk_src); >> >> ret = clk_bulk_prepare_enable(res->num_clks, res->clks); >> @@ -1284,7 +1283,7 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie) >> struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0; >> >> /* Set pipe clock as clock source for pcie_pipe_clk_src */ >> - if (pcie->pipe_clk_need_muxing) >> + if (pcie->cfg->pipe_clk_need_muxing) >> clk_set_parent(res->pipe_clk_src, res->phy_pipe_clk); >> >> return clk_prepare_enable(res->pipe_clk); >> @@ -1384,7 +1383,7 @@ static int qcom_pcie_host_init(struct pcie_port *pp) >> >> qcom_ep_reset_assert(pcie); >> >> - ret = pcie->ops->init(pcie); >> + ret = pcie->cfg->ops->init(pcie); >> if (ret) >> return ret; >> >> @@ -1392,16 +1391,16 @@ static int qcom_pcie_host_init(struct pcie_port *pp) >> if (ret) >> goto err_deinit; >> >> - if (pcie->ops->post_init) { >> - ret = pcie->ops->post_init(pcie); >> + if (pcie->cfg->ops->post_init) { >> + ret = pcie->cfg->ops->post_init(pcie); >> if (ret) >> goto err_disable_phy; >> } >> >> qcom_ep_reset_deassert(pcie); >> >> - if (pcie->ops->config_sid) { >> - ret = pcie->ops->config_sid(pcie); >> + if (pcie->cfg->ops->config_sid) { >> + ret = pcie->cfg->ops->config_sid(pcie); >> if (ret) >> goto err; >> } >> @@ -1410,12 +1409,12 @@ static int qcom_pcie_host_init(struct pcie_port *pp) >> >> err: >> qcom_ep_reset_assert(pcie); >> - if (pcie->ops->post_deinit) >> - pcie->ops->post_deinit(pcie); >> + if (pcie->cfg->ops->post_deinit) >> + pcie->cfg->ops->post_deinit(pcie); >> err_disable_phy: >> phy_power_off(pcie->phy); >> err_deinit: >> - pcie->ops->deinit(pcie); >> + pcie->cfg->ops->deinit(pcie); >> >> return ret; >> } >> @@ -1531,7 +1530,6 @@ static int qcom_pcie_probe(struct platform_device *pdev) >> struct pcie_port *pp; >> struct dw_pcie *pci; >> struct qcom_pcie *pcie; >> - const struct qcom_pcie_cfg *pcie_cfg; >> int ret; >> >> pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); >> @@ -1553,15 +1551,12 @@ static int qcom_pcie_probe(struct platform_device *pdev) >> >> pcie->pci = pci; >> >> - pcie_cfg = of_device_get_match_data(dev); >> - if (!pcie_cfg || !pcie_cfg->ops) { >> + pcie->cfg = of_device_get_match_data(dev); >> + if (!pcie->cfg || !pcie->cfg->ops) { >> dev_err(dev, "Invalid platform data\n"); >> return -EINVAL; >> } >> >> - pcie->ops = pcie_cfg->ops; >> - pcie->pipe_clk_need_muxing = pcie_cfg->pipe_clk_need_muxing; >> - >> pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH); >> if (IS_ERR(pcie->reset)) { >> ret = PTR_ERR(pcie->reset); >> @@ -1586,7 +1581,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) >> goto err_pm_runtime_put; >> } >> >> - ret = pcie->ops->get_resources(pcie); >> + ret = pcie->cfg->ops->get_resources(pcie); >> if (ret) >> goto err_pm_runtime_put; >> >> -- >> 2.33.0 >>
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 1c3d1116bb60..51a0475173fb 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -204,8 +204,7 @@ struct qcom_pcie { union qcom_pcie_resources res; struct phy *phy; struct gpio_desc *reset; - const struct qcom_pcie_ops *ops; - unsigned int pipe_clk_need_muxing:1; + const struct qcom_pcie_cfg *cfg; }; #define to_qcom_pcie(x) dev_get_drvdata((x)->dev) @@ -229,8 +228,8 @@ static int qcom_pcie_start_link(struct dw_pcie *pci) struct qcom_pcie *pcie = to_qcom_pcie(pci); /* Enable Link Training state machine */ - if (pcie->ops->ltssm_enable) - pcie->ops->ltssm_enable(pcie); + if (pcie->cfg->ops->ltssm_enable) + pcie->cfg->ops->ltssm_enable(pcie); return 0; } @@ -1176,7 +1175,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) if (ret < 0) return ret; - if (pcie->pipe_clk_need_muxing) { + if (pcie->cfg->pipe_clk_need_muxing) { res->pipe_clk_src = devm_clk_get(dev, "pipe_mux"); if (IS_ERR(res->pipe_clk_src)) return PTR_ERR(res->pipe_clk_src); @@ -1209,7 +1208,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) } /* Set TCXO as clock source for pcie_pipe_clk_src */ - if (pcie->pipe_clk_need_muxing) + if (pcie->cfg->pipe_clk_need_muxing) clk_set_parent(res->pipe_clk_src, res->ref_clk_src); ret = clk_bulk_prepare_enable(res->num_clks, res->clks); @@ -1284,7 +1283,7 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie) struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0; /* Set pipe clock as clock source for pcie_pipe_clk_src */ - if (pcie->pipe_clk_need_muxing) + if (pcie->cfg->pipe_clk_need_muxing) clk_set_parent(res->pipe_clk_src, res->phy_pipe_clk); return clk_prepare_enable(res->pipe_clk); @@ -1384,7 +1383,7 @@ static int qcom_pcie_host_init(struct pcie_port *pp) qcom_ep_reset_assert(pcie); - ret = pcie->ops->init(pcie); + ret = pcie->cfg->ops->init(pcie); if (ret) return ret; @@ -1392,16 +1391,16 @@ static int qcom_pcie_host_init(struct pcie_port *pp) if (ret) goto err_deinit; - if (pcie->ops->post_init) { - ret = pcie->ops->post_init(pcie); + if (pcie->cfg->ops->post_init) { + ret = pcie->cfg->ops->post_init(pcie); if (ret) goto err_disable_phy; } qcom_ep_reset_deassert(pcie); - if (pcie->ops->config_sid) { - ret = pcie->ops->config_sid(pcie); + if (pcie->cfg->ops->config_sid) { + ret = pcie->cfg->ops->config_sid(pcie); if (ret) goto err; } @@ -1410,12 +1409,12 @@ static int qcom_pcie_host_init(struct pcie_port *pp) err: qcom_ep_reset_assert(pcie); - if (pcie->ops->post_deinit) - pcie->ops->post_deinit(pcie); + if (pcie->cfg->ops->post_deinit) + pcie->cfg->ops->post_deinit(pcie); err_disable_phy: phy_power_off(pcie->phy); err_deinit: - pcie->ops->deinit(pcie); + pcie->cfg->ops->deinit(pcie); return ret; } @@ -1531,7 +1530,6 @@ static int qcom_pcie_probe(struct platform_device *pdev) struct pcie_port *pp; struct dw_pcie *pci; struct qcom_pcie *pcie; - const struct qcom_pcie_cfg *pcie_cfg; int ret; pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); @@ -1553,15 +1551,12 @@ static int qcom_pcie_probe(struct platform_device *pdev) pcie->pci = pci; - pcie_cfg = of_device_get_match_data(dev); - if (!pcie_cfg || !pcie_cfg->ops) { + pcie->cfg = of_device_get_match_data(dev); + if (!pcie->cfg || !pcie->cfg->ops) { dev_err(dev, "Invalid platform data\n"); return -EINVAL; } - pcie->ops = pcie_cfg->ops; - pcie->pipe_clk_need_muxing = pcie_cfg->pipe_clk_need_muxing; - pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH); if (IS_ERR(pcie->reset)) { ret = PTR_ERR(pcie->reset); @@ -1586,7 +1581,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_pm_runtime_put; } - ret = pcie->ops->get_resources(pcie); + ret = pcie->cfg->ops->get_resources(pcie); if (ret) goto err_pm_runtime_put;
In preparation to adding more flags to configuration data, use struct qcom_pcie_cfg directly inside struct qcom_pcie, rather than duplicating all its fields. This would save us from the boilerplate code that just copies flags values from one sruct to another one. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/pci/controller/dwc/pcie-qcom.c | 39 +++++++++++--------------- 1 file changed, 17 insertions(+), 22 deletions(-)