Re: [RFC v8 net-next 00/16] add support for VSC7512 control over SPI

Vladimir Oltean
2022-05-10T16:19:03Z

On Mon, May 09, 2022 at 07:57:48PM -0700, Colin Foster wrote:
> On Mon, May 09, 2022 at 05:13:05PM +0000, Vladimir Oltean wrote:
> > Hi Colin,
> >
> > On Sun, May 08, 2022 at 11:52:57AM -0700, Colin Foster wrote:
> >
> > Why does this get printed, if you put a dump_stack() in of_dma_configure_id()?
>
> Below. I'm one of the only users of IORESOURCE_REG, from what I can
> tell... Not sure if that's any consolation.
>
> >
> > > [ 2.835718] pinctrl-ocelot ocelot-pinctrl.0.auto: invalid resource
> > > [ 2.842717] gpiochip_find_base: found new base at 2026
> > > [ 2.842774] gpio gpiochip4: (ocelot-gpio): created GPIO range 0->21 ==> ocelot-pinctrl.0.auto PIN 0->21
> > > [ 2.845693] gpio gpiochip4: (ocelot-gpio): added GPIO chardev (254:4)
> > > [ 2.845828] gpio gpiochip4: registered GPIOs 2026 to 2047 on ocelot-gpio
> > > [ 2.845855] pinctrl-ocelot ocelot-pinctrl.0.auto: driver registered
> > > [ 2.855925] pinctrl-microchip-sgpio ocelot-sgpio.1.auto: DMA mask not set
> > > [ 2.863089] pinctrl-microchip-sgpio ocelot-sgpio.1.auto: invalid resource
> > > [ 2.870801] gpiochip_find_base: found new base at 1962
> > > [ 2.871528] gpio_stub_drv gpiochip5: (ocelot-sgpio.1.auto-input): added GPIO chardev (254:5)
> > > [ 2.871666] gpio_stub_drv gpiochip5: registered GPIOs 1962 to 2025 on ocelot-sgpio.1.auto-input
> > > [ 2.872364] gpiochip_find_base: found new base at 1898
> > > [ 2.873244] gpio_stub_drv gpiochip6: (ocelot-sgpio.1.auto-output): added GPIO chardev (254:6)
> > > [ 2.873354] gpio_stub_drv gpiochip6: registered GPIOs 1898 to 1961 on ocelot-sgpio.1.auto-output
> > > [ 2.881148] mscc-miim ocelot-miim0.2.auto: DMA mask not set
>
> [ 16.699517] CPU: 0 PID: 7 Comm: kworker/u2:0 Not tainted 5.18.0-rc5-01315-g0a0ea78e3a79-dirty #632
> [ 16.708574] Hardware name: Generic AM33XX (Flattened Device Tree)
> [ 16.714704] Workqueue: events_unbound deferred_probe_work_func
> [ 16.720608] Backtrace:
> [ 16.755335] of_dma_configure_id from platform_dma_configure+0x2c/0x38
> [ 16.772320] platform_dma_configure from really_probe+0x78/0x298
>
> platform_dma_configure gets called because...
>
> [ 16.778360] really_probe from __driver_probe_device+0x94/0xf4
> [ 16.789913] __driver_probe_device from driver_probe_device+0x44/0xe0
> [ 16.799980] driver_probe_device from __device_attach_driver+0x9c/0xc4
> [ 16.814326] __device_attach_driver from bus_for_each_drv+0x94/0xe4
> [ 16.826319] bus_for_each_drv from __device_attach+0x104/0x170
> [ 16.836827] __device_attach from device_initial_probe+0x1c/0x20
> [ 16.847507] device_initial_probe from bus_probe_device+0x94/0x9c
> [ 16.853637] bus_probe_device from device_add+0x3ec/0x8b4
> [ 16.864756] device_add from platform_device_add+0x100/0x210
> [ 16.880864] platform_device_add from mfd_add_devices+0x308/0x62c
>
> platform_device_add sets up pdev->bus = &platform_bus_type;

This part is clear. MFD cells are platform devices which have an
of_node, so platform_dma_configure() calls of_dma_configure_id().

> That assignment looks to date back to the before times... Now you have
> me curious. And a little scared :-)
>
> [ 16.898465] mfd_add_devices from devm_mfd_add_devices+0x80/0xc0
> [ 16.914924] devm_mfd_add_devices from ocelot_core_init+0x40/0x6c
> [ 16.927790] ocelot_core_init from ocelot_spi_probe+0xf4/0x188
> [ 16.937251] ocelot_spi_probe from spi_probe+0x94/0xb8
> [ 16.948118] spi_probe from really_probe+0x110/0x298
> [ 16.958800] really_probe from __driver_probe_device+0x94/0xf4
> [ 16.970354] __driver_probe_device from driver_probe_device+0x44/0xe0
> [ 16.980422] driver_probe_device from __device_attach_driver+0x9c/0xc4
> [ 16.994768] __device_attach_driver from bus_for_each_drv+0x94/0xe4
> [ 17.006762] bus_for_each_drv from __device_attach+0x104/0x170
> [ 17.017269] __device_attach from device_initial_probe+0x1c/0x20
> [ 17.027948] device_initial_probe from bus_probe_device+0x94/0x9c
> [ 17.034077] bus_probe_device from device_add+0x3ec/0x8b4
> [ 17.045197] device_add from __spi_add_device+0x7c/0x10c
> [ 17.060959] __spi_add_device from spi_add_device+0x48/0x78
> [ 17.072252] spi_add_device from of_register_spi_device+0x258/0x390
> [ 17.082147] of_register_spi_device from spi_register_controller+0x26c/0x6d8
> [ 17.095970] spi_register_controller from devm_spi_register_controller+0x24/0x60
> [ 17.113822] devm_spi_register_controller from omap2_mcspi_probe+0x4c8/0x574
> [ 17.126608] omap2_mcspi_probe from platform_probe+0x6c/0xc8
> [ 17.142717] platform_probe from really_probe+0x110/0x298
> [ 17.153835] really_probe from __driver_probe_device+0x94/0xf4
> [ 17.165387] __driver_probe_device from driver_probe_device+0x44/0xe0
> [ 17.175455] driver_probe_device from __device_attach_driver+0x9c/0xc4
> [ 17.189800] __device_attach_driver from bus_for_each_drv+0x94/0xe4
> [ 17.201792] bus_for_each_drv from __device_attach+0x104/0x170
> [ 17.212299] __device_attach from device_initial_probe+0x1c/0x20
> [ 17.222979] device_initial_probe from bus_probe_device+0x94/0x9c
> [ 17.229109] bus_probe_device from deferred_probe_work_func+0x8c/0xb8
> [ 17.241277] deferred_probe_work_func from process_one_work+0x1e0/0x53c
> [ 17.255728] process_one_work from worker_thread+0x238/0x4fc
> [ 17.271836] worker_thread from kthread+0x108/0x138
> [ 17.328184] mscc-miim ocelot-miim0.2.auto: DMA mask not set

Unfortunately I don't have any hardware to test, but I think what
happens is:

mfd_add_device()
{
calls platform_device_alloc()
calls setup_pdev_dma_masks()
sets up the default pdev->dev.dma_mask pointer

overwrites pdev->dev.dma_mask with parent->dma_mask;
// your parent->dma_mask is NULL

calls platform_device_add()
...
calls of_dma_configure_id
complains that pdev->dev.dma_mask is NULL (i.e.
the bus hasn't bothered setting up a default DMA
mask, which in fact it did)
}

I don't have enough background regarding the justification for commit
b018e1361bad ("mfd: core: Copy DMA mask and params from parent"), but it
might be detrimential in some cases?

The warning is printed since the slightly unrelated commit 4d8bde883bfb
("OF: Don't set default coherent DMA mask").