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 <ansuels...@gmail.com> > > >>>>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlow...@linaro.org> > > >>>>>> 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/20250314185941.27834-6-ansuels...@gmail.com/ > > > 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> -Sumit
