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 1 of 2  [1] 2  Next page →

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

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)

ie the mmc driver no longer instantiates. Reverting the patch fixes the problem.

Bisect log is attached.

Guenter

---
# bad: [64d9a3617b3b8bc0734ba97caeb433b7019c6187] Add linux-next specific files for 20160212
# good: [388f7b1d6e8ca06762e2454d28d6c3c55ad0fe95] Linux 4.5-rc3
git bisect start 'HEAD' 'v4.5-rc3'
# good: [597dc9d36e8bc04941b61b26ac7aa3f8a33aba53] Merge remote-tracking branch 'sound-asoc/for-next'
git bisect good 597dc9d36e8bc04941b61b26ac7aa3f8a33aba53
# bad: [91fe8ea815243ec595753ccf7e14126b6f87f2bf] Merge remote-tracking branch 'usb-chipidea-next/ci-for-usb-next'
git bisect bad 91fe8ea815243ec595753ccf7e14126b6f87f2bf
# good: [1d6796e67f265e835bcb1a19d27ba0433dbd75e4] Merge remote-tracking branch 'tip/auto-latest'
git bisect good 1d6796e67f265e835bcb1a19d27ba0433dbd75e4
# bad: [858163465b53ab87c3939cae9e6fd0ecbeb60bfa] Merge remote-tracking branch 'driver-core/driver-core-next'
git bisect bad 858163465b53ab87c3939cae9e6fd0ecbeb60bfa
# good: [5acd4c7ca23549bf4e480a92efb7d87d988be432] Merge remote-tracking branch 'kvm-arm/next'
git bisect good 5acd4c7ca23549bf4e480a92efb7d87d988be432
# good: [d28003ab55e09323bf1a026e804165c6d371ae6b] Merge remote-tracking branch 'drivers-x86/for-next'
git bisect good d28003ab55e09323bf1a026e804165c6d371ae6b
# good: [f28a8693f4b1eb8b4035167825f2bcd44bd95546] Merge remote-tracking branch 'hsi/for-next'
git bisect good f28a8693f4b1eb8b4035167825f2bcd44bd95546
# good: [d3a7387f8aae81ba0f3687518a9ad7a14bfb165d] Merge remote-tracking branch 'ipmi/for-next'
git bisect good d3a7387f8aae81ba0f3687518a9ad7a14bfb165d
# good: [75f3e8e47f381074801d0034874d20c638d9e3d9] firmware: introduce sysfs driver for QEMU's fw_cfg device
git bisect good 75f3e8e47f381074801d0034874d20c638d9e3d9
# good: [9e5b3d6f7f946a3fb4d83ac2ab6d2bfefcdafffb] drivers: dma-coherent: simplify dma_init_coherent_memory return value
git bisect good 9e5b3d6f7f946a3fb4d83ac2ab6d2bfefcdafffb
# good: [cf68d85529f7dccc24412887d46e364f4b422a5d] driver-core: platform: fix typo in documentation for multi-driver helper
git bisect good cf68d85529f7dccc24412887d46e364f4b422a5d
# bad: [67d02a1bbb334558e9380409a3cd426b36d4578b] driver-core: platform: probe of-devices only using list of compatibles
git bisect bad 67d02a1bbb334558e9380409a3cd426b36d4578b
# first bad commit: [67d02a1bbb334558e9380409a3cd426b36d4578b] driver-core: platform: probe of-devices only using list of compatibles

[toc] | [next] | [standalone]

#1333545

[adding lakml and rmk to Cc]

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)
> 
> ie the mmc driver no longer instantiates. Reverting the patch fixes the problem.

The driver is drivers/mmc/host/mmci.c, right? and the relevant device
tree snippet is:

	mmci@05000 {
		compatible = "arm,pl180", "arm,primecell";
		...
	};

? So the unexpected abnormality here is that even though this device is
instantiated by dt, the driver doesn't provide any compatibles.
Either my expectation is wrong, then 67d02a1bbb33455 should be reverted
(or handle this case in a different way), or the mmci driver should
declare compatibles (but then it needs to be a platform driver and not
an amba driver?).

