Date: Mon, 7 Mar 2005 22:44:25 -0800
From: Greg KH <greg@kroah.com>
To: Wen Xiong <wendyx@us.ibm.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [ patch 4/7] drivers/serial/jsm: new serial device driver
Message-ID: <20050308064424.GF17022@kroah.com>
References: <42225A47.3060206@us.ltcfwd.linux.ibm.com> <20050228063954.GB23595@kroah.com> <4228CE41.2000102@us.ltcfwd.linux.ibm.com> <20050304220116.GA1201@kroah.com> <422CD9DB.10103@us.ltcfwd.linux.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <422CD9DB.10103@us.ltcfwd.linux.ibm.com>
User-Agent: Mutt/1.5.8i
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1676
Lines: 51

On Mon, Mar 07, 2005 at 05:46:51PM -0500, Wen Xiong wrote:
> +static ssize_t jsm_driver_version_show(struct device_driver *ddp, char *buf)
> +{
> +	return snprintf(buf, PAGE_SIZE, "jsm_version: %s\n", JSM_VERSION);

Again, drop the "prefix:" from every sysfs file, it should not be there
(the data type is inferred by the name of the file, you do not have to
repeat it again.)

> +static ssize_t jsm_tty_state_show(struct class_device *class_dev, char *buf)
> +{
> +	struct jsm_channel *ch;
> +
> +	if (class_dev) {
> +		ch = class_get_devdata(class_dev);
> +		if ( ch && (ch->ch_bd->state == BOARD_READY))
> +			return snprintf(buf, PAGE_SIZE, "%s\n", ch->ch_open_count ? "Open" : "Closed");
> +	}
> +
> +	return -EINVAL;
> +}
> +static CLASS_DEVICE_ATTR(state, S_IRUGO, jsm_tty_state_show, NULL);

Who needs to know if a port is open or not?

> +static ssize_t jsm_tty_baud_show(struct class_device *class_dev, char *buf)
> +{
> +	struct jsm_channel *ch;
> +
> +	if (class_dev) {
> +		ch = class_get_devdata(class_dev);
> +		if ( ch && (ch->ch_bd->state == BOARD_READY))
> +		return snprintf(buf, PAGE_SIZE, "%d\n", ch->ch_old_baud);
> +	}
> +
> +	return -EINVAL;
> +}
> +static CLASS_DEVICE_ATTR(baud, S_IRUGO, jsm_tty_baud_show, NULL);

No, please delete these, and the other sysfs files that duplicate the
same info that you can get by using the standard Linux termios calls.
There is no need for them here.

thanks,

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