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: Windows password security audit tool. GUI, reports in PDF.

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

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

Hi Paul,

> A few minor comments follow.
>
>> +++ 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?

A proper GENIO driver is not yet submitted, but will be later on (see
patch 5/9).
When this driver is contributed I'm expecting a Kconfig with "config GENIO"
to show up, that XSHM should depend upon. This comment serves as
a reminder.

>> +     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."?

OK, thanks I'll update these things in the next patch-set.

>> +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?
Yes, you're right. I'll fix this.

>
>> +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?

Yes, that was the idea.

>If so, is there a reason why you can't use either
> one combined driver or two Kconfig symbols?

xshm exposes devices that can be of type stream or packet,
while xshm_chr and caif_xshm are drivers for stream and
packet devices. It looked as a good idea to keep these separate,
but I'm open for other opinions here. In a real life deployment
they will all be build-in anyway.

Regards,
Sjur
--
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/