Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH phy-next v2] phy: apple: apple: Use local variable for ioremap return value
From: Vladimir Oltean @ 2026-02-16  9:04 UTC (permalink / raw)
  To: Janne Grunau
  Cc: Sven Peter, Neal Gompa, Vinod Koul, Neil Armstrong, Philipp Zabel,
	asahi, linux-phy, linux-kernel, linux-arm-kernel, Dan Carpenter
In-Reply-To: <20260215-phy-apple-resource-err-ptr-v2-1-e43c22453682@jannau.net>

On Sun, Feb 15, 2026 at 09:02:51AM +0100, Janne Grunau wrote:
> The indirection through the resources array is unnecessarily complicated
> and resuling in using IS_ERR() and PTR_ERR() on a valid address. A local
> variable for the devm_ioremap_resource() return value is both easier to
> read and matches expectations when reading code.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/asahi/aYXvX1bYOXtYCgfC@stanley.mountain/
> Suggested-by: Vladimir Oltean <olteanv@gmail.com>
> Fixes: 8e98ca1e74db ("phy: apple: Add Apple Type-C PHY")
> Signed-off-by: Janne Grunau <j@jannau.net>
> ---
> Changes in v2:
> - Use a local variable instead of the complex indirection with the
>   resources array
> - Link to v1: https://lore.kernel.org/r/20260207-phy-apple-resource-err-ptr-v1-1-78735b07ed2d@jannau.net
> ---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

I hope this can be picked up for the linux-phy PR.

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

^ permalink raw reply

* Re: [PATCH] phy: hisilicon: Fix OF node reference leak
From: Andy Shevchenko @ 2026-02-16  9:09 UTC (permalink / raw)
  To: Vladimir Oltean; +Cc: Haotian Zhang, vkoul, kishon, linux-phy, linux-kernel
In-Reply-To: <20260216084348.e2ozdqy5unvfs7qe@skbuf>

On Mon, Feb 16, 2026 at 10:43:48AM +0200, Vladimir Oltean wrote:
> On Wed, Nov 12, 2025 at 02:22:46PM +0800, Haotian Zhang wrote:

...

>  	pcie_dev = bus_find_device_by_of_node(&platform_bus_type, pcie_port);
> +	of_node_put(pcie_port);
>  	if (!pcie_dev) {
>  		dev_err(dev, "Didn't find pcie device\n");
>  		return -ENODEV;
>  	}
> 
> Note that there exists a second reference leak bug in the same function.
> bus_find_device_by_of_node() requires put_device(pcie_dev)

Note, there is a pci_* wrapper for that.

> after it is no longer needed.

Is it only a local variable? If so, it's probably okay to put it, but that
action needs more investigations of the how the pcie_dev is being used.
Also, it might be (but I don't think it is) a (double) put_device() call
somewhere else. TL;DR: the summary of this investigation should be present
in the commit message.

-- 
With Best Regards,
Andy Shevchenko



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

^ permalink raw reply

* Re: [PATCH] phy: hisilicon: Fix OF node reference leak
From: Andy Shevchenko @ 2026-02-16  9:11 UTC (permalink / raw)
  To: Vladimir Oltean; +Cc: Haotian Zhang, vkoul, kishon, linux-phy, linux-kernel
In-Reply-To: <aZLe4-w0Jb50O63N@smile.fi.intel.com>

On Mon, Feb 16, 2026 at 11:09:59AM +0200, Andy Shevchenko wrote:
> On Mon, Feb 16, 2026 at 10:43:48AM +0200, Vladimir Oltean wrote:
> > On Wed, Nov 12, 2025 at 02:22:46PM +0800, Haotian Zhang wrote:

...

> >  	pcie_dev = bus_find_device_by_of_node(&platform_bus_type, pcie_port);
> > +	of_node_put(pcie_port);
> >  	if (!pcie_dev) {
> >  		dev_err(dev, "Didn't find pcie device\n");
> >  		return -ENODEV;
> >  	}
> > 
> > Note that there exists a second reference leak bug in the same function.
> > bus_find_device_by_of_node() requires put_device(pcie_dev)
> 
> Note, there is a pci_* wrapper for that.

Now I re-read this and found confusing comment, please, disregard this.

> > after it is no longer needed.
> 
> Is it only a local variable? If so, it's probably okay to put it, but that
> action needs more investigations of the how the pcie_dev is being used.
> Also, it might be (but I don't think it is) a (double) put_device() call
> somewhere else. TL;DR: the summary of this investigation should be present
> in the commit message.

-- 
With Best Regards,
Andy Shevchenko



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

^ permalink raw reply

* Re: [PATCH v9 1/7] phy: can-transceiver: rename temporary helper function to avoid conflict
From: Vladimir Oltean @ 2026-02-16  9:29 UTC (permalink / raw)
  To: Josua Mayer
  Cc: Geert Uytterhoeven, Marc Kleine-Budde, Vincent Mailhol,
	Vinod Koul, Neil Armstrong, Peter Rosin, Aaro Koskinen,
	Andreas Kemnade, Kevin Hilman, Roger Quadros, Tony Lindgren,
	Janusz Krzysztofik, Vignesh R, Andi Shyti, Ulf Hansson,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, Wolfram Sang, Yazan Shhady,
	Jon Nettleton, Mikhail Anikin, linux-can@vger.kernel.org,
	linux-phy@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-omap@vger.kernel.org, linux-i2c@vger.kernel.org,
	linux-mmc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
In-Reply-To: <f9ede0d3-6a37-449c-b62b-a5c761ece097@solid-run.com>

Hi Josua,

On Mon, Feb 16, 2026 at 08:19:27AM +0000, Josua Mayer wrote:
> >> In the future, when you have a series with cross-tree dependencies,
> >> please try to think of it as individual mini-series for each tree's
> >> 'next' branch, and specify clearly that you need stable tags (to be
> >> pulled into other trees).
> 
> I don't really understand how I could split my series up to avoid this 
> issue.
> 
> Due to the fact that one (and now two) drivers implemented local
> mux helpers, to undo that an atomic change must be made tree-wide.
> 
> Meanwhile it must be avoided that while the mux core helpers are being
> tested / reviewed, that any tree adds another driver-local mux helper
> like appears to have happened here.
> 
> Note that my patch-set did go to linux-phy@lists.infradead.org list, too.
> 
> The second challenge for this series was that mux framework is being
> enabled only by drivers Kconfig "select" - and not possible by menuconfig.
> This is e.g. responsible for being unable to test =m build with arm64
> defconfig - and lead to it only being detected through kernel robot
> x86_64 allmodconfig.

To avoid this, a combination of developer due diligence + maintainer due
diligence is probably required.

From linux-phy perspective, there will be some automated build testing
(which did not exist at the time of your submission). This would have
caught the 'hidden' devm_mux_state_get_optional() call present only in
linux-phy/next, when testing patch 2/7.

But, to work, the build automation needs to be able to apply the entire
patch set on linux-phy/next. So expect some pushback if it doesn't
(hence the recommendation to send a mini-series to linux-phy first, and
request a stable tag).

These are the tools we have, we need to find a way to make them work somehow.

Then there is the fact that local definitions of devm_mux_state_get_optional()
keep popping up, possibly in unrelated trees (not the case here). This seems
to be a bad practice which should be discouraged during review if caught.
Otherwise, some 'retries' will be required from the developer until all
occurrences are removed.

Note that the upcoming linux-phy automated build testing does have an
x86_64 allmodconfig test too.

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

^ permalink raw reply

* Re: [PATCH v2 5/6] arm64: dts: qcom: milos: Add UFS nodes
From: Abel Vesa @ 2026-02-16 10:05 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Luca Weiss, Herbert Xu, David S. Miller, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Alim Akhtar,
	Avri Altman, Bart Van Assche, Vinod Koul, Neil Armstrong,
	Konrad Dybcio, ~postmarketos/upstreaming, phone-devel,
	linux-arm-msm, linux-crypto, devicetree, linux-kernel, linux-scsi,
	linux-phy, Konrad Dybcio
In-Reply-To: <lvaxthcmqvjit4hnofqikxog3vi557elctiqc3nj3ere7rs47v@xcnwzrzc6koy>

On 26-02-13 23:06:51, Dmitry Baryshkov wrote:
> On Tue, Jan 20, 2026 at 04:52:43PM +0200, Abel Vesa wrote:
> > On 26-01-20 16:49:26, Abel Vesa wrote:
> > > On 26-01-12 14:53:18, Luca Weiss wrote:
> > > > Add the nodes for the UFS PHY and UFS host controller, along with the
> > > > ICE used for UFS.
> > > > 
> > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/milos.dtsi | 129 +++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 126 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/qcom/milos.dtsi b/arch/arm64/boot/dts/qcom/milos.dtsi
> > > > index e1a51d43943f..7c8a84bfaee1 100644
> > > > --- a/arch/arm64/boot/dts/qcom/milos.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/milos.dtsi
> > > > @@ -1151,6 +1151,129 @@ aggre2_noc: interconnect@1700000 {
> > > >  			qcom,bcm-voters = <&apps_bcm_voter>;
> > > >  		};
> > > >  
> > > > +		ufs_mem_phy: phy@1d80000 {
> > > > +			compatible = "qcom,milos-qmp-ufs-phy";
> > > > +			reg = <0x0 0x01d80000 0x0 0x2000>;
> > > > +
> > > > +			clocks = <&rpmhcc RPMH_CXO_CLK>,
> > > > +				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
> > > > +				 <&tcsr TCSR_UFS_CLKREF_EN>;
> > > > +			clock-names = "ref",
> > > > +				      "ref_aux",
> > > > +				      "qref";
> > > > +
> > > > +			resets = <&ufs_mem_hc 0>;
> > > > +			reset-names = "ufsphy";
> > > > +
> > > > +			power-domains = <&gcc UFS_MEM_PHY_GDSC>;
> > > > +
> > > > +			#clock-cells = <1>;
> > > > +			#phy-cells = <0>;
> > > > +
> > > > +			status = "disabled";
> > > > +		};
> > > > +
> > > > +		ufs_mem_hc: ufshc@1d84000 {
> > > > +			compatible = "qcom,milos-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
> > > > +			reg = <0x0 0x01d84000 0x0 0x3000>;
> > > > +
> > > > +			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH 0>;
> > > > +
> > > > +			clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
> > > > +				 <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
> > > > +				 <&gcc GCC_UFS_PHY_AHB_CLK>,
> > > > +				 <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
> > > > +				 <&tcsr TCSR_UFS_PAD_CLKREF_EN>,
> > > 
> > > Maybe I'm looking at the wrong documentation, but it doesn't seem to exist
> > > such clock on Milos. It does exist on SM8650 though. So maybe the TCSR CC
> > > driver is not really that much compatible between these two platforms.
> > > 
> > > I take it that the UFS works. Maybe because the actual TCSR UFS clkref
> > > is left enabled at boot?
> > 
> > Oh, nevemind. I think I was looking at the wrong SoC.
> 

Sorry, my bad. Yes. There you go:

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>

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

^ permalink raw reply

* Re: [PATCH v9 1/7] phy: can-transceiver: rename temporary helper function to avoid conflict
From: Geert Uytterhoeven @ 2026-02-16 10:01 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Josua Mayer, Marc Kleine-Budde, Vincent Mailhol, Vinod Koul,
	Neil Armstrong, Peter Rosin, Aaro Koskinen, Andreas Kemnade,
	Kevin Hilman, Roger Quadros, Tony Lindgren, Janusz Krzysztofik,
	Vignesh R, Andi Shyti, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, Wolfram Sang, Yazan Shhady, Jon Nettleton,
	Mikhail Anikin, linux-can@vger.kernel.org,
	linux-phy@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-omap@vger.kernel.org, linux-i2c@vger.kernel.org,
	linux-mmc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
In-Reply-To: <20260216092914.kmvl7aep7dantcsd@skbuf>

Hi Vladimir,

On Mon, 16 Feb 2026 at 10:29, Vladimir Oltean <olteanv@gmail.com> wrote:
> Then there is the fact that local definitions of devm_mux_state_get_optional()
> keep popping up, possibly in unrelated trees (not the case here). This seems
> to be a bad practice which should be discouraged during review if caught.

This was done on purpose, to (1) avoid having to make too many changes
to the file when a common helper would be introduced later, and (2) make
it easy to find all locations where a future common helper could be used.

The alternative is to use a completely different name (which is thus harder
to find), and having to fix up all the users of that name too.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply

* Re: [PATCH v3 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend
From: Johan Hovold @ 2026-02-16 10:41 UTC (permalink / raw)
  To: Loic Poulain
  Cc: vkoul, kishon, linux-arm-msm, linux-phy, dmitry.baryshkov,
	neil.armstrong, konrad.dybcio, Abel Vesa
In-Reply-To: <CAFEp6-0rzkqc0ajN72q=mv3U-TT0JHMSPmnJD6ohKuLmbPz2-A@mail.gmail.com>

On Fri, Feb 13, 2026 at 04:04:43PM +0100, Loic Poulain wrote:
> On Fri, Feb 13, 2026 at 11:45 AM Johan Hovold <johan@kernel.org> wrote:

> > But I'm literally asking for *what* would trigger the suspend in that
> > initial window between enable() and forbid() cause I don't see it.
> 
> To be honest, I had not initially looked into the exact cause of the
> suspend trigger until now, but here is what is happening.
> 
> The PHY is a supplier of the USB device. A USB device cannot be probed
> until all its suppliers are ready. As long as the PHY is not ready, the
> device core keeps retrying the probe, which fails with -EPROBE_DEFER.
> 
> At some point the PHY probe finally runs, but the device core may still be
> attempting to probe the USB device concurrently.
> 
> Inside __driver_probe_device(), we have:
> 
>     ret = really_probe(dev, drv);
>     pm_request_idle(dev);
> 
>     if (dev->parent)
>         pm_runtime_put(dev->parent);
> 
>     pm_runtime_put_suppliers(dev);
>     return ret;
> 
> This means that whenever a USB probe attempt completes, whether with an
> error or not, its suppliers are released via pm_runtime_put_suppliers().
> Releasing suppliers may in turn trigger a runtime suspend.
> 
> In our case, since the PHY is a supplier of the USB device, the USB core
> keeps 'looping' in __driver_probe_device() returning -EPROBE_DEFER until
> the PHY becomes ready. As a result, pm_runtime_put_suppliers() may run
> concurrently with the PHY's probe function. If this happens after
> runtime PM has been enabled for the PHY, but before the driver has
> forbidden suspend or taken a PM reference, the PHY may end up being
> runtime-suspended 'unexpectedly'.

Thanks for tracking that down. That's an unexpected side effect of
fw_devlink adding runtime pm enabled links (which is the default
behaviour since late 2023).

Johan

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

^ permalink raw reply

* Re: [PATCH v3 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend
From: Johan Hovold @ 2026-02-16 10:47 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Loic Poulain, vkoul, kishon, linux-arm-msm, linux-phy,
	dmitry.baryshkov, neil.armstrong, konrad.dybcio, Abel Vesa
In-Reply-To: <20260213201550.vk5cosmoewokarlx@skbuf>

On Fri, Feb 13, 2026 at 10:15:50PM +0200, Vladimir Oltean wrote:

> Another comment upon reviewing this driver's runtime PM use (although
> this is at most something that may result in a patch for "next"):
> 
> This driver uses hsphy->phy_initialized to make sure qcom_snps_hsphy_suspend()
> isn't called unless qcom_snps_hsphy_init() was called.
> 
> Don't we achieve the same behaviour by replacing "hsphy->phy_initialized = true"
> with pm_runtime_get_sync(dev) and "hsphy->phy_initialized = false" with
> pm_runtime_put(dev)?

No, the device can still suspend before phy_init() is called.

What would work, and which should probably be preferred over adding
these phy_initialized flags, is to increment the pm usage counter before
enabling runtime pm and decrementing it after the PHY has been created.

Johan

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

^ permalink raw reply

* [PATCH 1/3] phy: apple: atc: Make atcphy_dwc3_reset_ops variable static
From: Krzysztof Kozlowski @ 2026-02-16 11:04 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Neal Gompa, Vinod Koul, Neil Armstrong,
	Philipp Zabel, asahi, linux-arm-kernel, linux-phy, linux-kernel,
	linux-arm-msm
  Cc: Krzysztof Kozlowski

File-scope 'atcphy_dwc3_reset_ops' is not used outside of this unit, so
make it static to silence sparse warning:

  atc.c:2026:32: warning: symbol 'atcphy_dwc3_reset_ops' was not declared. Should it be static?

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 drivers/phy/apple/atc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/apple/atc.c b/drivers/phy/apple/atc.c
index dc867f368b68..32d97226e926 100644
--- a/drivers/phy/apple/atc.c
+++ b/drivers/phy/apple/atc.c
@@ -2023,7 +2023,7 @@ static int atcphy_dwc3_reset_deassert(struct reset_controller_dev *rcdev, unsign
 	return 0;
 }
 
-const struct reset_control_ops atcphy_dwc3_reset_ops = {
+static const struct reset_control_ops atcphy_dwc3_reset_ops = {
 	.assert = atcphy_dwc3_reset_assert,
 	.deassert = atcphy_dwc3_reset_deassert,
 };
-- 
2.51.0


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

^ permalink raw reply related

* [PATCH 2/3] phy: marvell: mmp3-hsic: Avoid re-casting __iomem
From: Krzysztof Kozlowski @ 2026-02-16 11:04 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Neal Gompa, Vinod Koul, Neil Armstrong,
	Philipp Zabel, asahi, linux-arm-kernel, linux-phy, linux-kernel,
	linux-arm-msm
  Cc: Krzysztof Kozlowski
In-Reply-To: <20260216110413.159994-4-krzysztof.kozlowski@oss.qualcomm.com>

__iomem annotated memory must be accessed via dedicated accessors, even
if actual code is correct (accessing the driver data in
mmp3_hsic_phy_init() brings back the __iomem cast), but dropping its
cast (with or without __force) when storing as driver data seems like
less readable code for any future changes.  Instead, add a dedicated
wrapping structure just to hold the pointer without changing the __iomem
cast.  This makes the code explicit, obvious and solves the sparse
warning:

  phy-mmp3-hsic.c:58:31: warning: cast removes address space '__iomem' of expression

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 drivers/phy/marvell/phy-mmp3-hsic.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/phy/marvell/phy-mmp3-hsic.c b/drivers/phy/marvell/phy-mmp3-hsic.c
index 271f1a2258ef..72ab6da0ebc3 100644
--- a/drivers/phy/marvell/phy-mmp3-hsic.c
+++ b/drivers/phy/marvell/phy-mmp3-hsic.c
@@ -14,15 +14,19 @@
 #define HSIC_ENABLE	BIT(7)
 #define PLL_BYPASS	BIT(4)
 
+struct mmp3_hsic_data {
+	void __iomem *base;
+};
+
 static int mmp3_hsic_phy_init(struct phy *phy)
 {
-	void __iomem *base = (void __iomem *)phy_get_drvdata(phy);
+	struct mmp3_hsic_data *mmp3 = phy_get_drvdata(phy);
 	u32 hsic_ctrl;
 
-	hsic_ctrl = readl_relaxed(base + HSIC_CTRL);
+	hsic_ctrl = readl_relaxed(mmp3->base + HSIC_CTRL);
 	hsic_ctrl |= HSIC_ENABLE;
 	hsic_ctrl |= PLL_BYPASS;
-	writel_relaxed(hsic_ctrl, base + HSIC_CTRL);
+	writel_relaxed(hsic_ctrl, mmp3->base + HSIC_CTRL);
 
 	return 0;
 }
@@ -41,13 +45,17 @@ MODULE_DEVICE_TABLE(of, mmp3_hsic_phy_of_match);
 static int mmp3_hsic_phy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct mmp3_hsic_data *mmp3;
 	struct phy_provider *provider;
-	void __iomem *base;
 	struct phy *phy;
 
-	base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
+	mmp3 = devm_kzalloc(dev, sizeof(*mmp3), GFP_KERNEL);
+	if (!mmp3)
+		return -ENOMEM;
+
+	mmp3->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+	if (IS_ERR(mmp3->base))
+		return PTR_ERR(mmp3->base);
 
 	phy = devm_phy_create(dev, NULL, &mmp3_hsic_phy_ops);
 	if (IS_ERR(phy)) {
@@ -55,7 +63,7 @@ static int mmp3_hsic_phy_probe(struct platform_device *pdev)
 		return PTR_ERR(phy);
 	}
 
-	phy_set_drvdata(phy, (void *)base);
+	phy_set_drvdata(phy, mmp3);
 	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
 	if (IS_ERR(provider)) {
 		dev_err(dev, "failed to register PHY provider\n");
-- 
2.51.0


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

^ permalink raw reply related

* [PATCH 3/3] phy: qcom: qmp-usbc: Simplify check for non-NULL pointer
From: Krzysztof Kozlowski @ 2026-02-16 11:04 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Neal Gompa, Vinod Koul, Neil Armstrong,
	Philipp Zabel, asahi, linux-arm-kernel, linux-phy, linux-kernel,
	linux-arm-msm
  Cc: Krzysztof Kozlowski
In-Reply-To: <20260216110413.159994-4-krzysztof.kozlowski@oss.qualcomm.com>

Pointers should not use explicit '0' comparison, so just use standard
evaluation as non-NULL:

  phy-qcom-qmp-usbc.c:1682:31: warning: Using plain integer as NULL pointer

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
index 14feb77789b3..c342479a3798 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
@@ -1679,7 +1679,7 @@ static int qmp_usbc_register_clocks(struct qmp_usbc *qmp, struct device_node *np
 	if (ret)
 		return ret;
 
-	if (qmp->dp_serdes != 0) {
+	if (qmp->dp_serdes) {
 		ret = phy_dp_clks_register(qmp, np);
 		if (ret)
 			return ret;
@@ -1833,7 +1833,7 @@ static int qmp_usbc_parse_dt(struct qmp_usbc *qmp)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-	if (offs->dp_serdes != 0) {
+	if (offs->dp_serdes) {
 		qmp->dp_serdes = base + offs->dp_serdes;
 		qmp->dp_tx = base + offs->dp_txa;
 		qmp->dp_tx2 = base + offs->dp_txb;
@@ -1982,7 +1982,7 @@ static int qmp_usbc_probe(struct platform_device *pdev)
 
 	phy_set_drvdata(qmp->usb_phy, qmp);
 
-	if (qmp->dp_serdes != 0) {
+	if (qmp->dp_serdes) {
 		qmp->dp_phy = devm_phy_create(dev, np, &qmp_usbc_dp_phy_ops);
 		if (IS_ERR(qmp->dp_phy)) {
 			ret = PTR_ERR(qmp->dp_phy);
-- 
2.51.0


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

^ permalink raw reply related

* Re: [PATCH 4/4] gpu: drm: mediatek: ovl: add specific entry for mt8167
From: AngeloGioacchino Del Regno @ 2026-02-16 11:09 UTC (permalink / raw)
  To: Luca Leonardo Scorcia, linux-mediatek
  Cc: Val Packett, Chun-Kuang Hu, Philipp Zabel, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chunfeng Yun,
	Vinod Koul, Neil Armstrong, Matthias Brugger, Jitao Shi,
	dri-devel, devicetree, linux-kernel, linux-arm-kernel, linux-phy
In-Reply-To: <b440a9c1972dedc4cfbf29a84366e635b0d7d984.1771144723.git.l.scorcia@gmail.com>

Il 15/02/26 09:53, Luca Leonardo Scorcia ha scritto:
> From: Val Packett <val@packett.cool>
> 
> While this configuration is otherwise identical to mt8173, according
> to Android kernel sources, this SoC does need smi_id_en.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>

Okay, sure.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
>   drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index e0236353d499..97a899e4bd99 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -671,6 +671,16 @@ static const struct mtk_disp_ovl_data mt2701_ovl_driver_data = {
>   	.num_formats = ARRAY_SIZE(mt8173_formats),
>   };
>   
> +static const struct mtk_disp_ovl_data mt8167_ovl_driver_data = {
> +	.addr = DISP_REG_OVL_ADDR_MT8173,
> +	.gmc_bits = 8,
> +	.layer_nr = 4,
> +	.fmt_rgb565_is_0 = true,
> +	.smi_id_en = true,
> +	.formats = mt8173_formats,
> +	.num_formats = ARRAY_SIZE(mt8173_formats),
> +};
> +
>   static const struct mtk_disp_ovl_data mt8173_ovl_driver_data = {
>   	.addr = DISP_REG_OVL_ADDR_MT8173,
>   	.gmc_bits = 8,
> @@ -742,6 +752,8 @@ static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
>   static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
>   	{ .compatible = "mediatek,mt2701-disp-ovl",
>   	  .data = &mt2701_ovl_driver_data},
> +	{ .compatible = "mediatek,mt8167-disp-ovl",
> +	  .data = &mt8167_ovl_driver_data},
>   	{ .compatible = "mediatek,mt8173-disp-ovl",
>   	  .data = &mt8173_ovl_driver_data},
>   	{ .compatible = "mediatek,mt8183-disp-ovl",



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

^ permalink raw reply

* Re: [PATCH 2/4] dt-bindings: display: mediatek: Add compatibles for MediaTek mt8167
From: AngeloGioacchino Del Regno @ 2026-02-16 11:09 UTC (permalink / raw)
  To: Luca Leonardo Scorcia, linux-mediatek
  Cc: Chun-Kuang Hu, Philipp Zabel, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chunfeng Yun, Vinod Koul,
	Neil Armstrong, Matthias Brugger, Jitao Shi, dri-devel,
	devicetree, linux-kernel, linux-arm-kernel, linux-phy
In-Reply-To: <826d54c75cfd1b8e4713431a9426e89edade9eb2.1771144723.git.l.scorcia@gmail.com>

Il 15/02/26 09:53, Luca Leonardo Scorcia ha scritto:
> Add compatibles for various display-related blocks of MediaTek mt8167.
> 
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>

Awesome. I'd give you my R-b seal of approval, but there's a few nitpicks that
you have to address. Please check below.

> ---
>   .../devicetree/bindings/display/mediatek/mediatek,aal.yaml   | 1 +
>   .../devicetree/bindings/display/mediatek/mediatek,ccorr.yaml | 4 +++-
>   .../bindings/display/mediatek/mediatek,dither.yaml           | 1 +
>   .../devicetree/bindings/display/mediatek/mediatek,dsi.yaml   | 5 ++++-
>   .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml | 1 +
>   .../devicetree/bindings/display/mediatek/mediatek,ovl.yaml   | 1 +
>   .../devicetree/bindings/display/mediatek/mediatek,rdma.yaml  | 1 +
>   .../devicetree/bindings/display/mediatek/mediatek,wdma.yaml  | 4 +++-
>   Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml  | 1 +
>   9 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml
> index daf90ebb39bf..4bbea72b292a 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml

...snip...

> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml
> index 27ffbccc2a08..bcbde16648c0 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml
> @@ -25,11 +25,14 @@ properties:
>         - enum:
>             - mediatek,mt2701-dsi
>             - mediatek,mt7623-dsi
> -          - mediatek,mt8167-dsi
>             - mediatek,mt8173-dsi
>             - mediatek,mt8183-dsi
>             - mediatek,mt8186-dsi
>             - mediatek,mt8188-dsi
> +      - items:
> +          - enum:
> +              - mediatek,mt8167-dsi
> +          - const: mediatek,mt2701-dsi

This needs its own patch with its own commit description - as this change is not
really "normal".

You have to mention that you're removing the "mediatek,mt8167-dsi" compatible from
that list and that it is safe to do so because:
  - Bootloader doesn't rely on this single compatible; and
  - There was never any upstreamed devicetree using this single compatible; and
  - The MT8167 DSI Controller is fully compatible with the one found in MT2701.

So, please, split this one out of the entire batch.

>         - items:
>             - enum:
>                 - mediatek,mt6795-dsi
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> index 48542dc7e784..ec1054bb06d4 100644

...snip...

> diff --git a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
> index acdbce937b0a..c6d0bbdbe0e2 100644
> --- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml
> @@ -23,6 +23,7 @@ properties:
>         - items:
>             - enum:
>                 - mediatek,mt7623-mipi-tx
> +              - mediatek,mt8167-mipi-tx
>             - const: mediatek,mt2701-mipi-tx
>         - items:
>             - enum:

The PHY bindings are not applied by the same maintainer as the display bindings:
please split that out in a different patch, so that each maintainer can apply
the relevant patch(es).

Besides - apart from those nits, everything in here looks great.

Cheers,
Angelo

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

^ permalink raw reply

* Re: [PATCH 1/4] arm64: dts: mt8167: Reorder nodes according to mmio address
From: AngeloGioacchino Del Regno @ 2026-02-16 11:09 UTC (permalink / raw)
  To: Luca Leonardo Scorcia, linux-mediatek
  Cc: Chun-Kuang Hu, Philipp Zabel, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chunfeng Yun, Vinod Koul,
	Neil Armstrong, Matthias Brugger, Jitao Shi, dri-devel,
	devicetree, linux-kernel, linux-arm-kernel, linux-phy
In-Reply-To: <b7d7c27dda4dc1779425c57d187b84abaf6a1093.1771144723.git.l.scorcia@gmail.com>

Il 15/02/26 09:53, Luca Leonardo Scorcia ha scritto:
> In preparation for adding display nodes. No other changes.
> 
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

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

^ permalink raw reply

* Re: [PATCH 3/4] arm64: dts: mediatek: mt8167: Add DRM nodes
From: AngeloGioacchino Del Regno @ 2026-02-16 11:10 UTC (permalink / raw)
  To: Luca Leonardo Scorcia, linux-mediatek
  Cc: Chun-Kuang Hu, Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chunfeng Yun, Vinod Koul,
	Neil Armstrong, Matthias Brugger, Jitao Shi, dri-devel,
	devicetree, linux-kernel, linux-arm-kernel, linux-phy
In-Reply-To: <efed7184f27f7f85e9b811070a072b81e134e57e.1771144723.git.l.scorcia@gmail.com>

Il 15/02/26 09:53, Luca Leonardo Scorcia ha scritto:
> Add all the DRM nodes required to get DSI to work on MT8167 SoC.
> 
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> ---
>   arch/arm64/boot/dts/mediatek/mt8167.dtsi | 386 +++++++++++++++++++++++
>   1 file changed, 386 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8167.dtsi b/arch/arm64/boot/dts/mediatek/mt8167.dtsi
> index 27cf32d7ae35..c6306234e592 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8167.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8167.dtsi
> @@ -16,6 +16,20 @@
>   / {
>   	compatible = "mediatek,mt8167";
>   
> +	aliases {
> +		aal0 = &aal;
> +		ccorr0 = &ccorr;
> +		color0 = &color;
> +		dither0 = &dither;
> +		dsi0 = &dsi;
> +		gamma0 = &gamma;
> +		ovl0 = &ovl0;
> +		pwm0 = &disp_pwm;
> +		rdma0 = &rdma0;
> +		rdma1 = &rdma1;
> +		wdma0 = &wdma;
> +	};
> +
>   	soc {
>   		topckgen: topckgen@10000000 {
>   			compatible = "mediatek,mt8167-topckgen", "syscon";
> @@ -120,10 +134,371 @@ iommu: m4u@10203000 {
>   			#iommu-cells = <1>;
>   		};
>   
> +		disp_pwm: pwm@1100f000 {
> +			compatible = "mediatek,mt8167-disp-pwm",
> +				     "mediatek,mt8173-disp-pwm";

The compatible strings fit in a single line. Up to 100 columns it's fine: for every
node additions that you're doing, please use a single line wherever you can.

> +			reg = <0 0x1100f000 0 0x1000>;
> +			clocks = <&mmsys CLK_MM_DISP_PWM_26M>,
> +				 <&mmsys CLK_MM_DISP_PWM_MM>;
> +			clock-names = "main",
> +				      "mm";

That also includes clock-names and, where possible, also clocks and anything
else really.

> +			power-domains = <&spm MT8167_POWER_DOMAIN_MM>;
> +			#pwm-cells = <2>;
> +			status = "disabled";
> +		};
> +
>   		mmsys: syscon@14000000 {
>   			compatible = "mediatek,mt8167-mmsys", "syscon";
>   			reg = <0 0x14000000 0 0x1000>;
> +			power-domains = <&spm MT8167_POWER_DOMAIN_MM>;
>   			#clock-cells = <1>;
> +
> +			port {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				mmsys_main: endpoint@0 {
> +					reg = <0>;
> +					remote-endpoint = <&ovl0_in>;
> +				};
> +
> +				mmsys_ext: endpoint@1 {
> +					reg = <1>;
> +					remote-endpoint = <&rdma1_in>;
> +				};

Nice! Thanks for expressing those connections with an OF Graph. That makes things
simpler for other devices based on the same SoC.

> +			};
> +		};
> +
> +		ovl0: ovl0@14007000 {
> +			compatible = "mediatek,mt8167-disp-ovl";
> +			reg = <0 0x14007000 0 0x1000>;
> +			clocks = <&mmsys CLK_MM_DISP_OVL0>;
> +			interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_LOW>;
> +			iommus = <&iommu M4U_PORT_DISP_OVL0>;
> +			power-domains = <&spm MT8167_POWER_DOMAIN_MM>;
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <0>;

For OVL, RDMA, COLOR, CCORR, and others, you have only one endpoint.

Since there is only one, you can compress the nodes and avoid the text bloat
(and please do), by declaring the nodes like:

			ports {
				#address-cells = <1>;
				#size-cells = <0>;

				port@0 {
					reg = <0>;
					ovl0_in: endpoint {
						remote-endpoint = <&mmsys_main>;
					};
				};

				port@1 {
					reg = <1>;
					ovl0_out: endpoint {
						remote-endpoint = <&color_in>;
					};
				};
			};

Please do this for all nodes where you have only a single endpoint (so, for
everything but mmsys).


> +					ovl0_in: endpoint@0 {
> +						reg = <0>;
> +						remote-endpoint = <&mmsys_main>;
> +					};
> +				};
> +
> +				port@1 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <1>;
> +					ovl0_out: endpoint@0 {
> +						reg = <0>;
> +						remote-endpoint = <&color_in>;
> +					};
> +				};
> +			};
> +		};

...snip...

> +
> +		dsi: dsi@14012000 {
> +			compatible = "mediatek,mt8167-dsi",
> +				     "mediatek,mt2701-dsi";
> +			reg = <0 0x14012000 0 0x1000>;
> +			clocks = <&mmsys CLK_MM_DSI_ENGINE>,
> +				 <&mmsys CLK_MM_DSI_DIGITAL>,
> +				 <&mipi_tx>;
> +			clock-names = "engine", "digital", "hs";
> +			interrupts = <GIC_SPI 171 IRQ_TYPE_LEVEL_LOW>;
> +			phys = <&mipi_tx>;
> +			phy-names = "dphy";
> +			power-domains = <&spm MT8167_POWER_DOMAIN_MM>;
> +			status = "disabled";
> +
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;

..snip..

> +
> +				port@1 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <1>;
> +					dsi_out: endpoint@0 {
> +						reg = <0>;
> +					};
> +				};

That's shorter, and simpler:
				port@1 {
					reg = <1>;
					dsi_out: endpoint { };
				};

> +			};
> +		};
> +
> +		mutex: mutex@14015000 {
> +			compatible = "mediatek,mt8167-disp-mutex";
> +			reg = <0 0x14015000 0 0x1000>;
> +			interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
> +			power-domains = <&spm MT8167_POWER_DOMAIN_MM>;
>   		};
>   
>   		larb0: larb@14016000 {
> @@ -145,6 +520,17 @@ smi_common: smi@14017000 {
>   			power-domains = <&spm MT8167_POWER_DOMAIN_MM>;
>   		};
>   
> +		mipi_tx: dsi-phy@14018000 {
> +			compatible = "mediatek,mt8167-mipi-tx",
> +				     "mediatek,mt2701-mipi-tx";

Fits in a single line :-)

> +			reg = <0 0x14018000 0 0x90>;
> +			clocks = <&topckgen CLK_TOP_MIPI_26M_DBG>;
> +			clock-output-names = "mipi_tx0_pll";
> +			#clock-cells = <0>;
> +			#phy-cells = <0>;
> +			status = "disabled";
> +		};
> +
>   		imgsys: syscon@15000000 {
>   			compatible = "mediatek,mt8167-imgsys", "syscon";
>   			reg = <0 0x15000000 0 0x1000>;
Nice job, btw.

Cheers,
Angelo


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

^ permalink raw reply

* Re: [PATCH 3/3] phy: qcom: qmp-usbc: Simplify check for non-NULL pointer
From: Konrad Dybcio @ 2026-02-16 11:33 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Sven Peter, Janne Grunau, Neal Gompa,
	Vinod Koul, Neil Armstrong, Philipp Zabel, asahi,
	linux-arm-kernel, linux-phy, linux-kernel, linux-arm-msm
In-Reply-To: <20260216110413.159994-6-krzysztof.kozlowski@oss.qualcomm.com>

On 2/16/26 12:04 PM, Krzysztof Kozlowski wrote:
> Pointers should not use explicit '0' comparison, so just use standard
> evaluation as non-NULL:
> 
>   phy-qcom-qmp-usbc.c:1682:31: warning: Using plain integer as NULL pointer
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

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

^ permalink raw reply

* Re: [PATCH v6 00/24] MediaTek UFS Cleanup and MT8196 Enablement
From: AngeloGioacchino Del Regno @ 2026-02-16 12:40 UTC (permalink / raw)
  To: Nicolas Frattaroli, Alim Akhtar, Avri Altman, Bart Van Assche,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	Chunfeng Yun, Vinod Koul, Kishon Vijay Abraham I, Peter Wang,
	Stanley Jhu, James E.J. Bottomley, Martin K. Petersen,
	Philipp Zabel, Liam Girdwood, Mark Brown, Chaotian Jing,
	Neil Armstrong
  Cc: Louis-Alexis Eyraud, kernel, linux-scsi, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, linux-phy, Conor Dooley,
	Krzysztof Kozlowski
In-Reply-To: <20260124-mt8196-ufs-v6-0-e7c005b60028@collabora.com>

Il 24/01/26 13:00, Nicolas Frattaroli ha scritto:
> In this series, the existing MediaTek UFS binding is expanded and
> completed to correctly describe not just the existing compatibles, but
> also to introduce a new compatible in the from of the MT8196 SoC.
> 
> The resets, which until now were completely absent from both the UFS
> host controller binding and the UFS PHY binding, are introduced to both.
> This also means the driver's undocumented and, in mainline, unused reset
> logic is reworked. In particular, the PHY reset is no longer a reset of
> the host controller node, but of the PHY node.
> 
> This means the host controller can reset the PHY through the common PHY
> framework.
> 
> The resets remain optional.
> 
> Additionally, a massive number of driver cleanups are introduced. These
> were prompted by me inspecting the driver more closely as I was
> adjusting it to correspond to the binding.
> 
> The driver still implements vendor properties that are undocumented in
> the binding. I did not touch most of those, as I neither want to
> convince the bindings maintainers that they are needed without knowing
> precisely what they're for, nor do I want to argue with the driver
> authors when removing them.
> 
> Due to the "Marie Kondo with a chainsaw" nature of the driver cleanup
> patches, I humbly request that reviewers do not comment on displeasing
> code they see in the context portion of a patch before they've read the
> whole patch series, as that displeasing code may in fact be reworked in
> a subsequent patch of this series. Please keep comments focused on the
> changed lines of the diff; I know there's more that can be done, but it
> doesn't necessarily need to be part of this series.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Nicolas, can you please rebase this series on the latest linux-next?

All that you have to do is resend everything as-is, but drop patch [14/24], as the
same thing that you've done there landed in form of..

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/ufs/host/ufs-mediatek.c?h=next-20260213&id=bbb8d98fb4536594cb104fd630ea0f7dce3771d6

Besides. It feels like we're again playing a resend-and-nobody-cares game again
with this UFS driver.

I want to remind everyone that we've been trying to cleanup this thing for more
than a year and that our patches keep getting mostly ignored, if not for some
random nitpick here and there. (we: as I myself tried to clean it up a year ago
with my own patches that I had sent to the list - multiple versions of those)

Expressing my huge frustration here: at this point I'm not even sure why we keep
trying to make this right, other than being stubborn about feeling the need to
have good stuff in the kernel, instead of downstream code dumps which should've
never landed in the first place.

Though everyone here is human, and humans make mistakes (a lot), that's fine: the
problem happens when people negate mistakes, don't fix those, and don't care (not
because of lack of time, but because not caring at all, again - we've tried to do
those cleanups for a year now). One. Year.
Actually, more than one year, I don't even remember anymore.

This is not good behavior in a community, and should never happen.

Yet, here we are. Again. On the same driver. In the same subsystem.

Oh well.

Regards,
Angelo

> ---
> Changes in v6:
> - Reword "Rework probe function" commit to better justify the changes
>    being made.
> - Drop "Add vendor prefix to clk-scale-up-vcore-min"
> - Add patch to remove clk-scale-up-vcore-min entirely, describing the
>    process for bringing it back (in a different form) in the commit
>    message.
> - Link to v5: https://lore.kernel.org/r/20260108-mt8196-ufs-v5-0-49215157ec41@collabora.com
> 
> Changes in v5:
> - Drop "scsi: ufs: mediatek: Make scale_us in setup_clk_gating const" as
>    someone else already got a patch in for this into next.
> - Make mtk_init_boost_crypt void
> - Don't disable/enable misc regulators during suspend/resume, but enable
>    them once when acquiring with a devm helper.
> - Link to v4: https://lore.kernel.org/r/20251218-mt8196-ufs-v4-0-ddec7a369dd2@collabora.com
> 
> Changes in v4:
> - bindings: Redo the supply situation, as the avdd pins don't describe
>    the vcc(q2) card supplies.
> - bindings: format clock in mt8196 example more tersely.
> - phy: use devm_reset_control_get_optional_exclusive directly
> - driver: get and enable/disable the aforementioned avdd supplies.
> - Link to v3: https://lore.kernel.org/r/20251023-mt8196-ufs-v3-0-0f04b4a795ff@collabora.com
> 
> Changes in v3:
> - Split mediatek,ufs bindings change into two patches, one for
>    completing the existing binding, one for the MT8196
> - Add over a dozen driver cleanup patches
> - Add explicit support for the MT8196 compatible to the driver
> - Note: next-20251023, on which I based this, currently has a broken
>    build due to an unrelated OPP core change that was merged with no
>    build testing. I can't use next-20251022 either, as that lacks the
>    recent mediatek UFS changes. It is what it is.
> - Link to v2: https://lore.kernel.org/r/20251016-mt8196-ufs-v2-0-c373834c4e7a@collabora.com
> 
> Changes in v2:
> - Reorder define in mtk_sip_svc.h
> - Use bulk reset APIs in UFS host driver
> - Link to v1: https://lore.kernel.org/r/20251014-mt8196-ufs-v1-0-195dceb83bc8@collabora.com
> 
> ---
> Nicolas Frattaroli (24):
>        dt-bindings: phy: Add mediatek,mt8196-ufsphy variant
>        dt-bindings: ufs: mediatek,ufs: Complete the binding
>        dt-bindings: ufs: mediatek,ufs: Add mt8196 variant
>        scsi: ufs: mediatek: Move MTK_SIP_UFS_CONTROL to mtk_sip_svc.h
>        phy: mediatek: ufs: Add support for resets
>        scsi: ufs: mediatek: Rework resets
>        scsi: ufs: mediatek: Rework 0.9V regulator
>        scsi: ufs: mediatek: Rework init function
>        scsi: ufs: mediatek: Rework the crypt-boost stuff
>        scsi: ufs: mediatek: Handle misc host voltage regulators
>        scsi: ufs: mediatek: Rework probe function
>        scsi: ufs: mediatek: Remove vendor kernel quirks cruft
>        scsi: ufs: mediatek: Use the common PHY framework
>        scsi: ufs: mediatek: Switch to newer PM ops helpers
>        scsi: ufs: mediatek: Remove mediatek,ufs-broken-rtc property
>        scsi: ufs: mediatek: Rework _ufs_mtk_clk_scale error paths
>        scsi: ufs: mediatek: Clean up logging prints
>        scsi: ufs: mediatek: Rework ufs_mtk_wait_idle_state
>        scsi: ufs: mediatek: Don't acquire dvfsrc-vcore twice
>        scsi: ufs: mediatek: Rework hardware version reading
>        scsi: ufs: mediatek: Back up idle timer in per-instance struct
>        scsi: ufs: mediatek: Remove ret local from link_startup_notify
>        scsi: ufs: mediatek: Remove undocumented "clk-scale-up-vcore-min"
>        scsi: ufs: mediatek: Add MT8196 compatible, update copyright
> 
>   .../devicetree/bindings/phy/mediatek,ufs-phy.yaml  |  16 +
>   .../devicetree/bindings/ufs/mediatek,ufs.yaml      | 173 +++-
>   drivers/phy/mediatek/phy-mtk-ufs.c                 |  71 ++
>   drivers/ufs/host/ufs-mediatek-sip.h                |   9 -
>   drivers/ufs/host/ufs-mediatek.c                    | 973 +++++++++------------
>   drivers/ufs/host/ufs-mediatek.h                    |  17 +-
>   include/linux/soc/mediatek/mtk_sip_svc.h           |   3 +
>   7 files changed, 655 insertions(+), 607 deletions(-)
> ---
> base-commit: 4af4e95edc37ae54f64cbd75b46f16ce15f3a6b8
> change-id: 20251014-mt8196-ufs-cec4b9a97e53
> 
> Best regards,


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

^ permalink raw reply

* [PATCH v7 00/23] MediaTek UFS Cleanup and MT8196 Enablement
From: Nicolas Frattaroli @ 2026-02-16 13:37 UTC (permalink / raw)
  To: Alim Akhtar, Avri Altman, Bart Van Assche, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Chunfeng Yun, Vinod Koul,
	Kishon Vijay Abraham I, Peter Wang, Stanley Jhu,
	James E.J. Bottomley, Martin K. Petersen, Philipp Zabel,
	Liam Girdwood, Mark Brown, Chaotian Jing, Neil Armstrong
  Cc: Louis-Alexis Eyraud, kernel, linux-scsi, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, linux-phy, Nicolas Frattaroli,
	Conor Dooley, Krzysztof Kozlowski

In this series, the existing MediaTek UFS binding is expanded and
completed to correctly describe not just the existing compatibles, but
also to introduce a new compatible in the from of the MT8196 SoC.

The resets, which until now were completely absent from both the UFS
host controller binding and the UFS PHY binding, are introduced to both.
This also means the driver's undocumented and, in mainline, unused reset
logic is reworked. In particular, the PHY reset is no longer a reset of
the host controller node, but of the PHY node.

This means the host controller can reset the PHY through the common PHY
framework.

The resets remain optional.

Additionally, a massive number of driver cleanups are introduced. These
were prompted by me inspecting the driver more closely as I was
adjusting it to correspond to the binding.

The driver still implements vendor properties that are undocumented in
the binding. I did not touch most of those, as I neither want to
convince the bindings maintainers that they are needed without knowing
precisely what they're for, nor do I want to argue with the driver
authors when removing them.

Due to the "Marie Kondo with a chainsaw" nature of the driver cleanup
patches, I humbly request that reviewers do not comment on displeasing
code they see in the context portion of a patch before they've read the
whole patch series, as that displeasing code may in fact be reworked in
a subsequent patch of this series. Please keep comments focused on the
changed lines of the diff; I know there's more that can be done, but it
doesn't necessarily need to be part of this series.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Changes in v7:
- Rebase onto next-20260205, which drops "scsi: ufs: mediatek: Switch to
  newer PM ops helpers" as Arnd sent an equivalent patch that also fixes
  the PM-less build failure.
- Link to v6: https://lore.kernel.org/r/20260124-mt8196-ufs-v6-0-e7c005b60028@collabora.com

Changes in v6:
- Reword "Rework probe function" commit to better justify the changes
  being made.
- Drop "Add vendor prefix to clk-scale-up-vcore-min"
- Add patch to remove clk-scale-up-vcore-min entirely, describing the
  process for bringing it back (in a different form) in the commit
  message.
- Link to v5: https://lore.kernel.org/r/20260108-mt8196-ufs-v5-0-49215157ec41@collabora.com

Changes in v5:
- Drop "scsi: ufs: mediatek: Make scale_us in setup_clk_gating const" as
  someone else already got a patch in for this into next.
- Make mtk_init_boost_crypt void
- Don't disable/enable misc regulators during suspend/resume, but enable
  them once when acquiring with a devm helper.
- Link to v4: https://lore.kernel.org/r/20251218-mt8196-ufs-v4-0-ddec7a369dd2@collabora.com

Changes in v4:
- bindings: Redo the supply situation, as the avdd pins don't describe
  the vcc(q2) card supplies.
- bindings: format clock in mt8196 example more tersely.
- phy: use devm_reset_control_get_optional_exclusive directly
- driver: get and enable/disable the aforementioned avdd supplies.
- Link to v3: https://lore.kernel.org/r/20251023-mt8196-ufs-v3-0-0f04b4a795ff@collabora.com

Changes in v3:
- Split mediatek,ufs bindings change into two patches, one for
  completing the existing binding, one for the MT8196
- Add over a dozen driver cleanup patches
- Add explicit support for the MT8196 compatible to the driver
- Note: next-20251023, on which I based this, currently has a broken
  build due to an unrelated OPP core change that was merged with no
  build testing. I can't use next-20251022 either, as that lacks the
  recent mediatek UFS changes. It is what it is.
- Link to v2: https://lore.kernel.org/r/20251016-mt8196-ufs-v2-0-c373834c4e7a@collabora.com

Changes in v2:
- Reorder define in mtk_sip_svc.h
- Use bulk reset APIs in UFS host driver
- Link to v1: https://lore.kernel.org/r/20251014-mt8196-ufs-v1-0-195dceb83bc8@collabora.com

---
Nicolas Frattaroli (23):
      dt-bindings: phy: Add mediatek,mt8196-ufsphy variant
      dt-bindings: ufs: mediatek,ufs: Complete the binding
      dt-bindings: ufs: mediatek,ufs: Add mt8196 variant
      scsi: ufs: mediatek: Move MTK_SIP_UFS_CONTROL to mtk_sip_svc.h
      phy: mediatek: ufs: Add support for resets
      scsi: ufs: mediatek: Rework resets
      scsi: ufs: mediatek: Rework 0.9V regulator
      scsi: ufs: mediatek: Rework init function
      scsi: ufs: mediatek: Rework the crypt-boost stuff
      scsi: ufs: mediatek: Handle misc host voltage regulators
      scsi: ufs: mediatek: Rework probe function
      scsi: ufs: mediatek: Remove vendor kernel quirks cruft
      scsi: ufs: mediatek: Use the common PHY framework
      scsi: ufs: mediatek: Remove mediatek,ufs-broken-rtc property
      scsi: ufs: mediatek: Rework _ufs_mtk_clk_scale error paths
      scsi: ufs: mediatek: Clean up logging prints
      scsi: ufs: mediatek: Rework ufs_mtk_wait_idle_state
      scsi: ufs: mediatek: Don't acquire dvfsrc-vcore twice
      scsi: ufs: mediatek: Rework hardware version reading
      scsi: ufs: mediatek: Back up idle timer in per-instance struct
      scsi: ufs: mediatek: Remove ret local from link_startup_notify
      scsi: ufs: mediatek: Remove undocumented "clk-scale-up-vcore-min"
      scsi: ufs: mediatek: Add MT8196 compatible, update copyright

 .../devicetree/bindings/phy/mediatek,ufs-phy.yaml  |  16 +
 .../devicetree/bindings/ufs/mediatek,ufs.yaml      | 173 +++-
 drivers/phy/mediatek/phy-mtk-ufs.c                 |  71 ++
 drivers/ufs/host/ufs-mediatek-sip.h                |   9 -
 drivers/ufs/host/ufs-mediatek.c                    | 961 +++++++++------------
 drivers/ufs/host/ufs-mediatek.h                    |  17 +-
 include/linux/soc/mediatek/mtk_sip_svc.h           |   3 +
 7 files changed, 652 insertions(+), 598 deletions(-)
---
base-commit: b6f9fd35d9481a416c75f1e9c8088bc81d24a286
change-id: 20251014-mt8196-ufs-cec4b9a97e53

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>


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

^ permalink raw reply

* [PATCH v7 01/23] dt-bindings: phy: Add mediatek,mt8196-ufsphy variant
From: Nicolas Frattaroli @ 2026-02-16 13:37 UTC (permalink / raw)
  To: Alim Akhtar, Avri Altman, Bart Van Assche, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Chunfeng Yun, Vinod Koul,
	Kishon Vijay Abraham I, Peter Wang, Stanley Jhu,
	James E.J. Bottomley, Martin K. Petersen, Philipp Zabel,
	Liam Girdwood, Mark Brown, Chaotian Jing, Neil Armstrong
  Cc: Louis-Alexis Eyraud, kernel, linux-scsi, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, linux-phy, Nicolas Frattaroli,
	Conor Dooley
In-Reply-To: <20260216-mt8196-ufs-v7-0-b5f2907c6da7@collabora.com>

The MediaTek MT8196 SoC includes an M-PHY compatible with the already
existing mt8183 binding.

However, one omission from the original binding was that all of these
variants may have an optional reset.

Add the new compatible, and also the resets property, with an example.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../devicetree/bindings/phy/mediatek,ufs-phy.yaml        | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/mediatek,ufs-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,ufs-phy.yaml
index 6e2edd43fc2a..ee71dfa4e0c0 100644
--- a/Documentation/devicetree/bindings/phy/mediatek,ufs-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/mediatek,ufs-phy.yaml
@@ -27,6 +27,7 @@ properties:
       - items:
           - enum:
               - mediatek,mt8195-ufsphy
+              - mediatek,mt8196-ufsphy
           - const: mediatek,mt8183-ufsphy
       - const: mediatek,mt8183-ufsphy
 
@@ -43,6 +44,10 @@ properties:
       - const: unipro
       - const: mp
 
+  resets:
+    items:
+      - description: Optional UFS M-PHY reset.
+
   "#phy-cells":
     const: 0
 
@@ -66,5 +71,16 @@ examples:
         clock-names = "unipro", "mp";
         #phy-cells = <0>;
     };
+  - |
+    #include <dt-bindings/reset/mediatek,mt8196-resets.h>
+    ufs-phy@16800000 {
+        compatible = "mediatek,mt8196-ufsphy", "mediatek,mt8183-ufsphy";
+        reg = <0x16800000 0x10000>;
+        clocks = <&ufs_ao_clk 3>,
+                 <&ufs_ao_clk 5>;
+        clock-names = "unipro", "mp";
+        resets = <&ufs_ao_clk MT8196_UFSAO_RST0_UFS_MPHY>;
+        #phy-cells = <0>;
+    };
 
 ...

-- 
2.53.0


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

^ permalink raw reply related

* [PATCH v7 02/23] dt-bindings: ufs: mediatek,ufs: Complete the binding
From: Nicolas Frattaroli @ 2026-02-16 13:37 UTC (permalink / raw)
  To: Alim Akhtar, Avri Altman, Bart Van Assche, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Chunfeng Yun, Vinod Koul,
	Kishon Vijay Abraham I, Peter Wang, Stanley Jhu,
	James E.J. Bottomley, Martin K. Petersen, Philipp Zabel,
	Liam Girdwood, Mark Brown, Chaotian Jing, Neil Armstrong
  Cc: Louis-Alexis Eyraud, kernel, linux-scsi, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, linux-phy, Nicolas Frattaroli,
	Conor Dooley
In-Reply-To: <20260216-mt8196-ufs-v7-0-b5f2907c6da7@collabora.com>

As it stands, the mediatek,ufs.yaml binding is startlingly incomplete.
Its one example, which is the only real "user" of this binding in
mainline, uses the deprecated freq-table-hz property.

The resets, of which there are three optional ones, are completely
absent.

The clock description for MT8195 is incomplete, as is the one for
MT8192. It's not known if the one clock binding for MT8183 is even
correct, but I do not have access to the necessary code and
documentation to find this out myself.

The power supply situation is not much better; the binding describes one
required power supply, but it's the UFS card supply, not any of the
supplies feeding the controller silicon.

No second example is present in the binding, making verification
difficult.

Disallow freq-table-hz and move to operating-points-v2. It's fine to
break compatibility here, as the binding is currently unused and would
be impossible to correctly use in its current state.

Add the three resets and the corresponding reset-names property. These
resets appear to be optional, i.e. not required for the functioning of
the device.

Move the list of clock names out of the if condition, and expand it for
the confirmed clocks I could find by cross-referencing several clock
drivers. For MT8195, increase the minimum number of clocks to include
the crypt and rx_symbol ones, as they're internal to the SoC and should
always be present, and should therefore not be omitted.

MT8192 gets to have at least 3 clocks, as these were the ones I could
quickly confirm from a glance at various trees. I can't say this was an
exhaustive search though, but it's better than the current situation.

Properly document all supplies, with which pin name on the SoCs they
supply. Complete the example with them.

Also add a MT8195 example to the binding, using supply labels that I am
pretty sure would be the right ones for e.g. the Radxa NIO 12L.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../devicetree/bindings/ufs/mediatek,ufs.yaml      | 117 ++++++++++++++++++---
 1 file changed, 100 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
index 15c347f5e660..e0aef3e5f56b 100644
--- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
@@ -19,11 +19,28 @@ properties:
 
   clocks:
     minItems: 1
-    maxItems: 8
+    maxItems: 13
 
   clock-names:
     minItems: 1
-    maxItems: 8
+    items:
+      - const: ufs
+      - const: ufs_aes
+      - const: ufs_tick
+      - const: unipro_sysclk
+      - const: unipro_tick
+      - const: unipro_mp_bclk
+      - const: ufs_tx_symbol
+      - const: ufs_mem_sub
+      - const: crypt_mux
+      - const: crypt_lp
+      - const: crypt_perf
+      - const: ufs_rx_symbol0
+      - const: ufs_rx_symbol1
+
+  operating-points-v2: true
+
+  freq-table-hz: false
 
   phys:
     maxItems: 1
@@ -31,8 +48,36 @@ properties:
   reg:
     maxItems: 1
 
+  resets:
+    items:
+      - description: reset for the UniPro layer
+      - description: reset for the cryptography engine
+      - description: reset for the host controller
+
+  reset-names:
+    items:
+      - const: unipro
+      - const: crypto
+      - const: hci
+
+  avdd09-supply:
+    description: Phandle to the 0.9V supply powering the AVDD09_UFS pin
+
+  avdd12-supply:
+    description: Phandle to the 1.2V supply powering the AVDD12_UFS pin
+
+  avdd12-ckbuf-supply:
+    description: Phandle to the 1.2V supply powering the AVDD12_CKBUF_UFS pin
+
+  avdd18-supply:
+    description: Phandle to the 1.8V supply powering the AVDD18_UFS pin
+
   vcc-supply: true
 
+  vccq-supply: true
+
+  vccq2-supply: true
+
   mediatek,ufs-disable-mcq:
     $ref: /schemas/types.yaml#/definitions/flag
     description: The mask to disable MCQ (Multi-Circular Queue) for UFS host.
@@ -54,29 +99,41 @@ allOf:
       properties:
         compatible:
           contains:
-            enum:
-              - mediatek,mt8195-ufshci
+            const: mediatek,mt8183-ufshci
     then:
       properties:
         clocks:
-          minItems: 8
+          maxItems: 1
         clock-names:
           items:
             - const: ufs
-            - const: ufs_aes
-            - const: ufs_tick
-            - const: unipro_sysclk
-            - const: unipro_tick
-            - const: unipro_mp_bclk
-            - const: ufs_tx_symbol
-            - const: ufs_mem_sub
-    else:
+        avdd12-ckbuf-supply: false
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mediatek,mt8192-ufshci
+    then:
       properties:
         clocks:
-          maxItems: 1
+          minItems: 3
+          maxItems: 3
+        clocks-names:
+          minItems: 3
+          maxItems: 3
+        avdd09-supply: false
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mediatek,mt8195-ufshci
+    then:
+      properties:
+        clocks:
+          minItems: 13
         clock-names:
-          items:
-            - const: ufs
+          minItems: 13
+        avdd09-supply: false
 
 examples:
   - |
@@ -95,8 +152,34 @@ examples:
 
             clocks = <&infracfg_ao CLK_INFRA_UFS>;
             clock-names = "ufs";
-            freq-table-hz = <0 0>;
 
             vcc-supply = <&mt_pmic_vemc_ldo_reg>;
         };
     };
+  - |
+    ufshci@11270000 {
+        compatible = "mediatek,mt8195-ufshci";
+        reg = <0x11270000 0x2300>;
+        interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;
+        phys = <&ufsphy>;
+        clocks = <&infracfg_ao 63>, <&infracfg_ao 64>, <&infracfg_ao 65>,
+                 <&infracfg_ao 54>, <&infracfg_ao 55>,
+                 <&infracfg_ao 56>, <&infracfg_ao 90>,
+                 <&infracfg_ao 93>, <&topckgen 60>, <&topckgen 152>,
+                 <&topckgen 125>, <&topckgen 212>, <&topckgen 215>;
+        clock-names = "ufs", "ufs_aes", "ufs_tick",
+                      "unipro_sysclk", "unipro_tick",
+                      "unipro_mp_bclk", "ufs_tx_symbol",
+                      "ufs_mem_sub", "crypt_mux", "crypt_lp",
+                      "crypt_perf", "ufs_rx_symbol0", "ufs_rx_symbol1";
+
+        operating-points-v2 = <&ufs_opp_table>;
+
+        avdd12-supply = <&mt6359_vrf12_ldo_reg>;
+        avdd12-ckbuf-supply = <&mt6359_vbbck_ldo_reg>;
+        avdd18-supply = <&mt6359_vio18_ldo_reg>;
+        vcc-supply = <&mt6359_vemc_1_ldo_reg>;
+        vccq2-supply = <&mt6359_vufs_ldo_reg>;
+
+        mediatek,ufs-disable-mcq;
+    };

-- 
2.53.0


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

^ permalink raw reply related

* [PATCH v7 04/23] scsi: ufs: mediatek: Move MTK_SIP_UFS_CONTROL to mtk_sip_svc.h
From: Nicolas Frattaroli @ 2026-02-16 13:37 UTC (permalink / raw)
  To: Alim Akhtar, Avri Altman, Bart Van Assche, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Chunfeng Yun, Vinod Koul,
	Kishon Vijay Abraham I, Peter Wang, Stanley Jhu,
	James E.J. Bottomley, Martin K. Petersen, Philipp Zabel,
	Liam Girdwood, Mark Brown, Chaotian Jing, Neil Armstrong
  Cc: Louis-Alexis Eyraud, kernel, linux-scsi, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, linux-phy, Nicolas Frattaroli
In-Reply-To: <20260216-mt8196-ufs-v7-0-b5f2907c6da7@collabora.com>

SMC commands used by multiple drivers need to live in a shared header
file somewhere to avoid code duplication. In order to rework the MPHY
reset control to be in the phy-mtk-ufs.c driver, both ufs-mediatek and
the phy driver need access to this command.

Move it to mtk_sip_svc.h, where other such command definitions already
live.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/ufs/host/ufs-mediatek-sip.h      | 1 -
 include/linux/soc/mediatek/mtk_sip_svc.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/host/ufs-mediatek-sip.h b/drivers/ufs/host/ufs-mediatek-sip.h
index 7d17aedf6fb8..d627dfb4a766 100644
--- a/drivers/ufs/host/ufs-mediatek-sip.h
+++ b/drivers/ufs/host/ufs-mediatek-sip.h
@@ -11,7 +11,6 @@
 /*
  * SiP (Slicon Partner) commands
  */
-#define MTK_SIP_UFS_CONTROL               MTK_SIP_SMC_CMD(0x276)
 #define UFS_MTK_SIP_VA09_PWR_CTRL         BIT(0)
 #define UFS_MTK_SIP_DEVICE_RESET          BIT(1)
 #define UFS_MTK_SIP_CRYPTO_CTRL           BIT(2)
diff --git a/include/linux/soc/mediatek/mtk_sip_svc.h b/include/linux/soc/mediatek/mtk_sip_svc.h
index abe24a73ee19..7265ff2a6e2a 100644
--- a/include/linux/soc/mediatek/mtk_sip_svc.h
+++ b/include/linux/soc/mediatek/mtk_sip_svc.h
@@ -22,6 +22,9 @@
 	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, MTK_SIP_SMC_CONVENTION, \
 			   ARM_SMCCC_OWNER_SIP, fn_id)
 
+/* UFS related SMC call */
+#define MTK_SIP_UFS_CONTROL		MTK_SIP_SMC_CMD(0x276)
+
 /* DVFSRC SMC calls */
 #define MTK_SIP_DVFSRC_VCOREFS_CONTROL	MTK_SIP_SMC_CMD(0x506)
 

-- 
2.53.0


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

^ permalink raw reply related

* [PATCH v7 03/23] dt-bindings: ufs: mediatek,ufs: Add mt8196 variant
From: Nicolas Frattaroli @ 2026-02-16 13:37 UTC (permalink / raw)
  To: Alim Akhtar, Avri Altman, Bart Van Assche, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Chunfeng Yun, Vinod Koul,
	Kishon Vijay Abraham I, Peter Wang, Stanley Jhu,
	James E.J. Bottomley, Martin K. Petersen, Philipp Zabel,
	Liam Girdwood, Mark Brown, Chaotian Jing, Neil Armstrong
  Cc: Louis-Alexis Eyraud, kernel, linux-scsi, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, linux-phy, Nicolas Frattaroli,
	Conor Dooley
In-Reply-To: <20260216-mt8196-ufs-v7-0-b5f2907c6da7@collabora.com>

The MediaTek MT8196 SoC's UFS controller uses three additional clocks
compared to the MT8195, and a different set of supplies. It is therefore
not compatible with the MT8195.

While it does have a AVDD09_UFS_1 pin in addition to the AVDD09_UFS pin,
it appears that these two pins are commoned together, as the board
schematic I have access to uses the same supply for both, and the
downstream driver does not distinguish between the two supplies either.

Add a compatible for it, and modify the binding correspondingly.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../devicetree/bindings/ufs/mediatek,ufs.yaml      | 58 +++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
index e0aef3e5f56b..a82119ecbfe8 100644
--- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
@@ -16,10 +16,11 @@ properties:
       - mediatek,mt8183-ufshci
       - mediatek,mt8192-ufshci
       - mediatek,mt8195-ufshci
+      - mediatek,mt8196-ufshci
 
   clocks:
     minItems: 1
-    maxItems: 13
+    maxItems: 16
 
   clock-names:
     minItems: 1
@@ -37,6 +38,9 @@ properties:
       - const: crypt_perf
       - const: ufs_rx_symbol0
       - const: ufs_rx_symbol1
+      - const: ufs_sel
+      - const: ufs_sel_min_src
+      - const: ufs_sel_max_src
 
   operating-points-v2: true
 
@@ -131,9 +135,27 @@ allOf:
       properties:
         clocks:
           minItems: 13
+          maxItems: 13
         clock-names:
           minItems: 13
+          maxItems: 13
         avdd09-supply: false
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mediatek,mt8196-ufshci
+    then:
+      properties:
+        clocks:
+          minItems: 16
+          maxItems: 16
+        clock-names:
+          minItems: 16
+          maxItems: 16
+        avdd18-supply: false
+      required:
+        - operating-points-v2
 
 examples:
   - |
@@ -183,3 +205,37 @@ examples:
 
         mediatek,ufs-disable-mcq;
     };
+  - |
+    #include <dt-bindings/reset/mediatek,mt8196-resets.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    ufshci@16810000 {
+        compatible = "mediatek,mt8196-ufshci";
+        reg = <0x16810000 0x2a00>;
+        interrupts = <GIC_SPI 320 IRQ_TYPE_LEVEL_HIGH>;
+
+        clocks = <&ufs_ao_clk 6>, <&ufs_ao_clk 7>, <&clk26m>, <&ufs_ao_clk 3>,
+                 <&clk26m>, <&ufs_ao_clk 4>, <&ufs_ao_clk 0>,
+                 <&topckgen 7>, <&topckgen 41>, <&topckgen 105>, <&topckgen 83>,
+                 <&ufs_ao_clk 1>, <&ufs_ao_clk 2>, <&topckgen 42>,
+                 <&topckgen 84>, <&topckgen 102>;
+        clock-names = "ufs", "ufs_aes", "ufs_tick", "unipro_sysclk",
+                      "unipro_tick", "unipro_mp_bclk", "ufs_tx_symbol",
+                      "ufs_mem_sub", "crypt_mux", "crypt_lp", "crypt_perf",
+                      "ufs_rx_symbol0", "ufs_rx_symbol1", "ufs_sel",
+                      "ufs_sel_min_src", "ufs_sel_max_src";
+
+        operating-points-v2 = <&ufs_opp_table>;
+
+        phys = <&ufsphy>;
+
+        avdd09-supply = <&mt6363_vsram_modem>;
+        vcc-supply = <&mt6363_vemc>;
+        vccq-supply = <&mt6363_vufs12>;
+
+        resets = <&ufs_ao_clk MT8196_UFSAO_RST1_UFS_UNIPRO>,
+                 <&ufs_ao_clk MT8196_UFSAO_RST1_UFS_CRYPTO>,
+                 <&ufs_ao_clk MT8196_UFSAO_RST1_UFSHCI>;
+        reset-names = "unipro", "crypto", "hci";
+        mediatek,ufs-disable-mcq;
+    };

-- 
2.53.0


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

^ permalink raw reply related

* [PATCH v7 05/23] phy: mediatek: ufs: Add support for resets
From: Nicolas Frattaroli @ 2026-02-16 13:37 UTC (permalink / raw)
  To: Alim Akhtar, Avri Altman, Bart Van Assche, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Chunfeng Yun, Vinod Koul,
	Kishon Vijay Abraham I, Peter Wang, Stanley Jhu,
	James E.J. Bottomley, Martin K. Petersen, Philipp Zabel,
	Liam Girdwood, Mark Brown, Chaotian Jing, Neil Armstrong
  Cc: Louis-Alexis Eyraud, kernel, linux-scsi, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, linux-phy, Nicolas Frattaroli
In-Reply-To: <20260216-mt8196-ufs-v7-0-b5f2907c6da7@collabora.com>

The MediaTek UFS PHY supports PHY resets. Until now, they've been
implemented in the UFS host driver. Since they were never documented in
the UFS HCI node's DT bindings, and no mainline DT uses it, it's fine if
it's moved to the correct location, which is the PHY driver.

Implement the MPHY reset logic in this driver and expose it through the
phy subsystem's reset op. The reset itself is optional, as judging by
other mainline devices that use this hardware, it's not required for the
device to function.

If no reset is present, the reset op returns -EOPNOTSUPP, which means
that the ufshci driver can detect it's present and not double sleep in
its own reset function, where it will call the phy reset.

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-ufs.c | 71 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/drivers/phy/mediatek/phy-mtk-ufs.c b/drivers/phy/mediatek/phy-mtk-ufs.c
index 0cb5a25b1b7a..48f8e4dbf928 100644
--- a/drivers/phy/mediatek/phy-mtk-ufs.c
+++ b/drivers/phy/mediatek/phy-mtk-ufs.c
@@ -4,6 +4,7 @@
  * Author: Stanley Chu <stanley.chu@mediatek.com>
  */
 
+#include <linux/arm-smccc.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/io.h>
@@ -11,6 +12,8 @@
 #include <linux/module.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/soc/mediatek/mtk_sip_svc.h>
 
 #include "phy-mtk-io.h"
 
@@ -36,9 +39,17 @@
 
 #define UFSPHY_CLKS_CNT    2
 
+#define UFS_MTK_SIP_MPHY_CTRL       BIT(8)
+
+enum ufs_mtk_mphy_op {
+	UFS_MPHY_BACKUP = 0,
+	UFS_MPHY_RESTORE
+};
+
 struct ufs_mtk_phy {
 	struct device *dev;
 	void __iomem *mmio;
+	struct reset_control *reset;
 	struct clk_bulk_data clks[UFSPHY_CLKS_CNT];
 };
 
@@ -141,9 +152,59 @@ static int ufs_mtk_phy_power_off(struct phy *generic_phy)
 	return 0;
 }
 
+static int ufs_mtk_phy_ctrl(struct ufs_mtk_phy *phy, enum ufs_mtk_mphy_op op)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(MTK_SIP_UFS_CONTROL, UFS_MTK_SIP_MPHY_CTRL, op,
+		      0, 0, 0, 0, 0, &res);
+
+	switch (res.a0) {
+	case SMCCC_RET_NOT_SUPPORTED:
+		return -EOPNOTSUPP;
+	case SMCCC_RET_INVALID_PARAMETER:
+		return -EINVAL;
+	default:
+		return 0;
+	}
+}
+
+static int ufs_mtk_phy_reset(struct phy *generic_phy)
+{
+	struct ufs_mtk_phy *phy = get_ufs_mtk_phy(generic_phy);
+	int ret;
+
+	if (!phy->reset)
+		return -EOPNOTSUPP;
+
+	ret = reset_control_assert(phy->reset);
+	if (ret)
+		return ret;
+
+	usleep_range(100, 110);
+
+	ret = reset_control_deassert(phy->reset);
+	if (ret)
+		return ret;
+
+	/*
+	 * To avoid double-sleep and other unintended side-effects in the ufshci
+	 * driver, don't return the phy_ctrl retval here, but just return -EPROTO.
+	 */
+	ret = ufs_mtk_phy_ctrl(phy, UFS_MPHY_RESTORE);
+	if (ret) {
+		dev_err(phy->dev, "UFS_MPHY_RESTORE SMC command failed: %pe\n",
+			ERR_PTR(ret));
+		return -EPROTO;
+	}
+
+	return 0;
+}
+
 static const struct phy_ops ufs_mtk_phy_ops = {
 	.power_on       = ufs_mtk_phy_power_on,
 	.power_off      = ufs_mtk_phy_power_off,
+	.reset          = ufs_mtk_phy_reset,
 	.owner          = THIS_MODULE,
 };
 
@@ -163,8 +224,18 @@ static int ufs_mtk_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(phy->mmio))
 		return PTR_ERR(phy->mmio);
 
+	phy->reset = devm_reset_control_get_optional_exclusive(dev, NULL);
+	if (IS_ERR(phy->reset))
+		return dev_err_probe(dev, PTR_ERR(phy->reset), "Failed to get reset\n");
+
 	phy->dev = dev;
 
+	if (phy->reset) {
+		ret = ufs_mtk_phy_ctrl(phy, UFS_MPHY_BACKUP);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to back up MPHY\n");
+	}
+
 	ret = ufs_mtk_phy_clk_init(phy);
 	if (ret)
 		return ret;

-- 
2.53.0


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

^ permalink raw reply related

* [PATCH v7 06/23] scsi: ufs: mediatek: Rework resets
From: Nicolas Frattaroli @ 2026-02-16 13:37 UTC (permalink / raw)
  To: Alim Akhtar, Avri Altman, Bart Van Assche, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Chunfeng Yun, Vinod Koul,
	Kishon Vijay Abraham I, Peter Wang, Stanley Jhu,
	James E.J. Bottomley, Martin K. Petersen, Philipp Zabel,
	Liam Girdwood, Mark Brown, Chaotian Jing, Neil Armstrong
  Cc: Louis-Alexis Eyraud, kernel, linux-scsi, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, linux-phy, Nicolas Frattaroli
In-Reply-To: <20260216-mt8196-ufs-v7-0-b5f2907c6da7@collabora.com>

Rework the reset control getting in the driver's probe function to use
the bulk reset APIs. Use the optional variant instead of defaulting to
NULL if the resets fail, so that absent resets can be distinguished from
erroneous resets.

Also remove all remnants of the MPHY reset ever having lived in this
driver.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/ufs/host/ufs-mediatek-sip.h |  8 ----
 drivers/ufs/host/ufs-mediatek.c     | 78 ++++++++++++++++++-------------------
 drivers/ufs/host/ufs-mediatek.h     |  7 ++--
 3 files changed, 42 insertions(+), 51 deletions(-)

diff --git a/drivers/ufs/host/ufs-mediatek-sip.h b/drivers/ufs/host/ufs-mediatek-sip.h
index d627dfb4a766..256598cc3b5b 100644
--- a/drivers/ufs/host/ufs-mediatek-sip.h
+++ b/drivers/ufs/host/ufs-mediatek-sip.h
@@ -31,11 +31,6 @@ enum ufs_mtk_vcc_num {
 	UFS_VCC_MAX
 };
 
-enum ufs_mtk_mphy_op {
-	UFS_MPHY_BACKUP = 0,
-	UFS_MPHY_RESTORE
-};
-
 /*
  * SMC call wrapper function
  */
@@ -84,9 +79,6 @@ static inline void _ufs_mtk_smc(struct ufs_mtk_smc_arg s)
 #define ufs_mtk_device_pwr_ctrl(on, ufs_version, res) \
 	ufs_mtk_smc(UFS_MTK_SIP_DEVICE_PWR_CTRL, &(res), on, ufs_version)
 
-#define ufs_mtk_mphy_ctrl(op, res) \
-	ufs_mtk_smc(UFS_MTK_SIP_MPHY_CTRL, &(res), op)
-
 #define ufs_mtk_mtcmos_ctrl(op, res) \
 	ufs_mtk_smc(UFS_MTK_SIP_MTCMOS_CTRL, &(res), op)
 
diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index b3daaa07e925..206794ce46c8 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -93,6 +93,12 @@ static const char *const ufs_uic_dl_err_str[] = {
 	"PA_INIT"
 };
 
+static const char *const ufs_reset_names[] = {
+	"unipro",
+	"crypto",
+	"hci",
+};
+
 static bool ufs_mtk_is_boost_crypt_enabled(struct ufs_hba *hba)
 {
 	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
@@ -203,49 +209,45 @@ static void ufs_mtk_crypto_enable(struct ufs_hba *hba)
 static void ufs_mtk_host_reset(struct ufs_hba *hba)
 {
 	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
-	struct arm_smccc_res res;
-
-	reset_control_assert(host->hci_reset);
-	reset_control_assert(host->crypto_reset);
-	reset_control_assert(host->unipro_reset);
-	reset_control_assert(host->mphy_reset);
-
-	usleep_range(100, 110);
+	int ret;
 
-	reset_control_deassert(host->unipro_reset);
-	reset_control_deassert(host->crypto_reset);
-	reset_control_deassert(host->hci_reset);
-	reset_control_deassert(host->mphy_reset);
+	ret = reset_control_bulk_assert(MTK_UFS_NUM_RESETS, host->resets);
+	if (ret)
+		dev_warn(hba->dev, "Host reset assert failed: %pe\n", ERR_PTR(ret));
 
-	/* restore mphy setting aftre mphy reset */
-	if (host->mphy_reset)
-		ufs_mtk_mphy_ctrl(UFS_MPHY_RESTORE, res);
-}
+	ret = phy_reset(host->mphy);
 
-static void ufs_mtk_init_reset_control(struct ufs_hba *hba,
-				       struct reset_control **rc,
-				       char *str)
-{
-	*rc = devm_reset_control_get(hba->dev, str);
-	if (IS_ERR(*rc)) {
-		dev_info(hba->dev, "Failed to get reset control %s: %ld\n",
-			 str, PTR_ERR(*rc));
-		*rc = NULL;
+	/*
+	 * Only sleep if MPHY doesn't have a reset implemented (which already
+	 * sleeps) or the PHY reset function failed somehow, just to be safe
+	 */
+	if (ret) {
+		usleep_range(100, 110);
+		if (ret != -EOPNOTSUPP)
+			dev_warn(hba->dev, "PHY reset failed: %pe\n", ERR_PTR(ret));
 	}
+
+	ret = reset_control_bulk_deassert(MTK_UFS_NUM_RESETS, host->resets);
+	if (ret)
+		dev_warn(hba->dev, "Host reset deassert failed: %pe\n", ERR_PTR(ret));
 }
 
-static void ufs_mtk_init_reset(struct ufs_hba *hba)
+static int ufs_mtk_init_reset(struct ufs_hba *hba)
 {
 	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
+	int ret, i;
+
+	for (i = 0; i < MTK_UFS_NUM_RESETS; i++)
+		host->resets[i].id = ufs_reset_names[i];
 
-	ufs_mtk_init_reset_control(hba, &host->hci_reset,
-				   "hci_rst");
-	ufs_mtk_init_reset_control(hba, &host->unipro_reset,
-				   "unipro_rst");
-	ufs_mtk_init_reset_control(hba, &host->crypto_reset,
-				   "crypto_rst");
-	ufs_mtk_init_reset_control(hba, &host->mphy_reset,
-				   "mphy_rst");
+	ret = devm_reset_control_bulk_get_optional_exclusive(hba->dev, MTK_UFS_NUM_RESETS,
+							     host->resets);
+	if (ret) {
+		dev_err(hba->dev, "Failed to get resets: %pe\n", ERR_PTR(ret));
+		return ret;
+	}
+
+	return 0;
 }
 
 static int ufs_mtk_hce_enable_notify(struct ufs_hba *hba,
@@ -1247,11 +1249,9 @@ static int ufs_mtk_init(struct ufs_hba *hba)
 	if (err)
 		goto out_variant_clear;
 
-	ufs_mtk_init_reset(hba);
-
-	/* backup mphy setting if mphy can reset */
-	if (host->mphy_reset)
-		ufs_mtk_mphy_ctrl(UFS_MPHY_BACKUP, res);
+	err = ufs_mtk_init_reset(hba);
+	if (err)
+		goto out_variant_clear;
 
 	/* Enable runtime autosuspend */
 	hba->caps |= UFSHCD_CAP_RPM_AUTOSUSPEND;
diff --git a/drivers/ufs/host/ufs-mediatek.h b/drivers/ufs/host/ufs-mediatek.h
index 9747277f11e8..4fce29d131d1 100644
--- a/drivers/ufs/host/ufs-mediatek.h
+++ b/drivers/ufs/host/ufs-mediatek.h
@@ -7,12 +7,14 @@
 #define _UFS_MEDIATEK_H
 
 #include <linux/bitops.h>
+#include <linux/reset.h>
 
 /*
  * MCQ define and struct
  */
 #define UFSHCD_MAX_Q_NR 8
 #define MTK_MCQ_INVALID_IRQ	0xFFFF
+#define MTK_UFS_NUM_RESETS 3
 
 /* REG_UFS_MMIO_OPT_CTRL_0 160h */
 #define EHS_EN                  BIT(0)
@@ -175,10 +177,7 @@ struct ufs_mtk_mcq_intr_info {
 struct ufs_mtk_host {
 	struct phy *mphy;
 	struct regulator *reg_va09;
-	struct reset_control *hci_reset;
-	struct reset_control *unipro_reset;
-	struct reset_control *crypto_reset;
-	struct reset_control *mphy_reset;
+	struct reset_control_bulk_data resets[MTK_UFS_NUM_RESETS];
 	struct ufs_hba *hba;
 	struct ufs_mtk_crypt_cfg *crypt;
 	struct ufs_mtk_clk mclk;

-- 
2.53.0


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

^ permalink raw reply related

* [PATCH v7 07/23] scsi: ufs: mediatek: Rework 0.9V regulator
From: Nicolas Frattaroli @ 2026-02-16 13:37 UTC (permalink / raw)
  To: Alim Akhtar, Avri Altman, Bart Van Assche, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, Chunfeng Yun, Vinod Koul,
	Kishon Vijay Abraham I, Peter Wang, Stanley Jhu,
	James E.J. Bottomley, Martin K. Petersen, Philipp Zabel,
	Liam Girdwood, Mark Brown, Chaotian Jing, Neil Armstrong
  Cc: Louis-Alexis Eyraud, kernel, linux-scsi, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, linux-phy, Nicolas Frattaroli
In-Reply-To: <20260216-mt8196-ufs-v7-0-b5f2907c6da7@collabora.com>

The mediatek UFS host driver does some pretty bad stuff with regards to
the 0.9V regulator. Instead of just checking for the presence of the
regulator, it adds a cap if it's there, and then checks for the cap. It
also sleeps to stabilise the supply after enabling the regulator, which
is something that should be done by the regulator framework with the
appropriate delay properties in the DTS instead of random sleeps in the
driver code.

Rework this code and rename it to the avdd09 name I've chosen in the
binding for this supply name, instead of the downstream "va09" name that
isn't used by the datasheets for any of these chips.

Reviewed-by: Peter Wang <peter.wang@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/ufs/host/ufs-mediatek.c | 153 ++++++++++++++++++++++++++--------------
 drivers/ufs/host/ufs-mediatek.h |   3 +-
 2 files changed, 101 insertions(+), 55 deletions(-)

diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 206794ce46c8..0d22ac4c925c 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -38,6 +38,10 @@ static void _ufs_mtk_clk_scale(struct ufs_hba *hba, bool scale_up);
 #define MAX_SUPP_MAC 64
 #define MCQ_QUEUE_OFFSET(c) ((((c) >> 16) & 0xFF) * 0x200)
 
+struct ufs_mtk_soc_data {
+	bool has_avdd09;
+};
+
 static const struct ufs_dev_quirk ufs_mtk_dev_fixups[] = {
 	{ .wmanufacturerid = UFS_ANY_VENDOR,
 	  .model = UFS_ANY_MODEL,
@@ -48,13 +52,6 @@ static const struct ufs_dev_quirk ufs_mtk_dev_fixups[] = {
 	{}
 };
 
-static const struct of_device_id ufs_mtk_of_match[] = {
-	{ .compatible = "mediatek,mt8183-ufshci" },
-	{ .compatible = "mediatek,mt8195-ufshci" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, ufs_mtk_of_match);
-
 /*
  * Details of UIC Errors
  */
@@ -106,13 +103,6 @@ static bool ufs_mtk_is_boost_crypt_enabled(struct ufs_hba *hba)
 	return host->caps & UFS_MTK_CAP_BOOST_CRYPT_ENGINE;
 }
 
-static bool ufs_mtk_is_va09_supported(struct ufs_hba *hba)
-{
-	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
-
-	return host->caps & UFS_MTK_CAP_VA09_PWR_CTRL;
-}
-
 static bool ufs_mtk_is_broken_vcc(struct ufs_hba *hba)
 {
 	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
@@ -506,44 +496,70 @@ static int ufs_mtk_wait_link_state(struct ufs_hba *hba, u32 state,
 	return -ETIMEDOUT;
 }
 
+static int ufs_mtk_09v_off(struct ufs_mtk_host *host)
+{
+	struct arm_smccc_res res;
+	int ret;
+
+	if (!host->reg_avdd09)
+		return 0;
+
+	ufs_mtk_va09_pwr_ctrl(res, 0);
+	ret = regulator_disable(host->reg_avdd09);
+	if (ret) {
+		dev_err(host->hba->dev, "Failed to disable avdd09-supply: %pe\n",
+			ERR_PTR(ret));
+		ufs_mtk_va09_pwr_ctrl(res, 1);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ufs_mtk_09v_on(struct ufs_mtk_host *host)
+{
+	struct arm_smccc_res res;
+	int ret;
+
+	if (!host->reg_avdd09)
+		return 0;
+
+	ret = regulator_enable(host->reg_avdd09);
+	if (ret) {
+		dev_err(host->hba->dev, "Failed to enable avdd09-supply: %pe\n",
+			ERR_PTR(ret));
+		return ret;
+	}
+
+	ufs_mtk_va09_pwr_ctrl(res, 1);
+
+	return 0;
+}
+
 static int ufs_mtk_mphy_power_on(struct ufs_hba *hba, bool on)
 {
 	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
 	struct phy *mphy = host->mphy;
-	struct arm_smccc_res res;
-	int ret = 0;
+	int ret;
 
-	if (!mphy || !(on ^ host->mphy_powered_on))
+	if (!mphy || on == host->mphy_powered_on)
 		return 0;
 
 	if (on) {
-		if (ufs_mtk_is_va09_supported(hba)) {
-			ret = regulator_enable(host->reg_va09);
-			if (ret < 0)
-				goto out;
-			/* wait 200 us to stablize VA09 */
-			usleep_range(200, 210);
-			ufs_mtk_va09_pwr_ctrl(res, 1);
-		}
+		ret = ufs_mtk_09v_on(host);
+		if (ret)
+			return ret;
 		phy_power_on(mphy);
 	} else {
 		phy_power_off(mphy);
-		if (ufs_mtk_is_va09_supported(hba)) {
-			ufs_mtk_va09_pwr_ctrl(res, 0);
-			ret = regulator_disable(host->reg_va09);
-		}
-	}
-out:
-	if (ret) {
-		dev_info(hba->dev,
-			 "failed to %s va09: %d\n",
-			 on ? "enable" : "disable",
-			 ret);
-	} else {
-		host->mphy_powered_on = on;
+		ret = ufs_mtk_09v_off(host);
+		if (ret)
+			return ret;
 	}
 
-	return ret;
+	host->mphy_powered_on = on;
+
+	return 0;
 }
 
 static int ufs_mtk_get_host_clk(struct device *dev, const char *name,
@@ -678,17 +694,6 @@ static void ufs_mtk_init_boost_crypt(struct ufs_hba *hba)
 	return;
 }
 
-static void ufs_mtk_init_va09_pwr_ctrl(struct ufs_hba *hba)
-{
-	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
-
-	host->reg_va09 = regulator_get(hba->dev, "va09");
-	if (IS_ERR(host->reg_va09))
-		dev_info(hba->dev, "failed to get va09");
-	else
-		host->caps |= UFS_MTK_CAP_VA09_PWR_CTRL;
-}
-
 static void ufs_mtk_init_host_caps(struct ufs_hba *hba)
 {
 	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
@@ -697,9 +702,6 @@ static void ufs_mtk_init_host_caps(struct ufs_hba *hba)
 	if (of_property_read_bool(np, "mediatek,ufs-boost-crypt"))
 		ufs_mtk_init_boost_crypt(hba);
 
-	if (of_property_read_bool(np, "mediatek,ufs-support-va09"))
-		ufs_mtk_init_va09_pwr_ctrl(hba);
-
 	if (of_property_read_bool(np, "mediatek,ufs-disable-ah8"))
 		host->caps |= UFS_MTK_CAP_DISABLE_AH8;
 
@@ -1205,6 +1207,35 @@ static void ufs_mtk_init_mcq_irq(struct ufs_hba *hba)
 	host->mcq_nr_intr = 0;
 }
 
+/**
+ * ufs_mtk_get_supplies - acquire variant-specific supplies
+ * @host: pointer to driver's private &struct ufs_mtk_host instance
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+static int ufs_mtk_get_supplies(struct ufs_mtk_host *host)
+{
+	struct device *dev = host->hba->dev;
+	const struct ufs_mtk_soc_data *data = of_device_get_match_data(dev);
+
+	if (!data || !data->has_avdd09)
+		return 0;
+
+	host->reg_avdd09 = devm_regulator_get_optional(dev, "avdd09");
+	if (IS_ERR(host->reg_avdd09)) {
+		if (PTR_ERR(host->reg_avdd09) == -ENODEV) {
+			host->reg_avdd09 = NULL;
+			return 0;
+		}
+
+		dev_err(dev, "Failed to get avdd09 regulator: %pe\n",
+			host->reg_avdd09);
+		return PTR_ERR(host->reg_avdd09);
+	}
+
+	return 0;
+}
+
 /**
  * ufs_mtk_init - find other essential mmio bases
  * @hba: host controller instance
@@ -1288,6 +1319,10 @@ static int ufs_mtk_init(struct ufs_hba *hba)
 
 	ufs_mtk_init_clocks(hba);
 
+	err = ufs_mtk_get_supplies(host);
+	if (err)
+		goto out_variant_clear;
+
 	/*
 	 * ufshcd_vops_init() is invoked after
 	 * ufshcd_setup_clock(true) in ufshcd_hba_init() thus
@@ -2336,6 +2371,18 @@ static const struct ufs_hba_variant_ops ufs_hba_mtk_vops = {
 	.config_scsi_dev     = ufs_mtk_config_scsi_dev,
 };
 
+static const struct ufs_mtk_soc_data mt8183_data = {
+	.has_avdd09 = true,
+};
+
+static const struct of_device_id ufs_mtk_of_match[] = {
+	{ .compatible = "mediatek,mt8183-ufshci", .data = &mt8183_data },
+	{ .compatible = "mediatek,mt8192-ufshci" },
+	{ .compatible = "mediatek,mt8195-ufshci" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ufs_mtk_of_match);
+
 /**
  * ufs_mtk_probe - probe routine of the driver
  * @pdev: pointer to Platform device handle
diff --git a/drivers/ufs/host/ufs-mediatek.h b/drivers/ufs/host/ufs-mediatek.h
index 4fce29d131d1..24c8941f6b86 100644
--- a/drivers/ufs/host/ufs-mediatek.h
+++ b/drivers/ufs/host/ufs-mediatek.h
@@ -125,7 +125,6 @@ enum {
  */
 enum ufs_mtk_host_caps {
 	UFS_MTK_CAP_BOOST_CRYPT_ENGINE         = 1 << 0,
-	UFS_MTK_CAP_VA09_PWR_CTRL              = 1 << 1,
 	UFS_MTK_CAP_DISABLE_AH8                = 1 << 2,
 	UFS_MTK_CAP_BROKEN_VCC                 = 1 << 3,
 
@@ -176,7 +175,7 @@ struct ufs_mtk_mcq_intr_info {
 
 struct ufs_mtk_host {
 	struct phy *mphy;
-	struct regulator *reg_va09;
+	struct regulator *reg_avdd09;
 	struct reset_control_bulk_data resets[MTK_UFS_NUM_RESETS];
 	struct ufs_hba *hba;
 	struct ufs_mtk_crypt_cfg *crypt;

-- 
2.53.0


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

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox