From: Sam Protsenko <semen.protsenko@linaro.org>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
Robin Murphy <robin.murphy@arm.com>,
Sumit Semwal <sumit.semwal@linaro.org>,
Alim Akhtar <alim.akhtar@samsung.com>,
Janghyuck Kim <janghyuck.kim@samsung.com>,
Cho KyongHo <pullip.cho@samsung.com>,
Daniel Mentz <danielmentz@google.com>,
David Virag <virag.david003@gmail.com>,
iommu@lists.linux.dev, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-samsung-soc@vger.kernel.org,
Saravana Kannan <saravanak@google.com>,
Rob Herring <robh@kernel.org>
Subject: Re: [PATCH v2 0/6] iommu/exynos: Convert to a module
Date: Fri, 11 Nov 2022 14:29:49 +0100 [thread overview]
Message-ID: <CAPLW+4=Y6qZG2XjJR_BkX-ar4GWdETKO1tteJjfbxVc664e4Kg@mail.gmail.com> (raw)
In-Reply-To: <b7ad6444-e7d2-1150-6134-3dae8129dcdb@samsung.com>
On Thu, 10 Nov 2022 at 15:36, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
[snip]
> I've finally made Exynos IOMMU working as a module on Exynos5433 based
> TM2e board. It looks that this will be a bit longer journey that I've
> initially thought. I've posted a simple update of the fix for the driver
> initialization sequence, but the real problem is in the platform driver
> framework and OF helpers.
>
> Basically to get it working as a module I had to apply the following
> changes:
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 3dda62503102..f6921f5fcab6 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -257,7 +257,7 @@ static int deferred_devs_show(struct seq_file *s,
> void *data)
> DEFINE_SHOW_ATTRIBUTE(deferred_devs);
>
> #ifdef CONFIG_MODULES
> -int driver_deferred_probe_timeout = 10;
> +int driver_deferred_probe_timeout = 30;
> #else
> int driver_deferred_probe_timeout;
> #endif
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 967f79b59016..e5df6672fee6 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1384,7 +1384,7 @@ static struct device_node *parse_interrupts(struct
> device_node *np,
> static const struct supplier_bindings of_supplier_bindings[] = {
> { .parse_prop = parse_clocks, },
> { .parse_prop = parse_interconnects, },
> - { .parse_prop = parse_iommus, .optional = true, },
> + { .parse_prop = parse_iommus, },
> { .parse_prop = parse_iommu_maps, .optional = true, },
> { .parse_prop = parse_mboxes, },
> { .parse_prop = parse_io_channels, },
>
> Without that a really nasty things happened.
>
> Initialization of the built-in drivers and loading modules takes time,
> so the default 10s deferred probe timeout is not enough to ensure that
> the built-in driver won't be probed before the Exynos IOMMU driver is
> loaded.
>
Yeah, the whole time-based sync looks nasty... I remember coming
across the slides by Andrzej Hajda called "Deferred Problem" [1], but
I guess the proposed solution was never applied. Just hope that
increasing the timeout is upstreamable solution.
[1] https://events19.linuxfoundation.org/wp-content/uploads/2017/12/Deferred-Problem-Issues-With-Complex-Dependencies-Between-Devices-in-Linux-Kernel-Andrzej-Hajda-Samsung.pdf
> The second change fixes the problem that driver core probes Exynos IOMMU
> controllers in parallel to probing the master devices, what results in
> calling exynos_iommu_of_xlate() and exynos_iommu_probe_device() even on
> the partially initialized IOMMU controllers or initializing the dma_ops
> under the already probed and working master device. This was easy to
> observe especially on the master devices with multiple IOMMU
> controllers. I wasn't able to solve this concurrency/race issues inside
> the Exynos IOMMU driver.
>
> Frankly speaking I don't know what is the rationale for making the
> 'iommus' property optional, but this simply doesn't work well with IOMMU
> driver being a module. CCed Saravana and Rob for this.
>
The patch which makes 'iommus' optional doesn't provide much of
insight on reasons in commit message either.
> Without fixing the above issues, I would add a warning that compiling
> the driver as a module leads to serious issues.
>
Nice catch! It doesn't reproduce on my platform, alas. Can I expect
you to submit those patches? If so, I'll probably just wait for those
to be applied, and then re-send my modularization series on top of it.
Does that sounds reasonable?
[snip]
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
next prev parent reply other threads:[~2022-11-11 13:30 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20221103195201eucas1p2a6ec2df41ebac3d9ccbb0b252c2cad34@eucas1p2.samsung.com>
2022-11-03 19:51 ` [PATCH v2 0/6] iommu/exynos: Convert to a module Sam Protsenko
2022-11-03 19:51 ` [PATCH v2 1/6] iommu: Export iommu_group_default_domain() API Sam Protsenko
2022-11-03 19:51 ` [PATCH v2 2/6] iommu/exynos: Fix retval on getting clocks in probe Sam Protsenko
2022-11-04 13:46 ` Krzysztof Kozlowski
2022-11-03 19:51 ` [PATCH v2 3/6] iommu/exynos: Modularize the driver Sam Protsenko
2022-11-04 13:46 ` Krzysztof Kozlowski
2022-11-03 19:51 ` [PATCH v2 4/6] iommu/exynos: Implement shutdown driver method Sam Protsenko
2022-11-04 13:47 ` Krzysztof Kozlowski
2022-11-03 19:51 ` [PATCH v2 5/6] iommu/exynos: Rearrange the platform driver code Sam Protsenko
2022-11-04 13:47 ` Krzysztof Kozlowski
2022-11-03 19:51 ` [PATCH v2 6/6] media: platform: Use IS_ENABLED() to check EXYNOS_IOMMU in s5p_mfc Sam Protsenko
2022-11-04 13:48 ` Krzysztof Kozlowski
2022-11-04 12:10 ` [PATCH v2 0/6] iommu/exynos: Convert to a module Marek Szyprowski
2022-11-04 13:22 ` Sam Protsenko
2022-11-10 14:36 ` Marek Szyprowski
2022-11-11 13:29 ` Sam Protsenko [this message]
2023-02-07 3:32 ` Saravana Kannan
2023-02-23 4:33 ` Sam Protsenko
2023-03-13 15:51 ` Marek Szyprowski
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='CAPLW+4=Y6qZG2XjJR_BkX-ar4GWdETKO1tteJjfbxVc664e4Kg@mail.gmail.com' \
--to=semen.protsenko@linaro.org \
--cc=alim.akhtar@samsung.com \
--cc=danielmentz@google.com \
--cc=iommu@lists.linux.dev \
--cc=janghyuck.kim@samsung.com \
--cc=joro@8bytes.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=pullip.cho@samsung.com \
--cc=robh@kernel.org \
--cc=robin.murphy@arm.com \
--cc=saravanak@google.com \
--cc=sumit.semwal@linaro.org \
--cc=virag.david003@gmail.com \
--cc=will@kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).