[PATCH] rockchip: efuse: fix rk3399 reading multiple values

[Jonas Karlman]

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/20230417160914.2838607-1-john@metanate.com/#3098312

Regards,
Jonas

> 
>>>>
>>>> Signed-off-by: John Keeping <john at metanate.com>
>>>> ---
>>>>  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);
>>>