On Tue, Sep 29, 2026 at 03:15:10PM +0530, Kartik Rajput wrote: > Add a driver for the UFS host controller found on NVIDIA Tegra264 SoCs. > The controller has Tegra-specific auxiliary registers, clocks and resets, > and it drives the four M-PHY lane directions exposed by the Tegra264 > M-PHY driver. > > Co-developed-by: Thierry Reding > Signed-off-by: Thierry Reding > Signed-off-by: Kartik Rajput > --- > drivers/ufs/host/Kconfig | 14 + > drivers/ufs/host/Makefile | 1 + > drivers/ufs/host/ufs-tegra.c | 686 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 701 insertions(+) > > diff --git a/drivers/ufs/host/Kconfig b/drivers/ufs/host/Kconfig > index ff170c0b6da0..7ca317d25251 100644 > --- a/drivers/ufs/host/Kconfig > +++ b/drivers/ufs/host/Kconfig > @@ -168,3 +168,17 @@ config SCSI_UFS_AMD_VERSAL2 > > Select this if you have UFS controller on AMD Versal Gen 2 SoC. > If unsure, say N. > + > +config SCSI_UFS_TEGRA > + tristate "NVIDIA Tegra264 UFS controller platform driver" > + depends on SCSI_UFSHCD_PLATFORM > + depends on ARCH_TEGRA_264_SOC || (COMPILE_TEST && 64BIT) From a quick look I couldn't spot anything specific to 64-bit in the driver. Did I miss anything? Also, we really shouldn't depend on ARCH_TEGRA_264_SOC since this controller exists on prior generations and we will eventually want to support them, too. > + select PHY_TEGRA_MPHY Maybe to complement what Krzysztof already said: Kconfig dependencies are primarily build-time dependencies. This driver uses the generic PHY API to access the M-PHY functionality, so GENERIC_PHY is the correct build-time dependency. The driver is purposefully agnostic to the specific implementation of the PHYs that it uses so that it can work with (potentially) many different PHY implementations. We use device tree to hook up the runtime dependencies. If the M-PHY driver is not enabled we get the probe deferred because the PHYs that were hooked up in DT haven't been registered and hence can't be found (by the generic PHY framework). On other devices the PHYs might end up being backed by completely different implementations and they could still work just fine. > + help > + Enable support for the UFS host controller on NVIDIA Tegra264 > + SoCs. The driver relies on the Tegra264 M-PHY driver I would suggest dropping references to Tegra264 here. The driver is not specific to Tegra264. There are older Tegra SoCs that have (earlier) generations of this IP and that should still work with this driver (maybe with some parameterization). > diff --git a/drivers/ufs/host/ufs-tegra.c b/drivers/ufs/host/ufs-tegra.c > new file mode 100644 > index 000000000000..f12265432592 > --- /dev/null > +++ b/drivers/ufs/host/ufs-tegra.c > @@ -0,0 +1,686 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. > +// NVIDIA Tegra264 UFS host controller driver. Tegra264 can be dropped here as well. [...] > +#define UFS_TEGRA_HS_CLK_RATE_HZ 5840000000UL Do we assume that this will always be the same value? Maybe this should be turned into SoC data? We could always do that when necessary, of course. [...] > +static int ufs_tegra_mphy_power_on(struct ufs_hba *hba) > +{ > + struct ufs_tegra *ufs = ufshcd_get_variant(hba); > + int err; > + > + err = phy_power_on(ufs->mphy_l0_rx); > + if (err) { > + dev_err(hba->dev, "failed to power on mphy-l0-rx\n"); > + return err; > + } > + > + err = phy_power_on(ufs->mphy_l0_tx); > + if (err) { > + dev_err(hba->dev, "failed to power on mphy-l0-tx\n"); > + goto out_power_off_l0_rx; > + } > + > + err = phy_power_on(ufs->mphy_l1_rx); > + if (err) { > + dev_err(hba->dev, "failed to power on mphy-l1-rx\n"); > + goto out_power_off_l0_tx; > + } > + > + err = phy_power_on(ufs->mphy_l1_tx); > + if (err) { > + dev_err(hba->dev, "failed to power on mphy-l1-tx\n"); > + goto out_power_off_l1_rx; > + } > + > + return 0; > + > +out_power_off_l1_rx: > + phy_power_off(ufs->mphy_l1_rx); > +out_power_off_l0_tx: > + phy_power_off(ufs->mphy_l0_tx); > +out_power_off_l0_rx: > + phy_power_off(ufs->mphy_l0_rx); > + > + return err; > +} Almost seems like we need phy_bulk APIs. Again, doesn't need to be part of this series, but maybe something to keep in mind. [...] > +static int ufs_tegra_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op, > + enum ufs_notify_change_status status) > +{ > + if (status == PRE_CHANGE) > + return 0; > + > + /* Runtime H8 park keeps the M-PHY powered; only tear it down > + * on the full-teardown paths (system suspend, shutdown, or > + * runtime PM with LINK_OFF). > + */ Comment style needs the first line to be /* on its own, except for the net subsystem, if I recall correctly. There are other occurrences of this elsewhere in the file. Also, I read "H8" as "hate" at first and it took me a little bit to realize it meant "hibernate" in this case. Looks like the rest of UFS uses "hibern8". Maybe stick to the convention? [...] > +static int ufs_tegra_pwr_change_notify(struct ufs_hba *hba, > + enum ufs_notify_change_status status, > + struct ufs_pa_layer_attr *dev_req_params) > +{ [...] > + if (dev_req_params->hs_rate == PA_HS_MODE_A || > + dev_req_params->hs_rate == PA_HS_MODE_B) { > + err = clk_set_rate(ufs->hs_clk, UFS_TEGRA_HS_CLK_RATE_HZ); What if hs_rate != PA_HS_MODE_A or PA_HS_MODE_B? > +static int ufs_tegra_set_dma_mask(struct ufs_hba *hba) > +{ > + return dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(32)); > +} I have a feeling like the 32 is here on purpose, but I can't fully remember why it's not something like 40, which is the standard on recent Tegra devices. Maybe we should add a comment explaining why this needs to be so narrow. Thierry