Re: linux-next: build failure after merge of the icc tree

[Georgi Djakov]

Hi Uwe,

On 18.08.22 9:49, Uwe Kleine-König wrote:
> Hello,
>
> On Thu, Aug 18, 2022 at 08:00:51AM +0300, Georgi Djakov wrote:
> > On 18.08.22 4:53, Stephen Rothwell wrote:
> > > Hi all,
> > >
> > > After merging the icc tree, today's linux-next build (x86_64 allmodconfig)
> > > failed like this:
> > >
> > > drivers/interconnect/imx/imx8mp.c: In function 'imx8mp_icc_remove':
> > > drivers/interconnect/imx/imx8mp.c:245:16: error: void value not ignored as it ought to be
> > >     245 |         return imx_icc_unregister(pdev);
> > >         |                ^~~~~~~~~~~~~~~~~~~~~~~~
> > > drivers/interconnect/imx/imx8mp.c:246:1: error: control reaches end of non-void function [-Werror=return-type]
> > >     246 | }
> > >         | ^
> > >
> > > Caused by commit
> > >
> > >     d761e0e9c8f2 ("interconnect: imx: Make imx_icc_unregister() return void")
> > >
> > > I have used the icc tree from next-20220817 for today.
>
> I tried to understand what went wrong here. The problem is that the
> patch "interconnect: imx: Make imx_icc_unregister() return void" was
> developed on top of v5.19-rc1, which doesn't contain
> drivers/interconnect/imx/imx8mp.c. This was only introduced in
>
> 	c14ec5c93dc8 ("interconnect: imx: Add platform driver for imx8mp")
>
> for v6.0-rc1.

Yes, exactly.

> > Thanks Stephen! Fixed.
>
> The history in the icc tree now looks as follows:
>
> $ git lgg linus/master..FETCH_HEAD
> *   8c9b6a59edb7 Merge branch 'icc-ignore-return-val' into icc-next
> |\
> | * f62e3f595c5f interconnect: imx: Make imx_icc_unregister() return void
> * |   c86cfdbaf8e3 Merge branch 'icc-ignore-return-val' into icc-next
> |\ \
> | * | d761e0e9c8f2 interconnect: imx: Make imx_icc_unregister() return void
> | |/
> | * 680f8666baf6 interconnect: Make icc_provider_del() return void
> | * fa80a2994d35 interconnect: sm8450: Ignore return value of icc_provider_del() in .remove()
> | * f221bd781f25 interconnect: osm-l3: Ignore return value of icc_provider_del() in .remove()
> | * 919d4e1a207e interconnect: msm8974: Ignore return value of icc_provider_del() in .remove()
> | * 4681086c9bec interconnect: icc-rpmh: Ignore return value of icc_provider_del() in .remove()
> | * 8ef2ca20754d interconnect: icc-rpm: Ignore return value of icc_provider_del() in .remove()
> | * 7ec26b8dcc5c interconnect: imx: Ignore return value of icc_provider_del() in .remove()
> |/
> o 568035b01cfb (tag: v6.0-rc1) Linux 6.0-rc1
>
> So the commit that doesn't build is still included and might annoy
> bisection. Also in my eyes it's kind of ugly to have two commits with
> identical commit log and nearly identical content in the same tree.
>
> I don't know your preferences about not rewriting your tree once it was
> exposed to the public, but if you are willing to rewrite your tree to
> improve, the possibilities (in order of my preference) are:
>
>   - Drop the broken commit and only include the fixed
>     icc-ignore-return-val branch.

Yes, that was actually my intention, but looks like i merged the fixed branch on top (without
un-merging the previous one). Sorry about that. It should be fine now.

Thanks!
Georgi

>   - On top of the broken branch add a commit that only fixes the problem
>     but doesn't duplicate most of d761e0e9c8f2, yielding something like:
>
> 	*   bcdefghijlkm Merge branch 'icc-ignore-return-val' into icc-next
> 	|\
> 	| * abcdefghijkl interconnect: imx: Fix imx8mp build
> 	| * d761e0e9c8f2 interconnect: imx: Make imx_icc_unregister() return void
> 	| * 680f8666baf6 interconnect: Make icc_provider_del() return void
> 	| * fa80a2994d35 interconnect: sm8450: Ignore return value of icc_provider_del() in .remove()
> 	| * f221bd781f25 interconnect: osm-l3: Ignore return value of icc_provider_del() in .remove()
> 	| * 919d4e1a207e interconnect: msm8974: Ignore return value of icc_provider_del() in .remove()
> 	| * 4681086c9bec interconnect: icc-rpmh: Ignore return value of icc_provider_del() in .remove()
> 	| * 8ef2ca20754d interconnect: icc-rpm: Ignore return value of icc_provider_del() in .remove()
> 	| * 7ec26b8dcc5c interconnect: imx: Ignore return value of icc_provider_del() in .remove()
> 	|/
> 	o 568035b01cfb (tag: v6.0-rc1) Linux 6.0-rc1
>
>     or
>
> 	* abcdefghijkm interconnect: imx: Fix imx8mp build
> 	*   c86cfdbaf8e3 Merge branch 'icc-ignore-return-val' into icc-next
> 	|\
> 	| * d761e0e9c8f2 interconnect: imx: Make imx_icc_unregister() return void
> 	| * 680f8666baf6 interconnect: Make icc_provider_del() return void
> 	| * fa80a2994d35 interconnect: sm8450: Ignore return value of icc_provider_del() in .remove()
> 	| * f221bd781f25 interconnect: osm-l3: Ignore return value of icc_provider_del() in .remove()
> 	| * 919d4e1a207e interconnect: msm8974: Ignore return value of icc_provider_del() in .remove()
> 	| * 4681086c9bec interconnect: icc-rpmh: Ignore return value of icc_provider_del() in .remove()
> 	| * 8ef2ca20754d interconnect: icc-rpm: Ignore return value of icc_provider_del() in .remove()
> 	| * 7ec26b8dcc5c interconnect: imx: Ignore return value of icc_provider_del() in .remove()
> 	|/
> 	o 568035b01cfb (tag: v6.0-rc1) Linux 6.0-rc1
>
>   - At least point out in the fixed variant, what and why it was
>     necessary to redo the commit.
>
> Thanks
> Uwe