Message-ID: <CAL_JsqLCLy0JxPtbg5sbXux-o8aQi3a8EOs0c=VEJCePew72nw@mail.gmail.com>
Date: Fri, 26 Sep 2025 09:47:09 -0500
From: Rob Herring <robh@...nel.org>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Bjorn Andersson <andersson@...nel.org>, Krzysztof Kozłowski <k.kozlowski.k@...il.com>, 
	Jingyi Wang <jingyi.wang@....qualcomm.com>, Konrad Dybcio <konradybcio@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, linux-arm-msm@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	aiqun.yu@....qualcomm.com, tingwei.zhang@....qualcomm.com, 
	trilok.soni@....qualcomm.com, yijie.yang@....qualcomm.com, 
	Ronak Raheja <ronak.raheja@....qualcomm.com>
Subject: Re: [PATCH 06/20] arm64: dts: qcom: kaanapali: Add USB support for
 Kaanapali SoC

On Fri, Sep 26, 2025 at 8:21 AM Konrad Dybcio
<konrad.dybcio@....qualcomm.com> wrote:
>
> On 9/25/25 11:31 PM, Rob Herring wrote:
> > 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@....qualcomm.com> wrote:
> >>>>
> >>>> From: Ronak Raheja <ronak.raheja@....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@....qualcomm.com>
> >>>> Signed-off-by: Jingyi Wang <jingyi.wang@....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.
>
> Unfortunately this is sort of beyond our control. We don't expect you to
> review or apply these patches immediately.

It is *only* in your (QCom) control. I would love to have control over
receiving patches to review, but sadly I do not.

Then you should mark them RFC at least if you know they are going into 6.18.

> The platform announcement just happened to occur at this and not any other
> time, and we can't just ask the entire company to shift it to better
> accommodate the kernel release cycle..

That's exactly what we expect. Companies following the rules or
preferences of the kernel community is exactly what is expected and
required. Companies that continuously fail to do that result in
requirements that all patches be first signed off by trusted kernel
developers in those companies.

What would you have done if the timing hit in the merge window where
you have trees which have policies of don't send new content during
merge windows? Just going to ignore that?


> We do have an interest in sharing the work at the earliest time possible,
> and with all the legal knots included, this is what it came down to.
>
> I (and many others) made an internal push to upstream any pre-requisite
> patches that we didn't need to disclose any platform details for in
> advance, so this patchbomb is actually somewhat reduced.. but of course
> DT and bindings are the main course size-wise and we simply couldn't do
> it earlier.
>
> Give or take 80% of the bindings will be "boring", i.e. "add compatbile"
> or "add compatible and adjust clocks" because our hw is rather
> standardized and the interesting changes often happen at a level beyond
> bindings
>
> > 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.
>
> Perhaps this would be useful to have in bindings commit messages, but
> the cover letter of >this< series states that Kaanapali is the newly
> announced Snapdragon 8 Elite Gen 5.

Patches should stand on their own. I'm talking about patches in other series.

> The product page states at the very bottom of the spec sheet that
> SM8850 is another name for it (although the shift to codenames
> happened precisely to disconnect from specific SKU numbers,
> because e.g. both SA8775P and QCS9100 are 'lemans' silicon)

Sorry, I'm not going to go read your product pages...

Rob

Powered by blists - more mailing lists