Re: Fwd: Telecom Clock Driver for MPCBL0010 ATCA computer blade

[Greg KH <greg@kroah.com>]

On Thu, Oct 06, 2005 at 03:54:34PM -0700, Mark Gross wrote:
> On Thursday 06 October 2005 11:20, Greg KH wrote:
> > > +?????????????printk(KERN_ERR" misc_register retruns %d \n", ret);
> > > +?????????????ret = ?-EBUSY;
> > > +?????????????goto out3;
> > > +?????}
> > > +?????class = tlclk_miscdev.class;
> > > +?????class_device_create_file(class, &class_device_attr_current_ref);
> > 
> > Try registering a whole attribute group instead. ?It's much nicer than
> > the 20 lines you have to register and unregister your devices (and you
> > don't handle the error condition properly if something goes wrong half
> > way through.)
> > 
> 
> I couldn't find such an API that wasn't static to class.c, or
> described in class.txt.  Any pointers on this would be helpful.

sysfs_create_group() is what you want.

> +ssize_t tlclk_read(struct file *filp, char __user *buf, size_t count,
> +		loff_t *f_pos)
> +{
> +	if (count < sizeof(struct tlclk_alarms))
> +		return -EIO;
> +
> +	wait_event_interruptible(wq, got_event);
> +	if (copy_to_user(buf, alarm_events, sizeof(struct tlclk_alarms)))
> +		return -EFAULT;
> +
> +	memset(alarm_events, 0, sizeof(struct tlclk_alarms));
> +	got_event = 0;
> +
> +	return  sizeof(struct tlclk_alarms);
> +}

Two spaces after the return :(

> +
> +/* Read telecom clock IRQ number (Set by BIOS) */

Indent the comment please.

> +	init_timer(&switchover_timer);
> +/*	switchover_timer.function = switchover_timeout; */
> +/*	switchover_timer.data = 0; */

Remove these commented out lines?

Other than these minor things, and the attribute group stuff, this looks
good.

thanks,

greg k-h