On Wed, Apr 02, 2025 at 12:42:54PM +0530, Sumit Garg wrote: > Sorry for jumping in late in this thread as it is still using my old > Linaro email ID which should be disabled by now. > > On Tue, Apr 01, 2025 at 12:02:17PM -0600, Tom Rini wrote: > > On Tue, Apr 01, 2025 at 07:28:36PM +0200, Krzysztof Kozlowski wrote: > > > On 01/04/2025 18:40, Tom Rini wrote: > > > > On Tue, Apr 01, 2025 at 05:27:30PM +0200, Krzysztof Kozlowski wrote: > > > >> On 01/04/2025 16:44, Tom Rini wrote: > > > >>> On Thu, Mar 27, 2025 at 03:58:52PM +0100, Krzysztof Kozlowski wrote: > > > >>>> On 27/03/2025 15:50, Christian Marangi wrote: > > > >>>>> On Thu, Mar 27, 2025 at 03:43:47PM +0100, Krzysztof Kozlowski wrote: > > > >>>>>> On 14/03/2025 19:59, Christian Marangi wrote: > > > >>>>>>> Drop NUM_CLOCKS define for EN7581 dts/upstream/src/include. This > > > >>>>>>> is not a binding and > > > >>>>>>> should not be placed here. Value is derived internally in the user > > > >>>>>>> driver. > > > >>>>>>> > > > >>>>>>> Signed-off-by: Christian Marangi <[email protected]> > > > >>>>>>> Acked-by: Krzysztof Kozlowski <[email protected]> > > > >>>>>> Please drop my Ack. I have never acked such patch for uboot. If I > > > >>>>>> did, > > > >>>>>> it was by mistake - probably you CC-ed me for some reason. > > > >>>>>> > > > >>>>> > > > >>>>> Some explaination, uboot introduced the concept of upstream where > > > >>>>> they > > > >>>>> "import" linux patch for dts and dt-bindings. > > > >>>> > > > >>>> I expected OF_UPSTREAM to be taking the sources, not patches. > > > >>>> > > > >>>>> > > > >>>>> This and the other patch are the exact upstream patch with only the > > > >>>>> path > > > >>>>> changed so I keep all the patch commit message with tags and added > > > >>>>> the > > > >>>>> > > > >>>>> [ upstream commit ] thing. > > > >>>>> > > > >>>>> Hope Tom can better suggest how this should be done. You were CC > > > >>>>> probably because the git send-email included you as present in the > > > >>>>> different tags. > > > >>>> > > > >>>> Well, Ack is still not valid because I did not Ack exactly that > > > >>>> change. > > > >>>> It does not matter for the ack, but for Reviewed-by it would matter, > > > >>>> because it is a statement (of oversight...). I cannot control what > > > >>>> you > > > >>>> put into patches taken out of kernel, but at least do not Cc me on > > > >>>> that. > > > >>> > > > >>> In specifics, yes, we should update doc/develop/devicetree/control.rst > > > >>> and maybe doc/develop/sending_patches.rst to use --suppress-cc=all for > > > >>> dts/upstream. > > > >>> > > > >>> But in general, what do you expect people to be doing with content > > > >>> from > > > >>> devicetree-rebasing? We're doing some direct cherry-picks in between > > > >>> merging of the tags. I think it would be weird to be dropping the tags > > > >>> and un-attributing peoples work. > > > >> > > > >> > > > >> I rather expected something like how kernel is importing dtc. You just > > > >> list the commits you get. If you want the full git history, then I > > > >> would > > > >> expect simple git submodule. In both cases there will be no such > > > >> patches > > > >> on the lists. > > > >> > > > >> For the Ack it does not matter, but I would feel uncomfortable if > > > >> people > > > >> were sending stripped and modified patches with my Rb tag. > > > > > > > > I guess I'm confused. Looking at > > > > https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/ > > > > we're doing the normal thing of havig "[ upstream commit <sha>]" after > > > > the imported log. When I merge the subtree and tag it indeed gives what > > > > you're expecting too. > > > > > > When you merge subtree, the patch is not modified and it lives in > > > separate repo. No one sends them over lists, no one modifies them. > > > Unlike here (even if modification did not happen, person was touching it > > > so how can anyone be sure? That's not a scripted process). > > > > We merge the subtree on tags, and people cherry-pick commits in between > > tags when needed. This is a case of the latter, which is why it says "[ > > upstream commit <sha> ]" in the commit message, which is the usual case. > > Although we have tooling to pick patches from devicetree-rebasing tree > but I can see Krzysztof's concerns here. We can't be sure if developer > has touched the cherry picked patch or not but I suppose there would be > similar concerns for the stable backports for Linux too. So IMHO, it's > really upto maintainer applying those cherry-picked patches to see if > there is any difference from upstream. > > However, there is an additional process change what we can do here is > for the developer to list just the commit IDs for the patches to be > cherry picked in dts/upstream in the cover letter. This way the > maintainer can just directly use the tooling to cherry pick those > patches before applying the patch-set. > > Tooling already available for cherry-picking subtree commits as: > > $ ./tools/update-subtree.sh pick dts <devicetree-rebasing-commit-id>
For U-Boot at least, at the end of the day the cherry-picks are a temporary measure. We do pick up and merge each release tag in turn (and so yeah, I should grab the newest one already) and if the merge doesn't Just Work, I drop in the correct file from upstream to resolve the merge conflict. -- Tom
signature.asc
Description: PGP signature
