[PATCH 0/6] Staging: kpc2000: kpc_dma: Clear Misc Checkpatch.pl WARNINGS.

[PATCH 0/6] Staging: kpc2000: kpc_dma: Clear Misc Checkpatch.pl WARNINGS.

[Sam Muhammed]

This driver has multiple WARNINGS and CHECKS,
Changes been made to match coding style and standards.
Modifications:
	- Remove Comparison to NULL.
	- Proper usage of kzalloc().
	- Reformat single statement blocks.
	- Including preferred header.
	- Putting spaces around operators.
	- Using the proper SPDK comment style.

Sam Muhammed (6):
  Staging: kpc2000: kpc_dma: Remove comparison to NULL.
  Staging: kpc2000: kpc_dma: Use sizeof(*var) in kzalloc().
  Staging: kpc2000: kpc_dma: Remove unnecessary braces.
  Staging: kpc2000: kpc_dma: Use spaces around operators.
  Staging: kpc2000: kpc_dma: Include the preferred header.
  Staging: kpc2000: kpc_dma: Use the SPDK comment style.

 drivers/staging/kpc2000/kpc_dma/dma.c         |  9 ++--
 drivers/staging/kpc2000/kpc_dma/fileops.c     | 44 +++++++++----------
 .../staging/kpc2000/kpc_dma/kpc_dma_driver.c  |  9 ++--
 .../staging/kpc2000/kpc_dma/kpc_dma_driver.h  |  4 +-
 4 files changed, 31 insertions(+), 35 deletions(-)

---
2.20.1

[PATCH 1/6] Staging: kpc2000: kpc_dma: Remove comparison to NULL.

[Sam Muhammed]

Comparison to NULL been used across the driver,
remove them and use (!var) instead.

Checkpatch.pl: CHECK:
Comparison to NULL could be written "!desc"... etc

Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
---
 drivers/staging/kpc2000/kpc_dma/dma.c            |  2 +-
 drivers/staging/kpc2000/kpc_dma/fileops.c        | 12 ++++++------
 drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.h |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/dma.c b/drivers/staging/kpc2000/kpc_dma/dma.c
