| Message ID | 1656062391-14567-2-git-send-email-quic_krichai@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: qcom: sc7280: add missing clocks | expand |
[+cc Prasad, who added sc7280] On Fri, Jun 24, 2022 at 02:49:49PM +0530, Krishna chaitanya chundru wrote: > Add missing clocks in the pcie driver. s/pcie/PCIe/ as in subject. Also, please mention the names of the clocks here, too. Does this add new functionality, or did sc7280 just never work in the first place? If this fixes a problem, it's nice to mention the user-visible symptom in the commit log to make it easier to connect this fix with the problem. Looks like sc7280 was added by Prasad with b89ff410253d ("PCI: qcom: Replace ops with struct pcie_cfg in pcie match data"), and I assume it was tested at that time. If I had noticed, I would have asked that b89ff410253d be a strict conversion to struct qcom_pcie_cfg with no functional changes, with sc7280 support being added in a separate patch by itself. > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > --- > drivers/pci/controller/dwc/pcie-qcom.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 2ea1375..a7202f0 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -1548,7 +1548,10 @@ static const struct qcom_pcie_cfg sm8450_pcie1_cfg = { > static const struct qcom_pcie_cfg sc7280_cfg = { > .ops = &ops_1_9_0, > .has_tbu_clk = true, > + .has_ddrss_sf_tbu_clk = true, > .pipe_clk_need_muxing = true, > + .has_aggre0_clk = true, > + .has_aggre1_clk = true, > }; > > static const struct qcom_pcie_cfg sc8180x_cfg = { > -- > 2.7.4 >
[update Prasad's email address since pmaliset@codeaurora.org bounced] On Fri, Jun 24, 2022 at 12:05:03PM -0500, Bjorn Helgaas wrote: > [+cc Prasad, who added sc7280] > > On Fri, Jun 24, 2022 at 02:49:49PM +0530, Krishna chaitanya chundru wrote: > > Add missing clocks in the pcie driver. > > s/pcie/PCIe/ as in subject. Also, please mention the names of the > clocks here, too. > > Does this add new functionality, or did sc7280 just never work in the > first place? > > If this fixes a problem, it's nice to mention the user-visible symptom > in the commit log to make it easier to connect this fix with the > problem. > > Looks like sc7280 was added by Prasad with b89ff410253d ("PCI: qcom: > Replace ops with struct pcie_cfg in pcie match data"), and I assume it > was tested at that time. > > If I had noticed, I would have asked that b89ff410253d be a strict > conversion to struct qcom_pcie_cfg with no functional changes, with > sc7280 support being added in a separate patch by itself. > > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > > --- > > drivers/pci/controller/dwc/pcie-qcom.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > > index 2ea1375..a7202f0 100644 > > --- a/drivers/pci/controller/dwc/pcie-qcom.c > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > > @@ -1548,7 +1548,10 @@ static const struct qcom_pcie_cfg sm8450_pcie1_cfg = { > > static const struct qcom_pcie_cfg sc7280_cfg = { > > .ops = &ops_1_9_0, > > .has_tbu_clk = true, > > + .has_ddrss_sf_tbu_clk = true, > > .pipe_clk_need_muxing = true, > > + .has_aggre0_clk = true, > > + .has_aggre1_clk = true, > > }; > > > > static const struct qcom_pcie_cfg sc8180x_cfg = { > > -- > > 2.7.4 > >
On 6/24/2022 10:35 PM, Bjorn Helgaas wrote: > [+cc Prasad, who added sc7280] > On Fri, Jun 24, 2022 at 02:49:49PM +0530, Krishna chaitanya chundru wrote: >> Add missing clocks in the pcie driver. > s/pcie/PCIe/ as in subject. Also, please mention the names of the > clocks here, too. Will correct in next patch. > Does this add new functionality, or did sc7280 just never work in the When ever system goes to suspend the pcie link is also going down even though we didn't unvote any clocks/regulators. When we added these two missing clocks, the link is stable now after suspend and resume also. We didn't test suspend/resume previously that is reason we didn't see any issue. > first place? > > If this fixes a problem, it's nice to mention the user-visible symptom > in the commit log to make it easier to connect this fix with the > problem. Will update in the next patch. > Looks like sc7280 was added by Prasad with b89ff410253d ("PCI: qcom: > Replace ops with struct pcie_cfg in pcie match data"), and I assume it > was tested at that time. > > If I had noticed, I would have asked that b89ff410253d be a strict > conversion to struct qcom_pcie_cfg with no functional changes, with > sc7280 support being added in a separate patch by itself. FYI. Prasad has left our org and no longer associated with our org now. >> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> >> --- >> drivers/pci/controller/dwc/pcie-qcom.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c >> index 2ea1375..a7202f0 100644 >> --- a/drivers/pci/controller/dwc/pcie-qcom.c >> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >> @@ -1548,7 +1548,10 @@ static const struct qcom_pcie_cfg sm8450_pcie1_cfg = { >> static const struct qcom_pcie_cfg sc7280_cfg = { >> .ops = &ops_1_9_0, >> .has_tbu_clk = true, >> + .has_ddrss_sf_tbu_clk = true, >> .pipe_clk_need_muxing = true, >> + .has_aggre0_clk = true, >> + .has_aggre1_clk = true, >> }; >> >> static const struct qcom_pcie_cfg sc8180x_cfg = { >> -- >> 2.7.4 >>
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 2ea1375..a7202f0 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -1548,7 +1548,10 @@ static const struct qcom_pcie_cfg sm8450_pcie1_cfg = { static const struct qcom_pcie_cfg sc7280_cfg = { .ops = &ops_1_9_0, .has_tbu_clk = true, + .has_ddrss_sf_tbu_clk = true, .pipe_clk_need_muxing = true, + .has_aggre0_clk = true, + .has_aggre1_clk = true, }; static const struct qcom_pcie_cfg sc8180x_cfg = {
Add missing clocks in the pcie driver. Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> --- drivers/pci/controller/dwc/pcie-qcom.c | 3 +++ 1 file changed, 3 insertions(+)