Patchwork [01/21] NTB: correct missing readq/writeq errors

login
register
mail settings
Submitter Jon Mason
Date Jan. 19, 2013, 9:02 a.m.
Message ID <1358586155-23322-2-git-send-email-jon.mason@intel.com>
Download mbox | patch
Permalink /patch/213802/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Jon Mason - Jan. 19, 2013, 9:02 a.m.
Atomic readq and writeq do not exist by default on some 32bit
architectures, thus causing compile errors due to non-existent symbols.
In those cases, use the definitions of those symbols from
include/asm-generic/io-64-nonatomic-hi-lo.h

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c |    1 +
 1 file changed, 1 insertion(+)
Greg KH - Jan. 20, 2013, 11:40 p.m.
On Sat, Jan 19, 2013 at 02:02:15AM -0700, Jon Mason wrote:
> Atomic readq and writeq do not exist by default on some 32bit
> architectures, thus causing compile errors due to non-existent symbols.
> In those cases, use the definitions of those symbols from
> include/asm-generic/io-64-nonatomic-hi-lo.h
> 
> Signed-off-by: Jon Mason <jon.mason@intel.com>
> ---
>  drivers/ntb/ntb_hw.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 4c71b17..0b46fef 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -45,6 +45,7 @@
>   * Contact Information:
>   * Jon Mason <jon.mason@intel.com>
>   */
> +#include <asm-generic/io-64-nonatomic-hi-lo.h>

Really?  This seems really odd.  Usually we just don't build the code
for any platform that doesn't have readq/writeq.  Otherwise, shouldn't
those arches just include this file themselves to keep everything
working?  Individual drivers shouldn't have to do this.

So, I'm not going to take this one just yet, sorry.

greg k-h
--
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
Jon Mason - Jan. 21, 2013, 5:38 p.m.
On Sun, Jan 20, 2013 at 03:40:05PM -0800, Greg KH wrote:
> On Sat, Jan 19, 2013 at 02:02:15AM -0700, Jon Mason wrote:
> > Atomic readq and writeq do not exist by default on some 32bit
> > architectures, thus causing compile errors due to non-existent symbols.
> > In those cases, use the definitions of those symbols from
> > include/asm-generic/io-64-nonatomic-hi-lo.h
> > 
> > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > ---
> >  drivers/ntb/ntb_hw.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > index 4c71b17..0b46fef 100644
> > --- a/drivers/ntb/ntb_hw.c
> > +++ b/drivers/ntb/ntb_hw.c
> > @@ -45,6 +45,7 @@
> >   * Contact Information:
> >   * Jon Mason <jon.mason@intel.com>
> >   */
> > +#include <asm-generic/io-64-nonatomic-hi-lo.h>
> 
> Really?  This seems really odd.  Usually we just don't build the code
> for any platform that doesn't have readq/writeq.  Otherwise, shouldn't
> those arches just include this file themselves to keep everything
> working?  Individual drivers shouldn't have to do this.

There is no readq/writeq for 32bit x86.  I was using this header file
to get around that.  I freely admit that I have done 0 testing on
32bit, but it should work.  If you prefer I modify the Kconfig to only
enable this for x86_64 until such time as I test on x86_32, I will
happily submit that patch.

Thanks,
Jon

> 
> So, I'm not going to take this one just yet, sorry.
> 
> greg k-h
--
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
Greg KH - Jan. 21, 2013, 6:23 p.m.
On Mon, Jan 21, 2013 at 10:38:18AM -0700, Jon Mason wrote:
> On Sun, Jan 20, 2013 at 03:40:05PM -0800, Greg KH wrote:
> > On Sat, Jan 19, 2013 at 02:02:15AM -0700, Jon Mason wrote:
> > > Atomic readq and writeq do not exist by default on some 32bit
> > > architectures, thus causing compile errors due to non-existent symbols.
> > > In those cases, use the definitions of those symbols from
> > > include/asm-generic/io-64-nonatomic-hi-lo.h
> > > 
> > > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > > ---
> > >  drivers/ntb/ntb_hw.c |    1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > > index 4c71b17..0b46fef 100644
> > > --- a/drivers/ntb/ntb_hw.c
> > > +++ b/drivers/ntb/ntb_hw.c
> > > @@ -45,6 +45,7 @@
> > >   * Contact Information:
> > >   * Jon Mason <jon.mason@intel.com>
> > >   */
> > > +#include <asm-generic/io-64-nonatomic-hi-lo.h>
> > 
> > Really?  This seems really odd.  Usually we just don't build the code
> > for any platform that doesn't have readq/writeq.  Otherwise, shouldn't
> > those arches just include this file themselves to keep everything
> > working?  Individual drivers shouldn't have to do this.
> 
> There is no readq/writeq for 32bit x86.  I was using this header file
> to get around that.  I freely admit that I have done 0 testing on
> 32bit, but it should work.  If you prefer I modify the Kconfig to only
> enable this for x86_64 until such time as I test on x86_32, I will
> happily submit that patch.

No, other drivers on 32bit x86 also use readq, and do not include this
asm header file.  So there is a proper way to do this, just dig through
the header file include mess to determine the proper one to include.
You should never have to include asm-generic file directly, that's the
issue here.

thanks,

greg k-h
--
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
Ben Hutchings - Jan. 21, 2013, 8:34 p.m.
On Mon, 2013-01-21 at 10:23 -0800, Greg KH wrote:
> On Mon, Jan 21, 2013 at 10:38:18AM -0700, Jon Mason wrote:
> > On Sun, Jan 20, 2013 at 03:40:05PM -0800, Greg KH wrote:
> > > On Sat, Jan 19, 2013 at 02:02:15AM -0700, Jon Mason wrote:
> > > > Atomic readq and writeq do not exist by default on some 32bit
> > > > architectures, thus causing compile errors due to non-existent symbols.
> > > > In those cases, use the definitions of those symbols from
> > > > include/asm-generic/io-64-nonatomic-hi-lo.h
> > > > 
> > > > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > > > ---
> > > >  drivers/ntb/ntb_hw.c |    1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > > > index 4c71b17..0b46fef 100644
> > > > --- a/drivers/ntb/ntb_hw.c
> > > > +++ b/drivers/ntb/ntb_hw.c
> > > > @@ -45,6 +45,7 @@
> > > >   * Contact Information:
> > > >   * Jon Mason <jon.mason@intel.com>
> > > >   */
> > > > +#include <asm-generic/io-64-nonatomic-hi-lo.h>
> > > 
> > > Really?  This seems really odd.  Usually we just don't build the code
> > > for any platform that doesn't have readq/writeq.  Otherwise, shouldn't
> > > those arches just include this file themselves to keep everything
> > > working?  Individual drivers shouldn't have to do this.
> > 
> > There is no readq/writeq for 32bit x86.  I was using this header file
> > to get around that.  I freely admit that I have done 0 testing on
> > 32bit, but it should work.  If you prefer I modify the Kconfig to only
> > enable this for x86_64 until such time as I test on x86_32, I will
> > happily submit that patch.
> 
> No, other drivers on 32bit x86 also use readq,

I don't think so, because it's not defined for CONFIG_X86_32.

> and do not include this
> asm header file.  So there is a proper way to do this, just dig through
> the header file include mess to determine the proper one to include.
> You should never have to include asm-generic file directly, that's the
> issue here.

Different devices can have different requirements for the order of
access to 32-bit halves of a 64-bit register.  Only the driver knows
which is right.

Ben.
Greg KH - Jan. 21, 2013, 8:47 p.m.
On Mon, Jan 21, 2013 at 08:34:53PM +0000, Ben Hutchings wrote:
> On Mon, 2013-01-21 at 10:23 -0800, Greg KH wrote:
> > On Mon, Jan 21, 2013 at 10:38:18AM -0700, Jon Mason wrote:
> > > On Sun, Jan 20, 2013 at 03:40:05PM -0800, Greg KH wrote:
> > > > On Sat, Jan 19, 2013 at 02:02:15AM -0700, Jon Mason wrote:
> > > > > Atomic readq and writeq do not exist by default on some 32bit
> > > > > architectures, thus causing compile errors due to non-existent symbols.
> > > > > In those cases, use the definitions of those symbols from
> > > > > include/asm-generic/io-64-nonatomic-hi-lo.h
> > > > > 
> > > > > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > > > > ---
> > > > >  drivers/ntb/ntb_hw.c |    1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > > > > index 4c71b17..0b46fef 100644
> > > > > --- a/drivers/ntb/ntb_hw.c
> > > > > +++ b/drivers/ntb/ntb_hw.c
> > > > > @@ -45,6 +45,7 @@
> > > > >   * Contact Information:
> > > > >   * Jon Mason <jon.mason@intel.com>
> > > > >   */
> > > > > +#include <asm-generic/io-64-nonatomic-hi-lo.h>
> > > > 
> > > > Really?  This seems really odd.  Usually we just don't build the code
> > > > for any platform that doesn't have readq/writeq.  Otherwise, shouldn't
> > > > those arches just include this file themselves to keep everything
> > > > working?  Individual drivers shouldn't have to do this.
> > > 
> > > There is no readq/writeq for 32bit x86.  I was using this header file
> > > to get around that.  I freely admit that I have done 0 testing on
> > > 32bit, but it should work.  If you prefer I modify the Kconfig to only
> > > enable this for x86_64 until such time as I test on x86_32, I will
> > > happily submit that patch.
> > 
> > No, other drivers on 32bit x86 also use readq,
> 
> I don't think so, because it's not defined for CONFIG_X86_32.

Ah, you are right, I was confused by drivers/char/hpet.c, which used
readq, but only on 64bit processors.

Jon, as you say you never tested this on 32bit processors, I'd recommend
just making the Kconfig file enforce that and only build on x86-64 for
now.

thanks,

greg k-h
--
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

Patch

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 4c71b17..0b46fef 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -45,6 +45,7 @@ 
  * Contact Information:
  * Jon Mason <jon.mason@intel.com>
  */
+#include <asm-generic/io-64-nonatomic-hi-lo.h>
 #include <linux/debugfs.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>