[PATCH 1/2] bluetooth: btnxpuart: Support for controller wakeup gpio config

[Neeraj Sanjay Kale <neeraj.sanjaykale@xxxxxxx>]

Hi Loic,

Thank you for your patch. Just a few suggestions below:
 
> @@ -616,6 +617,13 @@ static void ps_init(struct hci_dev *hdev)
>                 break;
>         }
> 
> +       if (!device_property_read_u8(&nxpdev->serdev->dev, "nxp,wakein-pin",
> +                                    &psdata->h2c_wakeup_gpio))
> +               psdata->h2c_wakeupmode = WAKEUP_METHOD_GPIO;
> +       if (!device_property_read_u8(&nxpdev->serdev->dev, "nxp,wakeout-
> pin",
> +                                    &psdata->c2h_wakeup_gpio))
> +               psdata->c2h_wakeupmode = BT_HOST_WAKEUP_METHOD_GPIO;
> +
>         psdata->cur_psmode = PS_MODE_DISABLE;
>         psdata->target_ps_mode = DEFAULT_PS_MODE;
> 
Please move device_property_read for "nxp,wakein-pin" to ps_setup(), after "device-wakeup" is read.

I think we should not set h2c_wakeupmode as WAKEUP_METHOD_GPIO based on "nxp,wakein-pin" alone.

In existing code, we are setting default_h2c_wakeup_mode to WAKEUP_METHOD_GPIO if "device-wakeup" is defined in DT, and psdata->h2c_wakeup_gpio = 0xff. WAKE_IN pin is not read.
In this case the FW considers default GPIO as WAKE_IN pin (as per datasheet), which is a valid scenario.

But this logic will fail if we specify only "nxp,wakein-pin", without "device-wakeup" in DT.
Hence, I recommend something as follows in ps_setup():
- if (!psdata->h2c_ps_gpio)
+ if (!psdata->h2c_ps_gpio || device_property_read_u8(&nxpdev->serdev->dev, "nxp,wakein-pin", &psdata->h2c_wakeup_gpio))
        psdata->h2c_wakeup_gpio = 0xff;

For "nxp,wakeout-pin", I have yet to submit patch for "host-wakeup-gpios". I can move "nxp,wakeout-pin" later if required.

Thanks,
Neeraj