RE: [PATCH rtw-next v2] wifi: rtw89: acpi: check for _DSM function support before evaluating it

[Ping-Ke Shih]

Rosen Penev <rosenp@xxxxxxxxx> wrote:
> When the WiFi device has no ACPI companion, or the firmware does not
> implement a given rtw89 _DSM function, rtw89_acpi_evaluate_dsm() calls
> acpi_evaluate_dsm() anyway. ACPI core then warns on every probe:
> 
>   ACPI: \: failed to evaluate _DSM e8c3a8d2-694b-004f-82bd-fe8607803aa7 rev:0 func:10 (0x1001)
>   ACPI: \: failed to evaluate _DSM e8c3a8d2-694b-004f-82bd-fe8607803aa7 rev:0 func:6 (0x1001)
> 
> Here the handle is NULL, and acpi_evaluate_object() returns
> AE_BAD_PARAMETER. All callers already treat failure as "no policy
> provided by the platform", so this is only noise.
> 
> Bail out early when there is no ACPI handle, as acpi_check_dsm() would
> otherwise hit the same warning while querying function 0. Then use
> acpi_check_dsm() so that functions the firmware does not advertise fail
> quietly at debug level, as rtw89_acpi_evaluate_method() already does.

Adding this check is fine to me. But my question in v1 is that 
acpi_check_dsm() checks func 0 to determine if BIT(func) is existing.
Is it possible func 0 not existing, but BIT(func) existing?

> 
> Seen on an RTL8852BE in a desktop PCIe slot.
> 
> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> ---
>  v2: check handle separately. Remove Fixes.
>  drivers/net/wireless/realtek/rtw89/acpi.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/acpi.c b/drivers/net/wireless/realtek/rtw89/acpi.c
> index ebb6cf555747..3396d42594b3 100644
> --- a/drivers/net/wireless/realtek/rtw89/acpi.c
> +++ b/drivers/net/wireless/realtek/rtw89/acpi.c
> @@ -372,11 +372,22 @@ int rtw89_acpi_evaluate_dsm(struct rtw89_dev *rtwdev,
>                             enum rtw89_acpi_dsm_func func,
>                             struct rtw89_acpi_dsm_result *res)
>  {
> +       acpi_handle handle = ACPI_HANDLE(rtwdev->dev);
>         union acpi_object *obj;
>         int ret;
> 
> -       obj = acpi_evaluate_dsm(ACPI_HANDLE(rtwdev->dev), &rtw89_guid,
> -                               0, func, NULL);
> +       if (!handle) {
> +               rtw89_debug(rtwdev, RTW89_DBG_ACPI, "acpi dsm no handle\n");
> +               return -ENODEV;
> +       }
> +
> +       if (!acpi_check_dsm(handle, &rtw89_guid, 0, BIT(func))) {
> +               rtw89_debug(rtwdev, RTW89_DBG_ACPI,
> +                           "acpi dsm func %d not supported\n", func);
> +               return -ENOENT;
> +       }
> +
> +       obj = acpi_evaluate_dsm(handle, &rtw89_guid, 0, func, NULL);
>         if (!obj) {
>                 rtw89_debug(rtwdev, RTW89_DBG_ACPI,
>                             "acpi dsm fail to evaluate func: %d\n", func);
> --
> 2.56.0