linux-kernel - Re: [PATCH v2 1/1] net5501: platform driver for Soekris Engineering net5501 single-board computer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 31 Jan 2012 18:38:26 -0700
From:	Philip Prindeville <philipp@...fish-solutions.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	Alessandro Zummo <a.zummo@...ertech.it>,
	Richard Purdie <rpurdie@...ys.net>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] net5501: platform driver for Soekris Engineering
 net5501 single-board computer

On 1/31/12 3:19 PM, Andrew Morton wrote:
> On Fri, 27 Jan 2012 22:02:19 -0700
> Philip Prindeville <philipp@...fish-solutions.com> wrote:
> 
>> ---
>> Trivial platform driver for Soekris Engineering net5501 single-board
>> computer. Probes well-known locations in ROM for BIOS signature to
>> confirm correct platform. Registers 1 LED and 1 GPIO-based button
>> (typically used for soft reset).
>>
>> ...
>>
>>  arch/x86/Kconfig                  |    6 ++
>>  arch/x86/platform/geode/Makefile  |    1 +
>>  arch/x86/platform/geode/net5501.c |  154 +++++++++++++++++++++++++++++++++++++
>>  drivers/leds/leds-net5501.c       |   97 -----------------------
> 
> Forgot to clean up the leftovers:
> 
> make[2]: *** No rule to make target `drivers/leds/leds-net5501.c', needed by `drivers/leds/leds-net5501.o'.  Stop.
> 
> Also, are you sure that the new Kconfig entry is sufficient?  The old
> driver had more dependencies (GPIO_CS5535 and LEDS_TRIGGERS) and
> selects LEDS_TRIGGER_DEFAULT_ON.  Should the new driver be doing such
> things?

Thanks for getting that.  Overlooked it.

The thing about the way the platform driver is now is it can wait until the subordinate modules get loaded and it will register the relevant devices then...

It's conceivable that one might want LEDs but not the reset button, or vice versa.

It might make sense to have the GPIO_CS5535 driver be selected, but also I'm good with the platform driver being statically linked in, and the LED or GPIO drivers being insmod'd.

I figure best to KISS and allow people to have as small a static kernel as they want, and then build up functionality with modules.

-Philip

> 
> 
> --- a/drivers/leds/Kconfig~net5501-platform-driver-for-soekris-engineering-net5501-single-board-computer-fix
> +++ a/drivers/leds/Kconfig
> @@ -89,16 +89,6 @@ config LEDS_NET48XX
>  	  This option enables support for the Soekris net4801 and net4826 error
>  	  LED.
>  
> -config LEDS_NET5501
> -	tristate "LED Support for Soekris net5501 series Error LED"
> -	depends on LEDS_TRIGGERS
> -	depends on X86 && GPIO_CS5535
> -	select LEDS_TRIGGER_DEFAULT_ON
> -	default n
> -	help
> -	  Add support for the Soekris net5501 board (detection, error led
> -	  and GPIO).
> -
>  config LEDS_FSG
>  	tristate "LED Support for the Freecom FSG-3"
>  	depends on LEDS_CLASS
> --- a/drivers/leds/Makefile~net5501-platform-driver-for-soekris-engineering-net5501-single-board-computer-fix
> +++ a/drivers/leds/Makefile
> @@ -14,7 +14,6 @@ obj-$(CONFIG_LEDS_MIKROTIK_RB532)	+= led
>  obj-$(CONFIG_LEDS_S3C24XX)		+= leds-s3c24xx.o
>  obj-$(CONFIG_LEDS_AMS_DELTA)		+= leds-ams-delta.o
>  obj-$(CONFIG_LEDS_NET48XX)		+= leds-net48xx.o
> -obj-$(CONFIG_LEDS_NET5501)		+= leds-net5501.o
>  obj-$(CONFIG_LEDS_WRAP)			+= leds-wrap.o
>  obj-$(CONFIG_LEDS_COBALT_QUBE)		+= leds-cobalt-qube.o
>  obj-$(CONFIG_LEDS_COBALT_RAQ)		+= leds-cobalt-raq.o
> _
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/