[PATCH v2] Staging: sm750fb: ddk750_help: Included KERN_facility level using dev_info

[PATCH v2] Staging: sm750fb: ddk750_help: Included KERN_facility level using dev_info

[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 <shivanib134@gmail.com>
---
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(-)

diff --git a/drivers/staging/sm750fb/ddk750_help.c b/drivers/staging/sm750fb/ddk750_help.c
index 9637dd3..b877704 100644
--- a/drivers/staging/sm750fb/ddk750_help.c
+++ b/drivers/staging/sm750fb/ddk750_help.c
@@ -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");
 }
 
 
-- 
2.1.0

Re: [Outreachy kernel] [PATCH v2] Staging: sm750fb: ddk750_help: Included KERN_facility level using dev_info

[Sudip Mukherjee]

On Fri, Oct 09, 2015 at 07:51:18PM +0530, Shivani Bhardwaj wrote:
> 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 <shivanib134@gmail.com>
> ---
> 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(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_help.c b/drivers/staging/sm750fb/ddk750_help.c
> index 9637dd3..b877704 100644
> --- a/drivers/staging/sm750fb/ddk750_help.c
> +++ b/drivers/staging/sm750fb/ddk750_help.c
> @@ -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");

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

regards
sudip

Re: [Outreachy kernel] [PATCH v2] Staging: sm750fb: ddk750_help: Included KERN_facility level using dev_info

[Julia Lawall]

On Sat, 10 Oct 2015, Sudip Mukherjee wrote:

> On Fri, Oct 09, 2015 at 07:51:18PM +0530, Shivani Bhardwaj wrote:
> > 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 <shivanib134@gmail.com>
> > ---
> > 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(-)
> > 
> > diff --git a/drivers/staging/sm750fb/ddk750_help.c b/drivers/staging/sm750fb/ddk750_help.c
> > index 9637dd3..b877704 100644
> > --- a/drivers/staging/sm750fb/ddk750_help.c
> > +++ b/drivers/staging/sm750fb/ddk750_help.c
> > @@ -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");
> 
> 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

> 
> regards
> sudip
> 
> -- 
> 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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
> 

Re: [Outreachy kernel] [PATCH v2] Staging: sm750fb: ddk750_help: Included KERN_facility level using dev_info

[Sudip Mukherjee]

On Sat, Oct 10, 2015 at 09:24:52AM +0200, Julia Lawall wrote:
> On Sat, 10 Oct 2015, Sudip Mukherjee wrote:
> 
> > On Fri, Oct 09, 2015 at 07:51:18PM +0530, Shivani Bhardwaj wrote:
> > > 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 <shivanib134@gmail.com>
> > > ---
> > > 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(-)
> > > 
> > > diff --git a/drivers/staging/sm750fb/ddk750_help.c b/drivers/staging/sm750fb/ddk750_help.c
> > > index 9637dd3..b877704 100644
> > > --- a/drivers/staging/sm750fb/ddk750_help.c
> > > +++ b/drivers/staging/sm750fb/ddk750_help.c
> > > @@ -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");
> > 
> > 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.

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.

> 
> 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

Re: [Outreachy kernel] [PATCH v2] Staging: sm750fb: ddk750_help: Included KERN_facility level using dev_info

[Julia Lawall]

On Sat, 10 Oct 2015, Sudip Mukherjee wrote:

> On Sat, Oct 10, 2015 at 09:24:52AM +0200, Julia Lawall wrote:
> > On Sat, 10 Oct 2015, Sudip Mukherjee wrote:
> > 
> > > On Fri, Oct 09, 2015 at 07:51:18PM +0530, Shivani Bhardwaj wrote:
> > > > 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 <shivanib134@gmail.com>
> > > > ---
> > > > 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(-)
> > > > 
> > > > diff --git a/drivers/staging/sm750fb/ddk750_help.c b/drivers/staging/sm750fb/ddk750_help.c
> > > > index 9637dd3..b877704 100644
> > > > --- a/drivers/staging/sm750fb/ddk750_help.c
> > > > +++ b/drivers/staging/sm750fb/ddk750_help.c
> > > > @@ -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");
> > > 
> > > 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.
> 
> 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.
> 
> > 
> > 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

Re: [Outreachy kernel] [PATCH v2] Staging: sm750fb: ddk750_help: Included KERN_facility level using dev_info

[Sudip Mukherjee]

On Sat, Oct 10, 2015 at 09:40:53AM +0200, Julia Lawall wrote:
> 
> 
> On Sat, 10 Oct 2015, Sudip Mukherjee wrote:
> 
> > On Sat, Oct 10, 2015 at 09:24:52AM +0200, Julia Lawall wrote:
> > > On Sat, 10 Oct 2015, Sudip Mukherjee wrote:
> > > 
> > > > On Fri, Oct 09, 2015 at 07:51:18PM +0530, Shivani Bhardwaj wrote:
<snip>
> > 
> > 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

[PATCH v2] Staging: sm750fb: ddk750_help: Included KERN_facility level using dev_info

[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 <shivanib134@gmail.com>
---
Changes in v2:
	- Replaced the function pr_info() with dev_info()

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

diff --git a/drivers/staging/sm750fb/ddk750_help.c b/drivers/staging/sm750fb/ddk750_help.c
index 9637dd3..d20cd92 100644
--- a/drivers/staging/sm750fb/ddk750_help.c
+++ b/drivers/staging/sm750fb/ddk750_help.c
@@ -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");
+		dev_info("found sm750le\n");
 }
 
 
-- 
2.1.0