Re: [PATCH 06/20] arm64: dts: qcom: kaanapali: Add USB support for Kaanapali SoC

Rob Herring
2025-09-25T21:31:52Z

On Thu, Sep 25, 2025 at 08:57:56AM -0500, Bjorn Andersson wrote:
> On Thu, Sep 25, 2025 at 10:50:10AM +0900, Krzysztof Kozłowski wrote:
> > On Thu, 25 Sept 2025 at 09:17, Jingyi Wang <jingyi.wang@oss.qualcomm.com> wrote:
> > >
> > > From: Ronak Raheja <ronak.raheja@oss.qualcomm.com>
> > >
> > > Add the base USB devicetree definitions for Kaanapali platform. The overall
> > > chipset contains a single DWC3 USB3 controller (rev. 200a), SS QMP PHY
> > > (rev. v8) and M31 eUSB2 PHY.
> > >
> > > Signed-off-by: Ronak Raheja <ronak.raheja@oss.qualcomm.com>
> > > Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> > > ---
> > > arch/arm64/boot/dts/qcom/kaanapali.dtsi | 155 ++++++++++++++++++++++++++++++++
> > > 1 file changed, 155 insertions(+)
> > >
> >
> >
> > Second try, without HTML:
> >
> > I really don't understand why you created such huge patchset.
>
> Because I looked at the logical changes that went into the big squash
> that was initially planned, and requested that some of those was kept
> intact - because they where independent logical changes.
>
> > Year
> > ago, two years ago, we were discussing it already and explained that's
> > just inflating the patchset without reason.
> >
>
> We used to add things node by node and that was indeed not
> comprehensible. Overall this adds features in large logical chunks, but
> there are a few of the patches that could have been squashed.
>
> > New Soc is one logical change. Maybe two. Not 18!
>
> I can see your argument for one patch to introduce the soc. But two
> doesn't make sense, because that incremental patch is going to be the
> kitchen sink.
>
> >
> > Not one patch per node or feature.
> >
>
> Definitely agree that we don't want one patch for every tiny block!
>
> > This hides big picture, makes difficult to review everything,
> > difficult to test.
>
> The big picture is already obscured due to the size of the content
> added.
>
> Comparing to previous targets, I see the baseline content in 2-3
> patches, and the remainder of the series being things that usually has
> been scattered in many more small changes in the following weeks or
> months.
>
> There's plenty of features in this series that are yet to be concluded
> for SM8750.
>
> > Your patch count for LWN stats doesn't matter to
> > us.
>
> I agree with this. That's why the QRD is 1 patch, and MTP is 4 (this I
> think should be squashed to 2) - compared to 13 patches for across the
> pair for SM8750 with less scope.
>
> >
> > NAK and I'm really disappointed I have to repeat the same review .
>
> I'm not sure what you're disappointed in, this initial series is larger
> than any we've seen before. I really like the work Jingyi has done here,
> aggregating the otherwise scattered patches into one series.

The QCom folks can review all this first because I don't care to review
the 50+ binding (just bindings!) patches sent all at once right before
the merge window.

One comment on commit messages though. Please explain how the h/w block
is or isn't compatible with some existing platforms. Many just state the
obvious "add a compatible" or such. I've yet to find what kaanapali is
in relation to any other QCom chip. It may be the next SoC for the smart
toaster market for all I know.

Rob