Re: [PATCHv3 1/4] mc13xxx-core: Consolidate common code to prepare for separate i2c and spi

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marc and Uwe,

On Mon, Dec 20, 2010 at 09:31:20AM +0100, Uwe Kleine-König wrote:
> On Mon, Dec 20, 2010 at 02:50:52PM +1100, Marc Reilly wrote:
> > This patch moves common mc13xxx definitions to the header file in
> > preparation for separate i2c and spi driver modules.
> > spi specific functions are also removed.
> > 
> > Changes to the mc13xxx struct are:
> > removing the redundant irq member,
> > adding driver read/write function ptrs,
> > adding ictype
> This list isn't complete, but see below.
> 
> I don't like that after this patch the driver isn't functional, because
> you removed the spi functionality which breaks bisection.

Ugh, yes.  Breaking bisection is absolutely a no-no.  I'll wait for
the next version of this series before I do a full review.

> 
> > Signed-off-by: Marc Reilly <marc@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/mfd/mc13xxx-core.c  |  207 +++++++-----------------------------------
> >  include/linux/mfd/mc13xxx.h |   77 +++++++++++-----
> >  2 files changed, 87 insertions(+), 197 deletions(-)
> > 
> > diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
> > index a2ac2ed..3367a40 100644
> > --- a/drivers/mfd/mc13xxx-core.c
> > +++ b/drivers/mfd/mc13xxx-core.c
> > @@ -9,24 +9,14 @@
> >   * the terms of the GNU General Public License version 2 as published by the
> >   * Free Software Foundation.
> >   */
> > -
> (hmm.  I think there is no style guide for that, but I know people who
> think that this nl should be there.  So IMHO don't touch this.)

A better comment is: don't make unrelated, non-functional whitespace
changes in patches for something else.  They add noise that make it
harder to review.

Cheers,
g.

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux