On Saturday 13 August 2022 23:02:34 Shinichiro Kawasaki wrote:

On Aug 13, 2022 / 14:50, Pali Rohár wrote:

quoted

On Saturday 13 August 2022 12:47:30 Shinichiro Kawasaki wrote:

quoted

On Aug 13, 2022 / 14:01, Pali Rohár wrote:

quoted

On Saturday 13 August 2022 11:53:39 Shinichiro Kawasaki wrote:

quoted

On Aug 13, 2022 / 11:32, Pali Rohár wrote:

quoted

On Saturday 13 August 2022 01:35:35 Shinichiro Kawasaki wrote:

[...]

quoted

Ok, thanks for testing. It looks like that reset code has some issues.

Could you please test this change?

@@ -1393,7 +1436,8 @@ static int mvebu_a3700_comphy_probe(struct platform_device *pdev)
 		 * To avoid relying on the bootloader/firmware configuration,
 		 * power off all comphys.
 		 */
-		mvebu_a3700_comphy_reset(phy);
+//		mvebu_a3700_comphy_reset(phy);
+		mvebu_a3700_comphy_power_off(phy);
 		lane->needs_reset = false;
 	}
 

It should replace reset code by power off at beginning / probe time.

This change also avoided the "ata1: SATA link down (SStatus 0 SControl 300)"
message, and my SSD was detected as /dev/sda. Good symptoms for me. FYI, I
attach the console log with this change. (This time, following messages were
printed. Not sure if they are important or not.)

[    2.937376] mvebu-a3700-comphy d0018300.phy: invalid COMPHY mode
[    2.943581] mvebu-a3700-comphy d0018300.phy: invalid COMPHY mode
[    2.949679] mvebu-a3700-comphy d0018300.phy: invalid COMPHY mode

Those errors are important, it means that power_off did nothing and was
skipped. So test did nothing.

Could you please replace mvebu_a3700_comphy_power_off implementation
with this one and try it again?

static int mvebu_a3700_comphy_power_off(struct phy *phy)
{
	struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);

	switch (lane->id) {
	case 0:
		mvebu_a3700_comphy_usb3_power_off(lane);
		mvebu_a3700_comphy_ethernet_power_off(lane);
		return 0;
	case 1:
		mvebu_a3700_comphy_pcie_power_off(lane);
		mvebu_a3700_comphy_ethernet_power_off(lane);
		return 0;
	case 2:
		mvebu_a3700_comphy_usb3_power_off(lane);
		mvebu_a3700_comphy_sata_power_off(lane);
		return 0;
	default:
		dev_err(lane->dev, "invalid COMPHY mode\n");
		return -EINVAL;
	}
}

Thanks, but this hunk failed to compile with the error below.

rivers/phy/marvell/phy-mvebu-a3700-comphy.c: In function 'mvebu_a3700_comphy_power_off':
drivers/phy/marvell/phy-mvebu-a3700-comphy.c:1229:17: error: implicit declaration of function 'mvebu_a3700_comphy_usb3_power_off'; did you mean 'mvebu_a3700_comphy_usb3_power_on'? [-Werror=implicit-function-declaration]
 1229 |                 mvebu_a3700_comphy_usb3_power_off(lane);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                 mvebu_a3700_comphy_usb3_power_on
cc1: some warnings being treated as errors

Do I need to add mvebu_a3700_comphy_usb3_power_off() function?

In my original version of this driver I have:

static void mvebu_a3700_comphy_usb3_power_off(struct mvebu_a3700_comphy_lane *lane)
{
	/*
	 * Currently the USB3 MAC will control the USB3 PHY to set it to low
	 * state, thus do not need to power off USB3 PHY again.
	 */
}

So please remove mvebu_a3700_comphy_usb3_power_off() calls as it does
nothing.

Okay, I tried with the edit and result looks positive:

- "invalid COMPHY mode" messages disappepared
- No "ata1: SATA link down (SStatus 0 SControl 300)" message
- My SSD was detected as /dev/sda

FYI, I attach console log and the patch I used.

-- 
Shin'ichiro Kawasaki

Perfect! So the issue is with mvebu_a3700_comphy_reset() function.

This function is not in TF-A code and neither in my original kernel
driver implementation (still available here):
https://git.kernel.org/pub/scm/linux/kernel/git/pali/linux.git/commit/?h=phy-mvebu-a3700-comphy&id=4588902a3528195bcfdda9f9e1e14262a1955df1

Marek, this function mvebu_a3700_comphy_reset() was implemented by you.
Could you please look at it, why you added this function and try to fix
it? Is this function needed at all?

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy