[PATCH v3 00/12] PHY: MediaTek DP PHY refactor and MT8196 eDP

[PATCH v3 00/12] PHY: MediaTek DP PHY refactor and MT8196 eDP

[AngeloGioacchino Del Regno]

Changes in v3:
 - Added missing bitfield.h, pm_runtime.h includes (Sashiko)
 - Set calibration data before registering PHY (Sashiko)
 - Enable PM Runtime before creating PHY device (Sashiko)
 - Continue poweroff if some write fails, but still return failure
   as that would be a partial poweroff anyway (Sashiko)
 - Added zero driving params to fix default calibration data setting (Sashiko)
 - Added NULL check on device_Get_match_data (Sashiko)

Changes in v2:
 - Fixed register offsets for ANA phy in MT8196
 - Set/clear FORCE_VOLT_SWING_EN bit when configuring and resetting
   the voltage swing registers
 - Fixed endianness issue in call to nvmem_read_u16 (Sashiko)
 - Added early exit during lanes disablement if none are enabled to
   avoid out-of-bounds bitwise shift (Sashiko)
 - Removed useless call to pm_runtime_get_sync() in probe(), which
   also fixes issues pointed out by Sashiko
 - Fixed wrong dig vs ana register array usage in power_on/off (Sashiko)


This series performs major refactoring on the MediaTek DisplayPort PHY
driver, makes it probe with devicetree instead of getting registered
by the mtk_dp DRM driver, adds power_on/off() callbacks, and honors
the phy configure opts' set_lanes and set_voltages for, respectively,
varying the number of lanes and setting the voltage pre-emphasis and
swing on the PHY, for each lane.

This driver now also properly gets the PHY (EYE) Calibration Data from
NVMEM (eFuse array) if provided, instead of getting it (improperly)
injected by the mtk_dp driver.

Additionally, all of the driving parameters calculations and most of
the other register definitions were refactored to greatly enhance the
human readability of this code.

As a last step, this also transfers the register offsets for both the
digital and analog phy registers in arrays assigned to soc specific
data, in an effort to both introduce support for new minor revisions
of the MediaTek DisplayPort PHY and to have a clearer view of the
register related differences between those (for example, it is easily
understandable that the analog part remained exactly the same between
MT8195 and MT8196, but the digital part gets a slight update).

Speaking of which, as a last step, this also adds support for the
MT8196 SoC (and its derivatives), which uses this PHY only for its
Embedded DisplayPort (eDP) IP (spoiler: the DP one seems to be way
too different and requiring an entirely new PHY driver).

In this state, this driver can also easily support the MT8189 SoC
with a few lines of code: even though I do have clean code to add
support for this one, I was not (*yet*) able to test it on upstream
based kernels, and for this reason I decided to leave that one out
for now (but it's coming later for sure).

NOTE!
Despite all the apparently breaking changes in the refactoring, full
compatibility with older MTK_DP driver and with old devicetrees was
retained and carefully tested on multiple platforms!

P.S.: I am aware of the BUILD_DRIVING_PARAM_0( 0, 2, 4, 7) checkpatch
warning and I didn't fix it in bigger favor of human readability.

AngeloGioacchino Del Regno (12):
  dt-bindings: phy: Document MT8195 and MT8196 DisplayPort PHYs
  phy: phy-mtk-dp: Rename regs to regmap in struct mtk_dp_phy
  phy: phy-mtk-dp: Allow probing with devicetree match
  phy: phy-mtk-dp: Migrate register offsets to SoC specific pdata
  phy: phy-mtk-dp: Implement power_on and power_off PHY callbacks
  phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup
  phy: phy-mtk-dp: Support setting volt swing and preemphasis values
  phy: phy-mtk-dp: Add support for digital and analog calibration
  phy: phy-mtk-dp: Rewrite and document default driving param macros
  phy: phy-mtk-dp: Add bitrate register val definitions to SoC data
  phy: phy-mtk-dp: Add PHYD Lane EN register mask to SoC data
  phy: phy-mtk-dp: Add support for MT8196 eDP PHY

 .../bindings/phy/mediatek,mt8195-dp-phy.yaml  |  77 ++
 drivers/phy/mediatek/phy-mtk-dp.c             | 882 ++++++++++++++++--
 2 files changed, 862 insertions(+), 97 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt8195-dp-phy.yaml

-- 
2.54.0


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

[PATCH v3 01/12] dt-bindings: phy: Document MT8195 and MT8196 DisplayPort PHYs

[AngeloGioacchino Del Regno]

This adds bindings for the DisplayPort and Embedded DisplayPort
PHYs found in the MediaTek MT8195 SoC (and variants of) and for
the Embedded DisplayPort found in the MT8196 SoC (and variants).

This PHY supports varying impedance calibrations for the various
signals to reach an optimal EYE signal pattern for any specific
board(s), especially useful for very high bitrates such as HBR3
and higher, depending on board design.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../bindings/phy/mediatek,mt8195-dp-phy.yaml  | 77 +++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt8195-dp-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt8195-dp-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt8195-dp-phy.yaml
new file mode 100644
index 000000000000..1c2dc4f03cd6
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mediatek,mt8195-dp-phy.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/mediatek,mt8195-dp-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek SoC DisplayPort Transmitter PHY
+
+maintainers:
+  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+
+properties:
+  compatible:
+    enum:
+      - mediatek,mt8195-dp-phy
+      - mediatek,mt8196-edp-phy
+
+  reg:
+    maxItems: 1
+
+  "#phy-cells":
+    const: 0
+
+  nvmem-cells:
+    description: PHY calibrations from eFuse for optimal EYE signal pattern
+    items:
+      - description: PHY-Global Reference Bias trim
+      - description: PHY-Global AUX Transmitter clock impedance adjustment
+      - description: Lane 0 Transmitter impedance selection (P-MOSFET)
+      - description: Lane 0 Transmitter impedance selection (N-MOSFET)
+      - description: Lane 1 Transmitter impedance selection (P-MOSFET)
+      - description: Lane 1 Transmitter impedance selection (N-MOSFET)
+      - description: Lane 2 Transmitter impedance selection (P-MOSFET)
+      - description: Lane 2 Transmitter impedance selection (N-MOSFET)
+      - description: Lane 3 Transmitter impedance selection (P-MOSFET)
+      - description: Lane 3 Transmitter impedance selection (N-MOSFET)
+
+  nvmem-cell-names:
+    items:
+      - const: rbias-trim
+      - const: impedance-txclk
+      - const: impedance-lane0p
+      - const: impedance-lane0n
+      - const: impedance-lane1p
+      - const: impedance-lane1n
+      - const: impedance-lane2p
+      - const: impedance-lane2n
+      - const: impedance-lane3p
+      - const: impedance-lane3n
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    phy@1c500000 {
+        compatible = "mediatek,mt8195-dp-phy";
+        reg = <0x1c500000 0x2000>;
+        #phy-cells = <0>;
+        nvmem-cells = <&edp_glb_bias_trim>, <&edp_clktx_impsel>,
+                      <&edp_imp_ln0_pmos>, <&edp_imp_ln0_nmos>,
+                      <&edp_imp_ln1_pmos>, <&edp_imp_ln1_nmos>,
+                      <&edp_imp_ln2_pmos>, <&edp_imp_ln2_nmos>,
+                      <&edp_imp_ln3_pmos>, <&edp_imp_ln3_nmos>;
+        nvmem-cell-names = "rbias-trim", "impedance-txclk",
+                           "impedance-lane0p", "impedance-lane0n",
+                           "impedance-lane1p", "impedance-lane1n",
+                           "impedance-lane2p", "impedance-lane2n",
+                           "impedance-lane3p", "impedance-lane3n";
+    };
-- 
2.54.0


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

Re: [PATCH v3 01/12] dt-bindings: phy: Document MT8195 and MT8196 DisplayPort PHYs

[Krzysztof Kozlowski]

On Tue, Jul 07, 2026 at 05:42:34PM +0200, AngeloGioacchino Del Regno wrote:
> This adds bindings for the DisplayPort and Embedded DisplayPort

"Add binding"


> PHYs found in the MediaTek MT8195 SoC (and variants of) and for
> the Embedded DisplayPort found in the MT8196 SoC (and variants).
> 
> This PHY supports varying impedance calibrations for the various
> signals to reach an optimal EYE signal pattern for any specific
> board(s), especially useful for very high bitrates such as HBR3
> and higher, depending on board design.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  .../bindings/phy/mediatek,mt8195-dp-phy.yaml  | 77 +++++++++++++++++++

I guess this answers my question about deprecated nvmem which I sent
some minutes ago. I did not see any lore link in that cover letter,
which would be nice in that case.

Anyway,

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof


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

[PATCH v3 02/12] phy: phy-mtk-dp: Rename regs to regmap in struct mtk_dp_phy

[AngeloGioacchino Del Regno]

In preparation to perform further cleanups and to extend the
driver to support more SoCs, rename the `regs` member to `regmap`
to improve readability, as this is a common name across many
kernel drivers for a struct regmap.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-dp.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index d7024a144335..bf7b3a95e72d 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -79,7 +79,7 @@
 				 XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
 
 struct mtk_dp_phy {
-	struct regmap *regs;
+	struct regmap *regmap;
 };
 
 static int mtk_dp_phy_init(struct phy *phy)
@@ -94,13 +94,13 @@ static int mtk_dp_phy_init(struct phy *phy)
 		DRIVING_PARAM_8_DEFAULT
 	};
 
