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

[Vladimir Oltean <vladimir.oltean@xxxxxxx>]

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