Re: [PATCH v2 1/4] clk: qcom: Add driver for SC7180 GCC

Hello Balaji,

Thank you for your feedback.>> +static const struct gate_clk
sc7180_clks[] = {
>> +    GATE_CLK(GCC_AGGRE_UFS_PHY_AXI_CLK, 0x82024, 0x000001),
>> +    GATE_CLK(GCC_AGGRE_USB3_PRIM_AXI_CLK, 0x8201c, 0x000001),
>> +    GATE_CLK(GCC_BOOT_ROM_AHB_CLK, 0x52000, 0x000400),
>> +    GATE_CLK(GCC_CAMERA_HF_AXI_CLK, 0xb020, 0x000001),
>> +    GATE_CLK(GCC_CAMERA_THROTTLE_HF_AXI_CLK, 0xb080, 0x000001),
>> +    GATE_CLK(GCC_CE1_AHB_CLK, 0x52000, 0x000008),
>> +    GATE_CLK(GCC_CE1_AXI_CLK, 0x52000, 0x000010),
>> +    GATE_CLK(GCC_CE1_CLK, 0x52000, 0x000020),
>> +    GATE_CLK(GCC_CFG_NOC_USB3_PRIM_AXI_CLK, 0x502c, 0x000001),
>> +    GATE_CLK(GCC_CPUSS_AHB_CLK, 0x52000, 0x200000),
>> +    GATE_CLK(GCC_CPUSS_RBCPR_CLK, 0x48008, 0x000001),
>> +    GATE_CLK(GCC_DDRSS_GPU_AXI_CLK, 0x4452c, 0x000001),
>> +    GATE_CLK(GCC_DISP_HF_AXI_CLK, 0xb024, 0x000001),
>> +    GATE_CLK(GCC_DISP_THROTTLE_HF_AXI_CLK, 0xb084, 0x000001),
>> +    GATE_CLK(GCC_GP1_CLK, 0x64000, 0x000001),
>> +    GATE_CLK(GCC_GP2_CLK, 0x65000, 0x000001),
>> +    GATE_CLK(GCC_GP3_CLK, 0x66000, 0x000001),
>> +    GATE_CLK(GCC_GPU_MEMNOC_GFX_CLK, 0x7100c, 0x000001),
>> +    GATE_CLK(GCC_GPU_SNOC_DVM_GFX_CLK, 0x71018, 0x000001),
>> +    GATE_CLK(GCC_NPU_AXI_CLK, 0x4d008, 0x000001),
>> +    GATE_CLK(GCC_NPU_BWMON_AXI_CLK, 0x73008, 0x000001),
>> +    GATE_CLK(GCC_NPU_BWMON_DMA_CFG_AHB_CLK, 0x73018, 0x000001),
>> +    GATE_CLK(GCC_NPU_BWMON_DSP_CFG_AHB_CLK, 0x7301c, 0x000001),
>> +    GATE_CLK(GCC_NPU_CFG_AHB_CLK, 0x4d004, 0x000001),
>> +    GATE_CLK(GCC_NPU_DMA_CLK, 0x4d1a0, 0x000001),
>> +    GATE_CLK(GCC_PDM2_CLK, 0x3300c, 0x000001),
>> +    GATE_CLK(GCC_PDM_AHB_CLK, 0x33004, 0x000001),
>> +    GATE_CLK(GCC_PDM_XO4_CLK, 0x33008, 0x000001),
>> +    GATE_CLK(GCC_PRNG_AHB_CLK, 0x52000, 0x002000),
>> +    GATE_CLK(GCC_QSPI_CNOC_PERIPH_AHB_CLK, 0x4b004, 0x000001),
>> +    GATE_CLK(GCC_QSPI_CORE_CLK, 0x4b008, 0x000001),
>> +    GATE_CLK(GCC_QUPV3_WRAP0_CORE_2X_CLK, 0x52008, 0x000200),
>> +    GATE_CLK(GCC_QUPV3_WRAP0_CORE_CLK, 0x52008, 0x000100),
>> +    GATE_CLK(GCC_QUPV3_WRAP0_S0_CLK, 0x52008, 0x000400),
>> +    GATE_CLK(GCC_QUPV3_WRAP0_S1_CLK, 0x52008, 0x000800),
>> +    GATE_CLK(GCC_QUPV3_WRAP0_S2_CLK, 0x52008, 0x001000),
>> +    GATE_CLK(GCC_QUPV3_WRAP0_S3_CLK, 0x52008, 0x002000),
>> +    GATE_CLK(GCC_QUPV3_WRAP0_S4_CLK, 0x52008, 0x004000),
>> +    GATE_CLK(GCC_QUPV3_WRAP0_S5_CLK, 0x52008, 0x008000),
>> +    GATE_CLK(GCC_QUPV3_WRAP1_CORE_2X_CLK, 0x52008, 0x040000),
>> +    GATE_CLK(GCC_QUPV3_WRAP1_CORE_CLK, 0x52008, 0x080000),
>> +    GATE_CLK(GCC_QUPV3_WRAP1_S0_CLK, 0x52008, 0x400000),
>> +    GATE_CLK(GCC_QUPV3_WRAP1_S1_CLK, 0x52008, 0x800000),
>> +    GATE_CLK(GCC_QUPV3_WRAP1_S2_CLK, 0x52008, 0x1000000),
>> +    GATE_CLK(GCC_QUPV3_WRAP1_S3_CLK, 0x52008, 0x2000000),
>> +    GATE_CLK(GCC_QUPV3_WRAP1_S4_CLK, 0x52008, 0x4000000),
>> +    GATE_CLK(GCC_QUPV3_WRAP1_S5_CLK, 0x52008, 0x8000000),
>> +    GATE_CLK(GCC_QUPV3_WRAP_0_M_AHB_CLK, 0x52008, 0x000040),
>> +    GATE_CLK(GCC_QUPV3_WRAP_0_S_AHB_CLK, 0x52008, 0x000080),
>> +    GATE_CLK(GCC_QUPV3_WRAP_1_M_AHB_CLK, 0x52008, 0x100000),
>> +    GATE_CLK(GCC_QUPV3_WRAP_1_S_AHB_CLK, 0x52008, 0x200000),
>> +    GATE_CLK(GCC_SDCC1_AHB_CLK, 0x12008, 0x000001),
>> +    GATE_CLK(GCC_SDCC1_APPS_CLK, 0x1200c, 0x000001),
>> +    GATE_CLK(GCC_SDCC1_ICE_CORE_CLK, 0x12040, 0x000001),
>> +    GATE_CLK(GCC_SDCC2_AHB_CLK, 0x14008, 0x000001),
>> +    GATE_CLK(GCC_SDCC2_APPS_CLK, 0x14004, 0x000001),
>> +    GATE_CLK(GCC_SYS_NOC_CPUSS_AHB_CLK, 0x52000, 0x000001),
>> +    GATE_CLK(GCC_UFS_MEM_CLKREF_CLK, 0x8c000, 0x000001),
>> +    GATE_CLK(GCC_UFS_PHY_AHB_CLK, 0x77014, 0x000001),
>> +    GATE_CLK(GCC_UFS_PHY_AXI_CLK, 0x77038, 0x000001),
>> +    GATE_CLK(GCC_UFS_PHY_ICE_CORE_CLK, 0x77090, 0x000001),
>> +    GATE_CLK(GCC_UFS_PHY_PHY_AUX_CLK, 0x77094, 0x000001),
>> +    GATE_CLK(GCC_UFS_PHY_RX_SYMBOL_0_CLK, 0x7701c, 0x000001),
>> +    GATE_CLK(GCC_UFS_PHY_TX_SYMBOL_0_CLK, 0x77018, 0x000001),
>> +    GATE_CLK(GCC_UFS_PHY_UNIPRO_CORE_CLK, 0x7708c, 0x000001),
>> +    GATE_CLK(GCC_USB30_PRIM_MASTER_CLK, 0xf010, 0x000001),
>> +    GATE_CLK(GCC_USB30_PRIM_MOCK_UTMI_CLK, 0xf018, 0x000001),
>> +    GATE_CLK(GCC_USB30_PRIM_SLEEP_CLK, 0xf014, 0x000001),
>> +    GATE_CLK(GCC_USB3_PRIM_CLKREF_CLK, 0x8c010, 0x000001),
>> +    GATE_CLK(GCC_USB3_PRIM_PHY_AUX_CLK, 0xf050, 0x000001),
>> +    GATE_CLK(GCC_USB3_PRIM_PHY_COM_AUX_CLK, 0xf054, 0x000001),
>> +    GATE_CLK(GCC_USB3_PRIM_PHY_PIPE_CLK, 0xf058, 0x000001),
>> +    GATE_CLK(GCC_USB_PHY_CFG_AHB2PHY_CLK, 0x6a004, 0x000001),
>> +    GATE_CLK(GCC_VIDEO_AXI_CLK, 0xb01c, 0x000001),
>> +    GATE_CLK(GCC_VIDEO_THROTTLE_AXI_CLK, 0xb07c, 0x000001),
>> +    GATE_CLK(GCC_MSS_CFG_AHB_CLK, 0x8a000, 0x000001),
>> +    GATE_CLK(GCC_MSS_MFAB_AXIS_CLK, 0x8a004, 0x000001),
>> +    GATE_CLK(GCC_MSS_NAV_AXI_CLK, 0x8a00c, 0x000001),
>> +    GATE_CLK(GCC_MSS_Q6_MEMNOC_AXI_CLK, 0x8a154, 0x000001),
>> +    GATE_CLK(GCC_MSS_SNOC_AXI_CLK, 0x8a150, 0x000001),
>> +    GATE_CLK(GCC_LPASS_CFG_NOC_SWAY_CLK, 0x47018, 0x000001),
> 
> Thanks for the patches.
> 
> Instead of 0x000001 shall we use BIT(0) and similarly BIT(N) for others?
> 
> GATE_CLK is deprecated, can you consider using GATE_CLK_POLLED for
> applicable clocks.
> 
After switching to GATE_CLK_POLLED, I am having issues with bringing up
UFS, MMC and USB. In particular, clocks GCC_UFS_PHY_AHB_CLK (0x77014)
and GCC_USB_PHY_CFG_AHB2PHY_CLK (0x6a004) get stuck at off:
Warning: Clock @ 0x77014 [0x80000000] stuck at off>> +};
>> +
>> +static int sc7180_enable(struct clk *clk)
>> +{
>> +    struct msm_clk_priv *priv = dev_get_priv(clk->dev);
>> +
>> +    if (priv->data->num_clks < clk->id) {
>> +        debug("%s: unknown clk id %lu\n", __func__, clk->id);
>> +        return 0;
>> +    }
>> +
>> +    debug("%s: clk %s\n", __func__, sc7180_clks[clk->id].name);
>> +
>> +    switch (clk->id) {
>> +    case GCC_USB30_PRIM_MASTER_CLK:
>> +        qcom_gate_clk_en(priv, GCC_USB3_PRIM_PHY_AUX_CLK);
>> +        qcom_gate_clk_en(priv, GCC_USB3_PRIM_PHY_COM_AUX_CLK);
>> +        break;
>> +    }
>> +
>> +    qcom_gate_clk_en(priv, clk->id);
>> +
>> +    return 0;
> 
> Shall we instead do return qcom_gate_clk_en(priv, clk->id); instead of
> silently returning success even if a clk enable fails.
> 
However, if I fail here silently instead of returning
qcom_gate_clk_en(priv, clk->id), UFS and USB bring up fine, but MMC (SD
card slot) does not.
All I could find about this issue was this year old patch series where
GATE_CLK is used and failures are silent:
https://lore.kernel.org/u-boot/[email protected]/
What would be the best way to go forward in this series?