Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1728434 > unrolled thread

[PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.

Started by<Tristram.Ha@microchip.com>
First post2017-09-07 23:20 +0200
Last post2017-09-08 11:00 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ  switch drivers. <Tristram.Ha@microchip.com> - 2017-09-07 23:20 +0200
    Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ  switch drivers. Andrew Lunn <andrew@lunn.ch> - 2017-09-07 23:30 +0200
      RE: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ  switch drivers. <Tristram.Ha@microchip.com> - 2017-09-08 00:30 +0200
        Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ  switch drivers. Andrew Lunn <andrew@lunn.ch> - 2017-09-08 00:50 +0200
        Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ  switch drivers. Pavel Machek <pavel@ucw.cz> - 2017-09-08 11:00 +0200
    Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ  switch drivers. Andrew Lunn <andrew@lunn.ch> - 2017-09-08 02:50 +0200
    Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ  switch drivers. Pavel Machek <pavel@ucw.cz> - 2017-09-08 11:00 +0200

#1728434 — [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.

From<Tristram.Ha@microchip.com>
Date2017-09-07 23:20 +0200
Subject[PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.
Message-ID<uncno-5CP-5@gated-at.bofh.it>
From: Tristram Ha <Tristram.Ha@microchip.com>

Break ksz_common.c into 2 files so that the common code can be used by other KSZ switch drivers.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile
index ed335e2..0961c30 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -1,2 +1,2 @@
-obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz_common.o
+obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz9477.o ksz_common.o
 obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER)	+= ksz_spi.o

[toc] | [next] | [standalone]

#1728452

FromAndrew Lunn <andrew@lunn.ch>
Date2017-09-07 23:30 +0200
Message-ID<uncx4-5G0-23@gated-at.bofh.it>
In reply to#1728434
On Thu, Sep 07, 2017 at 09:08:58PM +0000, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
> 
> Break ksz_common.c into 2 files so that the common code can be used by other KSZ switch drivers.
> 
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile
> index ed335e2..0961c30 100644
> --- a/drivers/net/dsa/microchip/Makefile
> +++ b/drivers/net/dsa/microchip/Makefile
> @@ -1,2 +1,2 @@
> -obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz_common.o
> +obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz9477.o ksz_common.o
>  obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER)	+= ksz_spi.o

Hi Tristram

I would of thought this would break the build. You don't add ksz9477.c
until the next patch.

Each patch needs to compile, otherwise you break git bisect.

     Andrew

[toc] | [prev] | [next] | [standalone]

#1728476

From<Tristram.Ha@microchip.com>
Date2017-09-08 00:30 +0200
Message-ID<undt8-6g1-19@gated-at.bofh.it>
In reply to#1728452
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 07, 2017 2:25 PM
> To: Tristram Ha - C24268
> Cc: muvarov@gmail.com; pavel@ucw.cz; nathan.leigh.conrad@gmail.com;
> vivien.didelot@savoirfairelinux.com; f.fainelli@gmail.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ
> switch drivers.
> 
> On Thu, Sep 07, 2017 at 09:08:58PM +0000, Tristram.Ha@microchip.com wrote:
> > From: Tristram Ha <Tristram.Ha@microchip.com>
> >
> > Break ksz_common.c into 2 files so that the common code can be used by other
> KSZ switch drivers.
> >
> > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > ---
> > diff --git a/drivers/net/dsa/microchip/Makefile
> > b/drivers/net/dsa/microchip/Makefile
> > index ed335e2..0961c30 100644
> > --- a/drivers/net/dsa/microchip/Makefile
> > +++ b/drivers/net/dsa/microchip/Makefile
> > @@ -1,2 +1,2 @@
> > -obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz_common.o
> > +obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz9477.o ksz_common.o
> >  obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER)	+= ksz_spi.o
> 
> Hi Tristram
> 
> I would of thought this would break the build. You don't add ksz9477.c until the
> next patch.
> 
> Each patch needs to compile, otherwise you break git bisect.
> 
>      Andrew

Eventually the file will need to be broken in two, so you would like to see all 3 changes (Makefile, ksz_common.c, and ksz9477.c) in 1 patch file?

[toc] | [prev] | [next] | [standalone]

#1728484

FromAndrew Lunn <andrew@lunn.ch>
Date2017-09-08 00:50 +0200
Message-ID<undMt-6ou-15@gated-at.bofh.it>
In reply to#1728476
> > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > > ---
> > > diff --git a/drivers/net/dsa/microchip/Makefile
> > > b/drivers/net/dsa/microchip/Makefile
> > > index ed335e2..0961c30 100644
> > > --- a/drivers/net/dsa/microchip/Makefile
> > > +++ b/drivers/net/dsa/microchip/Makefile
> > > @@ -1,2 +1,2 @@
> > > -obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz_common.o
> > > +obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz9477.o ksz_common.o
> > >  obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER)	+= ksz_spi.o
> > 
> > Hi Tristram
> > 
> > I would of thought this would break the build. You don't add ksz9477.c until the
> > next patch.
> > 
> > Each patch needs to compile, otherwise you break git bisect.
> > 
> >      Andrew
> 

> Eventually the file will need to be broken in two, so you would like
> to see all 3 changes (Makefile, ksz_common.c, and ksz9477.c) in 1
> patch file?

You cannot break the build. Each patch must compile on its own.

Breaking changes up into smaller chunks is good. Makes it easier to
review.  So think about how you can do it without breaking the build,
but have smaller changes. For example, move a group of functions at a
time?

	Andrew

[toc] | [prev] | [next] | [standalone]

#1728703

FromPavel Machek <pavel@ucw.cz>
Date2017-09-08 11:00 +0200
Message-ID<unniN-4qa-1@gated-at.bofh.it>
In reply to#1728476

[Multipart message — attachments visible in raw view] — view raw

> > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > > ---
> > > diff --git a/drivers/net/dsa/microchip/Makefile
> > > b/drivers/net/dsa/microchip/Makefile
> > > index ed335e2..0961c30 100644
> > > --- a/drivers/net/dsa/microchip/Makefile
> > > +++ b/drivers/net/dsa/microchip/Makefile
> > > @@ -1,2 +1,2 @@
> > > -obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz_common.o
> > > +obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz9477.o ksz_common.o
> > >  obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER)	+= ksz_spi.o
> > 
> > Hi Tristram
> > 
> > I would of thought this would break the build. You don't add ksz9477.c until the
> > next patch.
> > 
> > Each patch needs to compile, otherwise you break git bisect.
> > 
> >      Andrew
> 
> Eventually the file will need to be broken in two, so you would like
>to see all 3 changes (Makefile, ksz_common.c, and ksz9477.c) in 1
> patch file?

Yes please.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1728517

FromAndrew Lunn <andrew@lunn.ch>
Date2017-09-08 02:50 +0200
Message-ID<unfEB-7Rs-1@gated-at.bofh.it>
In reply to#1728434
On Thu, Sep 07, 2017 at 09:08:58PM +0000, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>

Hi Tristram

Another process thing you are missing. Patch subject should follow a
pattern:

git log --oneline drivers/net/dsa/mv88e6xxx
bb0a2675f72b net: dsa: mv88e6xxx: Enable CMODE config support for 6390X
b3e05aa12319 net: dsa: mv88e6xxx: add a multi_chip info flag
68b8f60cf70d net: dsa: mv88e6xxx: add Energy Detect ops
9069c13a4867 net: dsa: mv88e6xxx: add a global2_addr info flag
9e907d739cc3 net: dsa: mv88e6xxx: add POT operation
a2a05db8a5ed net: dsa: mv88e6xxx: add POT flag to 88E6390
51c901a77562 net: dsa: mv88e6xxx: distinguish Global 2 Rsvd2CPU
d6c5e6aff50c net: dsa: mv88e6xxx: add number of Global 2 IRQs
74e60241ce14 net: dsa: mv88e6xxx: remove 88E6185 G2 interrupt
2466f64ae4e9 net: dsa: mv88e6xxx: remove unused capabilities

Please set the subject line in your patches similarly.

       Andrew

[toc] | [prev] | [next] | [standalone]

#1728705

FromPavel Machek <pavel@ucw.cz>
Date2017-09-08 11:00 +0200
Message-ID<unniO-4qa-7@gated-at.bofh.it>
In reply to#1728434

[Multipart message — attachments visible in raw view] — view raw

On Thu 2017-09-07 21:08:58, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
> 
> Break ksz_common.c into 2 files so that the common code can be used by other KSZ switch drivers.
> 
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile
> index ed335e2..0961c30 100644
> --- a/drivers/net/dsa/microchip/Makefile
> +++ b/drivers/net/dsa/microchip/Makefile
> @@ -1,2 +1,2 @@
> -obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz_common.o
> +obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz9477.o ksz_common.o
>  obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER)	+= ksz_spi.o

I believe you should also rename option to CONFIG_MICROCHIP_KSZ_9477
here... and introduce appropriate Kconfig change.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web