Re: [PATCH v1 2/2] arm64: dts: qcom: ipq9574: Fix USB 'vdda-pll-supply'

[Varadarajan Narayanan <quic_varada@quicinc.com>]

On Wed, Dec 06, 2023 at 01:33:17PM +0100, Krzysztof Kozlowski wrote:
> On 06/12/2023 13:31, Krzysztof Kozlowski wrote:
> > On 06/12/2023 12:38, Varadarajan Narayanan wrote:
> >> On Tue, Nov 28, 2023 at 03:01:12PM +0100, Krzysztof Kozlowski wrote:
> >>> On 28/11/2023 11:14, Varadarajan Narayanan wrote:
> >>>> On Tue, Nov 28, 2023 at 09:51:50AM +0100, Krzysztof Kozlowski wrote:
> >>>>> On 28/11/2023 09:46, Varadarajan Narayanan wrote:
> >>>>>> From: Varadarajan Narayanan <quic_varada@quicinc.com>
> >>>>>>
> >>>>>> The earlier patch ec4f047679d5, incorrectly used 'l2'
> >>>>>> as the vdda-pll-supply. However, 'l5' is the correct
> >>>>>> ldo that supplies power to the USB PHY.
> >>>>>>
> >>>>>> Fixes: ec4f047679d5 ("arm64: dts: qcom: ipq9574: Enable USB")
> >>>>>
> >>>>> Doesn't this depend on the driver change?
> >>>>
> >>>> Yes, will mention in the cover letter.
> >>>
> >>> This commit should have it in its changelog ---
> >>>
> >>>>
> >>>>> It affects both existing
> >>>>> kernel and backports which you claim here should happen.
> >>>>
> >>>> Ok. Will include stable@vger.kernel.org in the next revision.
> >>>
> >>> I wasn't speaking about Cc. You indicated this should be backported.
> >>> Then please backport it, without previous commit, and check the result.
> >>> Is stable tree working correctly or not?
> >>
> >> Without the previous commit, it would fail in both the latest
> >> and stable tree. (Please see below for the error messages and
> >> stack dump)
> >>
> >> The previous commit is necessary for this commit to work.
> >
> > Yep, exactly. It's visible from the patches. I don't know how to solve
> > this exactly. The Fixes tag here is logically correct, but then any
> > backporting must include previous commit. Dependency can be provided in
> > cc-stable tag, but you did not cc-stable, I suppose on purpose.

It was not on purpose. Got lucky :).
Shall I separate the patches and wait till the first one gets
merged (in stable and top of tree) and then post the second one?

> > If this is chosen by AUTOSEL, are you going to check if backport
> > includes previous patch and object/review such backport?
>
> One more point. Except issues with backporting, you did not annotate any
> dependency so patches can be applied independently. This will lead to
> non-bisectable tree or even broken tree. What's more DTS goes always via
> separate tree and branches, so this patch must be delayed.
>
> You always must explicitly mention such dependencies and changes to
> default applying process, so maintainers know what to do. Nothing like
> this was explained anywhere here.

Sorry, my mistake. Will be careful in future.

Thanks for the feedback.

Regards
Varada