Skip to contentSkip to conversation

Default Kconfig state for hw_random drivers

PATCH In review Mark Brown 5 participants 10 messages Apr 26, 2012 – Apr 30, 2012 Hardware random number generator core

lore
Share & tools
lore

Mark Brown

author
#0 lore raw

An Atmel hw_random driver just appeared in -next. Since it's just a regular platform device it has no dependencies on any particular platform so following the behaviour of other hw_random drivers it's set to default enabled if hw_random is enabled, even on non-Atmel platforms.

This doesn't feel like the right thing to do - it'd mean that most of the embedded RNGs would end up getting enabled by default which probably isn't what we want.

#1 lore raw
Quoted 8 lines · Mark Brown

An Atmel hw_random driver just appeared in -next.  Since it's just a regular platform device it has no dependencies on any particular platform so following the behaviour of other hw_random drivers it's set to default enabled if hw_random is enabled, even on non-Atmel platforms.

This doesn't feel like the right thing to do - it'd mean that most of the embedded RNGs would end up getting enabled by default which probably isn't what we want.

Indeed, thanks for catching this. The driver isn't new but it had a previous dependency on just one AT91 chip family.

Looking at other options in the same Kconfig, it would be appropriate to make it depend on ARCH_AT91 instead.

Nicolas?

-Olof

author
#2 lore raw
Quoted 2 lines · Olof Johansson

Looking at other options in the same Kconfig, it would be appropriate to make it depend on ARCH_AT91 instead.

Possibly wants an || AVR32 dependency of some kind too?

#3 lore raw
In reply to Olof Johansson

On 04/26/2012 04:17 PM, Olof Johansson :

Quoted 11 lines
Quoted 8 lines · Mark Brown

An Atmel hw_random driver just appeared in -next. Since it's just a regular platform device it has no dependencies on any particular platform so following the behaviour of other hw_random drivers it's set to default enabled if hw_random is enabled, even on non-Atmel platforms.

This doesn't feel like the right thing to do - it'd mean that most of the embedded RNGs would end up getting enabled by default which probably isn't what we want.

Quoted 7 lines

Indeed, thanks for catching this. The driver isn't new but it had a previous dependency on just one AT91 chip family.

Looking at other options in the same Kconfig, it would be appropriate to make it depend on ARCH_AT91 instead.

Nicolas?

Hi,

I had the same kind of discussion with Felipe and Arnd in this thread:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/122248/focus=122304

The conclusion came out after Arnd's strong arguments:
http://article.gmane.org/gmane.linux.ports.arm.kernel/122962

Maybe adding a "default" directive can be an valid option?

Best regards,

Signature · 1 line
Nicolas Ferre
author
#4 lore raw
Quoted 7 lines · Nicolas Ferre

I had the same kind of discussion with Felipe and Arnd in this thread:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/122248/focus=122304

The conclusion came out after Arnd's strong arguments:
http://article.gmane.org/gmane.linux.ports.arm.kernel/122962

Maybe adding a "default" directive can be an valid option?

There is one, that's the problem - the driver is defaulting to the same state as the main hw_random option (as is idiomatic for this subsystem). This means that if you've enabled any hw_random device this driver will be enabled by default. I would argue that this default directive only makes sense if there's an explicit dependency limiting the platforms it can build on. If it can build on any platform then omitting the default and just defaulting to N as is normal for drivers would be better.

#5 lore raw
Quoted 7 lines · Mark Brown

There is one, that's the problem - the driver is defaulting to the same state as the main hw_random option (as is idiomatic for this subsystem). This means that if you've enabled any hw_random device this driver will be enabled by default. I would argue that this default directive only makes sense if there's an explicit dependency limiting the platforms it can build on. If it can build on any platform then omitting the default and just defaulting to N as is normal for drivers would be better.

How about making it default to (HW_RANDOM && ARCH_AT91) then?

Arnd

author
#6 lore raw
Quoted 11 lines · Arnd Bergmann
Quoted 7 lines · Mark Brown

There is one, that's the problem - the driver is defaulting to the same state as the main hw_random option (as is idiomatic for this subsystem). This means that if you've enabled any hw_random device this driver will be enabled by default. I would argue that this default directive only makes sense if there's an explicit dependency limiting the platforms it can build on. If it can build on any platform then omitting the default and just defaulting to N as is normal for drivers would be better.

How about making it default to (HW_RANDOM && ARCH_AT91) then?

Yes, something like that would do the job too.

#7 lore raw

PATCH 1/1 hwrng: Kconfig: modify default state for atmel-rng driver

In order to prevent building the Atmel hw_random driver for each and every configuration, add a "default" Kconfig state in relation with CONFIG_ARCH_AT91.

Signed-off-by Nicolas Ferre <[email protected]>
---
 drivers/char/hw_random/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/char/hw_random/Kconfig +1 -1
index b2402eb..d98998f 100644
@@ -63,7 +63,7 @@ config HW_RANDOM_AMD
 config HW_RANDOM_ATMEL
 	tristate "Atmel Random Number Generator support"
 	depends on HW_RANDOM && HAVE_CLK
-	default HW_RANDOM
+	default (HW_RANDOM && ARCH_AT91)
 	---help---
 	  This driver provides kernel-side support for the Random Number
 	  Generator hardware found on Atmel AT91 devices.
Signature · 1 line
1.7.10
author Reviewed-by on v1 1/1
#8 lore raw

Re: [PATCH] hwrng: Kconfig: modify default state for atmel-rng driver

Quoted 5 lines · Nicolas Ferre

In order to prevent building the Atmel hw_random driver for each and every configuration, add a "default" Kconfig state in relation with CONFIG_ARCH_AT91.

Signed-off-by Nicolas Ferre <[email protected]>
Reviewed-by Mark Brown <[email protected]>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120430/5f19a4a3/attachment.sig>

#9 lore raw

Re: [PATCH] hwrng: Kconfig: modify default state for atmel-rng driver

Quoted 8 lines · Mark Brown
Quoted 5 lines · Nicolas Ferre

In order to prevent building the Atmel hw_random driver for each and every configuration, add a "default" Kconfig state in relation with CONFIG_ARCH_AT91.

Signed-off-by Nicolas Ferre <[email protected]>
Reviewed-by Mark Brown <[email protected]>

Patch applied. Thanks!

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt