Re: [PATCH] staging: panel: fix lcd type

2015-03-24 Thread Willy Tarreau
Hi Sudip!

On Tue, Mar 24, 2015 at 10:18:10AM +0530, Sudip Mukherjee wrote:
> the lcd type as defined in the Kconfig is not matching in the code.
> as a result the rs, rw and en pins were getting interchanged.
> Kconfig defines the value of PANEL_LCD to be 1 if we select custom
> configuration but in the code LCD_TYPE_CUSTOM is defined as 5.
> 
> my hardware is LCD_TYPE_CUSTOM, but the pins were assigned to it
> as pins of LCD_TYPE_OLD, and it was not working.
> Now values are corrected with referenece to the values defined in
> Kconfig and it is working.
> checked on JHD204A lcd with LCD_TYPE_CUSTOM configuration.
> 
> Signed-off-by: Sudip Mukherjee 

OK but you also need to hange the MODULE_PARM_DESC(lcd_type) to
match these new settings. Given that this is a bug, you can even
think about adding a Cc: stable tag.

Thanks,
Willy

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] octeon-usb:Fix coding style issue with space between function name and opening bracket

2015-03-24 Thread Dan Carpenter
On Mon, Mar 23, 2015 at 05:08:52PM -0400, Nicholas Krause wrote:
> 
> 
> On March 23, 2015 9:06:26 AM EDT, Dan Carpenter  
> wrote:
> >On Mon, Mar 23, 2015 at 08:53:19AM -0400, Nicholas Krause wrote:
> >> 
> >> 
> >> On March 23, 2015 6:40:06 AM EDT, Dan Carpenter
> > wrote:
> >> >On Sun, Mar 22, 2015 at 08:04:35PM -0400, Nicholas Krause wrote:
> >> >> Fixes the checkpath.pl error where the opening bracket has a
> >unneeded
> >> >space between
> >> >> it and the function name for a if statement in the
> >> >marco,VMX_WAIT_FOR_FIELD32.
> >> >> 
> >> >> Signed-off-by: Nicholas Krause 
> >> >> ---
> >> >>  drivers/staging/octeon-usb/octeon-hcd.c | 2 +-
> >> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >> 
> >> >> diff --git a/drivers/staging/octeon-usb/octeon-hcd.c
> >> >b/drivers/staging/octeon-usb/octeon-hcd.c
> >> >> index 1daeb31..0408a12 100644
> >> >> --- a/drivers/staging/octeon-usb/octeon-hcd.c
> >> >> +++ b/drivers/staging/octeon-usb/octeon-hcd.c
> >> >> @@ -412,7 +412,7 @@ struct octeon_hcd {
> >> >> type c; 
> >> >> \
> >> >> while (1) { 
> >> >> \
> >> >> c.u32 = __cvmx_usb_read_csr32(usb, address);
> >> >> \
> >> >> -   if (c.s.field op (value)) { 
> >> >> \
> >> >> +   if (c.s.field op(value)) {  
> >> >> \
> >> >
> >> >"op" is not a function here.  This is macro and top is an operation
> >> >like
> >> >"+" or "*".  So checkpatch.pl is wrong.
> >> >
> >> >regards,
> >> >dan carpenter
> >> Very well then, I wasn't sure if we did the  same spacing style for
> >marcos. 
> >
> >We do but this is a very unusual case.
> >
> >regards,
> >dan carpenter
> Dan,  
> Sorry about that my mistake.  In addition, have you looked at my second patch 
> to see if that one is correct. 

Yes.  That one is fine.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Are you interested in photo editing solutions?

2015-03-24 Thread Harry

How are you? Are you interested in photo retouching or other photo editing
solutions?

We specialize in providing below photo retouching services:
Photoshop photos editing/retouching
Jewelry photos retouching
Ecommerce products photo editing
Photo cutting out/clipping path
Beauty/skin retouching,
Wedding photo editing and photo background manipulation.

You can send us a photo for free testing and check our quality
Waiting for your soonest response.

Best regards,
Harry
Email: marke...@tom.com

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: panel: fix lcd type

2015-03-24 Thread Sudip Mukherjee
On Tue, Mar 24, 2015 at 08:04:11AM +0100, Willy Tarreau wrote:
> Hi Sudip!
> 
> On Tue, Mar 24, 2015 at 10:18:10AM +0530, Sudip Mukherjee wrote:
> > the lcd type as defined in the Kconfig is not matching in the code.
> > as a result the rs, rw and en pins were getting interchanged.
> > Kconfig defines the value of PANEL_LCD to be 1 if we select custom
> > configuration but in the code LCD_TYPE_CUSTOM is defined as 5.
> > 
> > my hardware is LCD_TYPE_CUSTOM, but the pins were assigned to it
> > as pins of LCD_TYPE_OLD, and it was not working.
> > Now values are corrected with referenece to the values defined in
> > Kconfig and it is working.
> > checked on JHD204A lcd with LCD_TYPE_CUSTOM configuration.
> > 
> > Signed-off-by: Sudip Mukherjee 
> 
> OK but you also need to hange the MODULE_PARM_DESC(lcd_type) to
> match these new settings. Given that this is a bug, you can even
> think about adding a Cc: stable tag.
i missed seeing that MODULE_PARM_DESC. i wanted to have your
confirmation first before ccing stable.

will cc stable in v2.
but before that i think i need to read the Documentation about how to
cc stable for those many stable versions till 2.6.32

regards
sudip

> 
> Thanks,
> Willy
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: sm750fb: Fixed C99 comments warnings

2015-03-24 Thread Dan Carpenter
On Mon, Mar 23, 2015 at 09:51:54PM -0700, Ragavendra Nagraj wrote:
> diff --git a/drivers/staging/sm750fb/ddk750_chip.c 
> b/drivers/staging/sm750fb/ddk750_chip.c
> index 02f9326..1fb00a4 100644
> --- a/drivers/staging/sm750fb/ddk750_chip.c
> +++ b/drivers/staging/sm750fb/ddk750_chip.c
> @@ -17,7 +17,7 @@ logical_chip_type_t getChipType(void)
>   char physicalRev;
>   logical_chip_type_t chip;
>  
> - physicalID = devId750;//either 0x718 or 0x750
> + physicalID = devId750;/*either 0x718 or 0x750*/

This should be:

physicalID = devId750; /* either 0x718 or 0x750 */

> @@ -501,7 +501,7 @@ unsigned int calcPllValue(unsigned int 
> request_orig,pll_value_t *pll)
>   }
>   }
>  
> - //printk("Finally:  
> pll->n[%lu],m[%lu],od[%lu],pod[%lu]\n",pll->N,pll->M,pll->OD,pll->POD);
> + /*printk("Finally:  
> pll->n[%lu],m[%lu],od[%lu],pod[%lu]\n",pll->N,pll->M,pll->OD,pll->POD);*/

Just delete this dead code.

Please redo.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: rts5208: Fixed 80 char & indent warnings

2015-03-24 Thread Dan Carpenter
None of these are correct.  :(

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/4] staging: unisys: dev_t initialization

2015-03-24 Thread Dan Carpenter
On Mon, Mar 16, 2015 at 04:36:13PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Mar 12, 2015 at 11:00:10PM +0530, Sudip Mukherjee wrote:
> > dev_t is defined to be of unsigned int type, no use initializing
> > it to -1.
> > 
> > Signed-off-by: Sudip Mukherjee 
> > ---
> > v2: it was not in v1
> > 
> >  drivers/staging/unisys/visorchipset/file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/unisys/visorchipset/file.c 
> > b/drivers/staging/unisys/visorchipset/file.c
> > index 9ca7f1e..e9459af 100644
> > --- a/drivers/staging/unisys/visorchipset/file.c
> > +++ b/drivers/staging/unisys/visorchipset/file.c
> > @@ -30,7 +30,7 @@
> >  
> >  static struct cdev file_cdev;
> >  static struct visorchannel **file_controlvm_channel;
> > -static dev_t majordev = -1; /**< indicates major num for device */
> > +static dev_t majordev; /**< indicates major num for device */
> 
> I don't like this, please fix this up to handle the major number
> properly, no need for this -1 mess.  And you just broke the logic with
> this change, which isn't allowed in any patch, sorry.

The patch description is bad and the patch is not complete, but it
doesn't introduce a bug.

The -1 is never used.  By the time we call visorchipset_file_cleanup()
is called then majordev has been initialized to >= 0 so we can delete
the (MAJOR(majordev) >= 0) check.  We can also delete the "registered"
variable because visorchipset_file_cleanup() is never called unless
"registered" is true.

"registered" is set on successful module_init() and
visorchipset_file_cleanup() is only called from module_exit().

There is the problem that visorchipset_init() doesn't release any
resources on error.  But that's a separate issue and using globals like
"registered" is an ugly thing so it doesn't help.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: unisys: handle major number properly

2015-03-24 Thread Dan Carpenter
This patch also doesn't introduce bugs but it's sort of whacky and hard
to understand.  Also the subject and description imply or say "fix" but
it's just a cleanup.  The original code was also proper but just messy.

On Tue, Mar 17, 2015 at 08:31:24PM +0530, Sudip Mukherjee wrote:
> fixed the handling of dev_t and the major number.
> now the major and minor number is passed to the init function.
> similarly in the cleanup function dev_t is passed to unregister it.
> 
> Signed-off-by: Sudip Mukherjee 
> ---
>  drivers/staging/unisys/visorchipset/file.c | 18 
> --
>  drivers/staging/unisys/visorchipset/file.h |  4 ++--
>  .../staging/unisys/visorchipset/visorchipset_main.c| 10 +++---
>  3 files changed, 13 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorchipset/file.c 
> b/drivers/staging/unisys/visorchipset/file.c
> index 9ca7f1e..224e214 100644
> --- a/drivers/staging/unisys/visorchipset/file.c
> +++ b/drivers/staging/unisys/visorchipset/file.c
> @@ -30,7 +30,6 @@
>  
>  static struct cdev file_cdev;
>  static struct visorchannel **file_controlvm_channel;
> -static dev_t majordev = -1; /**< indicates major num for device */
>  static BOOL registered = FALSE;
>  
>  static int visorchipset_open(struct inode *inode, struct file *file);
> @@ -50,15 +49,17 @@ static const struct file_operations visorchipset_fops = {
>  };
>  
>  int
> -visorchipset_file_init(dev_t major_dev, struct visorchannel 
> **controlvm_channel)
> +visorchipset_file_init(int major, int minor,
> +struct visorchannel **controlvm_channel)

Pass the dev_t majordev;

1) Then it's consistent with visorchipset_file_cleanup()
2) You need majordev anyway.

Don't save majordev as a global because globals are bad and you already
have a copy in Visorchipset_platform_device.dev.devt.

>  {
>   int rc = 0;
> + dev_t majordev;
>  
>   file_controlvm_channel = controlvm_channel;
> - majordev = major_dev;
>   cdev_init(&file_cdev, &visorchipset_fops);
>   file_cdev.owner = THIS_MODULE;
> - if (MAJOR(majordev) == 0) {
> + majordev = MKDEV(major, minor);
> + if (major == 0) {
>   /* dynamic major device number registration required */
>   if (alloc_chrdev_region(&majordev, 0, 1, MYDRVNAME) < 0)
>   return -1;
> @@ -69,23 +70,20 @@ visorchipset_file_init(dev_t major_dev, struct 
> visorchannel **controlvm_channel)
>   return -1;
>   registered = TRUE;
>   }
> - rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
> + rc = cdev_add(&file_cdev, MKDEV(major, 0), 1);

This should just be:

rc = cdev_add(&file_cdev, majordev, 1);

So here is my suggestion:

[patch 1] delete dead code I mentioned in my previous email.
This deletes "registered" and the (MAJOR(majordev) >= 0) check.

[patch 2] pass majordev to visorchipset_file_cleanup()
This lets you delete the "majordev" global.

[patch 3] small cleanup in visorchipset_file_init()

-   rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
+   rc = cdev_add(&file_cdev, majordev, 1);

There are several other ways you could break it up but do something like
that.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: panel: fix lcd type

2015-03-24 Thread Willy Tarreau
On Tue, Mar 24, 2015 at 01:22:16PM +0530, Sudip Mukherjee wrote:
> On Tue, Mar 24, 2015 at 08:04:11AM +0100, Willy Tarreau wrote:
> > Hi Sudip!
> > 
> > On Tue, Mar 24, 2015 at 10:18:10AM +0530, Sudip Mukherjee wrote:
> > > the lcd type as defined in the Kconfig is not matching in the code.
> > > as a result the rs, rw and en pins were getting interchanged.
> > > Kconfig defines the value of PANEL_LCD to be 1 if we select custom
> > > configuration but in the code LCD_TYPE_CUSTOM is defined as 5.
> > > 
> > > my hardware is LCD_TYPE_CUSTOM, but the pins were assigned to it
> > > as pins of LCD_TYPE_OLD, and it was not working.
> > > Now values are corrected with referenece to the values defined in
> > > Kconfig and it is working.
> > > checked on JHD204A lcd with LCD_TYPE_CUSTOM configuration.
> > > 
> > > Signed-off-by: Sudip Mukherjee 
> > 
> > OK but you also need to hange the MODULE_PARM_DESC(lcd_type) to
> > match these new settings. Given that this is a bug, you can even
> > think about adding a Cc: stable tag.
> i missed seeing that MODULE_PARM_DESC. i wanted to have your
> confirmation first before ccing stable.
> 
> will cc stable in v2.
> but before that i think i need to read the Documentation about how to
> cc stable for those many stable versions till 2.6.32

Just put this before your signed-off-by at the bottom fo the commit message :

  Cc:  # 2.6.32+

Willy

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: unisys: handle major number properly

2015-03-24 Thread Sudip Mukherjee
On Tue, Mar 24, 2015 at 11:32:47AM +0300, Dan Carpenter wrote:

> > }
> > -   rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
> > +   rc = cdev_add(&file_cdev, MKDEV(major, 0), 1);
> 
> This should just be:
> 
>   rc = cdev_add(&file_cdev, majordev, 1);
> 
> So here is my suggestion:
> 
> [patch 1] delete dead code I mentioned in my previous email.
>   This deletes "registered" and the (MAJOR(majordev) >= 0) check.

that was initially my first patch.

> 
> [patch 2] pass majordev to visorchipset_file_cleanup()
>   This lets you delete the "majordev" global.
> 
> [patch 3] small cleanup in visorchipset_file_init()
> 
> - rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
> + rc = cdev_add(&file_cdev, majordev, 1);

and i can also include the removal of that global variable in this
3rd patch.
thanks.. after Greg's review i was thinking i understood the code wrong.
but then will this be a v2 or a whole new series?

regards
sudip

> 
> There are several other ways you could break it up but do something like
> that.
> 
> regards,
> dan carpenter
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained

2015-03-24 Thread Olaf Hering
On Mon, Mar 23, K. Y. Srinivasan wrote:

> @@ -653,32 +640,39 @@ static unsigned int copy_from_bounce_buffer(struct 
> scatterlist *orig_sgl,
>   unsigned long bounce_addr = 0;
>   unsigned long dest_addr = 0;
>   unsigned long flags;
> + struct scatterlist *cur_dest_sgl;
> + struct scatterlist *cur_src_sgl;
>  
>   local_irq_save(flags);
> -
> + cur_dest_sgl = orig_sgl;
> + cur_src_sgl = bounce_sgl;
>   for (i = 0; i < orig_sgl_count; i++) {
> - dest_addr = sg_kmap_atomic(orig_sgl,i) + orig_sgl[i].offset;
> + dest_addr = (unsigned long)
> + kmap_atomic(sg_page(cur_dest_sgl)) +
> + cur_dest_sgl->offset;
>   dest = dest_addr;
>   destlen = orig_sgl[i].length;
> + destlen = cur_dest_sgl->length;

Double assignment.

Olaf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: unisys: handle major number properly

2015-03-24 Thread Dan Carpenter
On Tue, Mar 24, 2015 at 02:13:16PM +0530, Sudip Mukherjee wrote:
> On Tue, Mar 24, 2015 at 11:32:47AM +0300, Dan Carpenter wrote:
> 
> > >   }
> > > - rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
> > > + rc = cdev_add(&file_cdev, MKDEV(major, 0), 1);
> > 
> > This should just be:
> > 
> > rc = cdev_add(&file_cdev, majordev, 1);
> > 
> > So here is my suggestion:
> > 
> > [patch 1] delete dead code I mentioned in my previous email.
> > This deletes "registered" and the (MAJOR(majordev) >= 0) check.
> 
> that was initially my first patch.

Yes, but the patch description wasn't clear.  You were talking saying
the condition was always true because of the types and I am saying the
condition is always true because of the order the functions are called.

> 
> > 
> > [patch 2] pass majordev to visorchipset_file_cleanup()
> > This lets you delete the "majordev" global.
> > 
> > [patch 3] small cleanup in visorchipset_file_init()
> > 
> > -   rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
> > +   rc = cdev_add(&file_cdev, majordev, 1);
> 
> and i can also include the removal of that global variable in this
> 3rd patch.
> thanks.. after Greg's review i was thinking i understood the code wrong.
> but then will this be a v2 or a whole new series?

I'm pretty sure we're up to v3 at least now.  This means redoing the
whole series, yes.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 2/2] staging: panel: fix lcd type in module parameters

2015-03-24 Thread Sudip Mukherjee
with reference to the previous patch of the series, fixed the
lcd type in module parameters.

Cc:  # 2.6.32+
Signed-off-by: Sudip Mukherjee 
---

v2: it was not in v1.
might not apply properly to old versions, some reordering was done
in commit <98e0e762e>

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

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 4724bef..ea54fb4 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -500,7 +500,7 @@ MODULE_PARM_DESC(keypad_type,
 static int lcd_type = NOT_SET;
 module_param(lcd_type, int, );
 MODULE_PARM_DESC(lcd_type,
-"LCD type: 0=none, 1=old //, 2=serial ks0074, 3=hantronix //, 
4=nexcom //, 5=compiled-in");
+"LCD type: 0=none, 1=compiled-in, 2=old, 3=serial ks0074, 
4=hantronix, 5=nexcom");
 
 static int lcd_height = NOT_SET;
 module_param(lcd_height, int, );
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 1/2] staging: panel: fix lcd type

2015-03-24 Thread Sudip Mukherjee
the lcd type as defined in the Kconfig is not matching in the code.
as a result the rs, rw and en pins were getting interchanged.
Kconfig defines the value of PANEL_LCD to be 1 if we select custom
configuration but in the code LCD_TYPE_CUSTOM is defined as 5.

my hardware is LCD_TYPE_CUSTOM, but the pins were assigned to it
as pins of LCD_TYPE_OLD, and it was not working.
Now values are corrected with referenece to the values defined in
Kconfig and it is working.
checked on JHD204A lcd with LCD_TYPE_CUSTOM configuration.

Cc:  # 2.6.32+
Signed-off-by: Sudip Mukherjee 
---

v2: added stable and the second patch of the series.

 drivers/staging/panel/panel.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index b7ffdfb..4724bef 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -335,11 +335,11 @@ static unsigned char 
lcd_bits[LCD_PORTS][LCD_BITS][BIT_STATES];
  * LCD types
  */
 #define LCD_TYPE_NONE  0
-#define LCD_TYPE_OLD   1
-#define LCD_TYPE_KS00742
-#define LCD_TYPE_HANTRONIX 3
-#define LCD_TYPE_NEXCOM4
-#define LCD_TYPE_CUSTOM5
+#define LCD_TYPE_CUSTOM1
+#define LCD_TYPE_OLD   2
+#define LCD_TYPE_KS00743
+#define LCD_TYPE_HANTRONIX 4
+#define LCD_TYPE_NEXCOM5
 
 /*
  * keypad types
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] staging: sm7xxfb: start using module parameters

2015-03-24 Thread Greg Kroah-Hartman
On Tue, Mar 24, 2015 at 10:20:08AM +0530, Sudip Mukherjee wrote:
> add module parameters so that we can configure X and Y resolutions
> and bpp when using this driver as a module.
> 
> Signed-off-by: Sudip Mukherjee 
> ---
>  drivers/staging/sm7xxfb/sm7xxfb.c | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c 
> b/drivers/staging/sm7xxfb/sm7xxfb.c
> index abdb021..6e9b2aa 100644
> --- a/drivers/staging/sm7xxfb/sm7xxfb.c
> +++ b/drivers/staging/sm7xxfb/sm7xxfb.c
> @@ -1033,6 +1033,24 @@ static int __init sm712fb_init(void)
>  
>  module_init(sm712fb_init);
>  
> +module_param(mode_option, charp, S_IRUGO);
> +
> +MODULE_PARM_DESC(mode_option, "\n\t\tOptions:\n"
> +  "\t\t0x301 = 640x480-8\n"
> +  "\t\t0x303 = 800x600-8\n"
> +  "\t\t0x305 = 1024x768-8\n"
> +  "\t\t0x307 = 1280x1024-8\n"
> +  "\t\t0x311 = 640x480-16\n"
> +  "\t\t0x314 = 800x600-16\n"
> +  "\t\t0x317 = 1024x768-16\n"
> +  "\t\t0x31A = 1280x1024-16\n"
> +  "\t\t0x312 = 640x480-24\n"
> +  "\t\t0x315 = 800x600-24\n"
> +  "\t\t0x318 = 1024x768-24\n"
> +  "\t\t0x31B = 1280x1024-24\n"
> +  "\t\tUsual example:\n"
> +  "\t\tinsmod ./sm7xxfb.ko mode_option=\"0x301\"\n");
> +

That's funny :)

And how do you handle multiple devices in the system?

:(

Seriously, never use module parameters for device parameters, they are
two different things.  The framebuffer core has options for handling
modes, why not use them?

And yes, lots of framebuffer drivers do have crazy module parameters,
but that doesn't mean you have to perpetuate the insanity, please do
things properly here.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/2] staging: panel: fix lcd type in module parameters

2015-03-24 Thread Dan Carpenter
On Tue, Mar 24, 2015 at 02:56:33PM +0530, Sudip Mukherjee wrote:
> with reference to the previous patch of the series, fixed the
> lcd type in module parameters.
> 
> Cc:  # 2.6.32+
> Signed-off-by: Sudip Mukherjee 
> ---

This should have been folded into the other patch because now if we
just apply 1/2 it doesn't make sense.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] staging: sm7xxfb: start using module parameters

2015-03-24 Thread Sudip Mukherjee
On Tue, Mar 24, 2015 at 10:48:26AM +0100, Greg Kroah-Hartman wrote:
> On Tue, Mar 24, 2015 at 10:20:08AM +0530, Sudip Mukherjee wrote:
> > +"\t\t0x31B = 1280x1024-24\n"
> > +"\t\tUsual example:\n"
> > +"\t\tinsmod ./sm7xxfb.ko mode_option=\"0x301\"\n");
> > +
> 
> That's funny :)
> 
> And how do you handle multiple devices in the system?
frankly speaking, never got the idea about multiple devices.

> 
> :(
> 
> Seriously, never use module parameters for device parameters, they are
> two different things.  The framebuffer core has options for handling
> modes, why not use them?
> 
> And yes, lots of framebuffer drivers do have crazy module parameters,
> but that doesn't mean you have to perpetuate the insanity, please do
> things properly here.
i am learning from other framebuffer drivers. i guess i should only
see at skeletonfb.c and not the others.
please drop this 1/2 patch, do i need to resend the 2/2 which adds
the MODULE_DEVICE_TABLE ? 

regards
sudip

> 
> thanks,
> 
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] staging: sm7xxfb: start using module parameters

2015-03-24 Thread Greg Kroah-Hartman
On Tue, Mar 24, 2015 at 03:58:35PM +0530, Sudip Mukherjee wrote:
> On Tue, Mar 24, 2015 at 10:48:26AM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Mar 24, 2015 at 10:20:08AM +0530, Sudip Mukherjee wrote:
> > > +  "\t\t0x31B = 1280x1024-24\n"
> > > +  "\t\tUsual example:\n"
> > > +  "\t\tinsmod ./sm7xxfb.ko mode_option=\"0x301\"\n");
> > > +
> > 
> > That's funny :)
> > 
> > And how do you handle multiple devices in the system?
> frankly speaking, never got the idea about multiple devices.
> 
> > 
> > :(
> > 
> > Seriously, never use module parameters for device parameters, they are
> > two different things.  The framebuffer core has options for handling
> > modes, why not use them?
> > 
> > And yes, lots of framebuffer drivers do have crazy module parameters,
> > but that doesn't mean you have to perpetuate the insanity, please do
> > things properly here.
> i am learning from other framebuffer drivers. i guess i should only
> see at skeletonfb.c and not the others.
> please drop this 1/2 patch, do i need to resend the 2/2 which adds
> the MODULE_DEVICE_TABLE ? 

Please do, it's long gone from my queue.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/2] staging: panel: fix lcd type in module parameters

2015-03-24 Thread Sudip Mukherjee
On Tue, Mar 24, 2015 at 01:05:20PM +0300, Dan Carpenter wrote:
> On Tue, Mar 24, 2015 at 02:56:33PM +0530, Sudip Mukherjee wrote:
> > with reference to the previous patch of the series, fixed the
> > lcd type in module parameters.
> > 
> > Cc:  # 2.6.32+
> > Signed-off-by: Sudip Mukherjee 
> > ---
> 
> This should have been folded into the other patch because now if we
> just apply 1/2 it doesn't make sense.
ohh, ok. but i thought of separating them as 1/2 can be aplied cleanly
on the stable versions, but this one, it might need some modifications.
will resend a single patch as v3.

regards
sudip
> 
> regards,
> dan carpenter
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH resend] staging: sm7xxfb: add MODULE_DEVICE_TABLE

2015-03-24 Thread Sudip Mukherjee
add MODULE_DEVICE_TABLE to support hot-plugging.

Signed-off-by: Sudip Mukherjee 
---

resending as discussed with Greg K-H

 drivers/staging/sm7xxfb/sm7xxfb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c 
b/drivers/staging/sm7xxfb/sm7xxfb.c
index abdb021..5b3e614 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c
@@ -922,6 +922,8 @@ static const struct pci_device_id smtcfb_pci_table[] = {
{0,}
 };
 
+MODULE_DEVICE_TABLE(pci, smtcfb_pci_table);
+
 static void smtcfb_pci_remove(struct pci_dev *pdev)
 {
struct smtcfb_info *sfb;
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v3] staging: panel: fix lcd type

2015-03-24 Thread Sudip Mukherjee
the lcd type as defined in the Kconfig is not matching in the code.
as a result the rs, rw and en pins were getting interchanged.
Kconfig defines the value of PANEL_LCD to be 1 if we select custom
configuration but in the code LCD_TYPE_CUSTOM is defined as 5.

my hardware is LCD_TYPE_CUSTOM, but the pins were assigned to it
as pins of LCD_TYPE_OLD, and it was not working.
Now values are corrected with referenece to the values defined in
Kconfig and it is working.
checked on JHD204A lcd with LCD_TYPE_CUSTOM configuration.

Cc:  # 2.6.32+
Signed-off-by: Sudip Mukherjee 
---

v3: added the module_parameters
v2: cc-ed  stable

 drivers/staging/panel/panel.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index b7ffdfb..ea54fb4 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -335,11 +335,11 @@ static unsigned char 
lcd_bits[LCD_PORTS][LCD_BITS][BIT_STATES];
  * LCD types
  */
 #define LCD_TYPE_NONE  0
-#define LCD_TYPE_OLD   1
-#define LCD_TYPE_KS00742
-#define LCD_TYPE_HANTRONIX 3
-#define LCD_TYPE_NEXCOM4
-#define LCD_TYPE_CUSTOM5
+#define LCD_TYPE_CUSTOM1
+#define LCD_TYPE_OLD   2
+#define LCD_TYPE_KS00743
+#define LCD_TYPE_HANTRONIX 4
+#define LCD_TYPE_NEXCOM5
 
 /*
  * keypad types
@@ -500,7 +500,7 @@ MODULE_PARM_DESC(keypad_type,
 static int lcd_type = NOT_SET;
 module_param(lcd_type, int, );
 MODULE_PARM_DESC(lcd_type,
-"LCD type: 0=none, 1=old //, 2=serial ks0074, 3=hantronix //, 
4=nexcom //, 5=compiled-in");
+"LCD type: 0=none, 1=compiled-in, 2=old, 3=serial ks0074, 
4=hantronix, 5=nexcom");
 
 static int lcd_height = NOT_SET;
 module_param(lcd_height, int, );
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v3 1/4] remove unneeded comparison

2015-03-24 Thread Sudip Mukherjee
visorchipset_file_cleanup() is called from visorchipset_exit() which
is the module_exit function and this function is executing means
module_init succeeded, so registered will always be true at this time.
and majordev has also been initialized in the init function. hence
these comparisons will always be true.

Signed-off-by: Sudip Mukherjee 
---

v3: broke the previous patch in this series

 drivers/staging/unisys/visorchipset/file.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index 9ca7f1e..cbed1ba2 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -81,13 +81,9 @@ visorchipset_file_cleanup(void)
if (file_cdev.ops != NULL)
cdev_del(&file_cdev);
file_cdev.ops = NULL;
-   if (registered) {
-   if (MAJOR(majordev) >= 0) {
-   unregister_chrdev_region(majordev, 1);
-   majordev = MKDEV(0, 0);
-   }
-   registered = FALSE;
-   }
+   unregister_chrdev_region(majordev, 1);
+   majordev = MKDEV(0, 0);
+   registered = FALSE;
 }
 
 static int
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v3 2/4] use local dev_t instead of global

2015-03-24 Thread Sudip Mukherjee
the dev_t is stored in visorchipset_platform_device.dev.devt, so we
can pass that value as an argument to visorchipset_file_cleanup()
instead of using the global variable in file.c

Signed-off-by: Sudip Mukherjee 
---

v3: broke the previous patch in this series

 drivers/staging/unisys/visorchipset/file.c  | 3 +--
 drivers/staging/unisys/visorchipset/file.h  | 2 +-
 drivers/staging/unisys/visorchipset/visorchipset_main.c | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index cbed1ba2..d62908d 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -76,13 +76,12 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel 
**controlvm_channel)
 }
 
 void
-visorchipset_file_cleanup(void)
+visorchipset_file_cleanup(dev_t majordev)
 {
if (file_cdev.ops != NULL)
cdev_del(&file_cdev);
file_cdev.ops = NULL;
unregister_chrdev_region(majordev, 1);
-   majordev = MKDEV(0, 0);
registered = FALSE;
 }
 
diff --git a/drivers/staging/unisys/visorchipset/file.h 
b/drivers/staging/unisys/visorchipset/file.h
index dc7a195..b32a472 100644
--- a/drivers/staging/unisys/visorchipset/file.h
+++ b/drivers/staging/unisys/visorchipset/file.h
@@ -22,6 +22,6 @@
 
 int visorchipset_file_init(dev_t majorDev,
   struct visorchannel **pControlVm_channel);
-void visorchipset_file_cleanup(void);
+void visorchipset_file_cleanup(dev_t majordev);
 
 #endif
diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c 
b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index 9c8605d..f2663d2c 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -2278,7 +2278,7 @@ visorchipset_exit(void)
 
visorchannel_destroy(controlvm_channel);
 
-   visorchipset_file_cleanup();
+   visorchipset_file_cleanup(visorchipset_platform_device.dev.devt);
POSTCODE_LINUX_2(DRIVER_EXIT_PC, POSTCODE_SEVERITY_INFO);
 }
 
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v3 3/4] use local variable instead of global

2015-03-24 Thread Sudip Mukherjee
dev_t was passed as an argument to visorchipset_file_init() which was
storing that value in a global variable so that it can be used while
cleanup. now we are no longer using the global variable in the cleanup
so we donot need to keep the global variable anymore.

Signed-off-by: Sudip Mukherjee 
---

v3: broke the previous patch in this series

 drivers/staging/unisys/visorchipset/file.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index d62908d..a52939a4 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -30,7 +30,6 @@
 
 static struct cdev file_cdev;
 static struct visorchannel **file_controlvm_channel;
-static dev_t majordev = -1; /**< indicates major num for device */
 static BOOL registered = FALSE;
 
 static int visorchipset_open(struct inode *inode, struct file *file);
@@ -55,21 +54,20 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel 
**controlvm_channel)
int rc = 0;
 
file_controlvm_channel = controlvm_channel;
-   majordev = major_dev;
cdev_init(&file_cdev, &visorchipset_fops);
file_cdev.owner = THIS_MODULE;
-   if (MAJOR(majordev) == 0) {
+   if (MAJOR(major_dev) == 0) {
/* dynamic major device number registration required */
-   if (alloc_chrdev_region(&majordev, 0, 1, MYDRVNAME) < 0)
+   if (alloc_chrdev_region(&major_dev, 0, 1, MYDRVNAME) < 0)
return -1;
registered = TRUE;
} else {
/* static major device number registration required */
-   if (register_chrdev_region(majordev, 1, MYDRVNAME) < 0)
+   if (register_chrdev_region(major_dev, 1, MYDRVNAME) < 0)
return -1;
registered = TRUE;
}
-   rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
+   rc = cdev_add(&file_cdev, major_dev, 1);
if (rc  < 0)
return -1;
return 0;
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v3 4/4] remove unused variable

2015-03-24 Thread Sudip Mukherjee
after the first patch of this series the registered variable was
no longer required.

Signed-off-by: Sudip Mukherjee 
---

v3: broke the previous patch in this series

 drivers/staging/unisys/visorchipset/file.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index a52939a4..24cac93 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -30,7 +30,6 @@
 
 static struct cdev file_cdev;
 static struct visorchannel **file_controlvm_channel;
-static BOOL registered = FALSE;
 
 static int visorchipset_open(struct inode *inode, struct file *file);
 static int visorchipset_release(struct inode *inode, struct file *file);
@@ -60,12 +59,10 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel 
**controlvm_channel)
/* dynamic major device number registration required */
if (alloc_chrdev_region(&major_dev, 0, 1, MYDRVNAME) < 0)
return -1;
-   registered = TRUE;
} else {
/* static major device number registration required */
if (register_chrdev_region(major_dev, 1, MYDRVNAME) < 0)
return -1;
-   registered = TRUE;
}
rc = cdev_add(&file_cdev, major_dev, 1);
if (rc  < 0)
@@ -80,7 +77,6 @@ visorchipset_file_cleanup(dev_t majordev)
cdev_del(&file_cdev);
file_cdev.ops = NULL;
unregister_chrdev_region(majordev, 1);
-   registered = FALSE;
 }
 
 static int
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] octeon-usb:Fix coding style issue with space between function name and opening bracket

