Re: [PATCH v4 2/2] soc: qcom: ubwc: Get HBB from SMEM

Thierry Reding <[email protected]> Wed, 5 Aug 2026 13:34:17 +0200
Newsgroups	dev.linux.lists.sashiko-reviews,org.freedesktop.lists.dri-devel
Message-ID	<anMeD_frp7Md3M4g@orome>
On Tue, Jul 21, 2026 at 12:40:05PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] A race condition between SMEM probing and `qcom_ubwc_config_get_data()` can cause the driver to bypass probe deferral, incorrectly fall back to default UBWC data, and return mismatched configurations to concurrent callers.
> - [Low] Missing Kconfig dependency on QCOM_SMEM causes linker errors when QCOM_UBWC_CONFIG is selected but QCOM_SMEM is disabled or built as a module.
> --
> 
> --- Patch [2]: [PATCH v4 2/2] soc: qcom: ubwc: Get HBB from SMEM ---
> commit e0bf0c57e180329e2b45423901bc2dc1b3492f77
> Author: Konrad Dybcio <[email protected]>
> 
> soc: qcom: ubwc: Get HBB from SMEM
> 
> This commit updates the UBWC configuration driver to dynamically retrieve
> the Highest Bank Bit (HBB) from SMEM. If SMEM is not yet available, it
> returns -EPROBE_DEFER. Once available, it allocates and updates a copy of
> the UBWC configuration data with the retrieved HBB value.
> 
> > diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
> > index 0209a02c390f0..23901a4c51f73 100644
> > --- a/drivers/soc/qcom/ubwc_config.c
> > +++ b/drivers/soc/qcom/ubwc_config.c
> [ ... ]
> > @@ -168,9 +169,25 @@ static const struct of_device_id qcom_ubwc_configs[] __maybe_unused = {
> >  	{ }
> >  };
> >  
> > +static struct qcom_ubwc_cfg_data *cfg;
> > +static DEFINE_MUTEX(cfg_mutex);
> > +/**
> > + * qcom_ubwc_config_get_data() - Retrieve UBWC data for the platform.
> > + *
> > + * Return: Pointer to valid struct qcom_ubwc_cfg_data on success, negative
> > + * errno on failure. Note that this may return EPROBE_DEFER.
> > + */
> >  const struct qcom_ubwc_cfg_data *qcom_ubwc_config_get_data(void)
> >  {
> >  	const struct qcom_ubwc_cfg_data *data;
> > +	int hbb;
> > +
> > +	guard(mutex)(&cfg_mutex);
> > +	if (cfg)
> > +		return cfg;
> > +
> > +	if (!qcom_smem_is_available())
> > +		return ERR_PTR(-EPROBE_DEFER);
> 
> [Severity: Low]
> Will this cause linker errors if the kernel is configured with
> CONFIG_QCOM_UBWC_CONFIG=y but CONFIG_QCOM_SMEM=m or CONFIG_QCOM_SMEM=n?
> 
> Since these SMEM functions are called unconditionally without stub
> implementations provided for when SMEM is disabled, it seems dependent drivers
> selecting QCOM_UBWC_CONFIG might trigger undefined reference errors during
> the build.

I've been seeing exactly these linker errors now since next-20270731.
This is the DRM MSM driver selecting QCOM_UBWC_CONFIG without anything
else selecting QCOM_SMEM. This should be addressed somehow. Locally I
did select QCOM_SMEM from QCOM_UBWC_CONFIG but that's a bit risky since
QCOM_SMEM is user-visible and shouldn't be selected. A depends doesn't
work here because it gives an unmet direct dependency (nothing selects
QCOM_SMEM while QCOM_UBWC_CONFIG is selected by DRM_MSM).

Thierry
signature.asc (application/pgp-signature, 833 B)