[lm-sensors] [PATCH] hwmon: (i5k_amb) Replace BUG with WARN

[lm-sensors] [PATCH] hwmon: (i5k_amb) Replace BUG with WARN

[Guenter Roeck]

Executing BUG as a result of an internal driver error seems to be a bit harsh.
Replace it with WARN and return -ENODEV if the condition is seen.

This also resolves the following compile warning seen with some random
configurations.

drivers/hwmon/i5k_amb.c: In function 'i5k_channel_pci_id':
drivers/hwmon/i5k_amb.c:492: warning: control reaches end of non-void function

Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
---
 drivers/hwmon/i5k_amb.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/hwmon/i5k_amb.c b/drivers/hwmon/i5k_amb.c
index c4c40be..24e37e2 100644
--- a/drivers/hwmon/i5k_amb.c
+++ b/drivers/hwmon/i5k_amb.c
@@ -478,8 +478,7 @@ out:
 	return res;
 }
 
-static unsigned long i5k_channel_pci_id(struct i5k_amb_data *data,
-					unsigned long channel)
+static long i5k_channel_pci_id(struct i5k_amb_data *data, unsigned long channel)
 {
 	switch (data->chipset_id) {
 	case PCI_DEVICE_ID_INTEL_5000_ERR:
@@ -487,7 +486,8 @@ static unsigned long i5k_channel_pci_id(struct i5k_amb_data *data,
 	case PCI_DEVICE_ID_INTEL_5400_ERR:
 		return PCI_DEVICE_ID_INTEL_5400_FBD0 + channel;
 	default:
-		BUG();
+		WARN(1, "Unexpected chipset ID 0x%lx\n", data->chipset_id);
+		return -ENODEV;
 	}
 }
 
@@ -528,14 +528,18 @@ static int __devinit i5k_amb_probe(struct platform_device *pdev)
 		goto err;
 
 	/* Copy the DIMM presence map for the first two channels */
-	res = i5k_channel_probe(&data->amb_present[0],
-				i5k_channel_pci_id(data, 0));
+	res = i5k_channel_pci_id(data, 0);
+	if (res < 0)
+		goto err;
+	res = i5k_channel_probe(&data->amb_present[0], res);
 	if (res)
 		goto err;
 
 	/* Copy the DIMM presence map for the optional second two channels */
-	i5k_channel_probe(&data->amb_present[2],
-			  i5k_channel_pci_id(data, 1));
+	res = i5k_channel_pci_id(data, 1);
+	if (res < 0)
+		goto err;
+	i5k_channel_probe(&data->amb_present[2], res);
 
 	/* Set up resource regions */
 	reso = request_mem_region(data->amb_base, data->amb_len, DRVNAME);
-- 
1.7.3.1


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (i5k_amb) Replace BUG with WARN

[Darrick J. Wong]

On Thu, Aug 18, 2011 at 10:49:31AM -0700, Guenter Roeck wrote:
> Executing BUG as a result of an internal driver error seems to be a bit harsh.
> Replace it with WARN and return -ENODEV if the condition is seen.
> 
> This also resolves the following compile warning seen with some random
> configurations.
> 
> drivers/hwmon/i5k_amb.c: In function 'i5k_channel_pci_id':
> drivers/hwmon/i5k_amb.c:492: warning: control reaches end of non-void function
> 
> Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> ---
>  drivers/hwmon/i5k_amb.c |   18 +++++++++++-------
>  1 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/hwmon/i5k_amb.c b/drivers/hwmon/i5k_amb.c
> index c4c40be..24e37e2 100644
> --- a/drivers/hwmon/i5k_amb.c
> +++ b/drivers/hwmon/i5k_amb.c
> @@ -478,8 +478,7 @@ out:
>  	return res;
>  }
> 
> -static unsigned long i5k_channel_pci_id(struct i5k_amb_data *data,
> -					unsigned long channel)
> +static long i5k_channel_pci_id(struct i5k_amb_data *data, unsigned long channel)
>  {
>  	switch (data->chipset_id) {
>  	case PCI_DEVICE_ID_INTEL_5000_ERR:
> @@ -487,7 +486,8 @@ static unsigned long i5k_channel_pci_id(struct i5k_amb_data *data,
>  	case PCI_DEVICE_ID_INTEL_5400_ERR:
>  		return PCI_DEVICE_ID_INTEL_5400_FBD0 + channel;
>  	default:
> -		BUG();
> +		WARN(1, "Unexpected chipset ID 0x%lx\n", data->chipset_id);
> +		return -ENODEV;
>  	}
>  }
> 
> @@ -528,14 +528,18 @@ static int __devinit i5k_amb_probe(struct platform_device *pdev)
>  		goto err;
> 
>  	/* Copy the DIMM presence map for the first two channels */
> -	res = i5k_channel_probe(&data->amb_present[0],
> -				i5k_channel_pci_id(data, 0));
> +	res = i5k_channel_pci_id(data, 0);
> +	if (res < 0)
> +		goto err;
> +	res = i5k_channel_probe(&data->amb_present[0], res);
>  	if (res)
>  		goto err;
> 
>  	/* Copy the DIMM presence map for the optional second two channels */
> -	i5k_channel_probe(&data->amb_present[2],
> -			  i5k_channel_pci_id(data, 1));
> +	res = i5k_channel_pci_id(data, 1);
> +	if (res < 0)
> +		goto err;
> +	i5k_channel_probe(&data->amb_present[2], res);
> 
>  	/* Set up resource regions */
>  	reso = request_mem_region(data->amb_base, data->amb_len, DRVNAME);

Looks good to me, so you can add:
Acked-by: Darrick J. Wong <djwong@us.ibm.com>

--D

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (i5k_amb) Replace BUG with WARN

[Jean Delvare]

Hi Guenter,

On Thu, 18 Aug 2011 10:49:31 -0700, Guenter Roeck wrote:
> Executing BUG as a result of an internal driver error seems to be a bit harsh.

Well, that's pretty much what BUG was designed for: internal errors
that aren't supposed to happen.

> Replace it with WARN and return -ENODEV if the condition is seen.

I'm not sure what is the benefit of WARN over pr_warn() in this case.

> 
> This also resolves the following compile warning seen with some random
> configurations.
> 
> drivers/hwmon/i5k_amb.c: In function 'i5k_channel_pci_id':
> drivers/hwmon/i5k_amb.c:492: warning: control reaches end of non-void function
> 
> Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> ---
>  drivers/hwmon/i5k_amb.c |   18 +++++++++++-------
>  1 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/hwmon/i5k_amb.c b/drivers/hwmon/i5k_amb.c
> index c4c40be..24e37e2 100644
> --- a/drivers/hwmon/i5k_amb.c
> +++ b/drivers/hwmon/i5k_amb.c
> @@ -478,8 +478,7 @@ out:
>  	return res;
>  }
>  
> -static unsigned long i5k_channel_pci_id(struct i5k_amb_data *data,
> -					unsigned long channel)
> +static long i5k_channel_pci_id(struct i5k_amb_data *data, unsigned long channel)
>  {
>  	switch (data->chipset_id) {
>  	case PCI_DEVICE_ID_INTEL_5000_ERR:
> @@ -487,7 +486,8 @@ static unsigned long i5k_channel_pci_id(struct i5k_amb_data *data,
>  	case PCI_DEVICE_ID_INTEL_5400_ERR:
>  		return PCI_DEVICE_ID_INTEL_5400_FBD0 + channel;
>  	default:
> -		BUG();
> +		WARN(1, "Unexpected chipset ID 0x%lx\n", data->chipset_id);
> +		return -ENODEV;
>  	}
>  }
>  
> @@ -528,14 +528,18 @@ static int __devinit i5k_amb_probe(struct platform_device *pdev)
>  		goto err;
>  
>  	/* Copy the DIMM presence map for the first two channels */
> -	res = i5k_channel_probe(&data->amb_present[0],
> -				i5k_channel_pci_id(data, 0));
> +	res = i5k_channel_pci_id(data, 0);
> +	if (res < 0)
> +		goto err;
> +	res = i5k_channel_probe(&data->amb_present[0], res);
>  	if (res)
>  		goto err;
>  
>  	/* Copy the DIMM presence map for the optional second two channels */
> -	i5k_channel_probe(&data->amb_present[2],
> -			  i5k_channel_pci_id(data, 1));
> +	res = i5k_channel_pci_id(data, 1);
> +	if (res < 0)

Note that this cannot happen, by construction. If the first call to
i5k_channel_pci_id succeeded then the second call must succeed too.

> +		goto err;
> +	i5k_channel_probe(&data->amb_present[2], res);
>  
>  	/* Set up resource regions */
>  	reso = request_mem_region(data->amb_base, data->amb_len, DRVNAME);

The whole problem here is that the way the code was designed makes it
seemingly possible for i5k_channel_pci_id to fail, while in practice I
don't think it ever can. The warning might be an opportunity to rework
it all so that not only the warning goes away but the code also gains
in clarity and efficiency. What do you think of the following patch?

Subject: hwmon: (i5k_amb) Drop i5k_channel_pci_id

Function i5k_channel_pci_id looks like it can fail, while a better
code design would make it more obvious that it can't. We can even get
rid of the function.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
---
Darrick, can you please test this alternative fix?

 drivers/hwmon/i5k_amb.c |   42 ++++++++++++++----------------------------
 1 file changed, 14 insertions(+), 28 deletions(-)

--- linux-3.1-rc2.orig/drivers/hwmon/i5k_amb.c	2011-07-22 04:17:23.000000000 +0200
+++ linux-3.1-rc2/drivers/hwmon/i5k_amb.c	2011-08-19 23:10:35.000000000 +0200
@@ -114,7 +114,6 @@ struct i5k_amb_data {
 	void __iomem *amb_mmio;
 	struct i5k_device_attribute *attrs;
 	unsigned int num_attrs;
-	unsigned long chipset_id;
 };
 
 static ssize_t show_name(struct device *dev, struct device_attribute *devattr,
@@ -444,8 +443,6 @@ static int __devinit i5k_find_amb_regist
 		goto out;
 	}
 
-	data->chipset_id = devid;
-
 	res = 0;
 out:
 	pci_dev_put(pcidev);
@@ -478,23 +475,13 @@ out:
 	return res;
 }
 
