| Message ID | 20250303225521.1780611-3-vladimir.zapolskiy@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: qcom: sm8550: camcc: Manage MMCX and MXC | expand |
On 03/03/2025 23:53, Dmitry Baryshkov wrote: > On Tue, Mar 04, 2025 at 12:55:21AM +0200, Vladimir Zapolskiy wrote: >> SM8550 Camera Clock Controller shall enable both MXC and MMCX power >> domains. > > Are those really required to access the registers of the cammcc? Or is > one of those (MXC?) required to setup PLLs? Also, is this applicable > only to sm8550 or to other similar clock controllers? > >> >> Fixes: e271b59e39a6f ("arm64: dts: qcom: sm8550: Add camera clock controller") >> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> >> --- >> arch/arm64/boot/dts/qcom/sm8550.dtsi | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi >> index d02d80d731b9..d22b1753d521 100644 >> --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi >> @@ -3329,7 +3329,8 @@ camcc: clock-controller@ade0000 { >> <&bi_tcxo_div2>, >> <&bi_tcxo_ao_div2>, >> <&sleep_clk>; >> - power-domains = <&rpmhpd SM8550_MMCX>; >> + power-domains = <&rpmhpd SM8550_MXC>, >> + <&rpmhpd SM8550_MMCX>; >> required-opps = <&rpmhpd_opp_low_svs>; >> #clock-cells = <1>; >> #reset-cells = <1>; >> -- >> 2.43.0 >> > I think both of these are required. Its a pattern we see again and again with videocc and camcc controllers. The GDSCs and => the hard-coded always on PLLs need to ensure these rails are on. --- bod
On 3/4/2025 2:10 PM, Dmitry Baryshkov wrote: > On Tue, 4 Mar 2025 at 09:37, Vladimir Zapolskiy > <vladimir.zapolskiy@linaro.org> wrote: >> >> On 3/4/25 01:53, Dmitry Baryshkov wrote: >>> On Tue, Mar 04, 2025 at 12:55:21AM +0200, Vladimir Zapolskiy wrote: >>>> SM8550 Camera Clock Controller shall enable both MXC and MMCX power >>>> domains. >>> >>> Are those really required to access the registers of the cammcc? Or is >>> one of those (MXC?) required to setup PLLs? Also, is this applicable >>> only to sm8550 or to other similar clock controllers? >> >> Due to the described problem I experience a fatal CPU stall on SM8550-QRD, >> not on any SM8450 or SM8650 powered board for instance, however it does >> not exclude an option that the problem has to be fixed for other clock >> controllers, but it's Qualcomm to confirm any other touched platforms, > > Please work with Taniya to identify used power domains. > CAMCC requires both MMCX and MXC to be functional. >> for instance x1e80100-camcc has it resolved right at the beginning. >> >> To my understanding here 'required-opps' shall also be generalized, so >> the done copy from x1e80100-camcc was improper, and the latter dt-binding >> should be fixed. > > Yes > required-opps is not mandatory for MXC as we ensure that MxC would never hit retention. https://lore.kernel.org/r/20240625-avoid_mxc_retention-v2-1-af9c2f549a5f@quicinc.com > >
On 3/4/2025 5:23 AM, Dmitry Baryshkov wrote: > @@ -3329,7 +3329,8 @@ camcc: clock-controller@ade0000 { > <&bi_tcxo_div2>, > <&bi_tcxo_ao_div2>, > <&sleep_clk>; > - power-domains = <&rpmhpd SM8550_MMCX>; > + power-domains = <&rpmhpd SM8550_MXC>, > + <&rpmhpd SM8550_MMCX>; power-domains = <&rpmhpd SM8550_MMCX>, <&rpmhpd SM8550_MXC>; Once the above change is incorporated, please add Reviewed-by: Taniya Das <quic_tdas@quicinc.com>
On Thu, Mar 13, 2025 at 10:09:05AM +0530, Taniya Das wrote: > > > On 3/4/2025 2:10 PM, Dmitry Baryshkov wrote: > > On Tue, 4 Mar 2025 at 09:37, Vladimir Zapolskiy > > <vladimir.zapolskiy@linaro.org> wrote: > >> > >> On 3/4/25 01:53, Dmitry Baryshkov wrote: > >>> On Tue, Mar 04, 2025 at 12:55:21AM +0200, Vladimir Zapolskiy wrote: > >>>> SM8550 Camera Clock Controller shall enable both MXC and MMCX power > >>>> domains. > >>> > >>> Are those really required to access the registers of the cammcc? Or is > >>> one of those (MXC?) required to setup PLLs? Also, is this applicable > >>> only to sm8550 or to other similar clock controllers? > >> > >> Due to the described problem I experience a fatal CPU stall on SM8550-QRD, > >> not on any SM8450 or SM8650 powered board for instance, however it does > >> not exclude an option that the problem has to be fixed for other clock > >> controllers, but it's Qualcomm to confirm any other touched platforms, > > > > Please work with Taniya to identify used power domains. > > > > CAMCC requires both MMCX and MXC to be functional. > > >> for instance x1e80100-camcc has it resolved right at the beginning. > >> > >> To my understanding here 'required-opps' shall also be generalized, so > >> the done copy from x1e80100-camcc was improper, and the latter dt-binding > >> should be fixed. > > > > Yes > > > > required-opps is not mandatory for MXC as we ensure that MxC would never > hit retention. > > https://lore.kernel.org/r/20240625-avoid_mxc_retention-v2-1-af9c2f549a5f@quicinc.com Yes. And the code in genpd_set_required_opp() tolerates not seting the extra opps. However I'd certainly suggest not doing that (I think passing <0> should work). Having different number of items in power-domains and required-opps makes it harder to read the DT.
Hi Taniya, On Thu Mar 13, 2025 at 5:39 AM CET, Taniya Das wrote: > > > On 3/4/2025 2:10 PM, Dmitry Baryshkov wrote: >> On Tue, 4 Mar 2025 at 09:37, Vladimir Zapolskiy >> <vladimir.zapolskiy@linaro.org> wrote: >>> >>> On 3/4/25 01:53, Dmitry Baryshkov wrote: >>>> On Tue, Mar 04, 2025 at 12:55:21AM +0200, Vladimir Zapolskiy wrote: >>>>> SM8550 Camera Clock Controller shall enable both MXC and MMCX power >>>>> domains. >>>> >>>> Are those really required to access the registers of the cammcc? Or is >>>> one of those (MXC?) required to setup PLLs? Also, is this applicable >>>> only to sm8550 or to other similar clock controllers? >>> >>> Due to the described problem I experience a fatal CPU stall on SM8550-QRD, >>> not on any SM8450 or SM8650 powered board for instance, however it does >>> not exclude an option that the problem has to be fixed for other clock >>> controllers, but it's Qualcomm to confirm any other touched platforms, >> >> Please work with Taniya to identify used power domains. >> > > CAMCC requires both MMCX and MXC to be functional. Could you check whether any clock controllers on SM6350/SM7225 (Bitra) need multiple power domains, or in general which clock controller uses which power domain. That SoC has camcc, dispcc, gcc, gpucc, npucc and videocc. That'd be highly appreciated since I've been hitting weird issues there that could be explained by some missing power domains. Regards Luca > >>> for instance x1e80100-camcc has it resolved right at the beginning. >>> >>> To my understanding here 'required-opps' shall also be generalized, so >>> the done copy from x1e80100-camcc was improper, and the latter dt-binding >>> should be fixed. >> >> Yes >> > > required-opps is not mandatory for MXC as we ensure that MxC would never > hit retention. > > https://lore.kernel.org/r/20240625-avoid_mxc_retention-v2-1-af9c2f549a5f@quicinc.com > > >> >>
On 13/03/2025 04:39, Taniya Das wrote: > power-domains = <&rpmhpd SM8550_MMCX>, > <&rpmhpd SM8550_MXC>; > > Once the above change is incorporated, please add > > Reviewed-by: Taniya Das<quic_tdas@quicinc.com> Why does the ordering matter ? It shouldn't, right ?
On 13/03/2025 09:10, Bryan O'Donoghue wrote: > On 13/03/2025 04:39, Taniya Das wrote: >> power-domains = <&rpmhpd SM8550_MMCX>, >> <&rpmhpd SM8550_MXC>; >> >> Once the above change is incorporated, please add >> >> Reviewed-by: Taniya Das<quic_tdas@quicinc.com> > > Why does the ordering matter ? > > It shouldn't, right ? Being clear. I just want to check that you aren't implying a dependency between the two domains, its just alphanumeric sorting you mean here, right ? --- bod
On 3/13/2025 2:55 PM, Bryan O'Donoghue wrote: > On 13/03/2025 09:10, Bryan O'Donoghue wrote: >> On 13/03/2025 04:39, Taniya Das wrote: >>> power-domains = <&rpmhpd SM8550_MMCX>, >>> <&rpmhpd SM8550_MXC>; >>> >>> Once the above change is incorporated, please add >>> >>> Reviewed-by: Taniya Das<quic_tdas@quicinc.com> >> >> Why does the ordering matter ? >> >> It shouldn't, right ? > The ordering does not matter, it is to keep the newly added domain at the end. > Being clear. > > I just want to check that you aren't implying a dependency between the > two domains, its just alphanumeric sorting you mean here, right ? > > --- > bod
On 3/13/2025 1:22 PM, Luca Weiss wrote: > Hi Taniya, > > On Thu Mar 13, 2025 at 5:39 AM CET, Taniya Das wrote: >> >> >> On 3/4/2025 2:10 PM, Dmitry Baryshkov wrote: >>> On Tue, 4 Mar 2025 at 09:37, Vladimir Zapolskiy >>> <vladimir.zapolskiy@linaro.org> wrote: >>>> >>>> On 3/4/25 01:53, Dmitry Baryshkov wrote: >>>>> On Tue, Mar 04, 2025 at 12:55:21AM +0200, Vladimir Zapolskiy wrote: >>>>>> SM8550 Camera Clock Controller shall enable both MXC and MMCX power >>>>>> domains. >>>>> >>>>> Are those really required to access the registers of the cammcc? Or is >>>>> one of those (MXC?) required to setup PLLs? Also, is this applicable >>>>> only to sm8550 or to other similar clock controllers? >>>> >>>> Due to the described problem I experience a fatal CPU stall on SM8550-QRD, >>>> not on any SM8450 or SM8650 powered board for instance, however it does >>>> not exclude an option that the problem has to be fixed for other clock >>>> controllers, but it's Qualcomm to confirm any other touched platforms, >>> >>> Please work with Taniya to identify used power domains. >>> >> >> CAMCC requires both MMCX and MXC to be functional. > > Could you check whether any clock controllers on SM6350/SM7225 (Bitra) > need multiple power domains, or in general which clock controller uses > which power domain. > > That SoC has camcc, dispcc, gcc, gpucc, npucc and videocc. > > That'd be highly appreciated since I've been hitting weird issues there > that could be explained by some missing power domains. > Hi Luca, The targets you mentioned does not have any have multiple rail dependency, but could you share the weird issues with respect to clock controller I can take a look. > Regards > Luca > >> >>>> for instance x1e80100-camcc has it resolved right at the beginning. >>>> >>>> To my understanding here 'required-opps' shall also be generalized, so >>>> the done copy from x1e80100-camcc was improper, and the latter dt-binding >>>> should be fixed. >>> >>> Yes >>> >> >> required-opps is not mandatory for MXC as we ensure that MxC would never >> hit retention. >> >> https://lore.kernel.org/r/20240625-avoid_mxc_retention-v2-1-af9c2f549a5f@quicinc.com >> >> >>> >>> >
diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi index d02d80d731b9..d22b1753d521 100644 --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi @@ -3329,7 +3329,8 @@ camcc: clock-controller@ade0000 { <&bi_tcxo_div2>, <&bi_tcxo_ao_div2>, <&sleep_clk>; - power-domains = <&rpmhpd SM8550_MMCX>; + power-domains = <&rpmhpd SM8550_MXC>, + <&rpmhpd SM8550_MMCX>; required-opps = <&rpmhpd_opp_low_svs>; #clock-cells = <1>; #reset-cells = <1>;
SM8550 Camera Clock Controller shall enable both MXC and MMCX power domains. Fixes: e271b59e39a6f ("arm64: dts: qcom: sm8550: Add camera clock controller") Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> --- arch/arm64/boot/dts/qcom/sm8550.dtsi | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)