2015-03-24 Thread Nicholas Krause


On March 24, 2015 3:08:09 AM EDT, Dan Carpenter  
wrote:
>On Mon, Mar 23, 2015 at 05:08:52PM -0400, Nicholas Krause wrote:
>> 
>> 
>> On March 23, 2015 9:06:26 AM EDT, Dan Carpenter
> wrote:
>> >On Mon, Mar 23, 2015 at 08:53:19AM -0400, Nicholas Krause wrote:
>> >> 
>> >> 
>> >> On March 23, 2015 6:40:06 AM EDT, Dan Carpenter
>> > wrote:
>> >> >On Sun, Mar 22, 2015 at 08:04:35PM -0400, Nicholas Krause wrote:
>> >> >> Fixes the checkpath.pl error where the opening bracket has a
>> >unneeded
>> >> >space between
>> >> >> it and the function name for a if statement in the
>> >> >marco,VMX_WAIT_FOR_FIELD32.
>> >> >> 
>> >> >> Signed-off-by: Nicholas Krause 
>> >> >> ---
>> >> >>  drivers/staging/octeon-usb/octeon-hcd.c | 2 +-
>> >> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >> 
>> >> >> diff --git a/drivers/staging/octeon-usb/octeon-hcd.c
>> >> >b/drivers/staging/octeon-usb/octeon-hcd.c
>> >> >> index 1daeb31..0408a12 100644
>> >> >> --- a/drivers/staging/octeon-usb/octeon-hcd.c
>> >> >> +++ b/drivers/staging/octeon-usb/octeon-hcd.c
>> >> >> @@ -412,7 +412,7 @@ struct octeon_hcd {
>> >> >>type c; 
>> >> >> \
>> >> >>while (1) { 
>> >> >> \
>> >> >>c.u32 = __cvmx_usb_read_csr32(usb, address);
>> >> >> \
>> >> >> -  if (c.s.field op (value)) { 
>> >> >> \
>> >> >> +  if (c.s.field op(value)) {  
>> >> >> \
>> >> >
>> >> >"op" is not a function here.  This is macro and top is an
>operation
>> >> >like
>> >> >"+" or "*".  So checkpatch.pl is wrong.
>> >> >
>> >> >regards,
>> >> >dan carpenter
>> >> Very well then, I wasn't sure if we did the  same spacing style
>for
>> >marcos. 
>> >
>> >We do but this is a very unusual case.
>> >
>> >regards,
>> >dan carpenter
>> Dan,  
>> Sorry about that my mistake.  In addition, have you looked at my
>second patch to see if that one is correct. 
>
>Yes.  That one is fine.
>
>regards,
>dan carpenter
Dan, 
Just one more question, is that patch going to be merged or should I resubmit 
it as a series of one patch?
Thanks, 
Nick 
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3 1/4] remove unneeded comparison

2015-03-24 Thread Sudip Mukherjee
On Tue, Mar 24, 2015 at 05:36:09PM +0530, Sudip Mukherjee wrote:

please scrap this series, I messed up the subject line and missed
mentioning "staging: unisys"

i am sending a corrected copies as v4.
sorry for the noise.

regards
sudip

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v4 1/4] staging: unisys: remove unneeded comparison

2015-03-24 Thread Sudip Mukherjee
visorchipset_file_cleanup() is called from visorchipset_exit() which
is the module_exit function and this function is executing means
module_init succeeded, so registered will always be true at this time.
and majordev has also been initialized in the init function. hence
these comparisons will always be true.

Signed-off-by: Sudip Mukherjee 
---

v4: messed up the subject in v3
v3: broke the previous patch in this series

 drivers/staging/unisys/visorchipset/file.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index 9ca7f1e..cbed1ba2 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -81,13 +81,9 @@ visorchipset_file_cleanup(void)
if (file_cdev.ops != NULL)
cdev_del(&file_cdev);
file_cdev.ops = NULL;
-   if (registered) {
-   if (MAJOR(majordev) >= 0) {
-   unregister_chrdev_region(majordev, 1);
-   majordev = MKDEV(0, 0);
-   }
-   registered = FALSE;
-   }
+   unregister_chrdev_region(majordev, 1);
+   majordev = MKDEV(0, 0);
+   registered = FALSE;
 }
 
 static int
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v4 3/4] staging: unisys: use local variable instead of global

2015-03-24 Thread Sudip Mukherjee
dev_t was passed as an argument to visorchipset_file_init() which was
storing that value in a global variable so that it can be used while
cleanup. now we are no longer using the global variable in the cleanup
so we donot need to keep the global variable anymore.

Signed-off-by: Sudip Mukherjee 
---

v4: messed up the subject in v3
v3: broke the previous patch in this series

 drivers/staging/unisys/visorchipset/file.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index d62908d..a52939a4 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -30,7 +30,6 @@
 
 static struct cdev file_cdev;
 static struct visorchannel **file_controlvm_channel;
-static dev_t majordev = -1; /**< indicates major num for device */
 static BOOL registered = FALSE;
 
 static int visorchipset_open(struct inode *inode, struct file *file);
@@ -55,21 +54,20 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel 
**controlvm_channel)
int rc = 0;
 
file_controlvm_channel = controlvm_channel;
-   majordev = major_dev;
cdev_init(&file_cdev, &visorchipset_fops);
file_cdev.owner = THIS_MODULE;
-   if (MAJOR(majordev) == 0) {
+   if (MAJOR(major_dev) == 0) {
/* dynamic major device number registration required */
-   if (alloc_chrdev_region(&majordev, 0, 1, MYDRVNAME) < 0)
+   if (alloc_chrdev_region(&major_dev, 0, 1, MYDRVNAME) < 0)
return -1;
registered = TRUE;
} else {
/* static major device number registration required */
-   if (register_chrdev_region(majordev, 1, MYDRVNAME) < 0)
+   if (register_chrdev_region(major_dev, 1, MYDRVNAME) < 0)
return -1;
registered = TRUE;
}
-   rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
+   rc = cdev_add(&file_cdev, major_dev, 1);
if (rc  < 0)
return -1;
return 0;
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v4 4/4] staging: unisys: remove unused variable

2015-03-24 Thread Sudip Mukherjee
after the first patch of this series the registered variable was
no longer required.

Signed-off-by: Sudip Mukherjee 
---

v4: messed up the subject in v3
v3: broke the previous patch in this series

 drivers/staging/unisys/visorchipset/file.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index a52939a4..24cac93 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -30,7 +30,6 @@
 
 static struct cdev file_cdev;
 static struct visorchannel **file_controlvm_channel;
-static BOOL registered = FALSE;
 
 static int visorchipset_open(struct inode *inode, struct file *file);
 static int visorchipset_release(struct inode *inode, struct file *file);
@@ -60,12 +59,10 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel 
**controlvm_channel)
/* dynamic major device number registration required */
if (alloc_chrdev_region(&major_dev, 0, 1, MYDRVNAME) < 0)
return -1;
-   registered = TRUE;
} else {
/* static major device number registration required */
if (register_chrdev_region(major_dev, 1, MYDRVNAME) < 0)
return -1;
-   registered = TRUE;
}
rc = cdev_add(&file_cdev, major_dev, 1);
if (rc  < 0)
@@ -80,7 +77,6 @@ visorchipset_file_cleanup(dev_t majordev)
cdev_del(&file_cdev);
file_cdev.ops = NULL;
unregister_chrdev_region(majordev, 1);
-   registered = FALSE;
 }
 
 static int
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v4 2/4] staging: unisys: use local dev_t instead of global

2015-03-24 Thread Sudip Mukherjee
the dev_t is stored in visorchipset_platform_device.dev.devt, so we
can pass that value as an argument to visorchipset_file_cleanup()
instead of using the global variable in file.c

Signed-off-by: Sudip Mukherjee 
---

v4: messed up the subject in v3
v3: broke the previous patch in this series

 drivers/staging/unisys/visorchipset/file.c  | 3 +--
 drivers/staging/unisys/visorchipset/file.h  | 2 +-
 drivers/staging/unisys/visorchipset/visorchipset_main.c | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index cbed1ba2..d62908d 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -76,13 +76,12 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel 
**controlvm_channel)
 }
 
 void
-visorchipset_file_cleanup(void)
+visorchipset_file_cleanup(dev_t majordev)
 {
if (file_cdev.ops != NULL)
cdev_del(&file_cdev);
file_cdev.ops = NULL;
unregister_chrdev_region(majordev, 1);
-   majordev = MKDEV(0, 0);
registered = FALSE;
 }
 
diff --git a/drivers/staging/unisys/visorchipset/file.h 
b/drivers/staging/unisys/visorchipset/file.h
index dc7a195..b32a472 100644
--- a/drivers/staging/unisys/visorchipset/file.h
+++ b/drivers/staging/unisys/visorchipset/file.h
@@ -22,6 +22,6 @@
 
 int visorchipset_file_init(dev_t majorDev,
   struct visorchannel **pControlVm_channel);
-void visorchipset_file_cleanup(void);
+void visorchipset_file_cleanup(dev_t majordev);
 
 #endif
diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c 
b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index 9c8605d..f2663d2c 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -2278,7 +2278,7 @@ visorchipset_exit(void)
 
visorchannel_destroy(controlvm_channel);
 
-   visorchipset_file_cleanup();
+   visorchipset_file_cleanup(visorchipset_platform_device.dev.devt);
POSTCODE_LINUX_2(DRIVER_EXIT_PC, POSTCODE_SEVERITY_INFO);
 }
 
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v4 1/4] staging: unisys: remove unneeded comparison

2015-03-24 Thread Dan Carpenter
On Tue, Mar 24, 2015 at 05:47:34PM +0530, Sudip Mukherjee wrote:
> visorchipset_file_cleanup() is called from visorchipset_exit() which
> is the module_exit function and this function is executing means
> module_init succeeded, so registered will always be true at this time.
> and majordev has also been initialized in the init function. hence
> these comparisons will always be true.
> 
> Signed-off-by: Sudip Mukherjee 

This is a partial patch.  The "one thing per patch" means don't do half
a thing per patch.

We now set "registered" but there are no users.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: octeon-usb: Made octeon_usb_match const

2015-03-24 Thread Greg KH
On Mon, Mar 09, 2015 at 11:09:03PM +0100, Mateusz Kulikowski wrote:
> An of_device_id should be const (checkpatch.pl warning).
> 
> Signed-off-by: Mateusz Kulikowski 
> Acked-by: Aaro Koskinen 
> ---
>  drivers/staging/octeon-usb/octeon-hcd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
> b/drivers/staging/octeon-usb/octeon-hcd.c
> index 1daeb31..9591232 100644
> --- a/drivers/staging/octeon-usb/octeon-hcd.c
> +++ b/drivers/staging/octeon-usb/octeon-hcd.c
> @@ -3885,7 +3885,7 @@ static int octeon_usb_remove(struct platform_device 
> *pdev)
>   return 0;
>  }
>  
> -static struct of_device_id octeon_usb_match[] = {
> +static const struct of_device_id octeon_usb_match[] = {
>   {
>   .compatible = "cavium,octeon-5750-usbc",
>   },

Sorry, someone else sent this patch, and I saw their patch first before
yours, and applied it, which was my fault :(

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v4 1/4] staging: unisys: remove unneeded comparison

2015-03-24 Thread Sudip Mukherjee
On Tue, Mar 24, 2015 at 03:30:21PM +0300, Dan Carpenter wrote:
> On Tue, Mar 24, 2015 at 05:47:34PM +0530, Sudip Mukherjee wrote:
> > visorchipset_file_cleanup() is called from visorchipset_exit() which
> > is the module_exit function and this function is executing means
> > module_init succeeded, so registered will always be true at this time.
> > and majordev has also been initialized in the init function. hence
> > these comparisons will always be true.
> > 
> > Signed-off-by: Sudip Mukherjee 
> 
> This is a partial patch.  The "one thing per patch" means don't do half
> a thing per patch.
> 
> We now set "registered" but there are no users.
it is being removed separately in the 4/4 patch.
do you want that to be combined here in this patch?
but then won't that become 2 different changes in a single patch?
my message says we are removing that as the comparison is always
true.

regards
sudip
> 
> regards,
> dan carpenter
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v4 3/4] staging: unisys: use local variable instead of global

2015-03-24 Thread Dan Carpenter
I don't like how these are broken up at all.  It's random grab bag of
renaming and deleting.

Do it like this, perhaps:

[patch 1] delete registered

Leave the if (MAJOR(majordev) >= 0) for now, because it just
confuses issues to mix it in.  Delete all references to registered.

[patch 1] use local variable in visorchipset_file_init()
Don't delete anything.

[patch 2] pass major_dev to visorchipset_file_cleanup()

This deletes the "if (MAJOR(majordev) >= 0)" condition because it's
obvious that can't be true.  This deletes all references to the
"majordev" global because that isn't needed any more.  The whole reason
it was needed originally was because of visorchipset_file_cleanup() so
it's wrong to do the deleting in this patch which is about
visorchipset_file_init().

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] octeon-usb:Fix coding style issue with space between function name and opening bracket

2015-03-24 Thread Dan Carpenter
On Tue, Mar 24, 2015 at 08:08:16AM -0400, Nicholas Krause wrote:

> Dan, 
> Just one more question, is that patch going to be merged or should I resubmit 
> it as a series of one patch?

Don't resubmit.  Wait for Greg to mail you.  It can take a several weeks
because he is busy.

regards,
dan carpetner


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] octeon-usb:Fix coding style issue with space between function name and opening bracket

2015-03-24 Thread Joe Perches
On Tue, 2015-03-24 at 16:00 +0300, Dan Carpenter wrote:
> On Tue, Mar 24, 2015 at 08:08:16AM -0400, Nicholas Krause wrote:
> > Just one more question, is that patch going to be merged or should I 
> > resubmit 
> > it as a series of one patch?
> Don't resubmit.  Wait for Greg to mail you.  It can take a several weeks
> because he is busy.

What Dan said is certainly true, but if after 4 or
more weeks with no reply, I think it's fine to
resubmit or send a polite "ping" requesting status.

There really no other option as no other feedback
mechanism exists and things can and do get lost in
high-volume process.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] octeon-usb:Fix coding style issue with space between function name and opening bracket

2015-03-24 Thread Nick Krause
On Tue, Mar 24, 2015 at 9:20 AM, Joe Perches  wrote:
> On Tue, 2015-03-24 at 16:00 +0300, Dan Carpenter wrote:
>> On Tue, Mar 24, 2015 at 08:08:16AM -0400, Nicholas Krause wrote:
>> > Just one more question, is that patch going to be merged or should I 
>> > resubmit
>> > it as a series of one patch?
>> Don't resubmit.  Wait for Greg to mail you.  It can take a several weeks
>> because he is busy.
>
> What Dan said is certainly true, but if after 4 or
> more weeks with no reply, I think it's fine to
> resubmit or send a polite "ping" requesting status.
>
> There really no other option as no other feedback
> mechanism exists and things can and do get lost in
> high-volume process.
>
>
That's fine, I will wait then for a reply from Greg.
Nick
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: rts5208: Fixed 80 char & indent warnings

2015-03-24 Thread Joe Perches
On Tue, 2015-03-24 at 11:00 +0300, Dan Carpenter wrote:
> None of these are correct.  :(

True.  Please run your proposed patches through checkpatch
before sending them.

I think it would be an improvement to rework the flow-hiding
TRACE_RET and TRACE_GOTO macros into two separate lines.

It would also reduce the code size quite a bit if the TRACE
portion of TRACE_RET/TRACE_GOTO was a separate function and
not an inline.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v5 5/5] staging: unisys: remove comparison

2015-03-24 Thread Sudip Mukherjee
the comparison is always true as the dev_t has been initialized in the
init function and we are sending that initialized dev_t to the
cleanup().

Signed-off-by: Sudip Mukherjee 
---

v5: reordered the patch series
v4: messed up the subject in v3
v3: broke the previous patch in series

 drivers/staging/unisys/visorchipset/file.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index 890869a..39b19af 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -76,9 +76,7 @@ visorchipset_file_cleanup(dev_t major_dev)
if (file_cdev.ops != NULL)
cdev_del(&file_cdev);
file_cdev.ops = NULL;
-   if (MAJOR(major_dev) >= 0) {
-   unregister_chrdev_region(major_dev, 1);
-   }
+   unregister_chrdev_region(major_dev, 1);
 }
 
 static int
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v5 1/5] staging: unisys: remove redundant variable

2015-03-24 Thread Sudip Mukherjee
remove the variable "registered", which was used in the cleanup() to
detect if the driver has successfully initialized. the cleanup()
is called from module_exit, so its obvious that the module has
successfully initialized. if the initialization had failed, then
we will never be in the cleanup().

Signed-off-by: Sudip Mukherjee 
---

v5: reordered the patch series
v4: messed up the subject in v3
v3: broke the previous patch in series

 drivers/staging/unisys/visorchipset/file.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index 9ca7f1e..b426762 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -31,7 +31,6 @@
 static struct cdev file_cdev;
 static struct visorchannel **file_controlvm_channel;
 static dev_t majordev = -1; /**< indicates major num for device */
-static BOOL registered = FALSE;
 
 static int visorchipset_open(struct inode *inode, struct file *file);
 static int visorchipset_release(struct inode *inode, struct file *file);
@@ -62,12 +61,10 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel 
**controlvm_channel)
/* dynamic major device number registration required */
if (alloc_chrdev_region(&majordev, 0, 1, MYDRVNAME) < 0)
return -1;
-   registered = TRUE;
} else {
/* static major device number registration required */
if (register_chrdev_region(majordev, 1, MYDRVNAME) < 0)
return -1;
-   registered = TRUE;
}
rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
if (rc  < 0)
@@ -81,12 +78,9 @@ visorchipset_file_cleanup(void)
if (file_cdev.ops != NULL)
cdev_del(&file_cdev);
file_cdev.ops = NULL;
-   if (registered) {
-   if (MAJOR(majordev) >= 0) {
-   unregister_chrdev_region(majordev, 1);
-   majordev = MKDEV(0, 0);
-   }
-   registered = FALSE;
+   if (MAJOR(majordev) >= 0) {
+   unregister_chrdev_region(majordev, 1);
+   majordev = MKDEV(0, 0);
}
 }
 
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v5 2/5] staging: unisys: use local variable

2015-03-24 Thread Sudip Mukherjee
we are getting dev_t as an argument in the function, so use the local
variable instead of the global variable "majordev".
this global variable will be removed in one of the next patch of the
series.

Signed-off-by: Sudip Mukherjee 
---

v5: reordered the patch series
v4: messed up the subject in v3
v3: broke the previous patch in series

 drivers/staging/unisys/visorchipset/file.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index b426762..6ebe7f7 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -57,16 +57,16 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel 
**controlvm_channel)
majordev = major_dev;
cdev_init(&file_cdev, &visorchipset_fops);
file_cdev.owner = THIS_MODULE;
-   if (MAJOR(majordev) == 0) {
+   if (MAJOR(major_dev) == 0) {
/* dynamic major device number registration required */
-   if (alloc_chrdev_region(&majordev, 0, 1, MYDRVNAME) < 0)
+   if (alloc_chrdev_region(&major_dev, 0, 1, MYDRVNAME) < 0)
return -1;
} else {
/* static major device number registration required */
-   if (register_chrdev_region(majordev, 1, MYDRVNAME) < 0)
+   if (register_chrdev_region(major_dev, 1, MYDRVNAME) < 0)
return -1;
}
-   rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
+   rc = cdev_add(&file_cdev, MKDEV(MAJOR(major_dev), 0), 1);
if (rc  < 0)
return -1;
return 0;
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v5 3/5] staging: unisys: use local variable in cleanup

2015-03-24 Thread Sudip Mukherjee
the dev_t was being stored in visorchipset_platform_device.dev.devt
while initializing the module. so pass that value as an argument to
cleanup() so that it can use this local variable instead of the global
variable.

Signed-off-by: Sudip Mukherjee 
---

v5: reordered the patch series
v4: messed up the subject in v3
v3: broke the previous patch in series

 drivers/staging/unisys/visorchipset/file.c  | 6 +++---
 drivers/staging/unisys/visorchipset/file.h  | 2 +-
 drivers/staging/unisys/visorchipset/visorchipset_main.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index 6ebe7f7..257cfdd 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -73,13 +73,13 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel 
**controlvm_channel)
 }
 
 void
-visorchipset_file_cleanup(void)
+visorchipset_file_cleanup(dev_t major_dev)
 {
if (file_cdev.ops != NULL)
cdev_del(&file_cdev);
file_cdev.ops = NULL;
-   if (MAJOR(majordev) >= 0) {
-   unregister_chrdev_region(majordev, 1);
+   if (MAJOR(major_dev) >= 0) {
+   unregister_chrdev_region(major_dev, 1);
majordev = MKDEV(0, 0);
}
 }
diff --git a/drivers/staging/unisys/visorchipset/file.h 
b/drivers/staging/unisys/visorchipset/file.h
index dc7a195..51f7699 100644
--- a/drivers/staging/unisys/visorchipset/file.h
+++ b/drivers/staging/unisys/visorchipset/file.h
@@ -22,6 +22,6 @@
 
 int visorchipset_file_init(dev_t majorDev,
   struct visorchannel **pControlVm_channel);
-void visorchipset_file_cleanup(void);
+void visorchipset_file_cleanup(dev_t major_dev);
 
 #endif
diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c 
b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index 9c8605d..f2663d2c 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -2278,7 +2278,7 @@ visorchipset_exit(void)
 
visorchannel_destroy(controlvm_channel);
 
-   visorchipset_file_cleanup();
+   visorchipset_file_cleanup(visorchipset_platform_device.dev.devt);
POSTCODE_LINUX_2(DRIVER_EXIT_PC, POSTCODE_SEVERITY_INFO);
 }
 
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v5 4/5] staging: unisys: remove global dev_t

