<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">On 17.01.2025 18:28, Andy Shevchenko
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:Z4qTQ9ypkX6iS1Pl@smile.fi.intel.com">
      <pre wrap="" class="moz-quote-pre">On Fri, Jan 17, 2025 at 05:05:42PM +0100, Marek Szyprowski wrote:
</pre>
      <blockquote type="cite">
        <pre wrap="" class="moz-quote-pre">On 17.01.2025 15:30, Andy Shevchenko wrote:
</pre>
        <blockquote type="cite">
          <pre wrap="" class="moz-quote-pre">On Fri, Jan 17, 2025 at 04:09:58PM +0200, Andy Shevchenko wrote:
</pre>
          <blockquote type="cite">
            <pre wrap="" class="moz-quote-pre">On Fri, Jan 17, 2025 at 02:57:52PM +0100, Marek Szyprowski wrote:
</pre>
            <blockquote type="cite">
              <pre wrap="" class="moz-quote-pre">On 16.01.2025 13:42, Andy Shevchenko wrote:
</pre>
              <blockquote type="cite">
                <pre wrap="" class="moz-quote-pre">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 <a class="moz-txt-link-rfc2396E" href="mailto:andriy.shevchenko@linux.intel.com"><andriy.shevchenko@linux.intel.com></a>
</pre>
              </blockquote>
              <pre wrap="" class="moz-quote-pre">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.
</pre>
            </blockquote>
            <pre wrap="" class="moz-quote-pre">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.)

</pre>
            <blockquote type="cite">
              <pre wrap="" class="moz-quote-pre">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.
</pre>
            </blockquote>
            <pre wrap="" class="moz-quote-pre">Yes, see below. And thank you for your report!
</pre>
          </blockquote>
        </blockquote>
      </blockquote>
      <pre wrap="" class="moz-quote-pre">
...

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <blockquote type="cite">
            <blockquote type="cite">
              <blockquote type="cite">
                <pre wrap="" class="moz-quote-pre">+          /*
+                * 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);
</pre>
              </blockquote>
            </blockquote>
            <pre wrap="" class="moz-quote-pre">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?
</pre>
          </blockquote>
          <pre wrap="" class="moz-quote-pre">Also try to apply this patch (while having the above lines not commented):
</pre>
        </blockquote>
        <pre wrap="" class="moz-quote-pre">
This patch applied alone doesn't change anything. Probe still fails.

However, with the mentioned 3 lines in the regmap code commented AND 
this patch applied, lt9611uxc driver probe also fails.
</pre>
      </blockquote>
      <pre wrap="" class="moz-quote-pre">
Does it fail in the same way?</pre>
    </blockquote>
    <p>Yes, the hw revision is reported as zero in this case: LT9611
      revision: 0x00.00.00
    </p>
    <p><span style="white-space: pre-wrap">
</span></p>
    <blockquote type="cite"
      cite="mid:Z4qTQ9ypkX6iS1Pl@smile.fi.intel.com">
      <blockquote type="cite">
        <pre wrap="" class="moz-quote-pre">Does it mean that there is really a bug in the driver?
</pre>
      </blockquote>
      <pre wrap="" class="moz-quote-pre">
Without looking at the datasheet it's hard to say. At least what I found so far
is one page of the I²C traffic dump on Windows as an example how to use their
evaluation board and software, but it doesn't unveil the bigger picture. At
least what I think is going on here is that the programming is not so easy as
just paging. Something is more complicated there.

But at least (and as Mark said) the most of the regmap based drivers got
the ranges wrong (so, at least there is one bug in the driver).</pre>
    </blockquote>
    <p>I can do more experiments if this helps. Do you need a dump of
      all regmap accesses or i2c traffic from this driver?<br>
    </p>
    <p><span style="white-space: pre-wrap">
</span></p>
    <blockquote type="cite"
      cite="mid:Z4qTQ9ypkX6iS1Pl@smile.fi.intel.com">
      <blockquote type="cite">
        <blockquote type="cite">
          <pre wrap="" class="moz-quote-pre">--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -69,7 +69,7 @@ struct lt9611uxc {
  static const struct regmap_range_cfg lt9611uxc_ranges[] = {
        {
                .name = "register_range",
-               .range_min =  0,
+               .range_min = 0x0100,
                .range_max = 0xd0ff,
                .selector_reg = LT9611_PAGE_CONTROL,
                .selector_mask = 0xff,
</pre>
        </blockquote>
      </blockquote>
      <pre wrap="" class="moz-quote-pre">
</pre>
    </blockquote>
    <pre class="moz-signature" cols="72">Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland</pre>
  <table id=bannersignimg data-cui-lock="true" namo_lock><tr><td><p> </p>
</td></tr></table><table id=confidentialsignimg data-cui-lock="true" namo_lock><tr><td><p><img unselectable="on" data-cui-image="true" style="display: inline-block; border: 0px solid; width: 520px; height: 144px;" src="cid:cafe_image_0@s-core.co.kr"><br></p>
</td></tr></table></body>
</html>
<table style='display: none;'><tbody><tr><td><img style='display: none;' border=0 src='http://ext.w1.samsung.net/mail/ext/v1/external/status/update?userid=m.szyprowski&do=bWFpbElEPTIwMjUwMTIxMDczMjIxZXVjYXMxcDJhMGYwMzhjY2U0YTA5ZTNjMjkzMDBjMjUzNTRjMDRiNiZyZWNpcGllbnRBZGRyZXNzPWRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmc_' width=0 height=0></td></tr></tbody></table>