Re: [PATCH v2 1/3] dt-bindings: PCI: brcmstb: Add two optional props

[Florian Fainelli]

On 4/12/23 08:37, Rob Herring wrote:
> On Wed, Apr 12, 2023 at 10:14:46AM -0400, Jim Quinlan wrote:
> > On Wed, Apr 12, 2023 at 7:56 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> > > On 12/04/2023 13:49, Florian Fainelli wrote:
> > > > On 4/12/2023 1:09 AM, Krzysztof Kozlowski wrote:
> > > > > On 11/04/2023 18:59, Jim Quinlan wrote:
> > > > > > Regarding "brcm,enable-l1ss":
> > > > > >
> > > > > >     The Broadcom STB/CM PCIe HW -- a core that is also used by RPi SOCs --
> > > > > >     requires the driver probe() to deliberately place the HW one of three
> > > > > >     CLKREQ# modes:
> > > > > >
> > > > > >     (a) CLKREQ# driven by the RC unconditionally
> > > > > >     (b) CLKREQ# driven by the EP for ASPM L0s, L1
> > > > > >     (c) Bidirectional CLKREQ#, as used for L1 Substates (L1SS).
> > > > > >
> > > > > >     The HW+driver can tell the difference between downstream devices that
> > > > > >     need (a) and (b), but does not know when to configure (c).  Further, the
> > > > > >     HW may cause a CPU abort on boot if guesses wrong regarding the need for
> > > > > >     (c).  So we introduce the boolean "brcm,enable-l1ss" property to indicate
> > > > > >     that (c) is desired.  Setting this property only makes sense when the
> > > > > >     downstream device is L1SS-capable and the OS is configured to activate
> > > > > >     this mode (e.g. policy==superpowersave).
> > > > > >
> > > > > >     This property is already present in the Raspian version of Linux, but the
> > > > > >     upstream driver implementaion that will follow adds more details and
> > > > >
> > > > > typo, implementation
> > > > >
> > > > > >     discerns between (a) and (b).
> > > > > >
> > > > > > Regarding "brcm,completion-timeout-us"
> > > > > >
> > > > > >     Our HW will cause a CPU abort if the L1SS exit time is longer than the
> > > > > >     PCIe transaction completion abort timeout.  We've been asked to make this
> > > > > >     configurable, so we are introducing "brcm,completion-timeout-us".
> > > > > >
> > > > > > Signed-off-by: Jim Quinlan <jim2101024@xxxxxxxxx>
> > > > >
> > > > > What happened here? Where is the changelog?
> > > >
> > > > It is in the cover letter:
> > > >
> > > > https://lore.kernel.org/all/20230411165919.23955-1-jim2101024@xxxxxxxxx/
> > > >
> > > > but it does not look like the cover letter was copied to you or Rob.
> > >
> > > As you said, I did not get it.
> >
> > Yes, sorry about that; I use a wrapper over the "cocci_cc" script and
> > I need to modify one or both scripts to send the cover to the
> > superset of recipients in the constituent commits.
>
> Try out 'b4'. It's much easier.
>
> In any case, I don't read cover letters. Changes to a patch belong with
> the patch.

This is not what most other maintainers do, and there does not appear to 
be a general consensus amongst maintainers that the changes belong in 
the individual patches, or in the cover letter. Some trees like the 
networking tree do merge commits of patch sets where the cover letter is 
used as part of the merge commit message. Other maintainers don't, and 
some want the change log after the '---' and some do not.
--
Florian