On Tue, May 19, 2015 at 04:17:33PM +0200, Martin Sperl wrote: > > On 19.05.2015, at 14:46, Mark Brown <broonie@xxxxxxxxxx> wrote: Quite a few process issues here which are rather annoying so I'm just going to ignore the technical content for now. > > Like I say I'm not entirely convinced we need the extra flags over just > > using can_dma(). > If you can tell me that: > (master->flags & (SPI_MASTER_MUST_TX | SPI_MASTER_MUST_RX)) && > master->can_dma && master->can_dma(...) > means that we do NOT need real memory allocated in the first place but > we only expect a scatter list to be created > and memory allocated in all other cases then that be it. I replied to your last mail on this in the other thread. > >> It also fixes the insufficient cleanup in case __spi_map_msg returns > >> an error. > > This should be a separate patch. > The problem is that this came up while developing the patch, so > I left it in the patch, assuming there would be some feedback > that requires another version of the patch... There is no barrier to creating a separate patch for unrelated improvments you happen to notice while working on an issue, it is something that is well supported by the tooling. Not doing this guarantees that a resubmission will be required, means that reviewers have to review the same code multiple times (which takes their time) and means that it takes longer for other people to get the benefit of the change. > > This is updating a specific driver to use the new API you're adding, > > this should be in a separate patch. > At one occasion I got scolded for separating things out into different > patches (in that case api from implemetation), now the other way around, > but we can do that... One patch per change as covered in SubmittingPatches. Prototypes for a function with no implementation are not a separate change since they can't be used without the implementation also being added. Changing a driver is a separate change to adding an API since people can use the API change even if they don't use that driver. Not doing this causes extra work for people, it makes review harder, causes resubmissions, and if the randomly mixed changes do end up getting applied somehow then it makes things like bisection and backporting harder.
Attachment:
signature.asc
Description: Digital signature
