Re: [PATCH 0/9] Support stm32h747-discovery board

[Sumit Garg <sumit.garg@kernel.org>]

On Mon, Jun 09, 2025 at 10:22:39AM -0600, Tom Rini wrote:
> On Mon, Jun 09, 2025 at 05:07:40PM +0100, Sumit Garg wrote:
> > On Mon, Jun 09, 2025 at 09:50:19AM -0600, Tom Rini wrote:
> > > On Mon, Jun 09, 2025 at 04:40:43PM +0100, Sumit Garg wrote:
> > > > On Mon, Jun 09, 2025 at 03:46:27PM +0200, Dario Binacchi wrote:
> > > > > Hi Sumit,
> > > > > 
> > > > > On Mon, Jun 9, 2025 at 3:25 PM Sumit Garg <sumit.garg@kernel.org> wrote:
> > > > > >
> > > > > > Hi Patrice,
> > > > > >
> > > > > > On Mon, Jun 09, 2025 at 03:15:14PM +0200, Patrice CHOTARD wrote:
> > > > > > >
> > > > > > >
> > > > > > > On 6/7/25 11:37, Dario Binacchi wrote:
> > > > > > > > The series adds support for stm32h747-discovery board.
> > > > > > > >
> > > > > > > > Detailed information can be found at:
> > > > > > > > https://www.st.com/en/evaluation-tools/stm32h747i-disco.html
> > > > > > > >
> > > > > > > >
> > > > > > > > Dario Binacchi (9):
> > > > > > > >   ARM: dts: stm32h7-pinctrl: add _a suffix to u[s]art_pins phandles
> > > > > > > >   dt-bindings: arm: stm32: add compatible for stm32h747i-disco board
> > > > > > > >   dt-bindings: clock: stm32h7: rename USART{7,8}_CK to UART{7,8}_CK
> > > > > > > >   ARM: dts: stm32: add uart8 node for stm32h743 MCU
> > > > > > > >   ARM: dts: stm32: add pin map for UART8 controller on stm32h743
> > > > > > > >   ARM: dts: stm32: add an extra pin map for USART1 on stm32h743
> > > > > > > >   ARM: dts: stm32: support STM32h747i-disco board
> > > > > > > >   ARM: dts: stm32: add stm32h747i-disco-u-boot DTS file
> > > > > > > >   board: stm32: add stm32h747-discovery board support
> > > > > > >
> > > > > > >
> > > > > > > Hi Dario
> > > > > > >
> > > > > > > For the whole series
> > > > > > > Applied to u-boot-stm32/next
> > > > > >
> > > > > > Please give some time for other maintainers to review this patch-set.
> > > > > > The dts/upstream patches in this series aren't clean cherry pick from
> > > > > > upstream.
> > > > > 
> > > > > All the commits are already in the mainline Linux kernel, specifically
> > > > > in v6.16-rc1.
> > > > > If you're referring to the fact that the patches can't be applied
> > > > > cleanly, I believe it's
> > > > > because the target path in the Linux kernel doesn't match the one in U-Boot.
> > > > > In fact, the DTS files are located in two different relative paths.
> > > > 
> > > > That's exactly why we have (refer here [1]):
> > > > 
> > > > ./tools/update-subtree.sh pick dts <commit-id-to-be-picked>
> > > > 
> > > > You should have waited v6.16-rc1 tag to be synced into
> > > > devicetree-rebasing [2] for the cherry-picks to work. This way of
> > > > manually patching dts/upstream is not allowed since it is going to break
> > > > DT syncs in one way or another.
> > > > 
> > > > So I would suggest you to wait for v6.16-rc1 to land in DT rebasing tree
> > > > and then send v2 with proper cherry picked patches.
> > > > 
> > > > [1] https://docs.u-boot.org/en/latest/develop/devicetree/control.html#resyncing-with-devicetree-rebasing
> > > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git
> > > 
> > > To be honest, I don't think this is a big deal. Git will be merging
> > > based on content and not specific hashes. And in the case of conflicts I
> > > just copy the file from the tag to our tree.
> > 
> > The essential problem here to me is we are going to allow manual
> > patching of dts/upstream tree given this example? How do we keep track
> > if all that manual patching landed in Linux DT mainline? The cherry
> > picks ensured that we always keep in sync with mainline.
> > 
> > Lets take an example what if Git automatically resolved a merge conflict
> > for you with duplicated content or if manually patching a DTS file
> > diverged from upstream and get unnoticed during DT syncs?
> > 
> > IMHO, we should try to avoid manual patching of DT subtree otherwise it
> > is hard to set a policy as to what level of manual patching is allowed
> > or not.
> 
> Part of the problem here is that from the standpoint of applying posted
> patches there's no functional difference between what Dario did here and
> what could be done once v6.16-rc1-dts is tagged (if it's not already).
> It's essentially a "manual patch" either way.

Nope, there is a difference here. The cherry-pick from DT rebasing
allows the custodian to rather just cherry pick corresponding DT patches
rather than applying patches posted on mailing list. I usually do that
when reviewing dts/upstream patches if they can be cherry-picked cleanly
or not. So there won't be manual patching in that process.

./tools/update-subtree.sh pick dts <commit-id-to-be-picked>

> We make it clear that
> dts/upstream/ *only* gets changes that are in Linus' tree. If someone
> tries to be sneaky and push something in that's not quite what's
> upstream, it will get stomped on later and there's not going to be any
> sympathy for the now broken platform.

For us the upstream sync path is via DT rebasing tree only. It usually
lags behind Linus' tree by maximum 1 week candence what I have noticed.

> 
> Yes, we document saying to use the cherry-pick script, and that's what
> people should do in general. But I don't think there's value in adding a
> further delay between "in Linus' tree" and "in devicetree-rebasing". In
> the linux kernel, there's thousands of people working on things and so
> strict rules can be understandable (someone will be running a bot to
> look for "(cherry pick from commit $hash)" and fail things where $hash
> doesn't exist, makes sense). Here if the ST custodians are happy just
> verifying the kernel commit, OK, that's fine. Or if they want to wait,
> that's fine too. We can be a little relaxed and let custodians do what
> they see as best.

The reason we adopted OF_UPSTREAM was just to get rid of the manual DT
patching and the syncs. So is it really that few days lag of DT rebasing
tree which is again pushing us towards manual DT patching? I am just
trying to understand the shortcomings that DT rebasing tree puts in
front of us.

-Sumit