Re: [RFCv1 8/9] xshm: Makefile and Kconfig for M7400 Shared MemoryDrivers

From: Paul Bolle
Date: Wed Nov 23 2011 - 05:49:37 EST


Sjur,

On Mon, 2011-11-21 at 22:15 +0100, Sjur BrÃndeland wrote:
> >> +++ 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.

Well, if you feel that reminder is needed please add a short explanation
(in another comment) so it won't be overlooked.

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

I have no opinion whether or not to use two devices. But if you do use
two devices you might consider exposing them through separate Kconfig
symbols. Perhaps you already did consider that and decided that it would
be of little benefit. Or maybe it's simply not possible. Well, my
message seems just to be to give this some thought.


Paul Bolle

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