linux-kernel - Re: [RFCv1 8/9] xshm: Makefile and Kconfig for M7400 Shared Memory Drivers

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 for Android: free password hash cracker in your pocket

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

Date:	Mon, 21 Nov 2011 21:30:36 +0100
From:	Paul Bolle <pebolle@...cali.nl>
To:	Sjur Brændeland 
	<sjur.brandeland@...ricsson.com>
Cc:	linux-kernel@...r.kernel.org,
	Linus Walleij <linus.walleij@...aro.org>, sjurbren@...il.com
Subject: Re: [RFCv1 8/9] xshm: Makefile and Kconfig for M7400 Shared Memory
 Drivers

A few minor comments follow.

On Mon, 2011-11-21 at 14:56 +0100, Sjur Brændeland wrote:
> Signed-off-by: Sjur Brændeland <sjur.brandeland@...ricsson.com>
> ---
>  drivers/Kconfig       |    2 ++
>  drivers/Makefile      |    1 +
>  drivers/xshm/Kconfig  |   10 ++++++++++
>  drivers/xshm/Makefile |    4 ++++
>  4 files changed, 17 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/xshm/Kconfig
>  create mode 100644 drivers/xshm/Makefile
> 
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index b5e6f24..c91a13c 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -136,4 +136,6 @@ source "drivers/hv/Kconfig"
>  
>  source "drivers/devfreq/Kconfig"
>  
> +source "drivers/xshm/Kconfig"
> +
>  endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 1b31421..eedd8e5 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -120,6 +120,7 @@ obj-$(CONFIG_VLYNQ)		+= vlynq/
>  obj-$(CONFIG_STAGING)		+= staging/
>  obj-y				+= platform/
>  obj-y				+= ieee802154/
> +obj-$(CONFIG_XSHM)		+= xshm/
>  #common clk code
>  obj-y				+= clk/
>  
> diff --git a/drivers/xshm/Kconfig b/drivers/xshm/Kconfig
> new file mode 100644
> index 0000000..fa627b8
> --- /dev/null
> +++ b/drivers/xshm/Kconfig
> @@ -0,0 +1,10 @@
> +config XSHM
> +	tristate "External Shared Memory Protocol (XSHM)"
> +#	depends on GENIO

Why is this added commented out?

> +	default n
> +	---help---
> +	Say "yes" if you want to support External Shared Memory (XSHM)

"Say Y"?

> +	IPC mechanism. XSHM is an IPC protocol used to talk to external
> +	device such as modem over a shared memory (e.g. Chip to Chip).
> +	This will normally be built-in, loadable module is used for testing.

Perhaps something like: "Only say M here if you want to test XSHM and
need to load and unload its module."?

> +	If unsure say N.
> diff --git a/drivers/xshm/Makefile b/drivers/xshm/Makefile
> new file mode 100644
> index 0000000..a630501
> --- /dev/null
> +++ b/drivers/xshm/Makefile
> @@ -0,0 +1,4 @@
> +obj-$(CONFIG_XSHM) +=  xshmchr.o
> +xshmchr-objs := xshm_chr.o

A quick scan of the other patches didn't show a file named xshmchr.c. If
that's correct, can't you rename either the object or the source file? 

> +obj-$(CONFIG_XSHM) += xshm.o
> +xshm-objs := xshm_boot.o xshm_pdev.o genio_dummy.o

Are these two separate drivers (modules) that both are built if one
Kconfig symbol is set? If so, is there a reason why you can't use either
one combined driver or two Kconfig symbols?


Paul Bolle

--
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/