Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
@ 2019-06-21 16:39 Puranjay Mohan
  2019-06-21 16:39 ` [PATCH 1/3] net: ethernet: atheros: atlx: Rename local PCI defines to generic names Puranjay Mohan
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Puranjay Mohan @ 2019-06-21 16:39 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Puranjay Mohan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, linux-pci

This patch series removes the private duplicates of PCI definitions in
favour of generic definitions defined in pci_regs.h.

Puranjay Mohan (3):
  net: ethernet: atheros: atlx: Rename local PCI defines to generic
    names
  net: ethernet: atheros: atlx: Include generic PCI definitions
  net: ethernet: atheros: atlx: Remove unused and private PCI
    definitions

 drivers/net/ethernet/atheros/atlx/atl2.c | 5 +++--
 drivers/net/ethernet/atheros/atlx/atl2.h | 2 --
 drivers/net/ethernet/atheros/atlx/atlx.h | 1 -
 3 files changed, 3 insertions(+), 5 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] net: ethernet: atheros: atlx: Rename local PCI defines to generic names
  2019-06-21 16:39 [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates Puranjay Mohan
@ 2019-06-21 16:39 ` Puranjay Mohan
  2019-06-21 16:39 ` [PATCH 2/3] net: ethernet: atheros: atlx: Include generic PCI definitions Puranjay Mohan
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Puranjay Mohan @ 2019-06-21 16:39 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Puranjay Mohan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, linux-pci

Rename all local PCI definitons to Generic PCI definitions to make them
compatible with generic definitions present in pci_regs.h

Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
 drivers/net/ethernet/atheros/atlx/atl2.c | 4 ++--
 drivers/net/ethernet/atheros/atlx/atl2.h | 2 +-
 drivers/net/ethernet/atheros/atlx/atlx.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl2.c b/drivers/net/ethernet/atheros/atlx/atl2.c
index 3a3fb5ce0fee..478db3fe920a 100644
--- a/drivers/net/ethernet/atheros/atlx/atl2.c
+++ b/drivers/net/ethernet/atheros/atlx/atl2.c
@@ -2103,13 +2103,13 @@ static s32 atl2_reset_hw(struct atl2_hw *hw)
 	int i;
 
 	/* Workaround for PCI problem when BIOS sets MMRBC incorrectly. */
-	atl2_read_pci_cfg(hw, PCI_REG_COMMAND, &pci_cfg_cmd_word);
+	atl2_read_pci_cfg(hw, PCI_COMMAND, &pci_cfg_cmd_word);
 	if ((pci_cfg_cmd_word &
 		(CMD_IO_SPACE|CMD_MEMORY_SPACE|CMD_BUS_MASTER)) !=
 		(CMD_IO_SPACE|CMD_MEMORY_SPACE|CMD_BUS_MASTER)) {
 		pci_cfg_cmd_word |=
 			(CMD_IO_SPACE|CMD_MEMORY_SPACE|CMD_BUS_MASTER);
-		atl2_write_pci_cfg(hw, PCI_REG_COMMAND, &pci_cfg_cmd_word);
+		atl2_write_pci_cfg(hw, PCI_COMMAND, &pci_cfg_cmd_word);
 	}
 
 	/* Clear Interrupt mask to stop board from generating
diff --git a/drivers/net/ethernet/atheros/atlx/atl2.h b/drivers/net/ethernet/atheros/atlx/atl2.h
index d97613bd15d5..c53b810a831d 100644
--- a/drivers/net/ethernet/atheros/atlx/atl2.h
+++ b/drivers/net/ethernet/atheros/atlx/atl2.h
@@ -202,7 +202,7 @@ static void atl2_force_ps(struct atl2_hw *hw);
 #define MII_DBG_DATA	0x1E
 
 /* PCI Command Register Bit Definitions */
-#define PCI_REG_COMMAND		0x04
+#define PCI_COMMAND		0x04
 #define CMD_IO_SPACE		0x0001
 #define CMD_MEMORY_SPACE	0x0002
 #define CMD_BUS_MASTER		0x0004
diff --git a/drivers/net/ethernet/atheros/atlx/atlx.h b/drivers/net/ethernet/atheros/atlx/atlx.h
index 7f5d4e24eb9f..09464cb02ce0 100644
--- a/drivers/net/ethernet/atheros/atlx/atlx.h
+++ b/drivers/net/ethernet/atheros/atlx/atlx.h
@@ -445,7 +445,7 @@
 #define MII_DBG_DATA			0x1E
 
 /* PCI Command Register Bit Definitions */
-#define PCI_REG_COMMAND			0x04	/* PCI Command Register */
+#define PCI_COMMAND			0x04	/* PCI Command Register */
 #define CMD_IO_SPACE			0x0001
 #define CMD_MEMORY_SPACE		0x0002
 #define CMD_BUS_MASTER			0x0004
-- 
2.21.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/3] net: ethernet: atheros: atlx: Include generic PCI definitions
  2019-06-21 16:39 [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates Puranjay Mohan
  2019-06-21 16:39 ` [PATCH 1/3] net: ethernet: atheros: atlx: Rename local PCI defines to generic names Puranjay Mohan
