Re: [GIT PULL 3/4] ARM: SoC-related driver updates

[Linus Torvalds]

On Wed, May 15, 2019 at 11:43 PM Olof Johansson <olof@xxxxxxxxx> wrote:
>
> Various driver updates for platforms and a couple of the small driver
> subsystems we merge through our tree:

Hmm. This moved the aspeed drivers from drivers/misc to
drivers/soc/aspeed (in commit 524feb799408 "soc: add aspeed folder and
misc drivers"), but in the meantime we also had a new aspeed soc
driver added (in commit 01c60dcea9f7 "drivers/misc: Add Aspeed P2A
control driver").

I ended up resolving that "conflict" by moving the new aspeed P2A
control driver to be with the other aspeed drivers too. That seemed to
be the cleanest model.

I'm used to doing these kinds of fixups in a merge, but I have to
admit that maybe I should have made it a separate commit, because now
it's kind of non-obvious, and it's sometimes harder to see changes
that are in a merge commit than in a separate commit.

In particular, it looks like "git log --follow" is not smart enough to
follow a rename through a merge. But I think that is a git problem,
and not a very serious one at that ("git blame" has no such problem).

And it means that now the merge has

 drivers/{misc => soc/aspeed}/aspeed-lpc-ctrl.c                   |   0
 drivers/{misc => soc/aspeed}/aspeed-lpc-snoop.c                  |   0
 drivers/{misc => soc/aspeed}/aspeed-p2a-ctrl.c                   |   0

when you do "git show --stat" on it, which looks correct, and it feels
like conceptually the right merge resolution to me.

Sending out this explanatory email to everybody involved, just so that
this doesn't take you by surprise. But it looks like Patrick Venture
is not just the author of that moved driver, he was also involved in
the move of the two other drivers, so I'm guessing there's not going
to be a lot of confusion here.

HOWEVER. More subtly, as part of my *testing* for this, I also
realized that commit 524feb799408 is buggy. In my tests, the config
worked fine, but the aspeed drivers were never actually *built*. The
reason is that commit 524feb799408 ends up doing

   obj-$(CONFIG_ARCH_ASPEED)      += aspeed/

which is completely wrong, because the Kconfig fules are

        depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON

so those drivers can be configured even if ARCH_ASPEED *isn't* set.
The Kconfig part works fine, because the soc/aspeed/Kconfig file is
included unconditionally, but the actual build process then never
builds anything in the drivers/soc/aspeed/ subdirectory.

I solved _that_ problem by adding a new config option:

  config SOC_ASPEED
      def_bool y
      depends on ARCH_ASPEED || COMPILE_TEST

and using that instead of ARCH_ASPEED.

End result: this was a somewhat messy merge, and the most subtle mess
was because of that buggy 524feb799408 "soc: add aspeed folder and
misc drivers").

I *think* I sorted it all out correctly, and now I see the aspeed
drivers being built (and cleanly at that) but I really *really* want
people to double-check this all.

Also, I think that the same "we don't actually build-test the end
result" problem exists else-where for the same reasons.

At the very least, drivers/soc/{atmel,rockchip,zte} seem to have the
exact same pattern: the Kconfig files enable the drivers, but the
Makefile in drivers/soc doesn't actually traverse into the
subdirectories.

End result: CONFIG_COMPILE_TEST doesn't actually do any compile
testing for those drivers.

I did not try to fix all of those things up, because I didn't do the
driver movements there.

                  Linus