diff mbox

phy: allwinner: sun4i-usb: poll vbus changes on A23/A33 when driving VBUS

Message ID 20180119092541.7508-1-wens@csie.org (mailing list archive)
State Mainlined, archived
Headers show

Commit Message

Chen-Yu Tsai Jan. 19, 2018, 9:25 a.m. UTC
The AXP223 PMIC, like the AXP221, does not generate VBUS change
interrupts when N_VBUSEN is used to drive VBUS for the OTG port
on the board.

This was not noticed until recently, as most A23/A33 boards use
a GPIO pin that does not support interrupts for OTG ID detection.
This forces the driver to use polling. However the A33-OlinuXino
uses a pin that does support interrupts, so the driver uses them.
However the VBUS interrupt never fires, and the driver never gets
to update the VBUS status. This results in musb timing out waiting
for VBUS to rise.

This was worked around for the AXP221 by resorting to polling
changes in commit 91d96f06a760 ("phy-sun4i-usb: Add workaround for
missing Vbus det interrupts on A31"). This patch adds the A23 and
A33 to the list of SoCs that need the workaround.

Fixes: fc1f45ed3043 ("phy-sun4i-usb: Add support for the usb-phys on the
		      sun8i-a33 SoC")
Fixes: 123dfdbcfaf5 ("phy-sun4i-usb: Add support for the usb-phys on the
		      sun8i-a23 SoC")
Cc: <stable@vger.kernel.org> # 4.3.x: 68dbc2ce77bb phy-sun4i-usb:
		Use of_match_node to get model specific config data
Cc: <stable@vger.kernel.org> # 4.3.x: 5cf700ac9d50 phy: phy-sun4i-usb:
		Fix optional gpios failing probe
Cc: <stable@vger.kernel.org> # 4.3.x: 04e59a0211ff phy-sun4i-usb:
		Fix irq free conditions to match request conditions
Cc: <stable@vger.kernel.org> # 4.3.x: 91d96f06a760 phy-sun4i-usb:
		Add workaround for missing Vbus det interrupts on A31
Cc: <stable@vger.kernel.org> # 4.3.x
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---

This list might get longer if the newer AXP8xx PMICs also have this
behavior. We could switch to a boolean in the per-compatible data,
or just always use polling. The yet-merged R40 support will have this
issue as well, as the R40 is paired with the AXP221. But for now,
I'd like to have something that is easier to backport to stable, so
we can at least fix this for the A23 and A33.

Also, checkpatch.pl doesn't like the stable kernel prerequisite lines.

---
 drivers/phy/allwinner/phy-sun4i-usb.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Maxime Ripard Jan. 19, 2018, 2:45 p.m. UTC | #1
On Fri, Jan 19, 2018 at 05:25:41PM +0800, Chen-Yu Tsai wrote:
> The AXP223 PMIC, like the AXP221, does not generate VBUS change
> interrupts when N_VBUSEN is used to drive VBUS for the OTG port
> on the board.
> 
> This was not noticed until recently, as most A23/A33 boards use
> a GPIO pin that does not support interrupts for OTG ID detection.
> This forces the driver to use polling. However the A33-OlinuXino
> uses a pin that does support interrupts, so the driver uses them.
> However the VBUS interrupt never fires, and the driver never gets
> to update the VBUS status. This results in musb timing out waiting
> for VBUS to rise.
> 
> This was worked around for the AXP221 by resorting to polling
> changes in commit 91d96f06a760 ("phy-sun4i-usb: Add workaround for
> missing Vbus det interrupts on A31"). This patch adds the A23 and
> A33 to the list of SoCs that need the workaround.
> 
> Fixes: fc1f45ed3043 ("phy-sun4i-usb: Add support for the usb-phys on the
> 		      sun8i-a33 SoC")
> Fixes: 123dfdbcfaf5 ("phy-sun4i-usb: Add support for the usb-phys on the
> 		      sun8i-a23 SoC")
> Cc: <stable@vger.kernel.org> # 4.3.x: 68dbc2ce77bb phy-sun4i-usb:
> 		Use of_match_node to get model specific config data
> Cc: <stable@vger.kernel.org> # 4.3.x: 5cf700ac9d50 phy: phy-sun4i-usb:
> 		Fix optional gpios failing probe
> Cc: <stable@vger.kernel.org> # 4.3.x: 04e59a0211ff phy-sun4i-usb:
> 		Fix irq free conditions to match request conditions
> Cc: <stable@vger.kernel.org> # 4.3.x: 91d96f06a760 phy-sun4i-usb:
> 		Add workaround for missing Vbus det interrupts on A31
> Cc: <stable@vger.kernel.org> # 4.3.x
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> 
> This list might get longer if the newer AXP8xx PMICs also have this
> behavior. We could switch to a boolean in the per-compatible data,
> or just always use polling. The yet-merged R40 support will have this
> issue as well, as the R40 is paired with the AXP221. But for now,
> I'd like to have something that is easier to backport to stable, so
> we can at least fix this for the A23 and A33.
> 
> Also, checkpatch.pl doesn't like the stable kernel prerequisite lines.

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Maxime
Kishon Vijay Abraham I Feb. 16, 2018, 11:20 a.m. UTC | #2
On Friday 19 January 2018 08:15 PM, Maxime Ripard wrote:
> On Fri, Jan 19, 2018 at 05:25:41PM +0800, Chen-Yu Tsai wrote:
>> The AXP223 PMIC, like the AXP221, does not generate VBUS change
>> interrupts when N_VBUSEN is used to drive VBUS for the OTG port
>> on the board.
>>
>> This was not noticed until recently, as most A23/A33 boards use
>> a GPIO pin that does not support interrupts for OTG ID detection.
>> This forces the driver to use polling. However the A33-OlinuXino
>> uses a pin that does support interrupts, so the driver uses them.
>> However the VBUS interrupt never fires, and the driver never gets
>> to update the VBUS status. This results in musb timing out waiting
>> for VBUS to rise.
>>
>> This was worked around for the AXP221 by resorting to polling
>> changes in commit 91d96f06a760 ("phy-sun4i-usb: Add workaround for
>> missing Vbus det interrupts on A31"). This patch adds the A23 and
>> A33 to the list of SoCs that need the workaround.
>>
>> Fixes: fc1f45ed3043 ("phy-sun4i-usb: Add support for the usb-phys on the
>> 		      sun8i-a33 SoC")
>> Fixes: 123dfdbcfaf5 ("phy-sun4i-usb: Add support for the usb-phys on the
>> 		      sun8i-a23 SoC")
>> Cc: <stable@vger.kernel.org> # 4.3.x: 68dbc2ce77bb phy-sun4i-usb:
>> 		Use of_match_node to get model specific config data
>> Cc: <stable@vger.kernel.org> # 4.3.x: 5cf700ac9d50 phy: phy-sun4i-usb:
>> 		Fix optional gpios failing probe
>> Cc: <stable@vger.kernel.org> # 4.3.x: 04e59a0211ff phy-sun4i-usb:
>> 		Fix irq free conditions to match request conditions
>> Cc: <stable@vger.kernel.org> # 4.3.x: 91d96f06a760 phy-sun4i-usb:
>> 		Add workaround for missing Vbus det interrupts on A31
>> Cc: <stable@vger.kernel.org> # 4.3.x
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>
>> This list might get longer if the newer AXP8xx PMICs also have this
>> behavior. We could switch to a boolean in the per-compatible data,
>> or just always use polling. The yet-merged R40 support will have this
>> issue as well, as the R40 is paired with the AXP221. But for now,
>> I'd like to have something that is easier to backport to stable, so
>> we can at least fix this for the A23 and A33.
>>
>> Also, checkpatch.pl doesn't like the stable kernel prerequisite lines.
> 
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

merged, thanks!

-Kishon
Chen-Yu Tsai Feb. 16, 2018, 12:52 p.m. UTC | #3
On Fri, Feb 16, 2018 at 7:20 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
>
> On Friday 19 January 2018 08:15 PM, Maxime Ripard wrote:
>> On Fri, Jan 19, 2018 at 05:25:41PM +0800, Chen-Yu Tsai wrote:
>>> The AXP223 PMIC, like the AXP221, does not generate VBUS change
>>> interrupts when N_VBUSEN is used to drive VBUS for the OTG port
>>> on the board.
>>>
>>> This was not noticed until recently, as most A23/A33 boards use
>>> a GPIO pin that does not support interrupts for OTG ID detection.
>>> This forces the driver to use polling. However the A33-OlinuXino
>>> uses a pin that does support interrupts, so the driver uses them.
>>> However the VBUS interrupt never fires, and the driver never gets
>>> to update the VBUS status. This results in musb timing out waiting
>>> for VBUS to rise.
>>>
>>> This was worked around for the AXP221 by resorting to polling
>>> changes in commit 91d96f06a760 ("phy-sun4i-usb: Add workaround for
>>> missing Vbus det interrupts on A31"). This patch adds the A23 and
>>> A33 to the list of SoCs that need the workaround.
>>>
>>> Fixes: fc1f45ed3043 ("phy-sun4i-usb: Add support for the usb-phys on the
>>>                    sun8i-a33 SoC")
>>> Fixes: 123dfdbcfaf5 ("phy-sun4i-usb: Add support for the usb-phys on the
>>>                    sun8i-a23 SoC")
>>> Cc: <stable@vger.kernel.org> # 4.3.x: 68dbc2ce77bb phy-sun4i-usb:
>>>              Use of_match_node to get model specific config data
>>> Cc: <stable@vger.kernel.org> # 4.3.x: 5cf700ac9d50 phy: phy-sun4i-usb:
>>>              Fix optional gpios failing probe
>>> Cc: <stable@vger.kernel.org> # 4.3.x: 04e59a0211ff phy-sun4i-usb:
>>>              Fix irq free conditions to match request conditions
>>> Cc: <stable@vger.kernel.org> # 4.3.x: 91d96f06a760 phy-sun4i-usb:
>>>              Add workaround for missing Vbus det interrupts on A31
>>> Cc: <stable@vger.kernel.org> # 4.3.x
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>>
>>> This list might get longer if the newer AXP8xx PMICs also have this
>>> behavior. We could switch to a boolean in the per-compatible data,
>>> or just always use polling. The yet-merged R40 support will have this
>>> issue as well, as the R40 is paired with the AXP221. But for now,
>>> I'd like to have something that is easier to backport to stable, so
>>> we can at least fix this for the A23 and A33.
>>>
>>> Also, checkpatch.pl doesn't like the stable kernel prerequisite lines.
>>
>> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> merged, thanks!

Thanks.

After some more experimentation on the A83T/AXP813, it seems this
is pretty standard behavior for the AXP PMICs. If one uses N_VBUSEN
to enable VBUS, the user/system should already know that VBUS is
on or present. The VBUS-related logic in the PMIC, save for the
VBUS status bits, is disconnected in this case.

So it looks like we either enable this workaround for most if not
all of the Allwinner family, or fix this by polling in the power
supply driver itself. This would require polling once every 50ms,
which might be too much work for the system, but is necessary for
VBUS detection to work properly for MUSB. Or we could limit polling
to whenever the drivevbus regulator is enabled. This would involve
creating a reference between the USB power supply and regulator in
the device tree. (There doesn't seem to be a way to fetch a regulator
directly by its device node.) The power supply would then register
a notifier on the regulator for on/off events, and configure polling
accordingly.

Any suggestions?

ChenYu
Chen-Yu Tsai March 15, 2018, 6:17 a.m. UTC | #4
Hi Kishon,

On Fri, Feb 16, 2018 at 7:20 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
>
> On Friday 19 January 2018 08:15 PM, Maxime Ripard wrote:
>> On Fri, Jan 19, 2018 at 05:25:41PM +0800, Chen-Yu Tsai wrote:
>>> The AXP223 PMIC, like the AXP221, does not generate VBUS change
>>> interrupts when N_VBUSEN is used to drive VBUS for the OTG port
>>> on the board.
>>>
>>> This was not noticed until recently, as most A23/A33 boards use
>>> a GPIO pin that does not support interrupts for OTG ID detection.
>>> This forces the driver to use polling. However the A33-OlinuXino
>>> uses a pin that does support interrupts, so the driver uses them.
>>> However the VBUS interrupt never fires, and the driver never gets
>>> to update the VBUS status. This results in musb timing out waiting
>>> for VBUS to rise.
>>>
>>> This was worked around for the AXP221 by resorting to polling
>>> changes in commit 91d96f06a760 ("phy-sun4i-usb: Add workaround for
>>> missing Vbus det interrupts on A31"). This patch adds the A23 and
>>> A33 to the list of SoCs that need the workaround.
>>>
>>> Fixes: fc1f45ed3043 ("phy-sun4i-usb: Add support for the usb-phys on the
>>>                    sun8i-a33 SoC")
>>> Fixes: 123dfdbcfaf5 ("phy-sun4i-usb: Add support for the usb-phys on the
>>>                    sun8i-a23 SoC")
>>> Cc: <stable@vger.kernel.org> # 4.3.x: 68dbc2ce77bb phy-sun4i-usb:
>>>              Use of_match_node to get model specific config data
>>> Cc: <stable@vger.kernel.org> # 4.3.x: 5cf700ac9d50 phy: phy-sun4i-usb:
>>>              Fix optional gpios failing probe
>>> Cc: <stable@vger.kernel.org> # 4.3.x: 04e59a0211ff phy-sun4i-usb:
>>>              Fix irq free conditions to match request conditions
>>> Cc: <stable@vger.kernel.org> # 4.3.x: 91d96f06a760 phy-sun4i-usb:
>>>              Add workaround for missing Vbus det interrupts on A31
>>> Cc: <stable@vger.kernel.org> # 4.3.x
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>>
>>> This list might get longer if the newer AXP8xx PMICs also have this
>>> behavior. We could switch to a boolean in the per-compatible data,
>>> or just always use polling. The yet-merged R40 support will have this
>>> issue as well, as the R40 is paired with the AXP221. But for now,
>>> I'd like to have something that is easier to backport to stable, so
>>> we can at least fix this for the A23 and A33.
>>>
>>> Also, checkpatch.pl doesn't like the stable kernel prerequisite lines.
>>
>> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> merged, thanks!

I see you moved this patch from your fixes branch to your -next branch.
Is there any problem with the patch? Merge issues?


Thanks
ChenYu
Kishon Vijay Abraham I March 15, 2018, 6:29 a.m. UTC | #5
Hi,

On Thursday 15 March 2018 11:47 AM, Chen-Yu Tsai wrote:
> Hi Kishon,
> 
> On Fri, Feb 16, 2018 at 7:20 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>
>>
>> On Friday 19 January 2018 08:15 PM, Maxime Ripard wrote:
>>> On Fri, Jan 19, 2018 at 05:25:41PM +0800, Chen-Yu Tsai wrote:
>>>> The AXP223 PMIC, like the AXP221, does not generate VBUS change
>>>> interrupts when N_VBUSEN is used to drive VBUS for the OTG port
>>>> on the board.
>>>>
>>>> This was not noticed until recently, as most A23/A33 boards use
>>>> a GPIO pin that does not support interrupts for OTG ID detection.
>>>> This forces the driver to use polling. However the A33-OlinuXino
>>>> uses a pin that does support interrupts, so the driver uses them.
>>>> However the VBUS interrupt never fires, and the driver never gets
>>>> to update the VBUS status. This results in musb timing out waiting
>>>> for VBUS to rise.
>>>>
>>>> This was worked around for the AXP221 by resorting to polling
>>>> changes in commit 91d96f06a760 ("phy-sun4i-usb: Add workaround for
>>>> missing Vbus det interrupts on A31"). This patch adds the A23 and
>>>> A33 to the list of SoCs that need the workaround.
>>>>
>>>> Fixes: fc1f45ed3043 ("phy-sun4i-usb: Add support for the usb-phys on the
>>>>                    sun8i-a33 SoC")
>>>> Fixes: 123dfdbcfaf5 ("phy-sun4i-usb: Add support for the usb-phys on the
>>>>                    sun8i-a23 SoC")
>>>> Cc: <stable@vger.kernel.org> # 4.3.x: 68dbc2ce77bb phy-sun4i-usb:
>>>>              Use of_match_node to get model specific config data
>>>> Cc: <stable@vger.kernel.org> # 4.3.x: 5cf700ac9d50 phy: phy-sun4i-usb:
>>>>              Fix optional gpios failing probe
>>>> Cc: <stable@vger.kernel.org> # 4.3.x: 04e59a0211ff phy-sun4i-usb:
>>>>              Fix irq free conditions to match request conditions
>>>> Cc: <stable@vger.kernel.org> # 4.3.x: 91d96f06a760 phy-sun4i-usb:
>>>>              Add workaround for missing Vbus det interrupts on A31
>>>> Cc: <stable@vger.kernel.org> # 4.3.x
>>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>>> ---
>>>>
>>>> This list might get longer if the newer AXP8xx PMICs also have this
>>>> behavior. We could switch to a boolean in the per-compatible data,
>>>> or just always use polling. The yet-merged R40 support will have this
>>>> issue as well, as the R40 is paired with the AXP221. But for now,
>>>> I'd like to have something that is easier to backport to stable, so
>>>> we can at least fix this for the A23 and A33.
>>>>
>>>> Also, checkpatch.pl doesn't like the stable kernel prerequisite lines.
>>>
>>> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>
>> merged, thanks!
> 
> I see you moved this patch from your fixes branch to your -next branch.
> Is there any problem with the patch? Merge issues?

Decided to keep in -next since this is not a regression introduced in 4.16
kernel and stable kernels will anyways be updated.

Thanks
Kishon
diff mbox

Patch

diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
index aa857be692cf..d5ae307ef4e1 100644
--- a/drivers/phy/allwinner/phy-sun4i-usb.c
+++ b/drivers/phy/allwinner/phy-sun4i-usb.c
@@ -410,11 +410,13 @@  static bool sun4i_usb_phy0_poll(struct sun4i_usb_phy_data *data)
 		return true;
 
 	/*
-	 * The A31 companion pmic (axp221) does not generate vbus change
-	 * interrupts when the board is driving vbus, so we must poll
+	 * The A31/A23/A33 companion pmics (AXP221/AXP223) do not
+	 * generate vbus change interrupts when the board is driving
+	 * vbus using the N_VBUSEN pin on the pmic, so we must poll
 	 * when using the pmic for vbus-det _and_ we're driving vbus.
 	 */
-	if (data->cfg->type == sun6i_a31_phy &&
+	if ((data->cfg->type == sun6i_a31_phy ||
+	     data->cfg->type == sun8i_a33_phy) &&
 	    data->vbus_power_supply && data->phys[0].regulator_on)
 		return true;
 
@@ -885,7 +887,7 @@  static const struct sun4i_usb_phy_cfg sun7i_a20_cfg = {
 
 static const struct sun4i_usb_phy_cfg sun8i_a23_cfg = {
 	.num_phys = 2,
-	.type = sun4i_a10_phy,
+	.type = sun6i_a31_phy,
 	.disc_thresh = 3,
 	.phyctl_offset = REG_PHYCTL_A10,
 	.dedicated_clocks = true,