lkml.org 
[lkml]   [2003]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] amd756 and amd8111 sensors support
    Hi!

    > Please add them inside drivers/i2c/, not at the toplevel.

    Done.

    > obj-$(CONFIG_PHONE) += telephony/
    > obj-$(CONFIG_MD) += md/
    > obj-$(CONFIG_BT) += bluetooth/
    > +#include <linux/version.h>
    > +#include <linux/module.h>
    > +#include <linux/pci.h>
    > +#include <asm/io.h>
    > +#include <linux/kernel.h>
    > +#include <linux/stddef.h>
    > +#include <linux/sched.h>
    > +#include <linux/ioport.h>
    > +#include <linux/i2c.h>
    > +#include <linux/init.h>
    >
    > <asm/*.h> after <linux/*.h>, please

    Fixed.

    > +static struct i2c_adapter amd756_adapter = {
    > + "unset",
    > + I2C_ALGO_SMBUS | I2C_HW_SMBUS_AMD756,
    > + &smbus_algorithm,
    > + NULL,
    > + amd756_inc,
    > + amd756_dec,
    > + NULL,
    > + NULL,
    > +};
    >
    > Please use named initializers for such sparse method vectors.

    Fixed.

    > +static int __initdata amd756_initialized;
    >
    > __initdata for function used in cleanup code is bogus. But this whole
    > flag is crap, see below.

    Killed.

    > +static unsigned short amd756_smba = 0;
    >
    > This is in .bss, no need to initialize to zero.

    Ok.

    > + if (check_region(amd756_smba, SMB_IOSIZE)) {
    > + printk
    > + ("i2c-amd756.o: SMB region 0x%x already in use!\n",
    > + amd756_smba);
    > + return -ENODEV;
    > + }
    > +
    > + /* Everything is happy, let's grab the memory and set things up. */
    > + request_region(amd756_smba, SMB_IOSIZE, "amd756-smbus");
    >
    > Don't use check_region.

    Fixed.

    > + amd756_initialized = 0;
    >
    > It's in .bss and thus already zero.

    Killed.

    > +EXPORT_NO_SYMBOLS;
    >
    > Remove it, it's a noop for 2.5.
    >
    > +#ifdef MODULE
    > +
    > +MODULE_AUTHOR("Merlin Hughes <merlin@merlin.org>");
    > +MODULE_DESCRIPTION("AMD756/766/768/nVidia nForce SMBus driver");
    > +
    > +#ifdef MODULE_LICENSE
    > +MODULE_LICENSE("GPL");
    > +#endif
    > +
    > +#endif /* MODULE */
    >
    > Please get rid of that ifdef mess, you can just always use it.

    Killed.

    Okay, I see lot more cleaning up is needed. I actually saved it as a
    "TODO" file... Thanks for your comments.

    Pavel

    --- linux-sensors.mid/drivers/Makefile 2002-12-18 22:28:25.000000000 +0100
    +++ linux-sensors/drivers/Makefile 2003-01-05 20:06:17.000000000 +0100
    @@ -38,8 +38,6 @@
    obj-$(CONFIG_SERIO) += input/serio/
    obj-$(CONFIG_I2O) += message/
    obj-$(CONFIG_I2C) += i2c/
    -obj-$(CONFIG_I2C_MAINBOARD) += i2c/busses/
    -obj-$(CONFIG_SENSORS) += i2c/chips/
    obj-$(CONFIG_PHONE) += telephony/
    obj-$(CONFIG_MD) += md/
    obj-$(CONFIG_BT) += bluetooth/
    --- linux-sensors.mid/drivers/i2c/Makefile 2002-12-18 22:28:30.000000000 +0100
    +++ linux-sensors/drivers/i2c/Makefile 2003-01-05 20:06:13.000000000 +0100
    @@ -6,6 +6,8 @@
    i2c-algo-ite.o i2c-proc.o i2c-algo-ibm_ocp.o

    obj-$(CONFIG_I2C) += i2c-core.o
    +obj-$(CONFIG_I2C_MAINBOARD) += busses/
    +obj-$(CONFIG_SENSORS) += chips/
    obj-$(CONFIG_I2C_CHARDEV) += i2c-dev.o
    obj-$(CONFIG_I2C_ALGOBIT) += i2c-algo-bit.o
    obj-$(CONFIG_I2C_PHILIPSPAR) += i2c-philips-par.o
    --- linux-sensors.mid/drivers/i2c/busses/i2c-amd756.c 2002-12-02 01:47:11.000000000 +0100
    +++ linux-sensors/drivers/i2c/busses/i2c-amd756.c 2003-01-05 20:23:25.000000000 +0100
    @@ -37,13 +37,13 @@
    #include <linux/version.h>
    #include <linux/module.h>
    #include <linux/pci.h>
    -#include <asm/io.h>
    #include <linux/kernel.h>
    #include <linux/stddef.h>
    #include <linux/sched.h>
    #include <linux/ioport.h>
    #include <linux/i2c.h>
    #include <linux/init.h>
    +#include <asm/io.h>

    struct sd {
    const unsigned short vendor;
    @@ -116,30 +116,24 @@
    static u32 amd756_func(struct i2c_adapter *adapter);

    static struct i2c_algorithm smbus_algorithm = {
    - /* name */ "Non-I2C SMBus adapter",
    - /* id */ I2C_ALGO_SMBUS,
    - /* master_xfer */ NULL,
    - /* smbus_access */ amd756_access,
    - /* slave;_send */ NULL,
    - /* slave_rcv */ NULL,
    - /* algo_control */ NULL,
    - /* functionality */ amd756_func,
    + .name = "Non-I2C SMBus adapter",
    + .id = I2C_ALGO_SMBUS,
    + .smbus_xfer = amd756_access,
    + .functionality = amd756_func,
    };

    static struct i2c_adapter amd756_adapter = {
    - "unset",
    - I2C_ALGO_SMBUS | I2C_HW_SMBUS_AMD756,
    - &smbus_algorithm,
    - NULL,
    - amd756_inc,
    - amd756_dec,
    - NULL,
    - NULL,
    + .name = "unset",
    + .id = I2C_ALGO_SMBUS | I2C_HW_SMBUS_AMD756,
    + .algo_data = &smbus_algorithm,
    + .inc_use = amd756_inc,
    + .dec_use = amd756_dec,
    };

    -static int __initdata amd756_initialized;
    -static struct sd *amd756_sd = NULL;
    -static unsigned short amd756_smba = 0;
    +static struct sd *amd756_sd;
    +static unsigned short amd756_smba;
    +static int amd756_initialized;
    +

    int amd756_setup(void)
    {
    @@ -191,16 +185,12 @@
    printk(KERN_ERR "i2c-amd756.o: Error: SMB base address uninitialized\n");
    return -ENODEV;
    }
    - if (check_region(amd756_smba, SMB_IOSIZE)) {
    - printk
    - ("i2c-amd756.o: SMB region 0x%x already in use!\n",
    - amd756_smba);
    + if (!request_region(amd756_smba, SMB_IOSIZE, "amd756-smbus")) {
    + printk("i2c-amd756.o: SMB region 0x%x already in use!\n",
    + amd756_smba);
    return -ENODEV;
    }

    - /* Everything is happy, let's grab the memory and set things up. */
    - request_region(amd756_smba, SMB_IOSIZE, "amd756-smbus");
    -
    #ifdef DEBUG
    pci_read_config_byte(AMD756_dev, SMBREV, &temp);
    printk("i2c-amd756.o: SMBREV = 0x%X\n", temp);
    @@ -456,15 +446,6 @@
    int __init i2c_amd756_init(void)
    {
    int res;
    -#ifdef DEBUG
    -/* PE- It might be good to make this a permanent part of the code! */
    - if (amd756_initialized) {
    - printk
    - ("i2c-amd756.o: Oops, amd756_init called a second time!\n");
    - return -EBUSY;
    - }
    -#endif
    - amd756_initialized = 0;
    if ((res = amd756_setup())) {
    printk
    ("i2c-amd756.o: AMD756 or compatible device not detected, module not inserted.\n");
    @@ -509,18 +490,10 @@
    return 0;
    }

    -EXPORT_NO_SYMBOLS;
    -
    -#ifdef MODULE
    -
    MODULE_AUTHOR("Merlin Hughes <merlin@merlin.org>");
    MODULE_DESCRIPTION("AMD756/766/768/nVidia nForce SMBus driver");

    -#ifdef MODULE_LICENSE
    MODULE_LICENSE("GPL");
    -#endif
    -
    -#endif /* MODULE */

    module_init(i2c_amd756_init)
    module_exit(i2c_amd756_exit)
    --- linux-sensors.mid/include/linux/i2c.h 2002-12-14 12:53:19.000000000 +0100
    +++ linux-sensors/include/linux/i2c.h 2003-01-05 20:11:00.000000000 +0100
    @@ -223,10 +223,6 @@
    u32 (*functionality) (struct i2c_adapter *);
    };

    -#if LINUX_VERSION_CODE < KERNEL_VERSION(2,1,29)
    -struct proc_dir_entry;
    -#endif
    -
    /*
    * i2c_adapter is the structure used to identify a physical i2c bus along
    * with the access algorithms necessary to access it.

    --
    Worst form of spam? Adding advertisment signatures ala sourceforge.net.
    What goes next? Inserting advertisment *into* email?
    -
    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/

    \
     
     \ /
      Last update: 2005-03-22 13:32    [W:0.032 / U:14.052 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site