Skip to content

Commit 0a6fc70

Browse files
palivinodkoul
authored andcommitted
phy: marvell: phy-mvebu-a3700-comphy: Remove broken reset support
Reset support for SATA PHY is somehow broken and after calling it, kernel is not able to detect and initialize SATA disk Samsung SSD 850 EMT0 [1]. Reset support was introduced in commit 9343370 ("phy: marvell: phy-mvebu-a3700-comphy: Add native kernel implementation") as part of complete rewrite of this driver. v1 patch series of that commit [2] did not contain reset support and was tested that is working fine with Ethernet, SATA and USB PHYs without issues too. So for now remove broken reset support and change implementation of power_off callback to power off all functions on specified lane (and not only selected function) because during startup kernel does not know which function was selected and configured by bootloader. Same logic was used also in v1 patch series of that commit. This change fixes issues with initialization of SATA disk Samsung SSD 850 and disk is working again, like before mentioned commit. Once problem with PHY reset callback is solved its functionality could be re-introduced. But for now it is unknown why it does not work. [1] - https://lore.kernel.org/r/20220531124159.3e4lgn2v462irbtz@shindev/ [2] - https://lore.kernel.org/r/[email protected]/ Reported-by: Shinichiro Kawasaki <[email protected]> Fixes: 9343370 ("phy: marvell: phy-mvebu-a3700-comphy: Add native kernel implementation") Cc: [email protected] # v5.18+ Signed-off-by: Pali Rohár <[email protected]> Tested-by: Shinichiro Kawasaki <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Vinod Koul <[email protected]>
1 parent 568035b commit 0a6fc70

File tree

1 file changed

+17
-70
lines changed

1 file changed

+17
-70
lines changed

drivers/phy/marvell/phy-mvebu-a3700-comphy.c

Lines changed: 17 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,6 @@ struct mvebu_a3700_comphy_lane {
274274
int submode;
275275
bool invert_tx;
276276
bool invert_rx;
277-
bool needs_reset;
278277
};
279278

280279
struct gbe_phy_init_data_fix {
@@ -1097,40 +1096,12 @@ mvebu_a3700_comphy_pcie_power_off(struct mvebu_a3700_comphy_lane *lane)
10971096
0x0, PU_PLL_BIT | PU_RX_BIT | PU_TX_BIT);
10981097
}
10991098

1100-
static int mvebu_a3700_comphy_reset(struct phy *phy)
1099+
static void mvebu_a3700_comphy_usb3_power_off(struct mvebu_a3700_comphy_lane *lane)
11011100
{
1102-
struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);
1103-
u16 mask, data;
1104-
1105-
dev_dbg(lane->dev, "resetting lane %d\n", lane->id);
1106-
1107-
/* COMPHY reset for internal logic */
1108-
comphy_lane_reg_set(lane, COMPHY_SFT_RESET,
1109-
SFT_RST_NO_REG, SFT_RST_NO_REG);
1110-
1111-
/* COMPHY register reset (cleared automatically) */
1112-
comphy_lane_reg_set(lane, COMPHY_SFT_RESET, SFT_RST, SFT_RST);
1113-
1114-
/* PIPE soft and register reset */
1115-
data = PIPE_SOFT_RESET | PIPE_REG_RESET;
1116-
mask = data;
1117-
comphy_lane_reg_set(lane, COMPHY_PIPE_RST_CLK_CTRL, data, mask);
1118-
1119-
/* Release PIPE register reset */
1120-
comphy_lane_reg_set(lane, COMPHY_PIPE_RST_CLK_CTRL,
1121-
0x0, PIPE_REG_RESET);
1122-
1123-
/* Reset SB configuration register (only for lanes 0 and 1) */
1124-
if (lane->id == 0 || lane->id == 1) {
1125-
u32 mask, data;
1126-
1127-
data = PIN_RESET_CORE_BIT | PIN_RESET_COMPHY_BIT |
1128-
PIN_PU_PLL_BIT | PIN_PU_RX_BIT | PIN_PU_TX_BIT;
1129-
mask = data | PIN_PU_IVREF_BIT | PIN_TX_IDLE_BIT;
1130-
comphy_periph_reg_set(lane, COMPHY_PHY_CFG1, data, mask);
1131-
}
1132-
1133-
return 0;
1101+
/*
1102+
* The USB3 MAC sets the USB3 PHY to low state, so we do not
1103+
* need to power off USB3 PHY again.
1104+
*/
11341105
}
11351106

