From 0a6fc70d76bddf98278af2ac000379c82aec8f11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 29 Aug 2022 10:30:46 +0200 Subject: [PATCH 1/3] phy: marvell: phy-mvebu-a3700-comphy: Remove broken reset support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 934337080c6c ("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/20211028184242.22105-1-kabel@kernel.org/ Reported-by: Shinichiro Kawasaki Fixes: 934337080c6c ("phy: marvell: phy-mvebu-a3700-comphy: Add native kernel implementation") Cc: stable@vger.kernel.org # v5.18+ Signed-off-by: Pali Rohár Tested-by: Shinichiro Kawasaki Link: https://lore.kernel.org/r/20220829083046.15082-1-pali@kernel.org Signed-off-by: Vinod Koul --- drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 87 ++++---------------- 1 file changed, 17 insertions(+), 70 deletions(-) diff --git a/drivers/phy/marvell/phy-mvebu-a3700-comphy.c b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c index a4d7d9bd100d3..67712c77d806f 100644 --- a/drivers/phy/marvell/phy-mvebu-a3700-comphy.c +++ b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c @@ -274,7 +274,6 @@ struct mvebu_a3700_comphy_lane { int submode; bool invert_tx; bool invert_rx; - bool needs_reset; }; struct gbe_phy_init_data_fix { @@ -1097,40 +1096,12 @@ mvebu_a3700_comphy_pcie_power_off(struct mvebu_a3700_comphy_lane *lane) 0x0, PU_PLL_BIT | PU_RX_BIT | PU_TX_BIT); } -static int mvebu_a3700_comphy_reset(struct phy *phy) +static void mvebu_a3700_comphy_usb3_power_off(struct mvebu_a3700_comphy_lane *lane) { - struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy); - u16 mask, data; - - dev_dbg(lane->dev, "resetting lane %d\n", lane->id); - - /* COMPHY reset for internal logic */ - comphy_lane_reg_set(lane, COMPHY_SFT_RESET, - SFT_RST_NO_REG, SFT_RST_NO_REG); - - /* COMPHY register reset (cleared automatically) */ - comphy_lane_reg_set(lane, COMPHY_SFT_RESET, SFT_RST, SFT_RST); - - /* PIPE soft and register reset */ - data = PIPE_SOFT_RESET | PIPE_REG_RESET; - mask = data; - comphy_lane_reg_set(lane, COMPHY_PIPE_RST_CLK_CTRL, data, mask); - - /* Release PIPE register reset */ - comphy_lane_reg_set(lane, COMPHY_PIPE_RST_CLK_CTRL, - 0x0, PIPE_REG_RESET); - - /* Reset SB configuration register (only for lanes 0 and 1) */ - if (lane->id == 0 || lane->id == 1) { - u32 mask, data; - - data = PIN_RESET_CORE_BIT | PIN_RESET_COMPHY_BIT | - PIN_PU_PLL_BIT | PIN_PU_RX_BIT | PIN_PU_TX_BIT; - mask = data | PIN_PU_IVREF_BIT | PIN_TX_IDLE_BIT; - comphy_periph_reg_set(lane, COMPHY_PHY_CFG1, data, mask); - } - - return 0; + /* + * The USB3 MAC sets the USB3 PHY to low state, so we do not + * need to power off USB3 PHY again. + */ } 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, (lane->mode != mode || lane->submode != submode)) return -EBUSY; - /* If changing mode, ensure reset is called */ - if (lane->mode != PHY_MODE_INVALID && lane->mode != mode) - lane->needs_reset = true; - /* Just remember the mode, ->power_on() will do the real setup */ lane->mode = mode; lane->submode = submode; @@ -1185,7 +1152,6 @@ static int mvebu_a3700_comphy_set_mode(struct phy *phy, enum phy_mode mode, static int mvebu_a3700_comphy_power_on(struct phy *phy) { struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy); - int ret; if (!mvebu_a3700_comphy_check_mode(lane->id, lane->mode, lane->submode)) { @@ -1193,14 +1159,6 @@ static int mvebu_a3700_comphy_power_on(struct phy *phy) return -EINVAL; } - if (lane->needs_reset) { - ret = mvebu_a3700_comphy_reset(phy); - if (ret) - return ret; - - lane->needs_reset = false; - } - switch (lane->mode) { case PHY_MODE_USB_HOST_SS: 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) { struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy); - switch (lane->mode) { - case PHY_MODE_USB_HOST_SS: - /* - * The USB3 MAC sets the USB3 PHY to low state, so we do not - * need to power off USB3 PHY again. - */ - break; - - case PHY_MODE_SATA: - mvebu_a3700_comphy_sata_power_off(lane); - break; - - case PHY_MODE_ETHERNET: + switch (lane->id) { + case 0: + mvebu_a3700_comphy_usb3_power_off(lane); mvebu_a3700_comphy_ethernet_power_off(lane); - break; - - case PHY_MODE_PCIE: + return 0; + case 1: mvebu_a3700_comphy_pcie_power_off(lane); - break; - + 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; } - - return 0; } static const struct phy_ops mvebu_a3700_comphy_ops = { .power_on = mvebu_a3700_comphy_power_on, .power_off = mvebu_a3700_comphy_power_off, - .reset = mvebu_a3700_comphy_reset, .set_mode = mvebu_a3700_comphy_set_mode, .owner = THIS_MODULE, }; @@ -1393,8 +1341,7 @@ 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); - lane->needs_reset = false; + mvebu_a3700_comphy_power_off(phy); } provider = devm_of_phy_provider_register(&pdev->dev, From 468c9d928a8f38fdfaa61b05e81473cc7c8a6461 Mon Sep 17 00:00:00 2001 From: Russ Weight Date: Fri, 16 Sep 2022 16:52:05 -0700 Subject: [PATCH 2/3] fpga: m10bmc-sec: Fix possible memory leak of flash_buf There is an error check following the allocation of flash_buf that returns without freeing flash_buf. It makes more sense to do the error check before the allocation and the reordering eliminates the memory leak. Reported-by: kernel test robot Reported-by: Dan Carpenter Fixes: 154afa5c31cd ("fpga: m10bmc-sec: expose max10 flash update count") Signed-off-by: Russ Weight Reviewed-by: Tom Rix Acked-by: Xu Yilun Cc: Link: https://lore.kernel.org/r/20220916235205.106873-1-russell.h.weight@intel.com Signed-off-by: Xu Yilun --- drivers/fpga/intel-m10-bmc-sec-update.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c index 72c677c910de3..133e511355c9f 100644 --- a/drivers/fpga/intel-m10-bmc-sec-update.c +++ b/drivers/fpga/intel-m10-bmc-sec-update.c @@ -148,10 +148,6 @@ static ssize_t flash_count_show(struct device *dev, stride = regmap_get_reg_stride(sec->m10bmc->regmap); num_bits = FLASH_COUNT_SIZE * 8; - flash_buf = kmalloc(FLASH_COUNT_SIZE, GFP_KERNEL); - if (!flash_buf) - return -ENOMEM; - if (FLASH_COUNT_SIZE % stride) { dev_err(sec->dev, "FLASH_COUNT_SIZE (0x%x) not aligned to stride (0x%x)\n", @@ -160,6 +156,10 @@ static ssize_t flash_count_show(struct device *dev, return -EINVAL; } + flash_buf = kmalloc(FLASH_COUNT_SIZE, GFP_KERNEL); + if (!flash_buf) + return -ENOMEM; + ret = regmap_bulk_read(sec->m10bmc->regmap, STAGING_FLASH_COUNT, flash_buf, FLASH_COUNT_SIZE / stride); if (ret) { From 2bc54aaa65d2126ae629919175708a28ce7ef06e Mon Sep 17 00:00:00 2001 From: William Breathitt Gray Date: Thu, 22 Sep 2022 07:20:56 -0400 Subject: [PATCH 3/3] counter: 104-quad-8: Fix skipped IRQ lines during events configuration IRQ trigger configuration is skipped if it has already been set before; however, the IRQ line still needs to be OR'd to irq_enabled because irq_enabled is reset for every events_configure call. This patch moves the irq_enabled OR operation update to before the irq_trigger check so that IRQ line enablement is not skipped. Fixes: c95cc0d95702 ("counter: 104-quad-8: Fix persistent enabled events bug") Cc: stable Link: https://lore.kernel.org/r/20220815122301.2750-1-william.gray@linaro.org/ Signed-off-by: William Breathitt Gray Link: https://lore.kernel.org/r/179eed11eaf225dbd908993b510df0c8f67b1230.1663844776.git.william.gray@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/counter/104-quad-8.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c index 62c2b7ac4339f..4407203e0c9b3 100644 --- a/drivers/counter/104-quad-8.c +++ b/drivers/counter/104-quad-8.c @@ -449,6 +449,9 @@ static int quad8_events_configure(struct counter_device *counter) return -EINVAL; } + /* Enable IRQ line */ + irq_enabled |= BIT(event_node->channel); + /* Skip configuration if it is the same as previously set */ if (priv->irq_trigger[event_node->channel] == next_irq_trigger) continue; @@ -462,9 +465,6 @@ static int quad8_events_configure(struct counter_device *counter) priv->irq_trigger[event_node->channel] << 3; iowrite8(QUAD8_CTR_IOR | ior_cfg, &priv->reg->channel[event_node->channel].control); - - /* Enable IRQ line */ - irq_enabled |= BIT(event_node->channel); } iowrite8(irq_enabled, &priv->reg->index_interrupt);