linux-kernel - Re: [PATCH v5 WIP 3/5] i2c-parport: define ports to connect

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]

Message-ID: <20150519095803.GA6337@sudip-PC>
Date:	Tue, 19 May 2015 15:28:03 +0530
From:	Sudip Mukherjee <sudipm.mukherjee@...il.com>
To:	Jean Delvare <jdelvare@...e.de>
Cc:	Greg KH <gregkh@...uxfoundation.org>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 WIP 3/5] i2c-parport: define ports to connect

On Tue, May 19, 2015 at 11:28:15AM +0200, Jean Delvare wrote:
> Le Tuesday 19 May 2015 à 14:14 +0530, Sudip Mukherjee a écrit :
> > On Tue, May 19, 2015 at 09:50:47AM +0200, Jean Delvare wrote:
> > > Hi Sudip,
> > > 
> > > Le Wednesday 06 May 2015 à 15:46 +0530, Sudip Mukherjee a écrit :
<snip>
> > then what about:
> > MODULE_PARM_DESC(parport, "Mention number of i2c-parport instances you
> > 		 want.\n Atmost " __stringify(MAX_DEVICE) " instances are
> > 		 allowed.\n Mention port numbers you want to use.\n"
> > 		 " If the port is not to be used mention -1.\n"
> > 		 " Default is one instance connected to parport0.\n");
> 
> I suggest formatting this in such a way that "\n"s are at the end of
> lines (as your patch had.) This makes this much more readable and
> unexpected embedded line breaks and spaces as you have here (the output
> of modinfo would look ugly.)
> 
> Also the initial description is still misleading, and there are too many
> occurrences of "mention" to my taste. I'd suggest the following:
> 
> MODULE_PARM_DESC(parport,
> 	"List of parallel ports to bind to, by index.\n"
> 	" Atmost " __stringify(MAX_DEVICE) " devices are supported.\n"
> 	" Default is one device connected to parport0.\n"
> );
> 
> I have removed the mention of -1 as this is an internal implementation
> detail and I can't think of any reason why the user would ever want to
> pass it.
will use this.
> 
> 
> > > 
> > <snip>
> > > 
> > > I tested this patch and it works fine.
> > > 
> > > Tested-by: Jean Delvare <jdelvare@...e.de>
> > 
> > do i add your Tested-by: to the main patch and this patch?
> 
> Yes.

Thanks Jean.

regards
sudip
> 
> -- 
> Jean Delvare
> SUSE L3 Support
> 
--
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/