Linux-Doc Archive on lore.kernel.org
 help / color / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Saravana Kannan <saravanak@google.com>
Cc: Jonathan Corbet <corbet@lwn.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Kevin Hilman <khilman@kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Len Brown <len.brown@intel.com>, Len Brown <lenb@kernel.org>,
	Pavel Machek <pavel@ucw.cz>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Marc Zyngier <maz@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux PM list <linux-pm@vger.kernel.org>,
	linux-clk <linux-clk@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Android Kernel Team <kernel-team@android.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH v4 0/8] Make fw_devlink=on more forgiving
Date: Tue, 16 Feb 2021 21:31:43 +0100
Message-ID: <CAMuHMdXTO8wQ3=woLMjDaf9g3tTr-dRB3Nu_XvZUrr+wGSXyeg@mail.gmail.com> (raw)
In-Reply-To: <CAGETcx_-yBvhXDPtOiKjenvx83oMNr32UvpMN0Dt-qz5ToXEbw@mail.gmail.com>

Hi Saravana,

On Tue, Feb 16, 2021 at 7:49 PM Saravana Kannan <saravanak@google.com> wrote:
> On Tue, Feb 16, 2021 at 12:05 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Mon, Feb 15, 2021 at 10:27 PM Saravana Kannan <saravanak@google.com> wrote:
> > > On Mon, Feb 15, 2021 at 4:38 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Fri, Feb 12, 2021 at 4:00 AM Saravana Kannan <saravanak@google.com> wrote:
> > > > > On Thu, Feb 11, 2021 at 5:00 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > >       - I2C on R-Car Gen3 does not seem to use DMA, according to
> > > > > >         /sys/kernel/debug/dmaengine/summary:
> > > > > >
> > > > > >             -dma4chan0    | e66d8000.i2c:tx
> > > > > >             -dma4chan1    | e66d8000.i2c:rx
> > > > > >             -dma5chan0    | e6510000.i2c:tx
> > > > >
> > > > > I think I need more context on the problem before I can try to fix it.
> > > > > I'm also very unfamiliar with that file. With fw_devlink=permissive,
> > > > > I2C was using DMA? If so, the next step is to see if the I2C relative
> > > > > probe order with DMA is getting changed and if so, why.
> > > >
> > > > More detailed log:
> > > >
> > > >     platform e66d8000.i2c: Linked as a consumer to e6150000.clock-controller
> > > >     platform e66d8000.i2c: Linked as a sync state only consumer to e6055400.gpio
> > > >
> > > > Why is e66d8000.i2c not linked as a consumer to e6700000.dma-controller?
> > >
> > > Because fw_devlink.strict=1 is not set and dma/iommu is considered an
> > > "optional"/"driver decides" dependency.
> >
> > Oh, I thought dma/iommu were considered mandatory initially,
> > but dropped as dependencies in the late boot process?
>
> No, I didn't do that in case the drivers that didn't need the
> IOMMU/DMA were sensitive to probe order.
>
> My goal was for fw_devlink=on to not affect probe order for devices
> that currently don't need to defer probe. But see below...
>
> >
> > >
> > > >     platform e6700000.dma-controller: Linked as a consumer to
> > > > e6150000.clock-controller
> > >
> > > Is this the only supplier of dma-controller?
> >
> > No, e6180000.system-controller is also a supplier.
> >
> > > >     platform e66d8000.i2c: Added to deferred list
> > > >     platform e6700000.dma-controller: Added to deferred list
> > > >
> > > >     bus: 'platform': driver_probe_device: matched device
> > > > e6700000.dma-controller with driver rcar-dmac
> > > >     bus: 'platform': really_probe: probing driver rcar-dmac with
> > > > device e6700000.dma-controller
> > > >     platform e6700000.dma-controller: Driver rcar-dmac requests probe deferral
> > > >
> > > >     bus: 'platform': driver_probe_device: matched device e66d8000.i2c
> > > > with driver i2c-rcar
> > > >     bus: 'platform': really_probe: probing driver i2c-rcar with device
> > > > e66d8000.i2c
> > > >
> > > > I2C becomes available...
> > > >
> > > >     i2c-rcar e66d8000.i2c: request_channel failed for tx (-517)
> > > >     [...]
> > > >
> > > > but DMA is not available yet, so the driver falls back to PIO.
> > > >
> > > >     driver: 'i2c-rcar': driver_bound: bound to device 'e66d8000.i2c'
> > > >     bus: 'platform': really_probe: bound device e66d8000.i2c to driver i2c-rcar
> > > >
> > > >     platform e6700000.dma-controller: Retrying from deferred list
> > > >     bus: 'platform': driver_probe_device: matched device
> > > > e6700000.dma-controller with driver rcar-dmac
> > > >     bus: 'platform': really_probe: probing driver rcar-dmac with
> > > > device e6700000.dma-controller
> > > >     platform e6700000.dma-controller: Driver rcar-dmac requests probe deferral
> > > >     platform e6700000.dma-controller: Added to deferred list
> > > >     platform e6700000.dma-controller: Retrying from deferred list
> > > >     bus: 'platform': driver_probe_device: matched device
> > > > e6700000.dma-controller with driver rcar-dmac
> > > >     bus: 'platform': really_probe: probing driver rcar-dmac with
> > > > device e6700000.dma-controller
> > > >     driver: 'rcar-dmac': driver_bound: bound to device 'e6700000.dma-controller'
> > > >     bus: 'platform': really_probe: bound device
> > > > e6700000.dma-controller to driver rcar-dmac
> > > >
> > > > DMA becomes available.
> > > >
> > > > Here userspace is entered. /sys/kernel/debug/dmaengine/summary shows
> > > > that the I2C controllers do not have DMA channels allocated, as the
> > > > kernel has performed no more I2C transfers after DMA became available.
> > > >
> > > > Using i2cdetect shows that DMA is used, which is good:
> > > >
> > > >     i2c-rcar e66d8000.i2c: got DMA channel for rx
> > > >
> > > > With permissive devlinks, the clock controller consumers are not added
> > > > to the deferred probing list, and probe order is slightly different.
> > > > The I2C controllers are still probed before the DMA controllers.
> > > > But DMA becomes available a bit earlier, before the probing of the last
> > > > I2C slave driver.
> > >
> > > This seems like a race? I'm guessing it's two different threads
> > > probing those two devices? And it just happens to work for
> > > "permissive" assuming the boot timing doesn't change?
> > >
> > > > Hence /sys/kernel/debug/dmaengine/summary shows that
> > > > some I2C transfers did use DMA.
> > > >
> > > > So the real issue is that e66d8000.i2c not linked as a consumer to
> > > > e6700000.dma-controller.
> > >
> > > That's because fw_devlink.strict=1 isn't set. If you need DMA to be
> > > treated as a mandatory supplier, you'll need to set the flag.
> > >
> > > Is fw_devlink=on really breaking anything here? It just seems like
> > > "permissive" got lucky with the timing and it could break at any point
> > > in the future. Thought?
> >
> > I don't think there is a race.
>
> Can you explain more please? This below makes it sound like DMA just
> sneaks in at the last minute.

Yes it does, as the DMAC also has a consumer link to the IOMMU.
If you ignore the consumer link from I2C to DMAC, the I2C device has
less dependencies than the DMAC, so the I2C device, and the
devices on the I2C bus, are probed much earlier than the DMAC.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

  reply index

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20210205222651eucas1p28ef87073dea33c1c5224c14aa203bec5@eucas1p2.samsung.com>
2021-02-05 22:26 ` Saravana Kannan
2021-02-05 22:26   ` [PATCH v4 1/8] driver core: fw_devlink: Detect supplier devices that will never be added Saravana Kannan
2021-02-05 22:26   ` [PATCH v4 2/8] of: property: Don't add links to absent suppliers Saravana Kannan
2021-02-09 21:25     ` Rob Herring
2021-02-05 22:26   ` [PATCH v4 3/8] driver core: Add fw_devlink.strict kernel param Saravana Kannan
2021-02-05 22:26   ` [PATCH v4 4/8] of: property: Add fw_devlink support for optional properties Saravana Kannan
2021-02-09 21:33     ` Rob Herring
2021-02-09 21:54       ` Saravana Kannan
2021-02-10  8:25         ` Geert Uytterhoeven
2021-02-05 22:26   ` [PATCH v4 5/8] driver core: fw_devlink: Handle suppliers that don't use driver core Saravana Kannan
2021-02-05 22:26   ` [PATCH v4 6/8] irqdomain: Mark fwnodes when their irqdomain is added/removed Saravana Kannan
2021-02-05 22:26   ` [PATCH v4 7/8] PM: domains: Mark fwnodes when their powerdomain " Saravana Kannan
2021-02-05 22:26   ` [PATCH v4 8/8] clk: Mark fwnodes when their clock provider " Saravana Kannan
2021-02-08 15:38     ` Rob Herring
2021-02-08 23:34       ` Saravana Kannan
2021-02-10 11:44     ` [PATCH] clk: Mark fwnodes when their clock provider is added Tudor Ambarus
2021-02-10 11:44       ` Tudor Ambarus
2021-02-11 13:00         ` Greg KH
2021-02-13  0:37           ` Stephen Boyd
     [not found]         ` <CGME20210325133159eucas1p297b769beb681743fb32d362a86cc6e3e@eucas1p2.samsung.com>