-static unsigned long i5k_channel_pci_id(struct i5k_amb_data *data,
-					unsigned long channel)
-{
-	switch (data->chipset_id) {
-	case PCI_DEVICE_ID_INTEL_5000_ERR:
-		return PCI_DEVICE_ID_INTEL_5000_FBD0 + channel;
-	case PCI_DEVICE_ID_INTEL_5400_ERR:
-		return PCI_DEVICE_ID_INTEL_5400_FBD0 + channel;
-	default:
-		BUG();
-	}
-}
-
-static unsigned long chipset_ids[] = {
-	PCI_DEVICE_ID_INTEL_5000_ERR,
-	PCI_DEVICE_ID_INTEL_5400_ERR,
-	0
+static struct {
+	unsigned long err;
+	unsigned long fbd0;
+} chipset_ids[] __devinitdata  = {
+	{ PCI_DEVICE_ID_INTEL_5000_ERR, PCI_DEVICE_ID_INTEL_5000_FBD0 },
+	{ PCI_DEVICE_ID_INTEL_5400_ERR, PCI_DEVICE_ID_INTEL_5400_FBD0 },
+	{ 0, 0 }
 };
 
 #ifdef MODULE
@@ -510,8 +497,7 @@ static int __devinit i5k_amb_probe(struc
 {
 	struct i5k_amb_data *data;
 	struct resource *reso;
-	int i;
-	int res = -ENODEV;
+	int i, res;
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -520,22 +506,22 @@ static int __devinit i5k_amb_probe(struc
 	/* Figure out where the AMB registers live */
 	i = 0;
 	do {
-		res = i5k_find_amb_registers(data, chipset_ids[i]);
+		res = i5k_find_amb_registers(data, chipset_ids[i].err);
+		if (res = 0)
+			break;
 		i++;
-	} while (res && chipset_ids[i]);
+	} while (chipset_ids[i].err);
 
 	if (res)
 		goto err;
 
 	/* Copy the DIMM presence map for the first two channels */
-	res = i5k_channel_probe(&data->amb_present[0],
-				i5k_channel_pci_id(data, 0));
+	res = i5k_channel_probe(&data->amb_present[0], chipset_ids[i].fbd0);
 	if (res)
 		goto err;
 
 	/* Copy the DIMM presence map for the optional second two channels */
-	i5k_channel_probe(&data->amb_present[2],
-			  i5k_channel_pci_id(data, 1));
+	i5k_channel_probe(&data->amb_present[2], chipset_ids[i].fbd0 + 1);
 
 	/* Set up resource regions */
 	reso = request_mem_region(data->amb_base, data->amb_len, DRVNAME);


-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (i5k_amb) Replace BUG with WARN

[Guenter Roeck]

Hi Jean,

On Fri, Aug 19, 2011 at 05:16:24PM -0400, Jean Delvare wrote:
> Hi Guenter,
> 
> On Thu, 18 Aug 2011 10:49:31 -0700, Guenter Roeck wrote:
> > Executing BUG as a result of an internal driver error seems to be a bit harsh.
> 
> Well, that's pretty much what BUG was designed for: internal errors
> that aren't supposed to happen.
> 
Problem with BUG is that it can be undefined for some configurations,
in which case it doesn't do anything. 

Also, while it may make sense to bring down the system if there is a severe bug 
which makes it impossible to continue, that seems to be a bit excessive
for a hwmon driver.

> > Replace it with WARN and return -ENODEV if the condition is seen.
> 
> I'm not sure what is the benefit of WARN over pr_warn() in this case.
> 
WARN generates a traceback, pr_warn doesn't. I think the scope is a bit different
- one warns about a programming error, the other warns about a system consition.

[ ... ]
> 
> The whole problem here is that the way the code was designed makes it
> seemingly possible for i5k_channel_pci_id to fail, while in practice I
> don't think it ever can. The warning might be an opportunity to rework
> it all so that not only the warning goes away but the code also gains
> in clarity and efficiency. What do you think of the following patch?
> 
> Subject: hwmon: (i5k_amb) Drop i5k_channel_pci_id
> 
> Function i5k_channel_pci_id looks like it can fail, while a better
> code design would make it more obvious that it can't. We can even get
> rid of the function.
> 
> Signed-off-by: Jean Delvare <khali@linux-fr.org>

Better and cleaner than mine ... I'll be happy to take it if Darrick Acks it.

Thanks,
Guenter

> ---
> Darrick, can you please test this alternative fix?
> 
>  drivers/hwmon/i5k_amb.c |   42 ++++++++++++++----------------------------
>  1 file changed, 14 insertions(+), 28 deletions(-)
> 
> --- linux-3.1-rc2.orig/drivers/hwmon/i5k_amb.c	2011-07-22 04:17:23.000000000 +0200
> +++ linux-3.1-rc2/drivers/hwmon/i5k_amb.c	2011-08-19 23:10:35.000000000 +0200
> @@ -114,7 +114,6 @@ struct i5k_amb_data {
>  	void __iomem *amb_mmio;
>  	struct i5k_device_attribute *attrs;
>  	unsigned int num_attrs;
> -	unsigned long chipset_id;
>  };
>  
>  static ssize_t show_name(struct device *dev, struct device_attribute *devattr,
> @@ -444,8 +443,6 @@ static int __devinit i5k_find_amb_regist
>  		goto out;
>  	}
>  
> -	data->chipset_id = devid;
> -
>  	res = 0;
>  out:
>  	pci_dev_put(pcidev);
> @@ -478,23 +475,13 @@ out:
>  	return res;
>  }
>  
> -static unsigned long i5k_channel_pci_id(struct i5k_amb_data *data,
> -					unsigned long channel)
> -{
> -	switch (data->chipset_id) {
> -	case PCI_DEVICE_ID_INTEL_5000_ERR:
> -		return PCI_DEVICE_ID_INTEL_5000_FBD0 + channel;
> -	case PCI_DEVICE_ID_INTEL_5400_ERR:
> -		return PCI_DEVICE_ID_INTEL_5400_FBD0 + channel;
> -	default:
> -		BUG();
> -	}
> -}
> -
> -static unsigned long chipset_ids[] = {
> -	PCI_DEVICE_ID_INTEL_5000_ERR,
> -	PCI_DEVICE_ID_INTEL_5400_ERR,
> -	0
> +static struct {
> +	unsigned long err;
> +	unsigned long fbd0;
> +} chipset_ids[] __devinitdata  = {
> +	{ PCI_DEVICE_ID_INTEL_5000_ERR, PCI_DEVICE_ID_INTEL_5000_FBD0 },
> +	{ PCI_DEVICE_ID_INTEL_5400_ERR, PCI_DEVICE_ID_INTEL_5400_FBD0 },
> +	{ 0, 0 }
>  };
>  
>  #ifdef MODULE
> @@ -510,8 +497,7 @@ static int __devinit i5k_amb_probe(struc
>  {
>  	struct i5k_amb_data *data;
>  	struct resource *reso;
> -	int i;
> -	int res = -ENODEV;
> +	int i, res;
>  
>  	data = kzalloc(sizeof(*data), GFP_KERNEL);
>  	if (!data)
> @@ -520,22 +506,22 @@ static int __devinit i5k_amb_probe(struc
>  	/* Figure out where the AMB registers live */
>  	i = 0;
>  	do {
> -		res = i5k_find_amb_registers(data, chipset_ids[i]);
> +		res = i5k_find_amb_registers(data, chipset_ids[i].err);
> +		if (res = 0)
> +			break;
>  		i++;
> -	} while (res && chipset_ids[i]);
> +	} while (chipset_ids[i].err);
>  
>  	if (res)
>  		goto err;
>  
>  	/* Copy the DIMM presence map for the first two channels */
> -	res = i5k_channel_probe(&data->amb_present[0],
> -				i5k_channel_pci_id(data, 0));
> +	res = i5k_channel_probe(&data->amb_present[0], chipset_ids[i].fbd0);
>  	if (res)
>  		goto err;
>  
>  	/* Copy the DIMM presence map for the optional second two channels */
> -	i5k_channel_probe(&data->amb_present[2],
> -			  i5k_channel_pci_id(data, 1));
> +	i5k_channel_probe(&data->amb_present[2], chipset_ids[i].fbd0 + 1);
>  
>  	/* Set up resource regions */
>  	reso = request_mem_region(data->amb_base, data->amb_len, DRVNAME);
> 
> 
> -- 
> Jean Delvare

-- 
Guenter Roeck
Distinguished Engineer
PDU IP Systems

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (i5k_amb) Replace BUG with WARN

[Guenter Roeck]

On Fri, Aug 19, 2011 at 07:05:50PM -0400, Guenter Roeck wrote:
> Hi Jean,
> 
> On Fri, Aug 19, 2011 at 05:16:24PM -0400, Jean Delvare wrote:
> > Hi Guenter,
> > 
> > On Thu, 18 Aug 2011 10:49:31 -0700, Guenter Roeck wrote:
> > > Executing BUG as a result of an internal driver error seems to be a bit harsh.
> > 
> > Well, that's pretty much what BUG was designed for: internal errors
> > that aren't supposed to happen.
> > 
> Problem with BUG is that it can be undefined for some configurations,
> in which case it doesn't do anything. 
> 
> Also, while it may make sense to bring down the system if there is a severe bug 
> which makes it impossible to continue, that seems to be a bit excessive
> for a hwmon driver.
> 
> > > Replace it with WARN and return -ENODEV if the condition is seen.
> > 
> > I'm not sure what is the benefit of WARN over pr_warn() in this case.
> > 
> WARN generates a traceback, pr_warn doesn't. I think the scope is a bit different
> - one warns about a programming error, the other warns about a system consition.
> 
Hi all,

As I don't see a consensus for my proposed patch, I'll drop it.

Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (i5k_amb) Replace BUG with WARN

[Darrick J. Wong]

On Mon, Aug 22, 2011 at 09:01:07PM -0700, Guenter Roeck wrote:
> On Fri, Aug 19, 2011 at 07:05:50PM -0400, Guenter Roeck wrote:
> > Hi Jean,
> > 
> > On Fri, Aug 19, 2011 at 05:16:24PM -0400, Jean Delvare wrote:
> > > Hi Guenter,
> > > 
> > > On Thu, 18 Aug 2011 10:49:31 -0700, Guenter Roeck wrote:
> > > > Executing BUG as a result of an internal driver error seems to be a bit harsh.
> > > 
> > > Well, that's pretty much what BUG was designed for: internal errors
> > > that aren't supposed to happen.
> > > 
> > Problem with BUG is that it can be undefined for some configurations,
> > in which case it doesn't do anything. 
> > 
> > Also, while it may make sense to bring down the system if there is a severe bug 
> > which makes it impossible to continue, that seems to be a bit excessive
> > for a hwmon driver.
> > 
> > > > Replace it with WARN and return -ENODEV if the condition is seen.
> > > 
> > > I'm not sure what is the benefit of WARN over pr_warn() in this case.
> > > 
> > WARN generates a traceback, pr_warn doesn't. I think the scope is a bit different
> > - one warns about a programming error, the other warns about a system consition.
> > 
> Hi all,
> 
> As I don't see a consensus for my proposed patch, I'll drop it.

Sorry, I'm just slow at testing things sometimes. :(

In general I like the idea of replacing BUG with WARN when possible,
particularly with hardware detection.  Your patch seems to load ok on all the
systems I can still claw together, so,

Acked-by: Darrick J. Wong <djwong@us.ibm.com>

--D
> 
> Guenter
> 
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (i5k_amb) Replace BUG with WARN

[Guenter Roeck]

On Tue, 2011-08-23 at 12:55 -0400, Darrick J. Wong wrote:
> On Mon, Aug 22, 2011 at 09:01:07PM -0700, Guenter Roeck wrote:
> > On Fri, Aug 19, 2011 at 07:05:50PM -0400, Guenter Roeck wrote:
> > > Hi Jean,
> > > 
> > > On Fri, Aug 19, 2011 at 05:16:24PM -0400, Jean Delvare wrote:
> > > > Hi Guenter,
> > > > 
> > > > On Thu, 18 Aug 2011 10:49:31 -0700, Guenter Roeck wrote:
> > > > > Executing BUG as a result of an internal driver error seems to be a bit harsh.
> > > > 
> > > > Well, that's pretty much what BUG was designed for: internal errors
> > > > that aren't supposed to happen.
> > > > 
> > > Problem with BUG is that it can be undefined for some configurations,
> > > in which case it doesn't do anything. 
> > > 
> > > Also, while it may make sense to bring down the system if there is a severe bug 
> > > which makes it impossible to continue, that seems to be a bit excessive
> > > for a hwmon driver.
> > > 
> > > > > Replace it with WARN and return -ENODEV if the condition is seen.
> > > > 
> > > > I'm not sure what is the benefit of WARN over pr_warn() in this case.
> > > > 
> > > WARN generates a traceback, pr_warn doesn't. I think the scope is a bit different
> > > - one warns about a programming error, the other warns about a system consition.
> > > 
> > Hi all,
> > 
> > As I don't see a consensus for my proposed patch, I'll drop it.
> 
> Sorry, I'm just slow at testing things sometimes. :(
> 
> In general I like the idea of replacing BUG with WARN when possible,
> particularly with hardware detection.  Your patch seems to load ok on all the
> systems I can still claw together, so,
> 
> Acked-by: Darrick J. Wong <djwong@us.ibm.com>

Hi Darrick,

thanks for the feedback.

Problem is that Jean disagreed with my solution and proposed a different
patch. I don't think my proposed patch is important enough to ignore or
override Jean's disagreement, so I decided to drop it.

Thanks,
Guenter



_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (i5k_amb) Replace BUG with WARN

[Jean Delvare]

On Tue, 23 Aug 2011 10:14:41 -0700, Guenter Roeck wrote:
> On Tue, 2011-08-23 at 12:55 -0400, Darrick J. Wong wrote:
> > On Mon, Aug 22, 2011 at 09:01:07PM -0700, Guenter Roeck wrote:
> > > As I don't see a consensus for my proposed patch, I'll drop it.
> > 
> > Sorry, I'm just slow at testing things sometimes. :(
> > 
> > In general I like the idea of replacing BUG with WARN when possible,
> > particularly with hardware detection.  Your patch seems to load ok on all the
> > systems I can still claw together, so,
> > 
> > Acked-by: Darrick J. Wong <djwong@us.ibm.com>
> 
> Hi Darrick,
> 
> thanks for the feedback.
> 
> Problem is that Jean disagreed with my solution and proposed a different
> patch. I don't think my proposed patch is important enough to ignore or
> override Jean's disagreement, so I decided to drop it.

Darrick, any chance you could test my alternative patch? I posted it to
the list, see:

http://marc.info/?l=lm-sensors&m\x131378870513822&w=2

I thought I had Cc'd you but I'm not sure if you received that copy.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (i5k_amb) Replace BUG with WARN

[Darrick J. Wong]

On Tue, Aug 23, 2011 at 08:05:55PM +0200, Jean Delvare wrote:
> On Tue, 23 Aug 2011 10:14:41 -0700, Guenter Roeck wrote:
> > On Tue, 2011-08-23 at 12:55 -0400, Darrick J. Wong wrote:
> > > On Mon, Aug 22, 2011 at 09:01:07PM -0700, Guenter Roeck wrote:
> > > > As I don't see a consensus for my proposed patch, I'll drop it.
> > > 
> > > Sorry, I'm just slow at testing things sometimes. :(
> > > 
> > > In general I like the idea of replacing BUG with WARN when possible,
> > > particularly with hardware detection.  Your patch seems to load ok on all the
> > > systems I can still claw together, so,
> > > 
> > > Acked-by: Darrick J. Wong <djwong@us.ibm.com>
> > 
> > Hi Darrick,
> > 
> > thanks for the feedback.
> > 
> > Problem is that Jean disagreed with my solution and proposed a different
> > patch. I don't think my proposed patch is important enough to ignore or
> > override Jean's disagreement, so I decided to drop it.
> 
> Darrick, any chance you could test my alternative patch? I posted it to
> the list, see:
> 
> http://marc.info/?l=lm-sensors&m\x131378870513822&w=2
> 
> I thought I had Cc'd you but I'm not sure if you received that copy.

Oops, I actually meant to ack Jean's new patch, not the old one; I think I
replied to the wrong thread.  Serves me right for writing email before coffee.
:/

--D
> 
> -- 
> Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (i5k_amb) Replace BUG with WARN

[Guenter Roeck]

On Tue, Aug 23, 2011 at 06:28:01PM -0400, Darrick J. Wong wrote:
> On Tue, Aug 23, 2011 at 08:05:55PM +0200, Jean Delvare wrote:
> > On Tue, 23 Aug 2011 10:14:41 -0700, Guenter Roeck wrote:
> > > On Tue, 2011-08-23 at 12:55 -0400, Darrick J. Wong wrote:
> > > > On Mon, Aug 22, 2011 at 09:01:07PM -0700, Guenter Roeck wrote:
> > > > > As I don't see a consensus for my proposed patch, I'll drop it.
> > > > 
> > > > Sorry, I'm just slow at testing things sometimes. :(
> > > > 
> > > > In general I like the idea of replacing BUG with WARN when possible,
> > > > particularly with hardware detection.  Your patch seems to load ok on all the
> > > > systems I can still claw together, so,
> > > > 
> > > > Acked-by: Darrick J. Wong <djwong@us.ibm.com>
> > > 
> > > Hi Darrick,
> > > 
> > > thanks for the feedback.
> > > 
> > > Problem is that Jean disagreed with my solution and proposed a different
> > > patch. I don't think my proposed patch is important enough to ignore or
> > > override Jean's disagreement, so I decided to drop it.
> > 
> > Darrick, any chance you could test my alternative patch? I posted it to
> > the list, see:
> > 
> > http://marc.info/?l=lm-sensors&m\x131378870513822&w=2
> > 
> > I thought I had Cc'd you but I'm not sure if you received that copy.
> 
> Oops, I actually meant to ack Jean's new patch, not the old one; I think I
> replied to the wrong thread.  Serves me right for writing email before coffee.
> :/
> 

Guess that explains why you sent a second Ack to my patch ... I did wonder about that ;).

Thanks for the Ack - I applied Jean's patch.

Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors