Staging: sm750fb: ddk750_help: Included KERN_facility level using dev_info

PATCH In review Shivani Bhardwaj 3 participants 6 messages Oct 9, 2015 – Oct 10, 2015

Subsystems (2)

Staging Staging - silicon motion SM750 frame buffer driver

lore

Share & tools

lore

Shivani Bhardwaj

author

2015-10-09 14:21 UTC #0 lore raw

PATCH v2 1/1 Staging: sm750fb: ddk750_help: Included KERN_facility level using dev_info

Replaced the function printk() with dev_info() to fix the warning generated by checkpatch : printk() should include KERN_ facility level.

Signed-off-by Shivani Bhardwaj <[email protected]>

---
Changes in v2:
-Replaced the function pr_info() with dev_info()

 drivers/staging/sm750fb/ddk750_help.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/staging/sm750fb/ddk750_help.c +3 -1

index 9637dd3..b877704 100644
@@ -1,5 +1,7 @@
 #include "ddk750_help.h"
+#include <linux/pci.h>
 
+static struct pci_dev *pdev;
 void __iomem *mmio750;
 char revId750;
 unsigned short devId750;

@@ -11,7 +13,7 @@ void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId)
 	devId750 = devId;
 	revId750 = revId;
 	if (revId == 0xfe)
-		printk("found sm750le\n");
+		dev_info(&pdev->dev, "found sm750le\n");
 }

Signature · 1 line

2.1.0

Sudip Mukherjee

maintainer

2015-10-10 07:14 UTC #1 lore raw

Quoted 24 lines · Shivani Bhardwaj

Replaced the function printk() with dev_info() to fix the warning generated by checkpatch : printk() should include KERN_ facility level.
Signed-off-by Shivani Bhardwaj <[email protected]>
---
Changes in v2:
-Replaced the function pr_info() with dev_info()
 drivers/staging/sm750fb/ddk750_help.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/sm750fb/ddk750_help.c +2 -0
index 9637dd3..b877704 100644
@@ -1,5 +1,7 @@
 #include "ddk750_help.h"
+#include <linux/pci.h>
 
+static struct pci_dev *pdev;
 void __iomem *mmio750;
 char revId750;
 unsigned short devId750;

drivers/staging/sm750fb/ddk750_help.c void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId) · line 16

Quoted 5 lines

drivers/staging/sm750fb/ddk750_help.c +1 -1

@@ -11,7 +13,7 @@ void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId)
 	devId750 = devId;
 	revId750 = revId;
 	if (revId == 0xfe)
-		printk("found sm750le\n");
+		dev_info(&pdev->dev, "found sm750le\n");

OOPS .. no..
pdev is initialized to NULL, so when you do pdev->dev, you are doing a dereference of NULL pointer.

regards
sudip

Julia Lawall

2015-10-10 07:24 UTC #2 lore raw

Quoted 20 lines · Sudip Mukherjee

Quoted 19 lines · Shivani Bhardwaj
Replaced the function printk() with dev_info() to fix the warning generated by checkpatch : printk() should include KERN_ facility level.
Signed-off-by Shivani Bhardwaj <[email protected]>
---
Changes in v2:
-Replaced the function pr_info() with dev_info()
 drivers/staging/sm750fb/ddk750_help.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/sm750fb/ddk750_help.c +1 -0
index 9637dd3..b877704 100644
@@ -1,5 +1,7 @@
 #include "ddk750_help.h"
+#include <linux/pci.h>
 

Quoted 14 lines

drivers/staging/sm750fb/ddk750_help.c void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId) · line 16
Quoted 10 lines
drivers/staging/sm750fb/ddk750_help.c +2 -1
+static struct pci_dev *pdev;
 void __iomem *mmio750;
 char revId750;
 unsigned short devId750;
@@ -11,7 +13,7 @@ void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId)
 	devId750 = devId;
 	revId750 = revId;
 	if (revId == 0xfe)
-		printk("found sm750le\n");
+		dev_info(&pdev->dev, "found sm750le\n");
OOPS .. no..
pdev is initialized to NULL, so when you do pdev->dev, you are doing a dereference of NULL pointer.

To be precise, Shivani's code isn't doing a dereference, it's doing an addition. But then dev_info will dereference the result of that addition (which is not likely to be NULL, but rather a small integer), and then there will be a problem.

So one would have to use pr_info here.

julia

Quoted 11 lines

regards
sudip

Signature · 5 lines

You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151010071443.GC4795%40sudip-pc.
For more options, visit https://groups.google.com/d/optout.

Sudip Mukherjee

maintainer

2015-10-10 07:38 UTC #3 lore raw

Quoted 37 lines · Julia Lawall

Quoted 20 lines · Sudip Mukherjee
Quoted 19 lines · Shivani Bhardwaj
Replaced the function printk() with dev_info() to fix the warning generated by checkpatch : printk() should include KERN_ facility level.
Signed-off-by Shivani Bhardwaj <[email protected]>
---
Changes in v2:
-Replaced the function pr_info() with dev_info()
 drivers/staging/sm750fb/ddk750_help.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/sm750fb/ddk750_help.c +1 -0
index 9637dd3..b877704 100644
@@ -1,5 +1,7 @@
 #include "ddk750_help.h"
+#include <linux/pci.h>
 
Quoted 14 lines
drivers/staging/sm750fb/ddk750_help.c void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId) · line 16
Quoted 10 lines
drivers/staging/sm750fb/ddk750_help.c +2 -1
+static struct pci_dev *pdev;
 void __iomem *mmio750;
 char revId750;
 unsigned short devId750;
@@ -11,7 +13,7 @@ void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId)
 	devId750 = devId;
 	revId750 = revId;
 	if (revId == 0xfe)
-		printk("found sm750le\n");
+		dev_info(&pdev->dev, "found sm750le\n");
OOPS .. no..
pdev is initialized to NULL, so when you do pdev->dev, you are doing a dereference of NULL pointer.

Quoted 4 lines

To be precise, Shivani's code isn't doing a dereference, it's doing an addition. But then dev_info will dereference the result of that addition (which is not likely to be NULL, but rather a small integer), and then there will be a problem.

Yes, dereference was wrong word. It is taking the address of that member. But my main point was that pdev is not initialized. So using &pdev->dev for dev_info is wrong.

Quoted 2 lines

So one would have to use pr_info here.

pr_info will be better. I can use dev_info here, but the changes that has to be made for that, I think that will become too complicated for her, atleast at this stage.

regards
sudip

Julia Lawall

2015-10-10 07:40 UTC #4 lore raw

Quoted 48 lines · Sudip Mukherjee

Quoted 37 lines · Julia Lawall
Quoted 20 lines · Sudip Mukherjee
Quoted 19 lines · Shivani Bhardwaj
Replaced the function printk() with dev_info() to fix the warning generated by checkpatch : printk() should include KERN_ facility level.
Signed-off-by Shivani Bhardwaj <[email protected]>
---
Changes in v2:
-Replaced the function pr_info() with dev_info()
 drivers/staging/sm750fb/ddk750_help.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/sm750fb/ddk750_help.c +1 -0
index 9637dd3..b877704 100644
@@ -1,5 +1,7 @@
 #include "ddk750_help.h"
+#include <linux/pci.h>
 
Quoted 14 lines
drivers/staging/sm750fb/ddk750_help.c void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId) · line 16
Quoted 10 lines
drivers/staging/sm750fb/ddk750_help.c +2 -1
+static struct pci_dev *pdev;
 void __iomem *mmio750;
 char revId750;
 unsigned short devId750;
@@ -11,7 +13,7 @@ void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId)
 	devId750 = devId;
 	revId750 = revId;
 	if (revId == 0xfe)
-		printk("found sm750le\n");
+		dev_info(&pdev->dev, "found sm750le\n");
OOPS .. no..
pdev is initialized to NULL, so when you do pdev->dev, you are doing a dereference of NULL pointer.
Quoted 4 lines
To be precise, Shivani's code isn't doing a dereference, it's doing an addition. But then dev_info will dereference the result of that addition (which is not likely to be NULL, but rather a small integer), and then there will be a problem.
Yes, dereference was wrong word. It is taking the address of that member. But my main point was that pdev is not initialized. So using &pdev->dev for dev_info is wrong.
Quoted 1 line

Quoted 5 lines

Quoted 1 line
So one would have to use pr_info here.
pr_info will be better. I can use dev_info here, but the changes that has to be made for that, I think that will become too complicated for her, atleast at this stage.

Maybe the callers have a device structure available?

julia

Sudip Mukherjee

maintainer

2015-10-10 08:07 UTC #5 lore raw

Quoted 8 lines · Julia Lawall

Quoted 4 lines · Sudip Mukherjee
Quoted 3 lines · Julia Lawall
Quoted 1 line · Sudip Mukherjee
On Fri, Oct 09, 2015 at 07:51:18PM +0530, Shivani Bhardwaj wrote:

<snip>

Quoted 6 lines

Quoted 4 lines
pr_info will be better. I can use dev_info here, but the changes that has to be made for that, I think that will become too complicated for her, atleast at this stage.
Maybe the callers have a device structure available?

Yes, they have. :)

regards
sudip