2021-03-25 13:31           ` Marek Szyprowski
2021-03-25 15:47             ` Geert Uytterhoeven
2021-03-25 18:25             ` Nicolas Saenz Julienne
2021-03-26 18:13               ` Stephen Boyd
2021-03-26 18:29                 ` Geert Uytterhoeven
     [not found]                   ` <161705310317.3012082.15148238105608149214@swboyd.mtv.corp.google.com>
2021-03-29 23:28                     ` Saravana Kannan
     [not found]                       ` <161706920822.3012082.10047587064612237296@swboyd.mtv.corp.google.com>
2021-03-30  6:58                         ` Geert Uytterhoeven
     [not found]                           ` <161715734080.2260335.881350237641202575@swboyd.mtv.corp.google.com>
2021-03-31  7:05                             ` Geert Uytterhoeven
     [not found]                               ` <161721871083.2260335.2392646934517115770@swboyd.mtv.corp.google.com>
2021-04-05 11:04                                 ` Nicolas Saenz Julienne
2021-04-21  3:26         ` Guenter Roeck
2021-04-21  7:00           ` Saravana Kannan
2021-02-10 19:13       ` Saravana Kannan
2021-03-30 15:42       ` Guenter Roeck
2021-03-30 16:26         ` Saravana Kannan
     [not found]     ` <161317679292.1254594.15797939257637374295@swboyd.mtv.corp.google.com>
2021-02-14 21:15       ` [PATCH v4 8/8] clk: Mark fwnodes when their clock provider is added/removed Saravana Kannan
2021-02-06  2:45   ` [PATCH v4 0/8] Make fw_devlink=on more forgiving Saravana Kannan
2021-02-06 19:41   ` Geert Uytterhoeven
2021-02-06 20:47     ` Saravana Kannan
2021-02-08  8:40   ` Marek Szyprowski
2021-02-08 23:57     ` Saravana Kannan
2021-02-10  8:19   ` Tudor.Ambarus
2021-02-10  8:54     ` Saravana Kannan
2021-02-10 10:02       ` Tudor.Ambarus
2021-02-10 19:14         ` Saravana Kannan
2021-02-11 13:00   ` Geert Uytterhoeven
2021-02-11 13:05     ` Geert Uytterhoeven
2021-02-12  2:59     ` Saravana Kannan
2021-02-12  8:14       ` Geert Uytterhoeven
2021-02-12 20:57         ` Saravana Kannan
2021-02-15 12:38       ` Geert Uytterhoeven
2021-02-15 21:26         ` Saravana Kannan
2021-02-16  8:05           ` Geert Uytterhoeven
2021-02-16 18:48             ` Saravana Kannan
2021-02-16 20:31               ` Geert Uytterhoeven [this message]
2021-02-17 23:57                 ` Saravana Kannan
2021-02-25  9:21                   ` Geert Uytterhoeven
2021-02-15 15:16       ` Geert Uytterhoeven
2021-02-15 21:57         ` Saravana Kannan
2021-02-16 12:58           ` Geert Uytterhoeven
2021-02-15 11:19     ` Geert Uytterhoeven

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='CAMuHMdXTO8wQ3=woLMjDaf9g3tTr-dRB3Nu_XvZUrr+wGSXyeg@mail.gmail.com' \
    --to=geert@linux-m68k.org \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=frowand.list@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-team@android.com \
    --cc=khilman@kernel.org \
    --cc=len.brown@intel.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=maz@kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=pavel@ucw.cz \
    --cc=rafael@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=saravanak@google.com \
    --cc=sboyd@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=ulf.hansson@linaro.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

Linux-Doc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-doc/0 linux-doc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-doc linux-doc/ https://lore.kernel.org/linux-doc \
		linux-doc@vger.kernel.org
	public-inbox-index linux-doc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-doc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git