Re: [PATCH 7/9] added issues description in TODO file

From: Greg Kroah-Hartman
Date: Wed Feb 27 2013 - 11:37:04 EST


On Wed, Feb 27, 2013 at 07:26:44PM +0300, Michail Kurachkin wrote:
> Signed-off-by: Michail Kurachkin <michail.kurachkin@xxxxxxxxxxx>
> ---
> drivers/staging/si3226x/TODO | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)

Please, when sending patches, make them descriptive of what part of the
kernel that this is for. In this specific instance, your Subject:
should be:
"staging: si3226x: add issues to TODO file"
that way it makes sense where things are being changed, and it makes it
easier to find the series in my inbox (hint, you should also use
git-send-email to chain them, not make them individual threads, but
that's a minor issue.)

> diff --git a/drivers/staging/si3226x/TODO b/drivers/staging/si3226x/TODO
> index 83c5958..b2cb27e 100644
> --- a/drivers/staging/si3226x/TODO
> +++ b/drivers/staging/si3226x/TODO
> @@ -1,2 +1,8 @@
> 1) mutex_lock(&slic_chr_dev_lock);
> -This locking is very heavy handed. You are holding it across the entire open, close, read, write, ioctl, and it is protecting a bunch of different things.
> +This locking is very heavy handed and should be reworked
> +
> +2) SLIC driver works unstable when compiled as module, loaded, then unloaded and finally reloaded again.
> +
> +3) Current version of Si3226x SLIC driver is limited. It implements only base functionality such as Answer, Hangup, receive DTMF, send DTMF, send Caller ID.

Please wrap your lines at 75 columns at the least, to make it readable.

Also, I need an email address in the TODO files as to who to cc: patches
to in order for them to be reviewed.

And really, these are the only reasons this driver isn't in the main
part of the kernel? That last item shouldn't be a limiting factor, and
the other two aren't really an issue either, right?

I need a list here (and for the tdm core) of what is needed to be done
to get the code out of the staging tree, and email addresses and names
of people who are going to be responsible for doing this work.

Oh, and finally, why isn't this driver under the drivers/staging/tdm/
directory?

thanks,

greg k-h
--
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/