On Thu, 25 Aug 2022 15:00:59 +0200 Pali Rohár wrote: > On Sunday 14 August 2022 01:10:50 Pali Rohár wrote: > > On Saturday 13 August 2022 23:02:34 Shinichiro Kawasaki wrote: > > > On Aug 13, 2022 / 14:50, Pali Rohár wrote: > > > > On Saturday 13 August 2022 12:47:30 Shinichiro Kawasaki wrote: > > > > > On Aug 13, 2022 / 14:01, Pali Rohár wrote: > > > > > > On Saturday 13 August 2022 11:53:39 Shinichiro Kawasaki wrote: > > > > > > > On Aug 13, 2022 / 11:32, Pali Rohár wrote: > > > > > > > > On Saturday 13 August 2022 01:35:35 Shinichiro Kawasaki wrote: > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > > 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? > > PING? Any progress here? Not yet, sorry. Maybe we could do something like I am attaching, until I have time to play with it? See attached file. Marek