| Message ID | 1456248071-30547-1-git-send-email-georgi.djakov@linaro.org (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Delegated to: | Andy Gross |
| Headers | show |
On 23/02/16 17:21, Georgi Djakov wrote: > When the SMEM is probed it defers as it depends on the hardware lock, which > is not available yet. But the SMD bus and RPM regulators and clocks depend > on SMEM and they defer too. The problem with this is that the order of > registering the devices is not optimal and also we may end with messed > up serial console as the RPM clocks are not registered yet.. I noticed the same issue but was wondering why would we end up with messed up serial console? Could you add more details on why serial console is messed up? I thought, serial driver has nothing to do with the rpm clocks directly! --srini > > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > --- > arch/arm64/boot/dts/qcom/msm8916.dtsi | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi > index 7705207872a5..c497c7b1ae70 100644 > --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi > @@ -113,15 +113,6 @@ > }; > }; > > - smem { > - compatible = "qcom,smem"; > - > - memory-region = <&smem_mem>; > - qcom,rpm-msg-ram = <&rpm_msg_ram>; > - > - hwlocks = <&tcsr_mutex 3>; > - }; > - > soc: soc { > #address-cells = <1>; > #size-cells = <1>; > @@ -512,6 +503,16 @@ > }; > }; > > + smem { > + compatible = "qcom,smem"; > + > + memory-region = <&smem_mem>; > + qcom,rpm-msg-ram = <&rpm_msg_ram>; > + > + hwlocks = <&tcsr_mutex 3>; > + }; > + > + > smd { > compatible = "qcom,smd"; > > -- > 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 > -- 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 Tue, Feb 23, 2016 at 07:21:11PM +0200, Georgi Djakov wrote: > When the SMEM is probed it defers as it depends on the hardware lock, which > is not available yet. But the SMD bus and RPM regulators and clocks depend > on SMEM and they defer too. The problem with this is that the order of > registering the devices is not optimal and also we may end with messed > up serial console as the RPM clocks are not registered yet.. Re-ordering the DT is fragile at best, and not a real solution to this class of problem. Why can the drivers not defer probing in this case? Mark. > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > --- > arch/arm64/boot/dts/qcom/msm8916.dtsi | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi > index 7705207872a5..c497c7b1ae70 100644 > --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi > @@ -113,15 +113,6 @@ > }; > }; > > - smem { > - compatible = "qcom,smem"; > - > - memory-region = <&smem_mem>; > - qcom,rpm-msg-ram = <&rpm_msg_ram>; > - > - hwlocks = <&tcsr_mutex 3>; > - }; > - > soc: soc { > #address-cells = <1>; > #size-cells = <1>; > @@ -512,6 +503,16 @@ > }; > }; > > + smem { > + compatible = "qcom,smem"; > + > + memory-region = <&smem_mem>; > + qcom,rpm-msg-ram = <&rpm_msg_ram>; > + > + hwlocks = <&tcsr_mutex 3>; > + }; > + > + > smd { > compatible = "qcom,smd"; > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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 23.02.16 ?. 19:29, Srinivas Kandagatla wrote: > > > On 23/02/16 17:21, Georgi Djakov wrote: >> When the SMEM is probed it defers as it depends on the hardware lock, which >> is not available yet. But the SMD bus and RPM regulators and clocks depend >> on SMEM and they defer too. The problem with this is that the order of >> registering the devices is not optimal and also we may end with messed >> up serial console as the RPM clocks are not registered yet.. > I noticed the same issue but was wondering why would we end up with messed up serial console? > > Could you add more details on why serial console is messed up? > > I thought, serial driver has nothing to do with the rpm clocks directly! > If we don't have the rpm clocks registered, the uart clock is an orphan and when clk_get_rate() is called on orphan clocks it returns 0 as rate. In our case the msm_serial driver calls clk_get_rate() and gets 0 rate as the parent rpm clock has not registered yet. The result is that the baudrate is set incorrectly. BR, Georgi -- 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 Tue, Feb 23, 2016 at 08:47:56PM +0200, Georgi Djakov wrote: > On 23.02.16 ?. 19:29, Srinivas Kandagatla wrote: > > > > > > On 23/02/16 17:21, Georgi Djakov wrote: > >> When the SMEM is probed it defers as it depends on the hardware lock, which > >> is not available yet. But the SMD bus and RPM regulators and clocks depend > >> on SMEM and they defer too. The problem with this is that the order of > >> registering the devices is not optimal and also we may end with messed > >> up serial console as the RPM clocks are not registered yet.. > > I noticed the same issue but was wondering why would we end up with messed up serial console? > > > > Could you add more details on why serial console is messed up? > > > > I thought, serial driver has nothing to do with the rpm clocks directly! > > > > If we don't have the rpm clocks registered, the uart clock is an orphan > and when clk_get_rate() is called on orphan clocks it returns 0 as rate. > In our case the msm_serial driver calls clk_get_rate() and gets 0 rate > as the parent rpm clock has not registered yet. The result is that the > baudrate is set incorrectly. This isn't a probe defer issue w/ the SMEM and hwspinlock. That works properly. This is an issue with the msm_serial either not probe deferring to wait for the rpm clocks or not handling the case of the clk framework giving us a 'bogus' clock. Can we queue off the clk_get_rate being 0 to probe defer for the rpm clocks? (although that is hacky). 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 23/02/16 18:47, Georgi Djakov wrote: > On 23.02.16 ?. 19:29, Srinivas Kandagatla wrote: >> >> >> On 23/02/16 17:21, Georgi Djakov wrote: >>> When the SMEM is probed it defers as it depends on the hardware lock, which >>> is not available yet. But the SMD bus and RPM regulators and clocks depend >>> on SMEM and they defer too. The problem with this is that the order of >>> registering the devices is not optimal and also we may end with messed >>> up serial console as the RPM clocks are not registered yet.. >> I noticed the same issue but was wondering why would we end up with messed up serial console? >> >> Could you add more details on why serial console is messed up? >> >> I thought, serial driver has nothing to do with the rpm clocks directly! >> > > If we don't have the rpm clocks registered, the uart clock is an orphan > and when clk_get_rate() is called on orphan clocks it returns 0 as rate. Shouldn't the actual uart clk provider registration fail/defer probe due to missing parent in this case? --srini > In our case the msm_serial driver calls clk_get_rate() and gets 0 rate > as the parent rpm clock has not registered yet. The result is that the > baudrate is set incorrectly. > > BR, > Georgi > -- 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 23.02.16 ?. 21:28, Srinivas Kandagatla wrote: > > > On 23/02/16 18:47, Georgi Djakov wrote: >> On 23.02.16 ?. 19:29, Srinivas Kandagatla wrote: >>> >>> >>> On 23/02/16 17:21, Georgi Djakov wrote: >>>> When the SMEM is probed it defers as it depends on the hardware lock, which >>>> is not available yet. But the SMD bus and RPM regulators and clocks depend >>>> on SMEM and they defer too. The problem with this is that the order of >>>> registering the devices is not optimal and also we may end with messed >>>> up serial console as the RPM clocks are not registered yet.. >>> I noticed the same issue but was wondering why would we end up with messed up serial console? >>> >>> Could you add more details on why serial console is messed up? >>> >>> I thought, serial driver has nothing to do with the rpm clocks directly! >>> >> >> If we don't have the rpm clocks registered, the uart clock is an orphan >> and when clk_get_rate() is called on orphan clocks it returns 0 as rate. > Shouldn't the actual uart clk provider registration fail/defer probe due to missing parent in this case? > Yes, this is a known issue and people are currently working on it. http://www.spinics.net/lists/linux-clk/msg00065.html http://www.spinics.net/lists/arm-kernel/msg475910.html -- 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 Tue 23 Feb 09:21 PST 2016, Georgi Djakov wrote: > When the SMEM is probed it defers as it depends on the hardware lock, which > is not available yet. But the SMD bus and RPM regulators and clocks depend > on SMEM and they defer too. The problem with this is that the order of > registering the devices is not optimal and also we may end with messed > up serial console as the RPM clocks are not registered yet.. > > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> Not only do you need the probe order to be "correct", you also need some luck to have the smd code execute its two workers to initialise the rpmcc driver before your serial driver is probed. So I don't think we should merge this upstream. I tried this on my linux-next tree on db410c and it doesn't hide the problem for me. 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 02/23/2016 09:03 PM, Andy Gross wrote: > On Tue, Feb 23, 2016 at 08:47:56PM +0200, Georgi Djakov wrote: >> On 23.02.16 ?. 19:29, Srinivas Kandagatla wrote: >>> >>> >>> On 23/02/16 17:21, Georgi Djakov wrote: >>>> When the SMEM is probed it defers as it depends on the hardware lock, which >>>> is not available yet. But the SMD bus and RPM regulators and clocks depend >>>> on SMEM and they defer too. The problem with this is that the order of >>>> registering the devices is not optimal and also we may end with messed >>>> up serial console as the RPM clocks are not registered yet.. >>> I noticed the same issue but was wondering why would we end up with messed up serial console? >>> >>> Could you add more details on why serial console is messed up? >>> >>> I thought, serial driver has nothing to do with the rpm clocks directly! >>> >> >> If we don't have the rpm clocks registered, the uart clock is an orphan >> and when clk_get_rate() is called on orphan clocks it returns 0 as rate. >> In our case the msm_serial driver calls clk_get_rate() and gets 0 rate >> as the parent rpm clock has not registered yet. The result is that the >> baudrate is set incorrectly. > > This isn't a probe defer issue w/ the SMEM and hwspinlock. That works properly. Ok, agree. > This is an issue with the msm_serial either not probe deferring to wait for the > rpm clocks or not handling the case of the clk framework giving us a 'bogus' > clock. Can we queue off the clk_get_rate being 0 to probe defer for the rpm > clocks? (although that is hacky). The proper solution would be to handle this in the clock framework. -- 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/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi index 7705207872a5..c497c7b1ae70 100644 --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi @@ -113,15 +113,6 @@ }; }; - smem { - compatible = "qcom,smem"; - - memory-region = <&smem_mem>; - qcom,rpm-msg-ram = <&rpm_msg_ram>; - - hwlocks = <&tcsr_mutex 3>; - }; - soc: soc { #address-cells = <1>; #size-cells = <1>; @@ -512,6 +503,16 @@ }; }; + smem { + compatible = "qcom,smem"; + + memory-region = <&smem_mem>; + qcom,rpm-msg-ram = <&rpm_msg_ram>; + + hwlocks = <&tcsr_mutex 3>; + }; + + smd { compatible = "qcom,smd";
When the SMEM is probed it defers as it depends on the hardware lock, which is not available yet. But the SMD bus and RPM regulators and clocks depend on SMEM and they defer too. The problem with this is that the order of registering the devices is not optimal and also we may end with messed up serial console as the RPM clocks are not registered yet.. Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> --- arch/arm64/boot/dts/qcom/msm8916.dtsi | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) -- 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