On 2023-04-20 13:16, John Keeping wrote: > On Mon, Apr 17, 2023 at 06:58:30PM +0100, John Keeping wrote: >> On Mon, Apr 17, 2023 at 05:39:24PM +0000, Jonas Karlman wrote: >>> On 2023-04-17 18:09, John Keeping wrote: >>>> Update rk3399 to match the pattern in the other device-specific >>>> implementations to ensure the previous address is cleared when reading >>>> multiple blocks. >>> >>> Does this fix a real issue for you? >>> >>> Compared to the other device-specific implementations this reg behaves >>> differently on RK3399. The addr field is not read back, or it gets >>> auto-cleared when EFUSE_STROBE is cleared, the use of clrsetbits_le32 >>> may mislead that this reg holds the addr value from prior iteration. >> >> I don't have an RK3399 to test with, but I spotted this when >> experimenting with RK3288's secure efuse which has a similar layout to >> the RK3399 efuses. >> >> Having looked through the RK3399 TRM, I can't see anything about the >> address auto-clearing and the documentation for redundancy read mode >> requiring two strobe cycles suggests that the address is preserved. >> >> Howe are you testing reads? dump_efuse only reads one block at a time >> so it isn't sufficient to trigger an issue here as there is an >> unconditional write to EFUSE_CTRL before this loop. > > I found an RK3399 which I had for a previous project and have now tested > this - I can't see any evidence of the address auto-clearing so it looks > to me like this patch fixes a real issue.
Please see my prior response at [1], my testing showed that it was not needed. That run was on a plain rk3399 and a op1 rk3399 variant showed same result. [1] https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/#3098312 Regards, Jonas > >>>> >>>> Signed-off-by: John Keeping <[email protected]> >>>> --- >>>> drivers/misc/rockchip-efuse.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/misc/rockchip-efuse.c b/drivers/misc/rockchip-efuse.c >>>> index 2f96b79ea4..d302271239 100644 >>>> --- a/drivers/misc/rockchip-efuse.c >>>> +++ b/drivers/misc/rockchip-efuse.c >>>> @@ -207,8 +207,8 @@ static int rockchip_rk3399_efuse_read(struct udevice >>>> *dev, int offset, >>>> udelay(1); >>>> >>>> while (size--) { >>>> - setbits_le32(efuse->base + EFUSE_CTRL, >>>> - EFUSE_STROBE | RK3399_ADDR(offset++)); >>>> + clrsetbits_le32(efuse->base + EFUSE_CTRL, RK3399_A_MASK, >>>> + EFUSE_STROBE | RK3399_ADDR(offset++)); >>>> udelay(1); >>>> *buffer++ = readl(efuse->base + EFUSE_DOUT); >>>> clrbits_le32(efuse->base + EFUSE_CTRL, EFUSE_STROBE); >>>
