linux-kernel - Re: [PATCH 2/3] usb: xhci: Add support for Google XHCI controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAGCq0La_XHJw4cscahiDWM-n03dWf8SUQ2Zw0YZgHyXwKcURyQ@mail.gmail.com>
Date: Thu, 22 Feb 2024 17:45:37 +0800
From: Puma Hsu <pumahsu@...gle.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: mathias.nyman@...el.com, gregkh@...uxfoundation.org, 
	Thinh.Nguyen@...opsys.com, badhri@...gle.com, royluo@...gle.com, 
	howardyen@...gle.com, albertccwang@...gle.com, raychi@...gle.com, 
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH 2/3] usb: xhci: Add support for Google XHCI controller

On Wed, Feb 21, 2024 at 5:53 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On 21/02/2024 10:31, Puma Hsu wrote:
> > On Mon, Feb 19, 2024 at 8:22 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
> >>
> >> On 19/02/2024 07:10, Puma Hsu wrote:
> >>> In our SoC platform, we support allocating dedicated memory spaces
> >>> other than system memory for XHCI, which also requires IOMMU mapping.
> >>> The rest of driver probing and executing will use the generic
> >>> xhci-plat driver.
> >>>
> >>> We support USB dual roles and switch roles by generic dwc3 driver,
> >>> the dwc3 driver always probes xhci-plat driver now, so we introduce
> >>> a device tree property to probe a XHCI glue driver.
> >>>
> >>> Sample:
> >>>   xhci_dma: xhci_dma@...0000 {
> >>>     compatible = "shared-dma-pool";
> >>>     reg = <0x00000000 0x99C0000 0x00000000 0x40000>;
> >>>     no-map;
> >>>   };
> >>>
> >>>   dwc3: dwc3@...0000 {
> >>>     compatible = "snps,dwc3";
> >>>     reg = <0 0x0c400000  0 0x10000>;
> >>>     xhci-glue = "xhci-hcd-goog";
> >>
> >> NAK, that's not DWC3 hardware in such case.
> >
> > By introducing this property, users can specify the name of their
> > dedicated driver in the device tree. The generic dwc3 driver will
>
> DT is not a place for driver stuff.
>
>
> > read this property to initiate the probing of the dedicated driver.
>
> I know, but it is not a reason to add stuff to DT.
>
> > The motivation behind this is that we have dedicated things
> > (described in commit message) to do for the XHCI driver in our
> > device. BTW, I put this property here because currently there is
> > no xhci node, xhci related properties are put under dwc3 node.
>
> Sorry, you miss the point. Either you have pure DWC3 hardware or not.
> You claim now you do not have pure hardware, which is reasonable,
> because it is always customized per-vendor. In such case you cannot
> claim this is a pure DWC3. You must provide bindings for your hardware.
>
> Now, if you claim you have a pure DWC3 hardware without need for any
> vendor customizations, then entire patchset is fake try to upstream your
> Android vendor stuff. We talked about such stuff many times on mailing
> list, so for obvious reasons I won't repeat it. Trying to push vendor
> hooks and vendor quirks is one of the most common mistakes, so several
> talks already say: don't do this.
>
> > It will be appreciated if there are alternative or more appropriate
> > approaches, we welcome discussion to explore the best possible
> > solution. Thanks.
>
> And what's wrong with all previous feedbacks for similar
> Google/Samsung/Artpec/Tensor vendor hacks? Once or twice per year some
> folks around Google or Samsung try to push such, they all receive the
> same feedback and they disappear, so I have to repeat the same feedback
> to the next person... Please go through previous patches from
> @samsung.com for various subsystems.
>
> Documentation/devicetree/bindings/submitting-patches.rst
> Documentation/devicetree/bindings/writing-bindings.rst
> +other people or my talks on Devicetree
>
> Summarizing: Devicetree is for hardware, not for your driver
> hooks/quirks/needs. Describe properly and fully the hardware, not your
> driver.

Thank you Krzysztof for the explanation. I will study and explore
the possibility of integrating the stuff we want into the generic driver.

>
> Best regards,
> Krzysztof
>