[PATCH] SSB: make it depend on PCI

[PATCH] SSB: make it depend on PCI

[Mike Frysinger]

Since the SSB code requires PCI, set the Kconfig dependencies accordingly.
Also tweak the menu display so that the menu does not show up at all if
SSB is not possible.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/ssb/Kconfig |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
index 307b1f6..f0a176a 100644
--- a/drivers/ssb/Kconfig
+++ b/drivers/ssb/Kconfig
@@ -1,10 +1,11 @@
-menu "Sonics Silicon Backplane"
-
 config SSB_POSSIBLE
 	bool
-	depends on HAS_IOMEM && HAS_DMA
+	depends on HAS_IOMEM && HAS_DMA && PCI
 	default y
 
+menu "Sonics Silicon Backplane"
+	depends on SSB_POSSIBLE
+
 config SSB
 	tristate "Sonics Silicon Backplane support"
 	depends on SSB_POSSIBLE
-- 
1.6.0.3

Re: [PATCH] SSB: make it depend on PCI

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 821 bytes --]

On Thu, 2008-11-06 at 04:51 -0500, Mike Frysinger wrote:
> Since the SSB code requires PCI, set the Kconfig dependencies accordingly.
> Also tweak the menu display so that the menu does not show up at all if
> SSB is not possible.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  drivers/ssb/Kconfig |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
> index 307b1f6..f0a176a 100644
> --- a/drivers/ssb/Kconfig
> +++ b/drivers/ssb/Kconfig
> @@ -1,10 +1,11 @@
> -menu "Sonics Silicon Backplane"
> -
>  config SSB_POSSIBLE
>  	bool
> -	depends on HAS_IOMEM && HAS_DMA
> +	depends on HAS_IOMEM && HAS_DMA && PCI

This is incorrect. SSB is also used on embedded systems that do not use
PCI at all.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] SSB: make it depend on PCI

[Mike Frysinger]

On Thu, Nov 6, 2008 at 06:07, Johannes Berg wrote:
> On Thu, 2008-11-06 at 04:51 -0500, Mike Frysinger wrote:
>> Since the SSB code requires PCI, set the Kconfig dependencies accordingly.
>> Also tweak the menu display so that the menu does not show up at all if
>> SSB is not possible.
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> ---
>>  drivers/ssb/Kconfig |    7 ++++---
>>  1 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
>> index 307b1f6..f0a176a 100644
>> --- a/drivers/ssb/Kconfig
>> +++ b/drivers/ssb/Kconfig
>> @@ -1,10 +1,11 @@
>> -menu "Sonics Silicon Backplane"
>> -
>>  config SSB_POSSIBLE
>>       bool
>> -     depends on HAS_IOMEM && HAS_DMA
>> +     depends on HAS_IOMEM && HAS_DMA && PCI
>
> This is incorrect. SSB is also used on embedded systems that do not use
> PCI at all.

while that may be the intent, it doesnt work today.

drivers/ssb/main.c always includes linux/ssb/ssb.h

linux/ssb/ssb.h always calls pci_dma_* functions

arches that do not support PCI fail to build:
  CC      drivers/ssb/main.o
In file included from drivers/ssb/ssb_private.h:5,
                 from drivers/ssb/main.c:12:
include/linux/ssb/ssb.h: In function 'ssb_dma_mapping_error':
include/linux/ssb/ssb.h:430: error: implicit declaration of function
'pci_dma_mapping_error'
include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_for_cpu':
include/linux/ssb/ssb.h:475: error: implicit declaration of function
'pci_dma_sync_single_for_cpu'
include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_for_device':
include/linux/ssb/ssb.h:493: error: implicit declaration of function
'pci_dma_sync_single_for_device'
include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_range_for_cpu':
include/linux/ssb/ssb.h:517: error: implicit declaration of function
'dma_sync_single_range_for_cpu'
include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_range_for_device':
include/linux/ssb/ssb.h:538: error: implicit declaration of function
'dma_sync_single_range_for_device'
drivers/ssb/main.c: In function 'ssb_dma_set_mask':
drivers/ssb/main.c:1183: error: implicit declaration of function 'dma_set_mask'
make[1]: *** [drivers/ssb/main.o] Error 1
make: *** [drivers/ssb/main.o] Error 2
-mike

Re: [PATCH] SSB: make it depend on PCI

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1678 bytes --]

On Thu, 2008-11-06 at 06:15 -0500, Mike Frysinger wrote:

> while that may be the intent, it doesnt work today.
> 
> drivers/ssb/main.c always includes linux/ssb/ssb.h
> 
> linux/ssb/ssb.h always calls pci_dma_* functions
> 
> arches that do not support PCI fail to build:
>   CC      drivers/ssb/main.o
> In file included from drivers/ssb/ssb_private.h:5,
>                  from drivers/ssb/main.c:12:
> include/linux/ssb/ssb.h: In function 'ssb_dma_mapping_error':
> include/linux/ssb/ssb.h:430: error: implicit declaration of function 'pci_dma_mapping_error'
> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_for_cpu':
> include/linux/ssb/ssb.h:475: error: implicit declaration of function 'pci_dma_sync_single_for_cpu'
> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_for_device':
> include/linux/ssb/ssb.h:493: error: implicit declaration of function 'pci_dma_sync_single_for_device'
> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_range_for_cpu':
> include/linux/ssb/ssb.h:517: error: implicit declaration of function 'dma_sync_single_range_for_cpu'
> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_range_for_device':
> include/linux/ssb/ssb.h:538: error: implicit declaration of function 'dma_sync_single_range_for_device'
> drivers/ssb/main.c: In function 'ssb_dma_set_mask':
> drivers/ssb/main.c:1183: error: implicit declaration of function 'dma_set_mask'


That seems odd, shouldn't at least the dma_sync_* functions and
dma_set_mask be supported?

There probably need to be ifdefs, if you look at those functions then
you'll see that it depends on the bustype the device is on.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] SSB: make it depend on PCI

[Mike Frysinger]

On Thu, Nov 6, 2008 at 06:22, Johannes Berg wrote:
> On Thu, 2008-11-06 at 06:15 -0500, Mike Frysinger wrote:
>> while that may be the intent, it doesnt work today.
>>
>> drivers/ssb/main.c always includes linux/ssb/ssb.h
>>
>> linux/ssb/ssb.h always calls pci_dma_* functions
>>
>> arches that do not support PCI fail to build:
>>   CC      drivers/ssb/main.o
>> In file included from drivers/ssb/ssb_private.h:5,
>>                  from drivers/ssb/main.c:12:
>> include/linux/ssb/ssb.h: In function 'ssb_dma_mapping_error':
>> include/linux/ssb/ssb.h:430: error: implicit declaration of function 'pci_dma_mapping_error'
>> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_for_cpu':
>> include/linux/ssb/ssb.h:475: error: implicit declaration of function 'pci_dma_sync_single_for_cpu'
>> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_for_device':
>> include/linux/ssb/ssb.h:493: error: implicit declaration of function 'pci_dma_sync_single_for_device'
>> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_range_for_cpu':
>> include/linux/ssb/ssb.h:517: error: implicit declaration of function 'dma_sync_single_range_for_cpu'
>> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_range_for_device':
>> include/linux/ssb/ssb.h:538: error: implicit declaration of function 'dma_sync_single_range_for_device'
>> drivers/ssb/main.c: In function 'ssb_dma_set_mask':
>> drivers/ssb/main.c:1183: error: implicit declaration of function 'dma_set_mask'
>
>
> That seems odd, shouldn't at least the dma_sync_* functions and
> dma_set_mask be supported?

i'm highlighting the pci-specific functions.  the common dma functions
missing are not a ssb problem, so ignore those ;).
-mike

Re: [PATCH] SSB: make it depend on PCI

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 2100 bytes --]

