Skip to contentSkip to conversation

staging: comedi: cleanup comedi_alloc_subdevices

PATCH In review H Hartley Sweeten 2 participants 4 messages Jun 12, 2012 – Jun 13, 2012 Staging

lore
Share & tools
lore

H Hartley Sweeten

author
#0 lore raw

PATCH 8/8 staging: comedi: cleanup comedi_alloc_subdevices

Quoted 1 line

From 65db79e570481b04bfc13663fd40de18560f0aa4 Mon Sep 17 00:00:00 2001

From: H Hartley Sweeten <[email protected]>
Date: Tue, 12 Jun 2012 11:37:09 -0700
Subject: [PATCH 8/8] staging: comedi: cleanup comedi_alloc_subdevices

Access the individual comedi_subdevices using a pointer instead of directly accessing as an array. This is how the rest of the comedi core accesses them.

Signed-off-by H Hartley Sweeten <[email protected]>
Cc Ian Abbott <[email protected]>
Cc Frank Mori Hess <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
---
 drivers/staging/comedi/drivers.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)
drivers/staging/comedi/drivers.c +11 -8
index 238c29d..c41ddb3 100644
@@ -58,21 +58,24 @@ struct comedi_driver *comedi_drivers;
 
 int comedi_alloc_subdevices(struct comedi_device *dev, int num_subdevices)
 {
+	struct comedi_subdevice *s;
 	int i;
 
 	if (num_subdevices < 1)
 		return -EINVAL;
-	dev->subdevices =
-	    kcalloc(num_subdevices, sizeof(struct comedi_subdevice),
-		    GFP_KERNEL);
-	if (!dev->subdevices)
+
+	s = kcalloc(num_subdevices, sizeof(*s), GFP_KERNEL);
+	if (!s)
 		return -ENOMEM;
+	dev->subdevices = s;
 	dev->n_subdevices = num_subdevices;
+
 	for (i = 0; i < num_subdevices; ++i) {
-		dev->subdevices[i].device = dev;
-		dev->subdevices[i].async_dma_dir = DMA_NONE;
-		spin_lock_init(&dev->subdevices[i].spin_lock);
-		dev->subdevices[i].minor = -1;
+		s = dev->subdevices + i;
+		s->device = dev;
+		s->async_dma_dir = DMA_NONE;
+		spin_lock_init(&s->spin_lock);
+		s->minor = -1;
 	}
 	return 0;
 }
Signature · 1 line
1.7.7
#1 lore raw

drivers/staging/comedi/drivers.c struct comedi_driver *comedi_drivers; · line 74

Quoted 6 lines · H Hartley Sweeten
drivers/staging/comedi/drivers.c +1 -4
 	for (i = 0; i < num_subdevices; ++i) {
-		dev->subdevices[i].device = dev;
-		dev->subdevices[i].async_dma_dir = DMA_NONE;
-		spin_lock_init(&dev->subdevices[i].spin_lock);
-		dev->subdevices[i].minor = -1;
+		s = dev->subdevices + i;

You don't have to resend, but I think this would look better as:

		s = &dev->subdevices[i];

drivers/staging/comedi/drivers.c struct comedi_driver *comedi_drivers; · line 79

Quoted 5 lines
drivers/staging/comedi/drivers.c +4 -0
+		s->device = dev;
+		s->async_dma_dir = DMA_NONE;
+		spin_lock_init(&s->spin_lock);
+		s->minor = -1;
 	}

Btw, this patchset is great. Nice.

regards,
dan carpenter

author
#2 lore raw
Quoted 11 lines · Dan Carpenter

drivers/staging/comedi/drivers.c struct comedi_driver *comedi_drivers; · line 74

Quoted 6 lines · H Hartley Sweeten
drivers/staging/comedi/drivers.c +1 -4
 	for (i = 0; i < num_subdevices; ++i) {
-		dev->subdevices[i].device = dev;
-		dev->subdevices[i].async_dma_dir = DMA_NONE;
-		spin_lock_init(&dev->subdevices[i].spin_lock);
-		dev->subdevices[i].minor = -1;
+		s = dev->subdevices + i;

You don't have to resend, but I think this would look better as:

		s = &dev->subdevices[i];

I don't disagree but the "dev->subdevices +i" format is consistently used in all the comedi stuff. If the format above is preferred we should probably update everything,

Quoted 7 lines

drivers/staging/comedi/drivers.c struct comedi_driver *comedi_drivers; · line 79

Quoted 5 lines
drivers/staging/comedi/drivers.c +4 -0
+		s->device = dev;
+		s->async_dma_dir = DMA_NONE;
+		spin_lock_init(&s->spin_lock);
+		s->minor = -1;
 	}

Btw, this patchset is great. Nice.

Thanks!
Hartley

#3 lore raw
Quoted 11 lines · H Hartley Sweeten
Quoted 10 lines · Dan Carpenter

drivers/staging/comedi/drivers.c struct comedi_driver *comedi_drivers; · line 74

Quoted 6 lines · H Hartley Sweeten
drivers/staging/comedi/drivers.c +1 -4
 	for (i = 0; i < num_subdevices; ++i) {
-		dev->subdevices[i].device = dev;
-		dev->subdevices[i].async_dma_dir = DMA_NONE;
-		spin_lock_init(&dev->subdevices[i].spin_lock);
-		dev->subdevices[i].minor = -1;
+		s = dev->subdevices + i;

You don't have to resend, but I think this would look better as:

Quoted 5 lines
Quoted 1 line
		s = &dev->subdevices[i];

I don't disagree but the "dev->subdevices +i" format is consistently used in all the comedi stuff. If the format above is preferred we should probably update everything,

To me the one is way more clear than the other. If we wanted we could write any array in terms of pointer math, but arrays are easier to understand.

regards,
dan carpenter