Re: [PATCH v1 2/4] dt-bindings: usb: dwc3: Add Google SoC DWC3 USB

[Krzysztof Kozlowski <krzk@kernel.org>]

On 07/10/2025 18:09, Peter Griffin wrote:
> Hi Krzysztof & Roy,
> 
> Firstly thanks Roy for your patches, it's great to see more Tensor
> support being posted upstream!
> 
> On Tue, 7 Oct 2025 at 01:44, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 07/10/2025 08:21, Roy Luo wrote:
>>> Document the DWC3 USB bindings for Google Tensor SoCs.
>>>
>>> Signed-off-by: Roy Luo <royluo@google.com>
>>> ---
>>>  .../bindings/usb/google,snps-dwc3.yaml        | 144 ++++++++++++++++++
>>>  1 file changed, 144 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/usb/google,snps-dwc3.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/google,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/google,snps-dwc3.yaml
>>> new file mode 100644
>>> index 000000000000..3e8bcc0c2cef
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/google,snps-dwc3.yaml
>>> @@ -0,0 +1,144 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +# Copyright (c) 2025, Google LLC
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/usb/google,snps-dwc3.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Google DWC3 USB SoC Controller
>>> +
>>> +maintainers:
>>> +  - Roy Luo <royluo@google.com>
>>> +
>>> +description:
>>> +  Describes the Google DWC3 USB block, based on Synopsys DWC3 IP.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - google,lga-dwc3
>>> +      - const: google,snps-dwc3
>>
>>
>> There is no such soc as snps, so you grossly misuse other company name
>> as name of SoC. Neither lga. Otherwise please point me to the top-level
>> bindings describing that SoC.
>>
>> You need to better describe the hardware here - why this is something
>> completely different than GS which. Or switch to existing bindings and
>> existing drivers. Did you align this with Peter Griffin?
> 
> I think (from what I've seen at least) this is the first submission
> for drivers in the Tensor G5 SoC used in Pixel 10 devices (which as I
> understand it isn't based on any Samsung IP). Hence the new drivers,
> bindings etc.


That could explain a lot. I would be happy to see background of hardware
in the bindings commit msg and the cover letter.

> 
> However the issue is that none of the other base SoC drivers on which
> this driver depends currently exist upstream (like clocks, reset
> driver, power domains, pinctrl etc). So it's very hard to reason about
> the correctness or otherwise of this submission. It is also likely
> that when those drivers are upstreamed things could change in the
> review process, to how it looks today in the downstream kernel.


Bindings and drivers can be contributed without core SoC support, but
then please describe the hardware (SoC) here. Having core support posted
earlier is of course preferred, but I think not a requirement.

Anyway, compatibles and all commit messages in this patchset are too
generic and need to reflect this actual SoC, not "Tensor" because we
already have a Tensor with USB. So basically based on existing support
all this patchset would be redundant, right? :)

Best regards,
Krzysztof