Change in coreboot[master]: drivers/generic/genesyslogic: Add driver for Genesys Loigc GL9763E
Ben Chuang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Loigc GL9763E ......................................................................
drivers/generic/genesyslogic: Add driver for Genesys Loigc GL9763E
Set single request AXI, disable ASPM L0s and enable SSC.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I158c79f5ac6e559f335b6b50092469c7b1646c56 --- A src/drivers/generic/genesyslogic/Kconfig A src/drivers/generic/genesyslogic/Makefile.inc A src/drivers/generic/genesyslogic/gli9763e.c A src/drivers/generic/genesyslogic/gli9763e.h 4 files changed, 118 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/43751/1
diff --git a/src/drivers/generic/genesyslogic/Kconfig b/src/drivers/generic/genesyslogic/Kconfig new file mode 100644 index 0000000..e215cfa --- /dev/null +++ b/src/drivers/generic/genesyslogic/Kconfig @@ -0,0 +1,2 @@ +config DRIVERS_GENERIC_GLI9763E + bool diff --git a/src/drivers/generic/genesyslogic/Makefile.inc b/src/drivers/generic/genesyslogic/Makefile.inc new file mode 100644 index 0000000..9f5a2eb --- /dev/null +++ b/src/drivers/generic/genesyslogic/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_DRIVERS_GENERIC_GLI9763E) += gli9763e.c diff --git a/src/drivers/generic/genesyslogic/gli9763e.c b/src/drivers/generic/genesyslogic/gli9763e.c new file mode 100644 index 0000000..66b0af5 --- /dev/null +++ b/src/drivers/generic/genesyslogic/gli9763e.c @@ -0,0 +1,87 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* Driver for GenesysLogic 9763E */ + +#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 "gli9763e.h" + +static inline int pci_read_config_dword(struct device *dev, int where, + u32 *val) +{ + *val = pci_read_config32(dev, where); + return 0; +} + +static inline int pci_write_config_dword(struct device *dev, int where, + u32 val) +{ + pci_write_config32(dev, where, val); + return 0; +} + +static void gli_set_gl9763e(struct device *pdev) +{ + u32 value; + + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value); + value &= ~GLI_9763E_VHS_REV; + value |= ((u32)GLI_9763E_VHS_REV_W << GLI_9763E_VHS_REV_SHIFT); + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value); + + pci_read_config_dword(pdev, PCIE_GLI_9763E_SCR, &value); + value |= GLI_9763E_SCR_AXI_REQ; + pci_write_config_dword(pdev, PCIE_GLI_9763E_SCR, value); + + pci_read_config_dword(pdev, PCIE_GLI_9763E_CFG_REG_2, &value); + value &= ~GLI_9763E_CFG_REG_2_L0S; + pci_write_config_dword(pdev, PCIE_GLI_9763E_CFG_REG_2, value); + + pci_read_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL_2, &value); + value &= ~GLI_9763E_PLL_CTL_2_MAX_SSC; + value |= ((u32)MAX_SSC_30000PPM << GLI_9763E_PLL_CTL_2_MAX_SSC_SHIFT); + pci_write_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL_2, value); + + pci_read_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL, &value); + value |= GLI_9763E_PLL_CTL_SSC; + pci_write_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL, value); + + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value); + value &= ~GLI_9763E_VHS_REV; + value |= ((u32)GLI_9763E_VHS_REV_R << GLI_9763E_VHS_REV_SHIFT); + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value); +} + +static void gli9763e_init(struct device *dev) +{ + printk(BIOS_INFO, "GL9763E: init\n"); + pci_dev_init(dev); + gli_set_gl9763e(dev); +} + +static struct device_operations gli9763e_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 = gli9763e_init, +}; + +static const unsigned short pci_device_ids[] = { + PCI_DEVICE_ID_GLI_9763E, + 0 +}; + +static const struct pci_driver genesyslogic_gli9763e __pci_driver = { + .ops = &gli9763e_ops, + .vendor = PCI_VENDOR_ID_GLI, + .devices = pci_device_ids, +}; + +struct chip_operations drivers_generic_genesyslogic_ops = { + CHIP_NAME("Genesys Logic GL9763E") +}; diff --git a/src/drivers/generic/genesyslogic/gli9763e.h b/src/drivers/generic/genesyslogic/gli9763e.h new file mode 100644 index 0000000..586870f --- /dev/null +++ b/src/drivers/generic/genesyslogic/gli9763e.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* Driver for GenesysLogic 9763E */ + +#include <types.h> + +#define PCI_VENDOR_ID_GLI 0x17a0 +#define PCI_DEVICE_ID_GLI_9763E 0xe763 + +#define PCIE_GLI_9763E_VHS 0x884 +#define GLI_9763E_VHS_REV 0xF00 +#define GLI_9763E_VHS_REV_SHIFT 16 +#define GLI_9763E_VHS_REV_R 0x0 +#define GLI_9763E_VHS_REV_M 0x1 +#define GLI_9763E_VHS_REV_W 0x2 +#define PCIE_GLI_9763E_SCR 0x8E0 +#define GLI_9763E_SCR_AXI_REQ BIT(9) + +#define PCIE_GLI_9763E_CFG_REG_2 0x8A4 +#define GLI_9763E_CFG_REG_2_L0S BIT(11) + +#define PCIE_GLI_9763E_PLL_CTL 0x938 +#define GLI_9763E_PLL_CTL_SSC BIT(19) + +#define PCIE_GLI_9763E_PLL_CTL_2 0x93C +#define GLI_9763E_PLL_CTL_2_MAX_SSC 0xFFFF0000 +#define GLI_9763E_PLL_CTL_2_MAX_SSC_SHIFT 16 +#define MAX_SSC_30000PPM 0xF5C3
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Loigc GL9763E ......................................................................
Patch Set 1:
(2 comments)
Welcome to coreboot!
The driver is currently unused, and therefore not build tested. What are you going to use it with?
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@7 PS1, Line 7: Loigc Logic
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@8 PS1, Line 8: Please quickly describe the device, and list the datasheet name and revision you used.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Loigc GL9763E ......................................................................
Patch Set 1: Code-Review+1
(21 comments)
Welcome!
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@7 PS1, Line 7: generic/genesyslogic I don't think the GL9763E is a "generic" device. I'd put it inside `drivers/genesyslogic/` instead.
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@7 PS1, Line 7: GL9763E Could you please mention in the commit message which kind of device the GL9763E is? I couldn't find a datasheet anywhere, only some Linux patches that hint at it being a PCIe to eMMC controller: https://lkml.org/lkml/2020/5/8/160
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/Kconfig:
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 1: config DRIVERS_GENERIC_GLI9763E Note that, since nothing currently selects this option, the code hasn't been build-tested by Jenkins. If this driver will be used by some board (I guess a chromebook?), it would be good to make a follow-up change (a new commit right after this one) that selects this option.
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/gli9763e.h:
PS1: nit: is it GL9763E or GL*I*9763?
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 3: Driver Definitions (the driver is the .c file)
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 7: #define PCI_VENDOR_ID_GLI 0x17a0 : #define PCI_DEVICE_ID_GLI_9763E 0xe763 I'd suggest adding these to `src/include/device/pci_ids.h`
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 10: VHS What does `VHS` mean? I don't think it's one of these, right? 😄 https://en.wikipedia.org/wiki/VHS
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 10: PCIE_GLI_9763E I think one would expect these definitions to be for GL9763E, given the file they're in. The repeated prefix only makes the definitions long and easy to mix up 😕
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 11: 0xF00 (0xf << 16)
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 11: GLI_9763E_VHS_REV I'd add a `_MASK` suffix for clarity
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 13: #define GLI_9763E_VHS_REV_R 0x0 : #define GLI_9763E_VHS_REV_M 0x1 : #define GLI_9763E_VHS_REV_W 0x2 I would define these with the value already shifted:
#define GLI_9763E_VHS_REV_R (0x0 << 16) #define GLI_9763E_VHS_REV_M (0x1 << 16) #define GLI_9763E_VHS_REV_W (0x2 << 16)
Also, please add an extra space after `#define` to make it clear that these are fields within a register
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 26: GLI_9763E_PLL_CTL_2_MAX_SSC I'd add a `_MASK` suffix for clarity
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/gli9763e.c:
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 13: static inline int pci_read_config_dword(struct device *dev, int where, : u32 *val) : { : *val = pci_read_config32(dev, where); : return 0; : } : : static inline int pci_write_config_dword(struct device *dev, int where, : u32 val) : { : pci_write_config32(dev, where, val); : return 0; : } Why add these wrappers?
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 27: gli_set_gl9763e I'd prefer a more descriptive name for this function. `gl9763e_init`, for instance?
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 27: pdev We use `dev` nearly everywhere, even if it's a pointer.
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 33: (u32) These casts aren't needed
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 36: pci_read_config_dword(pdev, PCIE_GLI_9763E_SCR, &value); : value |= GLI_9763E_SCR_AXI_REQ; : pci_write_config_dword(pdev, PCIE_GLI_9763E_SCR, value); Or, in a single line:
pci_or_config32(dev, PCIE_GLI_9763E_SCR, GLI_9763E_SCR_AXI_REQ);
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 40: pci_read_config_dword(pdev, PCIE_GLI_9763E_CFG_REG_2, &value); : value &= ~GLI_9763E_CFG_REG_2_L0S; : pci_write_config_dword(pdev, PCIE_GLI_9763E_CFG_REG_2, value); Or, in a single line:
pci_and_config32(dev, PCIE_GLI_9763E_CFG_REG_2, ~GLI_9763E_CFG_REG_2_L0S);
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 47: pci_write_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL_2, value); These could be done with `pci_update_config32` but it would be a very long line
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 49: pci_read_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL, &value); : value |= GLI_9763E_PLL_CTL_SSC; : pci_write_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL, value); Or, in a single line:
pci_or_config32(dev, PCIE_GLI_9763E_PLL_CTL, GLI_9763E_PLL_CTL_SSC);
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 56: pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value); I see this register is programmed twice with different values. It would be good to know why this is needed.
HsuanYang Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Loigc GL9763E ......................................................................
Patch Set 1:
Hello everybody, how are you ? I am Sean Chen from GenesysLogic.
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Loigc GL9763E ......................................................................
Patch Set 1:
Patch Set 1:
(2 comments)
Welcome to coreboot!
The driver is currently unused, and therefore not build tested. What are you going to use it with?
Thank you. I also wait the information but it will be used in a Chromebook.
陳建宏 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Loigc GL9763E ......................................................................
Patch Set 1:
Pleased to meet you. I'm Renius Chen from GenesysLogic.
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Loigc GL9763E ......................................................................
Patch Set 1:
(4 comments)
Patch Set 1:
(2 comments)
Welcome to coreboot!
The driver is currently unused, and therefore not build tested. What are you going to use it with?
Hi, I got the GL9763E datasheet. But I am not familiar with the operation of gerrit. Could you teach me how to upload the datasheet ?
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@7 PS1, Line 7: GL9763E
Could you please mention in the commit message which kind of device the GL9763E is? I couldn't find […]
Yes, it is a PCIe to eMMC controller. I have added this to the commit message.
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@7 PS1, Line 7: Loigc
Logic
fix it. Done
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@7 PS1, Line 7: generic/genesyslogic
I don't think the GL9763E is a "generic" device. I'd put it inside `drivers/genesyslogic/` instead.
Maybe this patch does not look like 'generic'.
But GL9763E is a standard PCIe controller and compliant with PCI Express Base Specification Rev. 1.1. The patches Genesys submitted to Coreboot do not change any PCIe standard behavior on GL9763E.
The purpose of these patches are to reduce the EMI radiation, to improve the read/write performance and to patch one controller hardware bug for GL9763E. With these patches implemented, GL9763E is still PCIe compliance.
Hence, Genesys would like to request Coreboot to categorize GL9763E into “generic device”.
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@8 PS1, Line 8:
Please quickly describe the device, and list the datasheet name and revision you used.
The device is a PCIe to eMMC bridge controller to be used in the Chromebook as the boot disk. The data sheet name is GL9763E and the revision is 02.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Loigc GL9763E ......................................................................
Patch Set 1:
(1 comment)
Patch Set 1:
(4 comments)
[…]
Hi, I got the GL9763E datasheet. But I am not familiar with the operation of gerrit. Could you teach me how to upload the datasheet ?
Gerrit does not support that. The preferred way is, that GenesysLogic would allow to download the datasheet from their Web site. Otherwise add a note, that the datasheet can be requested by email from GenesysLogic and list the email address.
Regarding my question, I was only asking for the name and revision of the datasheet (which you answered in the comment).
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@8 PS1, Line 8:
The device is a PCIe to eMMC bridge controller to be used in the Chromebook as the boot disk. […]
Thank you, if you amended the commit message (`git commit --amend` if it’s the commit on top, or `git rebase -i origin/master` otherwise; then `git push` with the Change-Id unaltered), we can mark this as resolved.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43751
to look at the new patch set (#2).
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Logic GL9763E ......................................................................
drivers/generic/genesyslogic: Add driver for Genesys Logic GL9763E
The device is a PCIe to eMMC bridge controller to be used in the Chromebook as the boot disk. The datasheet name is GL9763E and the revision is 02.
The patch set single request AXI, disable ASPM L0s and enable SSC.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I158c79f5ac6e559f335b6b50092469c7b1646c56 --- A src/drivers/generic/genesyslogic/Kconfig A src/drivers/generic/genesyslogic/Makefile.inc A src/drivers/generic/genesyslogic/gli9763e.c A src/drivers/generic/genesyslogic/gli9763e.h 4 files changed, 118 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/43751/2
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43751
to look at the new patch set (#3).
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Logic GL9763E ......................................................................
drivers/generic/genesyslogic: Add driver for Genesys Logic GL9763E
The device is a PCIe to eMMC bridge controller to be used in the Chromebook as the boot disk. The datasheet name is GL9763E and the revision is 02.
The patch sets single request AXI, disables ASPM L0s and enables SSC.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I158c79f5ac6e559f335b6b50092469c7b1646c56 --- A src/drivers/generic/genesyslogic/Kconfig A src/drivers/generic/genesyslogic/Makefile.inc A src/drivers/generic/genesyslogic/gl9763e.c A src/drivers/generic/genesyslogic/gl9763e.h M src/include/device/pci_ids.h 5 files changed, 77 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/43751/3
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 3:
(18 comments)
Updated.
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/gli9763e.h:
PS1:
nit: is it GL9763E or GL*I*9763?
Change the name to GL9763E.
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 3: Driver
Definitions (the driver is the . […]
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 7: #define PCI_VENDOR_ID_GLI 0x17a0 : #define PCI_DEVICE_ID_GLI_9763E 0xe763
I'd suggest adding these to `src/include/device/pci_ids. […]
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 10: PCIE_GLI_9763E
I think one would expect these definitions to be for GL9763E, given the file they're in. […]
Remove prefix XX_GLI_9763E_XX. Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 10: VHS
What does `VHS` mean? I don't think it's one of these, right? 😄 https://en.wikipedia. […]
Ha 😄, you can call it this if you want. When it is called "VHS", it will work. Therefore, it is called VHS. (VHS should be Vendor Header Space.)
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 11: GLI_9763E_VHS_REV
I'd add a `_MASK` suffix for clarity
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 11: 0xF00
(0xf << 16)
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 13: #define GLI_9763E_VHS_REV_R 0x0 : #define GLI_9763E_VHS_REV_M 0x1 : #define GLI_9763E_VHS_REV_W 0x2
I would define these with the value already shifted: […]
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 26: GLI_9763E_PLL_CTL_2_MAX_SSC
I'd add a `_MASK` suffix for clarity
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/gli9763e.c:
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 13: static inline int pci_read_config_dword(struct device *dev, int where, : u32 *val) : { : *val = pci_read_config32(dev, where); : return 0; : } : : static inline int pci_write_config_dword(struct device *dev, int where, : u32 val) : { : pci_write_config32(dev, where, val); : return 0; : }
Why add these wrappers?
Removed.
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 27: gli_set_gl9763e
I'd prefer a more descriptive name for this function. […]
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 27: pdev
We use `dev` nearly everywhere, even if it's a pointer.
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 33: (u32)
These casts aren't needed
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 36: pci_read_config_dword(pdev, PCIE_GLI_9763E_SCR, &value); : value |= GLI_9763E_SCR_AXI_REQ; : pci_write_config_dword(pdev, PCIE_GLI_9763E_SCR, value);
Or, in a single line: […]
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 40: pci_read_config_dword(pdev, PCIE_GLI_9763E_CFG_REG_2, &value); : value &= ~GLI_9763E_CFG_REG_2_L0S; : pci_write_config_dword(pdev, PCIE_GLI_9763E_CFG_REG_2, value);
Or, in a single line: […]
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 47: pci_write_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL_2, value);
These could be done with `pci_update_config32` but it would be a very long line
Modified.
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 49: pci_read_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL, &value); : value |= GLI_9763E_PLL_CTL_SSC; : pci_write_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL, value);
Or, in a single line: […]
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 56: pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value);
I see this register is programmed twice with different values. […]
VHS has write protection. Enable writing and disable writing. Thanks for your comments.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 3: Code-Review+1
(22 comments)
Much better!
Patch Set 1:
Hello everybody, how are you ? I am Sean Chen from GenesysLogic.
I'm doing well, nice to meet you.
Patch Set 1:
Pleased to meet you. I'm Renius Chen from GenesysLogic.
Nice to meet you as well.
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@7 PS1, Line 7: generic/genesyslogic
Maybe this patch does not look like 'generic'. […]
I think `generic` drivers are hardware-agnostic stuff like `cbfs-serial`. Since there are other vendor folders directly under `src/drivers/`, I thought `drivers/genesyslogic/` would be better.
In any case, it's not a big deal, it's only about folder placement.
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@8 PS1, Line 8:
Thank you, if you amended the commit message (`git commit --amend` if it’s the commit on top, or `gi […]
Done
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/Kconfig:
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... PS3, Line 1: GLI9763E GL9763E
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/gl9763e.c:
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... PS3, Line 22: MAX_SSC_30000PPM); nit: this fits on the previous line (coreboot's line length limit is 96 characters)
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/gli9763e.h:
PS1:
Change the name to GL9763E.
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 3: Driver
Done
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 7: #define PCI_VENDOR_ID_GLI 0x17a0 : #define PCI_DEVICE_ID_GLI_9763E 0xe763
Done
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 10: VHS
Ha 😄, you can call it this if you want. […]
Oh, Vendor Header Space makes more sense. Thank you for the explanation 😄
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 10: PCIE_GLI_9763E
Remove prefix XX_GLI_9763E_XX. […]
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 11: 0xF00
Done
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 11: GLI_9763E_VHS_REV
Done
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 13: #define GLI_9763E_VHS_REV_R 0x0 : #define GLI_9763E_VHS_REV_M 0x1 : #define GLI_9763E_VHS_REV_W 0x2
Done
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 26: GLI_9763E_PLL_CTL_2_MAX_SSC
Done
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/gli9763e.c:
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 13: static inline int pci_read_config_dword(struct device *dev, int where, : u32 *val) : { : *val = pci_read_config32(dev, where); : return 0; : } : : static inline int pci_write_config_dword(struct device *dev, int where, : u32 val) : { : pci_write_config32(dev, where, val); : return 0; : }
Removed.
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 27: gli_set_gl9763e
Done
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 27: pdev
Done
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 33: (u32)
Done
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 36: pci_read_config_dword(pdev, PCIE_GLI_9763E_SCR, &value); : value |= GLI_9763E_SCR_AXI_REQ; : pci_write_config_dword(pdev, PCIE_GLI_9763E_SCR, value);
Done
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 40: pci_read_config_dword(pdev, PCIE_GLI_9763E_CFG_REG_2, &value); : value &= ~GLI_9763E_CFG_REG_2_L0S; : pci_write_config_dword(pdev, PCIE_GLI_9763E_CFG_REG_2, value);
Done
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 47: pci_write_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL_2, value);
Modified.
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 49: pci_read_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL, &value); : value |= GLI_9763E_PLL_CTL_SSC; : pci_write_config_dword(pdev, PCIE_GLI_9763E_PLL_CTL, value);
Done
Done
https://review.coreboot.org/c/coreboot/+/43751/1/src/drivers/generic/genesys... PS1, Line 56: pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value);
VHS has write protection. Enable writing and disable writing. […]
Thank you for the explanation
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43751
to look at the new patch set (#4).
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Logic GL9763E ......................................................................
drivers/generic/genesyslogic: Add driver for Genesys Logic GL9763E
The device is a PCIe to eMMC bridge controller to be used in the Chromebook as the boot disk. The datasheet name is GL9763E and the revision is 02.
The patch sets single request AXI, disables ASPM L0s and enables SSC.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I158c79f5ac6e559f335b6b50092469c7b1646c56 --- A src/drivers/generic/genesyslogic/Kconfig A src/drivers/generic/genesyslogic/Makefile.inc A src/drivers/generic/genesyslogic/gl9763e.c A src/drivers/generic/genesyslogic/gl9763e.h M src/include/device/pci_ids.h 5 files changed, 76 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/43751/4
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/gl9763e.c:
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... PS3, Line 22: MAX_SSC_30000PPM);
nit: this fits on the previous line (coreboot's line length limit is 96 characters)
Oh. The length of Coreboot is longer than Linux. 😎 Done.
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@7 PS1, Line 7: generic/genesyslogic
I think `generic` drivers are hardware-agnostic stuff like `cbfs-serial`. […]
I can accept your opinion. I need to confirm that others also agree.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/generic/genesyslogic: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 4: Code-Review+1
Thank you for the quick reactions.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43751
to look at the new patch set (#5).
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E
The device is a PCIe to eMMC bridge controller to be used in the Chromebook as the boot disk. The datasheet name is GL9763E and the revision is 02.
The patch sets single request AXI, disables ASPM L0s and enables SSC.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I158c79f5ac6e559f335b6b50092469c7b1646c56 --- A src/drivers/genesyslogic/gl9763e/Kconfig A src/drivers/genesyslogic/gl9763e/Makefile.inc A src/drivers/genesyslogic/gl9763e/gl9763e.c A src/drivers/genesyslogic/gl9763e/gl9763e.h M src/include/device/pci_ids.h 5 files changed, 76 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/43751/5
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@7 PS1, Line 7: generic/genesyslogic
I can accept your opinion. […]
Moved it to `src/drivers/genesyslogic/gl9763e`. Done
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/Kconfig:
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... PS3, Line 1: GLI9763E
GL9763E
Done
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43751
to look at the new patch set (#6).
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E
The device is a PCIe to eMMC bridge controller to be used in the Chromebook as the boot disk. The datasheet name is GL9763E and the revision is 02.
The patch sets single request AXI, disables ASPM L0s and enables SSC.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I158c79f5ac6e559f335b6b50092469c7b1646c56 --- A src/drivers/genesyslogic/gl9763e/Kconfig A src/drivers/genesyslogic/gl9763e/Makefile.inc A src/drivers/genesyslogic/gl9763e/gl9763e.c A src/drivers/genesyslogic/gl9763e/gl9763e.h M src/include/device/pci_ids.h 5 files changed, 76 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/43751/6
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/Kconfig:
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... PS3, Line 1: GLI9763E
Done
replace `GENERIC' with `GENESYSLOGIC'. Done
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 6:
(3 comments)
Hi reviewers, Is there anything else I need to do? May I know the review status? Thanks
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43751/1//COMMIT_MSG@7 PS1, Line 7: generic/genesyslogic
Moved it to `src/drivers/genesyslogic/gl9763e`. […]
Done
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/Kconfig:
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... PS3, Line 1: GLI9763E
replace `GENERIC' with `GENESYSLOGIC'. […]
Done
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... File src/drivers/generic/genesyslogic/gl9763e.c:
https://review.coreboot.org/c/coreboot/+/43751/3/src/drivers/generic/genesys... PS3, Line 22: MAX_SSC_30000PPM);
Oh. The length of Coreboot is longer than Linux. 😎 […]
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 6: Code-Review+2
Builds fine locally
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 6:
Patch Set 6: Code-Review+2
Builds fine locally
Thank you for your review and help.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 6:
(1 comment)
What platform is this being targeted to?
https://review.coreboot.org/c/coreboot/+/43751/6/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9763e/gl9763e.c:
https://review.coreboot.org/c/coreboot/+/43751/6/src/drivers/genesyslogic/gl... PS6, Line 23: pci_update_config32(dev, VHS, ~VHS_REV_MASK, VHS_REV_R); Where are the ASPM controls? And what does each of these lines do?
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 6:
(1 comment)
Patch Set 6:
(1 comment)
What platform is this being targeted to?
A variant which is based on Volteer2.
https://review.coreboot.org/c/coreboot/+/43751/6/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9763e/gl9763e.c:
https://review.coreboot.org/c/coreboot/+/43751/6/src/drivers/genesyslogic/gl... PS6, Line 23: pci_update_config32(dev, VHS, ~VHS_REV_MASK, VHS_REV_R);
Where are the ASPM controls? And what does each of these lines do?
ASPM control is on Line 20.
L:18, set VHS to be writable. L:19, set single AXI request. L:20, L0s support is disabled. L:21, set SSC to 30000 ppm. L:22, enable SSC. L:23, set VHS to read-only.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43751/6/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9763e/gl9763e.c:
https://review.coreboot.org/c/coreboot/+/43751/6/src/drivers/genesyslogic/gl... PS6, Line 23: pci_update_config32(dev, VHS, ~VHS_REV_MASK, VHS_REV_R);
ASPM control is on Line 20. […]
Maybe add this information as comments above each line?
/* Set VHS (Vendor Header Space) to be writable */ pci_update_config32(dev, VHS, ~VHS_REV_MASK, VHS_REV_W);
/* Set single AXI request */ pci_or_config32(dev, SCR, SCR_AXI_REQ);
/* Disable L0s support */ pci_and_config32(dev, CFG_REG_2, ~CFG_REG_2_L0S);
/* Set SSC to 30000 ppm */ pci_update_config32(dev, PLL_CTL_2, ~PLL_CTL_2_MAX_SSC_MASK, MAX_SSC_30000PPM);
/* Enable SSC */ pci_or_config32(dev, PLL_CTL, PLL_CTL_SSC);
/* Set VHS to read-only */ pci_update_config32(dev, VHS, ~VHS_REV_MASK, VHS_REV_R);
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Paul Menzel, Angel Pons, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43751
to look at the new patch set (#7).
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E
The device is a PCIe to eMMC bridge controller to be used in the Chromebook as the boot disk. The datasheet name is GL9763E and the revision is 02.
The patch sets single request AXI, disables ASPM L0s and enables SSC.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I158c79f5ac6e559f335b6b50092469c7b1646c56 --- A src/drivers/genesyslogic/gl9763e/Kconfig A src/drivers/genesyslogic/gl9763e/Makefile.inc A src/drivers/genesyslogic/gl9763e/gl9763e.c A src/drivers/genesyslogic/gl9763e/gl9763e.h M src/include/device/pci_ids.h 5 files changed, 82 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/43751/7
Ben Chuang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43751/6/src/drivers/genesyslogic/gl... File src/drivers/genesyslogic/gl9763e/gl9763e.c:
https://review.coreboot.org/c/coreboot/+/43751/6/src/drivers/genesyslogic/gl... PS6, Line 23: pci_update_config32(dev, VHS, ~VHS_REV_MASK, VHS_REV_R);
Maybe add this information as comments above each line? […]
Done. Thanks.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 7: Code-Review+2
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
Patch Set 7: Code-Review+2
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43751 )
Change subject: drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E ......................................................................
drivers/genesyslogic/gl9763e: Add driver for Genesys Logic GL9763E
The device is a PCIe to eMMC bridge controller to be used in the Chromebook as the boot disk. The datasheet name is GL9763E and the revision is 02.
The patch sets single request AXI, disables ASPM L0s and enables SSC.
Signed-off-by: Ben Chuang benchuanggli@gmail.com Change-Id: I158c79f5ac6e559f335b6b50092469c7b1646c56 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43751 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Aaron Durbin adurbin@chromium.org --- A src/drivers/genesyslogic/gl9763e/Kconfig A src/drivers/genesyslogic/gl9763e/Makefile.inc A src/drivers/genesyslogic/gl9763e/gl9763e.c A src/drivers/genesyslogic/gl9763e/gl9763e.h M src/include/device/pci_ids.h 5 files changed, 82 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/drivers/genesyslogic/gl9763e/Kconfig b/src/drivers/genesyslogic/gl9763e/Kconfig new file mode 100644 index 0000000..c254707 --- /dev/null +++ b/src/drivers/genesyslogic/gl9763e/Kconfig @@ -0,0 +1,2 @@ +config DRIVERS_GENESYSLOGIC_GL9763E + bool diff --git a/src/drivers/genesyslogic/gl9763e/Makefile.inc b/src/drivers/genesyslogic/gl9763e/Makefile.inc new file mode 100644 index 0000000..61a63e6 --- /dev/null +++ b/src/drivers/genesyslogic/gl9763e/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_DRIVERS_GENESYSLOGIC_GL9763E) += gl9763e.c diff --git a/src/drivers/genesyslogic/gl9763e/gl9763e.c b/src/drivers/genesyslogic/gl9763e/gl9763e.c new file mode 100644 index 0000000..48e520b --- /dev/null +++ b/src/drivers/genesyslogic/gl9763e/gl9763e.c @@ -0,0 +1,53 @@ +/* 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 "gl9763e.h" + +static void gl9763e_init(struct device *dev) +{ + printk(BIOS_INFO, "GL9763E: init\n"); + pci_dev_init(dev); + + /* Set VHS (Vendor Header Space) to be writable */ + pci_update_config32(dev, VHS, ~VHS_REV_MASK, VHS_REV_W); + /* Set single AXI request */ + pci_or_config32(dev, SCR, SCR_AXI_REQ); + /* Disable L0s support */ + pci_and_config32(dev, CFG_REG_2, ~CFG_REG_2_L0S); + /* Set SSC to 30000 ppm */ + pci_update_config32(dev, PLL_CTL_2, ~PLL_CTL_2_MAX_SSC_MASK, MAX_SSC_30000PPM); + /* Enable SSC */ + pci_or_config32(dev, PLL_CTL, PLL_CTL_SSC); + /* Set VHS to read-only */ + pci_update_config32(dev, VHS, ~VHS_REV_MASK, VHS_REV_R); +} + +static struct device_operations gl9763e_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 = gl9763e_init, +}; + +static const unsigned short pci_device_ids[] = { + PCI_DEVICE_ID_GLI_9763E, + 0 +}; + +static const struct pci_driver genesyslogic_gl9763e __pci_driver = { + .ops = &gl9763e_ops, + .vendor = PCI_VENDOR_ID_GLI, + .devices = pci_device_ids, +}; + +struct chip_operations drivers_generic_genesyslogic_ops = { + CHIP_NAME("Genesys Logic GL9763E") +}; diff --git a/src/drivers/genesyslogic/gl9763e/gl9763e.h b/src/drivers/genesyslogic/gl9763e/gl9763e.h new file mode 100644 index 0000000..fd9c6ba --- /dev/null +++ b/src/drivers/genesyslogic/gl9763e/gl9763e.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* Definitions for Genesys Logic GL9763E */ + +#include <types.h> + +#define VHS 0x884 +#define VHS_REV_MASK (0xF << 16) +#define VHS_REV_R (0x0 << 16) +#define VHS_REV_M (0x1 << 16) +#define VHS_REV_W (0x2 << 16) +#define SCR 0x8E0 +#define SCR_AXI_REQ BIT(9) + +#define CFG_REG_2 0x8A4 +#define CFG_REG_2_L0S BIT(11) + +#define PLL_CTL 0x938 +#define PLL_CTL_SSC BIT(19) + +#define PLL_CTL_2 0x93C +#define PLL_CTL_2_MAX_SSC_MASK (0xFFFF << 16) +#define MAX_SSC_30000PPM (0xF5C3 << 16) diff --git a/src/include/device/pci_ids.h b/src/include/device/pci_ids.h index da41cb8..05900da 100644 --- a/src/include/device/pci_ids.h +++ b/src/include/device/pci_ids.h @@ -2033,6 +2033,9 @@ #define PCI_DEVICE_ID_ALTIMA_AC1000 0x03e8 #define PCI_DEVICE_ID_ALTIMA_AC9100 0x03ea
+#define PCI_VENDOR_ID_GLI 0x17a0 +#define PCI_DEVICE_ID_GLI_9763E 0xe763 + #define PCI_VENDOR_ID_XGI 0x18ca #define PCI_DEVICE_ID_XGI_20 0x0020 #define PCI_DEVICE_ID_XGI_40 0x0040
-
Aaron Durbin (Code Review) -
Angel Pons (Code Review)
-
Ben Chuang (Code Review)
-
HsuanYang Chen (Code Review)
-
Paul Menzel (Code Review)
-
陳建宏 (Code Review)