Re: [PATCH v3 1/1] regmap: Synchronize cache for the page selector

[Marek Szyprowski <m.szyprowski@samsung.com>]

On 17.01.2025 15:09, Andy Shevchenko wrote:

> On Fri, Jan 17, 2025 at 02:57:52PM +0100, Marek Szyprowski wrote:
>> On 16.01.2025 13:42, Andy Shevchenko wrote:
>>> If the selector register is represented in each page, its value
>>> in accordance to the debugfs is stale because it gets synchronized
>>> only after the real page switch happens. Synchronize cache for
>>> the page selector.
>>>
>>> Before (offset followed by hexdump, the first byte is selector):
>>>
>>>       // Real registers
>>>       18: 05 ff 00 00 ff 0f 00 00 f0 00 00 00
>>>       ...
>>>       // Virtual (per port)
>>>       40: 05 ff 00 00 e0 e0 00 00 00 00 00 1f
>>>       50: 00 ff 00 00 e0 e0 00 00 00 00 00 1f
>>>       60: 01 ff 00 00 ff ff 00 00 00 00 00 00
>>>       70: 02 ff 00 00 cf f3 00 00 00 00 00 0c
>>>       80: 03 ff 00 00 00 00 00 00 00 00 00 ff
>>>       90: 04 ff 00 00 ff 0f 00 00 f0 00 00 00
>>>
>>> After:
>>>
>>>       // Real registers
>>>       18: 05 ff 00 00 ff 0f 00 00 f0 00 00 00
>>>       ...
>>>       // Virtual (per port)
>>>       40: 00 ff 00 00 e0 e0 00 00 00 00 00 1f
>>>       50: 01 ff 00 00 e0 e0 00 00 00 00 00 1f
>>>       60: 02 ff 00 00 ff ff 00 00 00 00 00 00
>>>       70: 03 ff 00 00 cf f3 00 00 00 00 00 0c
>>>       80: 04 ff 00 00 00 00 00 00 00 00 00 ff
>>>       90: 05 ff 00 00 ff 0f 00 00 f0 00 00 00
>>>
>>> Fixes: 6863ca622759 ("regmap: Add support for register indirect addressing.")
>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> This patch landed in linux-next some time ago as commit 1fd60ed1700c
>> ("regmap: Synchronize cache for the page selector"). Today I've noticed
>> that it causes a regression for Lontium LT9611UXC HDMI bridge driver.
> Is there any datasheet link to the HW in question?
>
> (FWIW, I have tested this with the CY8C9540 GPIO I²C expander on Intel Galileo
>   Gen 1 board.)

I'm not aware of it, but I'm not related to lt9611uxc driver development 
at all...

>> With today's linux-next I got the following messages on QCom RB5 board:
>>
>> # dmesg | grep  lt9611uxc
>> [   13.737346] lt9611uxc 5-002b: LT9611 revision: 0x00.00.00
>> [   13.804190] lt9611uxc 5-002b: LT9611 version: 0x00
>> [   13.870564] lt9611uxc 5-002b: FW version 0, enforcing firmware update
>> [   13.877437] lt9611uxc 5-002b: Direct firmware load for
>> lt9611uxc_fw.bin failed with error -2
>> [   13.887517] lt9611uxc 5-002b: probe with driver lt9611uxc failed with
>> error -2
>>
>> after reverting the $subject patch, the driver probes fine on that board.
>>
>> I'm not sure if this is really a bug caused by this change or simply the
>> driver already was aligned to old regmap behavior. Dmitry, could you
>> check the regamp usage and review the changes introduced by this patch?
>> Let me know if there is anything to check on the real hardware to help
>> resolving this issue.
> Yes, see below. And thank you for your report!
>
> ...
>
>>> +		/*
>>> +		 * If selector register has been just updated, update the respective
>>> +		 * virtual copy as well.
>>> +		 */
>>> +		if (page_chg &&
>>> +		    in_range(range->selector_reg, range->window_start, range->window_len))
>>> +			_regmap_update_bits(map, sel_register, mask, val, NULL, false);
> Can you add a test printk() here to show
>
> page_chg
> range->selector_reg, range->window_start, range->window_len
> sel_register, mask, val
>
> ?
>
> And would commenting these three lines make it work again?

I've added the following debug message before that check:

printk("page_chg %d range->selector_reg %x range->window_start %x 
range->window_len %x sel_register %x mask %x val %x\n", page_chg, 
range->selector_reg, range->window_start, range->window_len, 
sel_register, mask, val);

Here is the result:

root@target:~# modprobe lontium_lt9611uxc
[   29.892962] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   29.907419] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 81ff mask ff val 81
[   29.926712] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 81ff mask ff val 81
[   29.958301] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 81ff mask ff val 81
[   29.979951] lt9611uxc 5-002b: LT9611 revision: 0x00.00.00
[   29.987002] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   30.052370] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   30.066808] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register b0ff mask ff val b0
[   30.079859] lt9611uxc 5-002b: LT9611 version: 0x00
[   30.085990] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   30.151735] lt9611uxc 5-002b: FW version 0, enforcing firmware update
[   30.158830] lt9611uxc 5-002b: Direct firmware load for 
lt9611uxc_fw.bin failed with error -2
[   30.168653] lt9611uxc 5-002b: probe with driver lt9611uxc failed with 
error -2

after disabling the above mentioned 3 lines:

root@target:~# modprobe lontium_lt9611uxc
[   44.584893] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   44.597589] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 81ff mask ff val 81
[   44.609936] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 81ff mask ff val 81
[   44.622277] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 81ff mask ff val 81
[   44.634579] lt9611uxc 5-002b: LT9611 revision: 0x17.04.93
[   44.641444] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   44.710694] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   44.724579] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register b0ff mask ff val b0
[   44.736853] lt9611uxc 5-002b: LT9611 version: 0x40
[   44.743253] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   44.816433] msm_dpu ae01000.display-controller: bound ae94000.dsi 
(ops dsi_ops [msm])
[   44.843944] msm_dpu ae01000.display-controller: bound 
ae90000.displayport-controller (ops msm_dp_display_comp_ops [msm])
[   44.862886] adreno 3d00000.gpu: supply vdd not found, using dummy 
regulator
[   44.870974] adreno 3d00000.gpu: supply vddcx not found, using dummy 
regulator
[   44.897116] platform 3d6a000.gmu: Adding to iommu group 27
[   44.938871] msm_dpu ae01000.display-controller: bound 3d00000.gpu 
(ops a3xx_ops [msm])
[   44.952435] [drm:dpu_kms_hw_init:1156] dpu hardware revision:0x60000000
[   44.976496] [drm] Initialized msm 1.12.0 for 
ae01000.display-controller on minor 0
...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland