Re: [PATCH v4 3/3] soc: qcom: ubwc: Use IS_REACHABLE() instead of IS_ENABLED()

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

New issues:
- [High] Using IS_REACHABLE() masks a Kconfig dependency mismatch, causing guaranteed runtime probe failures for built-in drivers when the dependency is built as a module.

Pre-existing issues:
- [Medium] Missing resource cleanup in a5xx_gpu_init() when qcom_ubwc_config_get_data() fails.
--

--- Patch [3]: [PATCH v4 3/3] soc: qcom: ubwc: Use IS_REACHABLE() instead of IS_ENABLED() ---
commit 27da698f8aa42c85fe3250533a0ac637d13515f3
Author: Daniel Baluta <[email protected]>

soc: qcom: ubwc: Use IS_REACHABLE() instead of IS_ENABLED()

This commit changes the conditional check for QCOM_UBWC_CONFIG from
IS_ENABLED to IS_REACHABLE. This aims to resolve a link error when
built-in callers try to use the function while the dependency is built
as a module.

> diff --git a/include/linux/soc/qcom/ubwc.h b/include/linux/soc/qcom/ubwc.h
> index a7372d9c25fbc..1a61238f6d4d5 100644
> --- a/include/linux/soc/qcom/ubwc.h
> +++ b/include/linux/soc/qcom/ubwc.h
[ ... ]
> @@ -36,7 +36,7 @@ struct qcom_ubwc_cfg_data {
>  #define UBWC_5_0 0x50000000
>  #define UBWC_6_0 0x60000000
>  
> -#if IS_ENABLED(CONFIG_QCOM_UBWC_CONFIG)
> +#if IS_REACHABLE(CONFIG_QCOM_UBWC_CONFIG)

[Severity: High]
Does using IS_REACHABLE() here mask a Kconfig dependency mismatch?

If a user configures DRM_MSM=y and QCOM_SMEM=m, IS_REACHABLE() evaluates
to false and provides the inline stub to the built-in driver. Since the
callers abort on error rather than treating the configuration as optional,
won't this cause guaranteed runtime probe failures for built-in drivers
instead of catching the unmet dependency at build time?

>  const struct qcom_ubwc_cfg_data *qcom_ubwc_config_get_data(void);
>  #else
>  static inline const struct qcom_ubwc_cfg_data *qcom_ubwc_config_get_data(void)

[Severity: Medium]
This is a pre-existing issue, but does falling back to this stub cause
a resource leak during device probe?

Looking at a5xx_gpu_init(), if the stub is used and returns an error pointer,
the function returns early without cleaning up resources allocated by
adreno_gpu_init():

drivers/gpu/drm/msm/adreno/a5xx_gpu.c:a5xx_gpu_init() {
    ...
    adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 1);
    ...
    adreno_gpu->ubwc_config = qcom_ubwc_config_get_data();
    if (IS_ERR(adreno_gpu->ubwc_config))
        return ERR_CAST(adreno_gpu->ubwc_config);
    ...
}

Will this leak memory and GPU resources like ringbuffers and the a5xx_gpu
structure itself if it fails to call a5xx_destroy() on the error path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3