On 22/07/2022 17:41, Doug Anderson wrote: > Hi, > > On Thu, Jul 21, 2022 at 5:22 PM Rob Herring <robh@xxxxxxxxxx> wrote: >> >> On Thu, Jul 21, 2022 at 11:29:13AM -0700, Doug Anderson wrote: >>> Hi, >>> >>> On Thu, Jul 21, 2022 at 9:52 AM Krzysztof Kozlowski >>> <krzysztof.kozlowski@xxxxxxxxxx> wrote: >>>> >>>> On 21/07/2022 18:43, Doug Anderson wrote: >>>>> Hi, >>>>> >>>>> On Thu, Jul 21, 2022 at 9:33 AM Krzysztof Kozlowski >>>>> <krzysztof.kozlowski@xxxxxxxxxx> wrote: >>>>>> >>>>>> On 21/07/2022 15:37, Doug Anderson wrote: >>>>>>> >>>>>>> Not worth sending a new version for, but normally I expect the >>>>>>> bindings to be patch #1 and the dts change to be patch #2. In any >>>>>>> case: >>>>>>> >>>>>>> Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx> >>>>>> >>>>>> I would say worth v4, because otherwise patches is not bisectable. >>>>> >>>>> You're saying because `dtbs_check` will fail between the two? >>>> >>>> Yes >>> >>> OK. Then I assume you agree that reversing the order of the patches >>> won't help, only combining the two patches into one. >>> >>> >>>>> How does >>>>> flipping the order help? If `dtbs_check` needs to be bisectable then >>>>> these two need to be one patch, but I was always under the impression >>>>> that we wanted bindings patches separate from dts patches. >>>> >>>> I don't think anyone said that bindings patches must be separate from >>>> DTS. The only restriction is DTS cannot go with drivers. >>> >>> I have always heard that best practice is to have bindings in a patch >>> by themselves. If I've misunderstood and/or folks have changed their >>> minds, that's fine, but historically I've been told to keep them >>> separate. >> >> Correct. >> >> >>>> Bindings for boards go pretty often with DTS (subarch). This is exactly >>>> what maintainers do, e.g.: >>>> https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/log/?h=arm64-for-5.20 >>>> Bindings for hardware should go via subsystem maintainer (drivers). >>> >>> OK, fair that in this case both the bindings and the yaml will land >>> through the Qualcomm tree. I guess it's really up to Bjorn and whether >>> he'd prefer "make dtbs_check" to be bisectable or whether he'd prefer >>> the bindings and dts change to be in separate patches from each other. >> >> Bindings go first if applied together because you have to define the >> binding before you use it. But sometimes things go via multiple trees >> and that's fine because it's just easier. In that case, the subsystem >> tree is preferred for bindings (i.e. with the driver). But in this case, >> Bjorn is the subsystem tree. > > Thanks! I'll interpret your response as: > > 1. Keep this as two patches and it's more important to keep dts and > bindings separate than it is to avoid breaking bisectability of "make > dtbs_check". No one questioned this here... > > 2. Bindings should have been patch #1, but it's not a massive deal. This started our discussion and I said it should be a v4 with a proper order. It's not massive deal, but hopefully the submitter will learn something. > > 3. I'll assume that Bjorn will yell if he'd like this series re-posted > with the reverse order. Best regards, Krzysztof