Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1278404 > unrolled thread

[PATCH] base/platform: fix panic when probe function is NULL

Back to article view | Back to linux.kernel

Contents

  [PATCH] base/platform: fix panic when probe function is NULL martin.wilck@ts.fujitsu.com - 2015-11-26 20:20 +0100
    Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-11-26 21:40 +0100
      Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL "Wilck, Martin" <martin.wilck@ts.fujitsu.com> - 2015-11-27 08:40 +0100
      Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-11-28 17:50 +0100
      Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-11-28 17:50 +0100
        Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-11-29 00:00 +0100
          Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-11-30 14:00 +0100
    Re: [PATCH] base/platform: fix panic when probe function is NULL Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2015-11-27 11:20 +0100
      Re: [PATCH] base/platform: fix panic when probe function is NULL "Wilck, Martin" <martin.wilck@ts.fujitsu.com> - 2015-11-30 08:50 +0100
      [PATCH v2] base/platform: return success when probe function is NULL martin.wilck@ts.fujitsu.com - 2015-11-30 13:00 +0100
    Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-11-28 17:40 +0100
      Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2015-11-29 11:00 +0100
        Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-11-30 14:00 +0100
          Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe  function is NULL Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-11-30 14:10 +0100

#1278404 — [PATCH] base/platform: fix panic when probe function is NULL

From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>

Since b8b2c7d845d5, platform_drv_probe() is called for all platform
devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
platform_drv_probe() will return the error code from dev_pm_domain_attach().

This causes real_probe() to enter the "probe_failed" path and set
dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
success if both dev->bus->probe and drv->probe are missing.

This may cause a panic later. For example, inserting the tpm_tis
driver with parameter "force=1" (i.e. registering tpm_tis as a platform
driver) will panic in tpmm_chip_alloc() because dev->driver is NULL:

     chip->cdev.owner = chip->pdev->driver->owner;

This patch fixes this by returning success in platform_drv_probe() if
"just" dev_pm_domain_attach() had failed. This restores the semantics
of platform_device_register_XXX() if the associated platform driver has
no "probe" function.

Fixes: b8b2c7d845d5 ("base/platform: assert that dev_pm_domain
callbacks are called unconditionally")

Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
---
 drivers/base/platform.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 1dd6d3b..c994e76 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -513,10 +513,14 @@ static int platform_drv_probe(struct device *_dev)
 		return ret;
 
 	ret = dev_pm_domain_attach(_dev, true);
-	if (ret != -EPROBE_DEFER && drv->probe) {
-		ret = drv->probe(dev);
-		if (ret)
-			dev_pm_domain_detach(_dev, true);
+	if (ret != -EPROBE_DEFER) {
+		if (drv->probe) {
+			ret = drv->probe(dev);
+			if (ret)
+				dev_pm_domain_detach(_dev, true);
+		} else
+			/* don't fail if just dev_pm_domain_attach failed */
+			ret = 0;
 	}
 
 	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1278430 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

On Thu, Nov 26, 2015 at 08:01:34PM +0100, martin.wilck@ts.fujitsu.com wrote:
> From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> 
> Since b8b2c7d845d5, platform_drv_probe() is called for all platform
> devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
> platform_drv_probe() will return the error code from dev_pm_domain_attach().
> 
> This causes real_probe() to enter the "probe_failed" path and set
> dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
> success if both dev->bus->probe and drv->probe are missing.
> 
> This may cause a panic later. For example, inserting the tpm_tis
> driver with parameter "force=1" (i.e. registering tpm_tis as a platform
> driver) will panic in tpmm_chip_alloc() because dev->driver is NULL:
> 
>      chip->cdev.owner = chip->pdev->driver->owner;

Is this happening because tpm_tis is not creating the platform device
properly? ie it just calls platform_device_register_simple and then
force initializes it via tpm_tis_init, which expects to be called from
a probe function with an attached driver.

Instead we should setup a proper platform device with the default
IO range for x86 and let the driver core call tpm_tis_init via
tis_drv.probe.

Would changing things in this way fix the problem you've observed?

I have some patches to do this that are part of my OF enablement
series, but I can make something simpler that would deal with this
fairly quickly if you can test.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278581 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

T24gRG8sIDIwMTUtMTEtMjYgYXQgMTM6MzAgLTA3MDAsIEphc29uIEd1bnRob3JwZSB3cm90ZToK
PiBPbiBUaHUsIE5vdiAyNiwgMjAxNSBhdCAwODowMTozNFBNICswMTAwLCBtYXJ0aW4ud2lsY2tA
dHMuZnVqaXRzdS5jb20gd3JvdGU6Cj4gPiBGcm9tOiBNYXJ0aW4gV2lsY2sgPE1hcnRpbi5XaWxj
a0B0cy5mdWppdHN1LmNvbT4KPiA+IAo+ID4gU2luY2UgYjhiMmM3ZDg0NWQ1LCBwbGF0Zm9ybV9k
cnZfcHJvYmUoKSBpcyBjYWxsZWQgZm9yIGFsbCBwbGF0Zm9ybQo+ID4gZGV2aWNlcy4gSWYgZHJ2
LT5wcm9iZSBpcyBOVUxMLCBhbmQgZGV2X3BtX2RvbWFpbl9hdHRhY2goKSBmYWlscywKPiA+IHBs
YXRmb3JtX2Rydl9wcm9iZSgpIHdpbGwgcmV0dXJuIHRoZSBlcnJvciBjb2RlIGZyb20gZGV2X3Bt
X2RvbWFpbl9hdHRhY2goKS4KPiA+IAo+ID4gVGhpcyBjYXVzZXMgcmVhbF9wcm9iZSgpIHRvIGVu
dGVyIHRoZSAicHJvYmVfZmFpbGVkIiBwYXRoIGFuZCBzZXQKPiA+IGRldi0+ZHJpdmVyIHRvIE5V
TEwuIEJlZm9yZSBiOGIyYzdkODQ1ZDUsIHJlYWxfcHJvYmUoKSB3b3VsZCBhc3N1bWUKPiA+IHN1
Y2Nlc3MgaWYgYm90aCBkZXYtPmJ1cy0+cHJvYmUgYW5kIGRydi0+cHJvYmUgYXJlIG1pc3Npbmcu
Cj4gPiAKPiA+IFRoaXMgbWF5IGNhdXNlIGEgcGFuaWMgbGF0ZXIuIEZvciBleGFtcGxlLCBpbnNl
cnRpbmcgdGhlIHRwbV90aXMKPiA+IGRyaXZlciB3aXRoIHBhcmFtZXRlciAiZm9yY2U9MSIgKGku
ZS4gcmVnaXN0ZXJpbmcgdHBtX3RpcyBhcyBhIHBsYXRmb3JtCj4gPiBkcml2ZXIpIHdpbGwgcGFu
aWMgaW4gdHBtbV9jaGlwX2FsbG9jKCkgYmVjYXVzZSBkZXYtPmRyaXZlciBpcyBOVUxMOgo+ID4g
Cj4gPiAgICAgIGNoaXAtPmNkZXYub3duZXIgPSBjaGlwLT5wZGV2LT5kcml2ZXItPm93bmVyOwo+
IAo+IElzIHRoaXMgaGFwcGVuaW5nIGJlY2F1c2UgdHBtX3RpcyBpcyBub3QgY3JlYXRpbmcgdGhl
IHBsYXRmb3JtIGRldmljZQo+IHByb3Blcmx5PyBpZSBpdCBqdXN0IGNhbGxzIHBsYXRmb3JtX2Rl
dmljZV9yZWdpc3Rlcl9zaW1wbGUgYW5kIHRoZW4KPiBmb3JjZSBpbml0aWFsaXplcyBpdCB2aWEg
dHBtX3Rpc19pbml0LCB3aGljaCBleHBlY3RzIHRvIGJlIGNhbGxlZCBmcm9tCj4gYSBwcm9iZSBm
dW5jdGlvbiB3aXRoIGFuIGF0dGFjaGVkIGRyaXZlci4KPiAKPiBJbnN0ZWFkIHdlIHNob3VsZCBz
ZXR1cCBhIHByb3BlciBwbGF0Zm9ybSBkZXZpY2Ugd2l0aCB0aGUgZGVmYXVsdAo+IElPIHJhbmdl
IGZvciB4ODYgYW5kIGxldCB0aGUgZHJpdmVyIGNvcmUgY2FsbCB0cG1fdGlzX2luaXQgdmlhCj4g
dGlzX2Rydi5wcm9iZS4KPiAKPiBXb3VsZCBjaGFuZ2luZyB0aGluZ3MgaW4gdGhpcyB3YXkgZml4
IHRoZSBwcm9ibGVtIHlvdSd2ZSBvYnNlcnZlZD8KCkkgdGhpbmsgc28uIE5vbmV0aGVsZXNzLCBw
YXRjaCBiOGIyYzdkODQ1ZDUgaW50cm9kdWNlZCBhIGNoYW5nZSBpbiB0aGUKd2F5IHBsYXRmb3Jt
IGRldmljZSByZWdpc3RyYXRpb24gYmVoYXZlcy4gVGhlIHBsYXRmb3JtIGRldmljZSBjb2RlIHNl
ZW1zCnRvIGJlIHByZXBhcmVkIGZvciBjYXNlcyB3aGVyZSBwbGF0Zm9ybV9kcml2ZXItPnByb2Jl
ID09IE5VTEwsIHNvIHRoYXQKY2FzZSBzaG91bGQgYmUgaGFuZGxlZCBncmFjZWZ1bGx5LiBPdGhl
cndpc2UsIGZhaWx1cmUgc2hvdWxkIG9jY3VyCmVhcmxpZXIsIGUuZy4gd2hlbiBwbGF0Zm9ybV9k
cml2ZXJfcmVnaXN0ZXIoKSBpcyBjYWxsZWQgd2l0aCAKcGxhdGZvcm1fZHJpdmVyLT5wcm9iZSA9
PSBOVUxMLiB0cG1fdGlzIG1heSBub3QgYmUgdGhlIG9ubHkgZHJpdmVyIHRoYXQKdXNlcyBwbGF0
Zm9ybV9kZXZpY2VfcmVnaXN0ZXJfc2ltcGxlKCkgaW4gdGhpcyB3YXkuCgo+IEkgaGF2ZSBzb21l
IHBhdGNoZXMgdG8gZG8gdGhpcyB0aGF0IGFyZSBwYXJ0IG9mIG15IE9GIGVuYWJsZW1lbnQKPiBz
ZXJpZXMsIGJ1dCBJIGNhbiBtYWtlIHNvbWV0aGluZyBzaW1wbGVyIHRoYXQgd291bGQgZGVhbCB3
aXRoIHRoaXMKPiBmYWlybHkgcXVpY2tseSBpZiB5b3UgY2FuIHRlc3QuCgpMZXQncyBmaXJzdCB3
YWl0IHdoYXQgdGhlIHBsYXRmb3JtIGd1eXMgc2F5LgoKTWFydGluCgo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279176 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

On Sat, Nov 28, 2015 at 06:40:03PM +0200, Jarkko Sakkinen wrote:
> On Thu, Nov 26, 2015 at 01:30:31PM -0700, Jason Gunthorpe wrote:
> > On Thu, Nov 26, 2015 at 08:01:34PM +0100, martin.wilck@ts.fujitsu.com wrote:
> > > From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> > > 
> > > Since b8b2c7d845d5, platform_drv_probe() is called for all platform
> > > devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
> > > platform_drv_probe() will return the error code from dev_pm_domain_attach().
> > > 
> > > This causes real_probe() to enter the "probe_failed" path and set
> > > dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
> > > success if both dev->bus->probe and drv->probe are missing.
> > > 
> > > This may cause a panic later. For example, inserting the tpm_tis
> > > driver with parameter "force=1" (i.e. registering tpm_tis as a platform
> > > driver) will panic in tpmm_chip_alloc() because dev->driver is NULL:
> > > 
> > >      chip->cdev.owner = chip->pdev->driver->owner;
> > 
> > Is this happening because tpm_tis is not creating the platform device
> > properly? ie it just calls platform_device_register_simple and then
> > force initializes it via tpm_tis_init, which expects to be called from
> > a probe function with an attached driver.
> 
> Agreed. We should have a probe callback.
> 
> > Instead we should setup a proper platform device with the default
> > IO range for x86 and let the driver core call tpm_tis_init via
> > tis_drv.probe.
> > 
> > Would changing things in this way fix the problem you've observed?
> > 
> > I have some patches to do this that are part of my OF enablement
> > series, but I can make something simpler that would deal with this
> > fairly quickly if you can test.
> 
> Does the patch set that you sent include the fix or not? I haven't yet
> reviewed them properly.

Another question: does you patch series include an alternative fix for
the probe bug or should I just pick Martins fix? As I sad previously I
was seriously lost with the race but now I understand what you and
Martin were saying (and feel utterly stupid + ashamed!). Now I'm just
thinking, which fix I should pick.

Anyway, I'll try to go through your code ASAP.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279177 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

On Thu, Nov 26, 2015 at 01:30:31PM -0700, Jason Gunthorpe wrote:
> On Thu, Nov 26, 2015 at 08:01:34PM +0100, martin.wilck@ts.fujitsu.com wrote:
> > From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> > 
> > Since b8b2c7d845d5, platform_drv_probe() is called for all platform
> > devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
> > platform_drv_probe() will return the error code from dev_pm_domain_attach().
> > 
> > This causes real_probe() to enter the "probe_failed" path and set
> > dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
> > success if both dev->bus->probe and drv->probe are missing.
> > 
> > This may cause a panic later. For example, inserting the tpm_tis
> > driver with parameter "force=1" (i.e. registering tpm_tis as a platform
> > driver) will panic in tpmm_chip_alloc() because dev->driver is NULL:
> > 
> >      chip->cdev.owner = chip->pdev->driver->owner;
> 
> Is this happening because tpm_tis is not creating the platform device
> properly? ie it just calls platform_device_register_simple and then
> force initializes it via tpm_tis_init, which expects to be called from
> a probe function with an attached driver.

Agreed. We should have a probe callback.

> Instead we should setup a proper platform device with the default
> IO range for x86 and let the driver core call tpm_tis_init via
> tis_drv.probe.
> 
> Would changing things in this way fix the problem you've observed?
> 
> I have some patches to do this that are part of my OF enablement
> series, but I can make something simpler that would deal with this
> fairly quickly if you can test.

Does the patch set that you sent include the fix or not? I haven't yet
reviewed them properly.

> Jason

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279243 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

On Sat, Nov 28, 2015 at 06:40:03PM +0200, Jarkko Sakkinen wrote:
> > I have some patches to do this that are part of my OF enablement
> > series, but I can make something simpler that would deal with this
> > fairly quickly if you can test.
> 
> Does the patch set that you sent include the fix or not? I haven't yet
> reviewed them properly.

No fixing probe is another task. I can send some patches for that when
we are done with the IRQ stuff. That is something we should fix no
matter what..

BTW, please test my IRQ series, I forgot to mention I was unable to
test it properly here...

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279845 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

On Sat, Nov 28, 2015 at 03:52:51PM -0700, Jason Gunthorpe wrote:
> On Sat, Nov 28, 2015 at 06:40:03PM +0200, Jarkko Sakkinen wrote:
> > > I have some patches to do this that are part of my OF enablement
> > > series, but I can make something simpler that would deal with this
> > > fairly quickly if you can test.
> > 
> > Does the patch set that you sent include the fix or not? I haven't yet
> > reviewed them properly.
> 
> No fixing probe is another task. I can send some patches for that when
> we are done with the IRQ stuff. That is something we should fix no
> matter what..
> 
> BTW, please test my IRQ series, I forgot to mention I was unable to
> test it properly here...

Got you. I need to at least test insmod/rmod (maybe couple of times in a
cycle).  Do you see any other code paths that could easily break because
of your patches?

> Jason

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278700

Hello Martin,

On Thu, Nov 26, 2015 at 08:01:34PM +0100, martin.wilck@ts.fujitsu.com wrote:
> From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> 
> Since b8b2c7d845d5, platform_drv_probe() is called for all platform
> devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
> platform_drv_probe() will return the error code from dev_pm_domain_attach().

Correct, this is an unintended change of behaviour introduced in
b8b2c7d845d5.

> This causes real_probe() to enter the "probe_failed" path and set
> dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
> success if both dev->bus->probe and drv->probe are missing.
> 
> This may cause a panic later. For example, inserting the tpm_tis
> driver with parameter "force=1" (i.e. registering tpm_tis as a platform
> driver) will panic in tpmm_chip_alloc() because dev->driver is NULL:
> 
>      chip->cdev.owner = chip->pdev->driver->owner;

This sounds like a separate issue though. Looking at init_tis there is:

        rc = platform_driver_register(&tis_drv);
        if (rc < 0)
                return rc;
        pdev = platform_device_register_simple("tpm_tis", -1, NULL, 0);
        if (IS_ERR(pdev)) {
                rc = PTR_ERR(pdev);
                goto err_dev;
        }
        rc = tpm_tis_init(&pdev->dev, &tis_default_info, NULL);

tpm_tis_init calls tpmm_chip_alloc which barfs when pdev (i.e. the return value
of platform_device_register_simple above) isn't bound. It is not allowed
to assume that the device is bound after the above function calls.

So I'd say drop the paragraph about tpm_tis and the change is fine.

> This patch fixes this by returning success in platform_drv_probe() if
> "just" dev_pm_domain_attach() had failed. This restores the semantics
> of platform_device_register_XXX() if the associated platform driver has
> no "probe" function.
> 
> Fixes: b8b2c7d845d5 ("base/platform: assert that dev_pm_domain
> callbacks are called unconditionally")
> 

I think line breaks in the Fixes: line are frowned on. Also usually
there is no empty line between Fixes: and S-o-b:.

> Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> ---
>  drivers/base/platform.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 1dd6d3b..c994e76 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -513,10 +513,14 @@ static int platform_drv_probe(struct device *_dev)
>  		return ret;
>  
>  	ret = dev_pm_domain_attach(_dev, true);
> -	if (ret != -EPROBE_DEFER && drv->probe) {
> -		ret = drv->probe(dev);
> -		if (ret)
> -			dev_pm_domain_detach(_dev, true);
> +	if (ret != -EPROBE_DEFER) {
> +		if (drv->probe) {
> +			ret = drv->probe(dev);
> +			if (ret)
> +				dev_pm_domain_detach(_dev, true);
> +		} else
> +			/* don't fail if just dev_pm_domain_attach failed */
> +			ret = 0;

An else that has a } should also have a {, according to 
checkpatch and Documentation/CodingStyle. You can write it
alternatively as:

	if (ret != -EPROBE_DEFER) {
		if (drv->probe)
			ret = drv->probe(dev);
		else
			ret = 0;

		if (ret)
			dev_pm_domain_detach(_dev, true);
	}

.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279650

SGVsbG8gVXdlLAoKdGhhbmtzIGZvciB5b3VyIHJldmlldy4KCj4gVGhpcyBtYXkgY2F1c2UgYSBw
YW5pYyBsYXRlci4gRm9yIGV4YW1wbGUsIGluc2VydGluZyB0aGUgdHBtX3Rpcwo+ID4gZHJpdmVy
IHdpdGggcGFyYW1ldGVyICJmb3JjZT0xIiAoaS5lLiByZWdpc3RlcmluZyB0cG1fdGlzIGFzIGEg
cGxhdGZvcm0KPiA+IGRyaXZlcikgd2lsbCBwYW5pYyBpbiB0cG1tX2NoaXBfYWxsb2MoKSBiZWNh
dXNlIGRldi0+ZHJpdmVyIGlzIE5VTEw6Cj4gPiAKPiA+ICAgICAgY2hpcC0+Y2Rldi5vd25lciA9
IGNoaXAtPnBkZXYtPmRyaXZlci0+b3duZXI7Cj4gCj4gVGhpcyBzb3VuZHMgbGlrZSBhIHNlcGFy
YXRlIGlzc3VlIHRob3VnaC4gTG9va2luZyBhdCBpbml0X3RpcyB0aGVyZSBpczoKPiAKPiAgICAg
ICAgIHJjID0gcGxhdGZvcm1fZHJpdmVyX3JlZ2lzdGVyKCZ0aXNfZHJ2KTsKPiAgICAgICAgIGlm
IChyYyA8IDApCj4gICAgICAgICAgICAgICAgIHJldHVybiByYzsKPiAgICAgICAgIHBkZXYgPSBw
bGF0Zm9ybV9kZXZpY2VfcmVnaXN0ZXJfc2ltcGxlKCJ0cG1fdGlzIiwgLTEsIE5VTEwsIDApOwo+
ICAgICAgICAgaWYgKElTX0VSUihwZGV2KSkgewo+ICAgICAgICAgICAgICAgICByYyA9IFBUUl9F
UlIocGRldik7Cj4gICAgICAgICAgICAgICAgIGdvdG8gZXJyX2RldjsKPiAgICAgICAgIH0KPiAg
ICAgICAgIHJjID0gdHBtX3Rpc19pbml0KCZwZGV2LT5kZXYsICZ0aXNfZGVmYXVsdF9pbmZvLCBO
VUxMKTsKPiAKPiB0cG1fdGlzX2luaXQgY2FsbHMgdHBtbV9jaGlwX2FsbG9jIHdoaWNoIGJhcmZz
IHdoZW4gcGRldiAoaS5lLiB0aGUgcmV0dXJuIHZhbHVlCj4gb2YgcGxhdGZvcm1fZGV2aWNlX3Jl
Z2lzdGVyX3NpbXBsZSBhYm92ZSkgaXNuJ3QgYm91bmQuIEl0IGlzIG5vdCBhbGxvd2VkCj4gdG8g
YXNzdW1lIHRoYXQgdGhlIGRldmljZSBpcyBib3VuZCBhZnRlciB0aGUgYWJvdmUgZnVuY3Rpb24g
Y2FsbHMuCgpJIGFncmVlIHRoYXQgdGhlIFRQTSBwbGF0Zm9ybSBkZXZpY2UgY29kZSBkZXNlcnZl
cyBpbXByb3ZlbWVudC4gSmFzb24Kd3JvdGUgdGhhdCBoZSBoYXMgYWxyZWFkeSBzb21lIHBhdGNo
ZXMgYXZhaWxhYmxlIGZvciB0aGF0LgoKSSBsYWNrIHRoZSBrbm93bGVkZ2UgdG8ganVkZ2Ugd2hl
dGhlciBvciBub3QgdHBtX2lzX2luaXQncyBhc3N1bXB0aW9uCndhcyBjb3JyZWN0LiBCdXQsIG1h
eWJlIGp1c3QgYnkgbHVjaywgdGhpcyBhc3N1bXB0aW9uIHVzZWQgdG8gYmUgKnRydWUqCnVudGls
IHBhdGNoIGI4YjJjN2Q4NDVkNS4gRHJpdmVyIGFuZCBkZXZpY2Ugd2VyZSBtYXRjaGVkIGJ5IG5h
bWUKKCJ0cG1fdGlzIikgYnkgdGhlIHBsYXRmb3JtIGRyaXZlciBwcm9iaW5nIGNvZGUsIGFuZCBk
ZXZpY2UgYW5kIGRyaXZlcgp3ZXJlIGFjdHVhbGx5IGJvdW5kIHRvIGVhY2ggb3RoZXIgYWZ0ZXIg
dGhpcyBzZXF1ZW5jZSBvZiBjYWxscy4gCgo+IFNvIEknZCBzYXkgZHJvcCB0aGUgcGFyYWdyYXBo
IGFib3V0IHRwbV90aXMgYW5kIHRoZSBjaGFuZ2UgaXMgZmluZS4KCkkgZGlkbid0IG1lYW4gdG8g
YmxhbWUgeW91ciBwYXRjaC4gQnV0IGEgbm90ZSBhYm91dCB0aGUgcGFuaWMgbWlnaHQgYmUKaGVs
cGZ1bCBqdXN0IGluIGNhc2Ugc29tZW9uZSBlbHNlIHJ1bnMgaW50byB0aGUgc2FtZSBwcm9ibGVt
LiBUaGUKY29ubmVjdGlvbiBiZXR3ZWVuIHlvdXIgcGF0Y2ggYW5kIHRwbV90aXMgbG9hZGluZyBp
cyBmYXIgZnJvbSBvYnZpb3VzLgpJIG1lbnRpb25lZCB0aGUgcGFuaWMgaW4gb3JkZXIgdG8gY2xh
cmlmeSB0aGF0IHRoaXMgd2Fzbid0IGp1c3QgYQp0aGVvcmV0aWNhbCBpc3N1ZS4KCkFueXdheSwg
SSdsbCByZXN1Ym1pdCB3aXRoIHlvdXIgc3R5bGUgaGludHMgYXBwbGllZCBhbmQgd2lsbCB0cnkg
dG8gZmluZAphIHdvcmRpbmcgZm9yIHRoZSBjb21taXQgbWVzc2FnZSB0aGF0IHdlIGNhbiBhZ3Jl
ZSB1cG9uLgoKQmVzdCBSZWdhcmRzLApNYXJ0aW4KCj4gCj4gPiBUaGlzIHBhdGNoIGZpeGVzIHRo
aXMgYnkgcmV0dXJuaW5nIHN1Y2Nlc3MgaW4gcGxhdGZvcm1fZHJ2X3Byb2JlKCkgaWYKPiA+ICJq
dXN0IiBkZXZfcG1fZG9tYWluX2F0dGFjaCgpIGhhZCBmYWlsZWQuIFRoaXMgcmVzdG9yZXMgdGhl
IHNlbWFudGljcwo+ID4gb2YgcGxhdGZvcm1fZGV2aWNlX3JlZ2lzdGVyX1hYWCgpIGlmIHRoZSBh
c3NvY2lhdGVkIHBsYXRmb3JtIGRyaXZlciBoYXMKPiA+IG5vICJwcm9iZSIgZnVuY3Rpb24uCj4g
PiAKPiA+IEZpeGVzOiBiOGIyYzdkODQ1ZDUgKCJiYXNlL3BsYXRmb3JtOiBhc3NlcnQgdGhhdCBk
ZXZfcG1fZG9tYWluCj4gPiBjYWxsYmFja3MgYXJlIGNhbGxlZCB1bmNvbmRpdGlvbmFsbHkiKQo+
ID4gCj4gCj4gSSB0aGluayBsaW5lIGJyZWFrcyBpbiB0aGUgRml4ZXM6IGxpbmUgYXJlIGZyb3du
ZWQgb24uIEFsc28gdXN1YWxseQo+IHRoZXJlIGlzIG5vIGVtcHR5IGxpbmUgYmV0d2VlbiBGaXhl
czogYW5kIFMtby1iOi4KPiAKPiA+IFNpZ25lZC1vZmYtYnk6IE1hcnRpbiBXaWxjayA8TWFydGlu
LldpbGNrQHRzLmZ1aml0c3UuY29tPgo+ID4gLS0tCj4gPiAgZHJpdmVycy9iYXNlL3BsYXRmb3Jt
LmMgfCAxMiArKysrKysrKy0tLS0KPiA+ICAxIGZpbGUgY2hhbmdlZCwgOCBpbnNlcnRpb25zKCsp
LCA0IGRlbGV0aW9ucygtKQo+ID4gCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9iYXNlL3BsYXRm
b3JtLmMgYi9kcml2ZXJzL2Jhc2UvcGxhdGZvcm0uYwo+ID4gaW5kZXggMWRkNmQzYi4uYzk5NGU3
NiAxMDA2NDQKPiA+IC0tLSBhL2RyaXZlcnMvYmFzZS9wbGF0Zm9ybS5jCj4gPiArKysgYi9kcml2
ZXJzL2Jhc2UvcGxhdGZvcm0uYwo+ID4gQEAgLTUxMywxMCArNTEzLDE0IEBAIHN0YXRpYyBpbnQg
cGxhdGZvcm1fZHJ2X3Byb2JlKHN0cnVjdCBkZXZpY2UgKl9kZXYpCj4gPiAgCQlyZXR1cm4gcmV0
Owo+ID4gIAo+ID4gIAlyZXQgPSBkZXZfcG1fZG9tYWluX2F0dGFjaChfZGV2LCB0cnVlKTsKPiA+
IC0JaWYgKHJldCAhPSAtRVBST0JFX0RFRkVSICYmIGRydi0+cHJvYmUpIHsKPiA+IC0JCXJldCA9
IGRydi0+cHJvYmUoZGV2KTsKPiA+IC0JCWlmIChyZXQpCj4gPiAtCQkJZGV2X3BtX2RvbWFpbl9k
ZXRhY2goX2RldiwgdHJ1ZSk7Cj4gPiArCWlmIChyZXQgIT0gLUVQUk9CRV9ERUZFUikgewo+ID4g
KwkJaWYgKGRydi0+cHJvYmUpIHsKPiA+ICsJCQlyZXQgPSBkcnYtPnByb2JlKGRldik7Cj4gPiAr
CQkJaWYgKHJldCkKPiA+ICsJCQkJZGV2X3BtX2RvbWFpbl9kZXRhY2goX2RldiwgdHJ1ZSk7Cj4g
PiArCQl9IGVsc2UKPiA+ICsJCQkvKiBkb24ndCBmYWlsIGlmIGp1c3QgZGV2X3BtX2RvbWFpbl9h
dHRhY2ggZmFpbGVkICovCj4gPiArCQkJcmV0ID0gMDsKPiAKPiBBbiBlbHNlIHRoYXQgaGFzIGEg
fSBzaG91bGQgYWxzbyBoYXZlIGEgeywgYWNjb3JkaW5nIHRvIAo+IGNoZWNrcGF0Y2ggYW5kIERv
Y3VtZW50YXRpb24vQ29kaW5nU3R5bGUuIFlvdSBjYW4gd3JpdGUgaXQKPiBhbHRlcm5hdGl2ZWx5
IGFzOgo+IAo+IAlpZiAocmV0ICE9IC1FUFJPQkVfREVGRVIpIHsKPiAJCWlmIChkcnYtPnByb2Jl
KQo+IAkJCXJldCA9IGRydi0+cHJvYmUoZGV2KTsKPiAJCWVsc2UKPiAJCQlyZXQgPSAwOwo+IAo+
IAkJaWYgKHJldCkKPiAJCQlkZXZfcG1fZG9tYWluX2RldGFjaChfZGV2LCB0cnVlKTsKPiAJfQo+
IAo+IC4KPiAKPiBCZXN0IHJlZ2FyZHMKPiBVd2UKPiAK
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279802 — [PATCH v2] base/platform: return success when probe function is NULL

From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>

Since b8b2c7d845d5, platform_drv_probe() is called for all platform
devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
platform_drv_probe() will return the error code from dev_pm_domain_attach().

This causes real_probe() to enter the "probe_failed" path and set
dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
success if both dev->bus->probe and drv->probe were missing. As a result,
a device and driver could be "bound" together just by matching their names;
this doesn't work any more after b8b2c7d845d5.

This may cause problems later for certain usage of platform_driver_register()
and platform_device_register_simple(). I observed a panic while loading
the tpm_tis driver with parameter "force=1" (i.e. registering tpm_tis as
a platform driver), because tpm_tis_init's assumption that the device
returned by platform_device_register_simple() was bound didn't hold any more
(tpmm_chip_alloc() dereferences chip->pdev->driver, causing panic).

This patch restores the previous (4.3.0 and earlier) behavior of
platform_drv_probe() in the case when the associated platform driver has
no "probe" function.

v2: fixed style issues, rephrased commit message.

Fixes: b8b2c7d845d5 ("base/platform: assert that dev_pm_domain callbacks are called unconditionally")
Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
---
 drivers/base/platform.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 1dd6d3b..176b59f 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -513,10 +513,15 @@ static int platform_drv_probe(struct device *_dev)
 		return ret;
 
 	ret = dev_pm_domain_attach(_dev, true);
-	if (ret != -EPROBE_DEFER && drv->probe) {
-		ret = drv->probe(dev);
-		if (ret)
-			dev_pm_domain_detach(_dev, true);
+	if (ret != -EPROBE_DEFER) {
+		if (drv->probe) {
+			ret = drv->probe(dev);
+			if (ret)
+				dev_pm_domain_detach(_dev, true);
+		} else {
+			/* don't fail if just dev_pm_domain_attach failed */
+			ret = 0;
+		}
 	}
 
 	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279173 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

On Thu, Nov 26, 2015 at 08:01:34PM +0100, martin.wilck@ts.fujitsu.com wrote:
> From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> 
> Since b8b2c7d845d5, platform_drv_probe() is called for all platform
> devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
> platform_drv_probe() will return the error code from dev_pm_domain_attach().
> 
> This causes real_probe() to enter the "probe_failed" path and set
> dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
> success if both dev->bus->probe and drv->probe are missing.
> 
> This may cause a panic later. For example, inserting the tpm_tis
> driver with parameter "force=1" (i.e. registering tpm_tis as a platform
> driver) will panic in tpmm_chip_alloc() because dev->driver is NULL:
> 
>      chip->cdev.owner = chip->pdev->driver->owner;
> 
> This patch fixes this by returning success in platform_drv_probe() if
> "just" dev_pm_domain_attach() had failed. This restores the semantics
> of platform_device_register_XXX() if the associated platform driver has
> no "probe" function.
> 
> Fixes: b8b2c7d845d5 ("base/platform: assert that dev_pm_domain
> callbacks are called unconditionally")
> 
> Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com>

Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

> ---
>  drivers/base/platform.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 1dd6d3b..c994e76 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -513,10 +513,14 @@ static int platform_drv_probe(struct device *_dev)
>  		return ret;
>  
>  	ret = dev_pm_domain_attach(_dev, true);
> -	if (ret != -EPROBE_DEFER && drv->probe) {
> -		ret = drv->probe(dev);
> -		if (ret)
> -			dev_pm_domain_detach(_dev, true);
> +	if (ret != -EPROBE_DEFER) {
> +		if (drv->probe) {
> +			ret = drv->probe(dev);
> +			if (ret)
> +				dev_pm_domain_detach(_dev, true);
> +		} else
> +			/* don't fail if just dev_pm_domain_attach failed */
> +			ret = 0;
>  	}
>  
>  	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
> -- 
> 1.8.3.1
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279311 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

Hello Jarkko,

On Sat, Nov 28, 2015 at 06:34:47PM +0200, Jarkko Sakkinen wrote:
> On Thu, Nov 26, 2015 at 08:01:34PM +0100, martin.wilck@ts.fujitsu.com wrote:
> > From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> > 
> > Since b8b2c7d845d5, platform_drv_probe() is called for all platform
> > devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
> > platform_drv_probe() will return the error code from dev_pm_domain_attach().
> > 
> > This causes real_probe() to enter the "probe_failed" path and set
> > dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
> > success if both dev->bus->probe and drv->probe are missing.
> > 
> > This may cause a panic later. For example, inserting the tpm_tis
> > driver with parameter "force=1" (i.e. registering tpm_tis as a platform
> > driver) will panic in tpmm_chip_alloc() because dev->driver is NULL:
> > 
> >      chip->cdev.owner = chip->pdev->driver->owner;
> > 
> > This patch fixes this by returning success in platform_drv_probe() if
> > "just" dev_pm_domain_attach() had failed. This restores the semantics
> > of platform_device_register_XXX() if the associated platform driver has
> > no "probe" function.
> > 
> > Fixes: b8b2c7d845d5 ("base/platform: assert that dev_pm_domain
> > callbacks are called unconditionally")
> > 
> > Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> 
> Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

While the patch is fine, the commit log is not. It blames b8b2c7d845d5
to be responsible for a panic, but in fact it only breaks the wrong
assumption of the tpm_tis driver.

So I'm not sure how to interpret your Ack, IMHO it should not make
gregkh pick up the patch as is.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279844 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

Hi Uwe,

On Sun, Nov 29, 2015 at 10:54:11AM +0100, Uwe Kleine-König wrote:
> Hello Jarkko,
> 
> On Sat, Nov 28, 2015 at 06:34:47PM +0200, Jarkko Sakkinen wrote:
> > On Thu, Nov 26, 2015 at 08:01:34PM +0100, martin.wilck@ts.fujitsu.com wrote:
> > > From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> > > 
> > > Since b8b2c7d845d5, platform_drv_probe() is called for all platform
> > > devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
> > > platform_drv_probe() will return the error code from dev_pm_domain_attach().
> > > 
> > > This causes real_probe() to enter the "probe_failed" path and set
> > > dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
> > > success if both dev->bus->probe and drv->probe are missing.
> > > 
> > > This may cause a panic later. For example, inserting the tpm_tis
> > > driver with parameter "force=1" (i.e. registering tpm_tis as a platform
> > > driver) will panic in tpmm_chip_alloc() because dev->driver is NULL:
> > > 
> > >      chip->cdev.owner = chip->pdev->driver->owner;
> > > 
> > > This patch fixes this by returning success in platform_drv_probe() if
> > > "just" dev_pm_domain_attach() had failed. This restores the semantics
> > > of platform_device_register_XXX() if the associated platform driver has
> > > no "probe" function.
> > > 
> > > Fixes: b8b2c7d845d5 ("base/platform: assert that dev_pm_domain
> > > callbacks are called unconditionally")
> > > 
> > > Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> > 
> > Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> While the patch is fine, the commit log is not. It blames b8b2c7d845d5
> to be responsible for a panic, but in fact it only breaks the wrong
> assumption of the tpm_tis driver.
> 
> So I'm not sure how to interpret your Ack, IMHO it should not make
> gregkh pick up the patch as is.

Alright. I don't think you can speak about *wrong assumptions* if the
semantics allowed not to have it before. *Where* it should be fixed is
another question. I'd keep the Fixes tag in all cases.

Jason, you had the fix for this issue directly to tpm_tis driver that
you haven't yet posted, right? Just double-checking this.

> Best regards
> Uwe
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279848 — Re: [tpmdd-devel] [PATCH] base/platform: fix panic when probe function is NULL

On Mon, Nov 30, 2015 at 02:56:31PM +0200, Jarkko Sakkinen wrote:
> Hi Uwe,
> 
> On Sun, Nov 29, 2015 at 10:54:11AM +0100, Uwe Kleine-König wrote:
> > Hello Jarkko,
> > 
> > On Sat, Nov 28, 2015 at 06:34:47PM +0200, Jarkko Sakkinen wrote:
> > > On Thu, Nov 26, 2015 at 08:01:34PM +0100, martin.wilck@ts.fujitsu.com wrote:
> > > > From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> > > > 
> > > > Since b8b2c7d845d5, platform_drv_probe() is called for all platform
> > > > devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
> > > > platform_drv_probe() will return the error code from dev_pm_domain_attach().
> > > > 
> > > > This causes real_probe() to enter the "probe_failed" path and set
> > > > dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
> > > > success if both dev->bus->probe and drv->probe are missing.
> > > > 
> > > > This may cause a panic later. For example, inserting the tpm_tis
> > > > driver with parameter "force=1" (i.e. registering tpm_tis as a platform
> > > > driver) will panic in tpmm_chip_alloc() because dev->driver is NULL:
> > > > 
> > > >      chip->cdev.owner = chip->pdev->driver->owner;
> > > > 
> > > > This patch fixes this by returning success in platform_drv_probe() if
> > > > "just" dev_pm_domain_attach() had failed. This restores the semantics
> > > > of platform_device_register_XXX() if the associated platform driver has
> > > > no "probe" function.
> > > > 
> > > > Fixes: b8b2c7d845d5 ("base/platform: assert that dev_pm_domain
> > > > callbacks are called unconditionally")
> > > > 
> > > > Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> > > 
> > > Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > 
> > While the patch is fine, the commit log is not. It blames b8b2c7d845d5
> > to be responsible for a panic, but in fact it only breaks the wrong
> > assumption of the tpm_tis driver.
> > 
> > So I'm not sure how to interpret your Ack, IMHO it should not make
> > gregkh pick up the patch as is.
> 
> Alright. I don't think you can speak about *wrong assumptions* if the
> semantics allowed not to have it before. *Where* it should be fixed is
> another question. I'd keep the Fixes tag in all cases.
> 
> Jason, you had the fix for this issue directly to tpm_tis driver that
> you haven't yet posted, right? Just double-checking this.

Uwe, please ignore this :) Saw your more in-depth comment about platform
driver creation. Thank you. I somehow have missed it before.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web