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

[Uwe Kleine-König]

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.

> 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.

 - 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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature