diff mbox

net: dm9000: Fix build

Message ID 1303124677-6168-1-git-send-email-broonie@opensource.wolfsonmicro.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Mark Brown April 18, 2011, 11:04 a.m. UTC
Commit c88fcb (net: dm9000: convert to hw_features) broke the build of
the dm9000 driver since it merged functions which use different names
for the board info structure used for I/O operations without updating
all the references to use the same name. Fix that.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/net/dm9000.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Michał Mirosław April 18, 2011, 11:12 a.m. UTC | #1
On Mon, Apr 18, 2011 at 12:04:37PM +0100, Mark Brown wrote:
> Commit c88fcb (net: dm9000: convert to hw_features) broke the build of
> the dm9000 driver since it merged functions which use different names
> for the board info structure used for I/O operations without updating
> all the references to use the same name. Fix that.

[...]

This brings the issue of build testing effectiveness. In current code
there is no configuration that makes all drivers build. I would like
to see something like 'make brokenconfig' that would allow most of
the code to be built, and not necessarily working. Maybe someone has
an idea how to implement that?

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown April 18, 2011, 12:25 p.m. UTC | #2
On Mon, Apr 18, 2011 at 01:12:03PM +0200, Micha? Miros?aw wrote:
> On Mon, Apr 18, 2011 at 12:04:37PM +0100, Mark Brown wrote:
> > Commit c88fcb (net: dm9000: convert to hw_features) broke the build of
> > the dm9000 driver since it merged functions which use different names
> > for the board info structure used for I/O operations without updating
> > all the references to use the same name. Fix that.

> This brings the issue of build testing effectiveness. In current code
> there is no configuration that makes all drivers build. I would like
> to see something like 'make brokenconfig' that would allow most of
> the code to be built, and not necessarily working. Maybe someone has
> an idea how to implement that?

For the drivers that genuinely are rather platform specific this tends
to fail very badly as you need headers that only come along with the
architecture.

In the case of DM9000 if it fails to build on your platform then the
driver is just buggy - looking at the Kconfig I rather suspect that the
dependency on architectures should just be removed.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michał Mirosław April 18, 2011, 2:51 p.m. UTC | #3
On Mon, Apr 18, 2011 at 01:25:22PM +0100, Mark Brown wrote:
> On Mon, Apr 18, 2011 at 01:12:03PM +0200, Micha? Miros?aw wrote:
> > On Mon, Apr 18, 2011 at 12:04:37PM +0100, Mark Brown wrote:
> > > Commit c88fcb (net: dm9000: convert to hw_features) broke the build of
> > > the dm9000 driver since it merged functions which use different names
> > > for the board info structure used for I/O operations without updating
> > > all the references to use the same name. Fix that.
> > This brings the issue of build testing effectiveness. In current code
> > there is no configuration that makes all drivers build. I would like
> > to see something like 'make brokenconfig' that would allow most of
> > the code to be built, and not necessarily working. Maybe someone has
> > an idea how to implement that?
> For the drivers that genuinely are rather platform specific this tends
> to fail very badly as you need headers that only come along with the
> architecture.
> 
> In the case of DM9000 if it fails to build on your platform then the
> driver is just buggy - looking at the Kconfig I rather suspect that the
> dependency on architectures should just be removed.

I wonder if allyesconfig/allmodconfig is supposed to include code that's
known not to work for a particular architecture.

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Randy.Dunlap April 18, 2011, 2:55 p.m. UTC | #4
On Mon, 18 Apr 2011 16:51:02 +0200 Micha? Miros?aw wrote:

> On Mon, Apr 18, 2011 at 01:25:22PM +0100, Mark Brown wrote:
> > On Mon, Apr 18, 2011 at 01:12:03PM +0200, Micha? Miros?aw wrote:
> > > On Mon, Apr 18, 2011 at 12:04:37PM +0100, Mark Brown wrote:
> > > > Commit c88fcb (net: dm9000: convert to hw_features) broke the build of
> > > > the dm9000 driver since it merged functions which use different names
> > > > for the board info structure used for I/O operations without updating
> > > > all the references to use the same name. Fix that.
> > > This brings the issue of build testing effectiveness. In current code
> > > there is no configuration that makes all drivers build. I would like
> > > to see something like 'make brokenconfig' that would allow most of
> > > the code to be built, and not necessarily working. Maybe someone has
> > > an idea how to implement that?
> > For the drivers that genuinely are rather platform specific this tends
> > to fail very badly as you need headers that only come along with the
> > architecture.
> > 
> > In the case of DM9000 if it fails to build on your platform then the
> > driver is just buggy - looking at the Kconfig I rather suspect that the
> > dependency on architectures should just be removed.
> 
> I wonder if allyesconfig/allmodconfig is supposed to include code that's
> known not to work for a particular architecture.

all*config just use whatever is in all of the various Kconfig* files.
If they say "depends on $somearch", then so be it.  If not, then the
remaining dependencies are used.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michał Mirosław April 18, 2011, 3:17 p.m. UTC | #5
On Mon, Apr 18, 2011 at 07:55:08AM -0700, Randy Dunlap wrote:
> On Mon, 18 Apr 2011 16:51:02 +0200 Micha? Miros?aw wrote:
> 
> > On Mon, Apr 18, 2011 at 01:25:22PM +0100, Mark Brown wrote:
> > > On Mon, Apr 18, 2011 at 01:12:03PM +0200, Micha? Miros?aw wrote:
> > > > This brings the issue of build testing effectiveness. In current code
> > > > there is no configuration that makes all drivers build. I would like
> > > > to see something like 'make brokenconfig' that would allow most of
> > > > the code to be built, and not necessarily working. Maybe someone has
> > > > an idea how to implement that?
> > > For the drivers that genuinely are rather platform specific this tends
> > > to fail very badly as you need headers that only come along with the
> > > architecture.
> > > 
> > > In the case of DM9000 if it fails to build on your platform then the
> > > driver is just buggy - looking at the Kconfig I rather suspect that the
> > > dependency on architectures should just be removed.
> > I wonder if allyesconfig/allmodconfig is supposed to include code that's
> > known not to work for a particular architecture.
> all*config just use whatever is in all of the various Kconfig* files.
> If they say "depends on $somearch", then so be it.  If not, then the
> remaining dependencies are used.

Yes, I know how it works. I just wonder if removing those dependencies so
that all drivers (even if known not to work) are built on all*config
is acceptable. Or maybe there should be a config like 'disable all drivers
that are known to build but not to work on this arch'?

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Randy.Dunlap April 18, 2011, 3:28 p.m. UTC | #6
On Mon, 18 Apr 2011 17:17:29 +0200 Micha? Miros?aw wrote:

> On Mon, Apr 18, 2011 at 07:55:08AM -0700, Randy Dunlap wrote:
> > On Mon, 18 Apr 2011 16:51:02 +0200 Micha? Miros?aw wrote:
> > 
> > > On Mon, Apr 18, 2011 at 01:25:22PM +0100, Mark Brown wrote:
> > > > On Mon, Apr 18, 2011 at 01:12:03PM +0200, Micha? Miros?aw wrote:
> > > > > This brings the issue of build testing effectiveness. In current code
> > > > > there is no configuration that makes all drivers build. I would like
> > > > > to see something like 'make brokenconfig' that would allow most of
> > > > > the code to be built, and not necessarily working. Maybe someone has
> > > > > an idea how to implement that?
> > > > For the drivers that genuinely are rather platform specific this tends
> > > > to fail very badly as you need headers that only come along with the
> > > > architecture.
> > > > 
> > > > In the case of DM9000 if it fails to build on your platform then the
> > > > driver is just buggy - looking at the Kconfig I rather suspect that the
> > > > dependency on architectures should just be removed.
> > > I wonder if allyesconfig/allmodconfig is supposed to include code that's
> > > known not to work for a particular architecture.
> > all*config just use whatever is in all of the various Kconfig* files.
> > If they say "depends on $somearch", then so be it.  If not, then the
> > remaining dependencies are used.
> 
> Yes, I know how it works. I just wonder if removing those dependencies so
> that all drivers (even if known not to work) are built on all*config
> is acceptable. Or maybe there should be a config like 'disable all drivers
> that are known to build but not to work on this arch'?

AFAIK, it's always the case that we prefer not to have
	depends on $somearch
for drivers, but the reality is that lots of them do depend on $ARCH
for header files etc., so they are listed that way.  There's not much
that we can do about that.  I don't think that removing those dependencies
is acceptable.  OTOH, it may be acceptable to enable CONFIG_BROKEN so that
the drivers that depend on BROKEN can try to be built.


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown April 18, 2011, 4:07 p.m. UTC | #7
On Mon, Apr 18, 2011 at 04:51:02PM +0200, Micha? Miros?aw wrote:

> I wonder if allyesconfig/allmodconfig is supposed to include code that's
> known not to work for a particular architecture.

Well, it's certainly supposed to include things that aren't *useful*
which is more the point here.  all*config is clearly building a
configuration which isn't terribly useful for any particular machine.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller April 18, 2011, 9:19 p.m. UTC | #8
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
Date: Mon, 18 Apr 2011 12:04:37 +0100

> Commit c88fcb (net: dm9000: convert to hw_features) broke the build of
> the dm9000 driver since it merged functions which use different names
> for the board info structure used for I/O operations without updating
> all the references to use the same name. Fix that.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller April 18, 2011, 9:23 p.m. UTC | #9
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
Date: Mon, 18 Apr 2011 17:07:33 +0100

> On Mon, Apr 18, 2011 at 04:51:02PM +0200, Micha? Miros?aw wrote:
> 
>> I wonder if allyesconfig/allmodconfig is supposed to include code that's
>> known not to work for a particular architecture.
> 
> Well, it's certainly supposed to include things that aren't *useful*
> which is more the point here.  all*config is clearly building a
> configuration which isn't terribly useful for any particular machine.

Right.

To be honest, any driver submitted now which has CONFIG_ARCH
dependencies is totally crap and we shouldn't merge.

We have portable interfaces for every aspect of operation that a
driver might need to use, and for where that isn't the case we should
fix that.

Even architecture specific hypervisor interfaces should have NOP
versions defined when building on other systems.

Frankly, every single driver under drivers/ should be available to
build on any platform, and it should build cleanly.

So the drivers that aren't available to build on all platforms right
now are just bugs waiting to be fixed :)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/dm9000.c b/drivers/net/dm9000.c
index f7bdebb..fbaff35 100644
--- a/drivers/net/dm9000.c
+++ b/drivers/net/dm9000.c
@@ -768,7 +768,7 @@  dm9000_init_dm9000(struct net_device *dev)
 
 	/* Checksum mode */
 	if (dev->hw_features & NETIF_F_RXCSUM)
-		iow(dm, DM9000_RCSR,
+		iow(db, DM9000_RCSR,
 			(dev->features & NETIF_F_RXCSUM) ? RCSR_CSUM : 0);
 
 	iow(db, DM9000_GPCR, GPCR_GEP_CNTL);	/* Let GPIO0 output */