@ 2019-06-21 16:39 ` Puranjay Mohan
  2019-06-21 16:39 ` [PATCH 3/3] net: ethernet: atheros: atlx: Remove unused and private " Puranjay Mohan
  2019-06-21 17:11 ` [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates Bjorn Helgaas
  3 siblings, 0 replies; 12+ messages in thread
From: Puranjay Mohan @ 2019-06-21 16:39 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Puranjay Mohan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, linux-pci

Include the uapi/linux/pci_regs.h header file which contains the generic
PCI defines.

Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
 drivers/net/ethernet/atheros/atlx/atl2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/atheros/atlx/atl2.c b/drivers/net/ethernet/atheros/atlx/atl2.c
index 478db3fe920a..58abadd18df2 100644
--- a/drivers/net/ethernet/atheros/atlx/atl2.c
+++ b/drivers/net/ethernet/atheros/atlx/atl2.c
@@ -24,6 +24,7 @@
 #include <linux/netdevice.h>
 #include <linux/pci.h>
 #include <linux/pci_ids.h>
+#include <linux/pci_regs.h>
 #include <linux/pm.h>
 #include <linux/skbuff.h>
 #include <linux/slab.h>
-- 
2.21.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/3] net: ethernet: atheros: atlx: Remove unused and private PCI definitions
  2019-06-21 16:39 [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates Puranjay Mohan
  2019-06-21 16:39 ` [PATCH 1/3] net: ethernet: atheros: atlx: Rename local PCI defines to generic names Puranjay Mohan
  2019-06-21 16:39 ` [PATCH 2/3] net: ethernet: atheros: atlx: Include generic PCI definitions Puranjay Mohan
@ 2019-06-21 16:39 ` " Puranjay Mohan
  2019-06-21 17:13   ` Bjorn Helgaas
  2019-06-26 20:07   ` David Miller
  2019-06-21 17:11 ` [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates Bjorn Helgaas
  3 siblings, 2 replies; 12+ messages in thread
From: Puranjay Mohan @ 2019-06-21 16:39 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Puranjay Mohan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, linux-pci

Remove unused private PCI definitions from skfbi.h because generic PCI
symbols are already included from pci_regs.h.

Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
 drivers/net/ethernet/atheros/atlx/atl2.h | 2 --
 drivers/net/ethernet/atheros/atlx/atlx.h | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl2.h b/drivers/net/ethernet/atheros/atlx/atl2.h
index c53b810a831d..1b25d6d747de 100644
--- a/drivers/net/ethernet/atheros/atlx/atl2.h
+++ b/drivers/net/ethernet/atheros/atlx/atl2.h
@@ -32,7 +32,6 @@
 int ethtool_ioctl(struct ifreq *ifr);
 #endif
 
-#define PCI_COMMAND_REGISTER	PCI_COMMAND
 #define CMD_MEM_WRT_INVALIDATE	PCI_COMMAND_INVALIDATE
 
 #define ATL2_WRITE_REG(a, reg, value) (iowrite32((value), \
@@ -202,7 +201,6 @@ static void atl2_force_ps(struct atl2_hw *hw);
 #define MII_DBG_DATA	0x1E
 
 /* PCI Command Register Bit Definitions */
-#define PCI_COMMAND		0x04
 #define CMD_IO_SPACE		0x0001
 #define CMD_MEMORY_SPACE	0x0002
 #define CMD_BUS_MASTER		0x0004
diff --git a/drivers/net/ethernet/atheros/atlx/atlx.h b/drivers/net/ethernet/atheros/atlx/atlx.h
index 09464cb02ce0..4d355dbc2d01 100644
--- a/drivers/net/ethernet/atheros/atlx/atlx.h
+++ b/drivers/net/ethernet/atheros/atlx/atlx.h
@@ -445,7 +445,6 @@
 #define MII_DBG_DATA			0x1E
 
 /* PCI Command Register Bit Definitions */
-#define PCI_COMMAND			0x04	/* PCI Command Register */
 #define CMD_IO_SPACE			0x0001
 #define CMD_MEMORY_SPACE		0x0002
 #define CMD_BUS_MASTER			0x0004
-- 
2.21.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
  2019-06-21 16:39 [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates Puranjay Mohan
                   ` (2 preceding siblings ...)
  2019-06-21 16:39 ` [PATCH 3/3] net: ethernet: atheros: atlx: Remove unused and private " Puranjay Mohan
@ 2019-06-21 17:11 ` Bjorn Helgaas
  2019-06-21 17:27   ` Joe Perches
  3 siblings, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2019-06-21 17:11 UTC (permalink / raw)
  To: Puranjay Mohan
  Cc: Shuah Khan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, Linux PCI

On Fri, Jun 21, 2019 at 11:39 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
>
> This patch series removes the private duplicates of PCI definitions in
> favour of generic definitions defined in pci_regs.h.
>
> Puranjay Mohan (3):
>   net: ethernet: atheros: atlx: Rename local PCI defines to generic
>     names
>   net: ethernet: atheros: atlx: Include generic PCI definitions
>   net: ethernet: atheros: atlx: Remove unused and private PCI
>     definitions
>
>  drivers/net/ethernet/atheros/atlx/atl2.c | 5 +++--
>  drivers/net/ethernet/atheros/atlx/atl2.h | 2 --
>  drivers/net/ethernet/atheros/atlx/atlx.h | 1 -
>  3 files changed, 3 insertions(+), 5 deletions(-)

Let's slow this down a little bit; I'm afraid we're going to overwhelm folks.

Before posting more to LKML/netdev, how about we first complete a
sweep of all the drivers to see what we're getting into.  It could be
that this will end up being more churn than it's worth.  You could
send the result to linux-kernel-mentees, Shuah, me (but not LKML and
netdev), and then we can talk about whether it should be posted all at
once, as several series, etc.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] net: ethernet: atheros: atlx: Remove unused and private PCI definitions
  2019-06-21 16:39 ` [PATCH 3/3] net: ethernet: atheros: atlx: Remove unused and private " Puranjay Mohan
