Re: [PATCH v3 00/12] usb: dwc3: qcom: Flatten dwc3 structure

Stephan Gerhold
2025-01-27T17:57:49Z

On Mon, Jan 13, 2025 at 09:11:33PM -0800, Bjorn Andersson wrote:
> The USB IP-block found in most Qualcomm platforms is modelled in the
> Linux kernel as 3 different independent device drivers, but as shown by
> the already existing layering violations in the Qualcomm glue driver
> they can not be operated independently.
>
> With the current implementation, the glue driver registers the core and
> has no way to know when this is done. As a result, e.g. the suspend
> callbacks needs to guard against NULL pointer dereferences when trying
> to peek into the struct dwc3 found in the drvdata of the child.
>
> Missing from the upstream Qualcomm USB support is proper handling of
> role switching, in which the glue needs to be notified upon DRD mode
> changes. Several attempts has been made through the years to register
> callbacks etc, but they always fall short when it comes to handling of
> the core's probe deferral on resources etc.
>
> Furhtermore, the DeviceTree binding is a direct representation of the
> Linux driver model, and doesn't necessarily describe "the USB IP-block".
>
> This series therefor attempts to flatten the driver split, and operate
> the glue and core out of the same platform_device instance. And in order
> to do this, the DeviceTree representation of the IP block is flattened.
>
> To avoid littering the dwc3-qcom driver with the migration code - which
> we should be able to drop again in a LTS or two - this is now placed in
> drivers/of/overlays.
>
> A patch to convert a single platform - sc8280xp - is included in the
> series. The broader conversion will be submitted in a follow up series.
>
> [...]
> ---
> Bjorn Andersson (12):
> dt-bindings: usb: snps,dwc3: Split core description
> dt-bindings: usb: Introduce qcom,snps-dwc3
> of: dynamic: Add of_changeset_add_prop_copy()
> of: overlays: Introduce dwc3 flattening overlay
> of: overlays: dwc3-flattening: Add Qualcomm Arm32 overlays
> of: overlays: dwc3-flattening: Add Qualcomm Arm64 board overlays
> of: overlays: dwc3-flattening: Provide overlay symbols
> usb: dwc3: core: Expose core driver as library
> usb: dwc3: core: Don't touch resets and clocks
> usb: dwc3: qcom: Don't rely on drvdata during probe
> usb: dwc3: qcom: Transition to flattened model
> arm64: dts: qcom: sc8280x: Flatten the USB nodes
>
> .../devicetree/bindings/usb/qcom,dwc3.yaml | 13 +-
> .../devicetree/bindings/usb/qcom,snps-dwc3.yaml | 618 ++++++++
> .../devicetree/bindings/usb/snps,dwc3-common.yaml | 415 ++++++
> .../devicetree/bindings/usb/snps,dwc3.yaml | 391 +----
> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 12 +-
> arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 5 +-
> arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 12 +-
> .../boot/dts/qcom/sc8280xp-huawei-gaokun3.dts | 10 +-
> .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 11 +-
> .../boot/dts/qcom/sc8280xp-microsoft-arcata.dts | 10 +-
> .../boot/dts/qcom/sc8280xp-microsoft-blackrock.dts | 18 +-
> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 157 +-
> drivers/of/Kconfig | 2 +
> drivers/of/Makefile | 2 +
> drivers/of/dynamic.c | 20 +
> drivers/of/overlays/Kconfig | 15 +
> drivers/of/overlays/Makefile | 3 +
> drivers/of/overlays/dwc3-flattening/Makefile | 94 ++
> .../of/overlays/dwc3-flattening/dwc3-flattening.c | 1552 ++++++++++++++++++++
> .../of/overlays/dwc3-flattening/dwc3-flattening.h | 188 +++
> .../overlays/dwc3-flattening/dwc3-qcom_apq8094.dts | 32 +
> .../overlays/dwc3-flattening/dwc3-qcom_apq8096.dts | 60 +
> .../dwc3-qcom_apq8096_inforce_ifc6640.dts | 58 +
> .../overlays/dwc3-flattening/dwc3-qcom_ipq4018.dts | 36 +
> .../dwc3-qcom_ipq4018_8dev_jalapeno.dts | 38 +
> .../overlays/dwc3-flattening/dwc3-qcom_ipq4019.dts | 38 +
> .../overlays/dwc3-flattening/dwc3-qcom_ipq5018.dts | 28 +
> .../overlays/dwc3-flattening/dwc3-qcom_ipq5332.dts | 32 +
> .../overlays/dwc3-flattening/dwc3-qcom_ipq5424.dts | 58 +
> .../overlays/dwc3-flattening/dwc3-qcom_ipq6018.dts | 54 +
> .../overlays/dwc3-flattening/dwc3-qcom_ipq8064.dts | 40 +
> .../overlays/dwc3-flattening/dwc3-qcom_ipq8074.dts | 58 +
> .../overlays/dwc3-flattening/dwc3-qcom_ipq9574.dts | 29 +
> .../overlays/dwc3-flattening/dwc3-qcom_msm8953.dts | 32 +
> .../overlays/dwc3-flattening/dwc3-qcom_msm8992.dts | 32 +
> .../overlays/dwc3-flattening/dwc3-qcom_msm8994.dts | 32 +
> .../overlays/dwc3-flattening/dwc3-qcom_msm8996.dts | 58 +
> .../dwc3-qcom_msm8996_oneplus_oneplus3.dts | 56 +
> .../dwc3-qcom_msm8996_oneplus_oneplus3t.dts | 56 +
> .../dwc3-qcom_msm8996_sony_dora_row.dts | 57 +
> .../dwc3-qcom_msm8996_sony_kagura_row.dts | 57 +
> .../dwc3-qcom_msm8996_sony_keyaki_row.dts | 57 +
> .../dwc3-qcom_msm8996_xiaomi_gemini.dts | 56 +
> .../dwc3-qcom_msm8996_xiaomi_natrium.dts | 56 +
> .../dwc3-qcom_msm8996_xiaomi_scorpio.dts | 56 +
> .../overlays/dwc3-flattening/dwc3-qcom_msm8998.dts | 34 +
> .../dwc3-qcom_msm8998_fxtec_pro1.dts | 35 +
> .../dwc3-qcom_msm8998_oneplus_cheeseburger.dts | 32 +
> .../dwc3-qcom_msm8998_oneplus_dumpling.dts | 32 +
> .../dwc3-qcom_msm8998_sony_xperia_lilac.dts | 35 +
> .../dwc3-qcom_msm8998_sony_xperia_maple.dts | 35 +
> .../dwc3-qcom_msm8998_sony_xperia_poplar.dts | 35 +
> .../dwc3-qcom_msm8998_xiaomi_sagit.dts | 32 +
> .../overlays/dwc3-flattening/dwc3-qcom_qcm2290.dts | 32 +
> .../overlays/dwc3-flattening/dwc3-qcom_qcm6490.dts | 63 +
> .../overlays/dwc3-flattening/dwc3-qcom_qcs404.dts | 56 +
> .../overlays/dwc3-flattening/dwc3-qcom_qcs615.dts | 62 +
> .../overlays/dwc3-flattening/dwc3-qcom_qcs8300.dts | 62 +
> .../overlays/dwc3-flattening/dwc3-qcom_qdu1000.dts | 38 +
> .../overlays/dwc3-flattening/dwc3-qcom_qru1000.dts | 38 +
> .../overlays/dwc3-flattening/dwc3-qcom_sa8155p.dts | 71 +
> .../overlays/dwc3-flattening/dwc3-qcom_sa8540p.dts | 129 ++
> .../overlays/dwc3-flattening/dwc3-qcom_sa8775p.dts | 90 ++
> .../dwc3-flattening/dwc3-qcom_sar2130p.dts | 39 +
> .../overlays/dwc3-flattening/dwc3-qcom_sc7180.dts | 39 +
> .../overlays/dwc3-flattening/dwc3-qcom_sc7280.dts | 63 +
> .../overlays/dwc3-flattening/dwc3-qcom_sc8180x.dts | 109 ++
> .../dwc3-flattening/dwc3-qcom_sc8280xp.dts | 129 ++
> .../dwc3-qcom_sc8280xp_microsoft_blackrock.dts | 121 ++
> .../overlays/dwc3-flattening/dwc3-qcom_sda660.dts | 59 +
> .../overlays/dwc3-flattening/dwc3-qcom_sdm450.dts | 33 +
> .../overlays/dwc3-flattening/dwc3-qcom_sdm630.dts | 57 +
> .../overlays/dwc3-flattening/dwc3-qcom_sdm632.dts | 32 +
> .../overlays/dwc3-flattening/dwc3-qcom_sdm636.dts | 59 +
> .../overlays/dwc3-flattening/dwc3-qcom_sdm660.dts | 57 +
> .../overlays/dwc3-flattening/dwc3-qcom_sdm670.dts | 36 +
> .../overlays/dwc3-flattening/dwc3-qcom_sdm845.dts | 64 +
> .../dwc3-qcom_sdm845_lenovo_yoga_c630.dts | 67 +
> .../dwc3-flattening/dwc3-qcom_sdm845_lg_judyln.dts | 67 +
> .../dwc3-flattening/dwc3-qcom_sdm845_lg_judyp.dts | 67 +
> .../dwc3-qcom_sdm845_qcom_sdm845_mtp.dts | 67 +
> .../dwc3-qcom_sdm845_samsung_starqltechn.dts | 67 +
> .../dwc3-qcom_sdm845_samsung_w737.dts | 67 +
> .../dwc3-qcom_sdm845_shift_axolotl.dts | 67 +
> .../dwc3-qcom_sdm845_thundercomm_db845c.dts | 67 +
> .../dwc3-qcom_sdm845_xiaomi_beryllium.dts | 67 +
> .../dwc3-qcom_sdm845_xiaomi_beryllium_ebbg.dts | 67 +
> .../overlays/dwc3-flattening/dwc3-qcom_sdx55.dts | 38 +
> .../overlays/dwc3-flattening/dwc3-qcom_sdx65.dts | 38 +
> .../overlays/dwc3-flattening/dwc3-qcom_sdx75.dts | 36 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm4250.dts | 37 +
> .../dwc3-qcom_sm4250_oneplus_billie2.dts | 35 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm6115.dts | 37 +
> .../dwc3-qcom_sm6115_lenovo_j606f.dts | 35 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm6125.dts | 36 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm6350.dts | 39 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm6375.dts | 36 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm7125.dts | 39 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm7225.dts | 39 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm7325.dts | 60 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm8150.dts | 67 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm8250.dts | 67 +
> .../dwc3-qcom_sm8250_xiaomi_elish.dts | 64 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm8350.dts | 67 +
> .../dwc3-qcom_sm8350_microsoft_surface_duo2.dts | 67 +
> .../dwc3-qcom_sm8350_qcom_sm8350_hdk.dts | 69 +
> .../dwc3-qcom_sm8350_qcom_sm8350_mtp.dts | 67 +
> .../dwc3-qcom_sm8350_sony_pdx214_generic.dts | 67 +
> .../dwc3-qcom_sm8350_sony_pdx215_generic.dts | 67 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm8450.dts | 39 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm8550.dts | 39 +
> .../overlays/dwc3-flattening/dwc3-qcom_sm8650.dts | 39 +
> .../dwc3-flattening/dwc3-qcom_x1e80100.dts | 153 ++
> .../dwc3-qcom_x1e80100_hp_omnibook_x14.dts | 149 ++
> drivers/usb/dwc3/core.c | 167 ++-
> drivers/usb/dwc3/dwc3-qcom.c | 149 +-
> drivers/usb/dwc3/glue.h | 22 +
> include/linux/of.h | 3 +
> 118 files changed, 8389 insertions(+), 670 deletions(-)
> ---

This is quite a lot of code and new files for a temporary migration.
It's also difficult to test these changes fully, since there are
separate overlays for each SoC and sometimes even each board.

Would it be easier to just duplicate the dwc3-qcom driver for now?
Making a copy of the current dwc3-qcom.c would be just 1000 lines of
extra code, compared to more than 7000 for the overlay approach.

The copy (e.g. dwc3-qcom-legacy.c) would keep handling the old bindings
with the existing code (that is known to work to some extent). We can
then improve upon the main version without risk of breaking any old
DTBs. If we decide to drop support for the old DTBs at some point, we
can just drop dwc3-qcom-legacy.

This approach is also not pretty, but I think the risk and effort would
be lower than making sure the overlay approach works on all the affected
targets.

Thanks,
Stephan