Re: [PATCH] via686a sensors support

Christoph Hellwig (hch@infradead.org)
Sun, 12 Jan 2003 18:34:38 +0000


> @@ -16,7 +16,7 @@
> while the kernel is running. If you want to compile it as a module,
> say M here and read <file:Documentation/modules.txt>.
>
> - The module will be called i2c-amd756.o.
> + The module will be called i2c-amd756.ko.

Please don't submit unrelated changes in this patch. While I agree with
this documentation fix it's clearly a separate issue.

> +#include <linux/version.h>

you don't need this header.

> +#ifndef PCI_DEVICE_ID_VIA_82C686_4
> +#define PCI_DEVICE_ID_VIA_82C686_4 0x3057
> +#endif

This one should be in include/linux/pci_ids.h

> +extern inline long IN_FROM_REG(u8 val, int inNum)

All these extern inlines should be static inline instead.

> +static int via686a_find(int *address)
> +{
> + u16 val;
> +
> + if (!pci_present())
> + return -ENODEV;
> +
> + if (!(s_bridge = pci_find_device(PCI_VENDOR_ID_VIA,
> + PCI_DEVICE_ID_VIA_82C686_4,
> + NULL)))
> + return -ENODEV;

I think this should converted to a 2.4/2.5-style PCI driver. See the
amd drivers.

> +/* No commands defined yet */
> +static int via686a_command(struct i2c_client *client, unsigned int cmd, void *arg)
> +{
> + return 0;
> +}

->command is optional, so there's no need to have this stub.

> +
> + if ((jiffies - data->last_updated > HZ + HZ / 2) ||
> + (jiffies < data->last_updated) || !data->valid) {

Use time_after here?

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