[PATCH v2 5/6] dt-bindings: clock: drop NUM_CLOCKS define for EN7581

[Tom Rini]

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 <ansuelsmth at gmail.com>
> > > >>>>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski at 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-ansuelsmth@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>

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250402/991e5367/attachment.sig>