Re: [PATCH] hw: ppc: sam460ex: Disable Ethernet devicetree nodes

[Guenter Roeck]

On 8/16/21 3:26 AM, Peter Maydell wrote:
> On Mon, 16 Aug 2021 at 06:46, David Gibson <david@gibson.dropbear.id.au> wrote:
> > On Sun, Aug 15, 2021 at 07:59:15PM -0700, Guenter Roeck wrote:
> > > IBM EMAC Ethernet controllers are not emulated by qemu. If they are
> > > enabled in devicetree files, they are instantiated in Linux but
> > > obviously won't work. Disable associated devicetree nodes to prevent
> > > unpredictable behavior.
> > >
> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >
> > I'll wait for Zoltan's opinion on this, but this sort of thing is why
> > I was always pretty dubious about qemu *loading* a dtb file, rather
> > than generating a dt internally.
>
> My take is that if you have to modify the dtb file to get QEMU to
> work, then that's a bug in QEMU that should be fixed. It's no
> worse than for the machines we have that don't use dtb and where
> the kernel just knows what the hardware is. (In my experience
> Arm kernels can be a bit finicky about wanting the right dtb
> and not some earlier or later one, so I think at least for Arm
> trying to generate a dt for our non-virt boards would have been
> pretty painful and liable to "new kernels don't boot any more" bugs.)
>
> Is it sufficient to create stub "unimplemented-device" ethernet
> controllers here, or does the guest want more behaviour than that?

The controllers are instantiated (it looks like the Linux driver doesn't
really check during probe if the hardware is present but relies on DT),
but when trying to access them there is a PHY error. If a different Ethernet
device is explicitly specified and instantiated, it either ends up as eth2
or as eth0, depending on the driver load order. That makes it difficult
to test those other Ethernet devices (and with it the PCI controller).
Plus, of course, there is always the danger of hitting a more severe problem.

No problem though if this patch isn't accepted - I can carry it locally for
my testing. I thought it would be acceptable because there is already other
code doing the same, but I don't really depend on it.

Thanks,
Guenter