Received-SPF: pass (google.com: domain of linux-kernel+bounces-76266-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240219061008.1761102-1-pumahsu@google.com> <20240219061008.1761102-3-pumahsu@google.com>
 <0b4464eb-631e-4c9f-a7c8-3451be15d8b4@kernel.org> <CAGCq0LYFMrFmxeKZE9g-O61+N03rJoGL0XvXJVya0Yx-ZasvBA@mail.gmail.com>
 <bea850fe-19e8-492e-b885-6d01b389c32c@kernel.org>
In-Reply-To: <bea850fe-19e8-492e-b885-6d01b389c32c@kernel.org>
From: Puma Hsu <pumahsu@google.com>
Date: Thu, 22 Feb 2024 17:45:37 +0800
Message-ID: <CAGCq0La_XHJw4cscahiDWM-n03dWf8SUQ2Zw0YZgHyXwKcURyQ@mail.gmail.com>
Subject: Re: [PATCH 2/3] usb: xhci: Add support for Google XHCI controller
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: mathias.nyman@intel.com, gregkh@linuxfoundation.org, 
	Thinh.Nguyen@synopsys.com, badhri@google.com, royluo@google.com, 
	howardyen@google.com, albertccwang@google.com, raychi@google.com, 
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 21, 2024 at 5:53=E2=80=AFPM Krzysztof Kozlowski <krzk@kernel.or=
g> wrote:
>
> On 21/02/2024 10:31, Puma Hsu wrote:
> > On Mon, Feb 19, 2024 at 8:22=E2=80=AFPM Krzysztof Kozlowski <krzk@kerne=
l.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@99C0000 {
> >>>     compatible =3D "shared-dma-pool";
> >>>     reg =3D <0x00000000 0x99C0000 0x00000000 0x40000>;
> >>>     no-map;
> >>>   };
> >>>
> >>>   dwc3: dwc3@c400000 {
> >>>     compatible =3D "snps,dwc3";
> >>>     reg =3D <0 0x0c400000  0 0x10000>;
> >>>     xhci-glue =3D "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
>