staging: sm750fb: Fix printk() style warning

PATCH In review Edward Lipinsky 3 participants 7 messages Oct 2, 2016 – Oct 14, 2016

Subsystems (3)

Staging Framebuffer layer Staging - silicon motion SM750 frame buffer driver

lore

Share & tools

lore

Edward Lipinsky

author

2016-10-02 18:05 UTC #0 lore raw

PATCH 1/1 staging: sm750fb: Fix printk() style warning

This patch fixes the checkpatch.pl warning:

WARNING: printk() should include KERN_ facility level

Signed-off-by Edward Lipinsky <[email protected]>

---
 drivers/staging/sm750fb/ddk750_help.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

index 9637dd3..e72a29c 100644
@@ -11,7 +11,7 @@ void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId)
 	devId750 = devId;
 	revId750 = revId;
 	if (revId = 0xfe)
-		printk("found sm750le\n");
+		pr_info("found sm750le\n");
 }

Signature · 1 line

2.7.4

Greg KH

maintainer

2016-10-02 18:13 UTC #1 lore raw

Quoted 14 lines · Edward Lipinsky

This patch fixes the checkpatch.pl warning:
WARNING: printk() should include KERN_ facility level
Signed-off-by Edward Lipinsky <[email protected]>
---
 drivers/staging/sm750fb/ddk750_help.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/sm750fb/ddk750_help.c +0 -0
index 9637dd3..e72a29c 100644

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

Quoted 5 lines

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

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

Why can't you use dev_info() here?

thanks,

greg k-h

Edward Lipinsky

author

2016-10-07 04:27 UTC #2 lore raw

Quoted 23 lines · Greg KH

Quoted 14 lines · Edward Lipinsky
This patch fixes the checkpatch.pl warning:
WARNING: printk() should include KERN_ facility level
Signed-off-by Edward Lipinsky <[email protected]>
---
 drivers/staging/sm750fb/ddk750_help.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/sm750fb/ddk750_help.c +0 -0
index 9637dd3..e72a29c 100644
drivers/staging/sm750fb/ddk750_help.c void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId) · line 14
Quoted 5 lines
drivers/staging/sm750fb/ddk750_help.c +1 -1
@@ -11,7 +11,7 @@ void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId)
 	devId750 = devId;
 	revId750 = revId;
 	if (revId = 0xfe)
-		printk("found sm750le\n");
+		pr_info("found sm750le\n");
Why can't you use dev_info() here?

Quoted 3 lines

thanks,
greg k-h

It should work, but I'm not sure what should change in the header files to do it--esp. to make the dev parameter available in ddk750_help.c. (Only sm750.c uses dev_ style logging now, the rest of the driver still uses pr_*.)

Thanks,

Ed Lipinsky

Mike Rapoport

2016-10-07 09:12 UTC #3 lore raw

Quoted 24 lines · Edward Lipinsky

Quoted 23 lines · Greg KH
Quoted 14 lines · Edward Lipinsky
This patch fixes the checkpatch.pl warning:
WARNING: printk() should include KERN_ facility level
Signed-off-by Edward Lipinsky <[email protected]>
---
 drivers/staging/sm750fb/ddk750_help.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/sm750fb/ddk750_help.c +0 -0
index 9637dd3..e72a29c 100644
drivers/staging/sm750fb/ddk750_help.c void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId) · line 14
Quoted 5 lines
drivers/staging/sm750fb/ddk750_help.c +1 -1
@@ -11,7 +11,7 @@ void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId)
 	devId750 = devId;
 	revId750 = revId;
 	if (revId = 0xfe)
-		printk("found sm750le\n");
+		pr_info("found sm750le\n");
Why can't you use dev_info() here?

Quoted 7 lines

Quoted 3 lines
thanks,
greg k-h
It should work, but I'm not sure what should change in the header files to do it--esp. to make the dev parameter available in ddk750_help.c. (Only sm750.c uses dev_ style logging now, the rest of the driver still uses pr_*.)

This printk can be moved to lynxfb_pci_probe, and then it should be no problem to use dev_info. Just make sure to update the commit message appropriately.

Quoted 3 lines

Thanks,
Ed Lipinsky

--
Sincerely yours,
Mike.

Edward Lipinsky

author

2016-10-10 14:31 UTC #4 lore raw

Quoted 37 lines · Mike Rapoport

Quoted 24 lines · Edward Lipinsky
Quoted 23 lines · Greg KH
Quoted 14 lines · Edward Lipinsky
This patch fixes the checkpatch.pl warning:
WARNING: printk() should include KERN_ facility level
Signed-off-by Edward Lipinsky <[email protected]>
---
 drivers/staging/sm750fb/ddk750_help.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/sm750fb/ddk750_help.c +0 -0
index 9637dd3..e72a29c 100644
drivers/staging/sm750fb/ddk750_help.c void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId) · line 14
Quoted 5 lines
drivers/staging/sm750fb/ddk750_help.c +1 -1
@@ -11,7 +11,7 @@ void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId)
 	devId750 = devId;
 	revId750 = revId;
 	if (revId = 0xfe)
-		printk("found sm750le\n");
+		pr_info("found sm750le\n");
Why can't you use dev_info() here?
Quoted 7 lines
Quoted 3 lines
thanks,
greg k-h
It should work, but I'm not sure what should change in the header files to do it--esp. to make the dev parameter available in ddk750_help.c. (Only sm750.c uses dev_ style logging now, the rest of the driver still uses pr_*.)
This printk can be moved to lynxfb_pci_probe, and then it should be no problem to use dev_info. Just make sure to update the commit message appropriately.

Quoted 8 lines

Quoted 3 lines
Thanks,
Ed Lipinsky
--
Sincerely yours,
Mike.

Thanks, that makes sense. I tried deleting the if statement and printk() from ddk750_help.c, and adding the following in lynxfb_pci_probe() after calling hw_sm750_map() and checking for errors:

if (sm750_dev->revid = 0xfe)

	dev_info(&pdev->dev, "found sm750le\n");

The driver compiles, loads and unloads ok with this code.

Does this look correct?

Regards,
Ed L.

Mike Rapoport

2016-10-13 14:39 UTC #5 lore raw

Quoted 11 lines · Edward Lipinsky

Thanks, that makes sense. I tried deleting the if statement and printk() from ddk750_help.c, and adding the following in lynxfb_pci_probe() after calling hw_sm750_map() and checking for errors:
if (sm750_dev->revid = 0xfe)
	dev_info(&pdev->dev, "found sm750le\n");
The driver compiles, loads and unloads ok with this code.
Does this look correct?

Yep, seems just right.

Quoted 2 lines

Regards,
Ed L.

Edward Lipinsky

author

2016-10-14 05:18 UTC #6 lore raw

Quoted 6 lines · Mike Rapoport

Quoted 5 lines · Edward Lipinsky
Thanks, that makes sense. I tried deleting the if statement and printk() from ddk750_help.c, and adding the following in lynxfb_pci_probe() after calling hw_sm750_map() and checking for errors:

Quoted 11 lines

Quoted 6 lines
if (sm750_dev->revid = 0xfe)
	dev_info(&pdev->dev, "found sm750le\n");
The driver compiles, loads and unloads ok with this code.
Does this look correct?
Yep, seems just right.
Quoted 2 lines
Regards,
Ed L.

Good, thanks. I will submit an updated patch.

Regards,
Ed L.