WTF: patch "[PATCH] tpm_tis: Move ilb_base_addr to tpm_tis_data" was seriously submitted to be applied to the 4.15-stable tree?

From c382babccba2c82fe57f9e647f290fb7bf4d130d Mon Sep 17 00:00:00 2001

-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: Thursday, February 1, 2018 4:19 AM
To: Shaikh, Azhar <[email protected]>; [email protected]
Cc: [email protected]
Subject: WTF: patch "[PATCH] tpm_tis: Move ilb_base_addr to tpm_tis_data" was seriously submitted to be applied to the 4.15-stable tree?

The patch below was submitted to be applied to the 4.15-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From c382babccba2c82fe57f9e647f290fb7bf4d130d Mon Sep 17 00:00:00 2001

From: Azhar Shaikh <[email protected]>
Date: Fri, 22 Dec 2017 12:13:43 -0800
Subject: [PATCH] tpm_tis: Move ilb_base_addr to tpm_tis_data

Move static variable ilb_base_addr to tpm_tis_data.

Cc [email protected]

Signed-off-by Azhar Shaikh <[email protected]>

Reviewed-by Jarkko Sakkinen <[email protected]>

Tested-by Jarkko Sakkinen <[email protected]>

Signed-off-by Jarkko Sakkinen <[email protected]>

drivers/char/tpm/tpm_tis.c index renamed from drivers/char/tpm/tpm_tis.c +0 -0

e2d1055fb814..923f8f2cbaca 100644

drivers/char/tpm/tpm_tis.c +2 -11

@@ -134,33 +134,24 @@ static int check_acpi_tpm2(struct device *dev)
 #endif
 
 #ifdef CONFIG_X86
-#define INTEL_LEGACY_BLK_BASE_ADDR      0xFED08000
-#define ILB_REMAP_SIZE			0x100
-#define LPC_CNTRL_REG_OFFSET            0x84
-#define LPC_CLKRUN_EN                   (1 << 2)
-
-static void __iomem *ilb_base_addr;
-
-static inline bool is_bsw(void)
-{
-	return ((boot_cpu_data.x86_model ==
 INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0);
-}
+#define LPC_CNTRL_OFFSET		0x84
+#define LPC_CLKRUN_EN			(1 << 2)
 
 /**
 * tpm_platform_begin_xfer() - clear LPC CLKRUN_EN i.e. clocks will be

running
*/

drivers/char/tpm/tpm_tis.c +1 -1

-static void tpm_platform_begin_xfer(void)
+static void tpm_platform_begin_xfer(struct tpm_tis_data *data)

{
	u32 clkrun_val;

	if (!is_bsw())
		return;

drivers/char/tpm/tpm_tis.c +1 -1

-	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+	clkrun_val = ioread32(data->ilb_base_addr + LPC_CNTRL_OFFSET);

	/* Disable LPC CLKRUN# */
	clkrun_val &= ~LPC_CLKRUN_EN;

drivers/char/tpm/tpm_tis.c +1 -1

-	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+	iowrite32(clkrun_val, data->ilb_base_addr + LPC_CNTRL_OFFSET);

	/*
	 * Write any random value on port 0x80 which is on LPC, to make @@ -

173,18 +164,18 @@ static void tpm_platform_begin_xfer(void) /**
* tpm_platform_end_xfer() - set LPC CLKRUN_EN i.e. clocks can be turned
off
*/

drivers/char/tpm/tpm_tis.c +1 -1

-static void tpm_platform_end_xfer(void)
+static void tpm_platform_end_xfer(struct tpm_tis_data *data)

{
	u32 clkrun_val;

	if (!is_bsw())
		return;

drivers/char/tpm/tpm_tis.c +1 -1

-	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+	clkrun_val = ioread32(data->ilb_base_addr + LPC_CNTRL_OFFSET);

	/* Enable LPC CLKRUN# */
	clkrun_val |= LPC_CLKRUN_EN;

drivers/char/tpm/tpm_tis.c +1 -1

-	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+	iowrite32(clkrun_val, data->ilb_base_addr + LPC_CNTRL_OFFSET);

	/*
	 * Write any random value on port 0x80 which is on LPC, to make @@ -

194,16 +185,11 @@ static void tpm_platform_end_xfer(void)

}
#else

drivers/char/tpm/tpm_tis.c +1 -6

-static inline bool is_bsw(void)
-{
-	return false;
-}
-
-static void tpm_platform_begin_xfer(void)
+static void tpm_platform_begin_xfer(struct tpm_tis_data *data)

{
}

drivers/char/tpm/tpm_tis.c +1 -1

-static void tpm_platform_end_xfer(void)
+static void tpm_platform_end_xfer(struct tpm_tis_data *data)

{
}
#endif

drivers/char/tpm/tpm_tis.c +14 -10

@@ -213,12 +199,12 @@ static int tpm_tcg_read_bytes(struct tpm_tis_data
 *data, u32 addr, u16 len,  {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
-	tpm_platform_begin_xfer();
+	tpm_platform_begin_xfer(data);
 
 	while (len--)
 		*result++ = ioread8(phy->iobase + addr);
 
-	tpm_platform_end_xfer();
+	tpm_platform_end_xfer(data);
 
 	return 0;
 }

@@ -228,12 +214,12 @@ static int tpm_tcg_write_bytes(struct tpm_tis_data
 *data, u32 addr, u16 len,  {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
-	tpm_platform_begin_xfer();
+	tpm_platform_begin_xfer(data);
 
 	while (len--)
 		iowrite8(*value++, phy->iobase + addr);
 
-	tpm_platform_end_xfer();
+	tpm_platform_end_xfer(data);
 
 	return 0;
 }

@@ -242,11 +228,11 @@ static int tpm_tcg_read16(struct tpm_tis_data
 *data, u32 addr, u16 *result)  {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
-	tpm_platform_begin_xfer();
+	tpm_platform_begin_xfer(data);
 
 	*result = ioread16(phy->iobase + addr);
 
-	tpm_platform_end_xfer();
+	tpm_platform_end_xfer(data);
 
 	return 0;
 }

@@ -255,11 +241,11 @@ static int tpm_tcg_read32(struct tpm_tis_data
 *data, u32 addr, u32 *result)  {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
-	tpm_platform_begin_xfer();
+	tpm_platform_begin_xfer(data);
 
 	*result = ioread32(phy->iobase + addr);
 
-	tpm_platform_end_xfer();
+	tpm_platform_end_xfer(data);
 
 	return 0;
 }

@@ -268,11 +254,11 @@ static int tpm_tcg_write32(struct tpm_tis_data
 *data, u32 addr, u32 value)  {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
-	tpm_platform_begin_xfer();
+	tpm_platform_begin_xfer(data);
 
 	iowrite32(value, phy->iobase + addr);
 
-	tpm_platform_end_xfer();
+	tpm_platform_end_xfer(data);
 
 	return 0;
 }

@@ -351,9 +337,13 @@ MODULE_DEVICE_TABLE(pnp, tpm_pnp_tbl);  static
 void tpm_tis_pnp_remove(struct pnp_dev *dev)  {
 	struct tpm_chip *chip = pnp_get_drvdata(dev);
+	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
 
 	tpm_chip_unregister(chip);
 	tpm_tis_remove(chip);
+	if (is_bsw())
+		iounmap(priv->ilb_base_addr);
+
 }
 

static struct pnp_driver tis_pnp_driver = { @@ -400,10 +390,14 @@ static int tpm_tis_plat_probe(struct platform_device *pdev) static int

tpm_tis_plat_remove(struct platform_device *pdev)  {
	struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);

drivers/char/tpm/tpm_tis.c +1 -0

+	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);

	tpm_chip_unregister(chip);
	tpm_tis_remove(chip);

drivers/char/tpm/tpm_tis.c +3 -0

+	if (is_bsw())
+		iounmap(priv->ilb_base_addr);
+

	return 0;
}

drivers/char/tpm/tpm_tis.c +0 -5

@@ -461,11 +455,6 @@ static int __init init_tis(void)
 	if (rc)
 		goto err_force;
 
-#ifdef CONFIG_X86
-	if (is_bsw())
-		ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
-					ILB_REMAP_SIZE);
-#endif
 	rc = platform_driver_register(&tis_drv);
 	if (rc)
 		goto err_platform;

@@ -484,10 +473,6 @@ err_pnp:

err_platform:

	if (force_pdev)
		platform_device_unregister(force_pdev);

drivers/char/tpm/tpm_tis.c +0 -4

-#ifdef CONFIG_X86
-	if (is_bsw())
-		iounmap(ilb_base_addr);
-#endif

err_force:

	return rc;
}

drivers/char/tpm/tpm_tis.c +0 -4

@@ -497,10 +482,6 @@ static void __exit cleanup_tis(void)
 	pnp_unregister_driver(&tis_pnp_driver);
 	platform_driver_unregister(&tis_drv);
 
-#ifdef CONFIG_X86
-	if (is_bsw())
-		iounmap(ilb_base_addr);
-#endif
 	if (force_pdev)
 		platform_device_unregister(force_pdev);
 }

(quoted diff) +0 -0

b/drivers/char/tpm/tpm_tis_core.c index ca6b2b527d52..aff567840e50 100644

drivers/char/tpm/tpm_tis_core.c +7 -0

@@ -766,6 +766,13 @@ int tpm_tis_core_init(struct device *dev, struct
 tpm_tis_data *priv, int irq,
 	priv->phy_ops = phy_ops;
 	dev_set_drvdata(&chip->dev, priv);
 
+	if (is_bsw()) {
+		priv->ilb_base_addr =
 ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
+					ILB_REMAP_SIZE);
+		if (!priv->ilb_base_addr)
+			return -ENOMEM;
+	}
+
 	if (wait_startup(chip, 0) != 0) {

		rc = -ENODEV;
		goto out_err;

drivers/char/tpm/tpm_tis_core.c +5 -1

@@ -856,9 +863,16 @@ int tpm_tis_core_init(struct device *dev, struct
 tpm_tis_data *priv, int irq,
 		}
 	}
 
-	return tpm_chip_register(chip);
+	rc = tpm_chip_register(chip);
+	if (rc && is_bsw())
+		iounmap(priv->ilb_base_addr);
+
+	return rc;

out_err:

	tpm_tis_remove(chip);

drivers/char/tpm/tpm_tis_core.c +3 -0

+	if (is_bsw())
+		iounmap(priv->ilb_base_addr);
+

	return rc;
}

EXPORT_SYMBOL_GPL(tpm_tis_core_init);

(quoted diff) +0 -0

b/drivers/char/tpm/tpm_tis_core.h index 6bbac319ff3b..458847f72758 100644

drivers/char/tpm/tpm_tis_core.h +4 -0

@@ -79,6 +79,9 @@ enum tis_defaults {
 #define	TPM_DID_VID(l)			(0x0F00 | ((l) << 12))
 #define	TPM_RID(l)			(0x0F04 | ((l) << 12))
 
+#define INTEL_LEGACY_BLK_BASE_ADDR	0xFED08000
+#define ILB_REMAP_SIZE			0x100
+
 enum tpm_tis_flags {
 	TPM_TIS_ITPM_WORKAROUND		= BIT(0),
 };

@@ -89,6 +92,7 @@ struct tpm_tis_data {
 	int irq;
 	bool irq_tested;
 	unsigned int flags;
+	void __iomem *ilb_base_addr;
 	wait_queue_head_t int_queue;
 	wait_queue_head_t read_queue;
 	const struct tpm_tis_phy_ops *phy_ops; @@ -144,6 +148,15 @@

static inline int tpm_tis_write32(struct tpm_tis_data *data, u32 addr,

	return data->phy_ops->write32(data, addr, value);  }

drivers/char/tpm/tpm_tis_core.h +4 -0

+static inline bool is_bsw(void)
+{
+#ifdef CONFIG_X86
+	return ((boot_cpu_data.x86_model ==

INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0);

drivers/char/tpm/tpm_tis_core.h +5 -0

+#else
+	return false;
+#endif
+}
+

void tpm_tis_remove(struct tpm_chip *chip); int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
const struct tpm_tis_phy_ops *phy_ops,

Hi Greg,

[1] is merged on the 4.15 stable tree. But this caused 2 issues:
i. PS/2 keyboard and mouse were broken on an Intel Braswell system. The fix for this is [4].
ii. There were some corner cases where TPM suspend was failing on Braswell systems. The fix for this is [2] and [3].

The patch pointed out by you is [2].
[3] is dependent on [2]. So to fix the issue on stable tree we will need [2], [3] and [4].

[4] is not there yet on Linus's branch. I think it is queued for 4.16.

[5] fixes compile time warning introduced in [3].

-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: Friday, February 2, 2018 9:15 PM
To: Shaikh, Azhar <[email protected]>
Cc: [email protected]; [email protected]
Subject: Re: patch "[PATCH] tpm_tis: Move ilb_base_addr to tpm_tis_data" was seriously submitted to be applied to the 4.15-stable tree?

Quoted 5 lines · Azhar

Hi Greg,

[1] is merged on the 4.15 stable tree. But this caused 2 issues:
i. PS/2 keyboard and mouse were broken on an Intel Braswell system. The

fix for this is [4].

Quoted 1 line

ii. There were some corner cases where TPM suspend was failing on

Braswell systems. The fix for this is [2] and [3].

Quoted 3 lines

The patch pointed out by you is [2].
[3] is dependent on [2]. So to fix the issue on stable tree we will need [2],

[3] and [4].

Quoted 4 lines

[4] is not there yet on Linus's branch. I think it is queued for 4.16.

[5] fixes compile time warning introduced in [3].

This email was crazy difficult to read and understand :(

Please, footnotes in emails are for things you can go back and look at later, after reading the body of a message, not to try to understand exactly what in the world is going on and what I am supposed to do with something.

In the future, just put the git sha1 in the email text, no need to link to anything, we all have zillions of copies of the kernel source on our own machines. And sometimes we do not have reliable web access (emails works great as store and forward.)

Just use the format:
c382babccba2 ("tpm_tis: Move ilb_base_addr to tpm_tis_data") to
describe a commit id, which can be easily obtained by doing:

	git show -s --abbrev-commit --abbrev=12 --pretty=format:"%h

(\"%s\")%n"'
(hint, I have an alias for it so I don't have to ever type that again).

So, with all of that, I still have no idea what I am supposed to do here. The patch that triggered this email is obviously not a stable tree patch, as all it does is move code around, right? Is there something else I should be doing here? What action do you want me to take?

totally confused,

greg k-h