@ 2019-06-21 17:13   ` Bjorn Helgaas
  2019-06-26 20:07   ` David Miller
  1 sibling, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2019-06-21 17:13 UTC (permalink / raw)
  To: Puranjay Mohan
  Cc: Shuah Khan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, Linux PCI

On Fri, Jun 21, 2019 at 11:40 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
>
> Remove unused private PCI definitions from skfbi.h because generic PCI
> symbols are already included from pci_regs.h.
>
> Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> ---
>  drivers/net/ethernet/atheros/atlx/atl2.h | 2 --
>  drivers/net/ethernet/atheros/atlx/atlx.h | 1 -
>  2 files changed, 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/atheros/atlx/atl2.h b/drivers/net/ethernet/atheros/atlx/atl2.h
> index c53b810a831d..1b25d6d747de 100644
> --- a/drivers/net/ethernet/atheros/atlx/atl2.h
> +++ b/drivers/net/ethernet/atheros/atlx/atl2.h
> @@ -32,7 +32,6 @@
>  int ethtool_ioctl(struct ifreq *ifr);
>  #endif
>
> -#define PCI_COMMAND_REGISTER   PCI_COMMAND
>  #define CMD_MEM_WRT_INVALIDATE PCI_COMMAND_INVALIDATE
>
>  #define ATL2_WRITE_REG(a, reg, value) (iowrite32((value), \
> @@ -202,7 +201,6 @@ static void atl2_force_ps(struct atl2_hw *hw);
>  #define MII_DBG_DATA   0x1E
>
>  /* PCI Command Register Bit Definitions */
> -#define PCI_COMMAND            0x04
>  #define CMD_IO_SPACE           0x0001
>  #define CMD_MEMORY_SPACE       0x0002
>  #define CMD_BUS_MASTER         0x0004

These bit definitions (CMD_IO_SPACE, CMD_MEMORY_SPACE, etc) are also
generic PCI things that should be replaced with PCI_COMMAND_IO,
PCI_COMMAND_MEMORY, etc.  I haven't looked at the file, but there are
likely more.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
  2019-06-21 17:11 ` [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates Bjorn Helgaas
@ 2019-06-21 17:27   ` Joe Perches
  2019-06-21 18:12     ` Bjorn Helgaas
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2019-06-21 17:27 UTC (permalink / raw)
  To: Bjorn Helgaas, Puranjay Mohan, Jay Cliburn, Chris Snook
  Cc: Shuah Khan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, Linux PCI

(adding the atlx maintainers to cc)

On Fri, 2019-06-21 at 12:11 -0500, Bjorn Helgaas wrote:
> On Fri, Jun 21, 2019 at 11:39 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> > This patch series removes the private duplicates of PCI definitions in
> > favour of generic definitions defined in pci_regs.h.
> > 
> > Puranjay Mohan (3):
> >   net: ethernet: atheros: atlx: Rename local PCI defines to generic
> >     names
> >   net: ethernet: atheros: atlx: Include generic PCI definitions
> >   net: ethernet: atheros: atlx: Remove unused and private PCI
> >     definitions
> > 
> >  drivers/net/ethernet/atheros/atlx/atl2.c | 5 +++--
> >  drivers/net/ethernet/atheros/atlx/atl2.h | 2 --
> >  drivers/net/ethernet/atheros/atlx/atlx.h | 1 -
> >  3 files changed, 3 insertions(+), 5 deletions(-)
> 
> Let's slow this down a little bit; I'm afraid we're going to overwhelm folks.

I generally disagree.

Consolidation of these sorts of changes are generally
better done treewide all at once, posted as a series to
a list and maintainers allowing time (weeks to months)
for the specific maintainers to accept them and then
whatever remainder exists reposted and possibly applied
by an overall maintainer (e.g.: Dave M)

> Before posting more to LKML/netdev, how about we first complete a
> sweep of all the drivers to see what we're getting into.  It could be
> that this will end up being more churn than it's worth.

Also doubtful.

Subsystem specific local PCI #defines without generic
naming is poor style and makes treewide grep and
refactoring much more difficult.

The atlx maintainers should definitely have been cc'd
on these patches.

Jay Cliburn <jcliburn@gmail.com> (maintainer:ATLX ETHERNET DRIVERS)
Chris Snook <chris.snook@gmail.com> (maintainer:ATLX ETHERNET DRIVERS)

Puranjay, can you please do a few things more here:

1: Make sure you use scripts/get_maintainer.pl to cc the
   appropriate people.

2: Show that you compiled the object files and verified
   where possible that there are no object file changes.

3: State that there are no object changes in the proposed
   commit log.

thanks.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
  2019-06-21 17:27   ` Joe Perches
@ 2019-06-21 18:12     ` Bjorn Helgaas
  2019-06-21 18:33       ` Joe Perches
  0 siblings, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2019-06-21 18:12 UTC (permalink / raw)
  To: Joe Perches
  Cc: Puranjay Mohan, Jay Cliburn, Chris Snook, Shuah Khan,
	Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, Linux PCI

On Fri, Jun 21, 2019 at 12:27 PM Joe Perches <joe@perches.com> wrote:
>
> (adding the atlx maintainers to cc)
>
> On Fri, 2019-06-21 at 12:11 -0500, Bjorn Helgaas wrote:
> > On Fri, Jun 21, 2019 at 11:39 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> > > This patch series removes the private duplicates of PCI definitions in
> > > favour of generic definitions defined in pci_regs.h.
> > >
> > > Puranjay Mohan (3):
> > >   net: ethernet: atheros: atlx: Rename local PCI defines to generic
> > >     names
> > >   net: ethernet: atheros: atlx: Include generic PCI definitions
> > >   net: ethernet: atheros: atlx: Remove unused and private PCI
> > >     definitions
> > >
> > >  drivers/net/ethernet/atheros/atlx/atl2.c | 5 +++--
> > >  drivers/net/ethernet/atheros/atlx/atl2.h | 2 --
> > >  drivers/net/ethernet/atheros/atlx/atlx.h | 1 -
> > >  3 files changed, 3 insertions(+), 5 deletions(-)
> >
> > Let's slow this down a little bit; I'm afraid we're going to overwhelm folks.
>
> I generally disagree.
>
> Consolidation of these sorts of changes are generally
> better done treewide all at once, posted as a series to
> a list and maintainers allowing time (weeks to months)
> for the specific maintainers to accept them and then
> whatever remainder exists reposted and possibly applied
> by an overall maintainer (e.g.: Dave M)
>
> > Before posting more to LKML/netdev, how about we first complete a
> > sweep of all the drivers to see what we're getting into.  It could be
> > that this will end up being more churn than it's worth.
>
> Also doubtful.
>
> Subsystem specific local PCI #defines without generic
> naming is poor style and makes treewide grep and
> refactoring much more difficult.

Don't worry, we have the same objectives.  I totally agree that local
#defines are a bad thing, which is why I proposed this project in the
first place.

I'm just saying that this is a "first-patch" sort of learning project
and I think it'll avoid some list spamming and discouragement if we
can figure out the scope and shake out some of the teething problems
ahead of time.  I don't want to end up with multiple versions of
dozens of little 2-3 patch series posted every week or two.  I'd
rather be able to deal with a whole block of them at one time.

> The atlx maintainers should definitely have been cc'd
> on these patches.
>
> Jay Cliburn <jcliburn@gmail.com> (maintainer:ATLX ETHERNET DRIVERS)
> Chris Snook <chris.snook@gmail.com> (maintainer:ATLX ETHERNET DRIVERS)
>
> Puranjay, can you please do a few things more here:
>
> 1: Make sure you use scripts/get_maintainer.pl to cc the
>    appropriate people.
>
> 2: Show that you compiled the object files and verified
>    where possible that there are no object file changes.

Do you have any pointers for the best way to do this?  Is it as simple
as comparing output of "objdump -d"?

> 3: State that there are no object changes in the proposed
>    commit log.

Thanks for the additional tips.

Bjorn

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
  2019-06-21 18:12     ` Bjorn Helgaas
@ 2019-06-21 18:33       ` Joe Perches
  2019-06-21 22:45         ` Chris Snook
  2019-06-22  8:50         ` Puranjay Mohan
  0 siblings, 2 replies; 12+ messages in thread
From: Joe Perches @ 2019-06-21 18:33 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Puranjay Mohan, Jay Cliburn, Chris Snook, Shuah Khan,
	Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, Linux PCI

On Fri, 2019-06-21 at 13:12 -0500, Bjorn Helgaas wrote:
> On Fri, Jun 21, 2019 at 12:27 PM Joe Perches <joe@perches.com> wrote:
[]
> > Subsystem specific local PCI #defines without generic
> > naming is poor style and makes treewide grep and
> > refactoring much more difficult.
> 
> Don't worry, we have the same objectives.  I totally agree that local
> #defines are a bad thing, which is why I proposed this project in the
> first place.

Hi again Bjorn.

I didn't know that was your idea.  Good idea.

> I'm just saying that this is a "first-patch" sort of learning project
> and I think it'll avoid some list spamming and discouragement if we
> can figure out the scope and shake out some of the teething problems
> ahead of time.  I don't want to end up with multiple versions of
> dozens of little 2-3 patch series posted every week or two.

Great, that's sensible.

> I'd rather be able to deal with a whole block of them at one time.

Also very sensible.

> > 2: Show that you compiled the object files and verified
> >    where possible that there are no object file changes.
> 
> Do you have any pointers for the best way to do this?  Is it as simple
> as comparing output of "objdump -d"?

Generically, yes.

I have a little script that does the equivalent of:

<git reset>
make <foo.o>
mv <foo.o> <foo.o>.old
patch -P1 < <foo_patch>
make <foo.o>
mv <foo.o> <foo.o>.new
diff -urN <(objdump -d <foo.o>.old) <(objdump -d <foo.o>.new)

But it's not foolproof as gcc does not guarantee
compilation repeatability.

And some subsystems Makefiles do not allow per-file
compilation.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
  2019-06-21 18:33       ` Joe Perches
@ 2019-06-21 22:45         ` Chris Snook
  2019-06-22  8:50         ` Puranjay Mohan
  1 sibling, 0 replies; 12+ messages in thread
From: Chris Snook @ 2019-06-21 22:45 UTC (permalink / raw)
  To: Joe Perches
  Cc: Bjorn Helgaas, Puranjay Mohan, Jay Cliburn, Shuah Khan,
	Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, Linux PCI

On Fri, Jun 21, 2019 at 11:33 AM Joe Perches <joe@perches.com> wrote:
>
> On Fri, 2019-06-21 at 13:12 -0500, Bjorn Helgaas wrote:
> > On Fri, Jun 21, 2019 at 12:27 PM Joe Perches <joe@perches.com> wrote:
> []
> > > Subsystem specific local PCI #defines without generic
> > > naming is poor style and makes treewide grep and
> > > refactoring much more difficult.
> >
> > Don't worry, we have the same objectives.  I totally agree that local
> > #defines are a bad thing, which is why I proposed this project in the
> > first place.
>
> Hi again Bjorn.
>
> I didn't know that was your idea.  Good idea.
>
> > I'm just saying that this is a "first-patch" sort of learning project
> > and I think it'll avoid some list spamming and discouragement if we
> > can figure out the scope and shake out some of the teething problems
> > ahead of time.  I don't want to end up with multiple versions of
> > dozens of little 2-3 patch series posted every week or two.
>
> Great, that's sensible.
>
> > I'd rather be able to deal with a whole block of them at one time.
>
> Also very sensible.
>
> > > 2: Show that you compiled the object files and verified
> > >    where possible that there are no object file changes.
> >
> > Do you have any pointers for the best way to do this?  Is it as simple
> > as comparing output of "objdump -d"?
>
> Generically, yes.
>
> I have a little script that does the equivalent of:
>
> <git reset>
> make <foo.o>
> mv <foo.o> <foo.o>.old
> patch -P1 < <foo_patch>
> make <foo.o>
> mv <foo.o> <foo.o>.new
> diff -urN <(objdump -d <foo.o>.old) <(objdump -d <foo.o>.new)
>
> But it's not foolproof as gcc does not guarantee
> compilation repeatability.
>
> And some subsystems Makefiles do not allow per-file
> compilation.
>

This should work, but be aware that the older atlx drivers did some
regrettable things with file structure, so not all .c files are
expected to generate a corresponding .o file.

- Chris

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
  2019-06-21 18:33       ` Joe Perches
  2019-06-21 22:45         ` Chris Snook
@ 2019-06-22  8:50         ` Puranjay Mohan
  1 sibling, 0 replies; 12+ messages in thread
From: Puranjay Mohan @ 2019-06-22  8:50 UTC (permalink / raw)
  To: Joe Perches
  Cc: Bjorn Helgaas, Puranjay Mohan, Jay Cliburn, Shuah Khan,
	Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, Linux PCI, Chris Snook

On Fri, Jun 21, 2019 at 11:33:27AM -0700, Joe Perches wrote:
> On Fri, 2019-06-21 at 13:12 -0500, Bjorn Helgaas wrote:
> > On Fri, Jun 21, 2019 at 12:27 PM Joe Perches <joe@perches.com> wrote:
> []
> > > Subsystem specific local PCI #defines without generic
> > > naming is poor style and makes treewide grep and
> > > refactoring much more difficult.
> > 
> > Don't worry, we have the same objectives.  I totally agree that local
> > #defines are a bad thing, which is why I proposed this project in the
> > first place.
> 
> Hi again Bjorn.
> 
> I didn't know that was your idea.  Good idea.
> 
> > I'm just saying that this is a "first-patch" sort of learning project
> > and I think it'll avoid some list spamming and discouragement if we
> > can figure out the scope and shake out some of the teething problems
> > ahead of time.  I don't want to end up with multiple versions of
> > dozens of little 2-3 patch series posted every week or two.
> 
> Great, that's sensible.
> 
> > I'd rather be able to deal with a whole block of them at one time.
> 
> Also very sensible.
> 
> > > 2: Show that you compiled the object files and verified
> > >    where possible that there are no object file changes.
> > 
> > Do you have any pointers for the best way to do this?  Is it as simple
> > as comparing output of "objdump -d"?
> 
> Generically, yes.
> 
> I have a little script that does the equivalent of:
> 
> <git reset>
> make <foo.o>
> mv <foo.o> <foo.o>.old
> patch -P1 < <foo_patch>
> make <foo.o>
> mv <foo.o> <foo.o>.new
> diff -urN <(objdump -d <foo.o>.old) <(objdump -d <foo.o>.new)
> 
> But it's not foolproof as gcc does not guarantee
> compilation repeatability.
> 
> And some subsystems Makefiles do not allow per-file
> compilation.
>
Hi Joe,
I tried using your specified technique here are the steps I took and the
results I got.

I built the object file before the patch named it "atl2-old.o"
then I built it after the patch, named it "atl2-new.o"

then i ran these commands:-
$ objdump -d atl2-old.o > 1
$ objdump -d atl2-new.o > 2
$ diff -urN 1 2

--- 1	2019-06-22 13:56:17.881392372 +0530
+++ 2	2019-06-22 13:56:35.228018053 +0530
@@ -1,5 +1,5 @@

-atl2-old.o:     file format elf64-x86-64
+atl2-new.o:     file format elf64-x86-64


 Disassembly of section .text:

So both the object files are similar.

Thanks,
--Puranjay




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] net: ethernet: atheros: atlx: Remove unused and private PCI definitions
  2019-06-21 16:39 ` [PATCH 3/3] net: ethernet: atheros: atlx: Remove unused and private " Puranjay Mohan
  2019-06-21 17:13   ` Bjorn Helgaas
@ 2019-06-26 20:07   ` David Miller
  1 sibling, 0 replies; 12+ messages in thread
From: David Miller @ 2019-06-26 20:07 UTC (permalink / raw)
  To: puranjay12
  Cc: skhan, bjorn, netdev, linux-kernel, linux-kernel-mentees, linux-pci

From: Puranjay Mohan <puranjay12@gmail.com>
Date: Fri, 21 Jun 2019 22:09:21 +0530

> Remove unused private PCI definitions from skfbi.h because generic PCI
> symbols are already included from pci_regs.h.
> 
> Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>

Please also handle the PCI register bit value define duplications as well,
as pointed out by Bjorn.

Thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-21 16:39 [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates Puranjay Mohan
2019-06-21 16:39 ` [PATCH 1/3] net: ethernet: atheros: atlx: Rename local PCI defines to generic names Puranjay Mohan
2019-06-21 16:39 ` [PATCH 2/3] net: ethernet: atheros: atlx: Include generic PCI definitions Puranjay Mohan
2019-06-21 16:39 ` [PATCH 3/3] net: ethernet: atheros: atlx: Remove unused and private " Puranjay Mohan
2019-06-21 17:13   ` Bjorn Helgaas
2019-06-26 20:07   ` David Miller
2019-06-21 17:11 ` [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates Bjorn Helgaas
2019-06-21 17:27   ` Joe Perches
2019-06-21 18:12     ` Bjorn Helgaas
2019-06-21 18:33       ` Joe Perches
2019-06-21 22:45         ` Chris Snook
2019-06-22  8:50         ` Puranjay Mohan

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org netdev@archiver.kernel.org
	public-inbox-index netdev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox