[U-Boot] [PATCH 0/6] Migrate BOUNCE_BUFFER

[Tom Rini]

On Fri, Nov 30, 2018 at 04:30:53PM +0100, Philipp Tomsich wrote:
> Tom,
> 
> > On 30.11.2018, at 15:13, Philipp Tomsich <philipp.tomsich at theobroma-systems.com> wrote:
> > 
> > Simon,
> > 
> >> On 30.11.2018, at 14:55, Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com <mailto:simon.k.r.goldschmidt at gmail.com>> wrote:
> >> 
> >> [cut down CC list as gmail won't let me send to that many people :-( ]
> > 
> > Sometimes I wonder if patman will at some point get a feature to avoid those
> > endless CC lists for changes like this one.
> > 
> >> Am 30.11.2018 um 12:39 schrieb Philipp Tomsich:
> >>> A number of MMC drivers uses BOUNCE_BUFFER for their DMA buffers.
> >>> This moves it into Kconfig and performs a step-by-step migration for
> >>> the affected boards/drivers.
> >>> 
> >>> In doing so, it turns out that a few boards/configs enabled
> >>> CONFIG_BOUNCE_BUFFER in their config headers without an apparent need.
> >>> The migration (using moveconfig) for those boards is kept in a
> >>> separate patch, so it can be more easily reviewed by the affected
> >>> parties to make a determination whether it is actually needed.
> >>> 
> >>> Given that BOUNCE_BUFFER only controls whether the bounce_buffer_*
> >>> functions are build and linked, this configuration option could be
> >>> entirely removed and the utility functions built unconditionally.  For
> >>> platforms that don't make use of these functions, the linker will then
> >>> remove the unused symbols.
> >>> 
> >>> I'll leave the final decision if this would be a better implementation
> >>> (or if this should be done in a two-stage process) to someone else...
> >>> END.
> >> 
> >> I think this is a good idea, but I get build errors after applying patch 2/6 since CONFIG_BOUNCE_BUFFER is now enabled via Kconfig and defined as 1 while at the same time it is just defined (but to nothing) in socfpga_common.h.
> > 
> > This is a problem of having multiple individual patches for a moveconfig-style
> > change.  You need the final patch (6/6) in addition to (1/6) for socfpga.
> > 
> >> Can you change or reorder this series so that every commit compiles?
> > 
> > There’s a couple options (none of they appeal to me):
> > 1.	I lump everything together into one big change.
> > 2.	I have a complete moveconfig (i.e. polluting all defconfig files) in the
> > 	first patch and the individual changes to the MMC drivers then remove
> > 	the defconfig entries for their platforms.
> > 3.	We skip this part of the conversion and directly skip forward to having
> > 	bounce-buffer always included and leave the cleanup to the linker…
> > 
> > I’m happy to change this to any of the 3 options (although I’ll probably need
> > a shower afterwards to feel less dirty, if we go with option #1 …)
> 
> I now tried option #2, but that leads to the final patch being subsumed into the
> first one (i.e. the maintainers of socfpga and the bcm* ports don’t get a chance
> to say “we confirm that this is not needed, just drop patch 6/6”).
> 
> So this boils down to whether Tom is ok with this being a series that has failures
> if not fully applied (i.e. patch 1/1 leaving loose ends that the follow-on patches
> address on a per-MMC-controller and per-platform basis)…
> …or if we skip the conversion and trust the linker to do the right thing.

So, what I was thinking I would do here at first was see if we see any
size changes with this series, and if none, then see what happens if we
drop these "enabling but doesn't look used" cases, and see what's going
on.  It wouldn't be the first case where it turns out we have something
enabled and then being discarded as unused and people didn't realize it.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181130/4361ba91/attachment.sig>