Best regards
Uwe

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

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

#1333548

On Sun, Feb 14, 2016 at 08:55:01PM +0100, Uwe Kleine-König wrote:
> So the unexpected abnormality here is that even though this device is
> instantiated by dt, the driver doesn't provide any compatibles.
> Either my expectation is wrong, then 67d02a1bbb33455 should be reverted

Your expectation is wrong.  AMBA primecell devices have hardware IDs
and are matched to their drivers by those IDs.  Just like PCI.

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

#1334228

Hello Russell,

On Sun, Feb 14, 2016 at 08:07:55PM +0000, Russell King - ARM Linux wrote:
> On Sun, Feb 14, 2016 at 08:55:01PM +0100, Uwe Kleine-König wrote:
> > So the unexpected abnormality here is that even though this device is
> > instantiated by dt, the driver doesn't provide any compatibles.
> > Either my expectation is wrong, then 67d02a1bbb33455 should be reverted
> 
> Your expectation is wrong.  AMBA primecell devices have hardware IDs
> and are matched to their drivers by those IDs.  Just like PCI.

pci devices don't appear in dt, do they? I don't see the connection
between amba devices and platform devices, see my other mail in this
thread for some more details.

Best regards
Uwe

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

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

#1334292

On Mon, Feb 15, 2016 at 09:17:50AM +0100, Uwe Kleine-König wrote:
> Hello Russell,
> 
> On Sun, Feb 14, 2016 at 08:07:55PM +0000, Russell King - ARM Linux wrote:
> > On Sun, Feb 14, 2016 at 08:55:01PM +0100, Uwe Kleine-König wrote:
> > > So the unexpected abnormality here is that even though this device is
> > > instantiated by dt, the driver doesn't provide any compatibles.
> > > Either my expectation is wrong, then 67d02a1bbb33455 should be reverted
> > 
> > Your expectation is wrong.  AMBA primecell devices have hardware IDs
> > and are matched to their drivers by those IDs.  Just like PCI.
> 
> pci devices don't appear in dt, do they? I don't see the connection
> between amba devices and platform devices, see my other mail in this
> thread for some more details.

They both have hardware IDs, and they are both matched via those hardware
IDs.

Your change has introduced a regression and is therefore wrong.

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

#1334305

Hello Russell,

On Mon, Feb 15, 2016 at 08:58:18AM +0000, Russell King - ARM Linux wrote:
> On Mon, Feb 15, 2016 at 09:17:50AM +0100, Uwe Kleine-König wrote:
> > On Sun, Feb 14, 2016 at 08:07:55PM +0000, Russell King - ARM Linux wrote:
> > > On Sun, Feb 14, 2016 at 08:55:01PM +0100, Uwe Kleine-König wrote:
> > > > So the unexpected abnormality here is that even though this device is
> > > > instantiated by dt, the driver doesn't provide any compatibles.
> > > > Either my expectation is wrong, then 67d02a1bbb33455 should be reverted
> > > 
> > > Your expectation is wrong.  AMBA primecell devices have hardware IDs
> > > and are matched to their drivers by those IDs.  Just like PCI.
> > 
> > pci devices don't appear in dt, do they? I don't see the connection
> > between amba devices and platform devices, see my other mail in this
> > thread for some more details.
> 
> They both have hardware IDs, and they are both matched via those hardware
> IDs.

I changed platform_match which is about matching by dt compatible, acpi
and/or device name. I don't see how this can affect an amba device given
they match to a driver by a hardware id.

> Your change has introduced a regression and is therefore wrong.

I'd like to understand though why and how my commit is wrong to be able
to fix it instead of getting it reverted.

Best regards
Uwe

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

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

#1334337

On Mon, Feb 15, 2016 at 10:14:06AM +0100, Uwe Kleine-König wrote:
> Hello Russell,
> 
> On Mon, Feb 15, 2016 at 08:58:18AM +0000, Russell King - ARM Linux wrote:
> > On Mon, Feb 15, 2016 at 09:17:50AM +0100, Uwe Kleine-König wrote:
> > > On Sun, Feb 14, 2016 at 08:07:55PM +0000, Russell King - ARM Linux wrote:
> > > > On Sun, Feb 14, 2016 at 08:55:01PM +0100, Uwe Kleine-König wrote:
> > > > > So the unexpected abnormality here is that even though this device is
> > > > > instantiated by dt, the driver doesn't provide any compatibles.
> > > > > Either my expectation is wrong, then 67d02a1bbb33455 should be reverted
> > > > 
> > > > Your expectation is wrong.  AMBA primecell devices have hardware IDs
> > > > and are matched to their drivers by those IDs.  Just like PCI.
> > > 
> > > pci devices don't appear in dt, do they? I don't see the connection
> > > between amba devices and platform devices, see my other mail in this
> > > thread for some more details.
> > 
> > They both have hardware IDs, and they are both matched via those hardware
> > IDs.
> 
> I changed platform_match which is about matching by dt compatible, acpi
> and/or device name. I don't see how this can affect an amba device given
> they match to a driver by a hardware id.
> 
> > Your change has introduced a regression and is therefore wrong.
> 
> I'd like to understand though why and how my commit is wrong to be able
> to fix it instead of getting it reverted.

I don't have the commit, and I haven't seen the patch so I can't
comment further, sorry.

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

#1334342

Hello Russell,

On Mon, Feb 15, 2016 at 10:04:15AM +0000, Russell King - ARM Linux wrote:
> On Mon, Feb 15, 2016 at 10:14:06AM +0100, Uwe Kleine-König wrote:
> > On Mon, Feb 15, 2016 at 08:58:18AM +0000, Russell King - ARM Linux wrote:
> > > On Mon, Feb 15, 2016 at 09:17:50AM +0100, Uwe Kleine-König wrote:
> > > > On Sun, Feb 14, 2016 at 08:07:55PM +0000, Russell King - ARM Linux wrote:
> > > > > On Sun, Feb 14, 2016 at 08:55:01PM +0100, Uwe Kleine-König wrote:
> > > > > > So the unexpected abnormality here is that even though this device is
> > > > > > instantiated by dt, the driver doesn't provide any compatibles.
> > > > > > Either my expectation is wrong, then 67d02a1bbb33455 should be reverted
> > > > > 
> > > > > Your expectation is wrong.  AMBA primecell devices have hardware IDs
> > > > > and are matched to their drivers by those IDs.  Just like PCI.
> > > > 
> > > > pci devices don't appear in dt, do they? I don't see the connection
> > > > between amba devices and platform devices, see my other mail in this
> > > > thread for some more details.
> > > 
> > > They both have hardware IDs, and they are both matched via those hardware
> > > IDs.
> > 
> > I changed platform_match which is about matching by dt compatible, acpi
> > and/or device name. I don't see how this can affect an amba device given
> > they match to a driver by a hardware id.
> > 
> > > Your change has introduced a regression and is therefore wrong.
> > 
> > I'd like to understand though why and how my commit is wrong to be able
> > to fix it instead of getting it reverted.
> 
> I don't have the commit, and I haven't seen the patch so I can't
> comment further, sorry.

It's in -next. For a quick look:

	https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=67d02a1bbb33455

Best regards
Uwe

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

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

#1334345

On Mon, Feb 15, 2016 at 11:10:14AM +0100, Uwe Kleine-König wrote:
> Hello Russell,
> 
> On Mon, Feb 15, 2016 at 10:04:15AM +0000, Russell King - ARM Linux wrote:
> > On Mon, Feb 15, 2016 at 10:14:06AM +0100, Uwe Kleine-König wrote:
> > > On Mon, Feb 15, 2016 at 08:58:18AM +0000, Russell King - ARM Linux wrote:
> > > > On Mon, Feb 15, 2016 at 09:17:50AM +0100, Uwe Kleine-König wrote:
> > > > > On Sun, Feb 14, 2016 at 08:07:55PM +0000, Russell King - ARM Linux wrote:
> > > > > > On Sun, Feb 14, 2016 at 08:55:01PM +0100, Uwe Kleine-König wrote:
> > > > > > > So the unexpected abnormality here is that even though this device is
> > > > > > > instantiated by dt, the driver doesn't provide any compatibles.
> > > > > > > Either my expectation is wrong, then 67d02a1bbb33455 should be reverted
> > > > > > 
> > > > > > Your expectation is wrong.  AMBA primecell devices have hardware IDs
> > > > > > and are matched to their drivers by those IDs.  Just like PCI.
> > > > > 
> > > > > pci devices don't appear in dt, do they? I don't see the connection
> > > > > between amba devices and platform devices, see my other mail in this
> > > > > thread for some more details.
> > > > 
> > > > They both have hardware IDs, and they are both matched via those hardware
> > > > IDs.
> > > 
> > > I changed platform_match which is about matching by dt compatible, acpi
> > > and/or device name. I don't see how this can affect an amba device given
> > > they match to a driver by a hardware id.
> > > 
> > > > Your change has introduced a regression and is therefore wrong.
> > > 
> > > I'd like to understand though why and how my commit is wrong to be able
> > > to fix it instead of getting it reverted.
> > 
> > I don't have the commit, and I haven't seen the patch so I can't
> > comment further, sorry.
> 
> It's in -next. For a quick look:
> 
> 	https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=67d02a1bbb33455

Well, if that's only touching the platform device matching, it can't have
any effect on AMBA bus matching, which uses completely different code.
The AMBA bus code is entirely separate from platform devices.

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

#1333571

On 02/14/2016 11:55 AM, Uwe Kleine-König wrote:
> [adding lakml and rmk to Cc]
>
> 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)
>>
>> ie the mmc driver no longer instantiates. Reverting the patch fixes the problem.
>
> The driver is drivers/mmc/host/mmci.c, right? and the relevant device
> tree snippet is:
>
> 	mmci@05000 {
> 		compatible = "arm,pl180", "arm,primecell";
> 		...
> 	};
>

Yes, I think so, or one of the many other similar mmc entries.

> ? So the unexpected abnormality here is that even though this device is
> instantiated by dt, the driver doesn't provide any compatibles.
> Either my expectation is wrong, then 67d02a1bbb33455 should be reverted
> (or handle this case in a different way), or the mmci driver should
> declare compatibles (but then it needs to be a platform driver and not
> an amba driver?).
>

No idea what the correct solution would be. I do see

         if (of_device_is_compatible(bus, "arm,primecell")) {
                 /*
                  * Don't return an error here to keep compatibility with older
                  * device tree files.
                  */
                 of_amba_device_create(bus, bus_id, platform_data, parent);
                 return 0;
         }

in drivers/of/platform.c, which suggests some special handling for amba
devices. No idea if and how that is related, but I do have some concern
that fixing the problem for mmc alone might not fix it for all the other
devices instantiated with "arm,primecell". After all, my boot tests are
really rudimentary (it boots, therefore it works).

Thanks,
Guenter

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

#1334217

Hello Guenter,

On Sun, Feb 14, 2016 at 01:08:42PM -0800, Guenter Roeck wrote:
> On 02/14/2016 11:55 AM, Uwe Kleine-König wrote:
> >[adding lakml and rmk to Cc]
[adding some more people to Cc]

> >On Sun, Feb 14, 2016 at 08:50:10AM -0800, Guenter Roeck wrote:
> >>Your patch 'driver-core: platform: probe of-devices only using list of
> >>compatibles' causes the following qemu tests to crash in -next.

For the new readers, that is 67d02a1bbb334558e9380409a3cd426b36d4578b.

The original idea of this commit was to not bind a device created from
device tree when its name matches the driver name but none of the
driver's compatibles which might yield some surprises.

> >>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)
> >>
> >>ie the mmc driver no longer instantiates. Reverting the patch fixes the problem.
> >
> >The driver is drivers/mmc/host/mmci.c, right? and the relevant device
> >tree snippet is:
> >
> >	mmci@05000 {
> >		compatible = "arm,pl180", "arm,primecell";
> >		...
> >	};
> >
> 
> Yes, I think so, or one of the many other similar mmc entries.

So the driver in question is an amba_driver and it fails to bind because

	static int platform_match(struct device *dev, struct device_driver *drv)
	
was changed. This is the platform bus type's match function. Why is this
called for amba devices (that I would expect to use amba_bustype and so
amba_match)?

The driver isn't matched by of_driver_match_device, so the
following code must yield 1 for the mmci device:

        /* Then try ACPI style match */
        if (acpi_driver_match_device(dev, drv))
                return 1;

        /* Then try to match against the id table */
        if (pdrv->id_table)
                return platform_match_id(pdrv->id_table, pdev) != NULL;

        /* fall-back to driver name match */
        return (strcmp(pdev->name, drv->name) == 0);

acpi seems unlikely, and the other two match by the device's name which
feels wrong. And I also wonder, what drv is here, because platform_match
assumes it is a platform_driver, not an amba_driver.

> >? So the unexpected abnormality here is that even though this device is
> >instantiated by dt, the driver doesn't provide any compatibles.
> >Either my expectation is wrong, then 67d02a1bbb33455 should be reverted
> >(or handle this case in a different way), or the mmci driver should
> >declare compatibles (but then it needs to be a platform driver and not
> >an amba driver?).
> 
> No idea what the correct solution would be. I do see
> 
>         if (of_device_is_compatible(bus, "arm,primecell")) {
>                 /*
>                  * Don't return an error here to keep compatibility with older
>                  * device tree files.
>                  */
>                 of_amba_device_create(bus, bus_id, platform_data, parent);
>                 return 0;
>         }

So there is a new (and better?) way to instantiate amba devices?

> in drivers/of/platform.c, which suggests some special handling for amba
> devices. No idea if and how that is related, but I do have some concern
> that fixing the problem for mmc alone might not fix it for all the other
> devices instantiated with "arm,primecell". After all, my boot tests are
> really rudimentary (it boots, therefore it works).

I don't see the right thing to do either. Maybe someone else can shed
some light on this issue?

Best regards
Uwe

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

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

#1334377

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

Best regards
Uwe

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

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

#1334447

On 15/02/16 10:59, 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).

FWIW the PL180 on my Juno still works fine with this patch picked on top 
of -rc3, so the issue would seem to be something else - From a quick 
comparison between the DTs I see a slight difference in compatible 
strings for the clocks, but the more likely-looking suspect is that the 
VExpress DT references some GPIOs where the Juno DT doesn't.

Robin.

>
> Best regards
> Uwe
>

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

#1334499

On Mon, Feb 15, 2016 at 01:11:49PM +0000, Robin Murphy wrote:
> FWIW the PL180 on my Juno still works fine with this patch picked on top of
> -rc3, so the issue would seem to be something else - From a quick comparison
> between the DTs I see a slight difference in compatible strings for the
> clocks, but the more likely-looking suspect is that the VExpress DT
> references some GPIOs where the Juno DT doesn't.

Maybe it would be a good idea that Uwe creates a patch which initially
warns when a DT platform device falls back to matching via the platform
strings?

It's likely that the "basic subsystem" platform drivers are silent when
they probe, so having notification of a fallback would at least put
something into the kernel log when that happens - and then later change
that to be a hard failure (as Uwe is trying to do with his patch.)

However, I have to bring up another point: is what Uwe is trying to do
actually the right thing?  The DT platform device code has the ability
to create standard platform devices from DT, with an of_node, but with
standard names, and platform data.  It's there for compatibility with
older systems, and is there to allow systems to be transitioned over.

This patch breaks all that: despite the DT code changing the platform
device bus_id from the address.nodename format to the standard format
(thus allowing unconverted platform drivers to match), this patch
means that because the platform device has a of_node attached, this
will now fail.

Therefore, I think Uwe's patch is just wrong - or, if it's something we
want, the auxdata table support code needs to _also_ be ripped out of
the drivers/of/platform.c code, but that then means anyone who wants to
go through the conversion has a big flag-day change to go through.

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

#1334577

Hello Russell,

On Mon, Feb 15, 2016 at 02:43:44PM +0000, Russell King - ARM Linux wrote:
> On Mon, Feb 15, 2016 at 01:11:49PM +0000, Robin Murphy wrote:
> > FWIW the PL180 on my Juno still works fine with this patch picked on top of
> > -rc3, so the issue would seem to be something else - From a quick comparison
> > between the DTs I see a slight difference in compatible strings for the
> > clocks, but the more likely-looking suspect is that the VExpress DT
> > references some GPIOs where the Juno DT doesn't.
> 
> Maybe it would be a good idea that Uwe creates a patch which initially
> warns when a DT platform device falls back to matching via the platform
> strings?
> 
> It's likely that the "basic subsystem" platform drivers are silent when
> they probe, so having notification of a fallback would at least put
> something into the kernel log when that happens - and then later change
> that to be a hard failure (as Uwe is trying to do with his patch.)
> 
> However, I have to bring up another point: is what Uwe is trying to do
> actually the right thing?  The DT platform device code has the ability
> to create standard platform devices from DT, with an of_node, but with
> standard names, and platform data.  It's there for compatibility with
> older systems, and is there to allow systems to be transitioned over.
> 
> This patch breaks all that: despite the DT code changing the platform
> device bus_id from the address.nodename format to the standard format
> (thus allowing unconverted platform drivers to match), this patch
> means that because the platform device has a of_node attached, this
> will now fail.
> 
> Therefore, I think Uwe's patch is just wrong - or, if it's something we
> want, the auxdata table support code needs to _also_ be ripped out of
> the drivers/of/platform.c code, but that then means anyone who wants to
> go through the conversion has a big flag-day change to go through.

That's a valid concern I wasn't aware of when I created the patch.

So maybe just emitting a warning as you suggested is a good idea. And
additionally only emit it when the driver is dt aware, too.

Greg, can you drop this patch, or do you need a proper changelog for a
revert? On top of that I'd then create a new patch which is more
conservative.

Best regards
Uwe

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

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

#1334590

On Mon, Feb 15, 2016 at 05:27:53PM +0100, Uwe Kleine-König wrote:
> Hello Russell,
> 
> On Mon, Feb 15, 2016 at 02:43:44PM +0000, Russell King - ARM Linux wrote:
> > On Mon, Feb 15, 2016 at 01:11:49PM +0000, Robin Murphy wrote:
> > > FWIW the PL180 on my Juno still works fine with this patch picked on top of
> > > -rc3, so the issue would seem to be something else - From a quick comparison
> > > between the DTs I see a slight difference in compatible strings for the
> > > clocks, but the more likely-looking suspect is that the VExpress DT
> > > references some GPIOs where the Juno DT doesn't.
> > 
> > Maybe it would be a good idea that Uwe creates a patch which initially
> > warns when a DT platform device falls back to matching via the platform
> > strings?
> > 
> > It's likely that the "basic subsystem" platform drivers are silent when
> > they probe, so having notification of a fallback would at least put
> > something into the kernel log when that happens - and then later change
> > that to be a hard failure (as Uwe is trying to do with his patch.)
> > 
> > However, I have to bring up another point: is what Uwe is trying to do
> > actually the right thing?  The DT platform device code has the ability
> > to create standard platform devices from DT, with an of_node, but with
> > standard names, and platform data.  It's there for compatibility with
> > older systems, and is there to allow systems to be transitioned over.
> > 
> > This patch breaks all that: despite the DT code changing the platform
> > device bus_id from the address.nodename format to the standard format
> > (thus allowing unconverted platform drivers to match), this patch
> > means that because the platform device has a of_node attached, this
> > will now fail.
> > 
> > Therefore, I think Uwe's patch is just wrong - or, if it's something we
> > want, the auxdata table support code needs to _also_ be ripped out of
> > the drivers/of/platform.c code, but that then means anyone who wants to
> > go through the conversion has a big flag-day change to go through.
> 
> That's a valid concern I wasn't aware of when I created the patch.
> 
> So maybe just emitting a warning as you suggested is a good idea. And
> additionally only emit it when the driver is dt aware, too.
> 
> Greg, can you drop this patch, or do you need a proper changelog for a
> revert? On top of that I'd then create a new patch which is more
> conservative.

