power: sequencing: qcom-wcn: explain why we need the WLAN_EN GPIO hack
PATCH 1/1 power: sequencing: qcom-wcn: explain why we need the WLAN_EN GPIO hack
From: Bartosz Golaszewski <[email protected]>
With the recent rework of the PCI power control code, the workaround for the wlan-enable GPIO - where we don't set a default (low) state in the power sequencing driver, but instead request the pin as-is - should no longer be needed but some platforms still fail to probe the WLAN controller. This is caused by the Qcom PCIe controller and needs a workaround in the controller driver so add a FIXME to eventually remove the hack from this driver once this is done.
--- drivers/power/sequencing/pwrseq-qcom-wcn.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/power/sequencing/pwrseq-qcom-wcn.c +8 -0
@@ -396,6 +396,14 @@ static int pwrseq_qcom_wcn_probe(struct platform_device *pdev)return dev_err_probe (dev,PTR_ERR (ctx->bt_gpio),"Failed to get the Bluetooth enable GPIO\n" ); +/* +* FIXME: This should actually be GPIOD_OUT_LOW. The driver model can +* correctly handle provider <-> consumer dependencies but there is a +* known issue with Qcom PCIe controllers where, if the device is +* powered off abrubtly (without controller driver noticing), the PCIe +* link moves to link down state. Until the link-down handling is +* addressed in the controller driver, we need to keep this workaround. +*/ ctx->wlan_gpio =devm_gpiod_get_optional (dev,"wlan-enable" ,GPIOD_ASIS );if (IS_ERR (ctx->wlan_gpio))
Signature · 1 line
2.45.2
Quoted 20 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
With the recent rework of the PCI power control code, the workaround for the wlan-enable GPIO - where we don't set a default (low) state in the power sequencing driver, but instead request the pin as-is - should no longer be needed but some platforms still fail to probe the WLAN controller. This is caused by the Qcom PCIe controller and needs a workaround in the controller driver so add a FIXME to eventually remove the hack from this driver once this is done.
Signed-off-by Bartosz Golaszewski <[email protected]>--- drivers/power/sequencing/pwrseq-qcom-wcn.c | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/power/sequencing/pwrseq-qcom-wcn.c +0
-0
drivers/power/sequencing/pwrseq-qcom-wcn.c static int pwrseq_qcom_wcn_probe(struct platform_device *pdev) · line 405
Quoted 10 lines
drivers/power/sequencing/pwrseq-qcom-wcn.c +7
-0@@ -396,6 +396,14 @@ static int pwrseq_qcom_wcn_probe(struct platform_device *pdev)return dev_err_probe (dev,PTR_ERR (ctx->bt_gpio),"Failed to get the Bluetooth enable GPIO\n" ); +/* +* FIXME: This should actually be GPIOD_OUT_LOW. The driver model can +* correctly handle provider <-> consumer dependencies but there is a +* known issue with Qcom PCIe controllers where, if the device is +* powered off abrubtly (without controller driver noticing), the PCIe +* link moves to link down state. Until the link-down handling is +* addressed in the controller driver, we need to keep this workaround.
Maybe we should add some info on how GPIOD_OUT_LOW causes link down. Like,
/*
* FIXME: This should actually be GPIOD_OUT_LOW. But doing so would
* cause the WLAN power to be toggled, resulting in PCIe link down.
* Since the PCIe controller driver is not handling link down currently,
* the device becomes unusable. So we need to keep this workaround until
* the link down handling is implemented in the controller driver. */
But the comment applies to gpiod_direction_output() call as well, right?
- Mani
Signature · 1 line
மணிவண்ணன் சதாசிவம்
Quoted 25 lines · Manivannan Sadhasivam
Quoted 23 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
With the recent rework of the PCI power control code, the workaround for the wlan-enable GPIO - where we don't set a default (low) state in the power sequencing driver, but instead request the pin as-is - should no longer be needed but some platforms still fail to probe the WLAN controller. This is caused by the Qcom PCIe controller and needs a workaround in the controller driver so add a FIXME to eventually remove the hack from this driver once this is done.
Signed-off-by Bartosz Golaszewski <[email protected]>--- drivers/power/sequencing/pwrseq-qcom-wcn.c | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/power/sequencing/pwrseq-qcom-wcn.c +0
-0@@ -396,6 +396,14 @@ static int pwrseq_qcom_wcn_probe(struct platform_device *pdev)return dev_err_probe (dev,PTR_ERR (ctx->bt_gpio),"Failed to get the Bluetooth enable GPIO\n" );
Quoted 10 lines
drivers/power/sequencing/pwrseq-qcom-wcn.c static int pwrseq_qcom_wcn_probe(struct platform_device *pdev) · line 405
Quoted 7 lines
drivers/power/sequencing/pwrseq-qcom-wcn.c +7
-0+/* +* FIXME: This should actually be GPIOD_OUT_LOW. The driver model can +* correctly handle provider <-> consumer dependencies but there is a +* known issue with Qcom PCIe controllers where, if the device is +* powered off abrubtly (without controller driver noticing), the PCIe +* link moves to link down state. Until the link-down handling is +* addressed in the controller driver, we need to keep this workaround. Maybe we should add some info on how
GPIOD_OUT_LOWcauses link down. Like,
Sure.
Quoted 10 lines
/*
* FIXME: This should actually beGPIOD_OUT_LOW. But doing so would
* cause the WLAN power to be toggled, resulting in PCIe link down.
* Since the PCIe controller driver is not handling link down currently,
* the device becomes unusable. So we need to keep this workaround until
* the link down handling is implemented in the controller driver. */But the comment applies to
gpiod_direction_output()call as well, right?
Yes, but there is already a comment above it.
Bartosz