index 51a4dd534a0d..4bfbc717bc07 100644
--- a/drivers/staging/kpc2000/kpc_dma/dma.c
+++ b/drivers/staging/kpc2000/kpc_dma/dma.c
@@ -236,7 +236,7 @@ int  count_descriptors_available(struct kpc_dma_device *eng)
 	struct kpc_dma_descriptor *cur = eng->desc_next;

 	while (cur != eng->desc_completed) {
-		BUG_ON(cur == NULL);
+		BUG_ON(!cur);
 		count++;
 		cur = cur->Next;
 	}
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 40525540dde6..5eb6c5f24feb 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -124,7 +124,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
 		pcnt = count_parts_for_sge(sg);
 		for (p = 0 ; p < pcnt ; p++) {
 			// Fill out the descriptor
-			BUG_ON(desc == NULL);
+			BUG_ON(!desc);
 			clear_desc(desc);
 			if (p != pcnt-1) {
 				desc->DescByteCount = 0x80000;
@@ -203,11 +203,11 @@ void  transfer_complete_cb(struct aio_cb_data *acd, size_t xfr_count, u32 flags)
 {
 	unsigned int i;

-	BUG_ON(acd == NULL);
-	BUG_ON(acd->user_pages == NULL);
-	BUG_ON(acd->sgt.sgl == NULL);
-	BUG_ON(acd->ldev == NULL);
-	BUG_ON(acd->ldev->pldev == NULL);
+	BUG_ON(!acd);
+	BUG_ON(!acd->user_pages);
+	BUG_ON(!acd->sgt.sgl);
+	BUG_ON(!acd->ldev);
+	BUG_ON(!acd->ldev->pldev);

 	for (i = 0 ; i < acd->page_count ; i++) {
 		if (!PageReserved(acd->user_pages[i])) {
diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.h b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.h
index 4c8cc866b826..8b9c978257b9 100644
--- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.h
+++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.h
@@ -198,14 +198,14 @@ u32  GetEngineCompletePtr(struct kpc_dma_device *eng)
 static inline
 void  lock_engine(struct kpc_dma_device *eng)
 {
-	BUG_ON(eng == NULL);
+	BUG_ON(!eng);
 	mutex_lock(&eng->sem);
 }

 static inline
 void  unlock_engine(struct kpc_dma_device *eng)
 {
-	BUG_ON(eng == NULL);
+	BUG_ON(!eng);
 	mutex_unlock(&eng->sem);
 }

---
2.20.1

[PATCH 2/6] Staging: kpc2000: kpc_dma: Use sizeof(*var) in kzalloc().

[Sam Muhammed]

Modify struct allocations in kzalloc() to match the
coding standards.

Checkpatch.pl CHECK: Prefer kzalloc(sizeof(*var)...)
over kzalloc(sizeof(struct var)...)

Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
---
 drivers/staging/kpc2000/kpc_dma/fileops.c        | 2 +-
 drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 5eb6c5f24feb..21ebab7dbc7b 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -253,7 +253,7 @@ int  kpc_dma_open(struct inode *inode, struct file *filp)
 		return -EBUSY; /* already open */
 	}

-	priv = kzalloc(sizeof(struct dev_private_data), GFP_KERNEL);
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;

diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
index ec79a8500caf..25ea16a1981e 100644
--- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
+++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
@@ -98,7 +98,7 @@ int  kpc_dma_probe(struct platform_device *pldev)
 	int rv = 0;
 	dev_t dev;

-	struct kpc_dma_device *ldev = kzalloc(sizeof(struct kpc_dma_device), GFP_KERNEL);
+	struct kpc_dma_device *ldev = kzalloc(sizeof(*ldev), GFP_KERNEL);

 	if (!ldev) {
 		dev_err(&pldev->dev, "%s: unable to kzalloc space for kpc_dma_device\n", __func__);
---
2.20.1

[PATCH 3/6] Staging: kpc2000: kpc_dma: Remove unnecessary braces.

[Sam Muhammed]

Removing braces of single statement blocks,
they are not really needed.

Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
---
 drivers/staging/kpc2000/kpc_dma/dma.c         |  5 ++---
 drivers/staging/kpc2000/kpc_dma/fileops.c     | 20 +++++++++----------
 .../staging/kpc2000/kpc_dma/kpc_dma_driver.c  |  3 +--
 3 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/dma.c b/drivers/staging/kpc2000/kpc_dma/dma.c
index 4bfbc717bc07..5ccc93ed2b61 100644
--- a/drivers/staging/kpc2000/kpc_dma/dma.c
+++ b/drivers/staging/kpc2000/kpc_dma/dma.c
@@ -97,11 +97,10 @@ int  setup_dma_engine(struct kpc_dma_device *eng, u32 desc_cnt)
 	if (WARN(!(caps & ENG_CAP_PRESENT), "%s() called for DMA Engine at %p which isn't present in hardware!\n", __func__, eng))
 		return -ENXIO;

-	if (caps & ENG_CAP_DIRECTION) {
+	if (caps & ENG_CAP_DIRECTION)
 		eng->dir = DMA_FROM_DEVICE;
-	} else {
+	else
 		eng->dir = DMA_TO_DEVICE;
-	}

 	eng->desc_pool_cnt = desc_cnt;
 	eng->desc_pool = dma_pool_create("KPC DMA Descriptors", &eng->pldev->dev, sizeof(struct kpc_dma_descriptor), DMA_DESC_ALIGNMENT, 4096);
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 21ebab7dbc7b..d93097ed781c 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -126,11 +126,11 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
 			// Fill out the descriptor
 			BUG_ON(!desc);
 			clear_desc(desc);
-			if (p != pcnt-1) {
+			if (p != pcnt-1)
 				desc->DescByteCount = 0x80000;
-			} else {
+			else
 				desc->DescByteCount = sg_dma_len(sg) - (p * 0x80000);
-			}
+
 			desc->DescBufferByteCount = desc->DescByteCount;

 			desc->DescControlFlags |= DMA_DESC_CTL_IRQONERR;
@@ -148,9 +148,9 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
 			desc->DescSystemAddrMS = (dma_addr & 0xFFFFFFFF00000000UL) >> 32;

 			user_ctl = acd->priv->user_ctl;
-			if (i == acd->mapped_entry_count-1 && p == pcnt-1) {
+			if (i == acd->mapped_entry_count-1 && p == pcnt-1)
 				user_ctl = acd->priv->user_ctl_last;
-			}
+
 			desc->DescUserControlLS = (user_ctl & 0x00000000FFFFFFFFUL) >>  0;
 			desc->DescUserControlMS = (user_ctl & 0xFFFFFFFF00000000UL) >> 32;

@@ -188,9 +188,9 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
 	sg_free_table(&acd->sgt);
  err_dma_map_sg:
  err_alloc_sg_table:
-	for (i = 0 ; i < acd->page_count ; i++) {
+	for (i = 0 ; i < acd->page_count ; i++)
 		put_page(acd->user_pages[i]);
-	}
+
  err_get_user_pages:
 	kfree(acd->user_pages);
  err_alloc_userpages:
@@ -210,16 +210,14 @@ void  transfer_complete_cb(struct aio_cb_data *acd, size_t xfr_count, u32 flags)
 	BUG_ON(!acd->ldev->pldev);

 	for (i = 0 ; i < acd->page_count ; i++) {
-		if (!PageReserved(acd->user_pages[i])) {
+		if (!PageReserved(acd->user_pages[i]))
 			set_page_dirty(acd->user_pages[i]);
-		}
 	}

 	dma_unmap_sg(&acd->ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, acd->ldev->dir);

-	for (i = 0 ; i < acd->page_count ; i++) {
+	for (i = 0 ; i < acd->page_count ; i++)
 		put_page(acd->user_pages[i]);
-	}

 	sg_free_table(&acd->sgt);

diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
index 25ea16a1981e..bc81afb6ade2 100644
--- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
+++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
@@ -26,9 +26,8 @@ struct kpc_dma_device *kpc_dma_lookup_device(int minor)

 	mutex_lock(&kpc_dma_mtx);
 	list_for_each_entry(c, &kpc_dma_list, list) {
-		if (c->pldev->id == minor) {
+		if (c->pldev->id == minor)
 			goto out;
-		}
 	}
 	c = NULL; // not-found case
 out:
---
2.20.1

[PATCH 4/6] Staging: kpc2000: kpc_dma: Use spaces around operators.

[Sam Muhammed]

Cleanup Checkpatch.pl CHECKs about missing
spaces around multiple operators.

Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
---
 drivers/staging/kpc2000/kpc_dma/fileops.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
index d93097ed781c..e599ae1bdfd0 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -19,7 +19,7 @@ static inline
 unsigned int  count_pages(unsigned long iov_base, size_t iov_len)
 {
 	unsigned long first = (iov_base             & PAGE_MASK) >> PAGE_SHIFT;
-	unsigned long last  = ((iov_base+iov_len-1) & PAGE_MASK) >> PAGE_SHIFT;
+	unsigned long last  = ((iov_base + iov_len - 1) & PAGE_MASK) >> PAGE_SHIFT;

 	return last - first + 1;
 }
@@ -83,7 +83,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
 	}

 	// Allocate and setup the sg_table (scatterlist entries)
-	rv = sg_alloc_table_from_pages(&acd->sgt, acd->user_pages, acd->page_count, iov_base & (PAGE_SIZE-1), iov_len, GFP_KERNEL);
+	rv = sg_alloc_table_from_pages(&acd->sgt, acd->user_pages, acd->page_count, iov_base & (PAGE_SIZE - 1), iov_len, GFP_KERNEL);
 	if (rv) {
 		dev_err(&priv->ldev->pldev->dev, "Couldn't alloc sg_table (%ld)\n", rv);
 		goto err_alloc_sg_table;
@@ -126,7 +126,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
 			// Fill out the descriptor
 			BUG_ON(!desc);
 			clear_desc(desc);
-			if (p != pcnt-1)
+			if (p != pcnt - 1)
 				desc->DescByteCount = 0x80000;
 			else
 				desc->DescByteCount = sg_dma_len(sg) - (p * 0x80000);
@@ -136,7 +136,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
 			desc->DescControlFlags |= DMA_DESC_CTL_IRQONERR;
 			if (i == 0 && p == 0)
 				desc->DescControlFlags |= DMA_DESC_CTL_SOP;
-			if (i == acd->mapped_entry_count-1 && p == pcnt-1)
+			if (i == acd->mapped_entry_count - 1 && p == pcnt - 1)
 				desc->DescControlFlags |= DMA_DESC_CTL_EOP | DMA_DESC_CTL_IRQONDONE;

 			desc->DescCardAddrLS = (card_addr & 0xFFFFFFFF);
@@ -148,13 +148,13 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
 			desc->DescSystemAddrMS = (dma_addr & 0xFFFFFFFF00000000UL) >> 32;

 			user_ctl = acd->priv->user_ctl;
-			if (i == acd->mapped_entry_count-1 && p == pcnt-1)
+			if (i == acd->mapped_entry_count - 1 && p == pcnt - 1)
 				user_ctl = acd->priv->user_ctl_last;

 			desc->DescUserControlLS = (user_ctl & 0x00000000FFFFFFFFUL) >>  0;
 			desc->DescUserControlMS = (user_ctl & 0xFFFFFFFF00000000UL) >> 32;

-			if (i == acd->mapped_entry_count-1 && p == pcnt-1)
+			if (i == acd->mapped_entry_count - 1 && p == pcnt - 1)
 				desc->acd = acd;

 			dev_dbg(&priv->ldev->pldev->dev, "  Filled descriptor %p (acd = %p)\n", desc, desc->acd);
---
2.20.1

[PATCH 5/6] Staging: kpc2000: kpc_dma: Include the preferred header.

[Sam Muhammed]

<linux/io.h> is the preferred header to include
instead of <asm/io.h>.

Checkpatch.pl WARNING:
Use #include <linux/io.h> instead of <asm/io.h>

Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
---
 drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
index bc81afb6ade2..0bdd345cc8c8 100644
--- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
+++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
@@ -2,7 +2,7 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/types.h>
-#include <asm/io.h>
+#include <linux/io.h>
 #include <linux/export.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
---
2.20.1

[PATCH 6/6] Staging: kpc2000: kpc_dma: Use the SPDK comment style.

[Sam Muhammed]

SPDK-License-Identifier comment should have this form
// SPDX-License-Identifier: <GPL-...>

Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
---
 drivers/staging/kpc2000/kpc_dma/dma.c            | 2 +-
 drivers/staging/kpc2000/kpc_dma/fileops.c        | 2 +-
 drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/dma.c b/drivers/staging/kpc2000/kpc_dma/dma.c
index 5ccc93ed2b61..452a3f7c835d 100644
--- a/drivers/staging/kpc2000/kpc_dma/dma.c
+++ b/drivers/staging/kpc2000/kpc_dma/dma.c
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
+// SPDX-License-Identifier: GPL-2.0+
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/types.h>
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
index e599ae1bdfd0..15089a8f78c0 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
+// SPDX-License-Identifier: GPL-2.0+
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/mm.h>
diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
index 0bdd345cc8c8..c3b30551e0ca 100644
--- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
+++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
+// SPDX-License-Identifier: GPL-2.0+
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/types.h>
---
2.20.1

Re: [Outreachy kernel] [PATCH 4/6] Staging: kpc2000: kpc_dma: Use spaces around operators.

[Julia Lawall]

On Tue, 24 Mar 2020, Sam Muhammed wrote:

> Cleanup Checkpatch.pl CHECKs about missing
> spaces around multiple operators.
>
> Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> ---
>  drivers/staging/kpc2000/kpc_dma/fileops.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> index d93097ed781c..e599ae1bdfd0 100644
> --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> @@ -19,7 +19,7 @@ static inline
>  unsigned int  count_pages(unsigned long iov_base, size_t iov_len)
>  {
>  	unsigned long first = (iov_base             & PAGE_MASK) >> PAGE_SHIFT;
> -	unsigned long last  = ((iov_base+iov_len-1) & PAGE_MASK) >> PAGE_SHIFT;
> +	unsigned long last  = ((iov_base + iov_len - 1) & PAGE_MASK) >> PAGE_SHIFT;

It seems that the & operators were lined up before and that is not the
case any more.

julia

>
>  	return last - first + 1;
>  }
> @@ -83,7 +83,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
>  	}
>
>  	// Allocate and setup the sg_table (scatterlist entries)
> -	rv = sg_alloc_table_from_pages(&acd->sgt, acd->user_pages, acd->page_count, iov_base & (PAGE_SIZE-1), iov_len, GFP_KERNEL);
> +	rv = sg_alloc_table_from_pages(&acd->sgt, acd->user_pages, acd->page_count, iov_base & (PAGE_SIZE - 1), iov_len, GFP_KERNEL);
>  	if (rv) {
>  		dev_err(&priv->ldev->pldev->dev, "Couldn't alloc sg_table (%ld)\n", rv);
>  		goto err_alloc_sg_table;
> @@ -126,7 +126,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
>  			// Fill out the descriptor
>  			BUG_ON(!desc);
>  			clear_desc(desc);
> -			if (p != pcnt-1)
> +			if (p != pcnt - 1)
>  				desc->DescByteCount = 0x80000;
>  			else
>  				desc->DescByteCount = sg_dma_len(sg) - (p * 0x80000);
> @@ -136,7 +136,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
>  			desc->DescControlFlags |= DMA_DESC_CTL_IRQONERR;
>  			if (i == 0 && p == 0)
>  				desc->DescControlFlags |= DMA_DESC_CTL_SOP;
> -			if (i == acd->mapped_entry_count-1 && p == pcnt-1)
> +			if (i == acd->mapped_entry_count - 1 && p == pcnt - 1)
>  				desc->DescControlFlags |= DMA_DESC_CTL_EOP | DMA_DESC_CTL_IRQONDONE;
>
>  			desc->DescCardAddrLS = (card_addr & 0xFFFFFFFF);
> @@ -148,13 +148,13 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
>  			desc->DescSystemAddrMS = (dma_addr & 0xFFFFFFFF00000000UL) >> 32;
>
>  			user_ctl = acd->priv->user_ctl;
> -			if (i == acd->mapped_entry_count-1 && p == pcnt-1)
> +			if (i == acd->mapped_entry_count - 1 && p == pcnt - 1)
>  				user_ctl = acd->priv->user_ctl_last;
>
>  			desc->DescUserControlLS = (user_ctl & 0x00000000FFFFFFFFUL) >>  0;
>  			desc->DescUserControlMS = (user_ctl & 0xFFFFFFFF00000000UL) >> 32;
>
> -			if (i == acd->mapped_entry_count-1 && p == pcnt-1)
> +			if (i == acd->mapped_entry_count - 1 && p == pcnt - 1)
>  				desc->acd = acd;
>
>  			dev_dbg(&priv->ldev->pldev->dev, "  Filled descriptor %p (acd = %p)\n", desc, desc->acd);
> ---
> 2.20.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/d881bf9d5c442ae5a0d6669624355f7af73f5fac.1585082158.git.jane.pnx9%40gmail.com.
>

Re: [Outreachy kernel] [PATCH 3/6] Staging: kpc2000: kpc_dma: Remove unnecessary braces.

[Julia Lawall]

On Tue, 24 Mar 2020, Sam Muhammed wrote:

> Removing braces of single statement blocks,

removing is not imperative.

> they are not really needed.
>
> Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> ---
>  drivers/staging/kpc2000/kpc_dma/dma.c         |  5 ++---
>  drivers/staging/kpc2000/kpc_dma/fileops.c     | 20 +++++++++----------
>  .../staging/kpc2000/kpc_dma/kpc_dma_driver.c  |  3 +--
>  3 files changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/staging/kpc2000/kpc_dma/dma.c b/drivers/staging/kpc2000/kpc_dma/dma.c
> index 4bfbc717bc07..5ccc93ed2b61 100644
> --- a/drivers/staging/kpc2000/kpc_dma/dma.c
> +++ b/drivers/staging/kpc2000/kpc_dma/dma.c
> @@ -97,11 +97,10 @@ int  setup_dma_engine(struct kpc_dma_device *eng, u32 desc_cnt)
>  	if (WARN(!(caps & ENG_CAP_PRESENT), "%s() called for DMA Engine at %p which isn't present in hardware!\n", __func__, eng))
>  		return -ENXIO;
>
> -	if (caps & ENG_CAP_DIRECTION) {
> +	if (caps & ENG_CAP_DIRECTION)
>  		eng->dir = DMA_FROM_DEVICE;
> -	} else {
> +	else
>  		eng->dir = DMA_TO_DEVICE;
> -	}
>
>  	eng->desc_pool_cnt = desc_cnt;
>  	eng->desc_pool = dma_pool_create("KPC DMA Descriptors", &eng->pldev->dev, sizeof(struct kpc_dma_descriptor), DMA_DESC_ALIGNMENT, 4096);
> diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> index 21ebab7dbc7b..d93097ed781c 100644
> --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> @@ -126,11 +126,11 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
>  			// Fill out the descriptor
>  			BUG_ON(!desc);
>  			clear_desc(desc);
> -			if (p != pcnt-1) {
> +			if (p != pcnt-1)
>  				desc->DescByteCount = 0x80000;
> -			} else {
> +			else
>  				desc->DescByteCount = sg_dma_len(sg) - (p * 0x80000);
> -			}
> +
>  			desc->DescBufferByteCount = desc->DescByteCount;
>
>  			desc->DescControlFlags |= DMA_DESC_CTL_IRQONERR;
> @@ -148,9 +148,9 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
>  			desc->DescSystemAddrMS = (dma_addr & 0xFFFFFFFF00000000UL) >> 32;
>
>  			user_ctl = acd->priv->user_ctl;
> -			if (i == acd->mapped_entry_count-1 && p == pcnt-1) {
> +			if (i == acd->mapped_entry_count-1 && p == pcnt-1)
>  				user_ctl = acd->priv->user_ctl_last;
> -			}
> +
>  			desc->DescUserControlLS = (user_ctl & 0x00000000FFFFFFFFUL) >>  0;
>  			desc->DescUserControlMS = (user_ctl & 0xFFFFFFFF00000000UL) >> 32;
>
> @@ -188,9 +188,9 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
>  	sg_free_table(&acd->sgt);
>   err_dma_map_sg:
>   err_alloc_sg_table:
> -	for (i = 0 ; i < acd->page_count ; i++) {
> +	for (i = 0 ; i < acd->page_count ; i++)
>  		put_page(acd->user_pages[i]);
> -	}
> +
>   err_get_user_pages:
>  	kfree(acd->user_pages);
>   err_alloc_userpages:
> @@ -210,16 +210,14 @@ void  transfer_complete_cb(struct aio_cb_data *acd, size_t xfr_count, u32 flags)
>  	BUG_ON(!acd->ldev->pldev);
>
>  	for (i = 0 ; i < acd->page_count ; i++) {
> -		if (!PageReserved(acd->user_pages[i])) {
> +		if (!PageReserved(acd->user_pages[i]))
>  			set_page_dirty(acd->user_pages[i]);
> -		}
>  	}
>
>  	dma_unmap_sg(&acd->ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, acd->ldev->dir);
>
> -	for (i = 0 ; i < acd->page_count ; i++) {
> +	for (i = 0 ; i < acd->page_count ; i++)
>  		put_page(acd->user_pages[i]);
> -	}
>
>  	sg_free_table(&acd->sgt);
>
> diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> index 25ea16a1981e..bc81afb6ade2 100644
> --- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> +++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> @@ -26,9 +26,8 @@ struct kpc_dma_device *kpc_dma_lookup_device(int minor)
>
>  	mutex_lock(&kpc_dma_mtx);
>  	list_for_each_entry(c, &kpc_dma_list, list) {
> -		if (c->pldev->id == minor) {
> +		if (c->pldev->id == minor)
>  			goto out;
> -		}
>  	}
>  	c = NULL; // not-found case
>  out:
> ---
> 2.20.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/0a3ec63321dce008fc8dd790f42ef8490135b307.1585082158.git.jane.pnx9%40gmail.com.
>

Re: [Outreachy kernel] [PATCH 2/6] Staging: kpc2000: kpc_dma: Use sizeof(*var) in kzalloc().

[Julia Lawall]

On Tue, 24 Mar 2020, Sam Muhammed wrote:

> Modify struct allocations in kzalloc() to match the
> coding standards.

The subject line is more informative than the log message...  Modify
doesn't say anything about what you did.

julia

>
> Checkpatch.pl CHECK: Prefer kzalloc(sizeof(*var)...)
> over kzalloc(sizeof(struct var)...)
>
> Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> ---
>  drivers/staging/kpc2000/kpc_dma/fileops.c        | 2 +-
>  drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> index 5eb6c5f24feb..21ebab7dbc7b 100644
> --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> @@ -253,7 +253,7 @@ int  kpc_dma_open(struct inode *inode, struct file *filp)
>  		return -EBUSY; /* already open */
>  	}
>
> -	priv = kzalloc(sizeof(struct dev_private_data), GFP_KERNEL);
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
>  		return -ENOMEM;
>
> diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> index ec79a8500caf..25ea16a1981e 100644
> --- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> +++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> @@ -98,7 +98,7 @@ int  kpc_dma_probe(struct platform_device *pldev)
>  	int rv = 0;
>  	dev_t dev;
>
> -	struct kpc_dma_device *ldev = kzalloc(sizeof(struct kpc_dma_device), GFP_KERNEL);
> +	struct kpc_dma_device *ldev = kzalloc(sizeof(*ldev), GFP_KERNEL);
>
>  	if (!ldev) {
>  		dev_err(&pldev->dev, "%s: unable to kzalloc space for kpc_dma_device\n", __func__);
> ---
> 2.20.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/bbb3adbd20ae89db6a0d3360bc09d22eed778e86.1585082158.git.jane.pnx9%40gmail.com.
>

Re: [Outreachy kernel] [PATCH 2/6] Staging: kpc2000: kpc_dma: Use sizeof(*var) in kzalloc().

[Julia Lawall]

On Tue, 24 Mar 2020, Sam Muhammed wrote:

> Modify struct allocations in kzalloc() to match the
> coding standards.
>
> Checkpatch.pl CHECK: Prefer kzalloc(sizeof(*var)...)
> over kzalloc(sizeof(struct var)...)

You could consider why the former is better than the latter (although not
everyone agrees that this is the case).

julia

>
> Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> ---
>  drivers/staging/kpc2000/kpc_dma/fileops.c        | 2 +-
>  drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> index 5eb6c5f24feb..21ebab7dbc7b 100644
> --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> @@ -253,7 +253,7 @@ int  kpc_dma_open(struct inode *inode, struct file *filp)
>  		return -EBUSY; /* already open */
>  	}
>
> -	priv = kzalloc(sizeof(struct dev_private_data), GFP_KERNEL);
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
>  		return -ENOMEM;
>
> diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> index ec79a8500caf..25ea16a1981e 100644
> --- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> +++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> @@ -98,7 +98,7 @@ int  kpc_dma_probe(struct platform_device *pldev)
>  	int rv = 0;
>  	dev_t dev;
>
> -	struct kpc_dma_device *ldev = kzalloc(sizeof(struct kpc_dma_device), GFP_KERNEL);
> +	struct kpc_dma_device *ldev = kzalloc(sizeof(*ldev), GFP_KERNEL);
>
>  	if (!ldev) {
>  		dev_err(&pldev->dev, "%s: unable to kzalloc space for kpc_dma_device\n", __func__);
> ---
> 2.20.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/bbb3adbd20ae89db6a0d3360bc09d22eed778e86.1585082158.git.jane.pnx9%40gmail.com.
>

Re: [Outreachy kernel] [PATCH 4/6] Staging: kpc2000: kpc_dma: Use spaces around operators.

[Sam Muhammed]

On Tue, 2020-03-24 at 22:22 +0100, Julia Lawall wrote:
> 
> On Tue, 24 Mar 2020, Sam Muhammed wrote:
> 
> > Cleanup Checkpatch.pl CHECKs about missing
> > spaces around multiple operators.
> >
> > Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> > ---
> >  drivers/staging/kpc2000/kpc_dma/fileops.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > index d93097ed781c..e599ae1bdfd0 100644
> > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > @@ -19,7 +19,7 @@ static inline
> >  unsigned int  count_pages(unsigned long iov_base, size_t iov_len)
> >  {
> >  	unsigned long first = (iov_base             & PAGE_MASK) >> PAGE_SHIFT;
> > -	unsigned long last  = ((iov_base+iov_len-1) & PAGE_MASK) >> PAGE_SHIFT;
> > +	unsigned long last  = ((iov_base + iov_len - 1) & PAGE_MASK) >> PAGE_SHIFT;
> 
> It seems that the & operators were lined up before and that is not the
> case any more.
> 

Yes, why they were aligned anyways? 
should i align them back or remove the space in the first statement?

Sam

> julia
> 
> >
> >  	return last - first + 1;
> >  }
> > @@ -83,7 +83,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
> >  	}
> >
> >  	// Allocate and setup the sg_table (scatterlist entries)
> > -	rv = sg_alloc_table_from_pages(&acd->sgt, acd->user_pages, acd->page_count, iov_base & (PAGE_SIZE-1), iov_len, GFP_KERNEL);
> > +	rv = sg_alloc_table_from_pages(&acd->sgt, acd->user_pages, acd->page_count, iov_base & (PAGE_SIZE - 1), iov_len, GFP_KERNEL);
> >  	if (rv) {
> >  		dev_err(&priv->ldev->pldev->dev, "Couldn't alloc sg_table (%ld)\n", rv);
> >  		goto err_alloc_sg_table;
> > @@ -126,7 +126,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
> >  			// Fill out the descriptor
> >  			BUG_ON(!desc);
> >  			clear_desc(desc);
> > -			if (p != pcnt-1)
> > +			if (p != pcnt - 1)
> >  				desc->DescByteCount = 0x80000;
> >  			else
> >  				desc->DescByteCount = sg_dma_len(sg) - (p * 0x80000);
> > @@ -136,7 +136,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
> >  			desc->DescControlFlags |= DMA_DESC_CTL_IRQONERR;
> >  			if (i == 0 && p == 0)
> >  				desc->DescControlFlags |= DMA_DESC_CTL_SOP;
> > -			if (i == acd->mapped_entry_count-1 && p == pcnt-1)
> > +			if (i == acd->mapped_entry_count - 1 && p == pcnt - 1)
> >  				desc->DescControlFlags |= DMA_DESC_CTL_EOP | DMA_DESC_CTL_IRQONDONE;
> >
> >  			desc->DescCardAddrLS = (card_addr & 0xFFFFFFFF);
> > @@ -148,13 +148,13 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
> >  			desc->DescSystemAddrMS = (dma_addr & 0xFFFFFFFF00000000UL) >> 32;
> >
> >  			user_ctl = acd->priv->user_ctl;
> > -			if (i == acd->mapped_entry_count-1 && p == pcnt-1)
> > +			if (i == acd->mapped_entry_count - 1 && p == pcnt - 1)
> >  				user_ctl = acd->priv->user_ctl_last;
> >
> >  			desc->DescUserControlLS = (user_ctl & 0x00000000FFFFFFFFUL) >>  0;
> >  			desc->DescUserControlMS = (user_ctl & 0xFFFFFFFF00000000UL) >> 32;
> >
> > -			if (i == acd->mapped_entry_count-1 && p == pcnt-1)
> > +			if (i == acd->mapped_entry_count - 1 && p == pcnt - 1)
> >  				desc->acd = acd;
> >
> >  			dev_dbg(&priv->ldev->pldev->dev, "  Filled descriptor %p (acd = %p)\n", desc, desc->acd);
> > ---
> > 2.20.1
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/d881bf9d5c442ae5a0d6669624355f7af73f5fac.1585082158.git.jane.pnx9%40gmail.com.
> >

Re: [Outreachy kernel] [PATCH 2/6] Staging: kpc2000: kpc_dma: Use sizeof(*var) in kzalloc().

[Sam Muhammed]

On Tue, 2020-03-24 at 22:24 +0100, Julia Lawall wrote:
> 
> On Tue, 24 Mar 2020, Sam Muhammed wrote:
> 
> > Modify struct allocations in kzalloc() to match the
> > coding standards.
> >
> > Checkpatch.pl CHECK: Prefer kzalloc(sizeof(*var)...)
> > over kzalloc(sizeof(struct var)...)
> 
> You could consider why the former is better than the latter (although not
> everyone agrees that this is the case).
> 

That's right, this log message is poor.
i'll make another revision with these changes but i want to add another
small patch to the series, so this makes it a new series? or adding in
the log message that a new patch is added will be enough?

Thank You
Sam

> julia
> 
> >
> > Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> > ---
> >  drivers/staging/kpc2000/kpc_dma/fileops.c        | 2 +-
> >  drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > index 5eb6c5f24feb..21ebab7dbc7b 100644
> > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > @@ -253,7 +253,7 @@ int  kpc_dma_open(struct inode *inode, struct file *filp)
> >  		return -EBUSY; /* already open */
> >  	}
> >
> > -	priv = kzalloc(sizeof(struct dev_private_data), GFP_KERNEL);
> > +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> >  	if (!priv)
> >  		return -ENOMEM;
> >
> > diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> > index ec79a8500caf..25ea16a1981e 100644
> > --- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> > +++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> > @@ -98,7 +98,7 @@ int  kpc_dma_probe(struct platform_device *pldev)
> >  	int rv = 0;
> >  	dev_t dev;
> >
> > -	struct kpc_dma_device *ldev = kzalloc(sizeof(struct kpc_dma_device), GFP_KERNEL);
> > +	struct kpc_dma_device *ldev = kzalloc(sizeof(*ldev), GFP_KERNEL);
> >
> >  	if (!ldev) {
> >  		dev_err(&pldev->dev, "%s: unable to kzalloc space for kpc_dma_device\n", __func__);
> > ---
> > 2.20.1
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/bbb3adbd20ae89db6a0d3360bc09d22eed778e86.1585082158.git.jane.pnx9%40gmail.com.
> >

Re: [Outreachy kernel] [PATCH 4/6] Staging: kpc2000: kpc_dma: Use spaces around operators.

[Greg Kroah-Hartman]

On Wed, Mar 25, 2020 at 04:00:36AM -0400, Sam Muhammed wrote:
> On Tue, 2020-03-24 at 22:22 +0100, Julia Lawall wrote:
> > 
> > On Tue, 24 Mar 2020, Sam Muhammed wrote:
> > 
> > > Cleanup Checkpatch.pl CHECKs about missing
> > > spaces around multiple operators.
> > >
> > > Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> > > ---
> > >  drivers/staging/kpc2000/kpc_dma/fileops.c | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > index d93097ed781c..e599ae1bdfd0 100644
> > > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > @@ -19,7 +19,7 @@ static inline
> > >  unsigned int  count_pages(unsigned long iov_base, size_t iov_len)
> > >  {
> > >  	unsigned long first = (iov_base             & PAGE_MASK) >> PAGE_SHIFT;
> > > -	unsigned long last  = ((iov_base+iov_len-1) & PAGE_MASK) >> PAGE_SHIFT;
> > > +	unsigned long last  = ((iov_base + iov_len - 1) & PAGE_MASK) >> PAGE_SHIFT;
> > 
> > It seems that the & operators were lined up before and that is not the
> > case any more.
> > 
> 
> Yes, why they were aligned anyways? 

Because it is easier to understand the code that way, don't you think?

Re: [Outreachy kernel] [PATCH 4/6] Staging: kpc2000: kpc_dma: Use spaces around operators.

[Sam Muhammed]

On Wed, 2020-03-25 at 13:36 +0100, Greg Kroah-Hartman wrote:
> On Wed, Mar 25, 2020 at 04:00:36AM -0400, Sam Muhammed wrote:
> > On Tue, 2020-03-24 at 22:22 +0100, Julia Lawall wrote:
> > > 
> > > On Tue, 24 Mar 2020, Sam Muhammed wrote:
> > > 
> > > > Cleanup Checkpatch.pl CHECKs about missing
> > > > spaces around multiple operators.
> > > >
> > > > Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> > > > ---
> > > >  drivers/staging/kpc2000/kpc_dma/fileops.c | 12 ++++++------
> > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > index d93097ed781c..e599ae1bdfd0 100644
> > > > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > @@ -19,7 +19,7 @@ static inline
> > > >  unsigned int  count_pages(unsigned long iov_base, size_t iov_len)
> > > >  {
> > > >  	unsigned long first = (iov_base             & PAGE_MASK) >> PAGE_SHIFT;
> > > > -	unsigned long last  = ((iov_base+iov_len-1) & PAGE_MASK) >> PAGE_SHIFT;
> > > > +	unsigned long last  = ((iov_base + iov_len - 1) & PAGE_MASK) >> PAGE_SHIFT;
> > > 
> > > It seems that the & operators were lined up before and that is not the
> > > case any more.
> > > 
> > 
> > Yes, why they were aligned anyways? 
> 
> Because it is easier to understand the code that way, don't you think?
> 
My bad, iam not used to see the patterns yes, so aligning the first
statement to the second would be a different change? or since a change
is made anyways they both would go together? 

Re: [Outreachy kernel] [PATCH 2/6] Staging: kpc2000: kpc_dma: Use sizeof(*var) in kzalloc().

[Julia Lawall]

On Wed, 25 Mar 2020, Sam Muhammed wrote:

> On Tue, 2020-03-24 at 22:24 +0100, Julia Lawall wrote:
> >
> > On Tue, 24 Mar 2020, Sam Muhammed wrote:
> >
> > > Modify struct allocations in kzalloc() to match the
> > > coding standards.
> > >
> > > Checkpatch.pl CHECK: Prefer kzalloc(sizeof(*var)...)
> > > over kzalloc(sizeof(struct var)...)
> >
> > You could consider why the former is better than the latter (although not
> > everyone agrees that this is the case).
> >
>
> That's right, this log message is poor.
> i'll make another revision with these changes but i want to add another
> small patch to the series, so this makes it a new series? or adding in
> the log message that a new patch is added will be enough?

Should be enough.

julia

>
> Thank You
> Sam
>
> > julia
> >
> > >
> > > Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> > > ---
> > >  drivers/staging/kpc2000/kpc_dma/fileops.c        | 2 +-
> > >  drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c | 2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > index 5eb6c5f24feb..21ebab7dbc7b 100644
> > > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > @@ -253,7 +253,7 @@ int  kpc_dma_open(struct inode *inode, struct file *filp)
> > >  		return -EBUSY; /* already open */
> > >  	}
> > >
> > > -	priv = kzalloc(sizeof(struct dev_private_data), GFP_KERNEL);
> > > +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > >  	if (!priv)
> > >  		return -ENOMEM;
> > >
> > > diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> > > index ec79a8500caf..25ea16a1981e 100644
> > > --- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> > > +++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> > > @@ -98,7 +98,7 @@ int  kpc_dma_probe(struct platform_device *pldev)
> > >  	int rv = 0;
> > >  	dev_t dev;
> > >
> > > -	struct kpc_dma_device *ldev = kzalloc(sizeof(struct kpc_dma_device), GFP_KERNEL);
> > > +	struct kpc_dma_device *ldev = kzalloc(sizeof(*ldev), GFP_KERNEL);
> > >
> > >  	if (!ldev) {
> > >  		dev_err(&pldev->dev, "%s: unable to kzalloc space for kpc_dma_device\n", __func__);
> > > ---
> > > 2.20.1
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/bbb3adbd20ae89db6a0d3360bc09d22eed778e86.1585082158.git.jane.pnx9%40gmail.com.
> > >
>
>
>

Re: [Outreachy kernel] [PATCH 4/6] Staging: kpc2000: kpc_dma: Use spaces around operators.

[Stefano Brivio]

On Wed, 25 Mar 2020 08:57:30 -0400
Sam Muhammed <jane.pnx9@gmail.com> wrote:

> On Wed, 2020-03-25 at 13:36 +0100, Greg Kroah-Hartman wrote:
> > On Wed, Mar 25, 2020 at 04:00:36AM -0400, Sam Muhammed wrote:  
> > > On Tue, 2020-03-24 at 22:22 +0100, Julia Lawall wrote:  
> > > > 
> > > > On Tue, 24 Mar 2020, Sam Muhammed wrote:
> > > >   
> > > > > Cleanup Checkpatch.pl CHECKs about missing
> > > > > spaces around multiple operators.
> > > > >
> > > > > Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> > > > > ---
> > > > >  drivers/staging/kpc2000/kpc_dma/fileops.c | 12 ++++++------
> > > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > > index d93097ed781c..e599ae1bdfd0 100644
> > > > > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > > @@ -19,7 +19,7 @@ static inline
> > > > >  unsigned int  count_pages(unsigned long iov_base, size_t iov_len)
> > > > >  {
> > > > >  	unsigned long first = (iov_base             & PAGE_MASK) >> PAGE_SHIFT;
> > > > > -	unsigned long last  = ((iov_base+iov_len-1) & PAGE_MASK) >> PAGE_SHIFT;
> > > > > +	unsigned long last  = ((iov_base + iov_len - 1) & PAGE_MASK) >> PAGE_SHIFT;  
> > > > 
> > > > It seems that the & operators were lined up before and that is not the
> > > > case any more.
> > > >   
> > > 
> > > Yes, why they were aligned anyways?   
> > 
> > Because it is easier to understand the code that way, don't you think?
> >   
> My bad, iam not used to see the patterns yes, so aligning the first
> statement to the second would be a different change? or since a change
> is made anyways they both would go together? 

I think yes, this should be a single patch, because:

1. realigning the first line is needed only as you edit the second one,

2. with two patches the alignment would be "broken" after the first one
   (this is especially a hard rule with build failures and warnings, but
   the concept usually applies to these cases as well)

...however, why would you ask this question and anyway re-post the
series 89 minutes later? Mind that this is a good way to get your
patches ignored by some maintainers.

-- 
Stefano

Re: [Outreachy kernel] [PATCH 4/6] Staging: kpc2000: kpc_dma: Use spaces around operators.

[Sam Muhammed]

On Mon, 2020-03-30 at 03:02 +0200, Stefano Brivio wrote:
> On Wed, 25 Mar 2020 08:57:30 -0400
> Sam Muhammed <jane.pnx9@gmail.com> wrote:
> 
> > On Wed, 2020-03-25 at 13:36 +0100, Greg Kroah-Hartman wrote:
> > > On Wed, Mar 25, 2020 at 04:00:36AM -0400, Sam Muhammed wrote:  
> > > > On Tue, 2020-03-24 at 22:22 +0100, Julia Lawall wrote:  
> > > > > 
> > > > > On Tue, 24 Mar 2020, Sam Muhammed wrote:
> > > > >   
> > > > > > Cleanup Checkpatch.pl CHECKs about missing
> > > > > > spaces around multiple operators.
> > > > > >
> > > > > > Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> > > > > > ---
> > > > > >  drivers/staging/kpc2000/kpc_dma/fileops.c | 12 ++++++------
> > > > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > > > index d93097ed781c..e599ae1bdfd0 100644
> > > > > > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > > > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > > > @@ -19,7 +19,7 @@ static inline
> > > > > >  unsigned int  count_pages(unsigned long iov_base, size_t iov_len)
> > > > > >  {
> > > > > >  	unsigned long first = (iov_base             & PAGE_MASK) >> PAGE_SHIFT;
> > > > > > -	unsigned long last  = ((iov_base+iov_len-1) & PAGE_MASK) >> PAGE_SHIFT;
> > > > > > +	unsigned long last  = ((iov_base + iov_len - 1) & PAGE_MASK) >> PAGE_SHIFT;  
> > > > > 
> > > > > It seems that the & operators were lined up before and that is not the
> > > > > case any more.
> > > > >   
> > > > 
> > > > Yes, why they were aligned anyways?   
> > > 
> > > Because it is easier to understand the code that way, don't you think?
> > >   
> > My bad, iam not used to see the patterns yes, so aligning the first
> > statement to the second would be a different change? or since a change
> > is made anyways they both would go together? 
> 
> I think yes, this should be a single patch, because:
> 
> 1. realigning the first line is needed only as you edit the second one,
> 
> 2. with two patches the alignment would be "broken" after the first one
>    (this is especially a hard rule with build failures and warnings, but
>    the concept usually applies to these cases as well)
> 
> ...however, why would you ask this question and anyway re-post the
> series 89 minutes later? Mind that this is a good way to get your
> patches ignored by some maintainers.
> 

But reading through Julia's comment about it and re-considering the
change, my question was already answered with Julia's comment as of 
_if i make a change, i should keep the style and not break the
alignment_

So i probably shouldn't ask the question from the first place and read
Julia's comment more carefully. :)

Thank You.

Sam

Re: [Outreachy kernel] [PATCH 4/6] Staging: kpc2000: kpc_dma: Use spaces around operators.

[Stefano Brivio]

On Mon, 30 Mar 2020 07:55:45 -0400
Sam Muhammed <jane.pnx9@gmail.com> wrote:

> On Mon, 2020-03-30 at 03:02 +0200, Stefano Brivio wrote:
> > On Wed, 25 Mar 2020 08:57:30 -0400
> > Sam Muhammed <jane.pnx9@gmail.com> wrote:
> >   
> > > On Wed, 2020-03-25 at 13:36 +0100, Greg Kroah-Hartman wrote:  
> > > > On Wed, Mar 25, 2020 at 04:00:36AM -0400, Sam Muhammed wrote:    
> > > > > On Tue, 2020-03-24 at 22:22 +0100, Julia Lawall wrote:    
> > > > > > 
> > > > > > On Tue, 24 Mar 2020, Sam Muhammed wrote:
> > > > > >     
> > > > > > > Cleanup Checkpatch.pl CHECKs about missing
> > > > > > > spaces around multiple operators.
> > > > > > >
> > > > > > > Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> > > > > > > ---
> > > > > > >  drivers/staging/kpc2000/kpc_dma/fileops.c | 12 ++++++------
> > > > > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > > > > index d93097ed781c..e599ae1bdfd0 100644
> > > > > > > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > > > > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > > > > @@ -19,7 +19,7 @@ static inline
> > > > > > >  unsigned int  count_pages(unsigned long iov_base, size_t iov_len)
> > > > > > >  {
> > > > > > >  	unsigned long first = (iov_base             & PAGE_MASK) >> PAGE_SHIFT;
> > > > > > > -	unsigned long last  = ((iov_base+iov_len-1) & PAGE_MASK) >> PAGE_SHIFT;
> > > > > > > +	unsigned long last  = ((iov_base + iov_len - 1) & PAGE_MASK) >> PAGE_SHIFT;    
> > > > > > 
> > > > > > It seems that the & operators were lined up before and that is not the
> > > > > > case any more.
> > > > > >     
> > > > > 
> > > > > Yes, why they were aligned anyways?     
> > > > 
> > > > Because it is easier to understand the code that way, don't you think?
> > > >     
> > > My bad, iam not used to see the patterns yes, so aligning the first
> > > statement to the second would be a different change? or since a change
> > > is made anyways they both would go together?   
> > 
> > I think yes, this should be a single patch, because:
> > 
> > 1. realigning the first line is needed only as you edit the second one,
> > 
> > 2. with two patches the alignment would be "broken" after the first one
> >    (this is especially a hard rule with build failures and warnings, but
> >    the concept usually applies to these cases as well)
> > 
> > ...however, why would you ask this question and anyway re-post the
> > series 89 minutes later? Mind that this is a good way to get your
> > patches ignored by some maintainers.
> >   
> 
> But reading through Julia's comment about it and re-considering the
> change, my question was already answered with Julia's comment as of 
> _if i make a change, i should keep the style and not break the
> alignment_
> 
> So i probably shouldn't ask the question from the first place and read
> Julia's comment more carefully. :)

Fine, I really don't mind that part... but it would be nice if you
could answer your question yourself on the list, so that:

1. other applicants benefit from it

2. nobody has to wonder if you're still waiting for an answer (my case
   here)

-- 
Stefano