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:
>
> > Patch Set 3:
> >
> > > Patch Set 3: Code-Review-2
> > >
> > > 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
You really should check the history of drivers/wifi/generic, commit d436750 for example...
>
> >
> > > Another idea to solve one of the issues this chip driver
> > > currently handles: Allow to specify the PCI IDs in the
> > > devicetree if the device is `hidden`.
> >
> > Huh, that is already possible (devid)
>
> I meant a proper solution, not a "register" hack.
--
To view, visit https://review.coreboot.org/c/coreboot/+/48385
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic9c9398829b52e6b0523504b862aae9aff559bc7
Gerrit-Change-Number: 48385
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 09 Dec 2020 23:38:31 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Nico Huber 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:
>
> > Patch Set 3: Code-Review-2
> >
> > 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.
>
> > Another idea to solve one of the issues this chip driver
> > currently handles: Allow to specify the PCI IDs in the
> > devicetree if the device is `hidden`.
>
> Huh, that is already possible (devid)
I meant a proper solution, not a "register" hack.
--
To view, visit https://review.coreboot.org/c/coreboot/+/48385
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic9c9398829b52e6b0523504b862aae9aff559bc7
Gerrit-Change-Number: 48385
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 09 Dec 2020 23:27:28 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
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: Code-Review-2
>
> 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?
> about all the mainboards that use this driver by properly
> matching the PCI IDs?
>
> TBH, I don't like to see any more hacks with fake chips or
> devices. The old model of attaching drivers via PCI IDs
> served us well for a long time. Now Intel makes more and
> more PCI devices that actually aren't PCI devices. I think
> it's time we adapt and find a new model to attach drivers.
>
> As we have chipset devicetrees now, it makes things easy
> to match device drivers by name. Just like we do it for
> chip drivers. So my idea would be to do something like
> this:
>
> device pci 19.2 hidden
> driver soc/intel/common/block/uart
> end
>
> And then name the `struct pci_driver`
> `soc_intel_common_block_uart_driver` accordingly.
>
> Just from the top of my head. Alternative suggestions
> are most welcome :)
If we're going to do that, we should do this tree-wide,
else we have again multiple different models.
> Another idea to solve one of the issues this chip driver
> currently handles: Allow to specify the PCI IDs in the
> devicetree if the device is `hidden`.
Huh, that is already possible (devid)
--
To view, visit https://review.coreboot.org/c/coreboot/+/48385
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic9c9398829b52e6b0523504b862aae9aff559bc7
Gerrit-Change-Number: 48385
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 09 Dec 2020 23:16:40 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Nico Huber 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: Code-Review-2
I can't say if this is working, or even if it can work. What
about all the mainboards that use this driver by properly
matching the PCI IDs?
TBH, I don't like to see any more hacks with fake chips or
devices. The old model of attaching drivers via PCI IDs
served us well for a long time. Now Intel makes more and
more PCI devices that actually aren't PCI devices. I think
it's time we adapt and find a new model to attach drivers.
As we have chipset devicetrees now, it makes things easy
to match device drivers by name. Just like we do it for
chip drivers. So my idea would be to do something like
this:
device pci 19.2 hidden
driver soc/intel/common/block/uart
end
And then name the `struct pci_driver`
`soc_intel_common_block_uart_driver` accordingly.
Just from the top of my head. Alternative suggestions
are most welcome :)
Another idea to solve one of the issues this chip driver
currently handles: Allow to specify the PCI IDs in the
devicetree if the device is `hidden`.
--
To view, visit https://review.coreboot.org/c/coreboot/+/48385
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic9c9398829b52e6b0523504b862aae9aff559bc7
Gerrit-Change-Number: 48385
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 09 Dec 2020 23:01:49 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48388 )
Change subject: mb/siemens/chili/base: Fix state of PCI devices
......................................................................
mb/siemens/chili/base: Fix state of PCI devices
The PCI devices P2SB and PMC are hidden by the FSP and thus setting them
to `on` is not correct. Therefore, set their state to hidden.
Change-Id: Ib7c019cd7f389b2e487829e5550cc236ee5645b7
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
---
M src/mainboard/siemens/chili/variants/base/devicetree.cb
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/48388/1
diff --git a/src/mainboard/siemens/chili/variants/base/devicetree.cb b/src/mainboard/siemens/chili/variants/base/devicetree.cb
index 6bdec65..9493801 100644
--- a/src/mainboard/siemens/chili/variants/base/devicetree.cb
+++ b/src/mainboard/siemens/chili/variants/base/devicetree.cb
@@ -120,8 +120,8 @@
device pnp 0c31.0 on end
end
end
- device pci 1f.1 on end # P2SB
- device pci 1f.2 on end # Power Management Controller
+ device pci 1f.1 hidden end # P2SB
+ device pci 1f.2 hidden end # Power Management Controller
device pci 1f.3 on end # Intel HDA
device pci 1f.4 on end # SMBus
device pci 1f.5 on end # PCH SPI
--
To view, visit https://review.coreboot.org/c/coreboot/+/48388
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib7c019cd7f389b2e487829e5550cc236ee5645b7
Gerrit-Change-Number: 48388
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: newchange
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48194 )
Change subject: mb/google/volteer: Improve type-C Port 1 USB2 Eye Diagram for delbin
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/48194
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I41cda27f97287fae5c23dc9843fdf0a8a33057f8
Gerrit-Change-Number: 48194
Gerrit-PatchSet: 4
Gerrit-Owner: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com>
Gerrit-CC: Hank Lin <hank2_lin(a)pegatron.corp-partner.google.com>
Gerrit-CC: Kane Chen <kane_chen(a)pegatron.corp-partner.google.com>
Gerrit-CC: Ken Lu <ken_lu(a)pegatron.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 09 Dec 2020 22:27:36 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48098 )
Change subject: mb/supermicro/x11ssm-f: (re)configure and document various pads
......................................................................
Patch Set 14: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/48098
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9f9328e9ce6f7e291b171f776bb98bc617b64b93
Gerrit-Change-Number: 48098
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 09 Dec 2020 22:20:14 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48369 )
Change subject: mb/supermicro/x11ssm-f: enable AER for PCIe root ports
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/48369
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9d9b5afca0ca891e2812445db1d42a46ba16199e
Gerrit-Change-Number: 48369
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 09 Dec 2020 21:52:46 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment