Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1333521 > unrolled thread

arm qemu test failures due to 'driver-core: platform: probe of-devices only using list of compatibles'

Back to article view | Back to linux.kernel

Contents

  arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Guenter Roeck <linux@roeck-us.net> - 2016-02-14 18:00 +0100
    Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-02-14 21:00 +0100
      Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-02-14 21:10 +0100
        Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-02-15 09:20 +0100
          Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-02-15 10:00 +0100
            Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-02-15 10:20 +0100
              Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-02-15 11:10 +0100
                Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-02-15 11:20 +0100
                  Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-02-15 11:20 +0100
      Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Guenter Roeck <linux@roeck-us.net> - 2016-02-14 22:10 +0100
        Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-02-15 08:50 +0100
    Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-02-15 12:00 +0100
      Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Robin Murphy <robin.murphy@arm.com> - 2016-02-15 14:20 +0100
        Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-02-15 15:50 +0100
          Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-02-15 17:30 +0100
            Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 17:50 +0100
              Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-02-15 18:20 +0100
                Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 22:10 +0100
      Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Guenter Roeck <linux@roeck-us.net> - 2016-02-15 16:50 +0100
        Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-02-15 17:20 +0100
        Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-02-15 18:10 +0100
          Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Guenter Roeck <linux@roeck-us.net> - 2016-02-15 19:20 +0100
            Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Sudeep Holla <sudeep.holla@arm.com> - 2016-02-15 19:50 +0100
        Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Sudeep Holla <sudeep.holla@arm.com> - 2016-02-15 18:50 +0100
          Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-02-15 19:10 +0100
            Re: arm qemu test failures due to 'driver-core: platform: probe  of-devices only using list of compatibles' Sudeep Holla <sudeep.holla@arm.com> - 2016-02-15 19:20 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1334608

On Mon, Feb 15, 2016 at 07:41:19AM -0800, Guenter Roeck wrote:
> On 02/15/2016 02:59 AM, Uwe Kleine-König wrote:
> >Hello Guenter,
> >
> >On Sun, Feb 14, 2016 at 08:50:10AM -0800, Guenter Roeck wrote:
> >>Uwe,
> >>
> >>Your patch 'driver-core: platform: probe of-devices only using list of
> >>compatibles' causes the following qemu tests to crash in -next.
> >>
> >>arm:vexpress-a9:vexpress_defconfig:vexpress-v2p-ca9
> >>arm:vexpress-a15:vexpress_defconfig:vexpress-v2p-ca15-tc1
> >>arm:vexpress-a9:multi_v7_defconfig:vexpress-v2p-ca9
> >>arm:vexpress-a15:multi_v7_defconfig:vexpress-v2p-ca15-tc1
> >>
> >>Crash log:
> >>
> >>VFS: Cannot open root device "mmcblk0" or unknown-block(0,0): error -6
> >>Please append a correct "root=" boot option; here are the available partitions:
> >>1f00          131072 mtdblock0  (driver?)
> >>1f01           32768 mtdblock1  (driver?)
> >>Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
> >
> >Can you provide a complete boot log? This might already reveal which
> >device is failing. It might not be the mmci device but something it
> >depends on (clock, bus parent, irq).
> >
> 
> Sure, something else may be failing, but why does reverting your patch
> fix the problem ?

Well, my patch made matching of platform devices to platform drivers
more strict. Your machine relies on the respective binding though. So of
course reverting my patch "repairs" your machine, but that doesn't
necessarily mean that my patch is wrong. Even though I'm convinced in
the meantime by Russell that there are false positives it doesn't
necessarily imply that your case is such a false positive, too.

One example of a combination of driver + device I intended to break with
my patch was drivers/mtd/nand/mxc_nand.c and devices that got bound to
that by name. The driver does:

	const struct of_device_id *of_id =
		of_match_device(mxcnd_dt_ids, host->dev);

and doesn't handle of_id being NULL after that. Some people argued (also
for other drivers in similar situations) that this cannot happen because
all compatibles had a non-NULL device_id. That is an error that is easy
to make and so the idea was to just not bind in such a case and safe the
user from the surprise.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[toc] | [prev] | [next] | [standalone]

#1334686

On 02/15/2016 09:00 AM, Uwe Kleine-König wrote:
> On Mon, Feb 15, 2016 at 07:41:19AM -0800, Guenter Roeck wrote:
>> On 02/15/2016 02:59 AM, Uwe Kleine-König wrote:
>>> Hello Guenter,
>>>
>>> On Sun, Feb 14, 2016 at 08:50:10AM -0800, Guenter Roeck wrote:
>>>> Uwe,
>>>>
>>>> Your patch 'driver-core: platform: probe of-devices only using list of
>>>> compatibles' causes the following qemu tests to crash in -next.
>>>>
>>>> arm:vexpress-a9:vexpress_defconfig:vexpress-v2p-ca9
>>>> arm:vexpress-a15:vexpress_defconfig:vexpress-v2p-ca15-tc1
>>>> arm:vexpress-a9:multi_v7_defconfig:vexpress-v2p-ca9
>>>> arm:vexpress-a15:multi_v7_defconfig:vexpress-v2p-ca15-tc1
>>>>
>>>> Crash log:
>>>>
>>>> VFS: Cannot open root device "mmcblk0" or unknown-block(0,0): error -6
>>>> Please append a correct "root=" boot option; here are the available partitions:
>>>> 1f00          131072 mtdblock0  (driver?)
>>>> 1f01           32768 mtdblock1  (driver?)
>>>> Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
>>>
>>> Can you provide a complete boot log? This might already reveal which
>>> device is failing. It might not be the mmci device but something it
>>> depends on (clock, bus parent, irq).
>>>
>>
>> Sure, something else may be failing, but why does reverting your patch
>> fix the problem ?
>
> Well, my patch made matching of platform devices to platform drivers
> more strict. Your machine relies on the respective binding though. So of
> course reverting my patch "repairs" your machine, but that doesn't
> necessarily mean that my patch is wrong. Even though I'm convinced in
> the meantime by Russell that there are false positives it doesn't
> necessarily imply that your case is such a false positive, too.
>
> One example of a combination of driver + device I intended to break with
> my patch was drivers/mtd/nand/mxc_nand.c and devices that got bound to
> that by name. The driver does:
>
> 	const struct of_device_id *of_id =
> 		of_match_device(mxcnd_dt_ids, host->dev);
>
> and doesn't handle of_id being NULL after that. Some people argued (also
> for other drivers in similar situations) that this cannot happen because
> all compatibles had a non-NULL device_id. That is an error that is easy
> to make and so the idea was to just not bind in such a case and safe the
> user from the surprise.
>

I added some debugging on top of your patch, and get:

platform basic-mmio-gpio.1.auto: Device node exists [/smb/motherboard/iofpga@7,00000000/sysreg@00000/sys_led@08], of_driver_match_device() failed
platform basic-mmio-gpio.1.auto: platform_match_id() succeeded
platform basic-mmio-gpio.2.auto: Device node exists [/smb/motherboard/iofpga@7,00000000/sysreg@00000/sys_mci@48], of_driver_match_device() failed
platform basic-mmio-gpio.2.auto: platform_match_id() succeeded
platform basic-mmio-gpio.3.auto: Device node exists [/smb/motherboard/iofpga@7,00000000/sysreg@00000/sys_flash@4c], of_driver_match_device() failed
platform basic-mmio-gpio.3.auto: platform_match_id() succeeded

So it isn't the mmc driver failing to instantiate directly,
but (I think) vexpress-sysreg.

Guenter

[toc] | [prev] | [next] | [standalone]

#1334717

On 15/02/16 18:12, Guenter Roeck wrote:

[...]

>
> I added some debugging on top of your patch, and get:
>
> platform basic-mmio-gpio.1.auto: Device node exists
> [/smb/motherboard/iofpga@7,00000000/sysreg@00000/sys_led@08],
> of_driver_match_device() failed
> platform basic-mmio-gpio.1.auto: platform_match_id() succeeded
> platform basic-mmio-gpio.2.auto: Device node exists
> [/smb/motherboard/iofpga@7,00000000/sysreg@00000/sys_mci@48],
> of_driver_match_device() failed
> platform basic-mmio-gpio.2.auto: platform_match_id() succeeded
> platform basic-mmio-gpio.3.auto: Device node exists
> [/smb/motherboard/iofpga@7,00000000/sysreg@00000/sys_flash@4c],
> of_driver_match_device() failed
> platform basic-mmio-gpio.3.auto: platform_match_id() succeeded
>
> So it isn't the mmc driver failing to instantiate directly,
> but (I think) vexpress-sysreg.
>

That's correct, I could reproduce this issue and reported with similar
analysis earlier [1]

--
Regards,
Sudeep

[1] http://www.spinics.net/lists/arm-kernel/msg483107.html

[toc] | [prev] | [next] | [standalone]

#1334666

On 15/02/16 15:41, Guenter Roeck wrote:
> On 02/15/2016 02:59 AM, Uwe Kleine-König wrote:
>> Hello Guenter,
>>
>> On Sun, Feb 14, 2016 at 08:50:10AM -0800, Guenter Roeck wrote:
>>> Uwe,
>>>
>>> Your patch 'driver-core: platform: probe of-devices only using list of
>>> compatibles' causes the following qemu tests to crash in -next.
>>>
>>> arm:vexpress-a9:vexpress_defconfig:vexpress-v2p-ca9
>>> arm:vexpress-a15:vexpress_defconfig:vexpress-v2p-ca15-tc1
>>> arm:vexpress-a9:multi_v7_defconfig:vexpress-v2p-ca9
>>> arm:vexpress-a15:multi_v7_defconfig:vexpress-v2p-ca15-tc1
>>>
>>> Crash log:
>>>
>>> VFS: Cannot open root device "mmcblk0" or unknown-block(0,0): error -6
>>> Please append a correct "root=" boot option; here are the available
>>> partitions:
>>> 1f00          131072 mtdblock0  (driver?)
>>> 1f01           32768 mtdblock1  (driver?)
>>> Kernel panic - not syncing: VFS: Unable to mount root fs on
>>> unknown-block(0,0)
>>
>> Can you provide a complete boot log? This might already reveal which
>> device is failing. It might not be the mmci device but something it
>> depends on (clock, bus parent, irq).
>>
>
> Sure, something else may be failing, but why does reverting your patch
> fix the problem ?
>
> Anyway, complete logs are at http://kerneltests.org/builders.
>
> http://kerneltests.org/builders/qemu-arm-next/builds/376/steps/qemubuildcommand/logs/stdio
>
>

Sorry for missing this earlier, I could reproduce this on my TC2.
The issue is with card-detect gpio probing. It's not related to AMBA
probing as discussed on the mail thread.

mfd_add_device adds devices with of_node when cell->of_compatible is
matched, but the device created is expected to be matched based on name
which the patch under discussion clearly breaks.

One other option I see is to set driver_override for mfd devices
(something like below) but I am not sure that can be generic.

-- 
Regards,
Sudeep

[toc] | [prev] | [next] | [standalone]

#1334674

On Mon, Feb 15, 2016 at 05:41:42PM +0000, Sudeep Holla wrote:
> Sorry for missing this earlier, I could reproduce this on my TC2.
> The issue is with card-detect gpio probing. It's not related to AMBA
> probing as discussed on the mail thread.
> 
> mfd_add_device adds devices with of_node when cell->of_compatible is
> matched, but the device created is expected to be matched based on name
> which the patch under discussion clearly breaks.

If I'm understanding you correctly, you're saying that MFD re-adds
platform devices with the of_node of a new platform device pointing
to an existing of_node, but expects this new platform device to
match a _different_ driver?

Sounds like MFD needs fixing.  I've said this before: of_node's must
_never_ be copied between different device structures, especially
when they are on the _same_ bus - quite simply because the driver
core _can_ match using the DT compatible.

For example... let's say you have a platform device called "1234.foo"
created by DT with compatible of "example,foo".  You have two platform
drivers.  One of them matches compatible "example,foo" and the other
matches against platform devices with a name of "bar".

The "example,foo" device driver is matched against "1234.foo".  It
creates a platform device with a name of "bar" and bus ID "bar.0",
setting the of_node to the same as "1234.foo".

When scanning for a matching driver, if the "example,foo" driver is
found first, "bar.0" will be matched to this driver, and its probe
method called.  If it accepts this device, it will repeat its action,
creating "bar.1".  Repeat until you run out of memory.

If it instead finds the "bar" driver first, this driver will be used
and everything appears to work correctly.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1334680

On 15/02/16 18:03, Russell King - ARM Linux wrote:
> On Mon, Feb 15, 2016 at 05:41:42PM +0000, Sudeep Holla wrote:
>> Sorry for missing this earlier, I could reproduce this on my TC2.
>> The issue is with card-detect gpio probing. It's not related to AMBA
>> probing as discussed on the mail thread.
>>
>> mfd_add_device adds devices with of_node when cell->of_compatible is
>> matched, but the device created is expected to be matched based on name
>> which the patch under discussion clearly breaks.
>
> If I'm understanding you correctly, you're saying that MFD re-adds
> platform devices with the of_node of a new platform device pointing
> to an existing of_node, but expects this new platform device to
> match a _different_ driver?
>

Sorry if I was not clear.

I don't think it re-adds. IIUC mfd cells are specified inside the
mfd device DT node. MFD adds devices for it's child nodes with the
associated device node but with the name specified by MFD cells
matching the compatible.

> Sounds like MFD needs fixing.  I've said this before: of_node's must
> _never_ be copied between different device structures, especially
> when they are on the _same_ bus - quite simply because the driver
> core _can_ match using the DT compatible.
>

I don't think that's happening in this case at-least. For example:

Device node compatible: arm,vexpress-sysreg
Child node compatible: arm,vexpress-sysreg,sys_mci

MFD device is created for above child node with name
"basic-mmio-gpio.<id>.auto" as it matched the MFD cell of_compatible

Since there's no driver to match "arm,vexpress-sysreg,sys_mci", it fails
with $subject patch applied which otherwise would do normal name matching

-- 
Regards,
Sudeep

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web