From: Pavel Machek <pavel@ucw.cz>
To: Christoph Hellwig <hch@infradead.org>,
Arjan van de Ven <arjanv@redhat.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] amd756 and amd8111 sensors support
Date: Sun, 5 Jan 2003 20:34:50 +0100 [thread overview]
Message-ID: <20030105193450.GA260@elf.ucw.cz> (raw)
In-Reply-To: <20021228123655.A31843@infradead.org>
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?
prev parent reply other threads:[~2003-01-05 21:33 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <200212280303.gBS33o628113@hera.kernel.org>
2002-12-28 11:46 ` [PATCH] amd756 and amd8111 sensors support Arjan van de Ven
2002-12-28 12:36 ` Christoph Hellwig
2003-01-05 17:44 ` [PATCH] i2c-isa and w83871d " GertJan Spoelman
2003-01-11 8:22 ` Christoph Hellwig
2003-01-05 19:34 ` Pavel Machek [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20030105193450.GA260@elf.ucw.cz \
--to=pavel@ucw.cz \
--cc=arjanv@redhat.com \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox