[PATCH 0/4] tmp/tpm_crb: cleanups

[PATCH 0/4] tmp/tpm_crb: cleanups

[Tomas Winkler]

A set of cleanup patches, w/o any significant functional change.

Tomas Winkler (4):
  tmp/tpm_crb: drop include to platform_device
  tpm/tpm_crb: cache cmd_size register value.
  tpm/tpm_crb: drop useless cpu_to_le32 when writing to registers
  tpm/tpm_crb: fix the over 80 characters checkpatch warring

 drivers/char/tpm/tpm_crb.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

-- 
2.7.4


------------------------------------------------------------------------------

[PATCH 1/4] tmp/tpm_crb: drop include to platform_device

[Tomas Winkler]

The platform device is not used in this driver, drop the
include to linux/platform_device.h

Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/char/tpm/tpm_crb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 82a3ccd52a3a..6a4b32599869 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -19,7 +19,6 @@
 #include <linux/highmem.h>
 #include <linux/rculist.h>
 #include <linux/module.h>
-#include <linux/platform_device.h>
 #include "tpm.h"
 
 #define ACPI_SIG_TPM2 "TPM2"
-- 
2.7.4


------------------------------------------------------------------------------

Re: [PATCH 1/4] tmp/tpm_crb: drop include to platform_device

[Jarkko Sakkinen]

On Mon, Sep 12, 2016 at 02:03:52AM +0300, Tomas Winkler wrote:
> The platform device is not used in this driver, drop the
> include to linux/platform_device.h
> 
> Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

/Jarkko

> ---
>  drivers/char/tpm/tpm_crb.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 82a3ccd52a3a..6a4b32599869 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -19,7 +19,6 @@
>  #include <linux/highmem.h>
>  #include <linux/rculist.h>
>  #include <linux/module.h>
> -#include <linux/platform_device.h>
>  #include "tpm.h"
>  
>  #define ACPI_SIG_TPM2 "TPM2"
> -- 
> 2.7.4
> 

------------------------------------------------------------------------------

Re: [PATCH 1/4] tmp/tpm_crb: drop include to platform_device

[Jarkko Sakkinen]

On Mon, Sep 12, 2016 at 12:29:21PM +0300, Jarkko Sakkinen wrote:
> On Mon, Sep 12, 2016 at 02:03:52AM +0300, Tomas Winkler wrote:
> > The platform device is not used in this driver, drop the
> > include to linux/platform_device.h
> > 
> > Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Applied.

/Jarkko

> 
> /Jarkko
> 
> > ---
> >  drivers/char/tpm/tpm_crb.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > index 82a3ccd52a3a..6a4b32599869 100644
> > --- a/drivers/char/tpm/tpm_crb.c
> > +++ b/drivers/char/tpm/tpm_crb.c
> > @@ -19,7 +19,6 @@
> >  #include <linux/highmem.h>
> >  #include <linux/rculist.h>
> >  #include <linux/module.h>
> > -#include <linux/platform_device.h>
> >  #include "tpm.h"
> >  
> >  #define ACPI_SIG_TPM2 "TPM2"
> > -- 
> > 2.7.4
> > 

------------------------------------------------------------------------------

[PATCH 2/4 v2] tpm/tpm_crb: cache cmd_size register value.

[Tomas Winkler]

Instead of expensive register access on retrieving cmd_size
on each send, save the value during initialization in the private
context. The value doesn't change.

Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

---
V2: rebased

 drivers/char/tpm/tpm_crb.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 6a4b32599869..f066810bfc03 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -80,6 +80,7 @@ struct crb_priv {
 	struct crb_control_area __iomem *cca;
 	u8 __iomem *cmd;
 	u8 __iomem *rsp;
+	u32 cmd_size;
 };
 
 static SIMPLE_DEV_PM_OPS(crb_pm, tpm_pm_suspend, tpm_pm_resume);
@@ -141,11 +142,9 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
 	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
 	int rc = 0;
 
-	if (len > ioread32(&priv->cca->cmd_size)) {
-		dev_err(&chip->dev,
-			"invalid command count value %x %zx\n",
-			(unsigned int) len,
-			(size_t) ioread32(&priv->cca->cmd_size));
+	if (len > priv->cmd_size) {
+		dev_err(&chip->dev, "invalid command count value %zd %d\n",
+			len, priv->cmd_size);
 		return -E2BIG;
 	}
 
@@ -301,6 +300,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 		dev_err(dev, FW_BUG "overlapping command and response buffer sizes are not identical");
 		return -EINVAL;
 	}
+	priv->cmd_size = cmd_size;
 
 	priv->rsp = priv->cmd;
 	return 0;
-- 
2.7.4


------------------------------------------------------------------------------

Re: [PATCH 2/4 v2] tpm/tpm_crb: cache cmd_size register value.

[Jarkko Sakkinen]

On Mon, Sep 12, 2016 at 02:03:53AM +0300, Tomas Winkler wrote:
> Instead of expensive register access on retrieving cmd_size
> on each send, save the value during initialization in the private
> context. The value doesn't change.
> 
> Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

$ git am -3 ~/Downloads/2-4-v2-tpm-tpm_crb-cache-cmd_size-register-value..patch
Applying: tpm/tpm_crb: cache cmd_size register value.
fatal: sha1 information is lacking or useless (drivers/char/tpm/tpm_crb.c).
error: could not build fake ancestor
Patch failed at 0001 tpm/tpm_crb: cache cmd_size register value.
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
VCS_INFO_get_data_git:225: no such file or directory: .git/rebase-apply/msg-clean                                                                           

/Jarkko

> 
> ---
> V2: rebased
> 
>  drivers/char/tpm/tpm_crb.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 6a4b32599869..f066810bfc03 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -80,6 +80,7 @@ struct crb_priv {
>  	struct crb_control_area __iomem *cca;
>  	u8 __iomem *cmd;
>  	u8 __iomem *rsp;
> +	u32 cmd_size;
>  };
>  
>  static SIMPLE_DEV_PM_OPS(crb_pm, tpm_pm_suspend, tpm_pm_resume);
> @@ -141,11 +142,9 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
>  	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
>  	int rc = 0;
>  
> -	if (len > ioread32(&priv->cca->cmd_size)) {
> -		dev_err(&chip->dev,
> -			"invalid command count value %x %zx\n",
> -			(unsigned int) len,
> -			(size_t) ioread32(&priv->cca->cmd_size));
> +	if (len > priv->cmd_size) {
> +		dev_err(&chip->dev, "invalid command count value %zd %d\n",
> +			len, priv->cmd_size);
>  		return -E2BIG;
>  	}
>  
> @@ -301,6 +300,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
>  		dev_err(dev, FW_BUG "overlapping command and response buffer sizes are not identical");
>  		return -EINVAL;
>  	}
> +	priv->cmd_size = cmd_size;
>  
>  	priv->rsp = priv->cmd;
>  	return 0;
> -- 
> 2.7.4
> 

------------------------------------------------------------------------------

Re: [PATCH 2/4 v2] tpm/tpm_crb: cache cmd_size register value.

[Jarkko Sakkinen]

On Mon, Sep 12, 2016 at 01:49:16PM +0300, Jarkko Sakkinen wrote:
> On Mon, Sep 12, 2016 at 02:03:53AM +0300, Tomas Winkler wrote:
> > Instead of expensive register access on retrieving cmd_size
> > on each send, save the value during initialization in the private
> > context. The value doesn't change.
> > 
> > Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> $ git am -3 ~/Downloads/2-4-v2-tpm-tpm_crb-cache-cmd_size-register-value..patch
> Applying: tpm/tpm_crb: cache cmd_size register value.
> fatal: sha1 information is lacking or useless (drivers/char/tpm/tpm_crb.c).
> error: could not build fake ancestor
> Patch failed at 0001 tpm/tpm_crb: cache cmd_size register value.
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> VCS_INFO_get_data_git:225: no such file or directory: .git/rebase-apply/msg-clean                                                                           

I did this:

patch -p1 < ~/Downloads/2-4-v2-tpm-tpm_crb-cache-cmd_size-register-value..patch
git commit -a -s --author="Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>"

It's now pushed. Could you check that it looks OK for you so that I can
include this to the next pull request?

/Jarkko

------------------------------------------------------------------------------

Re: [PATCH 2/4 v2] tpm/tpm_crb: cache cmd_size register value.

[Winkler, Tomas]

> 
> On Mon, Sep 12, 2016 at 01:49:16PM +0300, Jarkko Sakkinen wrote:
> > On Mon, Sep 12, 2016 at 02:03:53AM +0300, Tomas Winkler wrote:
> > > Instead of expensive register access on retrieving cmd_size on each
> > > send, save the value during initialization in the private context.
> > > The value doesn't change.
> > >
> > > Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> >
> > $ git am -3
> > ~/Downloads/2-4-v2-tpm-tpm_crb-cache-cmd_size-register-value..patch
> > Applying: tpm/tpm_crb: cache cmd_size register value.
> > fatal: sha1 information is lacking or useless (drivers/char/tpm/tpm_crb.c).
> > error: could not build fake ancestor
> > Patch failed at 0001 tpm/tpm_crb: cache cmd_size register value.
> > The copy of the patch that failed is found in: .git/rebase-apply/patch
> > When you have resolved this problem, run "git am --continue".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".
> > VCS_INFO_get_data_git:225: no such file or directory: .git/rebase-
> apply/msg-clean
> 
> I did this:
> 
> patch -p1 < ~/Downloads/2-4-v2-tpm-tpm_crb-cache-cmd_size-register-
> value..patch
> git commit -a -s --author="Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>"
> 
> It's now pushed. Could you check that it looks OK for you so that I can include
> this to the next pull request?

Looks great. 

Thanks
Tomas

------------------------------------------------------------------------------

[PATCH 3/4] tpm/tpm_crb: drop useless cpu_to_le32 when writing to registers

[Tomas Winkler]

Don't apply endianity conversion when writing to the registers
this is already handled by the system.

Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/char/tpm/tpm_crb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index f066810bfc03..0c9989d0106e 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -154,7 +154,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
 	wmb();
 
 	if (priv->flags & CRB_FL_CRB_START)
-		iowrite32(cpu_to_le32(CRB_START_INVOKE), &priv->cca->start);
+		iowrite32(CRB_START_INVOKE, &priv->cca->start);
 
 	if (priv->flags & CRB_FL_ACPI_START)
 		rc = crb_do_acpi_start(chip);
@@ -166,7 +166,7 @@ static void crb_cancel(struct tpm_chip *chip)
 {
 	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
 
-	iowrite32(cpu_to_le32(CRB_CANCEL_INVOKE), &priv->cca->cancel);
+	iowrite32(CRB_CANCEL_INVOKE, &priv->cca->cancel);
 
 	/* Make sure that cmd is populated before issuing cancel. */
 	wmb();
-- 
2.7.4


------------------------------------------------------------------------------

Re: [PATCH 3/4] tpm/tpm_crb: drop useless cpu_to_le32 when writing to registers

[Jarkko Sakkinen]

On Mon, Sep 12, 2016 at 02:03:54AM +0300, Tomas Winkler wrote:
> Don't apply endianity conversion when writing to the registers
> this is already handled by the system.
> 
> Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

/Jarkko

> ---
>  drivers/char/tpm/tpm_crb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index f066810bfc03..0c9989d0106e 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -154,7 +154,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
>  	wmb();
>  
>  	if (priv->flags & CRB_FL_CRB_START)
> -		iowrite32(cpu_to_le32(CRB_START_INVOKE), &priv->cca->start);
> +		iowrite32(CRB_START_INVOKE, &priv->cca->start);
>  
>  	if (priv->flags & CRB_FL_ACPI_START)
>  		rc = crb_do_acpi_start(chip);
> @@ -166,7 +166,7 @@ static void crb_cancel(struct tpm_chip *chip)
>  {
>  	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
>  
> -	iowrite32(cpu_to_le32(CRB_CANCEL_INVOKE), &priv->cca->cancel);
> +	iowrite32(CRB_CANCEL_INVOKE, &priv->cca->cancel);
>  
>  	/* Make sure that cmd is populated before issuing cancel. */
>  	wmb();
> -- 
> 2.7.4
> 

------------------------------------------------------------------------------

[PATCH 4/4] tpm/tpm_crb: fix the over 80 characters checkpatch warring

[Tomas Winkler]

Because of the line break in the debug print the chackpatch is
not silent on 80 characters limitation.

The easiest fix is to straighten the lines, it's also more readable.

WARNING: line over 80 characters
+                       FW_BUG "TPM2 ACPI table does not define a memory
resource\n");

Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/char/tpm/tpm_crb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 0c9989d0106e..6e9d1bca712f 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -263,8 +263,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 	acpi_dev_free_resource_list(&resources);
 
 	if (resource_type(&io_res) != IORESOURCE_MEM) {
-		dev_err(dev,
-			FW_BUG "TPM2 ACPI table does not define a memory resource\n");
+		dev_err(dev, FW_BUG "TPM2 ACPI table does not define a memory resource\n");
 		return -EINVAL;
 	}
 
-- 
2.7.4


------------------------------------------------------------------------------

Re: [PATCH 4/4] tpm/tpm_crb: fix the over 80 characters checkpatch warring

[Jarkko Sakkinen]

On Mon, Sep 12, 2016 at 02:03:55AM +0300, Tomas Winkler wrote:
> Because of the line break in the debug print the chackpatch is
> not silent on 80 characters limitation.
> 
> The easiest fix is to straighten the lines, it's also more readable.
> 
> WARNING: line over 80 characters
> +                       FW_BUG "TPM2 ACPI table does not define a memory
> resource\n");
> 
> Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

/Jarkko

> ---
>  drivers/char/tpm/tpm_crb.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 0c9989d0106e..6e9d1bca712f 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -263,8 +263,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
>  	acpi_dev_free_resource_list(&resources);
>  
>  	if (resource_type(&io_res) != IORESOURCE_MEM) {
> -		dev_err(dev,
> -			FW_BUG "TPM2 ACPI table does not define a memory resource\n");
> +		dev_err(dev, FW_BUG "TPM2 ACPI table does not define a memory resource\n");
>  		return -EINVAL;
>  	}
>  
> -- 
> 2.7.4
> 

------------------------------------------------------------------------------

Re: [PATCH 0/4] tmp/tpm_crb: cleanups

[Jarkko Sakkinen]

On Mon, Sep 12, 2016 at 02:03:51AM +0300, Tomas Winkler wrote:
> A set of cleanup patches, w/o any significant functional change.
> 
> Tomas Winkler (4):
>   tmp/tpm_crb: drop include to platform_device
>   tpm/tpm_crb: cache cmd_size register value.
>   tpm/tpm_crb: drop useless cpu_to_le32 when writing to registers
>   tpm/tpm_crb: fix the over 80 characters checkpatch warring
> 
>  drivers/char/tpm/tpm_crb.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)

I applied all of these.

/Jarkko

------------------------------------------------------------------------------