On Thu, 2008-11-06 at 06:26 -0500, Mike Frysinger wrote:
> On Thu, Nov 6, 2008 at 06:22, Johannes Berg wrote:
> > On Thu, 2008-11-06 at 06:15 -0500, Mike Frysinger wrote:
> >> while that may be the intent, it doesnt work today.
> >>
> >> drivers/ssb/main.c always includes linux/ssb/ssb.h
> >>
> >> linux/ssb/ssb.h always calls pci_dma_* functions
> >>
> >> arches that do not support PCI fail to build:
> >>   CC      drivers/ssb/main.o
> >> In file included from drivers/ssb/ssb_private.h:5,
> >>                  from drivers/ssb/main.c:12:
> >> include/linux/ssb/ssb.h: In function 'ssb_dma_mapping_error':
> >> include/linux/ssb/ssb.h:430: error: implicit declaration of function 'pci_dma_mapping_error'
> >> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_for_cpu':
> >> include/linux/ssb/ssb.h:475: error: implicit declaration of function 'pci_dma_sync_single_for_cpu'
> >> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_for_device':
> >> include/linux/ssb/ssb.h:493: error: implicit declaration of function 'pci_dma_sync_single_for_device'
> >> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_range_for_cpu':
> >> include/linux/ssb/ssb.h:517: error: implicit declaration of function 'dma_sync_single_range_for_cpu'
> >> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_range_for_device':
> >> include/linux/ssb/ssb.h:538: error: implicit declaration of function 'dma_sync_single_range_for_device'
> >> drivers/ssb/main.c: In function 'ssb_dma_set_mask':
> >> drivers/ssb/main.c:1183: error: implicit declaration of function 'dma_set_mask'
> >
> >
> > That seems odd, shouldn't at least the dma_sync_* functions and
> > dma_set_mask be supported?
> 
> i'm highlighting the pci-specific functions.  the common dma functions
> missing are not a ssb problem, so ignore those ;).

Alright, but even then you'll fail build if you enable SSB ;) I guess we
need ifdefs or mandate that non-PCI architectures have dummy stubs (like
other architectures have in the !CONFIG_PCI case), but Michael's the SSB
expert.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] SSB: make it depend on PCI

[Mike Frysinger]

On Thu, Nov 6, 2008 at 06:29, Johannes Berg wrote:
> On Thu, 2008-11-06 at 06:26 -0500, Mike Frysinger wrote:
>> On Thu, Nov 6, 2008 at 06:22, Johannes Berg wrote:
>> > On Thu, 2008-11-06 at 06:15 -0500, Mike Frysinger wrote:
>> >> while that may be the intent, it doesnt work today.
>> >>
>> >> drivers/ssb/main.c always includes linux/ssb/ssb.h
>> >>
>> >> linux/ssb/ssb.h always calls pci_dma_* functions
>> >>
>> >> arches that do not support PCI fail to build:
>> >>   CC      drivers/ssb/main.o
>> >> In file included from drivers/ssb/ssb_private.h:5,
>> >>                  from drivers/ssb/main.c:12:
>> >> include/linux/ssb/ssb.h: In function 'ssb_dma_mapping_error':
>> >> include/linux/ssb/ssb.h:430: error: implicit declaration of function 'pci_dma_mapping_error'
>> >> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_for_cpu':
>> >> include/linux/ssb/ssb.h:475: error: implicit declaration of function 'pci_dma_sync_single_for_cpu'
>> >> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_for_device':
>> >> include/linux/ssb/ssb.h:493: error: implicit declaration of function 'pci_dma_sync_single_for_device'
>> >> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_range_for_cpu':
>> >> include/linux/ssb/ssb.h:517: error: implicit declaration of function 'dma_sync_single_range_for_cpu'
>> >> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_range_for_device':
>> >> include/linux/ssb/ssb.h:538: error: implicit declaration of function 'dma_sync_single_range_for_device'
>> >> drivers/ssb/main.c: In function 'ssb_dma_set_mask':
>> >> drivers/ssb/main.c:1183: error: implicit declaration of function 'dma_set_mask'
>> >
>> >
>> > That seems odd, shouldn't at least the dma_sync_* functions and
>> > dma_set_mask be supported?
>>
>> i'm highlighting the pci-specific functions.  the common dma functions
>> missing are not a ssb problem, so ignore those ;).
>
> Alright, but even then you'll fail build if you enable SSB ;)

