On Tue, Feb 16, 2021 at 12:05 AM Geert Uytterhoeven
[off-list ref] wrote:

Hi Saravana,

On Mon, Feb 15, 2021 at 10:27 PM Saravana Kannan [off-list ref] wrote:

quoted

On Mon, Feb 15, 2021 at 4:38 AM Geert Uytterhoeven [off-list ref] wrote:

quoted

On Fri, Feb 12, 2021 at 4:00 AM Saravana Kannan [off-list ref] wrote:

quoted

On Thu, Feb 11, 2021 at 5:00 AM Geert Uytterhoeven [off-list ref] wrote:

quoted

      - 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...

quoted

quoted

    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.

quoted

quoted

    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?

quoted

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.

quoted

quoted

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.

 fw_devlinks calling driver_deferred_probe_add()
on all consumers has a big impact on probe order.

Ugh... yeah. That's the real issue. This is really a device links
issue that fw_devlink is exposing. I already have a bunch of things in
my TODO list to improve deferred probing and probe ordering. Since
this is not causing boot issues (only DMA issue) with fw_devlink=on,
can we treat this as not a blocking item for fw_devlink=on? Once I go
through my TODO list, it should be fixed (by not changing probe
ordering unnecessarily). And if not, I can help find out a different
solution at that point.

Also, if you have IOMMU drivers, then fw_devlink.strict is also
another solution that's available. On a separate note (not a final
fix), I was wondering if we should have a config for fw_devlink.strict
default value and then have it selected when IOMMU drivers configs are
enabled.

-Saravana