[PATCH 0/5] tpm: tpm_ibmvtpm: - style fix

STALE3274d

12 messages, 6 authors, 2017-08-19 · open the first message on its own page

[PATCH 0/5] tpm: tpm_ibmvtpm: - style fix

From: SZ Lin <hidden>
Date: 2017-07-29 07:25:08

Fix styling WARNINGs and Errors of tpm_ibmvtpm.c driver by using checkpatch.pl

SZ Lin (5):
  Fix packed and aligned attribute warnings.
  Fix "ERROR: code indent should use tabs where possible"
  Fix 'void function return statements are not generally useful' warning
  Remove unneccessary 'out of memory' message
  Use __func__ instead of function name

 drivers/char/tpm/tpm_ibmvtpm.c | 23 +++++++++--------------
 drivers/char/tpm/tpm_ibmvtpm.h |  2 +-
 2 files changed, 10 insertions(+), 15 deletions(-)

--
2.13.3

[PATCH 1/5] Fix packed and aligned attribute warnings.

From: SZ Lin <hidden>
Date: 2017-07-29 07:25:10

WARNING: __packed is preferred over __attribute__((packed))
+} __attribute__((packed, aligned(8)));

WARNING: __aligned(size) is preferred over __attribute__((aligned(size)))
+} __attribute__((packed, aligned(8)));

Signed-off-by: SZ Lin <redacted>
---
 drivers/char/tpm/tpm_ibmvtpm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
index 91dfe766d080..9f708ca3dc84 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.h
+++ b/drivers/char/tpm/tpm_ibmvtpm.h

@@ -25,7 +25,7 @@ struct ibmvtpm_crq {
 	__be16 len;
 	__be32 data;
 	__be64 reserved;
-} __attribute__((packed, aligned(8)));
+} __packed __aligned(8);
 
 struct ibmvtpm_crq_queue {
 	struct ibmvtpm_crq *crq_addr;

-- 
2.13.3

[PATCH 3/5] Fix 'void function return statements are not generally useful' warning

From: SZ Lin <hidden>
Date: 2017-07-29 07:25:12

WARNING: void function return statements are not generally useful
+       return;
+}

Signed-off-by: SZ Lin <redacted>
---
 drivers/char/tpm/tpm_ibmvtpm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 23913fc86158..e53b9fb517d9 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c

@@ -531,7 +531,6 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq,
 			return;
 		}
 	}
-	return;
 }
 
 /**

-- 
2.13.3

[PATCH 2/5] Fix "ERROR: code indent should use tabs where possible"

From: SZ Lin <hidden>
Date: 2017-07-29 07:25:15

ERROR: code indent should use tabs where possible
+^I^I         "Need to wait for TPM to finish\n");$

Signed-off-by: SZ Lin <redacted>
---
 drivers/char/tpm/tpm_ibmvtpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index f01d083eced2..23913fc86158 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c

@@ -127,7 +127,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
 
 	if (ibmvtpm->tpm_processing_cmd) {
 		dev_info(ibmvtpm->dev,
-		         "Need to wait for TPM to finish\n");
+			"Need to wait for TPM to finish\n");
 		/* wait for previous command to finish */
 		sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
 		if (sig)

-- 
2.13.3

[PATCH 5/5] Use func instead of function name

From: SZ Lin <hidden>
Date: 2017-07-29 07:25:35

Fix following checkpatch.pl warning:
WARNING: Prefer using '"%s...", __func__' to using
the function's name, in a string

Signed-off-by: SZ Lin <redacted>
---
 drivers/char/tpm/tpm_ibmvtpm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index e75a674b44ac..2d33acc43e25 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c

@@ -151,7 +151,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
 	rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]),
 			      be64_to_cpu(word[1]));
 	if (rc != H_SUCCESS) {
-		dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
+		dev_err(ibmvtpm->dev, "%s failed rc=%d\n", __func__, rc);
 		rc = 0;
 		ibmvtpm->tpm_processing_cmd = false;
 	} else

@@ -193,7 +193,7 @@ static int ibmvtpm_crq_get_rtce_size(struct ibmvtpm_dev *ibmvtpm)
 			      cpu_to_be64(buf[1]));
 	if (rc != H_SUCCESS)
 		dev_err(ibmvtpm->dev,
-			"ibmvtpm_crq_get_rtce_size failed rc=%d\n", rc);
+			"%s failed rc=%d\n", __func__, rc);
 
 	return rc;
 }

@@ -221,7 +221,7 @@ static int ibmvtpm_crq_get_version(struct ibmvtpm_dev *ibmvtpm)
 			      cpu_to_be64(buf[1]));
 	if (rc != H_SUCCESS)
 		dev_err(ibmvtpm->dev,
-			"ibmvtpm_crq_get_version failed rc=%d\n", rc);
+			"%s failed rc=%d\n", __func__, rc);
 
 	return rc;
 }

@@ -241,7 +241,7 @@ static int ibmvtpm_crq_send_init_complete(struct ibmvtpm_dev *ibmvtpm)
 	rc = ibmvtpm_send_crq(ibmvtpm->vdev, INIT_CRQ_COMP_CMD, 0);
 	if (rc != H_SUCCESS)
 		dev_err(ibmvtpm->dev,
-			"ibmvtpm_crq_send_init_complete failed rc=%d\n", rc);
+			"%s rc=%d\n", __func__, rc);
 
 	return rc;
 }

@@ -261,7 +261,7 @@ static int ibmvtpm_crq_send_init(struct ibmvtpm_dev *ibmvtpm)
 	rc = ibmvtpm_send_crq(ibmvtpm->vdev, INIT_CRQ_CMD, 0);
 	if (rc != H_SUCCESS)
 		dev_err(ibmvtpm->dev,
-			"ibmvtpm_crq_send_init failed rc=%d\n", rc);
+			"%s failed rc=%d\n", __func__, rc);
 
 	return rc;
 }

@@ -351,7 +351,7 @@ static int tpm_ibmvtpm_suspend(struct device *dev)
 			      cpu_to_be64(buf[1]));
 	if (rc != H_SUCCESS)
 		dev_err(ibmvtpm->dev,
-			"tpm_ibmvtpm_suspend failed rc=%d\n", rc);
+			"%s failed rc=%d\n", __func__, rc);
 
 	return rc;
 }

-- 
2.13.3

[PATCH 4/5] Remove unneccessary 'out of memory' message

From: SZ Lin <hidden>
Date: 2017-07-29 07:25:51

WARNING: Possible unnecessary 'out of memory' message
+                       if (!ibmvtpm->rtce_buf) {
+                               dev_err(ibmvtpm->dev, "Failed to allocate memory for rtce buffer\n");

WARNING: Possible unnecessary 'out of memory' message
+       if (!ibmvtpm) {
+               dev_err(dev, "kzalloc for ibmvtpm failed\n");

Signed-off-by: SZ Lin <redacted>
---
 drivers/char/tpm/tpm_ibmvtpm.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index e53b9fb517d9..e75a674b44ac 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c

@@ -501,10 +501,8 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq,
 			ibmvtpm->rtce_size = be16_to_cpu(crq->len);
 			ibmvtpm->rtce_buf = kmalloc(ibmvtpm->rtce_size,
 						    GFP_ATOMIC);
-			if (!ibmvtpm->rtce_buf) {
-				dev_err(ibmvtpm->dev, "Failed to allocate memory for rtce buffer\n");
+			if (!ibmvtpm->rtce_buf)
 				return;
-			}
 
 			ibmvtpm->rtce_dma_handle = dma_map_single(ibmvtpm->dev,
 				ibmvtpm->rtce_buf, ibmvtpm->rtce_size,

@@ -584,10 +582,8 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
 		return PTR_ERR(chip);
 
 	ibmvtpm = kzalloc(sizeof(struct ibmvtpm_dev), GFP_KERNEL);
-	if (!ibmvtpm) {
-		dev_err(dev, "kzalloc for ibmvtpm failed\n");
+	if (!ibmvtpm)
 		goto cleanup;
-	}
 
 	ibmvtpm->dev = dev;
 	ibmvtpm->vdev = vio_dev;

-- 
2.13.3

Re: [PATCH 2/5] Fix "ERROR: code indent should use tabs where possible"

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2017-07-31 10:25:06

SZ Lin [off-list ref] writes:

quoted hunk

ERROR: code indent should use tabs where possible
+^I^I         "Need to wait for TPM to finish\n");$

Signed-off-by: SZ Lin <redacted>
---
 drivers/char/tpm/tpm_ibmvtpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index f01d083eced2..23913fc86158 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c

@@ -127,7 +127,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
 
 	if (ibmvtpm->tpm_processing_cmd) {
 		dev_info(ibmvtpm->dev,
-		         "Need to wait for TPM to finish\n");
+			"Need to wait for TPM to finish\n");

There's no reason for that to be on a separate line at all. Just make it
a single line dev_info( ... );

cheers

RE: [PATCH 1/5] Fix packed and aligned attribute warnings.

From: David Laight <hidden>
Date: 2017-07-31 13:27:55

From: SZ Lin

Sent: 29 July 2017 08:24

...

quoted hunk

diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtp=

m.h

quoted hunk

index 91dfe766d080..9f708ca3dc84 100644

--- a/drivers/char/tpm/tpm_ibmvtpm.h
+++ b/drivers/char/tpm/tpm_ibmvtpm.h

@@ -25,7 +25,7 @@ struct ibmvtpm_crq {
 	__be16 len;
 	__be32 data;
 	__be64 reserved;
-} __attribute__((packed, aligned(8)));
+} __packed __aligned(8);

You can't need __packed and __aligned(8) on that structure.
There are no gaps and you are saying it is always aligned.

So just remove the pointless attributes.

	David

Re: [PATCH 0/5] tpm: tpm_ibmvtpm: - style fix

From: Jarkko Sakkinen <hidden>
Date: 2017-08-02 12:36:25

On Sat, Jul 29, 2017 at 03:24:28PM +0800, SZ Lin wrote:

Fix styling WARNINGs and Errors of tpm_ibmvtpm.c driver by using checkpatch.pl

Changes are great but you should revise the patch series so that you
expain in each commit what goes wrong instead of copy paste of the
checkpatch output and why your changes fixes the problem.

SZ Lin (5):
  Fix packed and aligned attribute warnings.
  Fix "ERROR: code indent should use tabs where possible"
  Fix 'void function return statements are not generally useful' warning
  Remove unneccessary 'out of memory' message
  Use __func__ instead of function name

 drivers/char/tpm/tpm_ibmvtpm.c | 23 +++++++++--------------
 drivers/char/tpm/tpm_ibmvtpm.h |  2 +-
 2 files changed, 10 insertions(+), 15 deletions(-)

--
2.13.3

/Jarkko

Re: [PATCH 5/5] Use func instead of function name

From: Michal Suchánek <hidden>
Date: 2017-08-18 08:03:32

On 2017-07-29 09:24, SZ Lin wrote:

quoted hunk

Fix following checkpatch.pl warning:
WARNING: Prefer using '"%s...", __func__' to using
the function's name, in a string

Signed-off-by: SZ Lin <redacted>
---
 drivers/char/tpm/tpm_ibmvtpm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c

b/drivers/char/tpm/tpm_ibmvtpm.c
index e75a674b44ac..2d33acc43e25 100644

--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c

@@ -151,7 +151,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip,

u8 *buf, size_t count)
 	rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]),
 			      be64_to_cpu(word[1]));
 	if (rc != H_SUCCESS) {
-		dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
+		dev_err(ibmvtpm->dev, "%s failed rc=%d\n", __func__, rc);

Can function name contain a %?

I would prefer dev_err(ibmvtpm->dev, __func__ " failed rc=%d\n", rc);

It's not what checkpatch advises in the above message, though.

Presumably with many messages from the same function using %s would
save space but that is not the usual case.

Thanks

Michal

Re: [PATCH 5/5] Use func instead of function name

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2017-08-18 11:25:43

Michal Such=C3=A1nek [off-list ref] writes:

On 2017-07-29 09:24, SZ Lin wrote:

quoted

Fix following checkpatch.pl warning:
WARNING: Prefer using '"%s...", __func__' to using
the function's name, in a string
=20
Signed-off-by: SZ Lin <redacted>
---
 drivers/char/tpm/tpm_ibmvtpm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
=20

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c=20

b/drivers/char/tpm/tpm_ibmvtpm.c
index e75a674b44ac..2d33acc43e25 100644

--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c

@@ -151,7 +151,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip,

u8 *buf, size_t count)
 	rc =3D ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]),
 			      be64_to_cpu(word[1]));
 	if (rc !=3D H_SUCCESS) {
-		dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=3D%d\n", rc);
+		dev_err(ibmvtpm->dev, "%s failed rc=3D%d\n", __func__, rc);

Can function name contain a %?

Let's hope not.

$ git grep "%s.*__func__" | wc -l
16937

cheers

Re: [PATCH 5/5] Use func instead of function name

From: 김동현 <hidden>
Date: 2017-08-19 07:58:12

I guess below code would be better idea to gather more debug information.

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 643bba7..0aae785 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c

@@ -141,6 +141,11 @@ static int tpm_ibmvtpm_send(struct tpm_chip

*chip, u8 *buf, size_t count)
                return -EIO;
        }

+       if (!buf) {
+               dev_err(ibmvtpm->dev, "%s buf null at %d \n",__func__,
rc, __LINE__);
+               return 0;
+       }
+
        spin_lock(&ibmvtpm->rtce_lock);
        memcpy((void *)ibmvtpm->rtce_buf, (void *)buf, count);
        crq.valid = (u8)IBMVTPM_VALID_CMD;

@@ -150,8 +155,9 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip,

u8 *buf, size_t count)

        rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]),
                              be64_to_cpu(word[1]));
+
        if (rc != H_SUCCESS) {
-               dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
+               dev_err(ibmvtpm->dev, "%s failed rc=%d at %d
\n",__func__, rc, __LINE__);
                rc = 0;
        } else
                rc = count;

2017-07-29 16:24 GMT+09:00 SZ Lin [off-list ref]:

quoted hunk

Fix following checkpatch.pl warning:
WARNING: Prefer using '"%s...", __func__' to using
the function's name, in a string

Signed-off-by: SZ Lin <redacted>
---
 drivers/char/tpm/tpm_ibmvtpm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index e75a674b44ac..2d33acc43e25 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c

@@ -151,7 +151,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
        rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]),
                              be64_to_cpu(word[1]));
        if (rc != H_SUCCESS) {
-               dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
+               dev_err(ibmvtpm->dev, "%s failed rc=%d\n", __func__, rc);
                rc = 0;
                ibmvtpm->tpm_processing_cmd = false;
        } else

@@ -193,7 +193,7 @@ static int ibmvtpm_crq_get_rtce_size(struct ibmvtpm_dev *ibmvtpm)
                              cpu_to_be64(buf[1]));
        if (rc != H_SUCCESS)
                dev_err(ibmvtpm->dev,
-                       "ibmvtpm_crq_get_rtce_size failed rc=%d\n", rc);
+                       "%s failed rc=%d\n", __func__, rc);

        return rc;
 }

@@ -221,7 +221,7 @@ static int ibmvtpm_crq_get_version(struct ibmvtpm_dev *ibmvtpm)
                              cpu_to_be64(buf[1]));
        if (rc != H_SUCCESS)
                dev_err(ibmvtpm->dev,
-                       "ibmvtpm_crq_get_version failed rc=%d\n", rc);
+                       "%s failed rc=%d\n", __func__, rc);

        return rc;
 }

@@ -241,7 +241,7 @@ static int ibmvtpm_crq_send_init_complete(struct ibmvtpm_dev *ibmvtpm)
        rc = ibmvtpm_send_crq(ibmvtpm->vdev, INIT_CRQ_COMP_CMD, 0);
        if (rc != H_SUCCESS)
                dev_err(ibmvtpm->dev,
-                       "ibmvtpm_crq_send_init_complete failed rc=%d\n", rc);
+                       "%s rc=%d\n", __func__, rc);

        return rc;
 }

@@ -261,7 +261,7 @@ static int ibmvtpm_crq_send_init(struct ibmvtpm_dev *ibmvtpm)
        rc = ibmvtpm_send_crq(ibmvtpm->vdev, INIT_CRQ_CMD, 0);
        if (rc != H_SUCCESS)
                dev_err(ibmvtpm->dev,
-                       "ibmvtpm_crq_send_init failed rc=%d\n", rc);
+                       "%s failed rc=%d\n", __func__, rc);

        return rc;
 }

@@ -351,7 +351,7 @@ static int tpm_ibmvtpm_suspend(struct device *dev)
                              cpu_to_be64(buf[1]));
        if (rc != H_SUCCESS)
                dev_err(ibmvtpm->dev,
-                       "tpm_ibmvtpm_suspend failed rc=%d\n", rc);
+                       "%s failed rc=%d\n", __func__, rc);

        return rc;
 }
--

2.13.3

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help