2017-07-29 07:25:08

by SZ Lin

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

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

2017-07-29 07:25:15

by SZ Lin

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

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

Signed-off-by: SZ Lin <[email protected]>
---
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

2017-07-29 07:25:12

by SZ Lin

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

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

Signed-off-by: SZ Lin <[email protected]>
---
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

2017-07-29 07:25:35

by SZ Lin

Subject: [PATCH 5/5] Use __func__ instead of function name

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

Signed-off-by: SZ Lin <[email protected]>
---
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

2017-07-29 07:25:10

by SZ Lin

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

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 <[email protected]>
---
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

2017-07-29 07:25:51

by SZ Lin

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

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 <[email protected]>
---
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

2017-07-31 10:25:06

by Michael Ellerman

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

SZ Lin <[email protected]> writes:

> ERROR: code indent should use tabs where possible
> +^I^I "Need to wait for TPM to finish\n");$
>
> Signed-off-by: SZ Lin <[email protected]>
> ---
> 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

2017-07-31 13:27:55

by David Laight

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

From: SZ Lin
> Sent: 29 July 2017 08:24
...
> 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);

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

2017-08-02 12:36:25

by Jarkko Sakkinen

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

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

2017-08-18 08:03:32

by Michal Suchánek

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

On 2017-07-29 09:24, SZ Lin wrote:
> Fix following checkpatch.pl warning:
> WARNING: Prefer using '"%s...", __func__' to using
> the function's name, in a string
>
> Signed-off-by: SZ Lin <[email protected]>
> ---
> 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

2017-08-18 11:25:43

by Michael Ellerman

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

Michal Suchánek <[email protected]> writes:

> On 2017-07-29 09:24, SZ Lin wrote:
>> Fix following checkpatch.pl warning:
>> WARNING: Prefer using '"%s...", __func__' to using
>> the function's name, in a string
>>
>> Signed-off-by: SZ Lin <[email protected]>
>> ---
>> 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 %?

Let's hope not.

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

cheers

2017-08-19 07:58:12

by Austin Kim

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

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 <[email protected]>:
> Fix following checkpatch.pl warning:
> WARNING: Prefer using '"%s...", __func__' to using
> the function's name, in a string
>
> Signed-off-by: SZ Lin <[email protected]>
> ---
> 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
>