From: Jon Hunter <jonathanh@nvidia.com>
To: Stephen Warren <swarren@wwwdotorg.org>,
Thierry Reding <thierry.reding@gmail.com>,
Alexandre Courbot <gnurou@gmail.com>
Cc: <linux-tegra@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
Jon Hunter <jonathanh@nvidia.com>
Subject: [PATCH 6/6] soc/tegra: pmc: Don't probe pmc if early initialisation fails
Date: Tue, 28 Jun 2016 11:38:28 +0100 [thread overview]
Message-ID: <1467110308-22126-7-git-send-email-jonathanh@nvidia.com> (raw)
In-Reply-To: <1467110308-22126-1-git-send-email-jonathanh@nvidia.com>
Commit 0259f522e04f ('soc/tegra: pmc: Restore base address on probe
failure') fixes an issue where the PMC base address pointer is not
restored on probe failure. However, this fix creates another problem
where if early initialisation of the PMC driver fails and an initial
mapping for the PMC address space is not created, then when the PMC
device is probed, the PMC base address pointer will not be valid and
this will cause a crash when tegra_pmc_init() is called and attempts
to access a register.
Although the PMC address space is mapped a 2nd time during the probe
and so this could be fixed by populating the base address pointer
earlier during the probe, this adds more complexity to the code.
Moreover, the PMC probe also assumes the the soc data pointer is also
initialised when the device is probed and if not will also lead to a
crash when calling tegra_pmc_init_tsense_reset(). Given that if the
early initialisation does fail then something bad has happen, it seems
acceptable to allow the PMC device probe to fail as well. Therefore, if
the PMC base address pointer or soc data pointer are not valid when
probing the PMC device, WARN and return an error.
Fixes: 0259f522e04f ('soc/tegra: pmc: Restore base address on probe failure')
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
I am not too happy with this. The more I was looking at the transition
from the early init code to the driver probe the more holes I saw. I
tried to see if I could handle this better in the probe itself, but it
was adding a lot more code to the probe. So this was a simple way to
at least make it more robust.
If this approach is not acceptable, then how about if we simplify matters
in the probe, and if the probe fails then set the 'pmc->base' and 'pmc->soc'
pointers to NULL and not worry about restoring the initial base address?
Yes it may prevent users from controlling the powergates, but if the probe
fails then there are probably other problems and so may be we don't care.
drivers/soc/tegra/pmc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e62acaef140a..d5635db78b61 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -1228,6 +1228,14 @@ static int tegra_pmc_probe(struct platform_device *pdev)
struct resource *res;
int err;
+ /*
+ * Early initialisation should have configured an initial
+ * register mapping and setup the soc data pointer. If these
+ * are not valid then something went badly wrong!
+ */
+ if (WARN_ON(!pmc->base || !pmc->soc))
+ return -ENODEV;
+
err = tegra_pmc_parse_dt(pmc, pdev->dev.of_node);
if (err < 0)
return err;
--
2.1.4
next prev parent reply other threads:[~2016-06-28 10:44 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-28 10:38 [PATCH 0/6] soc/tegra: Various PMC fixes Jon Hunter
2016-06-28 10:38 ` [PATCH 1/6] soc/tegra: pmc: Ensure powergate is available when powering on Jon Hunter
2016-06-28 10:38 ` [PATCH 2/6] soc/tegra: pmc: Fix early initialisation of PMC Jon Hunter
2016-06-29 16:17 ` Jon Hunter
2016-06-30 10:03 ` Thierry Reding
2016-06-28 10:38 ` [PATCH 3/6] soc/tegra: pmc: Don't populate soc data until register space is mapped Jon Hunter
2016-06-28 10:38 ` [PATCH 4/6] soc/tegra: pmc: Ensure mutex is always initialised Jon Hunter
2016-06-28 10:38 ` [PATCH 5/6] soc/tegra: pmc: Add missing of_node_put Jon Hunter
2016-06-28 10:46 ` Jon Hunter
2016-06-28 10:38 ` Jon Hunter [this message]
2016-06-28 10:40 ` [PATCH 0/6] soc/tegra: Various PMC fixes Jon Hunter
2016-06-30 10:04 ` Thierry Reding
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=1467110308-22126-7-git-send-email-jonathanh@nvidia.com \
--to=jonathanh@nvidia.com \
--cc=gnurou@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=swarren@wwwdotorg.org \
--cc=thierry.reding@gmail.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome