On Fri, Mar 16, 2012 at 12:19:25PM +0600, Alexander Pazdnikov wrote:
> Thank you Greg, I've corrected driver taking into account your comments.
> This is my first friver, I've missed to check the first version with checkpatch script.
> Today it goes through checkpatch script with no errors and warnings.
> Appreciate any comments.
Looks better, some minor comments below.
Note, I don't know the comedi subsystem that well, so I'll let others
say if you messed up anything there.
> The ad7739.h file is need for inclusion into board specific initialization file
> and to configure the exact chip.
Ok, so it will end up in include/linux/platform_data/ eventually, right?
> +/* #define DEBUG 1 */
> +
> +#include "../comedidev.h"
> +#include "ad7739.h"
> +#include <linux/gpio.h>
> +#include <linux/completion.h>
> +#include <linux/spi/spi.h>
> +#include <linux/interrupt.h>
Extra line here would be nice.
> +#define MAX_DATA 0xFFFFFF /* 24-bit mode only */
> +#define CONVERT_TIMEOUT_MSECS 100 /* spec max conv time = 2689 usecs */
> +#define DRIVER_NAME "ad7739"
Why not just use KBUILD_MODNAME wherever you were using DRIVER_NAME?
That ensures you get things right.
> +#ifdef DEBUG
> +static int ad7739_print(struct device *dev, void *data)
> +{
> + dev_info(dev, "dev_name = %s\n", dev_name(dev));
> +
> + return 0;
> +}
> +#endif
You should probably take this out, along with the call below where you
use it, as it's not needed anymore, right?
> + snprintf(devname, sizeof(devname), "%s%u.%u", spi_bus_type.name,
> + (unsigned)((dev->iobase >> 8) & 0xFF),
> + (unsigned)(dev->iobase & 0xFF));
> +
> + d = bus_find_device_by_name(&spi_bus_type, NULL, devname);
> + if (!d) {
> + dev_err(dev->class_dev, "devices %s not found\n", devname);
> + return -EINVAL;
> + }
> +
> + priv->spi = to_spi_device(d);
Is that how you register a spi device normally? I don't see any other
SPI drivers doing this, why are you not just registering the driver with
the bus like others do?
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
