Re: adm1026 driver port for kernel 2.6.10-rc2 [RE-REVISED DRIVER]

[Justin Thiessen]

On Sat, Nov 20, 2004 at 11:13:56AM +0100, Arjan van de Ven wrote:
> On Thu, 2004-11-18 at 10:56 -0800, Justin Thiessen wrote:
> > MODULE_PARM(gpio_input,"1-17i");
> 
> new 2.6 drivers should NOT use MODULE_PARM, it's deprecated.
> use module_param() instead

Ok.  You mean module_param_array() in these particular cases, right?

> > int adm1026_attach_adapter(struct i2c_adapter *adapter)
> > {
> > 	if (!(adapter->class & I2C_CLASS_HWMON)) {
> > 		return 0;
> > 	}
> 
> no need for extra { }'s in such a case

Of course there's no _need_.  But I find the result stylistically easier to 
read.  Is there any real objection?

> > static ssize_t show_in(struct device *dev, char *buf, int nr)
> > {
> > 	struct adm1026_data *data = adm1026_update_device(dev);
> > 	return sprintf(buf,"%d\n", INS_FROM_REG(nr, data->in[nr]));
> > }
> 
> any chance you could make this use snprintf instead ?

I'll defer to Jean's response...

<snip awkward locking construct>

> this locking construct is rahter awkward; is it possible to refactor the
> code such that you can down and up in the same function ?

Yes, at the cost of some minor code duplication or the introduction of
another variable.  Is that preferable?  Is holding the lock across function
calls a Bad Idea?

Justin Thiessen
---------------
jthiessen@xxxxxxxxxxxxxxxxxxxx

-
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/