right ... but they're certainly nothing for you to worry about

> I guess we
> need ifdefs or mandate that non-PCI architectures have dummy stubs (like
> other architectures have in the !CONFIG_PCI case), but Michael's the SSB
> expert.

OK.  regarding the patch, if we drop the "&& PCI" part, i think the
rest should get merged ... having empty menus in kconfig is annoying.
agreed ?  i can send out another one ...
-mike

Re: [PATCH] SSB: make it depend on PCI

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 485 bytes --]

On Thu, 2008-11-06 at 06:32 -0500, Mike Frysinger wrote:

> > I guess we
> > need ifdefs or mandate that non-PCI architectures have dummy stubs (like
> > other architectures have in the !CONFIG_PCI case), but Michael's the SSB
> > expert.
> 
> OK.  regarding the patch, if we drop the "&& PCI" part, i think the
> rest should get merged ... having empty menus in kconfig is annoying.
> agreed ?  i can send out another one ...

Yeah that part looks fine to me.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] SSB: make it depend on PCI

[Michael Buesch]

On Thursday 06 November 2008 12:32:50 Mike Frysinger wrote:
> OK.  regarding the patch, if we drop the "&& PCI" part, i think the
> rest should get merged ... having empty menus in kconfig is annoying.
> agreed ?

We have these all over the place.
SSB_POSSIBLE is false for virtually nobody (only for some weird architectures).
The the menu will be there in almost any case.

-- 
Greetings Michael.

Re: [PATCH] SSB: make it depend on PCI

[Mike Frysinger]

On Thu, Nov 6, 2008 at 11:59, Michael Buesch wrote:
> On Thursday 06 November 2008 12:32:50 Mike Frysinger wrote:
>> OK.  regarding the patch, if we drop the "&& PCI" part, i think the
>> rest should get merged ... having empty menus in kconfig is annoying.
>> agreed ?
>
> We have these all over the place.

that's a weak argument.  empty menus should go away, not remain
"because other people are doing it too".  with that mentality, there's
many issues we wouldnt be able to address.
-mike

Re: [PATCH] SSB: make it depend on PCI

[Michael Buesch]

On Thursday 06 November 2008 22:14:30 Mike Frysinger wrote:
> On Thu, Nov 6, 2008 at 11:59, Michael Buesch wrote:
> > On Thursday 06 November 2008 12:32:50 Mike Frysinger wrote:
> >> OK.  regarding the patch, if we drop the "&& PCI" part, i think the
> >> rest should get merged ... having empty menus in kconfig is annoying.
> >> agreed ?
> >
> > We have these all over the place.
> 
> that's a weak argument.  empty menus should go away, not remain
> "because other people are doing it too".  with that mentality, there's
> many issues we wouldnt be able to address.

Well, I don't see this as an issue, as 99.9% of the people don't hit that
case and if they do it doesn't matter anyway.
But hey, if you want to apply this, feel free to do so.

-- 
Greetings Michael.

Re: [PATCH] SSB: make it depend on PCI

[Michael Buesch]

On Thursday 06 November 2008 12:15:24 Mike Frysinger wrote:
> linux/ssb/ssb.h always calls pci_dma_* functions

No it doesn't. Read the code.
pci_dma_xxx functions should probably declared to no-op inlines in
the pci.h header.
However, we can also ifdef them in ssb.h.

-- 
Greetings Michael.

Re: [PATCH] SSB: make it depend on PCI

[Geert Uytterhoeven]

