On Thu, Mar 06, 2025, Bjorn Andersson wrote: > On Wed, Mar 05, 2025 at 12:31:49AM +0000, Thinh Nguyen wrote: > > On Mon, Mar 03, 2025, Bjorn Andersson wrote: > > > On Tue, Mar 04, 2025 at 12:39:12AM +0000, Thinh Nguyen wrote: > > > > On Tue, Mar 04, 2025, Thinh Nguyen wrote: > > > > > On Wed, Feb 26, 2025, Bjorn Andersson wrote: > > > > > > In order to more tightly integrate the Qualcomm glue driver with the > > > > > > dwc3 core the driver is redesigned to avoid splitting the implementation > > > > > > using the driver model. But due to the strong coupling to the Devicetree > > > > > > binding needs to be updated as well. > > > > > > > > > > > > Various ways to provide backwards compatibility with existing Devicetree > > > > > > blobs has been explored, but migrating the Devicetree information > > > > > > between the old and the new binding is non-trivial. > > > > > > > > > > > > For the vast majority of boards out there, the kernel and Devicetree are > > > > > > generated and handled together, which in practice means that backwards > > > > > > compatibility needs to be managed across about 1 kernel release. > > > > > > > > > > > > For some though, such as the various Snapdragon laptops, the Devicetree > > > > > > blobs live a life separate of the kernel. In each one of these, with the > > > > > > continued extension of new features, it's recommended that users would > > > > > > upgrade their Devicetree somewhat frequently. > > > > > > > > > > > > With this in mind, simply carrying a snapshot/copy of the current driver > > > > > > is simpler than creating and maintaining the migration code. > > > > > > > > > > > > The driver is kept under the same Kconfig option, to ensure that Linux > > > > > > distributions doesn't drop USB support on these platforms. > > > > > > > > > > > > The driver, which is going to be refactored to handle the newly > > > > > > introduced qcom,snps-dwc3 compatible, is updated to temporarily not > > > > > > match against any compatible. > > > > > > > > > > > > This driver should be removed after 2 LTS releases. > > > > > > > > > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxxxxxxxx> > > > > > > --- > > > > > > drivers/usb/dwc3/Makefile | 1 + > > > > > > drivers/usb/dwc3/dwc3-qcom-legacy.c | 934 ++++++++++++++++++++++++++++++++++++ > > > > > > drivers/usb/dwc3/dwc3-qcom.c | 1 - > > > > > > 3 files changed, 935 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > This is a bit concerning if there's no matching compatible string. ie. > > > > > we don't have user for the new driver without downstream dependencies > > > > > (or some workaround in the driver binding). > > > > > > > > Ignore the comment above, I missed the "temporarily" in your log > > > > above. However, the comment below still stands. > > > > > > > > > > > > > > While I understand the intention, I'm afraid we may have to support and > > > > > maintain this much longer than the proposed 2 LTS releases (as seen with > > > > > anything tagged with "legacy" in the upstream kernel). > > > > > > There are no products shipping today using dwc3-qcom where Devicetree is > > > considered firmware. The primary audience for a longer transition is > > > users of the various laptops with Qualcomm-chip in them. But given the > > > rapid development in a variety of functional areas, these users will be > > > highly compelled to update their DTBs within 2 years. > > > > > > The other obvious user group is to make sure us upstream developers > > > don't loose USB when things get out of sync. > > > > > > > > > That said, if the model defined here is to be followed in other cases > > > (or my other vendors) where Devicetree is treated as firmware, your > > > concerns are valid - and it might be worth taking the cost of managing > > > the live-migration code. > > > > > > > > If possible, I'd > > > > > prefer the complications of maintenance of the migration code be handled > > > > > downstream. > > > > > > > > > > > I'm sorry, but here it sounds like you're saying that you don't want any > > > migration code upstream at all? This is not possible, as this will break > > > USB for developers and users short term. We can of course discuss the 2 > > > LTS though, if you want a shorter life span for this migration. > > > > > > > My first concern is now we have a legacy driver that should not be > > continued to be developed while we also need to address any > > regression/fixes found in the future from the legacy driver. While I > > would encourage users to start migrating to the new driver, I won't > > reject fixes to the legacy driver either. In the next 2 years+, my > > other concern is that I'm not confident that we can easily remove the > > legacy driver and the DTS then. > > > > The problem at hand is that the driver _needs_ a bunch of work. > Role-switching only works sometimes, extcon is (for older platforms) > duplicated in both glue and core - with the hope that each part does its > thing in a suitable fashion, the layering violations can trigger > NULL-pointer dereferences or use-after-free, PM runtime is marked > forbidden... > > We've looked at these problems for a few years now, without coming up > with any solution to address these issues within the current design. That's understood, and that's the incentive for your work here. > > Following this refactor, we will be able to work on these improvements. > For this to happen, I intend to transition all the > arch/*/boot/dts/qcom/* platforms to the new binding as soon as possible. > > > Looking ahead, when we hit the point of deprecating the dwc3-qcom-legacy > driver: > > The upstream-based product we have today do ship Devicetree in > combination with the kernel, so they would upgrade both together and get > the new driver. > > The other group would be kernel developers, enthusiasts, specific users > who for some reason is upgrading their kernel but not their Devicetree. > These users will want the new features and stability we're bringing. > > > Code can break, and that's not unexpected. If 2 LTS releases later and > > we remove the dwc3-qcom-legacy, things can break then too. This may just > > as be painful if we need fixes to the legacy driver due to some previous > > regression. Also, I'm sure your team did a fair share of testing the new > > driver right? Is there some major concern in the new driver that we > > haven't addressed? > > > > The new and old drivers are mostly identical at this point, and expected This was my expectation, that the new and old drivers are mostly identical at this point. This is one of the reasons why I suggested to directly switch to using the new driver at this point. > to diverge from here. This is what I want to avoid. > > The one thing I have identified to differ is that the "legacy" driver > supports 2 extcon handles in the glue, but this is not considered > acceptable by the binding so I haven't found anyone actually exercising > this code path - then again extcon and usb_role_switch is one of the > things this enables us to clean up. > > > That said, while this model seems suitable for Qualcomm, due to the > current state of things, I don't know if the same is true for Frank Li, > perhaps NXP has a broader user base and need the migration logic. > This was not expected. If the new driver doesn't support certain devices with extcon, how can we expect to remove/deprecate the legacy driver without dropping support to these devices. > > > > > > In my view, setting a flag date when the dwc3-qcom-legacy.c will be > > > removed will provide upstream users a transition period, at a very low > > > additional cost (934 lines of already tested code). If someone > > > downstream after that flag date wants to retain support for qcom,dwc3 > > > they can just revert the removal of dwc3-qcom-legacy.c. > > > > The same can be said that they can revert the update (or apply fixes) > > should they found issue with the new change. > > > > We're changing the Devicetree binding, which gives us two problems: > 1) Devicetree source code and DWC3 driver code are merged through > different trees. > > 2) The compiled Devicetree (.dtb) and kernel image are in some cases > separate software deliverables. > > So we absolutely need some migration mechanism to not just break USB for > everyone for the coming 1-2 releases - at least. > > That said, the "2 LTS" is completely arbitrary. If you prefer to limit > that, we can certainly have that discussion! E.g. I wouldn't argue > against setting the flag-date by the end of this year. > I don't know enough about the timeline so suggest a different number. > > > > > > The alternative is that I try to get the migration code suggested in v3 > > > to a state where it can be merged (right now it's 6x larger) and then > > > keep investing indefinitely in making sure it's not bit-rotting > > > (although Rob Herring did request a flag date of the migration code in > > > v3 as well...). > > > > > > > All that said, if you believe that this transition will be quite > > disruptive without preserving the legacy driver/dts, then we will do so. > > > > We absolutely need a transition period, per above reasons. The length of > it is an open question. Ok, but before we merge the new driver, do we have any plan to support devices that use extcon in the new driver? (Apologies if I had missed this discussion prior.) > > > Can I request that you make this snapshot as one of the first patches in > > the series so reverts/git-blames can easily be traced? > > > > Absolutely. Thanks. > > > BR, > > Thinh > > > > Side question: for Snapdragon laptops, without the corresponding kernel > > and DTS updates, don't things break easily? > > It certainly happens, but maintaining backwards compatibility is > something we're striving for. As the Devicetree bindings mature, the > easier this is though. > > One example where this is a problem will be clear here, that users > attempting to boot today's kernel with tomorrows Devicetree blobs will > not get USB - because today's kernel doesn't know how to make of the > information in that description. > > This is true for any hardware or firmware interface though, so there's > only so much one can do about that (and whatever that is, we're trying > to do - for the sake of user friendliness). > Right, for those that migrate the DTS and kernel separately, breakage should not come as a surprise. As you said, there's only so much we can do. I would expect for them to also have certain protocol to handle this when it happens. BR, Thinh
