| Message ID | 20231117092737.28362-1-quic_sibis@quicinc.com |
|---|---|
| Headers | show |
| Series | clk: qcom: Introduce clocks drivers for X1E80100 | expand |
On 17/11/2023 09:27, Sibi Sankar wrote:
> * Use shared ops in the x1e80100 gcc driver [Bryan].
This looks better to me now / more consistent with what we have in
sc8280xp - where we do try to hit suspend and => retention/parking matters.
Could you give a bit more detail on why SDCC* doesn't warrant parking on
X1E80100 as on SC8280XP though ?
---
bod
On 17.11.2023 10:27, Sibi Sankar wrote: > From: Rajendra Nayak <quic_rjendra@quicinc.com> > > Add support for the global clock controller found on X1E80100 > based devices. > > Co-developed-by: Abel Vesa <abel.vesa@linaro.org> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com> > Co-developed-by: Sibi Sankar <quic_sibis@quicinc.com> > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad
On 17.11.2023 21:50, Bryan O'Donoghue wrote: > On 17/11/2023 09:27, Sibi Sankar wrote: >> * Use shared ops in the x1e80100 gcc driver [Bryan]. > > This looks better to me now / more consistent with what we have in sc8280xp - where we do try to hit suspend and => retention/parking matters. Parking the clock is separate from putting the system to sleep. IIUC we usually use shared ops on clocks that may have different users (e.g. not only controlled by Linux) and/or that are crucial to the functioning of hardware (like AXI clocks, which if gated would make the system crash on any access attempt, from any subsystem, unless turned on beforehand) Perhaps Dmitry can use some clearer wording than me.. Konrad
On 18/11/2023 00:06, Konrad Dybcio wrote: > On 17.11.2023 21:50, Bryan O'Donoghue wrote: >> On 17/11/2023 09:27, Sibi Sankar wrote: >>> * Use shared ops in the x1e80100 gcc driver [Bryan]. >> >> This looks better to me now / more consistent with what we have in sc8280xp - where we do try to hit suspend and => retention/parking matters. > Parking the clock is separate from putting the system to sleep. Yes but several of our clocks want to be parked, not switched off.. which obviously does matter in suspend. > IIUC we usually use shared ops on clocks that may have different users > (e.g. not only controlled by Linux) and/or that are crucial to the > functioning of hardware (like AXI clocks, which if gated would make > the system crash on any access attempt, from any subsystem, unless > turned on beforehand) My question here for Sibi, is why sdcc2_apss_clk_src differs here from sc8280xp? Is it wrong on sc8280xp or if correct sc8280xp then why is it not replicated here ? https://lore.kernel.org/linux-arm-msm/e857c853-51ef-8314-2a21-fa6fd25162ca@quicinc.com/ Also @Sibi I realise alot of this code is autogenerated - it would be worthwhile finding/fixing the script that does the generation to plug in shared_ops instead of floor_ops if the input material has the necessary flags. --- bod
On 11/18/23 07:22, Bryan O'Donoghue wrote: > On 18/11/2023 00:06, Konrad Dybcio wrote: >> On 17.11.2023 21:50, Bryan O'Donoghue wrote: >>> On 17/11/2023 09:27, Sibi Sankar wrote: >>>> * Use shared ops in the x1e80100 gcc driver [Bryan]. >>> >>> This looks better to me now / more consistent with what we have in >>> sc8280xp - where we do try to hit suspend and => retention/parking >>> matters. >> Parking the clock is separate from putting the system to sleep. > > Yes but several of our clocks want to be parked, not switched off.. > which obviously does matter in suspend. > >> IIUC we usually use shared ops on clocks that may have different users >> (e.g. not only controlled by Linux) and/or that are crucial to the >> functioning of hardware (like AXI clocks, which if gated would make >> the system crash on any access attempt, from any subsystem, unless >> turned on beforehand) > > My question here for Sibi, is why sdcc2_apss_clk_src differs here from > sc8280xp? > > Is it wrong on sc8280xp or if correct sc8280xp then why is it not > replicated here ? > > https://lore.kernel.org/linux-arm-msm/e857c853-51ef-8314-2a21-fa6fd25162ca@quicinc.com/ Bryan, 5e4b7e82d497 ("clk: qcom: gcc-sdm845: Use floor ops for sdcc clks") The floor_ops was introduced in sdm845 ^^ and later propagated to all other QC SoCs later on. It makes sense to do the same for sc8280xp as well. > > Also @Sibi I realise alot of this code is autogenerated - it would be > worthwhile finding/fixing the script that does the generation to plug in > shared_ops instead of floor_ops if the input material has the necessary > flags. floor_ops part isn't auto-generated (it comes out as shared_ops, but like you said it might make sense to include it as part of the generation process itself. -Sibi > > --- > bod
On 20/11/2023 06:42, Sibi Sankar wrote: > Bryan, > > 5e4b7e82d497 ("clk: qcom: gcc-sdm845: Use floor ops for sdcc clks") > > The floor_ops was introduced in sdm845 ^^ and later propagated to all > other QC SoCs later on. It makes sense to do the same for sc8280xp as > well. OK good enough.