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

[Jonas Karlman]

On 2023-04-17 20:19, Jonas Karlman wrote:
> On 2023-04-17 19:58, 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.
> 
> Let me re-test this on one of my rk3399 boards, from what I can recall I
> made the same change when working on support for the other SoCs and same
> as you I was expecting it to be reading back bad/wrong data on rk3399.
> 
> At that time the dump cmd read all 128 byte using one call, unexpectedly
> the same data was read back regardless the use of clrsetbits or setbits.
> 
> Will re-test using a full 128 byte read call and report back result.

I did seem to recall correctly, the values written to ctrl reg is not
what is being read back on rk3399.

This is from the read of cpu_id where ctrl is the readl value and val is
the value used in setbits call. Here we can see that only the strobe bit
is sticky after the setbits call, addr is never read back.

rockchip_rk3399_efuse_read: before array read mode, size=5 ctrl=a9
rockchip_rk3399_efuse_read: after array read mode, size=5 ctrl=28c
rockchip_rk3399_efuse_read: before setbits, size=4 ctrl=28c val=10002
rockchip_rk3399_efuse_read: after setbits+udelay, size=4 ctrl=28e
rockchip_rk3399_efuse_read: before clrbits, size=4 ctrl=28e
rockchip_rk3399_efuse_read: after clrbits+udelay, size=4 ctrl=28c
rockchip_rk3399_efuse_read: before setbits, size=3 ctrl=28c val=20002
rockchip_rk3399_efuse_read: after setbits+udelay, size=3 ctrl=28e
rockchip_rk3399_efuse_read: before clrbits, size=3 ctrl=28e
rockchip_rk3399_efuse_read: after clrbits+udelay, size=3 ctrl=28c
rockchip_rk3399_efuse_read: before setbits, size=2 ctrl=28c val=30002
rockchip_rk3399_efuse_read: after setbits+udelay, size=2 ctrl=28e
rockchip_rk3399_efuse_read: before clrbits, size=2 ctrl=28e
rockchip_rk3399_efuse_read: after clrbits+udelay, size=2 ctrl=28c
rockchip_rk3399_efuse_read: before setbits, size=1 ctrl=28c val=40002
rockchip_rk3399_efuse_read: after setbits+udelay, size=1 ctrl=28e
rockchip_rk3399_efuse_read: before clrbits, size=1 ctrl=28e
rockchip_rk3399_efuse_read: after clrbits+udelay, size=1 ctrl=28c
rockchip_rk3399_efuse_read: before setbits, size=0 ctrl=28c val=50002
rockchip_rk3399_efuse_read: after setbits+udelay, size=0 ctrl=28e
rockchip_rk3399_efuse_read: before clrbits, size=0 ctrl=28e
rockchip_rk3399_efuse_read: after clrbits+udelay, size=0 ctrl=28c
rockchip_rk3399_efuse_read: before power-down mode, size=-1 ctrl=28c
rockchip_rk3399_efuse_read: after power-down mode, size=-1 ctrl=21

And data in efuse is read back correctly.

dump_efuse with 16 byte buffer:
00000000: 52 4b 33 99 91 fe 21 54 4d 53 36 31 35 2e 30 30  RK3...!TMS615.00
00000010: 00 00 00 00 04 02 84 26 1c 25 15 1a 08 00 00 00  .......&.%......
00000020: 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00  ................

dump_efuse with 4 byte buffer:
00000000: 52 4b 33 99  RK3.
00000004: 91 fe 21 54  ..!T
00000008: 4d 53 36 31  MS61
0000000c: 35 2e 30 30  5.00
00000010: 00 00 00 00  ....
00000014: 04 02 84 26  ...&
00000018: 1c 25 15 1a  .%..
0000001c: 08 00 00 00  ....
00000020: 00 00 10 00  ....

Above was from an old rk3399 board, also tested on a board with a newer
OP1 revision that showed the same behavior of the ctrl reg.

Regards,
Jonas

> 
> Also look like the EFUSE_STROBE bit is missing from the mask param.
> 
> 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);
>>>
>