update: results of some syntactical checks

kconfig In review Martin Walch 3 participants 7 messages Oct 19, 2013 – Jun 16, 2014

Subsystems (2)

Media (V4L/DVB) Siano DVB driver

lore

Share & tools

lore

Martin Walch

author

2013-10-19 22:03 UTC #0 lore raw

This is an update to the syntactic results that I sent, back in July.

With kernel 3.12 nearing completion, I would like to point to new sections in Kconfig files with potential problems:

drivers/media/common/siano/Kconfig:21-26

Quoted 6 lines

config SMS_SIANO_DEBUGFS
bool "Enable debugfs for smsdvb"
depends on SMS_SIANO_MDTV
depends on DEBUG_FS
depends on SMS_USB_DRV
depends on CONFIG_SMS_USB_DRV = CONFIG_SMS_SDIO_DRV

The last line adds the dependency CONFIG_SMS_USB_DRV = CONFIG_SMS_SDIO_DRV. This expression does not look sound as those two symbols are not declared anywhere. So, the two strings CONFIG_SMS_USB_DRV and CONFIG_SMS_SDIO_DRV are compared, yielding always 'n'. As a result, SMS_SIANO_DEBUGFS will never be enabled.

Probably, it was meant to say something like

Quoted 1 line

depends on SMS_USB_DRV = SMS_SDIO_DRV

Two other config sections that probably behave differently than expected:

drivers/staging/rtl8188eu/Kconfig: 13-15

Quoted 3 lines

config 88EU_AP_MODE
bool "Realtek RTL8188EU AP mode"
default Y

drivers/staging/rtl8188eu/Kconfig: 21-23

Quoted 3 lines

config 88EU_P2P
bool "Realtek RTL8188EU Peer-to-peer mode" default Y

The capital Y is different from the lowercase y. While y is an actually hard coded constant symbol, Y is undeclared and evaluates to n. The default values are probably only for convenience, so 88EU_AP_MODE and 88EU_P2P are activated together with 8188EU. They still can be turned off. Anyway, it should probably say "default y" in both cases.

Here the updated full two lists from July:

Actually declared symbols with names of constants: 2
8260 at
arch/powerpc/platforms/82xx/Kconfig:55
8272 at
arch/powerpc/platforms/82xx/Kconfig:64

Symbols used, but not declared: 34
Symbol: ARCH_ARM at
sound/soc/omap/Kconfig:28
sound/soc/omap/Kconfig:2
Symbol: ARCH_EFM32 at

    drivers/spi/Kconfig:167
    drivers/tty/serial/Kconfig:1430
  Symbol: ARCH_HI3xxx at
    drivers/dma/Kconfig:313
    arch/arm/Kconfig.debug:184
    arch/arm/Kconfig.debug:192
  Symbol: ARCH_MOXART at
    drivers/net/ethernet/moxa/Kconfig:22
    drivers/net/ethernet/moxa/Kconfig:7
  Symbol: ARCH_MULTI_V4 at
    arch/arm/Kconfig:916
  Symbol: ATHEROS_AR231X at
    drivers/net/wireless/ath/ath5k/Kconfig:56
    drivers/net/wireless/ath/ath5k/Kconfig:63
    drivers/net/wireless/ath/ath5k/Kconfig:9
    drivers/net/wireless/ath/ath5k/Kconfig:8
    drivers/net/wireless/ath/ath5k/Kconfig:2
  Symbol: CONFIG_SMS_SDIO_DRV at
    drivers/media/common/siano/Kconfig:25
  Symbol: CONFIG_SMS_USB_DRV at
    drivers/media/common/siano/Kconfig:25
  Symbol: CPU_MMP3 at
    drivers/video/mmp/hw/Kconfig:4
    drivers/usb/phy/Kconfig:44
    drivers/video/mmp/Kconfig:2
  Symbol: CPU_PXA988 at
    drivers/video/mmp/hw/Kconfig:4
    drivers/video/mmp/Kconfig:2
  Symbol: CPU_SUBTYPE_SH7764 at
    arch/sh/drivers/dma/Kconfig:14
  Symbol: DEPRECATED at
    arch/mn10300/Kconfig.debug:34
  Symbol: EXYNOS_DEV_SYSMMU at
    drivers/iommu/Kconfig:180
  Symbol: GENERIC_HAS_IOMAP at
    arch/score/Kconfig:39
    arch/score/Kconfig:27
    arch/score/Kconfig:33
  Symbol: GENERIC_TIME at
    arch/arm/mach-bcm/Kconfig:11
  Symbol: GPIO_BCM at
    arch/arm/mach-bcm/Kconfig:12
  Symbol: HAVE_GENERIC_HARDIRQS at
    arch/score/Kconfig:5
  Symbol: LOCAL_TIMERS at
    arch/arm/mach-shmobile/Kconfig:6
    arch/arm/mach-omap2/Kconfig:59
    arch/arm/mach-rockchip/Kconfig:10
    arch/arm/mach-rockchip/Kconfig:7
  Symbol: M at
    drivers/usb/host/Kconfig:547
    drivers/usb/misc/Kconfig:130
  Symbol: MACH_NOKIA_RM696 at
    arch/arm/mach-omap2/Kconfig:321
  Symbol: MACH_OMAP_H4_OTG at
    drivers/usb/gadget/Kconfig:207
  Symbol: MACH_SMDKC210 at
    sound/soc/samsung/Kconfig:138
  Symbol: MACH_SMDKV310 at
    sound/soc/samsung/Kconfig:138
  Symbol: MN10300_PROC_MN2WS0038 at
    arch/mn10300/Kconfig:184
  Symbol: MPILIB_EXTRA at
    crypto/asymmetric_keys/Kconfig:24
  Symbol: MV64360 at
    arch/powerpc/Kconfig:419
  Symbol: N at
    drivers/staging/rtl8187se/Kconfig:8
    drivers/staging/bcm/Kconfig:3
    drivers/staging/usbip/Kconfig:31
    arch/cris/arch-v32/drivers/Kconfig:13
    drivers/staging/rtl8712/Kconfig:14
    drivers/usb/host/Kconfig:340
    drivers/staging/rtl8712/Kconfig:6
    drivers/staging/android/Kconfig:61
    drivers/staging/usbip/Kconfig:42
    drivers/staging/usbip/Kconfig:20
    arch/cris/arch-v32/drivers/Kconfig:118
    drivers/usb/host/Kconfig:317
    drivers/staging/android/Kconfig:4
    drivers/staging/frontier/Kconfig:3
    drivers/usb/host/Kconfig:329
    drivers/staging/rtl8192u/Kconfig:7
    arch/arc/Kconfig:356
    drivers/staging/media/go7007/Kconfig:48
    drivers/staging/media/go7007/Kconfig:15
    drivers/usb/core/Kconfig:12
    drivers/staging/rtl8192e/rtl8192e/Kconfig:7
    drivers/staging/usbip/Kconfig:3
    drivers/staging/media/go7007/Kconfig:26
    drivers/staging/rtl8188eu/Kconfig:5
  Symbol: OF_I2C at
    drivers/i2c/busses/Kconfig:377
  Symbol: OMAP_PM_SRF at
    drivers/staging/tidspbridge/Kconfig:19
  Symbol: PICOXCELL_PC3X3 at
    drivers/char/hw_random/Kconfig:242
  Symbol: PLATFORM_MICROBLAZE_AUTO at
    arch/microblaze/platform/Kconfig.platform:10
  Symbol: PLAT_SPEAR_SINGLE at
    arch/arm/mach-spear/Kconfig:84
    arch/arm/mach-spear/Kconfig:99
    arch/arm/mach-spear/Kconfig:6
    arch/arm/mach-spear/Kconfig:51
    arch/arm/mach-spear/Kconfig:19
  Symbol: RCAR_CLK_ADG at
    sound/soc/sh/Kconfig:40
  Symbol: Y at
    drivers/staging/rtl8188eu/Kconfig:14
    drivers/staging/rtl8188eu/Kconfig:22

Paul Bolle

2013-11-02 19:20 UTC #1 lore raw

Quoted 8 lines · Martin Walch

drivers/media/common/siano/Kconfig:21-26
Quoted 6 lines
config SMS_SIANO_DEBUGFS
bool "Enable debugfs for smsdvb"
depends on SMS_SIANO_MDTV
depends on DEBUG_FS
depends on SMS_USB_DRV
depends on CONFIG_SMS_USB_DRV = CONFIG_SMS_SDIO_DRV

Quoted 5 lines

The last line adds the dependency CONFIG_SMS_USB_DRV = CONFIG_SMS_SDIO_DRV. This expression does not look sound as those two symbols are not declared anywhere. So, the two strings CONFIG_SMS_USB_DRV and CONFIG_SMS_SDIO_DRV are compared, yielding always 'n'. As a result, SMS_SIANO_DEBUGFS will never be enabled.

Those are obvious typos. Still present in v3.12-rc7. Perhaps you'd like to send the trivial patch to fix this?

Quoted 15 lines

Probably, it was meant to say something like
Quoted 1 line
depends on SMS_USB_DRV = SMS_SDIO_DRV
Two other config sections that probably behave differently than expected:
drivers/staging/rtl8188eu/Kconfig: 13-15
Quoted 3 lines
config 88EU_AP_MODE
bool "Realtek RTL8188EU AP mode"
default Y
drivers/staging/rtl8188eu/Kconfig: 21-23
Quoted 3 lines
config 88EU_P2P
bool "Realtek RTL8188EU Peer-to-peer mode" default Y

Quoted 5 lines

The capital Y is different from the lowercase y. While y is an actually hard coded constant symbol, Y is undeclared and evaluates to n. The default values are probably only for convenience, so 88EU_AP_MODE and 88EU_P2P are activated together with 8188EU. They still can be turned off. Anyway, it should probably say "default y" in both cases.

Ditto.

Paul Bolle

Mauro Carvalho Chehab

2013-11-02 19:40 UTC #2 lore raw

Em Sat, 02 Nov 2013 20:20:54 +0100
Paul Bolle <[email protected]> escreveu:

Quoted 9 lines · Martin Walch

Quoted 8 lines
drivers/media/common/siano/Kconfig:21-26
Quoted 6 lines
config SMS_SIANO_DEBUGFS
bool "Enable debugfs for smsdvb"
depends on SMS_SIANO_MDTV
depends on DEBUG_FS
depends on SMS_USB_DRV
depends on CONFIG_SMS_USB_DRV = CONFIG_SMS_SDIO_DRV

Quoted 8 lines

Quoted 5 lines
The last line adds the dependency CONFIG_SMS_USB_DRV = CONFIG_SMS_SDIO_DRV. This expression does not look sound as those two symbols are not declared anywhere. So, the two strings CONFIG_SMS_USB_DRV and CONFIG_SMS_SDIO_DRV are compared, yielding always 'n'. As a result, SMS_SIANO_DEBUGFS will never be enabled.
Those are obvious typos. Still present in v3.12-rc7. Perhaps you'd like to send the trivial patch to fix this?

Yes, it is a typo...

Quoted 3 lines

Quoted 3 lines
Probably, it was meant to say something like
Quoted 1 line
depends on SMS_USB_DRV = SMS_SDIO_DRV

But this is not the right thing to do. The Kconfig logic here is that it should depends on !SMS_SDIO_DRV or SMS_USB_DRV = SMS_SDIO_DRV.

I remember I made a patch like that while testing some things with this driver, but it seems that I forgot to push. I might have it somewhere on my test machine.

Signature · 2 lines

Cheers,
Mauro

Paul Bolle

2013-11-02 19:55 UTC #3 lore raw

Quoted 5 lines · Mauro Carvalho Chehab

Em Sat, 02 Nov 2013 20:20:54 +0100
Paul Bolle <[email protected]> escreveu:
Quoted 2 lines
Those are obvious typos. Still present in v3.12-rc7. Perhaps you'd like to send the trivial patch to fix this?

Quoted 7 lines

Yes, it is a typo...
Quoted 2 lines
Quoted 2 lines
Probably, it was meant to say something like
Quoted 1 line
depends on SMS_USB_DRV = SMS_SDIO_DRV
But this is not the right thing to do. The Kconfig logic here is that it should depends on !SMS_SDIO_DRV or SMS_USB_DRV = SMS_SDIO_DRV.

For the record: my reason for suggesting to Martin to send patches is that sending a (trivial) patch often turns out to be a quick way to get issues like that resolved. Maintainers seem to react quite quickly on patches. Perhaps even faster if a patch is not solving the issue at hand entirely correct! Messages like Martin's, that basically state that there are one or more tree-wide problems, seem to be overlooked easily.

Quoted 3 lines

I remember I made a patch like that while testing some things with this driver, but it seems that I forgot to push. I might have it somewhere on my test machine.

Paul Bolle

2014-02-11 12:45 UTC #4 lore raw

In reply to Mauro Carvalho Chehab

Mauro,

Quoted 17 lines · Mauro Carvalho Chehab

Em Sat, 02 Nov 2013 20:20:54 +0100
Paul Bolle <[email protected]> escreveu:
Quoted 9 lines · Martin Walch
Quoted 8 lines
drivers/media/common/siano/Kconfig:21-26
Quoted 6 lines
config SMS_SIANO_DEBUGFS
bool "Enable debugfs for smsdvb"
depends on SMS_SIANO_MDTV
depends on DEBUG_FS
depends on SMS_USB_DRV
depends on CONFIG_SMS_USB_DRV = CONFIG_SMS_SDIO_DRV
Quoted 5 lines
Quoted 5 lines
The last line adds the dependency CONFIG_SMS_USB_DRV = CONFIG_SMS_SDIO_DRV. This expression does not look sound as those two symbols are not declared anywhere. So, the two strings CONFIG_SMS_USB_DRV and CONFIG_SMS_SDIO_DRV are compared, yielding always 'n'. As a result, SMS_SIANO_DEBUGFS will never be enabled.

Quoted 6 lines

[...] The Kconfig logic here is that it
should depends on !SMS_SDIO_DRV or SMS_USB_DRV = SMS_SDIO_DRV.
I remember I made a patch like that while testing some things with this driver, but it seems that I forgot to push. I might have it somewhere on my test machine.

This line is still present in v3.14-rc2. Did you ever find that patch on your test machine?

Paul Bolle

2014-04-16 15:47 UTC #5 lore raw

PATCH 1/1 sms: Remove CONFIG_ prefix from Kconfig symbols

Remove the CONFIG_ prefix from two Kconfig symbols in a dependency for SMS_SIANO_DEBUGFS. This prefix is invalid inside Kconfig files.

Note that the current (common sense) dependency on SMS_USB_DRV and SMS_SDIO_DRV being equal ensures that SMS_SIANO_DEBUGFS will not violate its constraints. These constraint are that:
- it should only be built if SMS_USB_DRV is set;
- it can't be builtin if USB support is modular.

So drop the dependency on SMS_USB_DRV, as it is unneeded.

Fixes 6c84b214284e ("[media] sms: fix randconfig building error")

Reported-by Martin Walch <[email protected]>

Signed-off-by Paul Bolle <[email protected]>

---
This is not runtime tested, as I don't have the hardware.

A matrix of the three cases in which this symbol can be set, to aid review and to see whether I actually understood the constraints:

USB  SMS_USB_DRV  SMS_SDIO_DRV  SMS_SIANO_MDTV  SMS_SIANO_DEBUGFS
m    m            m             m               y 
y    m            m             m               y 
y    y            y             y               y

By the way, I found myself staring at the entries in this file for quite some time. Perhaps things would have been easier to understand if SMS_USB_DRV and SMS_SDIO_DRV both selected SMS_SIANO_MDTV. But I didn't dare to test that idea.

 drivers/media/common/siano/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/media/common/siano/Kconfig +1 -2

index f953d33ee151..4bfbd5f463d1 100644
@@ -22,8 +22,7 @@ config SMS_SIANO_DEBUGFS
 	bool "Enable debugfs for smsdvb"
 	depends on SMS_SIANO_MDTV
 	depends on DEBUG_FS
-	depends on SMS_USB_DRV
-	depends on CONFIG_SMS_USB_DRV = CONFIG_SMS_SDIO_DRV
+	depends on SMS_USB_DRV = SMS_SDIO_DRV
 
 	---help---
 	  Choose Y to enable visualizing a dump of the frontend

Signature · 1 line

1.9.0

Paul Bolle

2014-06-16 12:38 UTC #6 lore raw

Re: [PATCH] [media] sms: Remove CONFIG_ prefix from Kconfig symbols

Quoted 25 lines · Paul Bolle

Remove the CONFIG_ prefix from two Kconfig symbols in a dependency for SMS_SIANO_DEBUGFS. This prefix is invalid inside Kconfig files.
Note that the current (common sense) dependency on SMS_USB_DRV and SMS_SDIO_DRV being equal ensures that SMS_SIANO_DEBUGFS will not violate its constraints. These constraint are that:
- it should only be built if SMS_USB_DRV is set;
- it can't be builtin if USB support is modular.
So drop the dependency on SMS_USB_DRV, as it is unneeded.
Fixes 6c84b214284e ("[media] sms: fix randconfig building error")
Reported-by Martin Walch <[email protected]>
Signed-off-by Paul Bolle <[email protected]>
---
This is not runtime tested, as I don't have the hardware.
A matrix of the three cases in which this symbol can be set, to aid review and to see whether I actually understood the constraints:
USB  SMS_USB_DRV  SMS_SDIO_DRV  SMS_SIANO_MDTV  SMS_SIANO_DEBUGFS
m    m            m             m               y 
y    m            m             m               y 
y    y            y             y               y 

Quoted 4 lines

By the way, I found myself staring at the entries in this file for quite some time. Perhaps things would have been easier to understand if SMS_USB_DRV and SMS_SDIO_DRV both selected SMS_SIANO_MDTV. But I didn't dare to test that idea.

This can still be seen in v3.16-rc1 and in next-20140616.

Debugfs for smsdvb has been unbuildable since v3.12. That problem has been reported even before v3.12 was released. Does no one care and can it be removed?

Quoted 8 lines

 drivers/media/common/siano/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/media/common/siano/Kconfig +0 -0

index f953d33ee151..4bfbd5f463d1 100644

drivers/media/common/siano/Kconfig config SMS_SIANO_DEBUGFS · line 28

Quoted 9 lines

drivers/media/common/siano/Kconfig +1 -2

@@ -22,8 +22,7 @@ config SMS_SIANO_DEBUGFS
 	bool "Enable debugfs for smsdvb"
 	depends on SMS_SIANO_MDTV
 	depends on DEBUG_FS
-	depends on SMS_USB_DRV
-	depends on CONFIG_SMS_USB_DRV = CONFIG_SMS_SDIO_DRV
+	depends on SMS_USB_DRV = SMS_SDIO_DRV
 
 	---help---
 	  Choose Y to enable visualizing a dump of the frontend

Paul Bolle