Re: [PATCH v10 10/11] ufs: s32n7: Add NXP S32N79 UFS host controller driver

[Bart Van Assche]

On 11/3/26 5:32 AM, Larisa Grigore wrote:
> Add support for the UFS host controller on the NXP S32N79 SoC, built on
> the Synopsys DesignWare (DWC) UFS architecture, using the UFSHCD DWC and
> UFSHCD platform glue.

The following comments posted on v9 have not yet been addressed:
* device_get_match_data() is still called in both ufs_s32n7_probe() and
  ufs_s32n7_init(). Since all S32N7 variants use the same vops callbacks
  (or vops can be embedded in struct s32n7_soc_data and recovered via
  container_of(hba->vops, ...)), one lookup suffices.
* ufs_s32n7_link_startup_post_change() still open-codes a local-only
  subset of ufshcd_dwc_connection_setup() (setting N_DEVICEID = 0
  without N_DEVICEID_VALID, and skipping peer attributes). The v10
  commit message states this "follows the Synopsys recommended
  sequence", but still does not explain why
  ufshcd_dwc_connection_setup() (used by ufs-amd-versal2.c) cannot be
  used on S32N79.

Other issues:
* Back-to-back writes to SCM_STATUS: ufs_s32n7_link_startup_pre_change()
performs two consecutive posted writel() calls to ufs->reg_scm +
SCM_STATUS (SCM_STATUS_PHY_RESET(1) immediately followed by
SCM_STATUS_PHY_RESET(1) | SCM_STATUS_SRAM_BYPASS(1)) with no readback or
delay in between. Either the first write is redundant, or if the
hardware requires observing SRAM_BYPASS = 0 while PHY_RESET = 1, a
readback/delay is missing and should be documented.
* Magic hex constants for shared DWC M-PHY bitfields:
s32n79_reset_config, s32n79_calib_mphy_disabled,
s32n79_calib_mphy_enabled, and pwr_change_phy_regs use raw hex values
(0x8A for CBREFCLKCTRL2 even though ufshcd-dwc.h already defines
CBREFREFCLK_GATE_OVR_EN, plus 0x4, 0x100, 0xc, 0x8). ufs-amd-versal2.c
already defines MPHY_FAST_RX_AFE_CAL, MPHY_FW_CALIB_CFG_VAL,
MPHY_RX_OVRD_EN, and MPHY_RX_OVRD_VAL for these same registers—moving
those bit macros to ufshcd-dwc.h would avoid magic numbers.
 Thanks,

Bart.