diff -urN clean/drivers/i2c/chips/Kconfig work/drivers/i2c/chips/Kconfig
--- clean/drivers/i2c/chips/Kconfig 2003-03-21 12:42:02.000000000 +0100
+++ work/drivers/i2c/chips/Kconfig 2003-03-21 23:23:05.000000000 +0100
@@ -37,4 +37,17 @@
in the lm_sensors package, which you can download at
http://www.lm-sensors.nu
+config SENSORS_VIA686A
+ tristate " VIA686A"
+ depends on I2C && I2C_PROC
+ help
+ support for via686a
+ If you say yes here you get support for the integrated sensors in
+ Via 686A/B South Bridges. This can also be built as a module
+ which can be inserted and removed while the kernel is running.
+
+ You will also need the latest user-space utilties: you can find them
+ in the lm_sensors package, which you can download at
+ http://www.lm-sensors.nu
+
endmenu
diff -urN clean/drivers/i2c/chips/Makefile work/drivers/i2c/chips/Makefile
--- clean/drivers/i2c/chips/Makefile 2003-03-21 12:42:02.000000000 +0100
+++ work/drivers/i2c/chips/Makefile 2003-03-21 23:23:01.000000000 +0100
@@ -4,3 +4,4 @@
obj-$(CONFIG_SENSORS_ADM1021) += adm1021.o
obj-$(CONFIG_SENSORS_LM75) += lm75.o
+obj-$(CONFIG_SENSORS_VIA686A) += via686a.o
// The following register sets temp interrupt mode (bits 1-0 for temp1,
// 3-2 for temp2, 5-4 for temp3). Modes are:
// 00 interrupt stays as long as value is out-of-range
// 01 interrupt is cleared once register is read (default)
// 10 comparator mode- like 00, but ignores hysteresis
// 11 same as 00
Please don't use C++-style comments in kernel code.
static inline u8 TEMP_TO_REG(long val)
{
return (u8)
SENSORS_LIMIT(viaLUT[((val <= -500) ? 0 : (val >= 1100) ? 160 :
((val + 5) / 10 + 50))], 0, 255);
}
Dead code?
static int via686a_id = 0;
This doesn't need to be initialized.
ERROR4:
All-uppercase is ugly..
Am Sonntag, 23. M?rz 2003 21:27 schrieb Christoph Hellwig:
> // The following register sets temp interrupt mode (bits 1-0 for temp1,
> // 3-2 for temp2, 5-4 for temp3). Modes are:
> // 00 interrupt stays as long as value is out-of-range
> // 01 interrupt is cleared once register is read (default)
> // 10 comparator mode- like 00, but ignores hysteresis
> // 11 same as 00
>
> Please don't use C++-style comments in kernel code.
>
Why? It's a valid C99 feature and since the kernel already uses C99
initializers it won't compile with compilers that choke on C99 comments
anyway.
Dominik
--
Be at war with your voices, at peace with your neighbors, and let every new
year find you a better man. (Benjamin Franklin, 1706-1790)
On Sun, Mar 23, 2003 at 09:36:10PM +0100, Dominik Kubla wrote:
> Why? It's a valid C99 feature and since the kernel already uses C99
> initializers it won't compile with compilers that choke on C99 comments
> anyway.
Because there's a strong preference for traditional C style in the kernel.
typedefs are also a valid C feature and we try to avoid them.
Christoph Hellwig wrote:
> On Sun, Mar 23, 2003 at 09:36:10PM +0100, Dominik Kubla wrote:
>
>>Why? It's a valid C99 feature and since the kernel already uses C99
>>initializers it won't compile with compilers that choke on C99 comments
>>anyway.
>
>
> Because there's a strong preference for traditional C style in the kernel.
> typedefs are also a valid C feature and we try to avoid them.
>
I just copied the cvs driver without changing more than necessary.
Shouldn't this be up to the lm_sensors folk?
Jan
On Sun, Mar 23, 2003 at 10:33:34PM +0100, Jan Dittmer wrote:
> I just copied the cvs driver without changing more than necessary.
> Shouldn't this be up to the lm_sensors folk?
Not if you want the driver merged :)
Christoph Hellwig wrote:
> On Sun, Mar 23, 2003 at 09:36:10PM +0100, Dominik Kubla wrote:
>
>>Why? It's a valid C99 feature and since the kernel already uses C99
>>initializers it won't compile with compilers that choke on C99 comments
>>anyway.
>
>
> Because there's a strong preference for traditional C style in the kernel.
> typedefs are also a valid C feature and we try to avoid them.
>
Anyway, here is a corrected version.
Jan
On 2003-03-23, Christoph Hellwig wrote:
>Because there's a strong preference for traditional C style in the kernel.
>typedefs are also a valid C feature and we try to avoid them.
It's not so simple. I think trade-offs of maintainability
versus other benefits determine these preferences on a
feature-by-feature basis.
GNU features, such as inline routines, asm, __section__, and
variable sized arrays on the stack are used because the performance
benefits or other capabilities that they enable are generally
perceived to outweigh the loss in portability (although I think
__section__ is only used in macros).
Function prototypes are used consistently throughout the
kernel and are a newer feature than typedefs, presumably because the
benefits of the type checking are greater than the benefits of
compatability with old K&R compilers. Indeed, the benefits of such
compatability are apparently perceived as small enough, that they're
not even worth the readability costs of using macros to support
compilers with and without function prototypes.
The kernel frequently uses typedefs for sizing integer fields,
presumably because the benefits of easing cross-platform portability
and occasional changes to these fields' sizes are perceived to
outweigh the fact you have to look up or remember the definitions
types if you want to know their exact values from the source code.
On the other hand, it is true that typedefs seem to be avoided
in certain cases. The kernel usually does not use typedefs for
structs and enums. Those features provide their own name spaces, and
the readability benefits of seeing "struct" or "enum" in front of the
relevant type names is believed to outweigh the readability benefits
of having one less word to look at (but not immediately knowing if you
are looking at a struct, enum or other type).
There is also some interest in trying to be relatively
standard when the costs of doing so are small enough. For example,
most of the named initializers for struct fields have now been
converted from GNU style to C99 style.
In the case of the traditional C versus C++ style comments,
using one comment type throughout the kernel may have some slight
readability benefits. Also, partly because we have inline routines
that usually eliminate the performance cost of breaking up routines
into smaller carefully named routines for documentation purposes,
"chapter 5" of Documentation/CodingStyle says, "try to avoid putting
comments in a function body", a style for which the more
block-oriented "/*...*/" syntax is perhaps better suited.
On the other hand '//' appears in 1805 .c and .h files in
linux-2.5.65-bk4, so, while I would prefer sticking with C style
comments, it does not appear to be a requirement for integration into
the stock kernel. Given that your use of this feature seems to be for
a comment block, I would still encourage you to use traditional C
style comments, although it's certainly not my call.
Adam J. Richter __ ______________ 575 Oroville Road
[email protected] \ / Milpitas, California 95035
+1 408 309-6081 | g g d r a s i l United States of America
"Free Software For The Rest Of Us."
On Sun, Mar 23, 2003 at 10:38:43PM +0100, Jan Dittmer wrote:
> Christoph Hellwig wrote:
> >On Sun, Mar 23, 2003 at 09:36:10PM +0100, Dominik Kubla wrote:
> >
> >>Why? It's a valid C99 feature and since the kernel already uses C99
> >>initializers it won't compile with compilers that choke on C99 comments
> >>anyway.
> >
> >
> >Because there's a strong preference for traditional C style in the kernel.
> >typedefs are also a valid C feature and we try to avoid them.
> >
> Anyway, here is a corrected version.
Oops, one other thing. The pci_device_id structure should be
initialized by using the .field = method, not the way the driver is
currently.
Oh, and one patch that adds the Kconfig, Makefile, and driver to the
tree would be great.
thanks again,
greg k-h
On Sun, Mar 23, 2003 at 10:38:43PM +0100, Jan Dittmer wrote:
> Christoph Hellwig wrote:
> >On Sun, Mar 23, 2003 at 09:36:10PM +0100, Dominik Kubla wrote:
> >
> >>Why? It's a valid C99 feature and since the kernel already uses C99
> >>initializers it won't compile with compilers that choke on C99 comments
> >>anyway.
> >
> >
> >Because there's a strong preference for traditional C style in the kernel.
> >typedefs are also a valid C feature and we try to avoid them.
> >
> Anyway, here is a corrected version.
> Jan
Looks good, thanks.
But could you also convert all of the printk() calls to use the dev_*()
calls instead? That will also fix the problem that none of those calls
in this driver are using the KERN_* levels for printk(), which is
required.
Other than that minor thing, looks a lot better, thanks.
greg k-h
diff -urN clean/drivers/i2c/chips/Kconfig work/drivers/i2c/chips/Kconfig
--- clean/drivers/i2c/chips/Kconfig 2003-03-21 12:42:02.000000000 +0100
+++ work/drivers/i2c/chips/Kconfig 2003-03-21 23:23:05.000000000 +0100
@@ -37,4 +37,17 @@
in the lm_sensors package, which you can download at
http://www.lm-sensors.nu
+config SENSORS_VIA686A
+ tristate " VIA686A"
+ depends on I2C && I2C_PROC
+ help
+ support for via686a
+ If you say yes here you get support for the integrated sensors in
+ Via 686A/B South Bridges. This can also be built as a module
+ which can be inserted and removed while the kernel is running.
+
+ You will also need the latest user-space utilties: you can find them
+ in the lm_sensors package, which you can download at
+ http://www.lm-sensors.nu
+
endmenu
diff -urN clean/drivers/i2c/chips/Makefile work/drivers/i2c/chips/Makefile
--- clean/drivers/i2c/chips/Makefile 2003-03-21 12:42:02.000000000 +0100
+++ work/drivers/i2c/chips/Makefile 2003-03-21 23:23:01.000000000 +0100
@@ -4,3 +4,4 @@
obj-$(CONFIG_SENSORS_ADM1021) += adm1021.o
obj-$(CONFIG_SENSORS_LM75) += lm75.o
+obj-$(CONFIG_SENSORS_VIA686A) += via686a.o
On Tue, Mar 25, 2003 at 10:12:43AM +0100, Jan Dittmer wrote:
> Greg KH wrote:
> >On Sun, Mar 23, 2003 at 10:38:43PM +0100, Jan Dittmer wrote:
> >
> >>Anyway, here is a corrected version.
> >
> >
> >Oops, one other thing. The pci_device_id structure should be
> >initialized by using the .field = method, not the way the driver is
> >currently.
> >
> >Oh, and one patch that adds the Kconfig, Makefile, and driver to the
> >tree would be great.
> >
> I included that one with the first mail, but here it is again together
> with the hopefully correctly fixed driver.
Yes, but you didn't include it all in one patch. It's tough to apply a
.c file with 'patch' :)
Also, you need to set up the adapter.dev.parent pointer before calling
i2c_add_adapter().
Can you make that one change, and send a single patch?
thanks,
greg k-h