Hi,
first off, a short description would have been nice, would make it easier to
understand the problem and your solution.
So, from what I could see, this driver is already in here for 1 year and 4
months, though I could not yet find it upstream. How comes? Last time I
submitted a driver to hwmon, it made it into mainline within less than 4
months. And the hwmon-maintainer, Guenter Roeck, is very experienced and could
have pointed you to some of the problems in that driver. And keep in mind, that
the patches in openwrt should just be a temporary solution (also to backport
drivers to older kernel versions), but should go upstream ASAP.
Btw, a link to the datasheet (which is mandatory for hwmon) would also be of
big help for reviewing.
Some comments inline.
Tim Harvey schrieb:
> Signed-off-by: Tim Harvey <thar...@gateworks.com>
> ---
> .../generic/patches-3.10/880-gateworks_system_controller.patch | 7
> +++++--
> .../generic/patches-3.12/880-gateworks_system_controller.patch | 7
> +++++--
> .../generic/patches-3.13/880-gateworks_system_controller.patch | 7
> +++++--
> .../generic/patches-3.3/880-gateworks_system_controller.patch | 7
> +++++--
> .../generic/patches-3.6/880-gateworks_system_controller.patch | 7
> +++++--
> .../generic/patches-3.8/880-gateworks_system_controller.patch | 7
> +++++--
> .../generic/patches-3.9/880-gateworks_system_controller.patch | 7
> +++++--
> 7 files changed, 35 insertions(+), 14 deletions(-)
>
> diff --git
> a/target/linux/generic/patches-3.10/880-gateworks_system_controller.patch
> b/target/linux/generic/patches-3.10/880-gateworks_system_controller.patch
> index d923485..450551a 100644
> --- a/target/linux/generic/patches-3.10/880-gateworks_system_controller.patch
> +++ b/target/linux/generic/patches-3.10/880-gateworks_system_controller.patch
> @@ -28,7 +28,7 @@
>
> --- /dev/null
> +++ b/drivers/hwmon/gsc.c
> -@@ -0,0 +1,308 @@
> +@@ -0,0 +1,311 @@
> +/*
> + * A hwmon driver for the Gateworks System Controller
> + * Copyright (C) 2009 Gateworks Corporation
> @@ -132,11 +132,14 @@
> + */
> +static inline int gsp_read(struct i2c_client *client, u8 reg)
> +{
> -+ unsigned int adc = 0;
> ++ int adc = 0;
adc should be of type s16, and for the config you would need an int ret
> + if (reg == GSP_REG_TEMP_IN || reg > GSP_REG_CURRENT)
> + {
> + adc |= i2c_smbus_read_byte_data(client, reg);
> + adc |= i2c_smbus_read_byte_data(client, reg + 1) << 8;
adc = i2c_smbus_read_word_data(client, reg);
> ++ if (adc > 0x8000) { /* neg temps from two's-complement */
> ++ adc = adc - 0xffff;
> ++ }
not needed, you just may have to check if adc gets casted to int during return.
> + return adc;
> + }
> + else
Some error-check after calling a function is also the regular behavior when
coding properly. Then, from what I remember, hwmon also moved to the devm-API
about a year ago.
Take care
Hartmut
> diff --git
> a/target/linux/generic/patches-3.12/880-gateworks_system_controller.patch
> b/target/linux/generic/patches-3.12/880-gateworks_system_controller.patch
> index 6e4ae30..bf521f5 100644
> --- a/target/linux/generic/patches-3.12/880-gateworks_system_controller.patch
> +++ b/target/linux/generic/patches-3.12/880-gateworks_system_controller.patch
> @@ -28,7 +28,7 @@
>
> --- /dev/null
> +++ b/drivers/hwmon/gsc.c
> -@@ -0,0 +1,308 @@
> +@@ -0,0 +1,311 @@
> +/*
> + * A hwmon driver for the Gateworks System Controller
> + * Copyright (C) 2009 Gateworks Corporation
> @@ -132,11 +132,14 @@
> + */
> +static inline int gsp_read(struct i2c_client *client, u8 reg)
> +{
> -+ unsigned int adc = 0;
> ++ int adc = 0;
> + if (reg == GSP_REG_TEMP_IN || reg > GSP_REG_CURRENT)
> + {
> + adc |= i2c_smbus_read_byte_data(client, reg);
> + adc |= i2c_smbus_read_byte_data(client, reg + 1) << 8;
> ++ if (adc > 0x8000) { /* neg temps from two's-complement */
> ++ adc = adc - 0xffff;
> ++ }
> + return adc;
> + }
> + else
> diff --git
> a/target/linux/generic/patches-3.13/880-gateworks_system_controller.patch
> b/target/linux/generic/patches-3.13/880-gateworks_system_controller.patch
> index 6e4ae30..bf521f5 100644
> --- a/target/linux/generic/patches-3.13/880-gateworks_system_controller.patch
> +++ b/target/linux/generic/patches-3.13/880-gateworks_system_controller.patch
> @@ -28,7 +28,7 @@
>
> --- /dev/null
> +++ b/drivers/hwmon/gsc.c
> -@@ -0,0 +1,308 @@
> +@@ -0,0 +1,311 @@
> +/*
> + * A hwmon driver for the Gateworks System Controller
> + * Copyright (C) 2009 Gateworks Corporation
> @@ -132,11 +132,14 @@
> + */
> +static inline int gsp_read(struct i2c_client *client, u8 reg)
> +{
> -+ unsigned int adc = 0;
> ++ int adc = 0;
> + if (reg == GSP_REG_TEMP_IN || reg > GSP_REG_CURRENT)
> + {
> + adc |= i2c_smbus_read_byte_data(client, reg);
> + adc |= i2c_smbus_read_byte_data(client, reg + 1) << 8;
> ++ if (adc > 0x8000) { /* neg temps from two's-complement */
> ++ adc = adc - 0xffff;
> ++ }
> + return adc;
> + }
> + else
> diff --git
> a/target/linux/generic/patches-3.3/880-gateworks_system_controller.patch
> b/target/linux/generic/patches-3.3/880-gateworks_system_controller.patch
> index d9a6481..82fa617 100644
> --- a/target/linux/generic/patches-3.3/880-gateworks_system_controller.patch
> +++ b/target/linux/generic/patches-3.3/880-gateworks_system_controller.patch
> @@ -28,7 +28,7 @@
>
> --- /dev/null
> +++ b/drivers/hwmon/gsc.c
> -@@ -0,0 +1,308 @@
> +@@ -0,0 +1,311 @@
> +/*
> + * A hwmon driver for the Gateworks System Controller
> + * Copyright (C) 2009 Gateworks Corporation
> @@ -132,11 +132,14 @@
> + */
> +static inline int gsp_read(struct i2c_client *client, u8 reg)
> +{
> -+ unsigned int adc = 0;
> ++ int adc = 0;
> + if (reg == GSP_REG_TEMP_IN || reg > GSP_REG_CURRENT)
> + {
> + adc |= i2c_smbus_read_byte_data(client, reg);
> + adc |= i2c_smbus_read_byte_data(client, reg + 1) << 8;
> ++ if (adc > 0x8000) { /* neg temps from two's-complement */
> ++ adc = adc - 0xffff;
> ++ }
> + return adc;
> + }
> + else
> diff --git
> a/target/linux/generic/patches-3.6/880-gateworks_system_controller.patch
> b/target/linux/generic/patches-3.6/880-gateworks_system_controller.patch
> index c9690d7..59d3326 100644
> --- a/target/linux/generic/patches-3.6/880-gateworks_system_controller.patch
> +++ b/target/linux/generic/patches-3.6/880-gateworks_system_controller.patch
> @@ -28,7 +28,7 @@
>
> --- /dev/null
> +++ b/drivers/hwmon/gsc.c
> -@@ -0,0 +1,308 @@
> +@@ -0,0 +1,311 @@
> +/*
> + * A hwmon driver for the Gateworks System Controller
> + * Copyright (C) 2009 Gateworks Corporation
> @@ -132,11 +132,14 @@
> + */
> +static inline int gsp_read(struct i2c_client *client, u8 reg)
> +{
> -+ unsigned int adc = 0;
> ++ int adc = 0;
> + if (reg == GSP_REG_TEMP_IN || reg > GSP_REG_CURRENT)
> + {
> + adc |= i2c_smbus_read_byte_data(client, reg);
> + adc |= i2c_smbus_read_byte_data(client, reg + 1) << 8;
> ++ if (adc > 0x8000) { /* neg temps from two's-complement */
> ++ adc = adc - 0xffff;
> ++ }
> + return adc;
> + }
> + else
> diff --git
> a/target/linux/generic/patches-3.8/880-gateworks_system_controller.patch
> b/target/linux/generic/patches-3.8/880-gateworks_system_controller.patch
> index 1ec0960..0417189 100644
> --- a/target/linux/generic/patches-3.8/880-gateworks_system_controller.patch
> +++ b/target/linux/generic/patches-3.8/880-gateworks_system_controller.patch
> @@ -28,7 +28,7 @@
>
> --- /dev/null
> +++ b/drivers/hwmon/gsc.c
> -@@ -0,0 +1,308 @@
> +@@ -0,0 +1,311 @@
> +/*
> + * A hwmon driver for the Gateworks System Controller
> + * Copyright (C) 2009 Gateworks Corporation
> @@ -132,11 +132,14 @@
> + */
> +static inline int gsp_read(struct i2c_client *client, u8 reg)
> +{
> -+ unsigned int adc = 0;
> ++ int adc = 0;
> + if (reg == GSP_REG_TEMP_IN || reg > GSP_REG_CURRENT)
> + {
> + adc |= i2c_smbus_read_byte_data(client, reg);
> + adc |= i2c_smbus_read_byte_data(client, reg + 1) << 8;
> ++ if (adc > 0x8000) { /* neg temps from two's-complement */
> ++ adc = adc - 0xffff;
> ++ }
> + return adc;
> + }
> + else
> diff --git
> a/target/linux/generic/patches-3.9/880-gateworks_system_controller.patch
> b/target/linux/generic/patches-3.9/880-gateworks_system_controller.patch
> index 3bcca9c..03d39b9 100644
> --- a/target/linux/generic/patches-3.9/880-gateworks_system_controller.patch
> +++ b/target/linux/generic/patches-3.9/880-gateworks_system_controller.patch
> @@ -28,7 +28,7 @@
>
> --- /dev/null
> +++ b/drivers/hwmon/gsc.c
> -@@ -0,0 +1,308 @@
> +@@ -0,0 +1,311 @@
> +/*
> + * A hwmon driver for the Gateworks System Controller
> + * Copyright (C) 2009 Gateworks Corporation
> @@ -132,11 +132,14 @@
> + */
> +static inline int gsp_read(struct i2c_client *client, u8 reg)
> +{
> -+ unsigned int adc = 0;
> ++ int adc = 0;
> + if (reg == GSP_REG_TEMP_IN || reg > GSP_REG_CURRENT)
> + {
> + adc |= i2c_smbus_read_byte_data(client, reg);
> + adc |= i2c_smbus_read_byte_data(client, reg + 1) << 8;
> ++ if (adc > 0x8000) { /* neg temps from two's-complement */
> ++ adc = adc - 0xffff;
> ++ }
> + return adc;
> + }
> + else
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
