| Message ID | 20231107161802.855154-7-thomas.richard@bootlin.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Suspend to RAM support for K3 J7200 | expand |
On Tue, Nov 07, 2023 at 05:18:00PM +0100, Thomas Richard wrote: > Add the board specific part of the exit retention sequence for k3-ddrss > > Based on the work of Gregory CLEMENT <gregory.clement@bootlin.com> > > Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> If this ends up being physical board design specific and so someone making a new design for this SoC with a custom board layout entirely needs something else, we should think harder about what's in board_is_resuming() vs what we document the function does. If however, this is generic to the SoC and will be needed, it should be folded in with the previous patch and the commit message expanded, and the documentation part of this series explain clearly what the functions are responsible for.
On 11/7/23 19:18, Tom Rini wrote: > On Tue, Nov 07, 2023 at 05:18:00PM +0100, Thomas Richard wrote: > >> Add the board specific part of the exit retention sequence for k3-ddrss >> >> Based on the work of Gregory CLEMENT <gregory.clement@bootlin.com> >> >> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > > If this ends up being physical board design specific and so someone > making a new design for this SoC with a custom board layout entirely > needs something else, we should think harder about what's in > board_is_resuming() vs what we document the function does. If however, > this is generic to the SoC and will be needed, it should be folded in > with the previous patch and the commit message expanded, and the > documentation part of this series explain clearly what the functions are > responsible for. > Hi Tom, Thanks for all these interesting feedback's. From my point of view, board_is_resuming will work with all j7* SoCs and all new designs for these SoCs. This function only read and erase a scratchpad register in the PMIC A. So it should be ok for all designs which have the PMIC@48. For the function board_k3_ddrss_lpddr4_release_retention, it's a little bit more tricky. The sequence is specific to the SOC, but the GPIOs used are specific to the board. Regards, Thomas
On Thu, Nov 09, 2023 at 11:43:12AM +0100, Thomas Richard wrote: > On 11/7/23 19:18, Tom Rini wrote: > > On Tue, Nov 07, 2023 at 05:18:00PM +0100, Thomas Richard wrote: > > > >> Add the board specific part of the exit retention sequence for k3-ddrss > >> > >> Based on the work of Gregory CLEMENT <gregory.clement@bootlin.com> > >> > >> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > > > > If this ends up being physical board design specific and so someone > > making a new design for this SoC with a custom board layout entirely > > needs something else, we should think harder about what's in > > board_is_resuming() vs what we document the function does. If however, > > this is generic to the SoC and will be needed, it should be folded in > > with the previous patch and the commit message expanded, and the > > documentation part of this series explain clearly what the functions are > > responsible for. > > > > Hi Tom, > > Thanks for all these interesting feedback's. > > From my point of view, board_is_resuming will work with all j7* SoCs and > all new designs for these SoCs. > This function only read and erase a scratchpad register in the PMIC A. > So it should be ok for all designs which have the PMIC@48. > > For the function board_k3_ddrss_lpddr4_release_retention, it's a little > bit more tricky. The sequence is specific to the SOC, but the GPIOs used > are specific to the board. I suspect this is going to be the case where a judgement call will have to be made on theory vs practical on if the GPIOs will be duplicated to custom designs (it's tricky! TI gave us a working solution, just use it!) or changed based on other needs. I'll hope that perhaps people know who to talk with in-private to get some unofficial feedback on such things.
diff --git a/board/ti/j721e/evm.c b/board/ti/j721e/evm.c index b4b94c8c69..e6f46f911a 100644 --- a/board/ti/j721e/evm.c +++ b/board/ti/j721e/evm.c @@ -533,15 +533,22 @@ err_free_gpio: #if (IS_ENABLED(CONFIG_SPL_BUILD) && IS_ENABLED(CONFIG_TARGET_J7200_R5_EVM)) +static struct udevice *pmica; +static struct udevice *pmicb; + +#define GPIO_OUT_1 0x3D + #define SCRATCH_PAD_REG_3 0xCB #define MAGIC_SUSPEND 0xBA +#define DDR_RET_VAL BIT(1) +#define DDR_RET_CLK BIT(2) + static int resuming = -1; int board_is_resuming(void) { - struct udevice *pmica, *pmicb; int ret; if (resuming >= 0) @@ -580,6 +587,24 @@ end: return resuming; } +void board_k3_ddrss_lpddr4_release_retention(void) +{ + int regval; + + /* Set DDR_RET Signal Low on PMIC B */ + regval = pmic_reg_read(pmicb, GPIO_OUT_1) & ~DDR_RET_VAL; + + pmic_reg_write(pmicb, GPIO_OUT_1, regval); + + /* Now toggle the CLK of the latch for DDR ret */ + pmic_reg_write(pmicb, GPIO_OUT_1, regval | DDR_RET_CLK); + pmic_reg_write(pmicb, GPIO_OUT_1, regval & ~(DDR_RET_CLK)); + pmic_reg_write(pmicb, GPIO_OUT_1, regval | DDR_RET_CLK); + pmic_reg_write(pmicb, GPIO_OUT_1, regval & ~(DDR_RET_CLK)); + + pmic_reg_write(pmica, 0x86, 0x3); +} + #endif /* CONFIG_SPL_BUILD && CONFIG_TARGET_J7200_R5_EVM */ void spl_board_init(void)