Message ID | 20170215045157.11659-1-j.neuschaefer@gmx.net (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Andy Gross |
Headers | show |
On Wed, Feb 15, 2017 at 05:51:56AM +0100, Jonathan Neuschäfer wrote: > Without this patch (and with CONFIG_QCOM_ADSP_PIL), I get this error: > > [ 0.711529] qcom_adsp_pil adsp-pil: failed to get xo clock > [ 0.711540] remoteproc remoteproc0: releasing adsp-pil > > With this patch, adsp-pil can initialize correctly. > > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> > > --- > NOTE: I don't know if I actually chose the right clock source. > Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt > suggests that adsp-pil's xo should be controlled by the RPM > processor; Existing devicetrees and a recent patch to msm8996.dtsi > use &xo_board, though: > https://www.spinics.net/lists/linux-arm-msm/msg25711.html > --- > arch/arm/boot/dts/qcom-msm8974.dtsi | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi > index d3e1a61b8671..cccd8cba8872 100644 > --- a/arch/arm/boot/dts/qcom-msm8974.dtsi > +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi > @@ -250,6 +250,9 @@ > > cx-supply = <&pm8841_s2>; > > + clocks = <&xo_board>; > + clock-names = "xo"; > + I have to wonder if this wasn't done specifically so that the board specific DTS file would specify the XO clock. If you do it here, it applies to all boards using msm8974. Maybe someone can answer that question. Adding Bjorn to the CC. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 20 Feb 14:00 PST 2017, Andy Gross wrote: > On Wed, Feb 15, 2017 at 05:51:56AM +0100, Jonathan Neuschäfer wrote: > > Without this patch (and with CONFIG_QCOM_ADSP_PIL), I get this error: > > > > [ 0.711529] qcom_adsp_pil adsp-pil: failed to get xo clock > > [ 0.711540] remoteproc remoteproc0: releasing adsp-pil > > > > With this patch, adsp-pil can initialize correctly. > > > > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > > --- > > NOTE: I don't know if I actually chose the right clock source. > > Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt > > suggests that adsp-pil's xo should be controlled by the RPM > > processor; Existing devicetrees and a recent patch to msm8996.dtsi > > use &xo_board, though: > > https://www.spinics.net/lists/linux-arm-msm/msg25711.html The ADSP is running off a clock derived from XO, as such a vote for XO must be held while the ADSP is running. It's possible the all application CPUs hit idle after launching the ADSP, but before the ADSP firmware can communicate this need with the RPM, which would result in the votes hitting 0 and the RPM might gate the XO. To prevent this the ADSP remoteproc driver must hold an extra vote with the RPM until the ADSP signals that it has cast its vote. Unfortunately there are some issues with nested probe deferral in the clock framework, so we can't accurately describe the XO today and as we're still missing some last pieces of idle mechanism causing above events we can survive by faking it and using xo_board - for a little bit longer... > > --- > > arch/arm/boot/dts/qcom-msm8974.dtsi | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi > > index d3e1a61b8671..cccd8cba8872 100644 > > --- a/arch/arm/boot/dts/qcom-msm8974.dtsi > > +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi > > @@ -250,6 +250,9 @@ > > > > cx-supply = <&pm8841_s2>; > > > > + clocks = <&xo_board>; > > + clock-names = "xo"; > > + > > I have to wonder if this wasn't done specifically so that the board specific DTS > file would specify the XO clock. If you do it here, it applies to all boards > using msm8974. > The reason for the adsp node missing the xo reference is that it wasn't required originally. It is now and I didn't feel we had wide spread usage and omitted backwards compatibility in the driver > Maybe someone can answer that question. Adding Bjorn to the CC. > I forgot that I had sent you the original patch and hence didn't send you a fix for it. Sorry about that, perhaps we should flag this for stable? Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 20, 2017 at 09:45:04PM -0800, Bjorn Andersson wrote: > On Mon 20 Feb 14:00 PST 2017, Andy Gross wrote: > > > On Wed, Feb 15, 2017 at 05:51:56AM +0100, Jonathan Neuschäfer wrote: > > > Without this patch (and with CONFIG_QCOM_ADSP_PIL), I get this error: > > > > > > [ 0.711529] qcom_adsp_pil adsp-pil: failed to get xo clock > > > [ 0.711540] remoteproc remoteproc0: releasing adsp-pil > > > > > > With this patch, adsp-pil can initialize correctly. > > > > > > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Thanks! > The ADSP is running off a clock derived from XO, as such a vote for XO > must be held while the ADSP is running. It's possible the all > application CPUs hit idle after launching the ADSP, but before the ADSP > firmware can communicate this need with the RPM, which would result in > the votes hitting 0 and the RPM might gate the XO. > > To prevent this the ADSP remoteproc driver must hold an extra vote with > the RPM until the ADSP signals that it has cast its vote. > > Unfortunately there are some issues with nested probe deferral in the > clock framework, so we can't accurately describe the XO today and as > we're still missing some last pieces of idle mechanism causing above > events we can survive by faking it and using xo_board - for a little > bit longer... Ok, that makes sense. Jonathan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi index d3e1a61b8671..cccd8cba8872 100644 --- a/arch/arm/boot/dts/qcom-msm8974.dtsi +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi @@ -250,6 +250,9 @@ cx-supply = <&pm8841_s2>; + clocks = <&xo_board>; + clock-names = "xo"; + memory-region = <&adsp_region>; qcom,smem-states = <&adsp_smp2p_out 0>;
Without this patch (and with CONFIG_QCOM_ADSP_PIL), I get this error: [ 0.711529] qcom_adsp_pil adsp-pil: failed to get xo clock [ 0.711540] remoteproc remoteproc0: releasing adsp-pil With this patch, adsp-pil can initialize correctly. Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> --- NOTE: I don't know if I actually chose the right clock source. Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt suggests that adsp-pil's xo should be controlled by the RPM processor; Existing devicetrees and a recent patch to msm8996.dtsi use &xo_board, though: https://www.spinics.net/lists/linux-arm-msm/msg25711.html --- arch/arm/boot/dts/qcom-msm8974.dtsi | 3 +++ 1 file changed, 3 insertions(+)