[coreboot-gerrit] Change in coreboot[master]: soc/intel/common/block/uart: rework to use dummy device

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48385 )

Change subject: soc/intel/common/block/uart: rework to use dummy device
......................................................................

Patch Set 3:
> Patch Set 3:
>
> I only now realized that you stripped the most important
> part when quoting me:
> > > > > What
> > > > > about all the mainboards that use this driver by properly
> > > > > matching the PCI IDs?
>
> Maybe you misunderstood. This driver is not only used
> via a `chip` entry in the devicetree. It is foremost
> used as a usual PCI driver.
> $ git grep 'device pci 1[89e].* on.*UART' src/mainboard/
>
> This says there are at least 120 cases in the tree that
> potentially use it and might break due to this change. The
> driver would even be used if the device is not mentioned
> in the dt at all, it's PCI.

Ok, that's what *I* missed then...
> > > > > I can't say if this is working, or even if it can work.
> > > >
> > > > I wonder how this one is different from drivers/wifi/generic, which uses the same dummy model?
> > >
> > > Except it doesn't? It does not use a generic device below
> > > a PCI device. And it checks the device path so it doesn't
> > > attach PCI ops to a non-PCI device in the first place.
> >
> > What is this then?
> > device pci 14.3 on
> > 	chip drivers/wifi/generic
> > 		register "wake" = "GPE0_PME_B0"
> > 		device generic 0 on end
> > 	end
> > end  # CNVi wifi
>
> Sorry, my bad. I didn't realize it's used with two
> different topologies. The difference is that the driver
> is prepared for that and attaches the CNVi ops only
> in case of the generic device. And only then the parent
> is queried.
>
> Here, OTOH, you always use the parent, no matter the
> topology.

Ack, got it. Thanks
> > You really should check the history of drivers/wifi/generic, commit d436750 for example...
>
> Well, the history is one thing. The current code is what
> matters however. How about you read that?

I did ;) Actually I meant that one change, the wording is just misleading

Well, if we rework the driver/chip modeling like you proposed, we indeed can get rid of any fake devices. Also I'm not sure if we want to mix "drivers" and "chips" then, because it's not always certain which model needs to be used. Or would we keep chips for real devices then and only use "driver" when the port is the device?

Where would we place driver options then?
> device pci 19.2 hidden
>           driver soc/intel/common/block/uart
>   end
>
> device pci 14.3 on
>  driver drivers/wifi/generic
>  register "wake" = "GPE0_PME_B0"
> end  # CNVi wifi