| Message ID | 1305653342.2182.15.camel@anish-desktop (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On 17 May 2011 18:29, anish <anish198519851985@gmail.com> wrote: > When not able to allocate memory we were using KERN_INFO as > log level in printk so changed to KERN_ERR > Signed-off-by: anish kumar<anish198519851985@gmail.com> Maybe KERN_WARN?
On Wed, 2011-05-18 at 04:19 +0100, anish singh wrote: > On Tue, May 17, 2011 at 11:13 PM, Catalin Marinas <catalin.marinas@arm.com> wrote: > On 17 May 2011 18:29, anish <anish198519851985@gmail.com> wrote: > > When not able to allocate memory we were using KERN_INFO as > > log level in printk so changed to KERN_ERR > > Signed-off-by: anish kumar<anish198519851985@gmail.com> > > > Maybe KERN_WARN? > Then shouldn't we change below case also? > 467 ret = amba_request_regions(dev, NULL); > 468 if (ret) { > 469 printk(KERN_ERR "CLCD: unable to reserve regs region\n"); > 470 goto out; > > If yes,then i will resend the patch for this also. I think the register reserving is less likely to fail because of memory allocations but more because of overlapping regions, in which case it could be a programming error. Allocating a big framebuffer is likely to fail in some memory constraint systems but I don't consider this a kernel error. That's why I suggested warning.
On Wed, 2011-05-18 at 11:57 +0100, anish singh wrote: > On Wed, May 18, 2011 at 3:45 PM, Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Wed, 2011-05-18 at 04:19 +0100, anish singh wrote: > > On Tue, May 17, 2011 at 11:13 PM, Catalin Marinas <catalin.marinas@arm.com> wrote: > > On 17 May 2011 18:29, anish <anish198519851985@gmail.com> wrote: > > > When not able to allocate memory we were using KERN_INFO as > > > log level in printk so changed to KERN_ERR > > > Signed-off-by: anish kumar<anish198519851985@gmail.com> > > > > > > Maybe KERN_WARN? > > Then shouldn't we change below case also? > > 467 ret = amba_request_regions(dev, NULL); > > 468 if (ret) { > > 469 printk(KERN_ERR "CLCD: unable to reserve regs region\n"); > > 470 goto out; > > > > If yes,then i will resend the patch for this also. > > > I think the register reserving is less likely to fail because of memory > allocations but more because of overlapping regions, in which case it > could be a programming error. > > Allocating a big framebuffer is likely to fail in some memory constraint > systems but I don't consider this a kernel error. That's why I suggested > warning. > > Got it.So should i change both as KERN_WARN and resend? No, just the one KERN_INFO one. Anyway, Russell King is the maintainer of this driver so it's up to him to ack the patch (and don't forget to cc him).
On Wed, May 18, 2011 at 04:27:47PM +0530, anish singh wrote:
> Got it.So should i change both as KERN_WARN and resend?
Much better would be to change it to pr_warn() or even
dev_warn(&dev->dev, ...).
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 18, 2011 at 12:24:36PM +0100, Russell King - ARM Linux wrote: > On Wed, May 18, 2011 at 04:27:47PM +0530, anish singh wrote: > > Got it.So should i change both as KERN_WARN and resend? > > Much better would be to change it to pr_warn() or even > dev_warn(&dev->dev, ...). Actually, make that pr_err() or dev_err(). Both printks in clcdfb_probe() are errors and so should be reported at error severity - they cause things to stop working and so they're not just a warning. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From 19bbb666a6067a6ba280e2cb984c924bcaf06f52 Mon Sep 17 00:00:00 2001 From: anish kumar <anish198519851985@gmail.com> Date: Tue, 17 May 2011 22:50:59 +0530 Subject: [PATCH 2/2] Changed the printk loglevel when not able to allocate memory When not able to allocate memory we were using KERN_INFO so just changed to KERN_ERR Signed-off-by: anish kumar<anish198519851985@gmail.com> --- drivers/video/amba-clcd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c index cb7ec86..e87d98b 100644 --- a/drivers/video/amba-clcd.c +++ b/drivers/video/amba-clcd.c @@ -551,7 +551,7 @@ static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id) fb = kzalloc(sizeof(*fb), GFP_KERNEL); if (!fb) { - printk(KERN_INFO "CLCD: could not allocate new clcd_fb struct\n"); + printk(KERN_ERR "CLCD: could not allocate new clcd_fb struct\n"); ret = -ENOMEM; goto free_region; } -- 1.7.0.4
When not able to allocate memory we were using KERN_INFO as log level in printk so changed to KERN_ERR Signed-off-by: anish kumar<anish198519851985@gmail.com> --- drivers/video/amba-clcd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)