Hi Samuel,

On Fri 12 Aug 22, 17:31, Samuel Holland wrote:

Hi Paul,

On 8/12/22 7:03 AM, Paul Kocialkowski wrote:

quoted

On Fri 12 Aug 22, 02:56, Samuel Holland wrote:

quoted

The A100 variant of the DPHY requires configuring the analog registers
before setting the global enable bit. Since this order also works on the
other variants, always use it, to minimize the differences between them.

Did you get a chance to actually test this with either DSI/CSI-2 hardware?

I have tested DSI output with the Clockwork DevTerm (D1 SoC) and Pine64
PinePhone (A64 SoC). I do not have any MIPI CSI hardware to test with.

Sounds good to me then!

quoted

I vaguely remember that the order mattered. Do you have an idea of what the
Allwinner BSP does too?

The Allwinner BSP makes the same change as this commit in its "lowlevel_v2x"
copy of the code, which is used for R40 and T7 (original DPHY) and A100 and D1
(updated DPHY). It does not make the change in "lowlevel_sun50iw1" (A64 SoC,
original DPHY), but I tested A64 with this change, and it works fine.

Great, thanks for details.

quoted

Otherwise I could give it a try, at least with my MIPI CSI-2 setup
that uses the driver.

This commit only changes sun6i_dphy_tx_power_on(). The code for RX is unchanged
-- in fact, it already sets SUN6I_DPHY_GCTL_REG last.

Ah yes you're right, actually I remember being tempted to change this too when
adding the rx path, but didn't have hardware to easily test.

Thanks for the details, this is:

Reviewed-by: Paul Kocialkowski <redacted>

Cheers,

Paul

Regards,
Samuel

quoted

quoted

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
index 625c6e1e9990..9698d68d0db7 100644
--- a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
+++ b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c

@@ -183,10 +183,6 @@ static int sun6i_dphy_tx_power_on(struct sun6i_dphy *dphy)
 		     SUN6I_DPHY_TX_TIME4_HS_TX_ANA0(3) |
 		     SUN6I_DPHY_TX_TIME4_HS_TX_ANA1(3));
 
-	regmap_write(dphy->regs, SUN6I_DPHY_GCTL_REG,
-		     SUN6I_DPHY_GCTL_LANE_NUM(dphy->config.lanes) |
-		     SUN6I_DPHY_GCTL_EN);
-
 	regmap_write(dphy->regs, SUN6I_DPHY_ANA0_REG,
 		     SUN6I_DPHY_ANA0_REG_PWS |
 		     SUN6I_DPHY_ANA0_REG_DMPC |

@@ -244,6 +240,10 @@ static int sun6i_dphy_tx_power_on(struct sun6i_dphy *dphy)
 			   SUN6I_DPHY_ANA2_EN_P2S_CPU_MASK,
 			   SUN6I_DPHY_ANA2_EN_P2S_CPU(lanes_mask));
 
+	regmap_write(dphy->regs, SUN6I_DPHY_GCTL_REG,
+		     SUN6I_DPHY_GCTL_LANE_NUM(dphy->config.lanes) |
+		     SUN6I_DPHY_GCTL_EN);
+
 	return 0;
 }
 

-- 
2.35.1

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com