A hint as to what the git commit id was would be helpful, I can just
revert it based on that.

thanks,

greg k-h

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

#1334623

Hello Greg,

On Mon, Feb 15, 2016 at 08:49:37AM -0800, Greg Kroah-Hartman wrote:
> On Mon, Feb 15, 2016 at 05:27:53PM +0100, Uwe Kleine-König wrote:
> > Greg, can you drop this patch, or do you need a proper changelog for a
> > revert? On top of that I'd then create a new patch which is more
> > conservative.
> 
> A hint as to what the git commit id was would be helpful, I can just
> revert it based on that.

This is 67d02a1bbb33 ("driver-core: platform: probe of-devices only
using list of compatibles")

If you need a log, something like:

	Reallow binding of of-devices by name

	It turned out that there are valid reasons (e.g. step by step
	conversion to device tree probing using auxdata) to bind
	of-instantiated devices to drivers by name. So revert to the
	original logic.

Best regards
Uwe

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

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

#1334816

On Mon, Feb 15, 2016 at 06:12:01PM +0100, Uwe Kleine-König wrote:
> Hello Greg,
> 
> On Mon, Feb 15, 2016 at 08:49:37AM -0800, Greg Kroah-Hartman wrote:
> > On Mon, Feb 15, 2016 at 05:27:53PM +0100, Uwe Kleine-König wrote:
> > > Greg, can you drop this patch, or do you need a proper changelog for a
> > > revert? On top of that I'd then create a new patch which is more
> > > conservative.
> > 
> > A hint as to what the git commit id was would be helpful, I can just
> > revert it based on that.
> 
> This is 67d02a1bbb33 ("driver-core: platform: probe of-devices only
> using list of compatibles")
> 
> If you need a log, something like:
> 
> 	Reallow binding of of-devices by name
> 
> 	It turned out that there are valid reasons (e.g. step by step
> 	conversion to device tree probing using auxdata) to bind
> 	of-instantiated devices to drivers by name. So revert to the
> 	original logic.

Now reverted, thanks for the text.

greg k-h

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

#1334544

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

is the most recent log (next-20120215). Look for the vexpress crashes; the overo
crash bisected to to 'PM / OPP: Disable OPPs that aren't supported by the regulator',
in next-20160212, which I have not fully analyzed yet, and the beagle crashes
as well as the 'new' overo crash are brand new.

Guenter

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

#1334571

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 ?
> 
> Anyway, complete logs are at http://kerneltests.org/builders.
> 
> http://kerneltests.org/builders/qemu-arm-next/builds/376/steps/qemubuildcommand/logs/stdio
> 
> is the most recent log (next-20120215). Look for the vexpress crashes; the overo
> crash bisected to to 'PM / OPP: Disable OPPs that aren't supported by the regulator',
> in next-20160212, which I have not fully analyzed yet, and the beagle crashes
> as well as the 'new' overo crash are brand new.

Looking at the vexpress-ca9 one, nothing stands out to me apart from the
lack of messages about a MMC driver.  I don't see anything there which
indicates why that would be.

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web