11361107
static bool mvebu_a3700_comphy_check_mode(int lane,
@@ -1171,10 +1142,6 @@ static int mvebu_a3700_comphy_set_mode(struct phy *phy, enum phy_mode mode,
11711142
(lane->mode != mode || lane->submode != submode))
11721143
return -EBUSY;
11731144

1174-
/* If changing mode, ensure reset is called */
1175-
if (lane->mode != PHY_MODE_INVALID && lane->mode != mode)
1176-
lane->needs_reset = true;
1177-
11781145
/* Just remember the mode, ->power_on() will do the real setup */
11791146
lane->mode = mode;
11801147
lane->submode = submode;
@@ -1185,22 +1152,13 @@ static int mvebu_a3700_comphy_set_mode(struct phy *phy, enum phy_mode mode,
11851152
static int mvebu_a3700_comphy_power_on(struct phy *phy)
11861153
{
11871154
struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);
1188-
int ret;
11891155

11901156
if (!mvebu_a3700_comphy_check_mode(lane->id, lane->mode,
11911157
lane->submode)) {
11921158
dev_err(lane->dev, "invalid COMPHY mode\n");
11931159
return -EINVAL;
11941160
}
11951161

1196-
if (lane->needs_reset) {
1197-
ret = mvebu_a3700_comphy_reset(phy);
1198-
if (ret)
1199-
return ret;
1200-
1201-
lane->needs_reset = false;
1202-
}
1203-
12041162
switch (lane->mode) {
12051163
case PHY_MODE_USB_HOST_SS:
12061164
dev_dbg(lane->dev, "set lane %d to USB3 host mode\n", lane->id);
@@ -1224,38 +1182,28 @@ static int mvebu_a3700_comphy_power_off(struct phy *phy)
12241182
{
12251183
struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);
12261184

1227-
switch (lane->mode) {
1228-
case PHY_MODE_USB_HOST_SS:
1229-
/*
1230-
* The USB3 MAC sets the USB3 PHY to low state, so we do not
1231-
* need to power off USB3 PHY again.
1232-
*/
1233-
break;
1234-
1235-
case PHY_MODE_SATA:
1236-
mvebu_a3700_comphy_sata_power_off(lane);
1237-
break;
1238-
1239-
case PHY_MODE_ETHERNET:
1185+
switch (lane->id) {
1186+
case 0:
1187+
mvebu_a3700_comphy_usb3_power_off(lane);
12401188
mvebu_a3700_comphy_ethernet_power_off(lane);
1241-
break;
1242-
1243-
case PHY_MODE_PCIE:
1189+
return 0;
1190+
case 1:
12441191
mvebu_a3700_comphy_pcie_power_off(lane);
1245-
break;
1246-
1192+
mvebu_a3700_comphy_ethernet_power_off(lane);
1193+
return 0;
1194+
case 2:
1195+
mvebu_a3700_comphy_usb3_power_off(lane);
1196+
mvebu_a3700_comphy_sata_power_off(lane);
1197+
return 0;
12471198
default:
12481199
dev_err(lane->dev, "invalid COMPHY mode\n");
12491200
return -EINVAL;
12501201
}
1251-
1252-
return 0;
12531202
}
12541203

12551204
static const struct phy_ops mvebu_a3700_comphy_ops = {
12561205
.power_on = mvebu_a3700_comphy_power_on,
12571206
.power_off = mvebu_a3700_comphy_power_off,
1258-
.reset = mvebu_a3700_comphy_reset,
12591207
.set_mode = mvebu_a3700_comphy_set_mode,
12601208
.owner = THIS_MODULE,
12611209
};
@@ -1393,8 +1341,7 @@ static int mvebu_a3700_comphy_probe(struct platform_device *pdev)
13931341
* To avoid relying on the bootloader/firmware configuration,
13941342
* power off all comphys.
13951343
*/
1396-
mvebu_a3700_comphy_reset(phy);
1397-
lane->needs_reset = false;
1344+
mvebu_a3700_comphy_power_off(phy);
13981345
}
13991346

14001347
provider = devm_of_phy_provider_register(&pdev->dev,

0 commit comments

Comments
 (0)