* [PATCH RFC 0/2] tpm: Minor improvements @ 2024-09-06 6:09 Stefan Wahren 2024-09-06 6:09 ` [PATCH RFC 1/2] tpm: tpm_tis_spi: Ensure SPI mode 0 Stefan Wahren 2024-09-06 6:09 ` [PATCH RFC 2/2] tpm: tpm_tis: Provide error messages in tpm_tis_core_init Stefan Wahren 0 siblings, 2 replies; 9+ messages in thread From: Stefan Wahren @ 2024-09-06 6:09 UTC (permalink / raw) To: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe Cc: linux-integrity, linux-kernel, Stefan Berger, Stefan Wahren These minor improvements are the results of wiring up a Infineon SLB9670 chip on a custom board. Stefan Wahren (2): tpm: tpm_tis_spi: Ensure SPI mode 0 tpm: tpm_tis: Provide error messages in tpm_tis_core_init drivers/char/tpm/tpm_tis_core.c | 15 ++++++++++++--- drivers/char/tpm/tpm_tis_spi_main.c | 11 +++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH RFC 1/2] tpm: tpm_tis_spi: Ensure SPI mode 0 2024-09-06 6:09 [PATCH RFC 0/2] tpm: Minor improvements Stefan Wahren @ 2024-09-06 6:09 ` Stefan Wahren 2024-09-06 9:47 ` Jarkko Sakkinen 2024-09-06 6:09 ` [PATCH RFC 2/2] tpm: tpm_tis: Provide error messages in tpm_tis_core_init Stefan Wahren 1 sibling, 1 reply; 9+ messages in thread From: Stefan Wahren @ 2024-09-06 6:09 UTC (permalink / raw) To: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe Cc: linux-integrity, linux-kernel, Stefan Berger, Stefan Wahren According to TCG PC Client Platform TPM Profile (PTP) Specification only SPI mode 0 is supported. In order to ensure the SPI controller supports the necessary settings, call spi_setup() and bail out as soon as possible in error case. This change should affect all supported TPM SPI devices, because tpm_tis_spi_probe is either called direct or indirectly. Signed-off-by: Stefan Wahren <wahrenst@gmx.net> --- drivers/char/tpm/tpm_tis_spi_main.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c index 61b42c83ced8..e62d297b040e 100644 --- a/drivers/char/tpm/tpm_tis_spi_main.c +++ b/drivers/char/tpm/tpm_tis_spi_main.c @@ -248,6 +248,17 @@ static int tpm_tis_spi_write_bytes(struct tpm_tis_data *data, u32 addr, int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy, int irq, const struct tpm_tis_phy_ops *phy_ops) { + int ret; + + spi->mode &= ~SPI_MODE_X_MASK; + spi->mode |= SPI_MODE_0; + + ret = spi_setup(spi); + if (ret < 0) { + dev_err(&spi->dev, "SPI setup failed: %d\n", ret); + return ret; + } + phy->iobuf = devm_kmalloc(&spi->dev, SPI_HDRSIZE + MAX_SPI_FRAMESIZE, GFP_KERNEL); if (!phy->iobuf) return -ENOMEM; -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 1/2] tpm: tpm_tis_spi: Ensure SPI mode 0 2024-09-06 6:09 ` [PATCH RFC 1/2] tpm: tpm_tis_spi: Ensure SPI mode 0 Stefan Wahren @ 2024-09-06 9:47 ` Jarkko Sakkinen 2024-09-06 14:46 ` Stefan Wahren 0 siblings, 1 reply; 9+ messages in thread From: Jarkko Sakkinen @ 2024-09-06 9:47 UTC (permalink / raw) To: Stefan Wahren, Peter Huewe, Jason Gunthorpe Cc: linux-integrity, linux-kernel, Stefan Berger On Fri Sep 6, 2024 at 9:09 AM EEST, Stefan Wahren wrote: > According to TCG PC Client Platform TPM Profile (PTP) Specification > only SPI mode 0 is supported. In order to ensure the SPI controller > supports the necessary settings, call spi_setup() and bail out > as soon as possible in error case. > > This change should affect all supported TPM SPI devices, because > tpm_tis_spi_probe is either called direct or indirectly. > > Signed-off-by: Stefan Wahren <wahrenst@gmx.net> > --- > drivers/char/tpm/tpm_tis_spi_main.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c > index 61b42c83ced8..e62d297b040e 100644 > --- a/drivers/char/tpm/tpm_tis_spi_main.c > +++ b/drivers/char/tpm/tpm_tis_spi_main.c > @@ -248,6 +248,17 @@ static int tpm_tis_spi_write_bytes(struct tpm_tis_data *data, u32 addr, > int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy, > int irq, const struct tpm_tis_phy_ops *phy_ops) > { > + int ret; > + > + spi->mode &= ~SPI_MODE_X_MASK; > + spi->mode |= SPI_MODE_0; > + > + ret = spi_setup(spi); > + if (ret < 0) { > + dev_err(&spi->dev, "SPI setup failed: %d\n", ret); > + return ret; > + } > + > phy->iobuf = devm_kmalloc(&spi->dev, SPI_HDRSIZE + MAX_SPI_FRAMESIZE, GFP_KERNEL); > if (!phy->iobuf) > return -ENOMEM; > -- > 2.34.1 Why? BR, Jarkko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 1/2] tpm: tpm_tis_spi: Ensure SPI mode 0 2024-09-06 9:47 ` Jarkko Sakkinen @ 2024-09-06 14:46 ` Stefan Wahren 2024-09-09 17:05 ` Jarkko Sakkinen 0 siblings, 1 reply; 9+ messages in thread From: Stefan Wahren @ 2024-09-06 14:46 UTC (permalink / raw) To: Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe Cc: linux-integrity, linux-kernel, Stefan Berger Am 06.09.24 um 11:47 schrieb Jarkko Sakkinen: > On Fri Sep 6, 2024 at 9:09 AM EEST, Stefan Wahren wrote: >> According to TCG PC Client Platform TPM Profile (PTP) Specification >> only SPI mode 0 is supported. In order to ensure the SPI controller >> supports the necessary settings, call spi_setup() and bail out >> as soon as possible in error case. >> >> This change should affect all supported TPM SPI devices, because >> tpm_tis_spi_probe is either called direct or indirectly. >> >> Signed-off-by: Stefan Wahren <wahrenst@gmx.net> >> --- >> drivers/char/tpm/tpm_tis_spi_main.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c >> index 61b42c83ced8..e62d297b040e 100644 >> --- a/drivers/char/tpm/tpm_tis_spi_main.c >> +++ b/drivers/char/tpm/tpm_tis_spi_main.c >> @@ -248,6 +248,17 @@ static int tpm_tis_spi_write_bytes(struct tpm_tis_data *data, u32 addr, >> int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy, >> int irq, const struct tpm_tis_phy_ops *phy_ops) >> { >> + int ret; >> + >> + spi->mode &= ~SPI_MODE_X_MASK; >> + spi->mode |= SPI_MODE_0; >> + >> + ret = spi_setup(spi); >> + if (ret < 0) { >> + dev_err(&spi->dev, "SPI setup failed: %d\n", ret); >> + return ret; >> + } >> + >> phy->iobuf = devm_kmalloc(&spi->dev, SPI_HDRSIZE + MAX_SPI_FRAMESIZE, GFP_KERNEL); >> if (!phy->iobuf) >> return -ENOMEM; >> -- >> 2.34.1 > > Why? SPI protocol driver usually call spi_setup to verify that the SPI controller accept the settings (SPI mode, bit, clock rate ...). Bailing out early is more helpful for debugging issues. > > BR, Jarkko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 1/2] tpm: tpm_tis_spi: Ensure SPI mode 0 2024-09-06 14:46 ` Stefan Wahren @ 2024-09-09 17:05 ` Jarkko Sakkinen 2024-09-09 17:16 ` Stefan Wahren 0 siblings, 1 reply; 9+ messages in thread From: Jarkko Sakkinen @ 2024-09-09 17:05 UTC (permalink / raw) To: Stefan Wahren, Peter Huewe, Jason Gunthorpe Cc: linux-integrity, linux-kernel, Stefan Berger On Fri Sep 6, 2024 at 5:46 PM EEST, Stefan Wahren wrote: > Am 06.09.24 um 11:47 schrieb Jarkko Sakkinen: > > On Fri Sep 6, 2024 at 9:09 AM EEST, Stefan Wahren wrote: > >> According to TCG PC Client Platform TPM Profile (PTP) Specification > >> only SPI mode 0 is supported. In order to ensure the SPI controller > >> supports the necessary settings, call spi_setup() and bail out > >> as soon as possible in error case. > >> > >> This change should affect all supported TPM SPI devices, because > >> tpm_tis_spi_probe is either called direct or indirectly. > >> > >> Signed-off-by: Stefan Wahren <wahrenst@gmx.net> > >> --- > >> drivers/char/tpm/tpm_tis_spi_main.c | 11 +++++++++++ > >> 1 file changed, 11 insertions(+) > >> > >> diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c > >> index 61b42c83ced8..e62d297b040e 100644 > >> --- a/drivers/char/tpm/tpm_tis_spi_main.c > >> +++ b/drivers/char/tpm/tpm_tis_spi_main.c > >> @@ -248,6 +248,17 @@ static int tpm_tis_spi_write_bytes(struct tpm_tis_data *data, u32 addr, > >> int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy, > >> int irq, const struct tpm_tis_phy_ops *phy_ops) > >> { > >> + int ret; > >> + > >> + spi->mode &= ~SPI_MODE_X_MASK; > >> + spi->mode |= SPI_MODE_0; > >> + > >> + ret = spi_setup(spi); > >> + if (ret < 0) { > >> + dev_err(&spi->dev, "SPI setup failed: %d\n", ret); > >> + return ret; > >> + } > >> + > >> phy->iobuf = devm_kmalloc(&spi->dev, SPI_HDRSIZE + MAX_SPI_FRAMESIZE, GFP_KERNEL); > >> if (!phy->iobuf) > >> return -ENOMEM; > >> -- > >> 2.34.1 > > > > Why? > SPI protocol driver usually call spi_setup to verify that the SPI > controller accept the settings (SPI mode, bit, clock rate ...). Bailing > out early is more helpful for debugging issues. What problem has this patch solved for you? I think it makes the code only less robust and more error prone. BR, Jarkko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 1/2] tpm: tpm_tis_spi: Ensure SPI mode 0 2024-09-09 17:05 ` Jarkko Sakkinen @ 2024-09-09 17:16 ` Stefan Wahren 0 siblings, 0 replies; 9+ messages in thread From: Stefan Wahren @ 2024-09-09 17:16 UTC (permalink / raw) To: Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe Cc: linux-integrity, linux-kernel, Stefan Berger Hi Jarkko, Am 09.09.24 um 19:05 schrieb Jarkko Sakkinen: > On Fri Sep 6, 2024 at 5:46 PM EEST, Stefan Wahren wrote: >> Am 06.09.24 um 11:47 schrieb Jarkko Sakkinen: >>> On Fri Sep 6, 2024 at 9:09 AM EEST, Stefan Wahren wrote: >>>> According to TCG PC Client Platform TPM Profile (PTP) Specification >>>> only SPI mode 0 is supported. In order to ensure the SPI controller >>>> supports the necessary settings, call spi_setup() and bail out >>>> as soon as possible in error case. >>>> >>>> This change should affect all supported TPM SPI devices, because >>>> tpm_tis_spi_probe is either called direct or indirectly. >>>> >>>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net> >>>> --- >>>> drivers/char/tpm/tpm_tis_spi_main.c | 11 +++++++++++ >>>> 1 file changed, 11 insertions(+) >>>> >>>> diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c >>>> index 61b42c83ced8..e62d297b040e 100644 >>>> --- a/drivers/char/tpm/tpm_tis_spi_main.c >>>> +++ b/drivers/char/tpm/tpm_tis_spi_main.c >>>> @@ -248,6 +248,17 @@ static int tpm_tis_spi_write_bytes(struct tpm_tis_data *data, u32 addr, >>>> int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy, >>>> int irq, const struct tpm_tis_phy_ops *phy_ops) >>>> { >>>> + int ret; >>>> + >>>> + spi->mode &= ~SPI_MODE_X_MASK; >>>> + spi->mode |= SPI_MODE_0; >>>> + >>>> + ret = spi_setup(spi); >>>> + if (ret < 0) { >>>> + dev_err(&spi->dev, "SPI setup failed: %d\n", ret); >>>> + return ret; >>>> + } >>>> + >>>> phy->iobuf = devm_kmalloc(&spi->dev, SPI_HDRSIZE + MAX_SPI_FRAMESIZE, GFP_KERNEL); >>>> if (!phy->iobuf) >>>> return -ENOMEM; >>>> -- >>>> 2.34.1 >>> Why? >> SPI protocol driver usually call spi_setup to verify that the SPI >> controller accept the settings (SPI mode, bit, clock rate ...). Bailing >> out early is more helpful for debugging issues. > What problem has this patch solved for you? I think it makes the code > only less robust and more error prone. this is not a fix for an issue i was experiencing. It is just an improvement to catch possible invalid settings which has been passed via DT device for example or the SPI controller doesn't support SPI mode 0. > > BR, Jarkko ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH RFC 2/2] tpm: tpm_tis: Provide error messages in tpm_tis_core_init 2024-09-06 6:09 [PATCH RFC 0/2] tpm: Minor improvements Stefan Wahren 2024-09-06 6:09 ` [PATCH RFC 1/2] tpm: tpm_tis_spi: Ensure SPI mode 0 Stefan Wahren @ 2024-09-06 6:09 ` Stefan Wahren 2024-09-06 9:50 ` Jarkko Sakkinen 1 sibling, 1 reply; 9+ messages in thread From: Stefan Wahren @ 2024-09-06 6:09 UTC (permalink / raw) To: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe Cc: linux-integrity, linux-kernel, Stefan Berger, Stefan Wahren In case of SPI wiring issues (e.g. MISO pin permanent high) the core init won't provide any error message on failure. Add some helpful error messages to tpm_tis_core_init(), which helps to better understand why the driver won't probe. Signed-off-by: Stefan Wahren <wahrenst@gmx.net> --- drivers/char/tpm/tpm_tis_core.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index fdef214b9f6b..830e331fcebe 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -1133,8 +1133,10 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, dev_set_drvdata(&chip->dev, priv); rc = tpm_tis_read32(priv, TPM_DID_VID(0), &vendor); - if (rc < 0) + if (rc < 0) { + dev_err(dev, "Failed to read TPM_DID_VID register: %d\n", rc); return rc; + } priv->manufacturer_id = vendor; @@ -1162,19 +1164,25 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, chip->ops->clk_enable(chip, true); if (wait_startup(chip, 0) != 0) { + dev_err(dev, "TPM device not accessible (0x%0x)\n", + vendor); rc = -ENODEV; goto out_err; } /* Take control of the TPM's interrupt hardware and shut it off */ rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask); - if (rc < 0) + if (rc < 0) { + dev_err(dev, "Failed to read TPM_INT_ENABLE register: %d\n", rc); goto out_err; + } /* Figure out the capabilities */ rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps); - if (rc < 0) + if (rc < 0) { + dev_err(dev, "Failed to read TPM_INTF_CAPS register: %d\n", rc); goto out_err; + } dev_dbg(dev, "TPM interface capabilities (0x%x):\n", intfcaps); @@ -1209,6 +1217,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, rc = tpm_tis_request_locality(chip, 0); if (rc < 0) { + dev_err(dev, "Failed to request locality (0x%0x)\n", vendor); rc = -ENODEV; goto out_err; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 2/2] tpm: tpm_tis: Provide error messages in tpm_tis_core_init 2024-09-06 6:09 ` [PATCH RFC 2/2] tpm: tpm_tis: Provide error messages in tpm_tis_core_init Stefan Wahren @ 2024-09-06 9:50 ` Jarkko Sakkinen 2024-09-06 14:42 ` Stefan Wahren 0 siblings, 1 reply; 9+ messages in thread From: Jarkko Sakkinen @ 2024-09-06 9:50 UTC (permalink / raw) To: Stefan Wahren, Peter Huewe, Jason Gunthorpe Cc: linux-integrity, linux-kernel, Stefan Berger On Fri Sep 6, 2024 at 9:09 AM EEST, Stefan Wahren wrote: > In case of SPI wiring issues (e.g. MISO pin permanent high) the > core init won't provide any error message on failure. Add some > helpful error messages to tpm_tis_core_init(), which helps > to better understand why the driver won't probe. > > Signed-off-by: Stefan Wahren <wahrenst@gmx.net> > --- > drivers/char/tpm/tpm_tis_core.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c > index fdef214b9f6b..830e331fcebe 100644 > --- a/drivers/char/tpm/tpm_tis_core.c > +++ b/drivers/char/tpm/tpm_tis_core.c > @@ -1133,8 +1133,10 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, > dev_set_drvdata(&chip->dev, priv); > > rc = tpm_tis_read32(priv, TPM_DID_VID(0), &vendor); > - if (rc < 0) > + if (rc < 0) { > + dev_err(dev, "Failed to read TPM_DID_VID register: %d\n", rc); > return rc; > + } > > priv->manufacturer_id = vendor; > > @@ -1162,19 +1164,25 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, > chip->ops->clk_enable(chip, true); > > if (wait_startup(chip, 0) != 0) { > + dev_err(dev, "TPM device not accessible (0x%0x)\n", > + vendor); > rc = -ENODEV; > goto out_err; > } > > /* Take control of the TPM's interrupt hardware and shut it off */ > rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask); > - if (rc < 0) > + if (rc < 0) { > + dev_err(dev, "Failed to read TPM_INT_ENABLE register: %d\n", rc); > goto out_err; > + } > > /* Figure out the capabilities */ > rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps); > - if (rc < 0) > + if (rc < 0) { > + dev_err(dev, "Failed to read TPM_INTF_CAPS register: %d\n", rc); > goto out_err; > + } > > dev_dbg(dev, "TPM interface capabilities (0x%x):\n", > intfcaps); > @@ -1209,6 +1217,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, > > rc = tpm_tis_request_locality(chip, 0); > if (rc < 0) { > + dev_err(dev, "Failed to request locality (0x%0x)\n", vendor); > rc = -ENODEV; > goto out_err; > } > -- > 2.34.1 No thanks. It not only adds helpful messages but has potential to add unwanted merge conflicts. BR, Jarkko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 2/2] tpm: tpm_tis: Provide error messages in tpm_tis_core_init 2024-09-06 9:50 ` Jarkko Sakkinen @ 2024-09-06 14:42 ` Stefan Wahren 0 siblings, 0 replies; 9+ messages in thread From: Stefan Wahren @ 2024-09-06 14:42 UTC (permalink / raw) To: Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe Cc: linux-integrity, linux-kernel, Stefan Berger Am 06.09.24 um 11:50 schrieb Jarkko Sakkinen: > On Fri Sep 6, 2024 at 9:09 AM EEST, Stefan Wahren wrote: >> In case of SPI wiring issues (e.g. MISO pin permanent high) the >> core init won't provide any error message on failure. Add some >> helpful error messages to tpm_tis_core_init(), which helps >> to better understand why the driver won't probe. >> >> Signed-off-by: Stefan Wahren <wahrenst@gmx.net> >> --- >> drivers/char/tpm/tpm_tis_core.c | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c >> index fdef214b9f6b..830e331fcebe 100644 >> --- a/drivers/char/tpm/tpm_tis_core.c >> +++ b/drivers/char/tpm/tpm_tis_core.c >> @@ -1133,8 +1133,10 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, >> dev_set_drvdata(&chip->dev, priv); >> >> rc = tpm_tis_read32(priv, TPM_DID_VID(0), &vendor); >> - if (rc < 0) >> + if (rc < 0) { >> + dev_err(dev, "Failed to read TPM_DID_VID register: %d\n", rc); >> return rc; >> + } >> >> priv->manufacturer_id = vendor; >> >> @@ -1162,19 +1164,25 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, >> chip->ops->clk_enable(chip, true); >> >> if (wait_startup(chip, 0) != 0) { >> + dev_err(dev, "TPM device not accessible (0x%0x)\n", >> + vendor); >> rc = -ENODEV; >> goto out_err; >> } >> >> /* Take control of the TPM's interrupt hardware and shut it off */ >> rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask); >> - if (rc < 0) >> + if (rc < 0) { >> + dev_err(dev, "Failed to read TPM_INT_ENABLE register: %d\n", rc); >> goto out_err; >> + } >> >> /* Figure out the capabilities */ >> rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps); >> - if (rc < 0) >> + if (rc < 0) { >> + dev_err(dev, "Failed to read TPM_INTF_CAPS register: %d\n", rc); >> goto out_err; >> + } >> >> dev_dbg(dev, "TPM interface capabilities (0x%x):\n", >> intfcaps); >> @@ -1209,6 +1217,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, >> >> rc = tpm_tis_request_locality(chip, 0); >> if (rc < 0) { >> + dev_err(dev, "Failed to request locality (0x%0x)\n", vendor); >> rc = -ENODEV; >> goto out_err; >> } >> -- >> 2.34.1 > No thanks. It not only adds helpful messages but has potential to add > unwanted merge conflicts. So this is a general NAK? So with every setup where the driver exits without an error message, the developer should add its own debug? > > BR, Jarkko ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-09-09 17:16 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-06 6:09 [PATCH RFC 0/2] tpm: Minor improvements Stefan Wahren 2024-09-06 6:09 ` [PATCH RFC 1/2] tpm: tpm_tis_spi: Ensure SPI mode 0 Stefan Wahren 2024-09-06 9:47 ` Jarkko Sakkinen 2024-09-06 14:46 ` Stefan Wahren 2024-09-09 17:05 ` Jarkko Sakkinen 2024-09-09 17:16 ` Stefan Wahren 2024-09-06 6:09 ` [PATCH RFC 2/2] tpm: tpm_tis: Provide error messages in tpm_tis_core_init Stefan Wahren 2024-09-06 9:50 ` Jarkko Sakkinen 2024-09-06 14:42 ` Stefan Wahren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox