mbox series

[v2,00/12] Add OTG mode support to Tegra USB PHY, SMB347 and Nexus 7

Message ID 20210701234317.26393-1-digetx@gmail.com
Headers show
Series Add OTG mode support to Tegra USB PHY, SMB347 and Nexus 7 | expand

Message

Dmitry Osipenko July 1, 2021, 11:43 p.m. UTC
Hi,

This series adds USB OTG mode support to the NVIDIA Tegra USB PHY driver,
SMB347 charger driver and Nexus 7 tablet.

Changelog:

v2: - The PHY's interrupt is now enabled from PHY's set_wakeup() callback.
      It prevents getting a spurious interrupt during the CI driver probe
      time.

Dmitry Osipenko (12):
  dt-bindings: phy: tegra20-usb-phy: Convert to schema
  dt-bindings: phy: tegra20-usb-phy: Document properties needed for OTG
    mode
  soc/tegra: pmc: Expose USB regmap to all SoCs
  usb: phy: tegra: Support OTG mode programming
  usb: otg-fsm: Fix hrtimer list corruption
  dt-bindings: power: supply: smb347-charger: Document USB VBUS
    regulator
  power: supply: smb347-charger: Make smb347_set_writable() IRQ-safe
  power: supply: smb347-charger: Remove caching of charger state
  power: supply: smb347-charger: Implement USB VBUS regulator
  arm64: tegra132: Add new properties to USB PHY device-tree node
  ARM: tegra: Add new properties to USB PHY device-tree nodes
  ARM: tegra: nexus7: Enable USB OTG mode

 .../bindings/phy/nvidia,tegra20-usb-phy.txt   |  74 ----
 .../bindings/phy/nvidia,tegra20-usb-phy.yaml  | 377 ++++++++++++++++++
 .../power/supply/summit,smb347-charger.yaml   |  28 ++
 arch/arm/boot/dts/tegra114.dtsi               |   6 +
 arch/arm/boot/dts/tegra124.dtsi               |   9 +
 arch/arm/boot/dts/tegra20.dtsi                |   9 +
 .../tegra30-asus-nexus7-grouper-common.dtsi   |  25 +-
 arch/arm/boot/dts/tegra30.dtsi                |   9 +
 arch/arm64/boot/dts/nvidia/tegra132.dtsi      |   9 +
 drivers/power/supply/Kconfig                  |   1 +
 drivers/power/supply/smb347-charger.c         | 259 +++++++++++-
 drivers/soc/tegra/pmc.c                       |   6 +-
 drivers/usb/common/usb-otg-fsm.c              |   6 +-
 drivers/usb/phy/phy-tegra-usb.c               | 202 +++++++++-
 .../dt-bindings/power/summit,smb347-charger.h |   4 +
 include/linux/usb/otg-fsm.h                   |   1 +
 include/linux/usb/tegra_usb_phy.h             |   5 +
 17 files changed, 926 insertions(+), 104 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt
 create mode 100644 Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.yaml

Comments

Peter Chen July 3, 2021, 11:08 a.m. UTC | #1
On 21-07-02 02:43:10, Dmitry Osipenko wrote:
> The HNP work can be re-scheduled while it's still in-fly. This results in
> re-initialization of the busy work, resetting the hrtimer's list node of
> the work and crashing kernel with null dereference within kernel/timer
> once work's timer is expired. It's very easy to trigger this problem by
> re-plugging USB cable quickly. Initialize HNP work only once to fix this
> trouble.

Fully OTG compliance support has not maintained for years, what's the use case you
still want to use?

Peter
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/usb/common/usb-otg-fsm.c | 6 +++++-
>  include/linux/usb/otg-fsm.h      | 1 +
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
> index 3740cf95560e..0697fde51d00 100644
> --- a/drivers/usb/common/usb-otg-fsm.c
> +++ b/drivers/usb/common/usb-otg-fsm.c
> @@ -193,7 +193,11 @@ static void otg_start_hnp_polling(struct otg_fsm *fsm)
>  	if (!fsm->host_req_flag)
>  		return;
>  
> -	INIT_DELAYED_WORK(&fsm->hnp_polling_work, otg_hnp_polling_work);
> +	if (!fsm->hnp_work_inited) {
> +		INIT_DELAYED_WORK(&fsm->hnp_polling_work, otg_hnp_polling_work);
> +		fsm->hnp_work_inited = true;
> +	}
> +
>  	schedule_delayed_work(&fsm->hnp_polling_work,
>  					msecs_to_jiffies(T_HOST_REQ_POLL));
>  }
> diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
> index 3aee78dda16d..784659d4dc99 100644
> --- a/include/linux/usb/otg-fsm.h
> +++ b/include/linux/usb/otg-fsm.h
> @@ -196,6 +196,7 @@ struct otg_fsm {
>  	struct mutex lock;
>  	u8 *host_req_flag;
>  	struct delayed_work hnp_polling_work;
> +	bool hnp_work_inited;
>  	bool state_changed;
>  };
>  
> -- 
> 2.30.2
>
Dmitry Osipenko July 3, 2021, 5:22 p.m. UTC | #2
03.07.2021 14:08, Peter Chen пишет:
> On 21-07-02 02:43:10, Dmitry Osipenko wrote:
>> The HNP work can be re-scheduled while it's still in-fly. This results in
>> re-initialization of the busy work, resetting the hrtimer's list node of
>> the work and crashing kernel with null dereference within kernel/timer
>> once work's timer is expired. It's very easy to trigger this problem by
>> re-plugging USB cable quickly. Initialize HNP work only once to fix this
>> trouble.
> 
> Fully OTG compliance support has not maintained for years, what's the use case you
> still want to use?

I don't have any use case for it, but I had CONFIG_USB_OTG_FSM=y and it
was crashing kernel badly. The OTG works perfectly fine without the FSM.
Peter Chen July 5, 2021, 2:21 a.m. UTC | #3
On 21-07-03 20:22:38, Dmitry Osipenko wrote:
> 03.07.2021 14:08, Peter Chen пишет:
> > On 21-07-02 02:43:10, Dmitry Osipenko wrote:
> >> The HNP work can be re-scheduled while it's still in-fly. This results in
> >> re-initialization of the busy work, resetting the hrtimer's list node of
> >> the work and crashing kernel with null dereference within kernel/timer
> >> once work's timer is expired. It's very easy to trigger this problem by
> >> re-plugging USB cable quickly. Initialize HNP work only once to fix this
> >> trouble.
> > 
> > Fully OTG compliance support has not maintained for years, what's the use case you
> > still want to use?
> 
> I don't have any use case for it, but I had CONFIG_USB_OTG_FSM=y and it
> was crashing kernel badly. The OTG works perfectly fine without the FSM.

You could add below at your dts to disable OTG FSM:
hnp-disable
srp-disable
adp-disable

Since there are no users for OTG FSM, it hasn't maintained for years,
I am not sure if it still works OK. If I remember correctly, the VBUS
will be off if you enable HNP, and the device at the host port will be
disconnected, that's may not your expectation.
Dmitry Osipenko July 5, 2021, 2:45 a.m. UTC | #4
05.07.2021 05:21, Peter Chen пишет:
> On 21-07-03 20:22:38, Dmitry Osipenko wrote:
>> 03.07.2021 14:08, Peter Chen пишет:
>>> On 21-07-02 02:43:10, Dmitry Osipenko wrote:
>>>> The HNP work can be re-scheduled while it's still in-fly. This results in
>>>> re-initialization of the busy work, resetting the hrtimer's list node of
>>>> the work and crashing kernel with null dereference within kernel/timer
>>>> once work's timer is expired. It's very easy to trigger this problem by
>>>> re-plugging USB cable quickly. Initialize HNP work only once to fix this
>>>> trouble.
>>>
>>> Fully OTG compliance support has not maintained for years, what's the use case you
>>> still want to use?
>>
>> I don't have any use case for it, but I had CONFIG_USB_OTG_FSM=y and it
>> was crashing kernel badly. The OTG works perfectly fine without the FSM.
> 
> You could add below at your dts to disable OTG FSM:
> hnp-disable
> srp-disable
> adp-disable
> 
> Since there are no users for OTG FSM, it hasn't maintained for years,
> I am not sure if it still works OK. If I remember correctly, the VBUS
> will be off if you enable HNP, and the device at the host port will be
> disconnected, that's may not your expectation.
> 

Since OTG FSM is known to be in a bad shape, could you please make a
patch to remove it? I hope it's not enabled by default in a distro
kernels.. oh no, CONFIG_USB_OTG_FSM=y at least in ArchLinux [1].

[1] https://archlinuxarm.org/packages/armv7h/linux-armv7/files/config

I think we should fix that hrtimer bug, beackport the fix into stable
kernels and then remove OTG FSM. Does this sound good to you?