-	regmap_bulk_write(dp_phy->regs, MTK_DP_LANE0_DRIVING_PARAM_3,
+	regmap_bulk_write(dp_phy->regmap, MTK_DP_LANE0_DRIVING_PARAM_3,
 			  driving_params, ARRAY_SIZE(driving_params));
-	regmap_bulk_write(dp_phy->regs, MTK_DP_LANE1_DRIVING_PARAM_3,
+	regmap_bulk_write(dp_phy->regmap, MTK_DP_LANE1_DRIVING_PARAM_3,
 			  driving_params, ARRAY_SIZE(driving_params));
-	regmap_bulk_write(dp_phy->regs, MTK_DP_LANE2_DRIVING_PARAM_3,
+	regmap_bulk_write(dp_phy->regmap, MTK_DP_LANE2_DRIVING_PARAM_3,
 			  driving_params, ARRAY_SIZE(driving_params));
-	regmap_bulk_write(dp_phy->regs, MTK_DP_LANE3_DRIVING_PARAM_3,
+	regmap_bulk_write(dp_phy->regmap, MTK_DP_LANE3_DRIVING_PARAM_3,
 			  driving_params, ARRAY_SIZE(driving_params));
 
 	return 0;
@@ -131,10 +131,10 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
 			val = BIT_RATE_HBR3;
 			break;
 		}
-		regmap_write(dp_phy->regs, MTK_DP_PHY_DIG_BIT_RATE, val);
+		regmap_write(dp_phy->regmap, MTK_DP_PHY_DIG_BIT_RATE, val);
 	}
 
-	regmap_update_bits(dp_phy->regs, MTK_DP_PHY_DIG_PLL_CTL_1,
+	regmap_update_bits(dp_phy->regmap, MTK_DP_PHY_DIG_PLL_CTL_1,
 			   TPLL_SSC_EN, opts->dp.ssc ? TPLL_SSC_EN : 0);
 
 	return 0;
@@ -144,10 +144,10 @@ static int mtk_dp_phy_reset(struct phy *phy)
 {
 	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
 
-	regmap_update_bits(dp_phy->regs, MTK_DP_PHY_DIG_SW_RST,
+	regmap_update_bits(dp_phy->regmap, MTK_DP_PHY_DIG_SW_RST,
 			   DP_GLB_SW_RST_PHYD, 0);
 	usleep_range(50, 200);
-	regmap_update_bits(dp_phy->regs, MTK_DP_PHY_DIG_SW_RST,
+	regmap_update_bits(dp_phy->regmap, MTK_DP_PHY_DIG_SW_RST,
 			   DP_GLB_SW_RST_PHYD, 1);
 
 	return 0;
@@ -176,7 +176,7 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
 	if (!dp_phy)
 		return -ENOMEM;
 
-	dp_phy->regs = regs;
+	dp_phy->regmap = regs;
 	phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
 	if (IS_ERR(phy))
 		return dev_err_probe(dev, PTR_ERR(phy),
-- 
2.54.0


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

[PATCH v3 03/12] phy: phy-mtk-dp: Allow probing with devicetree match

[AngeloGioacchino Del Regno]

Make it possible to decouple the registration of the DisplayPort
PHY driver from the DisplayPort IP driver by adding a devicetree
match to probe the PHY, registering an OF PHY provider and this
device's own MMIO regmap - if, and only if, this PHY driver was
registered with an OF match.

In order to retain compatibility with older devicetrees that are
not declaring the DisplayPort PHY as a separate node, the legacy
code was moved in a `mtk_dp_phy_legacy_probe()` function, which
gets called if the driver was registered by the DisplayPort one.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-dp.c | 88 +++++++++++++++++++++++++++----
 1 file changed, 78 insertions(+), 10 deletions(-)

diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index bf7b3a95e72d..d241004e2828 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -12,6 +12,7 @@
 #include <linux/of.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
 #define PHY_OFFSET			0x1000
@@ -79,6 +80,7 @@
 				 XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
 
 struct mtk_dp_phy {
+	struct device *dev;
 	struct regmap *regmap;
 };
 
@@ -160,43 +162,109 @@ static const struct phy_ops mtk_dp_phy_dev_ops = {
 	.owner = THIS_MODULE,
 };
 
+static void mtk_dp_phy_legacy_remove_lookup(void *data)
+{
+	struct phy *phy = data;
+	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+
+	phy_remove_lookup(phy, "dp", dev_name(dp_phy->dev));
+}
+
+static int mtk_dp_phy_legacy_probe(struct platform_device *pdev, struct mtk_dp_phy *dp_phy)
+{
+	struct device *dev = &pdev->dev;
+	struct phy *phy;
+	int ret;
+
+	dp_phy->regmap = *(struct regmap **)dev->platform_data;
+	if (!dp_phy->regmap)
+		return dev_err_probe(dev, -EINVAL, "No platform data available\n");
+
+	phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
+	if (IS_ERR(phy))
+		return dev_err_probe(dev, PTR_ERR(phy),
+				     "Failed to create DP PHY\n");
+
+	phy_set_drvdata(phy, dp_phy);
+	ret = phy_create_lookup(phy, "dp", dev_name(dev));
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, mtk_dp_phy_legacy_remove_lookup, phy);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct regmap_config mtk_dp_phy_regmap_cfg = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.disable_locking = true,
+};
+
 static int mtk_dp_phy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct phy_provider *provider;
 	struct mtk_dp_phy *dp_phy;
+	void __iomem *base;
 	struct phy *phy;
-	struct regmap *regs;
-
-	regs = *(struct regmap **)dev->platform_data;
-	if (!regs)
-		return dev_err_probe(dev, -EINVAL,
-				     "No data passed, requires struct regmap**\n");
+	int ret;
 
 	dp_phy = devm_kzalloc(dev, sizeof(*dp_phy), GFP_KERNEL);
 	if (!dp_phy)
 		return -ENOMEM;
 
-	dp_phy->regmap = regs;
+	dp_phy->dev = dev;
+
+	/* If there's no devicetree, go for legacy pdev probe */
+	if (!dev->of_node)
+		return mtk_dp_phy_legacy_probe(pdev, dp_phy);
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	dp_phy->regmap = devm_regmap_init_mmio(dev, base, &mtk_dp_phy_regmap_cfg);
+	if (IS_ERR(dp_phy->regmap))
+		return PTR_ERR(dp_phy->regmap);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
 	phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
 	if (IS_ERR(phy))
 		return dev_err_probe(dev, PTR_ERR(phy),
 				     "Failed to create DP PHY\n");
 
 	phy_set_drvdata(phy, dp_phy);
-	if (!dev->of_node)
-		phy_create_lookup(phy, "dp", dev_name(dev));
+
+	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(provider))
+		return PTR_ERR(provider);
 
 	return 0;
 }
 
+static const struct of_device_id mtk_dp_phy_of_match[] = {
+	{ .compatible = "mediatek,mt8195-dp-phy" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk_dp_phy_of_match);
+
 static struct platform_driver mtk_dp_phy_driver = {
 	.probe = mtk_dp_phy_probe,
 	.driver = {
 		.name = "mediatek-dp-phy",
+		.of_match_table = mtk_dp_phy_of_match,
 	},
 };
 module_platform_driver(mtk_dp_phy_driver);
 
+MODULE_AUTHOR("AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>");
 MODULE_AUTHOR("Markus Schneider-Pargmann <msp@baylibre.com>");
-MODULE_DESCRIPTION("MediaTek DP PHY Driver");
+MODULE_DESCRIPTION("MediaTek DisplayPort PHY Driver");
 MODULE_LICENSE("GPL");
-- 
2.54.0


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

Re: [PATCH v3 03/12] phy: phy-mtk-dp: Allow probing with devicetree match

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Hardcoded `PHY_OFFSET` causes out-of-bounds register accesses or resource conflicts when the PHY is probed as a standalone devicetree node.
- [High] Hardware registers are accessed in `mtk_dp_phy_configure()` while the PHY device is runtime-suspended, causing a system crash or silent loss of configuration.

Pre-existing issues:
- [High] Dereferencing `dev->platform_data` before checking if it is NULL leads to a kernel crash.
--

--- Patch [3]: [PATCH v3 03/12] phy: phy-mtk-dp: Allow probing with devicetree match ---
commit 710ab5488fd0b66b2ea62388b977a4308f99a0c6
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

phy: phy-mtk-dp: Allow probing with devicetree match

Make it possible to decouple the registration of the DisplayPort
PHY driver from the DisplayPort IP driver by adding a devicetree
match to probe the PHY. In order to retain compatibility with older
devicetrees, the legacy code was moved in a mtk_dp_phy_legacy_probe()
function.

> diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
> index bf7b3a95e72d2..d241004e28286 100644
> --- a/drivers/phy/mediatek/phy-mtk-dp.c
> +++ b/drivers/phy/mediatek/phy-mtk-dp.c
[ ... ]
> +static int mtk_dp_phy_legacy_probe(struct platform_device *pdev, struct mtk_dp_phy *dp_phy)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct phy *phy;
> +	int ret;
> +
> +	dp_phy->regmap = *(struct regmap **)dev->platform_data;
> +	if (!dp_phy->regmap)
> +		return dev_err_probe(dev, -EINVAL, "No platform data available\n");

[Severity: High]
This is a pre-existing issue, but does casting and dereferencing
dev->platform_data before checking if it is NULL lead to a kernel crash?

If the driver is bound to a device without platform data via sysfs, this
unvalidated pointer will be dereferenced before the check for
!dp_phy->regmap can execute.

[ ... ]
>  static int mtk_dp_phy_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
[ ... ]
>  	dp_phy = devm_kzalloc(dev, sizeof(*dp_phy), GFP_KERNEL);
>  	if (!dp_phy)
>  		return -ENOMEM;
>  
> -	dp_phy->regmap = regs;
> +	dp_phy->dev = dev;
> +
> +	/* If there's no devicetree, go for legacy pdev probe */
> +	if (!dev->of_node)
> +		return mtk_dp_phy_legacy_probe(pdev, dp_phy);
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	dp_phy->regmap = devm_regmap_init_mmio(dev, base, &mtk_dp_phy_regmap_cfg);

[Severity: High]
Since the driver hardcodes PHY_OFFSET (0x1000) into its register macros like
MTK_DP_PHY_DIG_PLL_CTL_1, will this independent regmap cause out-of-bounds
accesses if the devicetree maps the PHY node to its true physical base?

If the devicetree instead sets the PHY node's reg to the parent DP IP base to
counteract this offset, won't this cause devm_platform_ioremap_resource() to
fail with -EBUSY due to resource conflicts between the PHY driver and the DP
IP driver?

> +	if (IS_ERR(dp_phy->regmap))
> +		return PTR_ERR(dp_phy->regmap);
> +
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return ret;

[Severity: High]
By enabling runtime PM here, could mtk_dp_phy_configure() access hardware
registers while the PHY device is runtime-suspended?

The consumer driver calls phy_configure() without calling phy_power_on().
The PHY framework takes a runtime PM reference during .init and .reset,
but explicitly drops it right before returning, and does not take a reference
during .configure.

When the consumer calls phy_configure(), it writes directly to hardware via
regmap_write() and regmap_update_bits() while the device PM usage count
is zero. Can this crash the system or cause silent loss of configuration?

> +
>  	phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707154245.198361-1-angelogioacchino.delregno@collabora.com?part=3

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

Re: [PATCH v3 03/12] phy: phy-mtk-dp: Allow probing with devicetree match

[AngeloGioacchino Del Regno]

On 7/7/26 18:06, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] Hardcoded `PHY_OFFSET` causes out-of-bounds register accesses or resource conflicts when the PHY is probed as a standalone devicetree node.
> - [High] Hardware registers are accessed in `mtk_dp_phy_configure()` while the PHY device is runtime-suspended, causing a system crash or silent loss of configuration.
> 
> Pre-existing issues:
> - [High] Dereferencing `dev->platform_data` before checking if it is NULL leads to a kernel crash.
> --
> 
> --- Patch [3]: [PATCH v3 03/12] phy: phy-mtk-dp: Allow probing with devicetree match ---
> commit 710ab5488fd0b66b2ea62388b977a4308f99a0c6
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> phy: phy-mtk-dp: Allow probing with devicetree match
> 
> Make it possible to decouple the registration of the DisplayPort
> PHY driver from the DisplayPort IP driver by adding a devicetree
> match to probe the PHY. In order to retain compatibility with older
> devicetrees, the legacy code was moved in a mtk_dp_phy_legacy_probe()
> function.
> 
>> diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
>> index bf7b3a95e72d2..d241004e28286 100644
>> --- a/drivers/phy/mediatek/phy-mtk-dp.c
>> +++ b/drivers/phy/mediatek/phy-mtk-dp.c
> [ ... ]
>> +static int mtk_dp_phy_legacy_probe(struct platform_device *pdev, struct mtk_dp_phy *dp_phy)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct phy *phy;
>> +	int ret;
>> +
>> +	dp_phy->regmap = *(struct regmap **)dev->platform_data;
>> +	if (!dp_phy->regmap)
>> +		return dev_err_probe(dev, -EINVAL, "No platform data available\n");
> 
> [Severity: High]
> This is a pre-existing issue, but does casting and dereferencing
> dev->platform_data before checking if it is NULL lead to a kernel crash?
> 
> If the driver is bound to a device without platform data via sysfs, this
> unvalidated pointer will be dereferenced before the check for
> !dp_phy->regmap can execute.
> 
> [ ... ]
>>   static int mtk_dp_phy_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
> [ ... ]
>>   	dp_phy = devm_kzalloc(dev, sizeof(*dp_phy), GFP_KERNEL);
>>   	if (!dp_phy)
>>   		return -ENOMEM;
>>   
>> -	dp_phy->regmap = regs;
>> +	dp_phy->dev = dev;
>> +
>> +	/* If there's no devicetree, go for legacy pdev probe */
>> +	if (!dev->of_node)
>> +		return mtk_dp_phy_legacy_probe(pdev, dp_phy);
>> +
>> +	base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(base))
>> +		return PTR_ERR(base);
>> +
>> +	dp_phy->regmap = devm_regmap_init_mmio(dev, base, &mtk_dp_phy_regmap_cfg);
> 
> [Severity: High]
> Since the driver hardcodes PHY_OFFSET (0x1000) into its register macros like
> MTK_DP_PHY_DIG_PLL_CTL_1, will this independent regmap cause out-of-bounds
> accesses if the devicetree maps the PHY node to its true physical base?
> 
> If the devicetree instead sets the PHY node's reg to the parent DP IP base to
> counteract this offset, won't this cause devm_platform_ioremap_resource() to
> fail with -EBUSY due to resource conflicts between the PHY driver and the DP
> IP driver?
> 

All of that is exactly why there's a "legacy probe" and a "devicetree probe"
mechanism. It's either one or the other, and the legacy one is there only for
retro compatibility purposes.

If the devicetree sets the PHY node's reg to something unrelated this is not
going to work; it's obvious, and should not even be evaluated as an option.

Magic 8 ball case again?

>> +	if (IS_ERR(dp_phy->regmap))
>> +		return PTR_ERR(dp_phy->regmap);
>> +
>> +	ret = devm_pm_runtime_enable(dev);
>> +	if (ret)
>> +		return ret;
> 
> [Severity: High]
> By enabling runtime PM here, could mtk_dp_phy_configure() access hardware
> registers while the PHY device is runtime-suspended?
> 
> The consumer driver calls phy_configure() without calling phy_power_on().
> The PHY framework takes a runtime PM reference during .init and .reset,
> but explicitly drops it right before returning, and does not take a reference
> during .configure.
> 
> When the consumer calls phy_configure(), it writes directly to hardware via
> regmap_write() and regmap_update_bits() while the device PM usage count
> is zero. Can this crash the system or cause silent loss of configuration?
> 

That won't happen because as I explained before, the PHY needs to be powered
on with phy_power_on() which calls runtime_pm_get_sync().

In any case, an unpowered PHY will cause a silent loss of configuration with
no SError nor any other crash.

>> +
>>   	phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
> 


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

[PATCH v3 04/12] phy: phy-mtk-dp: Migrate register offsets to SoC specific pdata

[AngeloGioacchino Del Regno]

In preparation for adding support for newer SoCs and for adding
more capabilities to this driver in an efficient manner, migrate
all of the hardcoded register offsets to SoC specific pdata and
assign that for both DT and platform probing.

While at it also cleanup writing the driving parameters to the PHY
by iterating through all lanes with a loop instead: note that for
that to happen the default 0/1/2 driving parameters were added,
those are really just zero.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-dp.c | 145 ++++++++++++++++++++++++------
 1 file changed, 116 insertions(+), 29 deletions(-)

diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index d241004e2828..20974650d940 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -4,6 +4,10 @@
  *
  * Copyright (c) 2022, BayLibre Inc.
  * Copyright (c) 2022, MediaTek Inc.
+ *
+ * Major refactoring
+ * Copyright (c) 2026, Collabora Ltd.
+ *                     AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
  */
 
 #include <linux/delay.h>
@@ -15,24 +19,29 @@
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
-#define PHY_OFFSET			0x1000
+#define MTK_DP_PHY_MAX_LANES		4
 
-#define MTK_DP_PHY_DIG_PLL_CTL_1	(PHY_OFFSET + 0x14)
+/* DP_PHYD_PLL_CTL_1 */
 #define TPLL_SSC_EN			BIT(3)
 
-#define MTK_DP_PHY_DIG_BIT_RATE		(PHY_OFFSET + 0x3C)
-#define BIT_RATE_RBR			0
-#define BIT_RATE_HBR			1
-#define BIT_RATE_HBR2			2
-#define BIT_RATE_HBR3			3
+/* DP_PHYD_BIT_RATE */
+#define PHYD_DIG_RG_BIT_RATE		GENMASK(1, 0)
+#  define BIT_RATE_RBR			0
+#  define BIT_RATE_HBR			1
+#  define BIT_RATE_HBR2			2
+#  define BIT_RATE_HBR3			3
+
+/* DP_PHYD_SW_RST */
+#define PHYD_DIG_GLB_SW_RST_B		GENMASK(7, 0)
+#  define DP_GLB_SW_RST_PHYD		BIT(0)
+#  define DP_GLB_SW_RST_TFIFO_ANA	BIT(1)
+#  define DP_GLB_SW_RST_XTAL_CLK	BIT(2)
+#  define DP_GLB_SW_RST_MAIN_LINK	BIT(3)
 
-#define MTK_DP_PHY_DIG_SW_RST		(PHY_OFFSET + 0x38)
-#define DP_GLB_SW_RST_PHYD		BIT(0)
+#define DRIVING_PARAM_0_DEFAULT	0x0
+#define DRIVING_PARAM_1_DEFAULT	0x0
+#define DRIVING_PARAM_2_DEFAULT	0x0
 
-#define MTK_DP_LANE0_DRIVING_PARAM_3		(PHY_OFFSET + 0x138)
-#define MTK_DP_LANE1_DRIVING_PARAM_3		(PHY_OFFSET + 0x238)
-#define MTK_DP_LANE2_DRIVING_PARAM_3		(PHY_OFFSET + 0x338)
-#define MTK_DP_LANE3_DRIVING_PARAM_3		(PHY_OFFSET + 0x438)
 #define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT	BIT(4)
 #define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT	(BIT(10) | BIT(12))
 #define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT	GENMASK(20, 19)
@@ -79,15 +88,62 @@
 #define DRIVING_PARAM_8_DEFAULT	(XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
 				 XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
 
+enum mtk_dp_phyd_dig_lane_regidx {
+	DP_PHYD_LAN_DRIVING_PARAM_0,
+	DP_PHYD_LAN_MAX
+};
+
+enum mtk_dp_phyd_dig_glb_regidx {
+	DP_PHYD_PLL_CTL_0,
+	DP_PHYD_PLL_CTL_1,
+	DP_PHYD_SW_RST,
+	DP_PHYD_BIT_RATE,
+	DP_PHYD_GLOBAL_MAX
+};
+
+static const u8 mt8195_phy_dig_lane_regs[DP_PHYD_LAN_MAX] = {
+	[DP_PHYD_LAN_DRIVING_PARAM_0] = 0x2c,
+};
+
+static const u8 mt8195_phy_dig_glb_regs[DP_PHYD_GLOBAL_MAX] = {
+	[DP_PHYD_PLL_CTL_0] = 0x10,
+	[DP_PHYD_PLL_CTL_1] = 0x14,
+	[DP_PHYD_SW_RST] = 0x38,
+	[DP_PHYD_BIT_RATE] = 0x3c,
+};
+
+/**
+ * struct mtk_dp_phy_pdata - Platform data and defaults for MediaTek DP/eDP PHY
+ * @off_dig_glb:    Base offset for dptx_phyd_sifslv_dig_glb
+ * @off_dig_lane:   Base offsets for dptx_phyd_sifslv_dig_lan (for each lane)
+ * @regs_dig_glb:   Register (layout) offsets for dig_glb
+ * @regs_dig_lane:  Register (layout) offsets for dig_lan
+ */
+struct mtk_dp_phy_pdata {
+	/* Register offsets */
+	u16 off_dig_glb;
+	u16 off_dig_lane[MTK_DP_PHY_MAX_LANES];
+
+	/* Register maps */
+	const u8 *regs_dig_glb;
+	const u8 *regs_dig_lane;
+};
+
 struct mtk_dp_phy {
 	struct device *dev;
 	struct regmap *regmap;
+	const struct mtk_dp_phy_pdata *pdata;
 };
 
 static int mtk_dp_phy_init(struct phy *phy)
 {
 	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+	const u32 reg = pdata->regs_dig_lane[DP_PHYD_LAN_DRIVING_PARAM_0];
 	static const u32 driving_params[] = {
+		DRIVING_PARAM_0_DEFAULT,
+		DRIVING_PARAM_1_DEFAULT,
+		DRIVING_PARAM_2_DEFAULT,
 		DRIVING_PARAM_3_DEFAULT,
 		DRIVING_PARAM_4_DEFAULT,
 		DRIVING_PARAM_5_DEFAULT,
@@ -95,15 +151,21 @@ static int mtk_dp_phy_init(struct phy *phy)
 		DRIVING_PARAM_7_DEFAULT,
 		DRIVING_PARAM_8_DEFAULT
 	};
-
-	regmap_bulk_write(dp_phy->regmap, MTK_DP_LANE0_DRIVING_PARAM_3,
-			  driving_params, ARRAY_SIZE(driving_params));
-	regmap_bulk_write(dp_phy->regmap, MTK_DP_LANE1_DRIVING_PARAM_3,
-			  driving_params, ARRAY_SIZE(driving_params));
-	regmap_bulk_write(dp_phy->regmap, MTK_DP_LANE2_DRIVING_PARAM_3,
-			  driving_params, ARRAY_SIZE(driving_params));
-	regmap_bulk_write(dp_phy->regmap, MTK_DP_LANE3_DRIVING_PARAM_3,
-			  driving_params, ARRAY_SIZE(driving_params));
+	int i, ret;
+
+	/*
+	 * Assume that all lanes need the same driving parameters: this
+	 * will bulk write from DRIVING_PARAM_0 to DRIVING_PARAM_8 on
+	 * all lanes (a grand total of [9 * num_lanes] 32-bit writes)
+	 */
+	for (i = 0; i < MTK_DP_PHY_MAX_LANES; i++) {
+		ret = regmap_bulk_write(dp_phy->regmap,
+					pdata->off_dig_lane[i] + reg,
+					driving_params,
+					ARRAY_SIZE(driving_params));
+		if (ret)
+			return ret;
+	};
 
 	return 0;
 }
@@ -111,9 +173,12 @@ static int mtk_dp_phy_init(struct phy *phy)
 static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
 {
 	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
 	u32 val;
 
 	if (opts->dp.set_rate) {
+		const u32 reg_bit_rate = pdata->regs_dig_glb[DP_PHYD_BIT_RATE];
+
 		switch (opts->dp.link_rate) {
 		default:
 			dev_err(&phy->dev,
@@ -133,10 +198,11 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
 			val = BIT_RATE_HBR3;
 			break;
 		}
-		regmap_write(dp_phy->regmap, MTK_DP_PHY_DIG_BIT_RATE, val);
+		regmap_write(dp_phy->regmap, pdata->off_dig_glb + reg_bit_rate, val);
 	}
 
-	regmap_update_bits(dp_phy->regmap, MTK_DP_PHY_DIG_PLL_CTL_1,
+	regmap_update_bits(dp_phy->regmap,
+			   pdata->off_dig_glb + pdata->regs_dig_glb[DP_PHYD_PLL_CTL_1],
 			   TPLL_SSC_EN, opts->dp.ssc ? TPLL_SSC_EN : 0);
 
 	return 0;
@@ -145,12 +211,17 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
 static int mtk_dp_phy_reset(struct phy *phy)
 {
 	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+	const u32 reg_rst = pdata->regs_dig_glb[DP_PHYD_SW_RST];
+
+	/* Clearing bits sets reset state */
+	regmap_clear_bits(dp_phy->regmap, pdata->off_dig_glb + reg_rst, DP_GLB_SW_RST_PHYD);
 
-	regmap_update_bits(dp_phy->regmap, MTK_DP_PHY_DIG_SW_RST,
-			   DP_GLB_SW_RST_PHYD, 0);
+	/* PHYD needs 50uS to guarantee reset done */
 	usleep_range(50, 200);
-	regmap_update_bits(dp_phy->regmap, MTK_DP_PHY_DIG_SW_RST,
-			   DP_GLB_SW_RST_PHYD, 1);
+
+	/* Setting bits means go out of reset */
+	regmap_set_bits(dp_phy->regmap, pdata->off_dig_glb + reg_rst, DP_GLB_SW_RST_PHYD);
 
 	return 0;
 }
@@ -170,12 +241,19 @@ static void mtk_dp_phy_legacy_remove_lookup(void *data)
 	phy_remove_lookup(phy, "dp", dev_name(dp_phy->dev));
 }
 
+static const struct mtk_dp_phy_pdata mt8195_dp_phy_data;
+
 static int mtk_dp_phy_legacy_probe(struct platform_device *pdev, struct mtk_dp_phy *dp_phy)
 {
 	struct device *dev = &pdev->dev;
 	struct phy *phy;
 	int ret;
 
+	/*
+	 * If legacy platform driver probe, assume this is MT8195 or compatible
+	 * with a devicetree that was not migrated to the new, proper bindings.
+	 */
+	dp_phy->pdata = &mt8195_dp_phy_data;
 	dp_phy->regmap = *(struct regmap **)dev->platform_data;
 	if (!dp_phy->regmap)
 		return dev_err_probe(dev, -EINVAL, "No platform data available\n");
@@ -235,6 +313,8 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	dp_phy->pdata = device_get_match_data(dev);
+
 	phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
 	if (IS_ERR(phy))
 		return dev_err_probe(dev, PTR_ERR(phy),
@@ -249,8 +329,15 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
+	.off_dig_glb = 0x1000,
+	.off_dig_lane = (const u16[]) { 0x1100, 0x1200, 0x1300, 0x1400 },
+	.regs_dig_glb = mt8195_phy_dig_glb_regs,
+	.regs_dig_lane = mt8195_phy_dig_lane_regs,
+};
+
 static const struct of_device_id mtk_dp_phy_of_match[] = {
-	{ .compatible = "mediatek,mt8195-dp-phy" },
+	{ .compatible = "mediatek,mt8195-dp-phy", .data = &mt8195_dp_phy_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, mtk_dp_phy_of_match);
-- 
2.54.0


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

Re: [PATCH v3 04/12] phy: phy-mtk-dp: Migrate register offsets to SoC specific pdata

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] Register accesses are performed in `mtk_dp_phy_configure()` while the device's PM runtime state is suspended, risking system hangs.
- [Medium] Missing validation of `device_get_match_data()` return value can lead to a NULL pointer dereference.
- [Low] Invalid compound literal initialization for the 'off_dig_lane' array member causes a compilation error.

Pre-existing issues:
- [High] The legacy probe path unconditionally dereferences `dev->platform_data` before validating that it is not NULL.
--

--- Patch [4]: [PATCH v3 04/12] phy: phy-mtk-dp: Migrate register offsets to SoC specific pdata ---
commit c49d221e57bf6a9816b6ce4e691a20564bc66896
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

phy: phy-mtk-dp: Migrate register offsets to SoC specific pdata

In preparation for adding support for newer SoCs and for adding
more capabilities to this driver in an efficient manner, migrate
all of the hardcoded register offsets to SoC specific pdata and
assign that for both DT and platform probing.

> diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
> index d241004e28286..20974650d9409 100644
> --- a/drivers/phy/mediatek/phy-mtk-dp.c
> +++ b/drivers/phy/mediatek/phy-mtk-dp.c
> @@ -111,9 +173,12 @@ static int mtk_dp_phy_init(struct phy *phy)
>  static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
>  {
>  	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
> +	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
>  	u32 val;
>  
>  	if (opts->dp.set_rate) {
> +		const u32 reg_bit_rate = pdata->regs_dig_glb[DP_PHYD_BIT_RATE];
> +

[ ... ]

> -		regmap_write(dp_phy->regmap, MTK_DP_PHY_DIG_BIT_RATE, val);
> +		regmap_write(dp_phy->regmap, pdata->off_dig_glb + reg_bit_rate, val);
>  	}
>  
> -	regmap_update_bits(dp_phy->regmap, MTK_DP_PHY_DIG_PLL_CTL_1,
> +	regmap_update_bits(dp_phy->regmap,
> +			   pdata->off_dig_glb + pdata->regs_dig_glb[DP_PHYD_PLL_CTL_1],
>  			   TPLL_SSC_EN, opts->dp.ssc ? TPLL_SSC_EN : 0);

[Severity: High]
Since phy_configure() is not wrapped by phy_pm_runtime_get_sync() in the
PHY core, and the consumer driver may not call phy_power_on() before this
point, the PHY PM runtime usage counter might remain 0 during
mtk_dp_phy_configure().

If the PHY power domain is turned off, will these unclocked regmap_write()
and regmap_update_bits() calls cause the system to hang?

Should there be PM runtime protection around these register accesses?

>  
>  	return 0;
>  }

[ ... ]

> @@ -170,12 +241,19 @@ static void mtk_dp_phy_legacy_remove_lookup(void *data)
>  	phy_remove_lookup(phy, "dp", dev_name(dp_phy->dev));
>  }
>  
> +static const struct mtk_dp_phy_pdata mt8195_dp_phy_data;
> +
>  static int mtk_dp_phy_legacy_probe(struct platform_device *pdev, struct mtk_dp_phy *dp_phy)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct phy *phy;
>  	int ret;
>  
> +	/*
> +	 * If legacy platform driver probe, assume this is MT8195 or compatible
> +	 * with a devicetree that was not migrated to the new, proper bindings.
> +	 */
> +	dp_phy->pdata = &mt8195_dp_phy_data;
>  	dp_phy->regmap = *(struct regmap **)dev->platform_data;

[Severity: High]
This is a pre-existing issue, but dev->platform_data is unconditionally
dereferenced here before it is checked for NULL.

If this legacy probe path is triggered without platform_data provided (for
example, via the sysfs new_device interface), won't this cause an immediate
NULL pointer dereference?

>  	if (!dp_phy->regmap)
>  		return dev_err_probe(dev, -EINVAL, "No platform data available\n");

[ ... ]

> @@ -235,6 +313,8 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	dp_phy->pdata = device_get_match_data(dev);
> +

[Severity: Medium]
If a user binds the mediatek-dp-phy driver via sysfs to a platform device
that doesn't match the driver's compatible strings, device_get_match_data()
will return NULL.

Does this need a NULL check before storing it in dp_phy->pdata?

Without validation, subsequent PHY operations like mtk_dp_phy_init() will
dereference the NULL pointer and crash.

>  	phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
>  	if (IS_ERR(phy))
>  		return dev_err_probe(dev, PTR_ERR(phy),

[ ... ]

> @@ -249,8 +329,15 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
> +	.off_dig_glb = 0x1000,
> +	.off_dig_lane = (const u16[]) { 0x1100, 0x1200, 0x1300, 0x1400 },

[Severity: Low]
Is this compound literal initialization valid for an array struct member?

Compiling this with GCC or Clang may throw an invalid initializer error
because C99 requires array members to be initialized directly with
brace-enclosed lists rather than compound literal expressions.

> +	.regs_dig_glb = mt8195_phy_dig_glb_regs,
> +	.regs_dig_lane = mt8195_phy_dig_lane_regs,
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707154245.198361-1-angelogioacchino.delregno@collabora.com?part=4

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

Re: [PATCH v3 04/12] phy: phy-mtk-dp: Migrate register offsets to SoC specific pdata

[AngeloGioacchino Del Regno]

On 7/7/26 17:56, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> 
> New issues:
> - [High] Register accesses are performed in `mtk_dp_phy_configure()` while the device's PM runtime state is suspended, risking system hangs.
> - [Medium] Missing validation of `device_get_match_data()` return value can lead to a NULL pointer dereference.
> - [Low] Invalid compound literal initialization for the 'off_dig_lane' array member causes a compilation error.
> 
> Pre-existing issues:
> - [High] The legacy probe path unconditionally dereferences `dev->platform_data` before validating that it is not NULL.
> --
> 
> --- Patch [4]: [PATCH v3 04/12] phy: phy-mtk-dp: Migrate register offsets to SoC specific pdata ---
> commit c49d221e57bf6a9816b6ce4e691a20564bc66896
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> phy: phy-mtk-dp: Migrate register offsets to SoC specific pdata
> 
> In preparation for adding support for newer SoCs and for adding
> more capabilities to this driver in an efficient manner, migrate
> all of the hardcoded register offsets to SoC specific pdata and
> assign that for both DT and platform probing.
> 
>> diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
>> index d241004e28286..20974650d9409 100644
>> --- a/drivers/phy/mediatek/phy-mtk-dp.c
>> +++ b/drivers/phy/mediatek/phy-mtk-dp.c
>> @@ -111,9 +173,12 @@ static int mtk_dp_phy_init(struct phy *phy)
>>   static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
>>   {
>>   	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
>> +	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
>>   	u32 val;
>>   
>>   	if (opts->dp.set_rate) {
>> +		const u32 reg_bit_rate = pdata->regs_dig_glb[DP_PHYD_BIT_RATE];
>> +
> 
> [ ... ]
> 
>> -		regmap_write(dp_phy->regmap, MTK_DP_PHY_DIG_BIT_RATE, val);
>> +		regmap_write(dp_phy->regmap, pdata->off_dig_glb + reg_bit_rate, val);
>>   	}
>>   
>> -	regmap_update_bits(dp_phy->regmap, MTK_DP_PHY_DIG_PLL_CTL_1,
>> +	regmap_update_bits(dp_phy->regmap,
>> +			   pdata->off_dig_glb + pdata->regs_dig_glb[DP_PHYD_PLL_CTL_1],
>>   			   TPLL_SSC_EN, opts->dp.ssc ? TPLL_SSC_EN : 0);
> 
> [Severity: High]
> Since phy_configure() is not wrapped by phy_pm_runtime_get_sync() in the
> PHY core, and the consumer driver may not call phy_power_on() before this
> point, the PHY PM runtime usage counter might remain 0 during
> mtk_dp_phy_configure().
> 
> If the PHY power domain is turned off, will these unclocked regmap_write()
> and regmap_update_bits() calls cause the system to hang?
> 
> Should there be PM runtime protection around these register accesses?
> 

This PHY needs to be powered on with its full power_on sequence before being able
to be accessed, and anyway the user is taking care of this.
phy_power_on() will take care of calling phy_pm_runtime_get_sync() so there's
absolutely no problem with that.

>>   
>>   	return 0;
>>   }
> 
> [ ... ]
> 
>> @@ -170,12 +241,19 @@ static void mtk_dp_phy_legacy_remove_lookup(void *data)
>>   	phy_remove_lookup(phy, "dp", dev_name(dp_phy->dev));
>>   }
>>   
>> +static const struct mtk_dp_phy_pdata mt8195_dp_phy_data;
>> +
>>   static int mtk_dp_phy_legacy_probe(struct platform_device *pdev, struct mtk_dp_phy *dp_phy)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	struct phy *phy;
>>   	int ret;
>>   
>> +	/*
>> +	 * If legacy platform driver probe, assume this is MT8195 or compatible
>> +	 * with a devicetree that was not migrated to the new, proper bindings.
>> +	 */
>> +	dp_phy->pdata = &mt8195_dp_phy_data;
>>   	dp_phy->regmap = *(struct regmap **)dev->platform_data;
> 
> [Severity: High]
> This is a pre-existing issue, but dev->platform_data is unconditionally
> dereferenced here before it is checked for NULL.
> 
> If this legacy probe path is triggered without platform_data provided (for
> example, via the sysfs new_device interface), won't this cause an immediate
> NULL pointer dereference?
> 

The only other alternative is to remove the legacy probe and break compatibility
with older devicetrees, so, no, that's that.

>>   	if (!dp_phy->regmap)
>>   		return dev_err_probe(dev, -EINVAL, "No platform data available\n");
> 
> [ ... ]
> 
>> @@ -235,6 +313,8 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		return ret;
>>   
>> +	dp_phy->pdata = device_get_match_data(dev);
>> +
> 
> [Severity: Medium]
> If a user binds the mediatek-dp-phy driver via sysfs to a platform device
> that doesn't match the driver's compatible strings, device_get_match_data()
> will return NULL.
> 
> Does this need a NULL check before storing it in dp_phy->pdata?
> 
> Without validation, subsequent PHY operations like mtk_dp_phy_init() will
> dereference the NULL pointer and crash.

Does this really happen, anyway?!?!?!?!? :-)

> 
>>   	phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
>>   	if (IS_ERR(phy))
>>   		return dev_err_probe(dev, PTR_ERR(phy),
> 
> [ ... ]
> 
>> @@ -249,8 +329,15 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
>>   	return 0;
>>   }
>>   
>> +static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
>> +	.off_dig_glb = 0x1000,
>> +	.off_dig_lane = (const u16[]) { 0x1100, 0x1200, 0x1300, 0x1400 },
> 
> [Severity: Low]
> Is this compound literal initialization valid for an array struct member?
> 

It's valid. I see no warnings nor errors during build.

> Compiling this with GCC or Clang may throw an invalid initializer error
> because C99 requires array members to be initialized directly with
> brace-enclosed lists rather than compound literal expressions.
> 
>> +	.regs_dig_glb = mt8195_phy_dig_glb_regs,
>> +	.regs_dig_lane = mt8195_phy_dig_lane_regs,
>> +};
> 


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

[PATCH v3 05/12] phy: phy-mtk-dp: Implement power_on and power_off PHY callbacks

[AngeloGioacchino Del Regno]

Add .power_on() and .power_off() callbacks to mtk_dp_phy_dev_ops
to be able to call those with phy_power_on() and phy_power_off()
API in the DisplayPort driver to be able to stop using all of
those hardcoded register writes in that external driver.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-dp.c | 88 +++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index 20974650d940..f6b0e83d4833 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -21,6 +21,9 @@
 
 #define MTK_DP_PHY_MAX_LANES		4
 
+/* DP_PHYA_GLB_FORCE_CTRL_1 */
+#define CKM_CKTX0_EN_FORCE_MODE		BIT(10)
+
 /* DP_PHYD_PLL_CTL_1 */
 #define TPLL_SSC_EN			BIT(3)
 
@@ -38,6 +41,11 @@
 #  define DP_GLB_SW_RST_XTAL_CLK	BIT(2)
 #  define DP_GLB_SW_RST_MAIN_LINK	BIT(3)
 
+/* DP_PHYD_AUX_RX_CTL */
+#define PHYD_DIG_DPAUX_RX_EN		BIT(0)
+#define PHYD_DIG_XTP_GLB_CKDET_EN	BIT(1)
+#define PHYD_DIG_DPAUX_RX_DEGLITCH_EN	BIT(2)
+
 #define DRIVING_PARAM_0_DEFAULT	0x0
 #define DRIVING_PARAM_1_DEFAULT	0x0
 #define DRIVING_PARAM_2_DEFAULT	0x0
@@ -88,6 +96,12 @@
 #define DRIVING_PARAM_8_DEFAULT	(XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
 				 XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
 
+enum mtk_dp_phya_ana_glb_regidx {
+	DP_PHYA_GLB_FORCE_CTRL_0,
+	DP_PHYA_GLB_FORCE_CTRL_1,
+	DP_PHYA_GLOBAL_MAX
+};
+
 enum mtk_dp_phyd_dig_lane_regidx {
 	DP_PHYD_LAN_DRIVING_PARAM_0,
 	DP_PHYD_LAN_MAX
@@ -98,9 +112,15 @@ enum mtk_dp_phyd_dig_glb_regidx {
 	DP_PHYD_PLL_CTL_1,
 	DP_PHYD_SW_RST,
 	DP_PHYD_BIT_RATE,
+	DP_PHYD_AUX_RX_CTL,
 	DP_PHYD_GLOBAL_MAX
 };
 
+static const u8 mt8195_phy_ana_glb_regs[DP_PHYA_GLOBAL_MAX] = {
+	[DP_PHYA_GLB_FORCE_CTRL_0] = 0x30,
+	[DP_PHYA_GLB_FORCE_CTRL_1] = 0x34,
+};
+
 static const u8 mt8195_phy_dig_lane_regs[DP_PHYD_LAN_MAX] = {
 	[DP_PHYD_LAN_DRIVING_PARAM_0] = 0x2c,
 };
@@ -110,21 +130,26 @@ static const u8 mt8195_phy_dig_glb_regs[DP_PHYD_GLOBAL_MAX] = {
 	[DP_PHYD_PLL_CTL_1] = 0x14,
 	[DP_PHYD_SW_RST] = 0x38,
 	[DP_PHYD_BIT_RATE] = 0x3c,
+	[DP_PHYD_AUX_RX_CTL] = 0x40,
 };
 
 /**
  * struct mtk_dp_phy_pdata - Platform data and defaults for MediaTek DP/eDP PHY
+ * @off_ana_glb:    Base offset for dptx_phyd_sifslv_ana_glb
  * @off_dig_glb:    Base offset for dptx_phyd_sifslv_dig_glb
  * @off_dig_lane:   Base offsets for dptx_phyd_sifslv_dig_lan (for each lane)
+ * @regs_ana_glb:   Register (layout) offsets for ana_glb
  * @regs_dig_glb:   Register (layout) offsets for dig_glb
  * @regs_dig_lane:  Register (layout) offsets for dig_lan
  */
 struct mtk_dp_phy_pdata {
 	/* Register offsets */
+	u16 off_ana_glb;
 	u16 off_dig_glb;
 	u16 off_dig_lane[MTK_DP_PHY_MAX_LANES];
 
 	/* Register maps */
+	const u8 *regs_ana_glb;
 	const u8 *regs_dig_glb;
 	const u8 *regs_dig_lane;
 };
@@ -208,6 +233,65 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
 	return 0;
 }
 
+static int mtk_dp_phy_power_on(struct phy *phy)
+{
+	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+	const u8 *regs_dig = pdata->regs_dig_glb;
+	const u8 *regs_ana = pdata->regs_ana_glb;
+	int ret;
+
+	/* Enable AUX Channel with RX De-Glitch and input clock detection */
+	ret = regmap_write(dp_phy->regmap,
+			   pdata->off_dig_glb + regs_dig[DP_PHYD_AUX_RX_CTL],
+			   PHYD_DIG_DPAUX_RX_EN |
+			   PHYD_DIG_XTP_GLB_CKDET_EN |
+			   PHYD_DIG_DPAUX_RX_DEGLITCH_EN);
+	if (ret)
+		return ret;
+
+	ret = regmap_clear_bits(dp_phy->regmap,
+				pdata->off_ana_glb + regs_ana[DP_PHYA_GLB_FORCE_CTRL_1],
+				CKM_CKTX0_EN_FORCE_MODE);
+	if (ret) {
+		/* Shut down AUX channel */
+		regmap_write(dp_phy->regmap,
+			     pdata->off_dig_glb + regs_dig[DP_PHYD_AUX_RX_CTL], 0);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int mtk_dp_phy_power_off(struct phy *phy)
+{
+	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+	const u8 *regs_dig = pdata->regs_dig_glb;
+	const u8 *regs_ana = pdata->regs_ana_glb;
+	int ret_cktx, ret_aux;
+
+	ret_cktx = regmap_set_bits(dp_phy->regmap,
+				   pdata->off_ana_glb + regs_ana[DP_PHYA_GLB_FORCE_CTRL_1],
+				   CKM_CKTX0_EN_FORCE_MODE);
+
+	/* Disable RX unconditionally */
+	ret_aux = regmap_write(dp_phy->regmap,
+			       pdata->off_dig_glb + regs_dig[DP_PHYD_AUX_RX_CTL], 0);
+	if (ret_aux) {
+		dev_err(&phy->dev, "Could not disable AUX RX: %d\n", ret_aux);
+		return ret_aux;
+	}
+
+	/* Still fail if CKTX0_EN was not disabled, but at least AUX is off */
+	if (ret_cktx) {
+		dev_err(&phy->dev, "Could not disable CKTX0: %d\n", ret_cktx);
+		return ret_cktx;
+	}
+
+	return 0;
+}
+
 static int mtk_dp_phy_reset(struct phy *phy)
 {
 	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
@@ -228,6 +312,8 @@ static int mtk_dp_phy_reset(struct phy *phy)
 
 static const struct phy_ops mtk_dp_phy_dev_ops = {
 	.init = mtk_dp_phy_init,
+	.power_on = mtk_dp_phy_power_on,
+	.power_off = mtk_dp_phy_power_off,
 	.configure = mtk_dp_phy_configure,
 	.reset = mtk_dp_phy_reset,
 	.owner = THIS_MODULE,
@@ -330,8 +416,10 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
 }
 
 static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
+	.off_ana_glb = 0x0,
 	.off_dig_glb = 0x1000,
 	.off_dig_lane = (const u16[]) { 0x1100, 0x1200, 0x1300, 0x1400 },
+	.regs_ana_glb = mt8195_phy_ana_glb_regs,
 	.regs_dig_glb = mt8195_phy_dig_glb_regs,
 	.regs_dig_lane = mt8195_phy_dig_lane_regs,
 };
-- 
2.54.0


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

[PATCH v3 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup

[AngeloGioacchino Del Regno]

Add support for enabling a specifically requested number of lanes
in the .configure() callback and disable all lanes in power off
and reset callbacks for proper hardware cleanup.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-dp.c | 75 ++++++++++++++++++++++++++++---
 1 file changed, 68 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index f6b0e83d4833..2c8bc0b37486 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -10,6 +10,7 @@
  *                     AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/mfd/syscon.h>
@@ -50,6 +51,9 @@
 #define DRIVING_PARAM_1_DEFAULT	0x0
 #define DRIVING_PARAM_2_DEFAULT	0x0
 
+/* DP_PHYD_TX_CTL_0 */
+#define PHYD_TX_LN_EN			GENMASK(7, 4)
+
 #define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT	BIT(4)
 #define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT	(BIT(10) | BIT(12))
 #define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT	GENMASK(20, 19)
@@ -113,6 +117,7 @@ enum mtk_dp_phyd_dig_glb_regidx {
 	DP_PHYD_SW_RST,
 	DP_PHYD_BIT_RATE,
 	DP_PHYD_AUX_RX_CTL,
+	DP_PHYD_TX_CTL_0,
 	DP_PHYD_GLOBAL_MAX
 };
 
@@ -131,6 +136,7 @@ static const u8 mt8195_phy_dig_glb_regs[DP_PHYD_GLOBAL_MAX] = {
 	[DP_PHYD_SW_RST] = 0x38,
 	[DP_PHYD_BIT_RATE] = 0x3c,
 	[DP_PHYD_AUX_RX_CTL] = 0x40,
+	[DP_PHYD_TX_CTL_0] = 0x44,
 };
 
 /**
@@ -200,6 +206,7 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
 	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
 	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
 	u32 val;
+	int i;
 
 	if (opts->dp.set_rate) {
 		const u32 reg_bit_rate = pdata->regs_dig_glb[DP_PHYD_BIT_RATE];
@@ -226,6 +233,17 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
 		regmap_write(dp_phy->regmap, pdata->off_dig_glb + reg_bit_rate, val);
 	}
 
+	if (opts->dp.set_lanes) {
+		const u32 reg_dig_tx_ctl = pdata->regs_dig_glb[DP_PHYD_TX_CTL_0];
+
+		val = 0;
+		for (i = 0; i < opts->dp.lanes; i++)
+			val |= FIELD_PREP(PHYD_TX_LN_EN, BIT(i));
+
+		regmap_update_bits(dp_phy->regmap, pdata->off_dig_glb + reg_dig_tx_ctl,
+				   PHYD_TX_LN_EN, val);
+	}
+
 	regmap_update_bits(dp_phy->regmap,
 			   pdata->off_dig_glb + pdata->regs_dig_glb[DP_PHYD_PLL_CTL_1],
 			   TPLL_SSC_EN, opts->dp.ssc ? TPLL_SSC_EN : 0);
@@ -263,31 +281,68 @@ static int mtk_dp_phy_power_on(struct phy *phy)
 	return 0;
 }
 
+static int mtk_dp_phy_disable_all_lanes(struct mtk_dp_phy *dp_phy)
+{
+	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+	const u8 *regs = pdata->regs_dig_glb;
+	int ret;
+	u32 val;
+
+	ret = regmap_read(dp_phy->regmap, pdata->off_dig_glb + regs[DP_PHYD_TX_CTL_0], &val);
+	if (ret)
+		return ret;
+
+	/* Get mask of currently enabled lane */
+	val = FIELD_GET(PHYD_TX_LN_EN, val);
+	if (val == 0)
+		return 0;
+
+	/* Disable all lanes (needs to be done one by one, from last to first) */
+	do {
+		u32 lane_num = fls(val) - 1;
+		val &= ~BIT(lane_num);
+
+		ret = regmap_clear_bits(dp_phy->regmap,
+					pdata->off_dig_glb + regs[DP_PHYD_TX_CTL_0],
+					FIELD_PREP(PHYD_TX_LN_EN, BIT(lane_num)));
+		if (ret)
+			return ret;
+	} while (val);
+
+	return 0;
+}
+
 static int mtk_dp_phy_power_off(struct phy *phy)
 {
 	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
 	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
 	const u8 *regs_dig = pdata->regs_dig_glb;
 	const u8 *regs_ana = pdata->regs_ana_glb;
-	int ret_cktx, ret_aux;
+	int ret_cktx, ret_aux, ret;
 
 	ret_cktx = regmap_set_bits(dp_phy->regmap,
 				   pdata->off_ana_glb + regs_ana[DP_PHYA_GLB_FORCE_CTRL_1],
 				   CKM_CKTX0_EN_FORCE_MODE);
+	if (ret_cktx)
+		dev_err(&phy->dev, "Could not disable CKTX0: %d\n", ret_cktx);
 
 	/* Disable RX unconditionally */
 	ret_aux = regmap_write(dp_phy->regmap,
 			       pdata->off_dig_glb + regs_dig[DP_PHYD_AUX_RX_CTL], 0);
-	if (ret_aux) {
+	if (ret_aux)
 		dev_err(&phy->dev, "Could not disable AUX RX: %d\n", ret_aux);
-		return ret_aux;
+
+	ret = mtk_dp_phy_disable_all_lanes(dp_phy);
+	if (ret) {
+		dev_err(dp_phy->dev, "Could not disable lanes for poweroff!\n");
+		return ret;
 	}
 
-	/* Still fail if CKTX0_EN was not disabled, but at least AUX is off */
-	if (ret_cktx) {
-		dev_err(&phy->dev, "Could not disable CKTX0: %d\n", ret_cktx);
+	/* Still return a failure if any of CKTX or AUX could not disable */
+	if (ret_cktx)
 		return ret_cktx;
-	}
+	if (ret_aux)
+		return ret_aux;
 
 	return 0;
 }
@@ -297,6 +352,7 @@ static int mtk_dp_phy_reset(struct phy *phy)
 	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
 	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
 	const u32 reg_rst = pdata->regs_dig_glb[DP_PHYD_SW_RST];
+	int ret;
 
 	/* Clearing bits sets reset state */
 	regmap_clear_bits(dp_phy->regmap, pdata->off_dig_glb + reg_rst, DP_GLB_SW_RST_PHYD);
@@ -307,6 +363,11 @@ static int mtk_dp_phy_reset(struct phy *phy)
 	/* Setting bits means go out of reset */
 	regmap_set_bits(dp_phy->regmap, pdata->off_dig_glb + reg_rst, DP_GLB_SW_RST_PHYD);
 
+	/* Disable all lanes and continue reset even if this fails, but notify */
+	ret = mtk_dp_phy_disable_all_lanes(dp_phy);
+	if (ret)
+		dev_err(dp_phy->dev, "Could not disable lanes during reset!\n");
+
 	return 0;
 }
 
-- 
2.54.0


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

Re: [PATCH v3 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Inconsistent use of device pointers for error logging in `mtk_dp_phy_power_off` and `mtk_dp_phy_reset`.
--

--- Patch [6]: [PATCH v3 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup ---
commit cab309557ac4fcb462c5b8978a8cd660acab1ec1
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup

This commit adds support for enabling a specifically requested number of
lanes in the .configure() callback. It also updates the power off and reset
callbacks to disable all lanes for proper hardware cleanup.

> --- a/drivers/phy/mediatek/phy-mtk-dp.c
> +++ b/drivers/phy/mediatek/phy-mtk-dp.c
[ ... ]
> @@ -263,31 +281,68 @@ static int mtk_dp_phy_power_on(struct phy *phy)
[ ... ]
>  static int mtk_dp_phy_power_off(struct phy *phy)
>  {
>  	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
>  	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
>  	const u8 *regs_dig = pdata->regs_dig_glb;
>  	const u8 *regs_ana = pdata->regs_ana_glb;
> -	int ret_cktx, ret_aux;
> +	int ret_cktx, ret_aux, ret;
>  
>  	ret_cktx = regmap_set_bits(dp_phy->regmap,
>  				   pdata->off_ana_glb + regs_ana[DP_PHYA_GLB_FORCE_CTRL_1],
>  				   CKM_CKTX0_EN_FORCE_MODE);
> +	if (ret_cktx)
> +		dev_err(&phy->dev, "Could not disable CKTX0: %d\n", ret_cktx);
[ ... ]
> +	ret = mtk_dp_phy_disable_all_lanes(dp_phy);
> +	if (ret) {
> +		dev_err(dp_phy->dev, "Could not disable lanes for poweroff!\n");

[Severity: Low]
Should this use &phy->dev instead of dp_phy->dev for error logging?

The adjacent error handlers for CKTX and AUX in this same function log 
errors using &phy->dev. Mixing device pointers might lead to inconsistent 
device names in the kernel log.

> +		return ret;
>  	}
[ ... ]
> @@ -307,6 +363,11 @@ static int mtk_dp_phy_reset(struct phy *phy)
[ ... ]
>  	/* Setting bits means go out of reset */
>  	regmap_set_bits(dp_phy->regmap, pdata->off_dig_glb + reg_rst, DP_GLB_SW_RST_PHYD);
>  
> +	/* Disable all lanes and continue reset even if this fails, but notify */
> +	ret = mtk_dp_phy_disable_all_lanes(dp_phy);
> +	if (ret)
> +		dev_err(dp_phy->dev, "Could not disable lanes during reset!\n");

[Severity: Low]
Similarly here, would it be better to use &phy->dev for consistency with 
the rest of the API implementation?

> +
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707154245.198361-1-angelogioacchino.delregno@collabora.com?part=6

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

[PATCH v3 07/12] phy: phy-mtk-dp: Support setting volt swing and preemphasis values

[AngeloGioacchino Del Regno]

Add support for honoring the set_voltages request to set the
voltage swing and preemphasis values in the .configure() callback.

For proper hardware cleanup, reset both in the .reset() callback.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-dp.c | 44 ++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index 2c8bc0b37486..c87e9fa710a4 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -54,6 +54,11 @@
 /* DP_PHYD_TX_CTL_0 */
 #define PHYD_TX_LN_EN			GENMASK(7, 4)
 
+/* DP_PHYD_DRIVING_FORCE */
+#define PHYD_DP_TX_FORCE_VOLT_SWING_EN	BIT(0)
+#define PHYD_DP_TX_FORCE_VOLT_SWING_VAL	GENMASK(2, 1)
+#define PHYD_DP_TX_FORCE_PRE_EMPH_VAL	GENMASK(4, 3)
+
 #define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT	BIT(4)
 #define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT	(BIT(10) | BIT(12))
 #define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT	GENMASK(20, 19)
@@ -107,6 +112,7 @@ enum mtk_dp_phya_ana_glb_regidx {
 };
 
 enum mtk_dp_phyd_dig_lane_regidx {
+	DP_PHYD_LAN_DRIVING_FORCE,
 	DP_PHYD_LAN_DRIVING_PARAM_0,
 	DP_PHYD_LAN_MAX
 };
@@ -127,6 +133,7 @@ static const u8 mt8195_phy_ana_glb_regs[DP_PHYA_GLOBAL_MAX] = {
 };
 
 static const u8 mt8195_phy_dig_lane_regs[DP_PHYD_LAN_MAX] = {
+	[DP_PHYD_LAN_DRIVING_FORCE] = 0x18,
 	[DP_PHYD_LAN_DRIVING_PARAM_0] = 0x2c,
 };
 
@@ -244,6 +251,30 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
 				   PHYD_TX_LN_EN, val);
 	}
 
+	if (opts->dp.set_voltages) {
+		const u32 reg_drv_force = pdata->regs_dig_lane[DP_PHYD_LAN_DRIVING_FORCE];
+
+		if (opts->dp.lanes > 4) {
+			dev_err(&phy->dev, "Wrong lanes config %u\n", opts->dp.lanes);
+			return -EINVAL;
+		}
+
+		for (i = 0; i < opts->dp.lanes; i++) {
+			const u32 off_dig_lane = pdata->off_dig_lane[i];
+			u32 val;
+
+			val = FIELD_PREP(PHYD_DP_TX_FORCE_VOLT_SWING_VAL, opts->dp.voltage[i]);
+			val |= FIELD_PREP(PHYD_DP_TX_FORCE_PRE_EMPH_VAL, opts->dp.pre[i]);
+			val |= PHYD_DP_TX_FORCE_VOLT_SWING_EN;
+
+			regmap_update_bits(dp_phy->regmap, off_dig_lane + reg_drv_force,
+					   PHYD_DP_TX_FORCE_VOLT_SWING_EN |
+					   PHYD_DP_TX_FORCE_VOLT_SWING_VAL |
+					   PHYD_DP_TX_FORCE_PRE_EMPH_VAL,
+					   val);
+		}
+	}
+
 	regmap_update_bits(dp_phy->regmap,
 			   pdata->off_dig_glb + pdata->regs_dig_glb[DP_PHYD_PLL_CTL_1],
 			   TPLL_SSC_EN, opts->dp.ssc ? TPLL_SSC_EN : 0);
@@ -352,7 +383,8 @@ static int mtk_dp_phy_reset(struct phy *phy)
 	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
 	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
 	const u32 reg_rst = pdata->regs_dig_glb[DP_PHYD_SW_RST];
-	int ret;
+	const u32 reg_drv_force = pdata->regs_dig_lane[DP_PHYD_LAN_DRIVING_FORCE];
+	int i, ret;
 
 	/* Clearing bits sets reset state */
 	regmap_clear_bits(dp_phy->regmap, pdata->off_dig_glb + reg_rst, DP_GLB_SW_RST_PHYD);
@@ -368,6 +400,16 @@ static int mtk_dp_phy_reset(struct phy *phy)
 	if (ret)
 		dev_err(dp_phy->dev, "Could not disable lanes during reset!\n");
 
+	/* Reset Voltage Swing and Preemphasis values */
+	for (i = 0; i < MTK_DP_PHY_MAX_LANES; i++) {
+		const u32 off_dig_lane = pdata->off_dig_lane[i];
+
+		regmap_clear_bits(dp_phy->regmap, off_dig_lane + reg_drv_force,
+				   PHYD_DP_TX_FORCE_VOLT_SWING_EN |
+				   PHYD_DP_TX_FORCE_VOLT_SWING_VAL |
+				   PHYD_DP_TX_FORCE_PRE_EMPH_VAL);
+	}
+
 	return 0;
 }
 
-- 
2.54.0


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

[PATCH v3 08/12] phy: phy-mtk-dp: Add support for digital and analog calibration

[AngeloGioacchino Del Regno]

Add support for reading the calibration values from eFuse: if
present, write those - otherwise, rely on the defaults from
SoC-specific data.
This also adds support for writing the calibration values for
the analog part of the PHY.

Note that before this change, only default hardcoded calibration
values were supported for the digital driving parameters.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-dp.c | 303 +++++++++++++++++++++++++++---
 1 file changed, 281 insertions(+), 22 deletions(-)

diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index c87e9fa710a4..f59a415b401c 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -14,6 +14,7 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/mfd/syscon.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
@@ -22,6 +23,19 @@
 
 #define MTK_DP_PHY_MAX_LANES		4
 
+/* DP_PHYA_GLB_BIAS_GEN_0 (PHYA - Analog) */
+#define XTP_GLB_BIAS_INT_R_CTRL		GENMASK(20, 16)
+
+/* DP_PHYA_GLB_FORCE_CTRL_1 */
+#define CKM_CKTX0_EN_FORCE_MODE		BIT(10)
+
+/* DP_PHYA_GLB_DPAUX_TX */
+#define CKM_PT0_CKTX_IMPSEL		GENMASK(23, 20)
+
+/* DP_PHYA_LAN_LANE_TX_0 */
+#define XTP_LN_TX_IMPSEL_PMOS		GENMASK(15, 12)
+#define XTP_LN_TX_IMPSEL_NMOS		GENMASK(19, 16)
+
 /* DP_PHYA_GLB_FORCE_CTRL_1 */
 #define CKM_CKTX0_EN_FORCE_MODE		BIT(10)
 
@@ -59,11 +73,29 @@
 #define PHYD_DP_TX_FORCE_VOLT_SWING_VAL	GENMASK(2, 1)
 #define PHYD_DP_TX_FORCE_PRE_EMPH_VAL	GENMASK(4, 3)
 
+/*
+ * DRIVING_PARAM_X (PHYD - Digital)
+ *
+ * Driving param registers are split in three sets, all containing settings
+ * for Voltage Swing and Pre-Emphasis for each lane's differential pair.
+ *
+ * All three sets share the same layout, but for different physical signals;
+ * In particular:
+ * [0-2]: LC TX CM (Minus / Negative Edge)
+ * [3-5]: LC TX C  (Logic State Change Point)
+ * [6-8]: LC TX CP (Plus / Positive Edge)
+ *
+ * And they contain values for:
+ * [0,3,6]: Swing 0 Pre[0-3]
+ * [1,4,7]: Swing 1 Pre[0-2] and Swing 2 Pre0
+ * [2,5,8]: Swing 2 Pre1 and Swing 3 Pre0
+ */
+#define PHYD_DIG_NUM_DRV_PARA_REGS	9
 #define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT	BIT(4)
 #define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT	(BIT(10) | BIT(12))
 #define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT	GENMASK(20, 19)
 #define XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT	GENMASK(29, 29)
-#define DRIVING_PARAM_3_DEFAULT	(XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT | \
+#define MT8195_DRIVING_PARAM_3_DEFAULT	(XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT | \
 				 XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT | \
 				 XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT | \
 				 XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT)
@@ -72,21 +104,21 @@
 #define XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT	GENMASK(12, 9)
 #define XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT	(BIT(18) | BIT(21))
 #define XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT	GENMASK(29, 29)
-#define DRIVING_PARAM_4_DEFAULT	(XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT | \
+#define MT8195_DRIVING_PARAM_4_DEFAULT	(XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT | \
 				 XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT | \
 				 XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT | \
 				 XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT)
 
 #define XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT	(BIT(3) | BIT(5))
 #define XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT	GENMASK(13, 12)
-#define DRIVING_PARAM_5_DEFAULT	(XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT | \
+#define MT8195_DRIVING_PARAM_5_DEFAULT	(XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT | \
 				 XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT)
 
 #define XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT	0
 #define XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT	GENMASK(10, 10)
 #define XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT	GENMASK(19, 19)
 #define XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT	GENMASK(28, 28)
-#define DRIVING_PARAM_6_DEFAULT	(XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT | \
+#define MT8195_DRIVING_PARAM_6_DEFAULT	(XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT | \
 				 XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT | \
 				 XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT | \
 				 XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT)
@@ -95,22 +127,30 @@
 #define XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT	GENMASK(10, 9)
 #define XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT	GENMASK(19, 18)
 #define XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT	0
-#define DRIVING_PARAM_7_DEFAULT	(XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT | \
+#define MT8195_DRIVING_PARAM_7_DEFAULT	(XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT | \
 				 XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT | \
 				 XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT | \
 				 XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT)
 
 #define XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT	GENMASK(3, 3)
 #define XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT	0
-#define DRIVING_PARAM_8_DEFAULT	(XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
+#define MT8195_DRIVING_PARAM_8_DEFAULT	(XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
 				 XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
 
 enum mtk_dp_phya_ana_glb_regidx {
+	DP_PHYA_GLB_BIAS_GEN_0,
+	DP_PHYA_GLB_BIAS_GEN_1,
+	DP_PHYA_GLB_DPAUX_TX,
 	DP_PHYA_GLB_FORCE_CTRL_0,
 	DP_PHYA_GLB_FORCE_CTRL_1,
 	DP_PHYA_GLOBAL_MAX
 };
 
+enum mtk_dp_phya_ana_lane_regidx {
+	DP_PHYA_LAN_LANE_TX_0,
+	DP_PHYA_LAN_MAX
+};
+
 enum mtk_dp_phyd_dig_lane_regidx {
 	DP_PHYD_LAN_DRIVING_FORCE,
 	DP_PHYD_LAN_DRIVING_PARAM_0,
@@ -128,10 +168,17 @@ enum mtk_dp_phyd_dig_glb_regidx {
 };
 
 static const u8 mt8195_phy_ana_glb_regs[DP_PHYA_GLOBAL_MAX] = {
+	[DP_PHYA_GLB_BIAS_GEN_0] = 0x0,
+	[DP_PHYA_GLB_BIAS_GEN_1] = 0x4,
+	[DP_PHYA_GLB_DPAUX_TX] = 0x8,
 	[DP_PHYA_GLB_FORCE_CTRL_0] = 0x30,
 	[DP_PHYA_GLB_FORCE_CTRL_1] = 0x34,
 };
 
+static const u8 mt8195_phy_ana_lane_regs[DP_PHYA_LAN_MAX] = {
+	[DP_PHYA_LAN_LANE_TX_0] = 0x4,
+};
+
 static const u8 mt8195_phy_dig_lane_regs[DP_PHYD_LAN_MAX] = {
 	[DP_PHYD_LAN_DRIVING_FORCE] = 0x18,
 	[DP_PHYD_LAN_DRIVING_PARAM_0] = 0x2c,
@@ -146,49 +193,101 @@ static const u8 mt8195_phy_dig_glb_regs[DP_PHYD_GLOBAL_MAX] = {
 	[DP_PHYD_TX_CTL_0] = 0x44,
 };
 
+/**
+ * struct mtk_dp_phya_imp_sel - Per-Lane Impedance Selection
+ * @pmos: Impedance selection for P-Channel MOSFET
+ * @nmos: Impedance selection for N-Channel MOSFET
+ */
+struct mtk_dp_phya_imp_sel {
+	u8 pmos : 4;
+	u8 nmos : 4;
+};
+
 /**
  * struct mtk_dp_phy_pdata - Platform data and defaults for MediaTek DP/eDP PHY
  * @off_ana_glb:    Base offset for dptx_phyd_sifslv_ana_glb
+ * @off_ana_lane:   Base offsets for dptx_phyd_sifslv_ana_lan (for each lane)
  * @off_dig_glb:    Base offset for dptx_phyd_sifslv_dig_glb
  * @off_dig_lane:   Base offsets for dptx_phyd_sifslv_dig_lan (for each lane)
  * @regs_ana_glb:   Register (layout) offsets for ana_glb
+ * @regs_ana_lane:  Register (layout) offsets for ana_lan
  * @regs_dig_glb:   Register (layout) offsets for dig_glb
  * @regs_dig_lane:  Register (layout) offsets for dig_lan
+ * @ana_bias_r:     Internal resistance "R" Selection Settings (global)
+ * @ana_cktx_imp:   TX Clock Impedance Selection Settings (global)
+ * @ana_lanes_imp:  TX Impedance Selection Settings (for all lanes)
+ * @driving_params: Voltage Swing and Pre-Emphasis settings (for all lanes)
  */
 struct mtk_dp_phy_pdata {
 	/* Register offsets */
 	u16 off_ana_glb;
+	u16 off_ana_lane[MTK_DP_PHY_MAX_LANES];
 	u16 off_dig_glb;
 	u16 off_dig_lane[MTK_DP_PHY_MAX_LANES];
 
 	/* Register maps */
 	const u8 *regs_ana_glb;
+	const u8 *regs_ana_lane;
 	const u8 *regs_dig_glb;
 	const u8 *regs_dig_lane;
+
+	/* Calibration defaults */
+	u8 ana_bias_r;
+	u8 ana_cktx_imp;
+	struct mtk_dp_phya_imp_sel ana_lanes_imp;
+	u32 driving_params[PHYD_DIG_NUM_DRV_PARA_REGS];
 };
 
 struct mtk_dp_phy {
 	struct device *dev;
 	struct regmap *regmap;
 	const struct mtk_dp_phy_pdata *pdata;
+
+	u8 ana_bias_r;
+	u8 ana_cktx_imp;
+	struct mtk_dp_phya_imp_sel ana_impsel[MTK_DP_PHY_MAX_LANES];
 };
 
-static int mtk_dp_phy_init(struct phy *phy)
+static int mtk_dp_phy_set_analog_calibration_params(struct mtk_dp_phy *dp_phy)
+{
+	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+	const u8 *regs_ana_glb = pdata->regs_ana_glb;
+	const u8 *regs_ana_lane = pdata->regs_ana_lane;
+	int i, ret;
+
+	ret = regmap_update_bits(dp_phy->regmap,
+				 pdata->off_ana_glb + regs_ana_glb[DP_PHYA_GLB_BIAS_GEN_0],
+				 XTP_GLB_BIAS_INT_R_CTRL,
+				 FIELD_PREP(XTP_GLB_BIAS_INT_R_CTRL, dp_phy->ana_bias_r));
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(dp_phy->regmap,
+				 pdata->off_ana_glb + regs_ana_glb[DP_PHYA_GLB_DPAUX_TX],
+				 CKM_PT0_CKTX_IMPSEL,
+				 FIELD_PREP(CKM_PT0_CKTX_IMPSEL, dp_phy->ana_cktx_imp));
+	if (ret)
+		return ret;
+
+	for (i = 0; i < MTK_DP_PHY_MAX_LANES; i++) {
+		struct mtk_dp_phya_imp_sel *ana_imp = &dp_phy->ana_impsel[i];
+		u32 val = FIELD_PREP(XTP_LN_TX_IMPSEL_PMOS, ana_imp->pmos) |
+			  FIELD_PREP(XTP_LN_TX_IMPSEL_NMOS, ana_imp->nmos);
+		u32 off_ana_lane = pdata->off_ana_lane[i];
+
+		ret = regmap_update_bits(dp_phy->regmap,
+					 off_ana_lane + regs_ana_lane[DP_PHYA_LAN_LANE_TX_0],
+					 XTP_LN_TX_IMPSEL_PMOS | XTP_LN_TX_IMPSEL_NMOS, val);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
+static int mtk_dp_phy_set_digital_drv_params(struct mtk_dp_phy *dp_phy)
 {
-	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
 	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
 	const u32 reg = pdata->regs_dig_lane[DP_PHYD_LAN_DRIVING_PARAM_0];
-	static const u32 driving_params[] = {
-		DRIVING_PARAM_0_DEFAULT,
-		DRIVING_PARAM_1_DEFAULT,
-		DRIVING_PARAM_2_DEFAULT,
-		DRIVING_PARAM_3_DEFAULT,
-		DRIVING_PARAM_4_DEFAULT,
-		DRIVING_PARAM_5_DEFAULT,
-		DRIVING_PARAM_6_DEFAULT,
-		DRIVING_PARAM_7_DEFAULT,
-		DRIVING_PARAM_8_DEFAULT
-	};
 	int i, ret;
 
 	/*
@@ -199,11 +298,31 @@ static int mtk_dp_phy_init(struct phy *phy)
 	for (i = 0; i < MTK_DP_PHY_MAX_LANES; i++) {
 		ret = regmap_bulk_write(dp_phy->regmap,
 					pdata->off_dig_lane[i] + reg,
-					driving_params,
-					ARRAY_SIZE(driving_params));
+					pdata->driving_params,
+					ARRAY_SIZE(pdata->driving_params));
 		if (ret)
 			return ret;
 	};
+	return 0;
+}
+
+static int mtk_dp_phy_init(struct phy *phy)
+{
+	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+	struct device *dev = &phy->dev;
+	int ret;
+
+	ret = mtk_dp_phy_set_digital_drv_params(dp_phy);
+	if (ret) {
+		dev_err(dev, "Cannot set driving params\n");
+		return ret;
+	}
+
+	ret = mtk_dp_phy_set_analog_calibration_params(dp_phy);
+	if (ret) {
+		dev_err(dev, "Cannot set analog calibration\n");
+		return ret;
+	}
 
 	return 0;
 }
@@ -422,6 +541,111 @@ static const struct phy_ops mtk_dp_phy_dev_ops = {
 	.owner = THIS_MODULE,
 };
 
+static void mtk_dp_phy_get_default_cal_data(struct mtk_dp_phy *dp_phy)
+{
+	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+	int i;
+
+	dp_phy->ana_bias_r = pdata->ana_bias_r;
+	dp_phy->ana_cktx_imp = pdata->ana_cktx_imp;
+
+	/* Copy the default lane impedance settings to all lanes */
+	for (i = 0; i < MTK_DP_PHY_MAX_LANES; i++)
+		memcpy(&dp_phy->ana_impsel[i], &pdata->ana_lanes_imp,
+		       sizeof(dp_phy->ana_impsel[0]));
+
+	return;
+}
+
+static int mtk_dp_phy_get_one_cal_para(struct device *dev, const char *name, u8 max_val)
+{
+	u8 buf_byte;
+	u16 buf;
+	int ret;
+
+	/*
+	 * All of the calibrations are always max 8 bits long, but some may
+	 * be split between two different 8-bits cells: handle this corner
+	 * case by retrying reading as u16.
+	 */
+	ret = nvmem_cell_read_u8(dev, name, &buf_byte);
+	if (ret)
+		ret = nvmem_cell_read_u16(dev, name, &buf);
+	else
+		buf = buf_byte;
+
+	if (ret) {
+		dev_err(dev, "Cannot get calibration data for %s: %d\n", name, ret);
+		return ret;
+	};
+
+	if (buf == 0) {
+		dev_warn(dev, "No calibration for %s. Using defaults\n", name);
+		return -ENOENT;
+	}
+
+	if (buf > max_val) {
+		dev_err(dev, "Bad value %u retrieved for %s. Returning.\n", buf, name);
+		return -ERANGE;
+	};
+
+	return buf;
+}
+
+static int mtk_dp_phy_get_calibration_data(struct mtk_dp_phy *dp_phy)
+{
+	char mtk_dp_cal_lane_imp_name[] = "impedance-laneXM";
+	struct device *dev = dp_phy->dev;
+	int i, ret;
+
+	ret = mtk_dp_phy_get_one_cal_para(dev, "rbias-trim", FIELD_MAX(XTP_GLB_BIAS_INT_R_CTRL));
+	if (ret < 0)
+		goto end;
+	dp_phy->ana_bias_r = ret;
+
+	ret = mtk_dp_phy_get_one_cal_para(dev, "impedance-txclk", FIELD_MAX(CKM_PT0_CKTX_IMPSEL));
+	if (ret < 0)
+		goto end;
+	dp_phy->ana_cktx_imp = ret;
+
+	/* Get impedance params for each lane */
+	for (i = 0; i < MTK_DP_PHY_MAX_LANES; i++) {
+		/* P-MOSFET first */
+		snprintf(mtk_dp_cal_lane_imp_name, ARRAY_SIZE(mtk_dp_cal_lane_imp_name),
+			 "impedance-lane%dp", i);
+		ret = mtk_dp_phy_get_one_cal_para(dev, mtk_dp_cal_lane_imp_name,
+						  FIELD_MAX(XTP_LN_TX_IMPSEL_PMOS));
+		if (ret < 0)
+			goto end;
+		dp_phy->ana_impsel[i].pmos = ret;
+
+		/* ...and then N-MOSFET too */
+		snprintf(mtk_dp_cal_lane_imp_name, ARRAY_SIZE(mtk_dp_cal_lane_imp_name),
+			 "impedance-lane%dn", i);
+		ret = mtk_dp_phy_get_one_cal_para(dev, mtk_dp_cal_lane_imp_name,
+						  FIELD_MAX(XTP_LN_TX_IMPSEL_NMOS));
+		if (ret < 0)
+			goto end;
+		dp_phy->ana_impsel[i].nmos = ret;
+	}
+end:
+	if (ret < 0) {
+		/*
+		 * If any of the calibration values is missing, or if there
+		 * is no calibration at all in the eFuses, copy the default
+		 * one entirely (as partial values shall not be mixed!)
+		 */
+		if (ret == -ENOENT) {
+			dev_info(dev, "Using calibration default values\n");
+			mtk_dp_phy_get_default_cal_data(dp_phy);
+			return 0;
+		}
+		return ret;
+	};
+
+	return 0;
+}
+
 static void mtk_dp_phy_legacy_remove_lookup(void *data)
 {
 	struct phy *phy = data;
@@ -453,6 +677,13 @@ static int mtk_dp_phy_legacy_probe(struct platform_device *pdev, struct mtk_dp_p
 				     "Failed to create DP PHY\n");
 
 	phy_set_drvdata(phy, dp_phy);
+
+	/*
+	 * Set default calibration data before exposing the PHY.
+	 * For legacy probe, mtk_dp will set calibrations from eFuse, if found.
+	 */
+	mtk_dp_phy_get_default_cal_data(dp_phy);
+
 	ret = phy_create_lookup(phy, "dp", dev_name(dev));
 	if (ret)
 		return ret;
@@ -504,6 +735,15 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
 
 	dp_phy->pdata = device_get_match_data(dev);
 
+	if (IS_REACHABLE(CONFIG_NVMEM)) {
+		ret = mtk_dp_phy_get_calibration_data(dp_phy);
+		if (ret)
+			return ret;
+	} else {
+		/* Use default calibration data */
+		mtk_dp_phy_get_default_cal_data(dp_phy);
+	}
+
 	phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
 	if (IS_ERR(phy))
 		return dev_err_probe(dev, PTR_ERR(phy),
@@ -519,12 +759,31 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
 }
 
 static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
-	.off_ana_glb = 0x0,
+	.off_ana_glb = 0,
+	.off_ana_lane = (const u16[]) { 0x100, 0x200, 0x300, 0x400 },
 	.off_dig_glb = 0x1000,
 	.off_dig_lane = (const u16[]) { 0x1100, 0x1200, 0x1300, 0x1400 },
 	.regs_ana_glb = mt8195_phy_ana_glb_regs,
+	.regs_ana_lane = mt8195_phy_ana_lane_regs,
 	.regs_dig_glb = mt8195_phy_dig_glb_regs,
 	.regs_dig_lane = mt8195_phy_dig_lane_regs,
+	.ana_bias_r = 15,
+	.ana_cktx_imp = 8,
+	.ana_lanes_imp = {
+		.pmos = 8,
+		.nmos = 8,
+	},
+	.driving_params = (const u32[]) {
+		[0] = 0,
+		[1] = 0,
+		[2] = 0,
+		[3] = MT8195_DRIVING_PARAM_3_DEFAULT,
+		[4] = MT8195_DRIVING_PARAM_4_DEFAULT,
+		[5] = MT8195_DRIVING_PARAM_5_DEFAULT,
+		[6] = MT8195_DRIVING_PARAM_6_DEFAULT,
+		[7] = MT8195_DRIVING_PARAM_7_DEFAULT,
+		[8] = MT8195_DRIVING_PARAM_8_DEFAULT
+	},
 };
 
 static const struct of_device_id mtk_dp_phy_of_match[] = {
-- 
2.54.0


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

Re: [PATCH v3 08/12] phy: phy-mtk-dp: Add support for digital and analog calibration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Unconditional `dev_err` logging in `mtk_dp_phy_get_one_cal_para()` causes log spam for expected fallback cases (missing eFuses) and probe deferrals.

Pre-existing issues:
- [High] Unconditional dereference of `dev->platform_data` in legacy probe before validating it is non-NULL.
--

--- Patch [8]: [PATCH v3 08/12] phy: phy-mtk-dp: Add support for digital and analog calibration ---
commit 637b7b00f751ea110c4b395e4e951eecc2ab535e
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

phy: phy-mtk-dp: Add support for digital and analog calibration

Add support for reading the calibration values from eFuse: if
present, write those - otherwise, rely on the defaults from
SoC-specific data. This also adds support for writing the calibration
values for the analog part of the PHY.

> diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
> index c87e9fa710a40..f59a415b401c5 100644
> --- a/drivers/phy/mediatek/phy-mtk-dp.c
> +++ b/drivers/phy/mediatek/phy-mtk-dp.c
[ ... ]
> @@ -422,6 +541,111 @@ static const struct phy_ops mtk_dp_phy_dev_ops = {
>  	.owner = THIS_MODULE,
>  };
>  
> +static void mtk_dp_phy_get_default_cal_data(struct mtk_dp_phy *dp_phy)
[ ... ]
> +static int mtk_dp_phy_get_one_cal_para(struct device *dev, const char *name, u8 max_val)
> +{
> +	u8 buf_byte;
> +	u16 buf;
> +	int ret;
> +
> +	/*
> +	 * All of the calibrations are always max 8 bits long, but some may
> +	 * be split between two different 8-bits cells: handle this corner
> +	 * case by retrying reading as u16.
> +	 */
> +	ret = nvmem_cell_read_u8(dev, name, &buf_byte);
> +	if (ret)
> +		ret = nvmem_cell_read_u16(dev, name, &buf);
> +	else
> +		buf = buf_byte;
> +
> +	if (ret) {
> +		dev_err(dev, "Cannot get calibration data for %s: %d\n", name, ret);
> +		return ret;
> +	};

[Severity: Medium]
Will this unconditionally log an error when calibration data is intentionally
omitted or when the nvmem provider returns -EPROBE_DEFER?

If the calibration data is missing, nvmem_cell_read_u8() returns -ENOENT,
which is later handled gracefully by the caller by falling back to defaults.
However, printing dev_err() here will cause log spam and false-positive
errors in dmesg. Could this be silenced for -ENOENT and -EPROBE_DEFER cases?

> +
> +	if (buf == 0) {
[ ... ]
> @@ -453,6 +677,13 @@ static int mtk_dp_phy_legacy_probe(struct platform_device *pdev, struct mtk_dp_p
>  				     "Failed to create DP PHY\n");
>  
>  	phy_set_drvdata(phy, dp_phy);

[Severity: High]
This is a pre-existing issue, but earlier in mtk_dp_phy_legacy_probe(),
does the code unconditionally dereference dev->platform_data before checking
if it is NULL?

drivers/phy/mediatek/phy-mtk-dp.c:mtk_dp_phy_legacy_probe() {
    ...
	dp_phy->regmap = *(struct regmap **)dev->platform_data;
	if (!dp_phy->regmap)
		return dev_err_probe(dev, -EINVAL, "No platform data available\n");
    ...
}

If the platform device is instantiated without platform data,
*(struct regmap **)NULL results in an immediate kernel panic, making the
subsequent check dead code.

> +
> +	/*
> +	 * Set default calibration data before exposing the PHY.
> +	 * For legacy probe, mtk_dp will set calibrations from eFuse, if found.
> +	 */
> +	mtk_dp_phy_get_default_cal_data(dp_phy);
> +
>  	ret = phy_create_lookup(phy, "dp", dev_name(dev));
>  	if (ret)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707154245.198361-1-angelogioacchino.delregno@collabora.com?part=8

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

[PATCH v3 09/12] phy: phy-mtk-dp: Rewrite and document default driving param macros

[AngeloGioacchino Del Regno]

Use FIELD_PREP_CONST and add nicer definitions/macros to build the
default driving parameters for the PHY and, while at it, also add
comments explaining what they are supposed to set in the PHY.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-dp.c | 86 +++++++++++++++----------------
 1 file changed, 41 insertions(+), 45 deletions(-)

diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index f59a415b401c..df193c712c77 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -91,51 +91,47 @@
  * [2,5,8]: Swing 2 Pre1 and Swing 3 Pre0
  */
 #define PHYD_DIG_NUM_DRV_PARA_REGS	9
-#define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT	BIT(4)
-#define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT	(BIT(10) | BIT(12))
-#define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT	GENMASK(20, 19)
-#define XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT	GENMASK(29, 29)
-#define MT8195_DRIVING_PARAM_3_DEFAULT	(XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT | \
-				 XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT | \
-				 XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT | \
-				 XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT)
-
-#define XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT	GENMASK(4, 3)
-#define XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT	GENMASK(12, 9)
-#define XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT	(BIT(18) | BIT(21))
-#define XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT	GENMASK(29, 29)
-#define MT8195_DRIVING_PARAM_4_DEFAULT	(XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT | \
-				 XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT | \
-				 XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT | \
-				 XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT)
-
-#define XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT	(BIT(3) | BIT(5))
-#define XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT	GENMASK(13, 12)
-#define MT8195_DRIVING_PARAM_5_DEFAULT	(XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT | \
-				 XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT)
-
-#define XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT	0
-#define XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT	GENMASK(10, 10)
-#define XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT	GENMASK(19, 19)
-#define XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT	GENMASK(28, 28)
-#define MT8195_DRIVING_PARAM_6_DEFAULT	(XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT | \
-				 XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT | \
-				 XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT | \
-				 XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT)
-
-#define XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT	0
-#define XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT	GENMASK(10, 9)
-#define XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT	GENMASK(19, 18)
-#define XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT	0
-#define MT8195_DRIVING_PARAM_7_DEFAULT	(XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT | \
-				 XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT | \
-				 XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT | \
-				 XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT)
-
-#define XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT	GENMASK(3, 3)
-#define XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT	0
-#define MT8195_DRIVING_PARAM_8_DEFAULT	(XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
-				 XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
+#define XTP_LN_TX_LCTXC_SW0_PRE0	GENMASK(5, 0)
+#define XTP_LN_TX_LCTXC_SW0_PRE1	GENMASK(13, 8)
+#define XTP_LN_TX_LCTXC_SW0_PRE2	GENMASK(21, 16)
+#define XTP_LN_TX_LCTXC_SW0_PRE3	GENMASK(29, 24)
+
+#define XTP_LN_TX_LCTXC_SW1_PRE0	GENMASK(5, 0)
+#define XTP_LN_TX_LCTXC_SW1_PRE1	GENMASK(13, 8)
+#define XTP_LN_TX_LCTXC_SW1_PRE2	GENMASK(21, 16)
+#define XTP_LN_TX_LCTXC_SW2_PRE0	GENMASK(29, 24)
+
+#define XTP_LN_TX_LCTXC_SW2_PRE1	GENMASK(5, 0)
+#define XTP_LN_TX_LCTXC_SW3_PRE0	GENMASK(13, 8)
+
+#define BUILD_DRIVING_PARAM_0(sw0_pre0, sw0_pre1, sw0_pre2, sw0_pre3) (	\
+	FIELD_PREP_CONST(XTP_LN_TX_LCTXC_SW0_PRE0, sw0_pre0) |		\
+	FIELD_PREP_CONST(XTP_LN_TX_LCTXC_SW0_PRE1, sw0_pre1) |		\
+	FIELD_PREP_CONST(XTP_LN_TX_LCTXC_SW0_PRE2, sw0_pre2) |		\
+	FIELD_PREP_CONST(XTP_LN_TX_LCTXC_SW0_PRE3, sw0_pre3)		\
+)
+
+#define BUILD_DRIVING_PARAM_12(sw1_pre0, sw1_pre1, sw1_pre2, sw2_pre0) (\
+	FIELD_PREP_CONST(XTP_LN_TX_LCTXC_SW1_PRE0, sw1_pre0) |		\
+	FIELD_PREP_CONST(XTP_LN_TX_LCTXC_SW1_PRE1, sw1_pre1) |		\
+	FIELD_PREP_CONST(XTP_LN_TX_LCTXC_SW1_PRE2, sw1_pre2) |		\
+	FIELD_PREP_CONST(XTP_LN_TX_LCTXC_SW2_PRE0, sw2_pre0)		\
+)
+
+#define BUILD_DRIVING_PARAM_23(sw2_pre1, sw3_pre0) (			\
+	FIELD_PREP_CONST(XTP_LN_TX_LCTXC_SW2_PRE1, sw2_pre1) |		\
+	FIELD_PREP_CONST(XTP_LN_TX_LCTXC_SW3_PRE0, sw3_pre0)		\
+)
+
+/* MT8195: Logic State Change Point (LC TX C) */
+#define MT8195_DRIVING_PARAM_3_DEFAULT	BUILD_DRIVING_PARAM_0( 16, 20, 24, 32)
+#define MT8195_DRIVING_PARAM_4_DEFAULT	BUILD_DRIVING_PARAM_12(24, 30, 36, 32)
+#define MT8195_DRIVING_PARAM_5_DEFAULT	BUILD_DRIVING_PARAM_23(40, 48)
+
+/* MT8195: Positive Edge (LC TX CP) */
+#define MT8195_DRIVING_PARAM_6_DEFAULT	BUILD_DRIVING_PARAM_0( 0, 4, 8, 16)
+#define MT8195_DRIVING_PARAM_7_DEFAULT	BUILD_DRIVING_PARAM_12(0, 6, 12, 0)
+#define MT8195_DRIVING_PARAM_8_DEFAULT	BUILD_DRIVING_PARAM_23(8, 0)
 
 enum mtk_dp_phya_ana_glb_regidx {
 	DP_PHYA_GLB_BIAS_GEN_0,
-- 
2.54.0


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

[PATCH v3 10/12] phy: phy-mtk-dp: Add bitrate register val definitions to SoC data

[AngeloGioacchino Del Regno]

In preparation for adding support for the eDP PHY found in newer
SoCs, transfer the bitrate register value definitions to SoC
specific data.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-dp.c | 36 +++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index df193c712c77..4603f28d1466 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -44,10 +44,6 @@
 
 /* DP_PHYD_BIT_RATE */
 #define PHYD_DIG_RG_BIT_RATE		GENMASK(1, 0)
-#  define BIT_RATE_RBR			0
-#  define BIT_RATE_HBR			1
-#  define BIT_RATE_HBR2			2
-#  define BIT_RATE_HBR3			3
 
 /* DP_PHYD_SW_RST */
 #define PHYD_DIG_GLB_SW_RST_B		GENMASK(7, 0)
@@ -163,6 +159,14 @@ enum mtk_dp_phyd_dig_glb_regidx {
 	DP_PHYD_GLOBAL_MAX
 };
 
+enum mtk_dp_phyd_bit_rate_regval {
+	DP_PHYD_BIT_RATE_RBR,
+	DP_PHYD_BIT_RATE_HBR,
+	DP_PHYD_BIT_RATE_HBR2,
+	DP_PHYD_BIT_RATE_HBR3,
+	DP_PHYD_BIT_RATE_MAX,
+};
+
 static const u8 mt8195_phy_ana_glb_regs[DP_PHYA_GLOBAL_MAX] = {
 	[DP_PHYA_GLB_BIAS_GEN_0] = 0x0,
 	[DP_PHYA_GLB_BIAS_GEN_1] = 0x4,
@@ -189,6 +193,13 @@ static const u8 mt8195_phy_dig_glb_regs[DP_PHYD_GLOBAL_MAX] = {
 	[DP_PHYD_TX_CTL_0] = 0x44,
 };
 
+static const u8 mt8195_phy_dig_bitrate_val[DP_PHYD_BIT_RATE_MAX] = {
+	[DP_PHYD_BIT_RATE_RBR] = 0,
+	[DP_PHYD_BIT_RATE_HBR] = 1,
+	[DP_PHYD_BIT_RATE_HBR2] = 2,
+	[DP_PHYD_BIT_RATE_HBR3] = 3
+};
+
 /**
  * struct mtk_dp_phya_imp_sel - Per-Lane Impedance Selection
  * @pmos: Impedance selection for P-Channel MOSFET
@@ -209,6 +220,7 @@ struct mtk_dp_phya_imp_sel {
  * @regs_ana_lane:  Register (layout) offsets for ana_lan
  * @regs_dig_glb:   Register (layout) offsets for dig_glb
  * @regs_dig_lane:  Register (layout) offsets for dig_lan
+ * @val_dig_bitrate:IP Version specific register values for Bit Rate setting
  * @ana_bias_r:     Internal resistance "R" Selection Settings (global)
  * @ana_cktx_imp:   TX Clock Impedance Selection Settings (global)
  * @ana_lanes_imp:  TX Impedance Selection Settings (for all lanes)
@@ -227,6 +239,9 @@ struct mtk_dp_phy_pdata {
 	const u8 *regs_dig_glb;
 	const u8 *regs_dig_lane;
 
+	/* IP-Version specific register value arrays */
+	const u8 *val_dig_bitrate;
+
 	/* Calibration defaults */
 	u8 ana_bias_r;
 	u8 ana_cktx_imp;
@@ -332,6 +347,7 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
 
 	if (opts->dp.set_rate) {
 		const u32 reg_bit_rate = pdata->regs_dig_glb[DP_PHYD_BIT_RATE];
+		enum mtk_dp_phyd_bit_rate_regval regval_idx;
 
 		switch (opts->dp.link_rate) {
 		default:
@@ -340,19 +356,20 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
 				opts->dp.link_rate);
 			return -EINVAL;
 		case 1620:
-			val = BIT_RATE_RBR;
+			regval_idx = DP_PHYD_BIT_RATE_RBR;
 			break;
 		case 2700:
-			val = BIT_RATE_HBR;
+			regval_idx = DP_PHYD_BIT_RATE_HBR;
 			break;
 		case 5400:
-			val = BIT_RATE_HBR2;
+			regval_idx = DP_PHYD_BIT_RATE_HBR2;
 			break;
 		case 8100:
-			val = BIT_RATE_HBR3;
+			regval_idx = DP_PHYD_BIT_RATE_HBR3;
 			break;
 		}
-		regmap_write(dp_phy->regmap, pdata->off_dig_glb + reg_bit_rate, val);
+		regmap_write(dp_phy->regmap, pdata->off_dig_glb + reg_bit_rate,
+			     pdata->val_dig_bitrate[regval_idx]);
 	}
 
 	if (opts->dp.set_lanes) {
@@ -763,6 +780,7 @@ static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
 	.regs_ana_lane = mt8195_phy_ana_lane_regs,
 	.regs_dig_glb = mt8195_phy_dig_glb_regs,
 	.regs_dig_lane = mt8195_phy_dig_lane_regs,
+	.val_dig_bitrate = mt8195_phy_dig_bitrate_val,
 	.ana_bias_r = 15,
 	.ana_cktx_imp = 8,
 	.ana_lanes_imp = {
-- 
2.54.0


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

[PATCH v3 11/12] phy: phy-mtk-dp: Add PHYD Lane EN register mask to SoC data

[AngeloGioacchino Del Regno]

In preparation for adding support for the eDP PHY found in newer
SoCs, transfer the register mask for PHYD_TX_LN_EN to SoC specific
data.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-dp.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index 4603f28d1466..f5aadc5fdc42 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -220,6 +220,7 @@ struct mtk_dp_phya_imp_sel {
  * @regs_ana_lane:  Register (layout) offsets for ana_lan
  * @regs_dig_glb:   Register (layout) offsets for dig_glb
  * @regs_dig_lane:  Register (layout) offsets for dig_lan
+ * @mask_dig_tx_ln: Register mask for PHYD_TX_LN_EN field
  * @val_dig_bitrate:IP Version specific register values for Bit Rate setting
  * @ana_bias_r:     Internal resistance "R" Selection Settings (global)
  * @ana_cktx_imp:   TX Clock Impedance Selection Settings (global)
@@ -239,6 +240,9 @@ struct mtk_dp_phy_pdata {
 	const u8 *regs_dig_glb;
 	const u8 *regs_dig_lane;
 
+	/* Register masks */
+	u32 mask_dig_tx_ln;
+
 	/* IP-Version specific register value arrays */
 	const u8 *val_dig_bitrate;
 
@@ -377,10 +381,10 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
 
 		val = 0;
 		for (i = 0; i < opts->dp.lanes; i++)
-			val |= FIELD_PREP(PHYD_TX_LN_EN, BIT(i));
+			val |= field_prep(pdata->mask_dig_tx_ln, BIT(i));
 
 		regmap_update_bits(dp_phy->regmap, pdata->off_dig_glb + reg_dig_tx_ctl,
-				   PHYD_TX_LN_EN, val);
+				   pdata->mask_dig_tx_ln, val);
 	}
 
 	if (opts->dp.set_voltages) {
@@ -456,7 +460,7 @@ static int mtk_dp_phy_disable_all_lanes(struct mtk_dp_phy *dp_phy)
 		return ret;
 
 	/* Get mask of currently enabled lane */
-	val = FIELD_GET(PHYD_TX_LN_EN, val);
+	val = field_get(pdata->mask_dig_tx_ln, val);
 	if (val == 0)
 		return 0;
 
@@ -467,7 +471,7 @@ static int mtk_dp_phy_disable_all_lanes(struct mtk_dp_phy *dp_phy)
 
 		ret = regmap_clear_bits(dp_phy->regmap,
 					pdata->off_dig_glb + regs[DP_PHYD_TX_CTL_0],
-					FIELD_PREP(PHYD_TX_LN_EN, BIT(lane_num)));
+					field_prep(pdata->mask_dig_tx_ln, BIT(lane_num)));
 		if (ret)
 			return ret;
 	} while (val);
@@ -780,6 +784,7 @@ static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
 	.regs_ana_lane = mt8195_phy_ana_lane_regs,
 	.regs_dig_glb = mt8195_phy_dig_glb_regs,
 	.regs_dig_lane = mt8195_phy_dig_lane_regs,
+	.mask_dig_tx_ln = PHYD_TX_LN_EN,
 	.val_dig_bitrate = mt8195_phy_dig_bitrate_val,
 	.ana_bias_r = 15,
 	.ana_cktx_imp = 8,
-- 
2.54.0


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

[PATCH v3 12/12] phy: phy-mtk-dp: Add support for MT8196 eDP PHY

[AngeloGioacchino Del Regno]

The MT8196 SoC features an updated PHY IP compared to the older
ones, and there is one that is specific to Embedded DisplayPort.

Add support for the eDP PHY found in the MediaTek MT8196 SoC and
all of its variants.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-dp.c | 66 ++++++++++++++++++++++++++++++-
 1 file changed, 65 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index f5aadc5fdc42..5decc29c77cb 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -5,7 +5,7 @@
  * Copyright (c) 2022, BayLibre Inc.
  * Copyright (c) 2022, MediaTek Inc.
  *
- * Major refactoring
+ * Major refactoring and new SoCs support
  * Copyright (c) 2026, Collabora Ltd.
  *                     AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
  */
@@ -43,6 +43,7 @@
 #define TPLL_SSC_EN			BIT(3)
 
 /* DP_PHYD_BIT_RATE */
+#define PHYD_DIG_RG_BIT_RATE_V2		GENMASK(3, 0)
 #define PHYD_DIG_RG_BIT_RATE		GENMASK(1, 0)
 
 /* DP_PHYD_SW_RST */
@@ -63,6 +64,7 @@
 
 /* DP_PHYD_TX_CTL_0 */
 #define PHYD_TX_LN_EN			GENMASK(7, 4)
+#define PHYD_TX_LN_EN_V2		GENMASK(3, 0)
 
 /* DP_PHYD_DRIVING_FORCE */
 #define PHYD_DP_TX_FORCE_VOLT_SWING_EN	BIT(0)
@@ -129,6 +131,16 @@
 #define MT8195_DRIVING_PARAM_7_DEFAULT	BUILD_DRIVING_PARAM_12(0, 6, 12, 0)
 #define MT8195_DRIVING_PARAM_8_DEFAULT	BUILD_DRIVING_PARAM_23(8, 0)
 
+/* MT8196/MT6991: Logic State Change Point (LC TX C) */
+#define MT8196_DRIVING_PARAM_3_DEFAULT	BUILD_DRIVING_PARAM_0( 10, 12, 14, 17)
+#define MT8196_DRIVING_PARAM_4_DEFAULT	BUILD_DRIVING_PARAM_12(14, 17, 18, 18)
+#define MT8196_DRIVING_PARAM_5_DEFAULT	BUILD_DRIVING_PARAM_23(21, 24)
+
+/* MT8196/MT6991: Positive Edge (LC TX CP) */
+#define MT8196_DRIVING_PARAM_6_DEFAULT	BUILD_DRIVING_PARAM_0( 0, 2, 4, 7)
+#define MT8196_DRIVING_PARAM_7_DEFAULT	BUILD_DRIVING_PARAM_12(0, 3, 6, 0)
+#define MT8196_DRIVING_PARAM_8_DEFAULT	BUILD_DRIVING_PARAM_23(3, 0)
+
 enum mtk_dp_phya_ana_glb_regidx {
 	DP_PHYA_GLB_BIAS_GEN_0,
 	DP_PHYA_GLB_BIAS_GEN_1,
@@ -184,6 +196,11 @@ static const u8 mt8195_phy_dig_lane_regs[DP_PHYD_LAN_MAX] = {
 	[DP_PHYD_LAN_DRIVING_PARAM_0] = 0x2c,
 };
 
+static const u8 mt8196_phy_dig_lane_regs[DP_PHYD_LAN_MAX] = {
+	[DP_PHYD_LAN_DRIVING_FORCE] = 0x30,
+	[DP_PHYD_LAN_DRIVING_PARAM_0] = 0x34,
+};
+
 static const u8 mt8195_phy_dig_glb_regs[DP_PHYD_GLOBAL_MAX] = {
 	[DP_PHYD_PLL_CTL_0] = 0x10,
 	[DP_PHYD_PLL_CTL_1] = 0x14,
@@ -193,6 +210,15 @@ static const u8 mt8195_phy_dig_glb_regs[DP_PHYD_GLOBAL_MAX] = {
 	[DP_PHYD_TX_CTL_0] = 0x44,
 };
 
+static const u8 mt8196_phy_dig_glb_regs[DP_PHYD_GLOBAL_MAX] = {
+	[DP_PHYD_PLL_CTL_0] = 0x10,
+	[DP_PHYD_PLL_CTL_1] = 0x14,
+	[DP_PHYD_SW_RST] = 0x38,
+	[DP_PHYD_BIT_RATE] = 0x3c,
+	[DP_PHYD_AUX_RX_CTL] = 0x40,
+	[DP_PHYD_TX_CTL_0] = 0x74,
+};
+
 static const u8 mt8195_phy_dig_bitrate_val[DP_PHYD_BIT_RATE_MAX] = {
 	[DP_PHYD_BIT_RATE_RBR] = 0,
 	[DP_PHYD_BIT_RATE_HBR] = 1,
@@ -200,6 +226,13 @@ static const u8 mt8195_phy_dig_bitrate_val[DP_PHYD_BIT_RATE_MAX] = {
 	[DP_PHYD_BIT_RATE_HBR3] = 3
 };
 
+static const u8 mt8196_edp_phy_dig_bitrate_val[DP_PHYD_BIT_RATE_MAX] = {
+	[DP_PHYD_BIT_RATE_RBR] = 1,
+	[DP_PHYD_BIT_RATE_HBR] = 4,
+	[DP_PHYD_BIT_RATE_HBR2] = 7,
+	[DP_PHYD_BIT_RATE_HBR3] = 9
+};
+
 /**
  * struct mtk_dp_phya_imp_sel - Per-Lane Impedance Selection
  * @pmos: Impedance selection for P-Channel MOSFET
@@ -805,8 +838,39 @@ static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
 	},
 };
 
+static const struct mtk_dp_phy_pdata mt8196_edp_phy_data = {
+	.off_ana_glb = 0x400,
+	.off_ana_lane = (const u16[]) { 0x0, 0x100, 0x200, 0x300 },
+	.off_dig_glb = 0x1400,
+	.off_dig_lane = (const u16[]) { 0x1000, 0x1100, 0x1200, 0x1300 },
+	.regs_ana_glb = mt8195_phy_ana_glb_regs,
+	.regs_ana_lane = mt8195_phy_ana_lane_regs,
+	.regs_dig_glb = mt8196_phy_dig_glb_regs,
+	.regs_dig_lane = mt8196_phy_dig_lane_regs,
+	.mask_dig_tx_ln = PHYD_TX_LN_EN_V2,
+	.val_dig_bitrate = mt8196_edp_phy_dig_bitrate_val,
+	.ana_bias_r = 15,
+	.ana_cktx_imp = 8,
+	.ana_lanes_imp = {
+		.pmos = 8,
+		.nmos = 8,
+	},
+	.driving_params = (const u32[]) {
+		[0] = 0,
+		[1] = 0,
+		[2] = 0,
+		[3] = MT8196_DRIVING_PARAM_3_DEFAULT,
+		[4] = MT8196_DRIVING_PARAM_4_DEFAULT,
+		[5] = MT8196_DRIVING_PARAM_5_DEFAULT,
+		[6] = MT8196_DRIVING_PARAM_6_DEFAULT,
+		[7] = MT8196_DRIVING_PARAM_7_DEFAULT,
+		[8] = MT8196_DRIVING_PARAM_8_DEFAULT
+	},
+};
+
 static const struct of_device_id mtk_dp_phy_of_match[] = {
 	{ .compatible = "mediatek,mt8195-dp-phy", .data = &mt8195_dp_phy_data },
+	{ .compatible = "mediatek,mt8196-edp-phy", .data = &mt8196_edp_phy_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, mtk_dp_phy_of_match);
-- 
2.54.0


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

Re: [PATCH v3 12/12] phy: phy-mtk-dp: Add support for MT8196 eDP PHY

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing NULL check for the result of `device_get_match_data(dev)` before dereferencing it.
--

--- Patch [12]: [PATCH v3 12/12] phy: phy-mtk-dp: Add support for MT8196 eDP PHY ---
commit 171cadff965a41b7aba8d81fbb60622f2a8e1a97
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

phy: phy-mtk-dp: Add support for MT8196 eDP PHY

The MT8196 SoC features an updated PHY IP compared to the older
ones, and there is one that is specific to Embedded DisplayPort.
This commit adds support for the eDP PHY found in the MediaTek MT8196 SoC
and all of its variants.

> diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
> index f5aadc5fdc42c..5decc29c77cb3 100644
> --- a/drivers/phy/mediatek/phy-mtk-dp.c
> +++ b/drivers/phy/mediatek/phy-mtk-dp.c

[ ... ]

> @@ -805,8 +838,39 @@ static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
>  	},
>  };
>  
> +static const struct mtk_dp_phy_pdata mt8196_edp_phy_data = {
> +	.off_ana_glb = 0x400,

[ ... ]

> +	.driving_params = (const u32[]) {
> +		[0] = 0,
> +		[1] = 0,
> +		[2] = 0,
> +		[3] = MT8196_DRIVING_PARAM_3_DEFAULT,
> +		[4] = MT8196_DRIVING_PARAM_4_DEFAULT,
> +		[5] = MT8196_DRIVING_PARAM_5_DEFAULT,
> +		[6] = MT8196_DRIVING_PARAM_6_DEFAULT,
> +		[7] = MT8196_DRIVING_PARAM_7_DEFAULT,
> +		[8] = MT8196_DRIVING_PARAM_8_DEFAULT
> +	},
> +};
> +
>  static const struct of_device_id mtk_dp_phy_of_match[] = {
>  	{ .compatible = "mediatek,mt8195-dp-phy", .data = &mt8195_dp_phy_data },
> +	{ .compatible = "mediatek,mt8196-edp-phy", .data = &mt8196_edp_phy_data },
>  	{ /* sentinel */ }
>  };

[Severity: High]
If the driver is forcefully bound to a device via the sysfs bind interface,
and the device has an of_node but its compatible string does not match any
entry in the driver's of_match_table, device_get_match_data(dev) will return
NULL.

This NULL pointer is stored in dp_phy->pdata:

drivers/phy/mediatek/phy-mtk-dp.c:mtk_dp_phy_probe() {
    ...
	dp_phy->pdata = device_get_match_data(dev);

	if (IS_REACHABLE(CONFIG_NVMEM)) {
		ret = mtk_dp_phy_get_calibration_data(dp_phy);
    ...
}

And is unconditionally dereferenced here:

drivers/phy/mediatek/phy-mtk-dp.c:mtk_dp_phy_get_default_cal_data() {
    ...
	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
	int i;

	dp_phy->ana_bias_r = pdata->ana_bias_r;
    ...
}

Should device_get_match_data() be checked for NULL before continuing in
mtk_dp_phy_probe() to prevent a potential kernel panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707154245.198361-1-angelogioacchino.delregno@collabora.com?part=12

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

Re: [PATCH v3 12/12] phy: phy-mtk-dp: Add support for MT8196 eDP PHY

[AngeloGioacchino Del Regno]

On 7/7/26 18:05, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing NULL check for the result of `device_get_match_data(dev)` before dereferencing it.
> --
> 
> --- Patch [12]: [PATCH v3 12/12] phy: phy-mtk-dp: Add support for MT8196 eDP PHY ---
> commit 171cadff965a41b7aba8d81fbb60622f2a8e1a97
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> phy: phy-mtk-dp: Add support for MT8196 eDP PHY
> 
> The MT8196 SoC features an updated PHY IP compared to the older
> ones, and there is one that is specific to Embedded DisplayPort.
> This commit adds support for the eDP PHY found in the MediaTek MT8196 SoC
> and all of its variants.
> 
>> diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
>> index f5aadc5fdc42c..5decc29c77cb3 100644
>> --- a/drivers/phy/mediatek/phy-mtk-dp.c
>> +++ b/drivers/phy/mediatek/phy-mtk-dp.c
> 
> [ ... ]
> 
>> @@ -805,8 +838,39 @@ static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
>>   	},
>>   };
>>   
>> +static const struct mtk_dp_phy_pdata mt8196_edp_phy_data = {
>> +	.off_ana_glb = 0x400,
> 
> [ ... ]
> 
>> +	.driving_params = (const u32[]) {
>> +		[0] = 0,
>> +		[1] = 0,
>> +		[2] = 0,
>> +		[3] = MT8196_DRIVING_PARAM_3_DEFAULT,
>> +		[4] = MT8196_DRIVING_PARAM_4_DEFAULT,
>> +		[5] = MT8196_DRIVING_PARAM_5_DEFAULT,
>> +		[6] = MT8196_DRIVING_PARAM_6_DEFAULT,
>> +		[7] = MT8196_DRIVING_PARAM_7_DEFAULT,
>> +		[8] = MT8196_DRIVING_PARAM_8_DEFAULT
>> +	},
>> +};
>> +
>>   static const struct of_device_id mtk_dp_phy_of_match[] = {
>>   	{ .compatible = "mediatek,mt8195-dp-phy", .data = &mt8195_dp_phy_data },
>> +	{ .compatible = "mediatek,mt8196-edp-phy", .data = &mt8196_edp_phy_data },
>>   	{ /* sentinel */ }
>>   };
> 
> [Severity: High]
> If the driver is forcefully bound to a device via the sysfs bind interface,
> and the device has an of_node but its compatible string does not match any
> entry in the driver's of_match_table, device_get_match_data(dev) will return
> NULL.
> 
> This NULL pointer is stored in dp_phy->pdata:
> 
> drivers/phy/mediatek/phy-mtk-dp.c:mtk_dp_phy_probe() {
>      ...
> 	dp_phy->pdata = device_get_match_data(dev);
> 
> 	if (IS_REACHABLE(CONFIG_NVMEM)) {
> 		ret = mtk_dp_phy_get_calibration_data(dp_phy);
>      ...
> }
> 
> And is unconditionally dereferenced here:
> 
> drivers/phy/mediatek/phy-mtk-dp.c:mtk_dp_phy_get_default_cal_data() {
>      ...
> 	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
> 	int i;
> 
> 	dp_phy->ana_bias_r = pdata->ana_bias_r;
>      ...
> }
> 
> Should device_get_match_data() be checked for NULL before continuing in
> mtk_dp_phy_probe() to prevent a potential kernel panic?
> 

Oh just shut up, Sashiko! :-)

Too many paranoid checks proposed, are we trying to slow down Linux by a factor
of 1000 with all the defensive coding?

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