Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46878 )
Change subject: soc/mediatek/mt8192: add clkbuf and srclken_rc MT6359P driver ......................................................................
Patch Set 48:
(20 comments)
https://review.coreboot.org/c/coreboot/+/46878/45//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46878/45//COMMIT_MSG@9 PS45, Line 9: Add clkbuf and srclken_rc init for low power.
Please add the name and revision of the used datasheet.
Done
https://review.coreboot.org/c/coreboot/+/46878/22/src/soc/mediatek/mt8192/cl... File src/soc/mediatek/mt8192/clkbuf.c:
https://review.coreboot.org/c/coreboot/+/46878/22/src/soc/mediatek/mt8192/cl... PS22, Line 78: 0x%x
%#x
Done
https://review.coreboot.org/c/coreboot/+/46878/22/src/soc/mediatek/mt8192/cl... PS22, Line 81: 0x%x
%#x
Done
https://review.coreboot.org/c/coreboot/+/46878/22/src/soc/mediatek/mt8192/cl... PS22, Line 86: 0x%x
%#x
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/cl... File src/soc/mediatek/mt8192/clkbuf.c:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/cl... PS41, Line 23: static void buf_read(u32 addr, u32 *rdata)
can we reuse rtc_read/rtc_write ?
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/cl... PS41, Line 66: pmic_cw00 = buf_read_field(PMIC_RG_DCXO_CW00, 0xffff, 0);
use PMIC_REG_MASK and PMIC_REG_SHIFT ?
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/cl... PS41, Line 77: buf_info("DCXO_CW00/09/12/13/15/19=0x%x %x %x %x %x %x\n",
%#x
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/cl... PS41, Line 95: buf_write_field(PMIC_TOP_TMA_KEY, 0x9CA6, 0xFFFF, 0);
please define this number
Done
https://review.coreboot.org/c/coreboot/+/46878/44/src/soc/mediatek/mt8192/cl... File src/soc/mediatek/mt8192/clkbuf.c:
https://review.coreboot.org/c/coreboot/+/46878/44/src/soc/mediatek/mt8192/cl... PS44, Line 175:
remove these blank lines
Done
https://review.coreboot.org/c/coreboot/+/46878/44/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/clkbuf.h:
https://review.coreboot.org/c/coreboot/+/46878/44/src/soc/mediatek/mt8192/in... PS44, Line 68:
remove these blank lines
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/clkbuf.h:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... PS41, Line 17:
remove extra blank line
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/pmif.h:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... PS41, Line 176: #define PMIFSPI_INF_EN_SRCLKEN_RC_HW_MSK 0x1
use tab for indent
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... PS41, Line 176: #define PMIFSPI_INF_EN_SRCLKEN_RC_HW_MSK 0x1 : #define PMIFSPI_INF_EN_SRCLKEN_RC_HW_SHFT 4
DEFINE_BIT
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... PS41, Line 188: #define PMIFSPI_SPM_SLEEP_REQ_SEL_MSK 0x3 : #define PMIFSPI_SPM_SLEEP_REQ_SEL_SHFT 0
DEFINE_BITFIELD
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/rtc.h:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... PS41, Line 151: #if 0
just remove
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/pm... File src/soc/mediatek/mt8192/pmif.c:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/pm... PS41, Line 183: unsigned int spi_sleep_ctrl, spmi_sleep_ctrl; : unsigned int spi_mode_ctrl, spmi_mode_ctrl; : unsigned int inf_en, other_inf_en, arb_en; : : spi_sleep_ctrl = read32(&pmif_spi_arb[0].mtk_pmif->sleep_protection_ctrl); : spmi_sleep_ctrl = read32(&pmif_spmi_arb[0].mtk_pmif->sleep_protection_ctrl); : spi_mode_ctrl = read32(&pmif_spi_arb[0].mtk_pmif->spi_mode_ctrl); : spmi_mode_ctrl = read32(&pmif_spmi_arb[0].mtk_pmif->spi_mode_ctrl); : inf_en = read32(&pmif_spi_arb[0].mtk_pmif->inf_en); : other_inf_en = read32(&pmif_spi_arb[0].mtk_pmif->other_inf_en); : arb_en = read32(&pmif_spi_arb[0].mtk_pmif->arb_en); : : if (mode == PMIF_VLD_RDY) { : /* spm and scp sleep request disable spi and spmi */ : spi_sleep_ctrl = (spi_sleep_ctrl : & ~(PMIFSPI_SPM_SLEEP_REQ_SEL_MSK << PMIFSPI_SPM_SLEEP_REQ_SEL_SHFT)) : | (0x1 << PMIFSPI_SPM_SLEEP_REQ_SEL_SHFT); : spi_sleep_ctrl = (spi_sleep_ctrl : & ~(PMIFSPI_SCP_SLEEP_REQ_SEL_MSK << PMIFSPI_SCP_SLEEP_REQ_SEL_SHFT)) : | (0x1 << PMIFSPI_SCP_SLEEP_REQ_SEL_SHFT); : : spmi_sleep_ctrl = (spmi_sleep_ctrl : & ~(PMIFSPMI_SPM_SLEEP_REQ_SEL_MSK << PMIFSPMI_SPM_SLEEP_REQ_SEL_SHFT)) : | (0x1 << PMIFSPMI_SPM_SLEEP_REQ_SEL_SHFT); : spmi_sleep_ctrl = (spmi_sleep_ctrl : & ~(PMIFSPMI_SCP_SLEEP_REQ_SEL_MSK << PMIFSPMI_SCP_SLEEP_REQ_SEL_SHFT)) : | (0x1 << PMIFSPMI_SCP_SLEEP_REQ_SEL_SHFT); : : /* pmic vld/rdy control spi mode enable*/ : spi_mode_ctrl = (spi_mode_ctrl : & ~(PMIFSPI_MD_CTL_PMIF_RDY_MSK << PMIFSPI_MD_CTL_PMIF_RDY_SHFT)) : | (0x1 << PMIFSPI_MD_CTL_PMIF_RDY_SHFT); : /* srclken control spi mode disable*/ : spi_mode_ctrl = (spi_mode_ctrl : & ~(PMIFSPI_MD_CTL_SRCLK_EN_MSK << PMIFSPI_MD_CTL_SRCLK_EN_SHFT)) : | (0x0 << PMIFSPI_MD_CTL_SRCLK_EN_SHFT); : /* vreq control spi mode disable*/ : spi_mode_ctrl = (spi_mode_ctrl : & ~(PMIFSPI_MD_CTL_SRVOL_EN_MSK << PMIFSPI_MD_CTL_SRVOL_EN_SHFT)) : | (0x0 << PMIFSPI_MD_CTL_SRVOL_EN_SHFT); : spmi_mode_ctrl = (spmi_mode_ctrl : & ~(PMIFSPMI_MD_CTL_PMIF_RDY_MSK << PMIFSPMI_MD_CTL_PMIF_RDY_SHFT)) : | (0x1 << PMIFSPMI_MD_CTL_PMIF_RDY_SHFT); : spmi_mode_ctrl = (spmi_mode_ctrl : & ~(PMIFSPMI_MD_CTL_SRCLK_EN_MSK << PMIFSPMI_MD_CTL_SRCLK_EN_SHFT)) : | (0x0 << PMIFSPMI_MD_CTL_SRCLK_EN_SHFT); : spmi_mode_ctrl = (spmi_mode_ctrl : & ~(PMIFSPMI_MD_CTL_SRVOL_EN_MSK << PMIFSPMI_MD_CTL_SRVOL_EN_SHFT)) : | (0x0 << PMIFSPMI_MD_CTL_SRVOL_EN_SHFT); : : /* srclken rc interface enable*/ : inf_en = (inf_en & ~(PMIFSPI_INF_EN_SRCLKEN_RC_HW_MSK : << PMIFSPI_INF_EN_SRCLKEN_RC_HW_SHFT)) : | (0x1 << PMIFSPI_INF_EN_SRCLKEN_RC_HW_SHFT); : : /* dcxo interface disable */ : other_inf_en = (other_inf_en : & ~(PMIFSPI_OTHER_INF_DXCO0_EN_MSK << PMIFSPI_OTHER_INF_DXCO0_EN_SHFT)) : | (0x0 << PMIFSPI_OTHER_INF_DXCO0_EN_SHFT); : other_inf_en = (other_inf_en : & ~(PMIFSPI_OTHER_INF_DXCO1_EN_MSK << PMIFSPI_OTHER_INF_DXCO1_EN_SHFT)) : | (0x0 << PMIFSPI_OTHER_INF_DXCO1_EN_SHFT); : /*srclken enable, dcxo0,1 disable*/ : arb_en = (arb_en & ~(PMIFSPI_ARB_EN_SRCLKEN_RC_HW_MSK : << PMIFSPI_ARB_EN_SRCLKEN_RC_HW_SHFT)) : | (0x1 << PMIFSPI_ARB_EN_SRCLKEN_RC_HW_SHFT); : arb_en = (arb_en & ~(PMIFSPI_ARB_EN_DCXO_CONN_MSK : << PMIFSPI_ARB_EN_DCXO_CONN_SHFT)) : | (0x0 << PMIFSPI_ARB_EN_DCXO_CONN_SHFT); : arb_en = (arb_en & ~(PMIFSPI_ARB_EN_DCXO_NFC_MSK : << PMIFSPI_ARB_EN_DCXO_NFC_SHFT)) : | (0x0 << PMIFSPI_ARB_EN_DCXO_NFC_SHFT); : } else if (mode == PMIF_SLP_REQ) { : /* spm and scp sleep request enable spi and spmi */ : spi_sleep_ctrl = (spi_sleep_ctrl : & ~(PMIFSPI_SPM_SLEEP_REQ_SEL_MSK << PMIFSPI_SPM_SLEEP_REQ_SEL_SHFT)) : | (0x0 << PMIFSPI_SPM_SLEEP_REQ_SEL_SHFT); : spi_sleep_ctrl = (spi_sleep_ctrl : & ~(PMIFSPI_SCP_SLEEP_REQ_SEL_MSK << PMIFSPI_SCP_SLEEP_REQ_SEL_SHFT)) : | (0x0 << PMIFSPI_SCP_SLEEP_REQ_SEL_SHFT); : : spmi_sleep_ctrl = (spmi_sleep_ctrl : & ~(PMIFSPMI_SPM_SLEEP_REQ_SEL_MSK << PMIFSPMI_SPM_SLEEP_REQ_SEL_SHFT)) : | (0x0 << PMIFSPMI_SPM_SLEEP_REQ_SEL_SHFT); : spmi_sleep_ctrl = (spmi_sleep_ctrl : & ~(PMIFSPMI_SCP_SLEEP_REQ_SEL_MSK << PMIFSPMI_SCP_SLEEP_REQ_SEL_SHFT)) : | (0x0 << PMIFSPMI_SCP_SLEEP_REQ_SEL_SHFT); : : /* pmic vld/rdy control spi mode disable*/ : spi_mode_ctrl = (spi_mode_ctrl : & ~(PMIFSPI_MD_CTL_PMIF_RDY_MSK << PMIFSPI_MD_CTL_PMIF_RDY_SHFT)) : | (0x0 << PMIFSPI_MD_CTL_PMIF_RDY_SHFT); : /* srclken control spi mode enable*/ : spi_mode_ctrl = (spi_mode_ctrl : & ~(PMIFSPI_MD_CTL_SRCLK_EN_MSK << PMIFSPI_MD_CTL_SRCLK_EN_SHFT)) : | (0x1 << PMIFSPI_MD_CTL_SRCLK_EN_SHFT); : /* vreq control spi mode enable*/ : spi_mode_ctrl = (spi_mode_ctrl : & ~(PMIFSPI_MD_CTL_SRVOL_EN_MSK << PMIFSPI_MD_CTL_SRVOL_EN_SHFT)) : | (0x1 << PMIFSPI_MD_CTL_SRVOL_EN_SHFT); : : spmi_mode_ctrl = (spmi_mode_ctrl : & ~(PMIFSPMI_MD_CTL_PMIF_RDY_MSK << PMIFSPMI_MD_CTL_PMIF_RDY_SHFT)) : | (0x0 << PMIFSPMI_MD_CTL_PMIF_RDY_SHFT); : spmi_mode_ctrl = (spmi_mode_ctrl : & ~(PMIFSPMI_MD_CTL_SRCLK_EN_MSK << PMIFSPMI_MD_CTL_SRCLK_EN_SHFT)) : | (0x1 << PMIFSPMI_MD_CTL_SRCLK_EN_SHFT); : spmi_mode_ctrl = (spmi_mode_ctrl : & ~(PMIFSPMI_MD_CTL_SRVOL_EN_MSK << PMIFSPMI_MD_CTL_SRVOL_EN_SHFT)) : | (0x1 << PMIFSPMI_MD_CTL_SRVOL_EN_SHFT); : : /* srclken rc interface disable*/ : inf_en = (inf_en & ~(PMIFSPI_INF_EN_SRCLKEN_RC_HW_MSK : << PMIFSPI_INF_EN_SRCLKEN_RC_HW_SHFT)) : | (0x0 << PMIFSPI_INF_EN_SRCLKEN_RC_HW_SHFT); : /* dcxo interface enable */ : other_inf_en = (other_inf_en : & ~(PMIFSPI_OTHER_INF_DXCO0_EN_MSK << PMIFSPI_OTHER_INF_DXCO0_EN_SHFT)) : | (0x1 << PMIFSPI_OTHER_INF_DXCO0_EN_SHFT); : other_inf_en = (other_inf_en : & ~(PMIFSPI_OTHER_INF_DXCO1_EN_MSK << PMIFSPI_OTHER_INF_DXCO1_EN_SHFT)) : | (0x1 << PMIFSPI_OTHER_INF_DXCO1_EN_SHFT); : : /*srclken disable, dcxo0,1 enable*/ : arb_en = (arb_en & ~(PMIFSPI_ARB_EN_SRCLKEN_RC_HW_MSK : << PMIFSPI_ARB_EN_SRCLKEN_RC_HW_SHFT)) : | (0x0 << PMIFSPI_ARB_EN_SRCLKEN_RC_HW_SHFT); : arb_en = (arb_en & ~(PMIFSPI_ARB_EN_DCXO_CONN_MSK : << PMIFSPI_ARB_EN_DCXO_CONN_SHFT)) : | (0x1 << PMIFSPI_ARB_EN_DCXO_CONN_SHFT); : arb_en = (arb_en & ~(PMIFSPI_ARB_EN_DCXO_NFC_MSK : << PMIFSPI_ARB_EN_DCXO_NFC_SHFT)) : | (0x1 << PMIFSPI_ARB_EN_DCXO_NFC_SHFT); : : } else : return; : : write32(&pmif_spi_arb[0].mtk_pmif->sleep_protection_ctrl, spi_sleep_ctrl); : write32(&pmif_spmi_arb[0].mtk_pmif->sleep_protection_ctrl, spmi_sleep_ctrl); : write32(&pmif_spi_arb[0].mtk_pmif->spi_mode_ctrl, spi_mode_ctrl); : write32(&pmif_spmi_arb[0].mtk_pmif->spi_mode_ctrl, spmi_mode_ctrl); : write32(&pmif_spi_arb[0].mtk_pmif->inf_en, inf_en); : write32(&pmif_spi_arb[0].mtk_pmif->other_inf_en, other_inf_en); : write32(&pmif_spi_arb[0].mtk_pmif->arb_en, arb_en); : }
use DEFINE_BITFIELD, DEFINE_BIT and SET32_BITFIELDS […]
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/rt... File src/soc/mediatek/mt8192/rtc.c:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/rt... PS41, Line 316: #if 0
just remove the code if dcox is not used.
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/sr... File src/soc/mediatek/mt8192/srclken_rc.c:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/sr... PS41, Line 155: if (mode == INIT_MODE) { : value = ((rc_ctrl[id].dcxo_settle_blk_en & DCXO_SETTLE_BLK_EN_MSK) : << DCXO_SETTLE_BLK_EN_SHFT) : | ((rc_ctrl[id].bypass_cmd & BYPASS_CMD_EN_MSK) << BYPASS_CMD_EN_SHFT) : | ((rc_ctrl[id].sw_rc & SW_SRCLKEN_RC_MSK) << SW_SRCLKEN_RC_SHFT) : | ((rc_ctrl[id].sw_fpm & SW_SRCLKEN_FPM_MSK) << SW_SRCLKEN_FPM_SHFT) : | ((rc_ctrl[id].sw_bblpm & SW_SRCLKEN_BBLPM_MSK) : << SW_SRCLKEN_BBLPM_SHFT) : | ((rc_ctrl[id].xo_soc_link_en & XO_SOC_LINK_EN_MSK) : << XO_SOC_LINK_EN_SHFT) : | ((rc_ctrl[id].req_ack_imd_en & REQ_ACK_LOW_IMD_EN_MSK) : << REQ_ACK_LOW_IMD_EN_SHFT) : | ((rc_ctrl[id].track_en & SRCLKEN_TRACK_M_EN_MSK) : << SRCLKEN_TRACK_M_EN_SHFT) : | ((rc_ctrl[id].cnt_step & CNT_PRD_STEP_MSK) << CNT_PRD_STEP_SHFT) : | ((rc_ctrl[id].xo_prd & XO_STABLE_PRD_MSK) << XO_STABLE_PRD_SHFT) : | ((rc_ctrl[id].dcxo_prd & DCXO_STABLE_PRD_MSK) : << DCXO_STABLE_PRD_SHFT); : } else if (mode == SW_MODE) { : value = read32(&rc_regs->rc_mxx_srclken_cfg[id]) | (0x1 << SW_SRCLKEN_RC_SHFT); : } else if (mode == HW_MODE) { : value = read32(&rc_regs->rc_mxx_srclken_cfg[id]) : & ~(SW_SRCLKEN_RC_MSK << SW_SRCLKEN_RC_SHFT); : } else : return; : : write32(&rc_regs->rc_mxx_srclken_cfg[id], value);
SET32_BITFIELDS
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/sr... PS41, Line 194: if (mode == SW_FPM_HIGH) : value = read32(&rc_regs->rc_mxx_srclken_cfg[id]) | (0x1 << SW_SRCLKEN_FPM_SHFT); : else if (mode == SW_FPM_LOW) : value = read32(&rc_regs->rc_mxx_srclken_cfg[id]) : & ~(SW_SRCLKEN_FPM_MSK << SW_SRCLKEN_FPM_SHFT); : else : return; : : rc_ctrl[id].sw_fpm = mode; : write32(&rc_regs->rc_mxx_srclken_cfg[id], value);
SET32_BITFIELDS
Done
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/sr... PS41, Line 214: if (mode == SW_BBLPM_HIGH) : value = read32(&rc_regs->rc_mxx_srclken_cfg[id]) : | (0x1 << SW_SRCLKEN_BBLPM_SHFT); : else if (mode == SW_BBLPM_LOW) : value = read32(&rc_regs->rc_mxx_srclken_cfg[id]) : & ~(SW_SRCLKEN_BBLPM_MSK << SW_SRCLKEN_BBLPM_SHFT); : else : return; : : rc_ctrl[id].sw_bblpm = mode; : write32(&rc_regs->rc_mxx_srclken_cfg[id], value);
ditto
Done