Change in coreboot[master]: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755
Ben Chuang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755
The device is a PCIe Gen2 to SD 4.0 card reader controller to be used in the Chromebook. The datasheet name is GL9755S the revision is 05.
The patch sets LTR value.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I16048dde348be248c748d50ca4a8a62c8a781430 --- A src/drivers/genesyslogic/gl9755/Kconfig A src/drivers/genesyslogic/gl9755/Makefile.inc A src/drivers/genesyslogic/gl9755/gl9755.c A src/drivers/genesyslogic/gl9755/gl9755.h M src/include/device/pci_ids.h 5 files changed, 66 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/45062/1
diff --git a/src/drivers/genesyslogic/gl9755/Kconfig b/src/drivers/genesyslogic/gl9755/Kconfig new file mode 100644 index 0000000..0d1fe57 --- /dev/null +++ b/src/drivers/genesyslogic/gl9755/Kconfig @@ -0,0 +1,2 @@ +config DRIVERS_GENESYSLOGIC_GL9755 + bool diff --git a/src/drivers/genesyslogic/gl9755/Makefile.inc b/src/drivers/genesyslogic/gl9755/Makefile.inc new file mode 100644 index 0000000..995cfd3 --- /dev/null +++ b/src/drivers/genesyslogic/gl9755/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_DRIVERS_GENESYSLOGIC_GL9755) += gl9755.c diff --git a/src/drivers/genesyslogic/gl9755/gl9755.c b/src/drivers/genesyslogic/gl9755/gl9755.c new file mode 100644 index 0000000..f66344b --- /dev/null +++ b/src/drivers/genesyslogic/gl9755/gl9755.c @@ -0,0 +1,47 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* Driver for Genesys Logic GL9763E */ + +#include <console/console.h> +#include <device/device.h> +#include <device/path.h> +#include <device/pci.h> +#include <device/pci_ops.h> +#include <device/pci_ids.h> +#include "gl9755.h" + +static void gl9755_init(struct device *dev) +{ + printk(BIOS_INFO, "GL9755: init\n"); + pci_dev_init(dev); + + /* Set Vendor Config to be configurable */ + pci_or_config32(dev, CFG, CFG_EN); + /* Set LTR value */ + pci_update_config32(dev, LTR, 0, NO_SNOOP_SCALE|NO_SNOOP_VALUE|SNOOP_SCALE|SNOOP_VALUE); + /* Set Vendor Config to be non-configurable */ + pci_and_config32(dev, CFG, ~CFG_EN); +} + +static struct device_operations gl9755_ops = { + .read_resources = pci_dev_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = pci_dev_enable_resources, + .ops_pci = &pci_dev_ops_pci, + .init = gl9755_init, +}; + +static const unsigned short pci_device_ids[] = { + PCI_DEVICE_ID_GLI_9755, + 0 +}; + +static const struct pci_driver genesyslogic_gl9755 __pci_driver = { + .ops = &gl9755_ops, + .vendor = PCI_VENDOR_ID_GLI, + .devices = pci_device_ids, +}; + +struct chip_operations drivers_generic_genesyslogic_gl9755_ops = { + CHIP_NAME("Genesys Logic GL9755") +}; diff --git a/src/drivers/genesyslogic/gl9755/gl9755.h b/src/drivers/genesyslogic/gl9755/gl9755.h new file mode 100644 index 0000000..04d25ef --- /dev/null +++ b/src/drivers/genesyslogic/gl9755/gl9755.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* Definitions for Genesys Logic GL9755 */ + +#include <types.h> + +#define CFG 0x800 +#define CFG_EN 0x1 +#define LTR 0x5C +#define SNOOP_VALUE 0x25 +#define SNOOP_SCALE (0x3 << 10) +#define NO_SNOOP_VALUE (0x25 << 16) +#define NO_SNOOP_SCALE (0x3 << 26) + + diff --git a/src/include/device/pci_ids.h b/src/include/device/pci_ids.h index 5aaf7b3..9b45d7e 100644 --- a/src/include/device/pci_ids.h +++ b/src/include/device/pci_ids.h @@ -2035,6 +2035,7 @@
#define PCI_VENDOR_ID_GLI 0x17a0 #define PCI_DEVICE_ID_GLI_9763E 0xe763 +#define PCI_DEVICE_ID_GLI_9755 0x9755
#define PCI_VENDOR_ID_XGI 0x18ca #define PCI_DEVICE_ID_XGI_20 0x0020
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45062
to look at the new patch set (#2).
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755
The device is a PCIe Gen2 to SD 4.0 card reader controller to be used in the Chromebook. The datasheet name is GL9755S the revision is 05.
The patch sets LTR value.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I16048dde348be248c748d50ca4a8a62c8a781430 --- A src/drivers/genesyslogic/gl9755/Kconfig A src/drivers/genesyslogic/gl9755/Makefile.inc A src/drivers/genesyslogic/gl9755/gl9755.c A src/drivers/genesyslogic/gl9755/gl9755.h M src/include/device/pci_ids.h 5 files changed, 64 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/45062/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 2:
Thank you for the changes.
What is the difference to `src/drivers/genesyslogic/gl9763e/`?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 2:
(1 comment)
Patch Set 2:
What is the difference to `src/drivers/genesyslogic/gl9763e/`?
Ah, looks like a totally different device.
https://review.coreboot.org/c/coreboot/+/45062/2/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.h:
https://review.coreboot.org/c/coreboot/+/45062/2/src/drivers/genesyslogic/gl... PS2, Line 7: #define CFG 0x800 Please align the values using tabs. Maybe you can run the file through clang-format?
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45062/2/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.h:
https://review.coreboot.org/c/coreboot/+/45062/2/src/drivers/genesyslogic/gl... PS2, Line 5: #include <types.h> not used. please remove.
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45062/2/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.h:
https://review.coreboot.org/c/coreboot/+/45062/2/src/drivers/genesyslogic/gl... PS2, Line 5: #include <types.h>
not used. […]
It will be removed. Thanks. Done.
https://review.coreboot.org/c/coreboot/+/45062/2/src/drivers/genesyslogic/gl... PS2, Line 7: #define CFG 0x800
Please align the values using tabs. […]
Thanks. I Will align the values using tabs. Done.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45062
to look at the new patch set (#3).
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755
The device is a PCIe Gen2 to SD 4.0 card reader controller to be used in the Chromebook. The datasheet name is GL9755S the revision is 05.
The patch sets LTR value.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I16048dde348be248c748d50ca4a8a62c8a781430 --- A src/drivers/genesyslogic/gl9755/Kconfig A src/drivers/genesyslogic/gl9755/Makefile.inc A src/drivers/genesyslogic/gl9755/gl9755.c A src/drivers/genesyslogic/gl9755/gl9755.h M src/include/device/pci_ids.h 5 files changed, 62 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/45062/3
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45062
to look at the new patch set (#4).
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755
The device is a PCIe Gen2 to SD 4.0 card reader controller to be used in the Chromebook. The datasheet name is GL9755S and the revision is 05.
The patch sets LTR value.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I16048dde348be248c748d50ca4a8a62c8a781430 --- A src/drivers/genesyslogic/gl9755/Kconfig A src/drivers/genesyslogic/gl9755/Makefile.inc A src/drivers/genesyslogic/gl9755/gl9755.c A src/drivers/genesyslogic/gl9755/gl9755.h M src/include/device/pci_ids.h 5 files changed, 62 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/45062/4
Sukumar Ghorai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 4: Code-Review+1
Verified,
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/45062/4/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/4/src/drivers/genesyslogic/gl... PS4, Line 21: pci_update_config32(dev, LTR, 0, NO_SNOOP_SCALE|NO_SNOOP_VALUE|SNOOP_SCALE|SNOOP_VALUE); This should be a `pci_write_config32` as all bits will be rewritten.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 4:
I'd do something like CB:44161 to have this code get build-tested while there's no board using it.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Angel Pons, Nick Vaccaro, Tim Wawrzynczak, Sukumar Ghorai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45062
to look at the new patch set (#5).
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755
The device is a PCIe Gen2 to SD 4.0 card reader controller to be used in the Chromebook. The datasheet name is GL9755S and the revision is 05.
The patch sets LTR value.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I16048dde348be248c748d50ca4a8a62c8a781430 --- A src/drivers/genesyslogic/gl9755/Kconfig A src/drivers/genesyslogic/gl9755/Makefile.inc A src/drivers/genesyslogic/gl9755/gl9755.c A src/drivers/genesyslogic/gl9755/gl9755.h M src/include/device/pci_ids.h 5 files changed, 62 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/45062/5
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 4:
(1 comment)
Thank you for your review.
https://review.coreboot.org/c/coreboot/+/45062/4/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/4/src/drivers/genesyslogic/gl... PS4, Line 21: pci_update_config32(dev, LTR, 0, NO_SNOOP_SCALE|NO_SNOOP_VALUE|SNOOP_SCALE|SNOOP_VALUE);
This should be a `pci_write_config32` as all bits will be rewritten.
Done.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/Kconfig:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 3: Maybe a line of help text here? Maybe something similar to your website text? ```GL9755 is a PCI Express Rev. 2.1 compliant card reader controller which integrates PCI Express PHY, UHS-II PHY, memory card access interface, regulators (3.3V-to-1.8V and 3.3V-to-1.2V) and card power switch.```
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 15: printk(BIOS_INFO, "GL9755: init\n"); Does this line add much value? If you want to see if the device is being initialized, you can follow the resource allocator output and look for VID/DID.
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/Kconfig:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 3:
Maybe a line of help text here? Maybe something similar to your website text? […]
I can add the help text in next version.
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 15: printk(BIOS_INFO, "GL9755: init\n");
Does this line add much value? If you want to see if the device is being initialized, you can follow […]
I want to know if the GL9755 driver has been executed and has set LTR value. Without this printk, how to know whether GL9755 driver is loaded or not ?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 15: printk(BIOS_INFO, "GL9755: init\n");
I want to know if the GL9755 driver has been executed and has set LTR value. […]
Once the driver is added to a mainboard's devicetree, you will see a line in the resource allocator output similar to: "PCI: 00:02.0 [8086/0000] bus ops" which indicates that the device was discovered during enumeration. But this print also doesn't hurt, just letting you know there are other ways besides a print to validate this.
Did you verify the LTR value from the OS?
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 15: printk(BIOS_INFO, "GL9755: init\n");
Once the driver is added to a mainboard's devicetree, you will see a line in the resource allocator […]
Thank you for providing the information. I'd like to keep this print as it doesn't hurt.
If the driver is executed, the original ltr value will be changed to a larger value.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Angel Pons, Nick Vaccaro, Tim Wawrzynczak, Sukumar Ghorai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45062
to look at the new patch set (#6).
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755
The device is a PCIe Gen2 to SD 4.0 card reader controller to be used in the Chromebook. The datasheet name is GL9755S and the revision is 05.
The patch sets LTR value.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I16048dde348be248c748d50ca4a8a62c8a781430 --- A src/drivers/genesyslogic/gl9755/Kconfig A src/drivers/genesyslogic/gl9755/Makefile.inc A src/drivers/genesyslogic/gl9755/gl9755.c A src/drivers/genesyslogic/gl9755/gl9755.h M src/include/device/pci_ids.h 5 files changed, 67 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/45062/6
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/Kconfig:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 3:
I can add the help text in next version.
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 6: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 15: printk(BIOS_INFO, "GL9755: init\n");
Thank you for providing the information. […]
If it's a debug print, I'd use the `BIOS_DEBUG` log level instead.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 15: printk(BIOS_INFO, "GL9755: init\n");
If it's a debug print, I'd use the `BIOS_DEBUG` log level instead.
Indeed. But for consistency all other parts should be changed too, like `src/drivers/genesyslogic/gl9763e/gl9763e.c` and `src/soc/cavium/cn81xx/ecam0.c`.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 15: printk(BIOS_INFO, "GL9755: init\n");
Indeed. […]
What does Cavium have to do with this?
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 15: printk(BIOS_INFO, "GL9755: init\n");
What does Cavium have to do with this?
Yes, it is a debug print. I will change the log level to `BIOS_DEBUG'. About the log level in a gl9763e.c, should I create a new change-id for GL9763E?
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 15: printk(BIOS_INFO, "GL9755: init\n");
Yes, it is a debug print. I will change the log level to `BIOS_DEBUG'. […]
Hi Angel, There are two line about `BIOS_INFO' in ecam0_init() refers to (https://github.com/coreboot/coreboot/blob/master/src/soc/cavium/cn81xx/ecam0...) (https://github.com/coreboot/coreboot/blob/master/src/soc/cavium/cn81xx/ecam0...). Do I need to modify these, too?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 6: Code-Review+2
I think INFO is fine, loglevel.h says about INFO: "Log level for when the system has experienced some typical event. Messages should be superficial in nature"
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 15: printk(BIOS_INFO, "GL9755: init\n");
Hi Angel, […]
Don't worry.
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 6:
(1 comment)
Patch Set 6: Code-Review+2
I think INFO is fine, loglevel.h says about INFO: "Log level for when the system has experienced some typical event. Messages should be superficial in nature"
Thanks for your comments.
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9755/gl9755.c:
https://review.coreboot.org/c/coreboot/+/45062/5/src/drivers/genesyslogic/gl... PS5, Line 15: printk(BIOS_INFO, "GL9755: init\n");
Don't worry.
Well, I will keep the BIOS_INFO level. 😊
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755
The device is a PCIe Gen2 to SD 4.0 card reader controller to be used in the Chromebook. The datasheet name is GL9755S and the revision is 05.
The patch sets LTR value.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I16048dde348be248c748d50ca4a8a62c8a781430 Reviewed-on: https://review.coreboot.org/c/coreboot/+/45062 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- A src/drivers/genesyslogic/gl9755/Kconfig A src/drivers/genesyslogic/gl9755/Makefile.inc A src/drivers/genesyslogic/gl9755/gl9755.c A src/drivers/genesyslogic/gl9755/gl9755.h M src/include/device/pci_ids.h 5 files changed, 67 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/drivers/genesyslogic/gl9755/Kconfig b/src/drivers/genesyslogic/gl9755/Kconfig new file mode 100644 index 0000000..5bccb9b --- /dev/null +++ b/src/drivers/genesyslogic/gl9755/Kconfig @@ -0,0 +1,7 @@ +config DRIVERS_GENESYSLOGIC_GL9755 + bool "Genesys Logic GL9755" + help + GL9755 is a PCI Express Rev. 2.1 compliant card reader controller + which integrates PCI Express PHY, UHS-II PHY, memory card access + interface, regulators (3.3V-to-1.8V and 3.3V-to-1.2V) and card + power switch. diff --git a/src/drivers/genesyslogic/gl9755/Makefile.inc b/src/drivers/genesyslogic/gl9755/Makefile.inc new file mode 100644 index 0000000..995cfd3 --- /dev/null +++ b/src/drivers/genesyslogic/gl9755/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_DRIVERS_GENESYSLOGIC_GL9755) += gl9755.c diff --git a/src/drivers/genesyslogic/gl9755/gl9755.c b/src/drivers/genesyslogic/gl9755/gl9755.c new file mode 100644 index 0000000..c3cdef1 --- /dev/null +++ b/src/drivers/genesyslogic/gl9755/gl9755.c @@ -0,0 +1,47 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* Driver for Genesys Logic GL9755 */ + +#include <console/console.h> +#include <device/device.h> +#include <device/path.h> +#include <device/pci.h> +#include <device/pci_ops.h> +#include <device/pci_ids.h> +#include "gl9755.h" + +static void gl9755_init(struct device *dev) +{ + printk(BIOS_INFO, "GL9755: init\n"); + pci_dev_init(dev); + + /* Set Vendor Config to be configurable */ + pci_or_config32(dev, CFG, CFG_EN); + /* Set LTR value */ + pci_write_config32(dev, LTR, NO_SNOOP_SCALE|NO_SNOOP_VALUE|SNOOP_SCALE|SNOOP_VALUE); + /* Set Vendor Config to be non-configurable */ + pci_and_config32(dev, CFG, ~CFG_EN); +} + +static struct device_operations gl9755_ops = { + .read_resources = pci_dev_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = pci_dev_enable_resources, + .ops_pci = &pci_dev_ops_pci, + .init = gl9755_init, +}; + +static const unsigned short pci_device_ids[] = { + PCI_DEVICE_ID_GLI_9755, + 0 +}; + +static const struct pci_driver genesyslogic_gl9755 __pci_driver = { + .ops = &gl9755_ops, + .vendor = PCI_VENDOR_ID_GLI, + .devices = pci_device_ids, +}; + +struct chip_operations drivers_generic_genesyslogic_gl9755_ops = { + CHIP_NAME("Genesys Logic GL9755") +}; diff --git a/src/drivers/genesyslogic/gl9755/gl9755.h b/src/drivers/genesyslogic/gl9755/gl9755.h new file mode 100644 index 0000000..2d20faf --- /dev/null +++ b/src/drivers/genesyslogic/gl9755/gl9755.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* Definitions for Genesys Logic GL9755 */ + +#define CFG 0x800 +#define CFG_EN 0x1 +#define LTR 0x5C +#define SNOOP_VALUE 0x25 +#define SNOOP_SCALE (0x3 << 10) +#define NO_SNOOP_VALUE (0x25 << 16) +#define NO_SNOOP_SCALE (0x3 << 26) diff --git a/src/include/device/pci_ids.h b/src/include/device/pci_ids.h index 944c20e..83cf25e 100644 --- a/src/include/device/pci_ids.h +++ b/src/include/device/pci_ids.h @@ -2033,6 +2033,7 @@
#define PCI_VENDOR_ID_GLI 0x17a0 #define PCI_DEVICE_ID_GLI_9763E 0xe763 +#define PCI_DEVICE_ID_GLI_9755 0x9755
#define PCI_VENDOR_ID_XGI 0x18ca #define PCI_DEVICE_ID_XGI_20 0x0020
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45062 )
Change subject: drivers/genesyslogic/gl9755: Add driver for Genesys Logic GL9755 ......................................................................
Patch Set 7:
Automatic boot test returned (PASS/FAIL/TOTAL): 8/1/9 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/19963 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19962 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/19961 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19960 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/19959 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/19967 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/19966 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/19965 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19964
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
Ben Chuang (Code Review)
-
HAOUAS Elyes (Code Review)
-
Paul Menzel (Code Review)
-
Sukumar Ghorai (Code Review)
-
Tim Wawrzynczak (Code Review)