Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Sleeping in Atomic Context - [Low] Dead Code / Hardware Capability Restriction --
--- Patch [2]: [PATCH v2 2/2] phy: mediatek: Add support for MT8196 MIPI DSI PHY --- commit 290d9427c436e302c4aba5a26f5e3e1e9bb728f5 Author: AngeloGioacchino Del Regno <[email protected]> phy: mediatek: Add support for MT8196 MIPI DSI PHY Add support for the MIPI DSI PHY found in the MediaTek MT8196 SoC and its variants. This PHY has a different register layout and provides support for more hardware features compared to the previous generation. > diff --git a/drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8196.c > b/drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8196.c > new file mode 100644 > index 0000000000000..273f236fa7e9e > --- /dev/null > +++ b/drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8196.c > @@ -0,0 +1,196 @@ [ ... ] > +static int mtk_mipi_tx_pll_enable(struct clk_hw *hw) > +{ > + struct mtk_mipi_tx *mipi_tx = mtk_mipi_tx_from_clk_hw(hw); > + void __iomem *base = mipi_tx->regs; > + u32 voltage = RG_DSI_PRD_REF_MINI; > + u32 pres = MIPITX_PRESERVED_MINI; > + unsigned long long pcw_calc; > + unsigned int txdiv, txdiv0; > + u32 pcw; > + > + dev_dbg(mipi_tx->dev, "enable: %u bps\n", mipi_tx->data_rate); > + > + if (mipi_tx->data_rate >= 2000000000) { > + /* Select higher signaling voltage for fast data rates */ > + voltage = RG_DSI_PRD_REF_DEF; [Severity: Low] Is this block reachable? Later in mtk_mipi_tx_pll_determine_rate(), req->rate is clamped to a maximum of 1,600,000,000 Hz. If the hardware supports rates above 2.0 GHz, does the clamp limit need to be increased, or is this dead code? > + pres = MIPITX_PRESERVED_DEF; > + txdiv = 1; > + txdiv0 = 0; > + } else if (mipi_tx->data_rate >= 1000000000) { [ ... ] > + mtk_phy_set_bits(base + MIPITX_PLL_PWR, AD_DSI_PLL_SDM_PWR_ON); > + mtk_phy_clear_bits(base + MIPITX_PLL_CON1, RG_DSI_PLL_EN); > + usleep_range(30, 60); [Severity: High] Does this code sleep in an atomic context? The mtk_mipi_tx_pll_enable() function is registered as the .enable callback in mtk_mipi_tx_pll_ops below. The Common Clock Framework invokes .enable under an enable_lock spinlock with interrupts disabled. Calling usleep_range() here, which internally calls schedule(), will lead to scheduling while atomic. Should this use an atomic-safe delay like udelay(30), or should this logic be moved to the sleepable .prepare callback? > + > + mtk_phy_clear_bits(base + MIPITX_PLL_PWR, AD_DSI_PLL_SDM_ISO_EN); > + writel(pcw, base + MIPITX_PLL_CON0); > + mtk_phy_update_field(base + MIPITX_PLL_CON1, RG_DSI_PLL_POSDIV, txdiv0); > + usleep_range(30, 60); > + > + mtk_phy_set_bits(base + MIPITX_PLL_CON1, RG_DSI_PLL_EN); > + usleep_range(30, 60); > + > + return 0; > +} [ ... ] > +static int mtk_mipi_tx_pll_determine_rate(struct clk_hw *hw, > + struct clk_rate_request *req) > +{ > + req->rate = clamp_val(req->rate, 125000000, 1600000000); > + > + return 0; > +} > + > +static const struct clk_ops mtk_mipi_tx_pll_ops = { > + .enable = mtk_mipi_tx_pll_enable, > + .disable = mtk_mipi_tx_pll_disable, > + .determine_rate = mtk_mipi_tx_pll_determine_rate, > + .set_rate = mtk_mipi_tx_pll_set_rate, > + .recalc_rate = mtk_mipi_tx_pll_recalc_rate, > +}; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
