From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: martin.wilck@ts.fujitsu.com
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
tpmdd-devel@lists.sourceforge.net
Subject: Re: [PATCH v2] base/platform: return success when probe function is NULL
Date: Tue, 1 Dec 2015 21:30:24 +0100 [thread overview]
Message-ID: <20151201203024.GI5072@pengutronix.de> (raw)
In-Reply-To: <1448884205-17989-1-git-send-email-martin.wilck@ts.fujitsu.com>
Hello,
On Mon, Nov 30, 2015 at 12:50:05PM +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 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).
I'm a bit uncertain if I should be happy with this wording or not. While
b8b2c7d845d5 has a bug which made the tpm_tis driver provoke a panic and
which needs fixing, this doesn't mean that the tpm_tis driver was right.
So "This may cause problems later ..." isn't a correct justification
because even in the presence of the bug introduced by b8b2c7d845d5, the
tpm_tis driver must not assume that it's device is bound on return of
platform_device_register_simple.
It's just that the bug in b8b2c7d845d5 made a wrong assumption of the
tpm_tis driver obvious, and now both need fixing.
If this were my patch, I'd not talk about tpm_tis at all. b8b2c7d845d5
broke binding for platform drivers with no probe function in certain
situations. This is corrected here.
> 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.
This must go after the triple-dash below to not be included in the
commit.
> Fixes: b8b2c7d845d5 ("base/platform: assert that dev_pm_domain callbacks are called unconditionally")
> Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> ---
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
next prev parent reply other threads:[~2015-12-01 20:30 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-26 19:01 [PATCH] base/platform: fix panic when probe function is NULL martin.wilck
2015-11-26 20:30 ` [tpmdd-devel] " Jason Gunthorpe
2015-11-27 7:32 ` Wilck, Martin
2015-11-28 16:40 ` Jarkko Sakkinen
2015-11-28 16:49 ` Jarkko Sakkinen
2015-11-28 22:52 ` Jason Gunthorpe
2015-11-30 12:50 ` Jarkko Sakkinen
2015-11-27 10:11 ` Uwe Kleine-König
2015-11-30 7:42 ` Wilck, Martin
2015-11-30 11:50 ` [PATCH v2] base/platform: return success " martin.wilck
2015-12-01 20:30 ` Uwe Kleine-König [this message]
2015-12-01 10:41 ` [PATCH] base/platform: fix panic " Wilck, Martin
2015-12-01 13:24 ` Uwe Kleine-König
2015-12-01 15:19 ` Wilck, Martin
2015-12-01 17:25 ` [tpmdd-devel] " Jason Gunthorpe
2015-12-01 18:26 ` Peter Huewe
2015-12-01 18:40 ` Jason Gunthorpe
2015-12-01 18:54 ` Aw: " Peter Huewe
2015-12-01 19:03 ` Jason Gunthorpe
2015-11-28 16:34 ` Jarkko Sakkinen
2015-11-29 9:54 ` Uwe Kleine-König
2015-11-30 12:56 ` Jarkko Sakkinen
2015-11-30 13:06 ` Jarkko Sakkinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151201203024.GI5072@pengutronix.de \
--to=u.kleine-koenig@pengutronix.de \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.wilck@ts.fujitsu.com \
--cc=tpmdd-devel@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.