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

[Jonas Karlman]

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.

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);
>>