* [PATCH v4 2/3] soc: imx8m: Remove global soc_uid
2024-09-29 18:49 [PATCH v4 1/3] soc: imx8m: Probe the SoC driver as platform driver Marek Vasut
@ 2024-09-29 18:49 ` Marek Vasut
2024-09-29 18:49 ` [PATCH v4 3/3] soc: imx8m: Use devm_* to simplify probe failure handling Marek Vasut
2024-10-18 3:14 ` [PATCH v4 1/3] soc: imx8m: Probe the SoC driver as platform driver Shawn Guo
2 siblings, 0 replies; 4+ messages in thread
From: Marek Vasut @ 2024-09-29 18:49 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kernel, Marek Vasut, AngeloGioacchino Del Regno, Arnd Bergmann,
Fabio Estevam, Jeff Johnson, Neil Armstrong,
Pengutronix Kernel Team, Saravana Kannan, Sascha Hauer, Shawn Guo,
imx, linux-kernel
The static global soc_uid is only ever used as kasprintf() parameter in
imx8m_soc_probe(). Pass pointer to local u64 variable to .soc_revision()
callback instead and let the .soc_revision() callback fill in the content.
Remove the unnecessary static global variable.
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Jeff Johnson <quic_jjohnson@quicinc.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Saravana Kannan <saravanak@google.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
V3: New patch
V4: No change
---
drivers/soc/imx/soc-imx8m.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
index 5ea8887828c06..966593320e28d 100644
--- a/drivers/soc/imx/soc-imx8m.c
+++ b/drivers/soc/imx/soc-imx8m.c
@@ -30,11 +30,9 @@
struct imx8_soc_data {
char *name;
- int (*soc_revision)(u32 *socrev);
+ int (*soc_revision)(u32 *socrev, u64 *socuid);
};
-static u64 soc_uid;
-
#ifdef CONFIG_HAVE_ARM_SMCCC
static u32 imx8mq_soc_revision_from_atf(void)
{
@@ -51,7 +49,7 @@ static u32 imx8mq_soc_revision_from_atf(void)
static inline u32 imx8mq_soc_revision_from_atf(void) { return 0; };
#endif
-static int imx8mq_soc_revision(u32 *socrev)
+static int imx8mq_soc_revision(u32 *socrev, u64 *socuid)
{
struct device_node *np;
void __iomem *ocotp_base;
@@ -89,9 +87,9 @@ static int imx8mq_soc_revision(u32 *socrev)
rev = REV_B1;
}
- soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
- soc_uid <<= 32;
- soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
+ *socuid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
+ *socuid <<= 32;
+ *socuid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
*socrev = rev;
@@ -109,7 +107,7 @@ static int imx8mq_soc_revision(u32 *socrev)
return ret;
}
-static int imx8mm_soc_uid(void)
+static int imx8mm_soc_uid(u64 *socuid)
{
void __iomem *ocotp_base;
struct device_node *np;
@@ -136,9 +134,9 @@ static int imx8mm_soc_uid(void)
clk_prepare_enable(clk);
- soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH + offset);
- soc_uid <<= 32;
- soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW + offset);
+ *socuid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH + offset);
+ *socuid <<= 32;
+ *socuid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW + offset);
clk_disable_unprepare(clk);
clk_put(clk);
@@ -151,7 +149,7 @@ static int imx8mm_soc_uid(void)
return ret;
}
-static int imx8mm_soc_revision(u32 *socrev)
+static int imx8mm_soc_revision(u32 *socrev, u64 *socuid)
{
struct device_node *np;
void __iomem *anatop_base;
@@ -172,7 +170,7 @@ static int imx8mm_soc_revision(u32 *socrev)
iounmap(anatop_base);
of_node_put(np);
- return imx8mm_soc_uid();
+ return imx8mm_soc_uid(socuid);
err_iomap:
of_node_put(np);
@@ -215,10 +213,11 @@ static __maybe_unused const struct of_device_id imx8_soc_match[] = {
static int imx8m_soc_probe(struct platform_device *pdev)
{
struct soc_device_attribute *soc_dev_attr;
- struct soc_device *soc_dev;
+ const struct imx8_soc_data *data;
const struct of_device_id *id;
+ struct soc_device *soc_dev;
u32 soc_rev = 0;
- const struct imx8_soc_data *data;
+ u64 soc_uid = 0;
int ret;
soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
@@ -241,7 +240,7 @@ static int imx8m_soc_probe(struct platform_device *pdev)
if (data) {
soc_dev_attr->soc_id = data->name;
if (data->soc_revision) {
- ret = data->soc_revision(&soc_rev);
+ ret = data->soc_revision(&soc_rev, &soc_uid);
if (ret)
goto free_soc;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v4 3/3] soc: imx8m: Use devm_* to simplify probe failure handling
2024-09-29 18:49 [PATCH v4 1/3] soc: imx8m: Probe the SoC driver as platform driver Marek Vasut
2024-09-29 18:49 ` [PATCH v4 2/3] soc: imx8m: Remove global soc_uid Marek Vasut
@ 2024-09-29 18:49 ` Marek Vasut
2024-10-18 3:14 ` [PATCH v4 1/3] soc: imx8m: Probe the SoC driver as platform driver Shawn Guo
2 siblings, 0 replies; 4+ messages in thread
From: Marek Vasut @ 2024-09-29 18:49 UTC (permalink / raw)
To: linux-arm-kernel
Cc: kernel, Marek Vasut, AngeloGioacchino Del Regno, Arnd Bergmann,
Fabio Estevam, Jeff Johnson, Neil Armstrong,
Pengutronix Kernel Team, Saravana Kannan, Sascha Hauer, Shawn Guo,
imx, linux-kernel
Use device managed functions to simplify handling of failures during
probe. Remove fail paths which are no longer necessary.
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Jeff Johnson <quic_jjohnson@quicinc.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Saravana Kannan <saravanak@google.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
V4: New patch
---
drivers/soc/imx/soc-imx8m.c | 92 ++++++++++++-------------------------
1 file changed, 29 insertions(+), 63 deletions(-)
diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
index 966593320e28d..8ac7658e3d525 100644
--- a/drivers/soc/imx/soc-imx8m.c
+++ b/drivers/soc/imx/soc-imx8m.c
@@ -51,22 +51,20 @@ static inline u32 imx8mq_soc_revision_from_atf(void) { return 0; };
static int imx8mq_soc_revision(u32 *socrev, u64 *socuid)
{
- struct device_node *np;
+ struct device_node *np __free(device_node) =
+ of_find_compatible_node(NULL, NULL, "fsl,imx8mq-ocotp");
void __iomem *ocotp_base;
u32 magic;
u32 rev;
struct clk *clk;
int ret;
- np = of_find_compatible_node(NULL, NULL, "fsl,imx8mq-ocotp");
if (!np)
return -EINVAL;
ocotp_base = of_iomap(np, 0);
- if (!ocotp_base) {
- ret = -EINVAL;
- goto err_iomap;
- }
+ if (!ocotp_base)
+ return -EINVAL;
clk = of_clk_get_by_name(np, NULL);
if (IS_ERR(clk)) {
@@ -96,35 +94,30 @@ static int imx8mq_soc_revision(u32 *socrev, u64 *socuid)
clk_disable_unprepare(clk);
clk_put(clk);
iounmap(ocotp_base);
- of_node_put(np);
return 0;
err_clk:
iounmap(ocotp_base);
-err_iomap:
- of_node_put(np);
return ret;
}
static int imx8mm_soc_uid(u64 *socuid)
{
+ struct device_node *np __free(device_node) =
+ of_find_compatible_node(NULL, NULL, "fsl,imx8mm-ocotp");
void __iomem *ocotp_base;
- struct device_node *np;
struct clk *clk;
int ret = 0;
u32 offset = of_machine_is_compatible("fsl,imx8mp") ?
IMX8MP_OCOTP_UID_OFFSET : 0;
- np = of_find_compatible_node(NULL, NULL, "fsl,imx8mm-ocotp");
if (!np)
return -EINVAL;
ocotp_base = of_iomap(np, 0);
- if (!ocotp_base) {
- ret = -EINVAL;
- goto err_iomap;
- }
+ if (!ocotp_base)
+ return -EINVAL;
clk = of_clk_get_by_name(np, NULL);
if (IS_ERR(clk)) {
@@ -143,38 +136,27 @@ static int imx8mm_soc_uid(u64 *socuid)
err_clk:
iounmap(ocotp_base);
-err_iomap:
- of_node_put(np);
-
return ret;
}
static int imx8mm_soc_revision(u32 *socrev, u64 *socuid)
{
- struct device_node *np;
+ struct device_node *np __free(device_node) =
+ of_find_compatible_node(NULL, NULL, "fsl,imx8mm-anatop");
void __iomem *anatop_base;
- int ret;
- np = of_find_compatible_node(NULL, NULL, "fsl,imx8mm-anatop");
if (!np)
return -EINVAL;
anatop_base = of_iomap(np, 0);
- if (!anatop_base) {
- ret = -EINVAL;
- goto err_iomap;
- }
+ if (!anatop_base)
+ return -EINVAL;
*socrev = readl_relaxed(anatop_base + ANADIG_DIGPROG_IMX8MM);
iounmap(anatop_base);
- of_node_put(np);
return imx8mm_soc_uid(socuid);
-
-err_iomap:
- of_node_put(np);
- return ret;
}
static const struct imx8_soc_data imx8mq_soc_data = {
@@ -205,22 +187,23 @@ static __maybe_unused const struct of_device_id imx8_soc_match[] = {
{ }
};
-#define imx8_revision(soc_rev) \
- soc_rev ? \
- kasprintf(GFP_KERNEL, "%d.%d", (soc_rev >> 4) & 0xf, soc_rev & 0xf) : \
+#define imx8_revision(dev, soc_rev) \
+ (soc_rev) ? \
+ devm_kasprintf((dev), GFP_KERNEL, "%d.%d", ((soc_rev) >> 4) & 0xf, (soc_rev) & 0xf) : \
"unknown"
static int imx8m_soc_probe(struct platform_device *pdev)
{
struct soc_device_attribute *soc_dev_attr;
const struct imx8_soc_data *data;
+ struct device *dev = &pdev->dev;
const struct of_device_id *id;
struct soc_device *soc_dev;
u32 soc_rev = 0;
u64 soc_uid = 0;
int ret;
- soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
+ soc_dev_attr = devm_kzalloc(dev, sizeof(*soc_dev_attr), GFP_KERNEL);
if (!soc_dev_attr)
return -ENOMEM;
@@ -228,13 +211,11 @@ static int imx8m_soc_probe(struct platform_device *pdev)
ret = of_property_read_string(of_root, "model", &soc_dev_attr->machine);
if (ret)
- goto free_soc;
+ return ret;
id = of_match_node(imx8_soc_match, of_root);
- if (!id) {
- ret = -ENODEV;
- goto free_soc;
- }
+ if (!id)
+ return -ENODEV;
data = id->data;
if (data) {
@@ -242,27 +223,21 @@ static int imx8m_soc_probe(struct platform_device *pdev)
if (data->soc_revision) {
ret = data->soc_revision(&soc_rev, &soc_uid);
if (ret)
- goto free_soc;
+ return ret;
}
}
- soc_dev_attr->revision = imx8_revision(soc_rev);
- if (!soc_dev_attr->revision) {
- ret = -ENOMEM;
- goto free_soc;
- }
+ soc_dev_attr->revision = imx8_revision(dev, soc_rev);
+ if (!soc_dev_attr->revision)
+ return -ENOMEM;
- soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX", soc_uid);
- if (!soc_dev_attr->serial_number) {
- ret = -ENOMEM;
- goto free_rev;
- }
+ soc_dev_attr->serial_number = devm_kasprintf(dev, GFP_KERNEL, "%016llX", soc_uid);
+ if (!soc_dev_attr->serial_number)
+ return -ENOMEM;
soc_dev = soc_device_register(soc_dev_attr);
- if (IS_ERR(soc_dev)) {
- ret = PTR_ERR(soc_dev);
- goto free_serial_number;
- }
+ if (IS_ERR(soc_dev))
+ return PTR_ERR(soc_dev);
pr_info("SoC: %s revision %s\n", soc_dev_attr->soc_id,
soc_dev_attr->revision);
@@ -271,15 +246,6 @@ static int imx8m_soc_probe(struct platform_device *pdev)
platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
return 0;
-
-free_serial_number:
- kfree(soc_dev_attr->serial_number);
-free_rev:
- if (strcmp(soc_dev_attr->revision, "unknown"))
- kfree(soc_dev_attr->revision);
-free_soc:
- kfree(soc_dev_attr);
- return ret;
}
static struct platform_driver imx8m_soc_driver = {
--
2.45.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v4 1/3] soc: imx8m: Probe the SoC driver as platform driver
2024-09-29 18:49 [PATCH v4 1/3] soc: imx8m: Probe the SoC driver as platform driver Marek Vasut
2024-09-29 18:49 ` [PATCH v4 2/3] soc: imx8m: Remove global soc_uid Marek Vasut
2024-09-29 18:49 ` [PATCH v4 3/3] soc: imx8m: Use devm_* to simplify probe failure handling Marek Vasut
@ 2024-10-18 3:14 ` Shawn Guo
2 siblings, 0 replies; 4+ messages in thread
From: Shawn Guo @ 2024-10-18 3:14 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-arm-kernel, kernel, AngeloGioacchino Del Regno,
Arnd Bergmann, Fabio Estevam, Jeff Johnson, Neil Armstrong,
Pengutronix Kernel Team, Saravana Kannan, Sascha Hauer, Shawn Guo,
imx, linux-kernel
On Sun, Sep 29, 2024 at 08:49:16PM +0200, Marek Vasut wrote:
> With driver_async_probe=* on kernel command line, the following trace is
> produced because on i.MX8M Plus hardware because the soc-imx8m.c driver
> calls of_clk_get_by_name() which returns -EPROBE_DEFER because the clock
> driver is not yet probed. This was not detected during regular testing
> without driver_async_probe.
>
> Convert the SoC code to platform driver and instantiate a platform device
> in its current device_initcall() to probe the platform driver. Rework
> .soc_revision callback to always return valid error code and return SoC
> revision via parameter. This way, if anything in the .soc_revision callback
> return -EPROBE_DEFER, it gets propagated to .probe and the .probe will get
> retried later.
>
> "
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 1 at drivers/soc/imx/soc-imx8m.c:115 imx8mm_soc_revision+0xdc/0x180
> CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.11.0-next-20240924-00002-g2062bb554dea #603
> Hardware name: DH electronics i.MX8M Plus DHCOM Premium Developer Kit (3) (DT)
> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : imx8mm_soc_revision+0xdc/0x180
> lr : imx8mm_soc_revision+0xd0/0x180
> sp : ffff8000821fbcc0
> x29: ffff8000821fbce0 x28: 0000000000000000 x27: ffff800081810120
> x26: ffff8000818a9970 x25: 0000000000000006 x24: 0000000000824311
> x23: ffff8000817f42c8 x22: ffff0000df8be210 x21: fffffffffffffdfb
> x20: ffff800082780000 x19: 0000000000000001 x18: ffffffffffffffff
> x17: ffff800081fff418 x16: ffff8000823e1000 x15: ffff0000c03b65e8
> x14: ffff0000c00051b0 x13: ffff800082790000 x12: 0000000000000801
> x11: ffff80008278ffff x10: ffff80008209d3a6 x9 : ffff80008062e95c
> x8 : ffff8000821fb9a0 x7 : 0000000000000000 x6 : 00000000000080e3
> x5 : ffff0000df8c03d8 x4 : 0000000000000000 x3 : 0000000000000000
> x2 : 0000000000000000 x1 : fffffffffffffdfb x0 : fffffffffffffdfb
> Call trace:
> imx8mm_soc_revision+0xdc/0x180
> imx8_soc_init+0xb0/0x1e0
> do_one_initcall+0x94/0x1a8
> kernel_init_freeable+0x240/0x2a8
> kernel_init+0x28/0x140
> ret_from_fork+0x10/0x20
> ---[ end trace 0000000000000000 ]---
> SoC: i.MX8MP revision 1.1
> "
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Applied all, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread