On Mon, Jan 07, 2019 at 01:58:33PM +0530, Vinod Koul wrote: > On 22-12-18, 08:28, Lukas Wunner wrote: > > The BCM2835 DMA driver deletes a channel from a list upon termination > > without having added it to a list first. Moreover that operation is > > protected by a spinlock which isn't taken anywhere else. These appear > > to be remnants of an older version of the driver which accidentally > > got mainlined. Remove the dead code. > > > > While at it remove an outdated comment claiming the driver only supports > > Ditto, whenever you use also, while at it... think if this is a > candidate for splitting up. > > A patch should do one thing only.. If a set of changes could provoke a regression, separating each into an individual commit makes sense to ease bisecting. However separating trivial and obviously correct cleanups into individual commits actually makes bisecting *harder* because it increases the number of steps to find the offending commit. Thus, clustering related cleanups together makes sense. It is never a good idea to follow rules such as "a patch should do one thing only" slavishly. Nevertheless, happy to oblige if that's what it takes to get the patches in. Thanks, Lukas