Re: [PATCH v3 5/6] clk: qcom: Add support for Global clock controller on Eliza

[Abel Vesa]

On 26-02-17 11:15:25, Konrad Dybcio wrote:
> On 2/17/26 8:58 AM, Abel Vesa wrote:
> > On 26-02-16 16:38:15, Konrad Dybcio wrote:
> >> On 2/16/26 2:43 PM, Abel Vesa wrote:
> >>> From: Taniya Das <taniya.das@xxxxxxxxxxxxxxxx>
> >>>
> >>> Add support for Global clock controller for Eliza Qualcomm SoC.
> >>>
> >>> Signed-off-by: Taniya Das <taniya.das@xxxxxxxxxxxxxxxx>
> >>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> >>> Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx>
> >>> ---
> >>
> >> [...]
> >>
> >>
> >>> +static struct clk_regmap_mux gcc_pcie_0_pipe_clk_src = {
> >>
> >> This and a number of others should be struct clk_regmap_phy_mux instead,
> >> cross-check with kaanapali and remember to drop the then-unused parentmaps
> > 
> > Oups, missed this in v4. Will fix in v5.
> > 
> >>
> >> [...]
> >>
> >>> +	[GCC_CAMERA_HF_CLK_EN_SLP_STG] = { 0x26018, 1 },
> >>> +	[GCC_CAMERA_SF_CLK_EN_SLP_STG] = { 0x26028, 1 },
> >>> +	[GCC_CAMERA_HF_CLK_EN_SEL_SLP_STG] = { 0x26018, 2 },
> >>> +	[GCC_CAMERA_SF_CLK_EN_SEL_SLP_STG] = { 0x26028, 2 },
> >>
> >> So SEL_SLP_STG=0x1 allows setting SLP_STG to =0x1 to assert the ARES,
> >> otherwise if SEL_SLP_STG=0x0, it's controlled by the HW state machine
> >>
> >> I don't think modelling the prior one as a reset is valid, but I don't
> >> know what considerations we need to take wrt dis/allowing hw control
> >> here and what the consumer expectations are.
> >>
> >> Taniya, would you know?
> 
> I see you've already sent a v5, but I'd like to see the above resolved..
> 
> Adding more context, bits 1 and 2 control the gating of the SLP_STG
> *clock*, whereas bits 4 and 5 control the reset.. I think if we don't have
> an immediate need for these, we may temporarily drop them as we try and
> figure out a way forward to avoid adding fake ABI

Will send v5 (you were refering to v4) with these dropped.

Taniya, can you explain why these were added in the first place ?

> 
> Konrad