On Thu, 6 Nov 2008, Mike Frysinger wrote:
> On Thu, Nov 6, 2008 at 06:07, Johannes Berg wrote:
> > On Thu, 2008-11-06 at 04:51 -0500, Mike Frysinger wrote:
> >> Since the SSB code requires PCI, set the Kconfig dependencies accordingly.
> >> Also tweak the menu display so that the menu does not show up at all if
> >> SSB is not possible.
> >>
> >> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> >> ---
> >>  drivers/ssb/Kconfig |    7 ++++---
> >>  1 files changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
> >> index 307b1f6..f0a176a 100644
> >> --- a/drivers/ssb/Kconfig
> >> +++ b/drivers/ssb/Kconfig
> >> @@ -1,10 +1,11 @@
> >> -menu "Sonics Silicon Backplane"
> >> -
> >>  config SSB_POSSIBLE
> >>       bool
> >> -     depends on HAS_IOMEM && HAS_DMA
> >> +     depends on HAS_IOMEM && HAS_DMA && PCI
> >
> > This is incorrect. SSB is also used on embedded systems that do not use
> > PCI at all.
> 
> while that may be the intent, it doesnt work today.
> 
> drivers/ssb/main.c always includes linux/ssb/ssb.h
> 
> linux/ssb/ssb.h always calls pci_dma_* functions
> 
> arches that do not support PCI fail to build:
>   CC      drivers/ssb/main.o
> In file included from drivers/ssb/ssb_private.h:5,
>                  from drivers/ssb/main.c:12:
> include/linux/ssb/ssb.h: In function 'ssb_dma_mapping_error':
> include/linux/ssb/ssb.h:430: error: implicit declaration of function
> 'pci_dma_mapping_error'
> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_for_cpu':
> include/linux/ssb/ssb.h:475: error: implicit declaration of function
> 'pci_dma_sync_single_for_cpu'
> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_for_device':
> include/linux/ssb/ssb.h:493: error: implicit declaration of function
> 'pci_dma_sync_single_for_device'
> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_range_for_cpu':
> include/linux/ssb/ssb.h:517: error: implicit declaration of function
> 'dma_sync_single_range_for_cpu'
> include/linux/ssb/ssb.h: In function 'ssb_dma_sync_single_range_for_device':
> include/linux/ssb/ssb.h:538: error: implicit declaration of function
> 'dma_sync_single_range_for_device'
> drivers/ssb/main.c: In function 'ssb_dma_set_mask':
> drivers/ssb/main.c:1183: error: implicit declaration of function 'dma_set_mask'
> make[1]: *** [drivers/ssb/main.o] Error 1
> make: *** [drivers/ssb/main.o] Error 2

I had the same issue on m68k, not only with SSB but also with other legacy
drivers (hp100 Ethernet, AIC7xxx SCSI). It was fixed in commit
998aaf01c6f6f3dffc1ea9e7b20b131e38fdbc78 ("m68k: <asm/pci.h> needs
<asm-generic/pci-dma-compat.h>").

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] SSB: make it depend on PCI

[Michael Buesch]

On Sunday 09 November 2008 11:51:37 Geert Uytterhoeven wrote:
> I had the same issue on m68k, not only with SSB but also with other legacy
> drivers (hp100 Ethernet, AIC7xxx SCSI). It was fixed in commit
> 998aaf01c6f6f3dffc1ea9e7b20b131e38fdbc78 ("m68k: <asm/pci.h> needs
> <asm-generic/pci-dma-compat.h>").

Ok, nice.
I'd suggest we additionally fix the architectures this way.

My patch already is on its way upstream and I think that's OK.
The codepaths that it #ifdefs out are dead code on non-PCI, so I think
it's OK to #ifdef them.

-- 
Greetings Michael.

Re: [PATCH] SSB: make it depend on PCI

[Michael Buesch]

On Thursday 06 November 2008 10:51:22 Mike Frysinger wrote:
> Since the SSB code requires PCI,

Ehm, no. it doesn't.

-- 
Greetings Michael.