2015-03-24 Thread Sudip Mukherjee
the global variable majordev is no longer required, as it is not being
used anywhere.

Signed-off-by: Sudip Mukherjee 
---

v5: reordered the patch series
v4: messed up the subject in v3
v3: broke the previous patch in series

 drivers/staging/unisys/visorchipset/file.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c 
b/drivers/staging/unisys/visorchipset/file.c
index 257cfdd..890869a 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -30,7 +30,6 @@
 
 static struct cdev file_cdev;
 static struct visorchannel **file_controlvm_channel;
-static dev_t majordev = -1; /**< indicates major num for device */
 
 static int visorchipset_open(struct inode *inode, struct file *file);
 static int visorchipset_release(struct inode *inode, struct file *file);
@@ -54,7 +53,6 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel 
**controlvm_channel)
int rc = 0;
 
file_controlvm_channel = controlvm_channel;
-   majordev = major_dev;
cdev_init(&file_cdev, &visorchipset_fops);
file_cdev.owner = THIS_MODULE;
if (MAJOR(major_dev) == 0) {
@@ -80,7 +78,6 @@ visorchipset_file_cleanup(dev_t major_dev)
file_cdev.ops = NULL;
if (MAJOR(major_dev) >= 0) {
unregister_chrdev_region(major_dev, 1);
-   majordev = MKDEV(0, 0);
}
 }
 
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v5 1/5] staging: unisys: remove redundant variable

2015-03-24 Thread Dan Carpenter
Great.  :)

Reviewed-by: Dan Carpenter 

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained

2015-03-24 Thread KY Srinivasan


> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Tuesday, March 24, 2015 1:56 AM
> To: KY Srinivasan
> Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
> de...@linuxdriverproject.org; jbottom...@parallels.com;
> h...@infradead.org; linux-s...@vger.kernel.org; a...@canonical.com;
> vkuzn...@redhat.com; jasow...@redhat.com
> Subject: Re: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is 
> not
> chained
> 
> On Mon, Mar 23, K. Y. Srinivasan wrote:
> 
> > @@ -653,32 +640,39 @@ static unsigned int
> copy_from_bounce_buffer(struct scatterlist *orig_sgl,
> > unsigned long bounce_addr = 0;
> > unsigned long dest_addr = 0;
> > unsigned long flags;
> > +   struct scatterlist *cur_dest_sgl;
> > +   struct scatterlist *cur_src_sgl;
> >
> > local_irq_save(flags);
> > -
> > +   cur_dest_sgl = orig_sgl;
> > +   cur_src_sgl = bounce_sgl;
> > for (i = 0; i < orig_sgl_count; i++) {
> > -   dest_addr = sg_kmap_atomic(orig_sgl,i) + orig_sgl[i].offset;
> > +   dest_addr = (unsigned long)
> > +   kmap_atomic(sg_page(cur_dest_sgl)) +
> > +   cur_dest_sgl->offset;
> > dest = dest_addr;
> > destlen = orig_sgl[i].length;
> > +   destlen = cur_dest_sgl->length;
> 
> Double assignment.

Thanks Olaf; I will fix this and resubmit.

K. Y

> 
> Olaf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: rts5208: Fixed 80 char & indent warnings

2015-03-24 Thread Vatika Harlalka
Hi,

Please refer :

http://lxr.linux.no/#linux+v3.19.1/Documentation/CodingStyle

for reference on kernel coding conventions. These are also the
basis for various checkpatch.pl errors.

You can also refer to :
http://kernelnewbies.org/CheckpatchTips
for reference on things to look out for while resolving checkpatch
errors.

Thanks :)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: rtl8723au: Remove uses of MAC_FMT and MAC_ARG

2015-03-24 Thread Joe Perches
Use the standard vsprintf kernel extension to format
mac addresses.

This reduces object code size a bit.

Miscellanea:

o Coalesce formats
o Realign arguments
o Remove the now unused MAC_FMT and MAC_ARG #defines

Signed-off-by: Joe Perches 
---
 drivers/staging/rtl8723au/core/rtw_ap.c   | 41 ++---
 drivers/staging/rtl8723au/core/rtw_mlme.c | 26 
 drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 72 +++
 drivers/staging/rtl8723au/core/rtw_recv.c | 42 +
 drivers/staging/rtl8723au/core/rtw_wlan_util.c|  6 +-
 drivers/staging/rtl8723au/core/rtw_xmit.c |  8 +--
 drivers/staging/rtl8723au/include/ieee80211.h |  3 -
 drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c | 18 +++---
 drivers/staging/rtl8723au/os_dep/os_intfs.c   |  7 +--
 9 files changed, 101 insertions(+), 122 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_ap.c 
b/drivers/staging/rtl8723au/core/rtw_ap.c
index bed13db..6456689 100644
--- a/drivers/staging/rtl8723au/core/rtw_ap.c
+++ b/drivers/staging/rtl8723au/core/rtw_ap.c
@@ -260,15 +260,17 @@ void  expire_timeout_chk23a(struct rtw_adapter 
*padapter)
list_del_init(&psta->asoc_list);
pstapriv->asoc_list_cnt--;
 
-   DBG_8723A("asoc expire "MAC_FMT", state = 0x%x\n", 
MAC_ARG(psta->hwaddr), psta->state);
+   DBG_8723A("asoc expire %pM, state = 0x%x\n",
+ psta->hwaddr, psta->state);
updated = ap_free_sta23a(padapter, psta, false, 
WLAN_REASON_DEAUTH_LEAVING);
} else {
/* TODO: Aging mechanism to digest frames in sleep_q to 
avoid running out of xmitframe */
if (psta->sleepq_len > 
(NR_XMITFRAME/pstapriv->asoc_list_cnt)
&& padapter->xmitpriv.free_xmitframe_cnt < 
((NR_XMITFRAME/pstapriv->asoc_list_cnt)/2)
) {
-   DBG_8723A("%s sta:"MAC_FMT", sleepq_len:%u, 
free_xmitframe_cnt:%u, asoc_list_cnt:%u, clear sleep_q\n", __func__,
- MAC_ARG(psta->hwaddr),
+   DBG_8723A("%s sta:%pM, sleepq_len:%u, 
free_xmitframe_cnt:%u, asoc_list_cnt:%u, clear sleep_q\n",
+ __func__,
+ psta->hwaddr,
  psta->sleepq_len,
  padapter->xmitpriv.free_xmitframe_cnt,
  pstapriv->asoc_list_cnt);
@@ -305,7 +307,8 @@ voidexpire_timeout_chk23a(struct rtw_adapter 
*padapter)
 
psta->keep_alive_trycnt++;
if (ret == _SUCCESS) {
-   DBG_8723A("asoc check, sta(" MAC_FMT ") is alive\n", 
MAC_ARG(psta->hwaddr));
+   DBG_8723A("asoc check, sta(%pM) is alive\n",
+ psta->hwaddr);
psta->expire_to = pstapriv->expire_to;
psta->keep_alive_trycnt = 0;
continue;
@@ -317,7 +320,8 @@ voidexpire_timeout_chk23a(struct rtw_adapter 
*padapter)
 
psta->keep_alive_trycnt = 0;
 
-   DBG_8723A("asoc expire "MAC_FMT", state = 0x%x\n", 
MAC_ARG(psta->hwaddr), psta->state);
+   DBG_8723A("asoc expire %pM, state = 0x%x\n",
+ psta->hwaddr, psta->state);
spin_lock_bh(&pstapriv->asoc_list_lock);
if (!list_empty(&psta->asoc_list)) {
list_del_init(&psta->asoc_list);
@@ -1044,7 +1048,7 @@ int rtw_acl_add_sta23a(struct rtw_adapter *padapter, u8 
*addr)
struct wlan_acl_pool *pacl_list = &pstapriv->acl_list;
struct rtw_queue *pacl_node_q = &pacl_list->acl_node_q;
 
-   DBG_8723A("%s(acl_num =%d) =" MAC_FMT "\n", __func__, pacl_list->num, 
MAC_ARG(addr));
+   DBG_8723A("%s(acl_num =%d) =%pM\n", __func__, pacl_list->num, addr);
 
if ((NUM_ACL-1) < pacl_list->num)
return -1;
@@ -1476,8 +1480,8 @@ void bss_cap_update_on_sta_join23a(struct rtw_adapter 
*padapter, struct sta_info
if (psta->flags & WLAN_STA_HT) {
u16 ht_capab = le16_to_cpu(psta->htpriv.ht_cap.cap_info);
 
-   DBG_8723A("HT: STA " MAC_FMT " HT Capabilities Info: 0x%04x\n",
-   MAC_ARG(psta->hwaddr), ht_capab);
+   DBG_8723A("HT: STA %pM HT Capabilities Info: 0x%04x\n",
+ psta->hwaddr, ht_capab);
 
if (psta->no_ht_set) {
psta->no_ht_set = 0;
@@ -1489,10 +1493,9 @@ void bss_cap_update_on_sta_join23a(struct rtw_adapter 
*padapter, struct sta_info
psta->no_ht_gf_set = 1;
pmlmepriv->num_sta_ht_no_gf++;

Re: [PATCH] staging: rtl8723au: Remove uses of MAC_FMT and MAC_ARG

2015-03-24 Thread Jes Sorensen
Joe Perches  writes:
> Use the standard vsprintf kernel extension to format
> mac addresses.
>
> This reduces object code size a bit.
>
> Miscellanea:
>
> o Coalesce formats
> o Realign arguments
> o Remove the now unused MAC_FMT and MAC_ARG #defines
>
> Signed-off-by: Joe Perches 
> ---
>  drivers/staging/rtl8723au/core/rtw_ap.c   | 41 ++---
>  drivers/staging/rtl8723au/core/rtw_mlme.c | 26 
>  drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 72 
> +++
>  drivers/staging/rtl8723au/core/rtw_recv.c | 42 +
>  drivers/staging/rtl8723au/core/rtw_wlan_util.c|  6 +-
>  drivers/staging/rtl8723au/core/rtw_xmit.c |  8 +--
>  drivers/staging/rtl8723au/include/ieee80211.h |  3 -
>  drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c | 18 +++---
>  drivers/staging/rtl8723au/os_dep/os_intfs.c   |  7 +--
>  9 files changed, 101 insertions(+), 122 deletions(-)

Looks good to me

Acked-by: Jes Sorensen 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Staging: slicoss: Fix checkpatch.pl issues

2015-03-24 Thread Niranjan Dighe
The following files had coding style issues that I tried to address.
It was mostly about lines spanning more than 80 characters.

Signed-off-by: Niranjan Dighe 

diff --git a/drivers/staging/slicoss/slicoss.c 
b/drivers/staging/slicoss/slicoss.c
index 3104cb0..2161bdb 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -98,7 +98,8 @@
 #include "slic.h"
 
 static uint slic_first_init = 1;
-static char *slic_banner = "Alacritech SLIC Technology(tm) Server and Storage 
Accelerator (Non-Accelerated)";
+static char *slic_banner = "Alacritech SLIC Technology(tm) Server "
+   "and Storage Accelerator (Non-Accelerated)";
 
 static char *slic_proc_version = "2.0.351  2006/07/14 12:26:00";
 
@@ -755,10 +756,10 @@ static bool slic_mac_filter(struct adapter *adapter,
 
while (mcaddr) {
if (ether_addr_equal(mcaddr->address,
-ether_frame->ether_dhost)) 
{
-   adapter->rcv_multicasts++;
-   netdev->stats.multicast++;
-   return true;
+   ether_frame->ether_dhost)) {
+   adapter->rcv_multicasts++;
+   netdev->stats.multicast++;
+   return true;
}
mcaddr = mcaddr->next;
}
@@ -2561,8 +2562,9 @@ static int slic_ioctl(struct net_device *dev, struct 
ifreq *rq, int cmd)
DBG_IOCTL("slic_ioctl  SIOCSLIC_TRACE_DUMP\n");
 
if (copy_from_user(data, rq->ifr_data, 28)) {
-   PRINT_ERROR
-   ("slic: copy_from_user FAILED getting 
initial simba param\n");
+   PRINT_ERROR(
+   "slic: copy_from_user FAILED getting initial simba param\n"
+   );
return -EFAULT;
}
 
@@ -2576,8 +2578,9 @@ static int slic_ioctl(struct net_device *dev, struct 
ifreq *rq, int cmd)
} else if ((tracemon_request == SLIC_DUMP_REQUESTED) ||
   (tracemon_request ==
SLIC_DUMP_IN_PROGRESS)) {
-   PRINT_ERROR
-   ("ATK Diagnostic Trace Dump Requested but 
already in progress... ignore\n");
+   PRINT_ERROR(
+   "ATK Diagnostic Trace Dump Requested but already in progress... 
ignore\n"
+   );
} else {
PRINT_ERROR
("ATK Diagnostic Trace Dump Requested\n");
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: rtl8188eu: replace kzalloc and memcpy by kmemdup

2015-03-24 Thread Niranjan Dighe
This was generated by 'make coccicheck' using scripts at
scripts/coccinelle/api/memdup.cocci.

Signed-off-by: Niranjan Dighe 

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index 86d955f..be9e34a 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -5431,15 +5431,14 @@ u8 set_tx_beacon_cmd(struct adapter *padapter)
goto exit;
}
 
-   ptxBeacon_parm = kzalloc(sizeof(struct wlan_bssid_ex), GFP_KERNEL);
+   ptxBeacon_parm = kmemdup(&(pmlmeinfo->network),
+   sizeof(struct wlan_bssid_ex), GFP_KERNEL);
if (ptxBeacon_parm == NULL) {
kfree(ph2c);
res = _FAIL;
goto exit;
}
 
-   memcpy(ptxBeacon_parm, &(pmlmeinfo->network), sizeof(struct 
wlan_bssid_ex));
-
len_diff = update_hidden_ssid(ptxBeacon_parm->IEs+_BEACON_IE_OFFSET_,
  
ptxBeacon_parm->IELength-_BEACON_IE_OFFSET_,
  pmlmeinfo->hidden_ssid_mode);
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: Staging: slicoss: Fix checkpatch.pl issues

2015-03-24 Thread Joe Perches
On Wed, 2015-03-25 at 00:39 +0530, Niranjan Dighe wrote:
> The following files had coding style issues that I tried to address.
> It was mostly about lines spanning more than 80 characters.
[]
> diff --git a/drivers/staging/slicoss/slicoss.c 
> b/drivers/staging/slicoss/slicoss.c
[]
> @@ -98,7 +98,8 @@
>  #include "slic.h"
>  
>  static uint slic_first_init = 1;
> -static char *slic_banner = "Alacritech SLIC Technology(tm) Server and 
> Storage Accelerator (Non-Accelerated)";
> +static char *slic_banner = "Alacritech SLIC Technology(tm) Server "
> + "and Storage Accelerator (Non-Accelerated)";

Please don't split strings.

Maybe remove this and update the format in the one place
it's used instead.

> @@ -755,10 +756,10 @@ static bool slic_mac_filter(struct adapter *adapter,
>  
>   while (mcaddr) {
>   if (ether_addr_equal(mcaddr->address,
> -  ether_frame->ether_dhost)) 
> {
> - adapter->rcv_multicasts++;
> - netdev->stats.multicast++;
> - return true;
> + ether_frame->ether_dhost)) {
> + adapter->rcv_multicasts++;
> + netdev->stats.multicast++;
> + return true;

80 columns is a preferred limit not a hard one.
The original is correct and your new indentation is not.

> @@ -2561,8 +2562,9 @@ static int slic_ioctl(struct net_device *dev, struct 
> ifreq *rq, int cmd)
>   DBG_IOCTL("slic_ioctl  SIOCSLIC_TRACE_DUMP\n");
>  
>   if (copy_from_user(data, rq->ifr_data, 28)) {
> - PRINT_ERROR
> - ("slic: copy_from_user FAILED getting 
> initial simba param\n");
> + PRINT_ERROR(
> + "slic: copy_from_user FAILED getting initial simba param\n"
> + );

Just remove all the #ifdef SLIC_TRACE_DUMP_ENABLED blocks,
PRINT_ERROR isn't defined anywhere. and this can't be compiled.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: rtl8192u: Fix static decleration sparse warning.

2015-03-24 Thread Cihangir Akturk
The function 'ieee80211_check_auth_response' is used only in this
file, so make it static. This patch fixes the following sparse
warning.

'ieee80211_check_auth_response' was not declared. Should it be static?

Signed-off-by: Cihangir Akturk 
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c 
b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index 66b1587..f98752c 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -1921,8 +1921,8 @@ static void ieee80211_process_action(struct 
ieee80211_device *ieee,
 
 }
 
-void ieee80211_check_auth_response(struct ieee80211_device *ieee,
-  struct sk_buff *skb)
+static void ieee80211_check_auth_response(struct ieee80211_device *ieee,
+ struct sk_buff *skb)
 {
/* default support N mode, disable halfNmode */
bool bSupportNmode = true, bHalfSupportNmode = false;
-- 
2.1.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: slicoss: Fix checkpatch.pl issues

2015-03-24 Thread Niranjan Dighe
Removed unused block of code guarded by #ifdef SLIC_TRACE_DUMP_ENABLED
And removed redundant static char *slic_banner and replaced actual string in
place of format string.

Signed-off-by: Niranjan Dighe 

diff --git a/drivers/staging/slicoss/slicoss.c 
b/drivers/staging/slicoss/slicoss.c
index 3104cb0..f3110f7 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -98,7 +98,6 @@
 #include "slic.h"
 
 static uint slic_first_init = 1;
-static char *slic_banner = "Alacritech SLIC Technology(tm) Server and Storage 
Accelerator (Non-Accelerated)";
 
 static char *slic_proc_version = "2.0.351  2006/07/14 12:26:00";
 
@@ -2553,41 +2552,6 @@ static int slic_ioctl(struct net_device *dev, struct 
ifreq *rq, int cmd)
slic_intagg_set(adapter, intagg);
return 0;
 
-#ifdef SLIC_TRACE_DUMP_ENABLED
-   case SIOCSLICTRACEDUMP:
-   {
-   u32 value;
-
-   DBG_IOCTL("slic_ioctl  SIOCSLIC_TRACE_DUMP\n");
-
-   if (copy_from_user(data, rq->ifr_data, 28)) {
-   PRINT_ERROR
-   ("slic: copy_from_user FAILED getting 
initial simba param\n");
-   return -EFAULT;
-   }
-
-   value = data[0];
-   if (tracemon_request == SLIC_DUMP_DONE) {
-   PRINT_ERROR
-   ("ATK Diagnostic Trace Dump Requested\n");
-   tracemon_request = SLIC_DUMP_REQUESTED;
-   tracemon_request_type = value;
-   tracemon_timestamp = jiffies;
-   } else if ((tracemon_request == SLIC_DUMP_REQUESTED) ||
-  (tracemon_request ==
-   SLIC_DUMP_IN_PROGRESS)) {
-   PRINT_ERROR
-   ("ATK Diagnostic Trace Dump Requested but 
already in progress... ignore\n");
-   } else {
-   PRINT_ERROR
-   ("ATK Diagnostic Trace Dump Requested\n");
-   tracemon_request = SLIC_DUMP_REQUESTED;
-   tracemon_request_type = value;
-   tracemon_timestamp = jiffies;
-   }
-   return 0;
-   }
-#endif
case SIOCETHTOOL:
if (copy_from_user(&ecmd, rq->ifr_data, sizeof(ecmd)))
return -EFAULT;
@@ -3081,7 +3045,8 @@ static int slic_entry_probe(struct pci_dev *pcidev,
return err;
 
if (did_version++ == 0) {
-   dev_info(&pcidev->dev, "%s\n", slic_banner);
+   dev_info(&pcidev->dev,
+   "Alacritech SLIC Technology(tm) Server and Storage Accelerator 
(Non-Accelerated)\n");
dev_info(&pcidev->dev, "%s\n", slic_proc_version);
}
 
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: Staging: slicoss: Fix checkpatch.pl issues

2015-03-24 Thread Dan Carpenter
On Wed, Mar 25, 2015 at 12:39:06AM +0530, Niranjan Dighe wrote:
> The following files had coding style issues that I tried to address.
> It was mostly about lines spanning more than 80 characters.
> 
> Signed-off-by: Niranjan Dighe 
> 
> diff --git a/drivers/staging/slicoss/slicoss.c 
> b/drivers/staging/slicoss/slicoss.c
> index 3104cb0..2161bdb 100644
> --- a/drivers/staging/slicoss/slicoss.c
> +++ b/drivers/staging/slicoss/slicoss.c
> @@ -98,7 +98,8 @@
>  #include "slic.h"
>  
>  static uint slic_first_init = 1;
> -static char *slic_banner = "Alacritech SLIC Technology(tm) Server and 
> Storage Accelerator (Non-Accelerated)";
> +static char *slic_banner = "Alacritech SLIC Technology(tm) Server "
> + "and Storage Accelerator (Non-Accelerated)";

Just leave it.  The original is better for grepping.

>  
>  static char *slic_proc_version = "2.0.351  2006/07/14 12:26:00";
>  
> @@ -755,10 +756,10 @@ static bool slic_mac_filter(struct adapter *adapter,
>  
>   while (mcaddr) {
>   if (ether_addr_equal(mcaddr->address,
> -  ether_frame->ether_dhost)) 
> {
> - adapter->rcv_multicasts++;
> - netdev->stats.multicast++;
> - return true;
> + ether_frame->ether_dhost)) {
> + adapter->rcv_multicasts++;
> + netdev->stats.multicast++;
> + return true;
>   }
>   mcaddr = mcaddr->next;
>   }

Check patch likes the new code but the original is better for human
beings.  Sorry.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: Staging: slicoss: Fix checkpatch.pl issues

2015-03-24 Thread Joe Perches
On Tue, 2015-03-24 at 23:14 +0300, Dan Carpenter wrote:
> On Wed, Mar 25, 2015 at 12:39:06AM +0530, Niranjan Dighe wrote:
> > The following files had coding style issues that I tried to address.
> > It was mostly about lines spanning more than 80 characters.
[]
> > diff --git a/drivers/staging/slicoss/slicoss.c 
> > b/drivers/staging/slicoss/slicoss.c
[]
> > @@ -755,10 +756,10 @@ static bool slic_mac_filter(struct adapter *adapter,
> >  
> > while (mcaddr) {
> > if (ether_addr_equal(mcaddr->address,
> > -ether_frame->ether_dhost)) 
> > {
> > -   adapter->rcv_multicasts++;
> > -   netdev->stats.multicast++;
> > -   return true;
> > +   ether_frame->ether_dhost)) {
> > +   adapter->rcv_multicasts++;
> > +   netdev->stats.multicast++;
> > +   return true;
> > }
> > mcaddr = mcaddr->next;
> > }
> 
> Check patch likes the new code but the original is better for human
> beings.  Sorry.

The indentation isn't correct in the new code either.

That's actually a checkpatch defect because it's not
able to track deletion/insertion alignment well enough.

Niranjan, checkpatch is a brainless little tool.
Always use yours whenever using checkpatch.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] octeon-usb:Fix coding style issue with space between function name and opening bracket

2015-03-24 Thread Aaro Koskinen
Hi,

On Tue, Mar 24, 2015 at 08:08:16AM -0400, Nicholas Krause wrote:
> On March 24, 2015 3:08:09 AM EDT, Dan Carpenter  
> wrote:
> >On Mon, Mar 23, 2015 at 05:08:52PM -0400, Nicholas Krause wrote:
> >> On March 23, 2015 9:06:26 AM EDT, Dan Carpenter
> > wrote:
> >> >On Mon, Mar 23, 2015 at 08:53:19AM -0400, Nicholas Krause wrote:
> >> >> On March 23, 2015 6:40:06 AM EDT, Dan Carpenter
> >> > wrote:
> >> >> >On Sun, Mar 22, 2015 at 08:04:35PM -0400, Nicholas Krause wrote:
> >> >> >> Fixes the checkpath.pl error where the opening bracket has a
> >> >unneeded
> >> >> >space between
> >> >> >> it and the function name for a if statement in the
> >> >> >marco,VMX_WAIT_FOR_FIELD32.
> >> >> >> 
> >> >> >> Signed-off-by: Nicholas Krause 
> >> >> >> ---
> >> >> >>  drivers/staging/octeon-usb/octeon-hcd.c | 2 +-
> >> >> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >> >> 
> >> >> >> diff --git a/drivers/staging/octeon-usb/octeon-hcd.c
> >> >> >b/drivers/staging/octeon-usb/octeon-hcd.c
> >> >> >> index 1daeb31..0408a12 100644
> >> >> >> --- a/drivers/staging/octeon-usb/octeon-hcd.c
> >> >> >> +++ b/drivers/staging/octeon-usb/octeon-hcd.c
> >> >> >> @@ -412,7 +412,7 @@ struct octeon_hcd {
> >> >> >>  type c; 
> >> >> >> \
> >> >> >>  while (1) { 
> >> >> >> \
> >> >> >>  c.u32 = __cvmx_usb_read_csr32(usb, address);
> >> >> >> \
> >> >> >> -if (c.s.field op (value)) { 
> >> >> >> \
> >> >> >> +if (c.s.field op(value)) {  
> >> >> >> \
> >> >> >
> >> >> >"op" is not a function here.  This is macro and top is an
> >operation
> >> >> >like
> >> >> >"+" or "*".  So checkpatch.pl is wrong.
> >> >> >
> >> >> >regards,
> >> >> >dan carpenter
> >> >> Very well then, I wasn't sure if we did the  same spacing style
> >for
> >> >marcos. 
> >> >
> >> >We do but this is a very unusual case.
> >> >
> >> >regards,
> >> >dan carpenter
> >> Dan,  
> >> Sorry about that my mistake.  In addition, have you looked at my
> >second patch to see if that one is correct. 
> >
> >Yes.  That one is fine.
> >
> >regards,
> >dan carpenter
> Dan, 
> Just one more question, is that patch going to be merged or should I resubmit 
> it as a series of one patch?

Someone else already sent the same patch that got applied:

https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/commit/drivers/staging/octeon-usb?h=staging-testing&id=877945759df5ff6e7fb434cb5b6cddb606c2f66f

A.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: fbtft: Do not use binary constants

2015-03-24 Thread Greg Kroah-Hartman
On Fri, Mar 20, 2015 at 04:19:36PM +0100, Geert Uytterhoeven wrote:
> Hi Greg,
> 
> On Fri, Mar 20, 2015 at 1:54 PM, Greg Kroah-Hartman
>  wrote:
> > On Tue, Mar 10, 2015 at 10:34:43PM +0100, Geert Uytterhoeven wrote:
> >> Gcc < 4.3 doesn't understand binary constants (0b*):
> >>
> >> drivers/staging/fbtft/fbtft-sysfs.c:156:19: error: invalid suffix "b111" 
> >> on integer constant
> >> drivers/staging/fbtft/fb_hx8340bn.c:159:3: error: invalid suffix "b" 
> >> on integer constant
> >> drivers/staging/fbtft/fb_hx8340bn.c:159:11: error: invalid suffix "b" 
> >> on integer constant
> >> drivers/staging/fbtft/fb_hx8340bn.c:159:19: error: invalid suffix "b1" 
> >> on integer constant
> >> drivers/staging/fbtft/fb_hx8340bn.c:159:28: error: invalid suffix "b" 
> >> on integer constant
> >> drivers/staging/fbtft/fb_hx8340bn.c:159:36: error: invalid suffix "b" 
> >> on integer constant
> >> drivers/staging/fbtft/fb_hx8340bn.c:159:44: error: invalid suffix "b" 
> >> on integer constant
> >> drivers/staging/fbtft/fb_hx8340bn.c:159:52: error: invalid suffix "b1" 
> >> on integer constant
> >> drivers/staging/fbtft/fb_hx8340bn.c:160:3: error: invalid suffix "b111" on 
> >> integer constant
> >> ...
> >>
> >> Hence use hexadecimal constants (0x*) instead.
> >>
> >> Signed-off-by: Geert Uytterhoeven 
> >> ---
> >> This is against v4.0-rc3. In next-20150310 there are two whitespace
> >> differences.
> >
> > Can you make it against -next?  I can't get this to apply as-is, sorry.
> 
> Sure, will do. I was wondering if you wanted to sent this fix to Linus for 
> v4.0,
> as the build is broken for gcc < 4.3, and keep the changes in -next for v4.1.

Nah, it's just staging code, it's ok to be broken on old compilers :)

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/2] staging: panel: fix lcd type in module parameters

2015-03-24 Thread Willy Tarreau
On Tue, Mar 24, 2015 at 02:56:33PM +0530, Sudip Mukherjee wrote:
> with reference to the previous patch of the series, fixed the
> lcd type in module parameters.

Sudip, it's better to avoid fragmenting patches like you did, because
it will result in a kernel state where there is an inconsistency between
the parameters actually used by the kernel and those reported by modinfo.
This can happen for example if someone does a bisect and ends up on patch
1/2 applied only. Obviously in this case there is very little harm, but
you get the idea : each patch should be a functional change, address one
thing and do it consistently. So if you change the #defines or enums or
whatever, all the locations where their old values were referenced must
be changed in the same patch, simply because in fact they are duplicate
entries. Another point is that someone who notices your patch v1 and
does not notice patch v2 could pick v1 for his kernel and end up with
something inconsistent. For this reason it would be better to merge your
patches into a single one here.

> might not apply properly to old versions, some reordering was done
> in commit <98e0e762e>

Appreciated, thanks for checking this!

Best regards,
Willy

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3] staging: panel: fix lcd type

2015-03-24 Thread Willy Tarreau
On Tue, Mar 24, 2015 at 04:29:32PM +0530, Sudip Mukherjee wrote:
> the lcd type as defined in the Kconfig is not matching in the code.
> as a result the rs, rw and en pins were getting interchanged.
> Kconfig defines the value of PANEL_LCD to be 1 if we select custom
> configuration but in the code LCD_TYPE_CUSTOM is defined as 5.
> 
> my hardware is LCD_TYPE_CUSTOM, but the pins were assigned to it
> as pins of LCD_TYPE_OLD, and it was not working.
> Now values are corrected with referenece to the values defined in
> Kconfig and it is working.
> checked on JHD204A lcd with LCD_TYPE_CUSTOM configuration.
> 
> Cc:  # 2.6.32+
> Signed-off-by: Sudip Mukherjee 
> ---
> 
> v3: added the module_parameters
> v2: cc-ed  stable

Acked-by: Willy Tarreau 

[ next time I'll read my mails in reverse order :-) ]

Willy

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: rtl8723au: Update RT_TRACE macro and uses

2015-03-24 Thread Jes Sorensen
Joe Perches  writes:
> Create an rt_trace function using %pV to reduce overall code size.
> Update the macro uses to remove unnecessary and now harmful parentheses.
>
> Miscellanea around these changes:
>
> o Coalesce formats
> o Realign arguments
> o Remove commented-out RT_TRACE uses
> o Spelling fixes in formats
> o Add missing newlines to formats
> o Remove multiple newlines from formats
> o Neaten formats where noticed
> o Use %pM in one instance
>
> Reduces code size ~20KB
>
> Signed-off-by: Joe Perches 
> ---
> Mostly done by various scripts&emacs.
> Compiled, untested, no hardware,

This could be further improved by fixing up all the places where the
function name is hard coded into the print statement, instead of using
__func__. In particular as a lot of it is carried over from old code
and has been renamed since.

It's OK with me to do this in a follow-on patch though.

Jes
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: rtl8723au: Update RT_TRACE macro and uses

2015-03-24 Thread Joe Perches
On Tue, 2015-03-24 at 20:28 -0400, Jes Sorensen wrote:
> Joe Perches  writes:
> > Create an rt_trace function using %pV to reduce overall code size.
> > Update the macro uses to remove unnecessary and now harmful parentheses.
[]
> This could be further improved by fixing up all the places where the
> function name is hard coded into the print statement, instead of using
> __func__. In particular as a lot of it is carried over from old code
> and has been renamed since.

There's a cocci script for that.

http://cocci.systeme.lip6.narkive.com/nKXf6Bmy/finding-embedded-function-names

I expect someone will run it one day on staging.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/2] staging: panel: fix lcd type in module parameters

2015-03-24 Thread Sudip Mukherjee
On Wed, Mar 25, 2015 at 12:25:59AM +0100, Willy Tarreau wrote:
> On Tue, Mar 24, 2015 at 02:56:33PM +0530, Sudip Mukherjee wrote:
> > with reference to the previous patch of the series, fixed the
> > lcd type in module parameters.
> 
> Sudip, it's better to avoid fragmenting patches like you did, because
> it will result in a kernel state where there is an inconsistency between
> the parameters actually used by the kernel and those reported by modinfo.
> This can happen for example if someone does a bisect and ends up on patch
> 1/2 applied only. Obviously in this case there is very little harm, but
> you get the idea : each patch should be a functional change, address one
> thing and do it consistently. So if you change the #defines or enums or
> whatever, all the locations where their old values were referenced must
> be changed in the same patch, simply because in fact they are duplicate
> entries. Another point is that someone who notices your patch v1 and
> does not notice patch v2 could pick v1 for his kernel and end up with
> something inconsistent. For this reason it would be better to merge your
> patches into a single one here.
explained very well. thanks. i was wondering why Dan asked me to merge
them into one.

regards
sudip

> 
> > might not apply properly to old versions, some reordering was done
> > in commit <98e0e762e>
> 
> Appreciated, thanks for checking this!
> 
> Best regards,
> Willy
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel