Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31270
Change subject: intel/spi: Define PCI accessor aliases only once ......................................................................
intel/spi: Define PCI accessor aliases only once
These files copied code from flashrom project that uses different names for PCI config accessors.
Also fix cases of using ENV_SMM where __SIMPLE_DEVICE__ should be used, if the file is to be build for bootblock or romstage later.
Change-Id: If7190ac105b2a65a9576709955c3cc840b95dcdf Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- A src/include/device/pci_compat.h M src/soc/intel/baytrail/spi.c M src/soc/intel/braswell/spi.c M src/soc/intel/broadwell/spi.c M src/soc/intel/fsp_baytrail/spi.c M src/southbridge/intel/common/spi.c M src/southbridge/intel/fsp_rangeley/spi.c 7 files changed, 70 insertions(+), 180 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/31270/1
diff --git a/src/include/device/pci_compat.h b/src/include/device/pci_compat.h new file mode 100644 index 0000000..d80fe01 --- /dev/null +++ b/src/include/device/pci_compat.h @@ -0,0 +1,35 @@ +/* + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but without any warranty; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __PCI_COMPAT_H__ +#define __PCI_COMPAT_H__ + +#include <device/pci_ops.h> + +#if defined(__PCI_COMPAT_FLASHROM__) + +#define pci_read_config_byte(dev, reg, targ)\ + *(targ) = pci_read_config8(dev, reg) +#define pci_read_config_word(dev, reg, targ)\ + *(targ) = pci_read_config16(dev, reg) +#define pci_read_config_dword(dev, reg, targ)\ + *(targ) = pci_read_config32(dev, reg) +#define pci_write_config_byte(dev, reg, val)\ + pci_write_config8(dev, reg, val) +#define pci_write_config_word(dev, reg, val)\ + pci_write_config16(dev, reg, val) +#define pci_write_config_dword(dev, reg, val)\ + pci_write_config32(dev, reg, val) + +#endif /* __PCI_COMPAT_FLASHROM__ */ + +#endif /* __PCI_COMPAT_H__ */ diff --git a/src/soc/intel/baytrail/spi.c b/src/soc/intel/baytrail/spi.c index 81e118c..4a11f33 100644 --- a/src/soc/intel/baytrail/spi.c +++ b/src/soc/intel/baytrail/spi.c @@ -21,41 +21,17 @@ #include <arch/io.h> #include <commonlib/helpers.h> #include <console/console.h> +#include <device/device.h> +#include <device/pci.h> #include <spi_flash.h> #include <spi-generic.h>
#include <soc/lpc.h> #include <soc/pci_devs.h>
-#ifdef __SMM__ -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#else /* !__SMM__ */ -#include <device/device.h> -#include <device/pci.h> -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#endif /* !__SMM__ */ +/* This file uses PCI accessor in style of flashrom project. */ +#define __PCI_COMPAT_FLASHROM__ +#include <device/pci_compat.h>
typedef struct spi_slave ich_spi_slave;
@@ -264,7 +240,7 @@ { uint32_t sbase;
-#ifdef __SMM__ +#ifdef __SIMPLE_DEVICE__ pci_devfn_t dev = PCI_DEV(0, LPC_DEV, LPC_FUNC); #else struct device *dev = pcidev_on_root(LPC_DEV, LPC_FUNC); diff --git a/src/soc/intel/braswell/spi.c b/src/soc/intel/braswell/spi.c index febf1d2..8ed8d17 100644 --- a/src/soc/intel/braswell/spi.c +++ b/src/soc/intel/braswell/spi.c @@ -19,6 +19,8 @@ #include <commonlib/helpers.h> #include <console/console.h> #include <delay.h> +#include <device/device.h> +#include <device/pci.h> #include <soc/lpc.h> #include <soc/pci_devs.h> #include <spi_flash.h> @@ -27,35 +29,9 @@ #include <stdlib.h> #include <string.h>
-#if ENV_SMM -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#else /* ENV_SMM */ -#include <device/device.h> -#include <device/pci.h> -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#endif /* ENV_SMM */ +/* This file uses PCI accessor in style of flashrom project. */ +#define __PCI_COMPAT_FLASHROM__ +#include <device/pci_compat.h>
typedef struct spi_slave ich_spi_slave;
@@ -232,7 +208,7 @@ { uint32_t sbase;
-#if ENV_SMM +#ifdef __SIMPLE_DEVICE__ pci_devfn_t dev = PCI_DEV(0, LPC_DEV, LPC_FUNC); #else struct device *dev = pcidev_on_root(LPC_DEV, LPC_FUNC); diff --git a/src/soc/intel/broadwell/spi.c b/src/soc/intel/broadwell/spi.c index de3d061..6b9e962 100644 --- a/src/soc/intel/broadwell/spi.c +++ b/src/soc/intel/broadwell/spi.c @@ -20,41 +20,17 @@ #include <delay.h> #include <arch/io.h> #include <console/console.h> +#include <device/device.h> +#include <device/pci.h> #include <spi_flash.h> #include <spi-generic.h> #include <soc/pci_devs.h> #include <soc/rcba.h> #include <soc/spi.h>
-#ifdef __SMM__ -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#else /* !__SMM__ */ -#include <device/device.h> -#include <device/pci.h> -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#endif /* !__SMM__ */ +/* This file uses PCI accessor in style of flashrom project. */ +#define __PCI_COMPAT_FLASHROM__ +#include <device/pci_compat.h>
typedef struct spi_slave ich_spi_slave;
diff --git a/src/soc/intel/fsp_baytrail/spi.c b/src/soc/intel/fsp_baytrail/spi.c index 41d5150..6ecae37 100644 --- a/src/soc/intel/fsp_baytrail/spi.c +++ b/src/soc/intel/fsp_baytrail/spi.c @@ -22,41 +22,17 @@ #include <delay.h> #include <arch/io.h> #include <console/console.h> +#include <device/device.h> +#include <device/pci.h> #include <spi_flash.h> #include <spi-generic.h>
#include <soc/lpc.h> #include <soc/pci_devs.h>
-#ifdef __SMM__ -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#else /* !__SMM__ */ -#include <device/device.h> -#include <device/pci.h> -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#endif /* !__SMM__ */ +/* This file uses PCI accessor in style of flashrom project. */ +#define __PCI_COMPAT_FLASHROM__ +#include <device/pci_compat.h>
typedef struct spi_slave ich_spi_slave;
@@ -253,7 +229,7 @@ { uint32_t sbase;
-#ifdef __SMM__ +#ifdef __SIMPLE_DEVICE__ pci_devfn_t dev = PCI_DEV(0, LPC_DEV, LPC_FUNC); #else struct device *dev = pcidev_on_root(LPC_DEV, LPC_FUNC); diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c index 4c59399..13e1a44 100644 --- a/src/southbridge/intel/common/spi.c +++ b/src/southbridge/intel/common/spi.c @@ -25,46 +25,21 @@ #include <delay.h> #include <arch/io.h> #include <console/console.h> +#include <device/device.h> #include <device/pci.h> #include <spi_flash.h>
#include <spi-generic.h>
+/* This file uses PCI accessor in style of flashrom project. */ +#define __PCI_COMPAT_FLASHROM__ +#include <device/pci_compat.h> + #define HSFC_FCYCLE_OFF 1 /* 1-2: FLASH Cycle */ #define HSFC_FCYCLE (0x3 << HSFC_FCYCLE_OFF) #define HSFC_FDBC_OFF 8 /* 8-13: Flash Data Byte Count */ #define HSFC_FDBC (0x3f << HSFC_FDBC_OFF)
- -#ifdef __SMM__ -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#else /* !__SMM__ */ -#include <device/device.h> -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#endif /* !__SMM__ */ - static int spi_is_multichip(void);
typedef struct spi_slave ich_spi_slave; diff --git a/src/southbridge/intel/fsp_rangeley/spi.c b/src/southbridge/intel/fsp_rangeley/spi.c index 1571925..462eed6 100644 --- a/src/southbridge/intel/fsp_rangeley/spi.c +++ b/src/southbridge/intel/fsp_rangeley/spi.c @@ -21,42 +21,18 @@ #include <delay.h> #include <arch/io.h> #include <console/console.h> +#include <device/device.h> +#include <device/pci.h> #include <device/pci_ids.h>
#include <spi_flash.h> #include <spi-generic.h>
-static int ich_status_poll(u16 bitmask, int wait_til_set); +/* This file uses PCI accessor in style of flashrom project. */ +#define __PCI_COMPAT_FLASHROM__ +#include <device/pci_compat.h>
-#ifdef __SMM__ -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#else /* !__SMM__ */ -#include <device/device.h> -#include <device/pci.h> -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#endif /* !__SMM__ */ +static int ich_status_poll(u16 bitmask, int wait_til_set);
typedef struct spi_slave ich_spi_slave;
@@ -344,7 +320,7 @@ uint32_t ids; uint16_t vendor_id, device_id;
-#ifdef __SMM__ +#ifdef __SIMPLE_DEVICE__ pci_devfn_t dev = PCI_DEV(0, 31, 0); #else struct device *dev = pcidev_on_root(31, 0);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Define PCI accessor aliases only once ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/31270/1/src/include/device/pci_compat.h File src/include/device/pci_compat.h:
https://review.coreboot.org/#/c/31270/1/src/include/device/pci_compat.h@20 PS1, Line 20: #define pci_read_config_byte(dev, reg, targ)\ Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/#/c/31270/1/src/include/device/pci_compat.h@22 PS1, Line 22: #define pci_read_config_word(dev, reg, targ)\ Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/#/c/31270/1/src/include/device/pci_compat.h@24 PS1, Line 24: #define pci_read_config_dword(dev, reg, targ)\ Macros with complex values should be enclosed in parentheses
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Define PCI accessor aliases only once ......................................................................
Patch Set 1: Code-Review+1
David Guckian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Define PCI accessor aliases only once ......................................................................
Patch Set 1: Code-Review+1
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Define PCI accessor aliases only once ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/#/c/31270/2/src/include/device/pci_compat.h File src/include/device/pci_compat.h:
https://review.coreboot.org/#/c/31270/2/src/include/device/pci_compat.h@20 PS2, Line 20: #define pci_read_config_byte(dev, reg, targ)\ Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/#/c/31270/2/src/include/device/pci_compat.h@22 PS2, Line 22: #define pci_read_config_word(dev, reg, targ)\ Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/#/c/31270/2/src/include/device/pci_compat.h@24 PS2, Line 24: #define pci_read_config_dword(dev, reg, targ)\ Macros with complex values should be enclosed in parentheses
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Define PCI accessor aliases only once ......................................................................
Patch Set 3: Code-Review+2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Define PCI accessor aliases only once ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/#/c/31270/4/src/include/device/pci_compat.h File src/include/device/pci_compat.h:
https://review.coreboot.org/#/c/31270/4/src/include/device/pci_compat.h@20 PS4, Line 20: #define pci_read_config_byte(dev, reg, targ)\ Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/#/c/31270/4/src/include/device/pci_compat.h@22 PS4, Line 22: #define pci_read_config_word(dev, reg, targ)\ Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/#/c/31270/4/src/include/device/pci_compat.h@24 PS4, Line 24: #define pci_read_config_dword(dev, reg, targ)\ Macros with complex values should be enclosed in parentheses
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Define PCI accessor aliases only once ......................................................................
Patch Set 4:
If this comes from flashrom it must have been long ago. flashrom uses libpci. Why not just use coreboot functions proper? (or integrate libflashrom :p)
In any case, I don't think it makes sense to mention flashrom in the source. And ofc, it might be a good idea to consolidate the copies so the next cleanup doesn't have to do the same changes 6 times again.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Define PCI accessor aliases only once ......................................................................
Patch Set 4: Code-Review+1
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Define PCI accessor aliases only once ......................................................................
Patch Set 4:
Patch Set 4:
If this comes from flashrom it must have been long ago. flashrom uses libpci. Why not just use coreboot functions proper? (or integrate libflashrom :p)
In any case, I don't think it makes sense to mention flashrom in the source. And ofc, it might be a good idea to consolidate the copies so the next cleanup doesn't have to do the same changes 6 times again.
Well consolidating the copies is not the scope here, the files have drifted too much to make them one. So would using coreboot proper functions score +2 here?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Define PCI accessor aliases only once ......................................................................
Patch Set 4:
If this comes from flashrom it must have been long ago. flashrom uses libpci. Why not just use coreboot functions proper? (or integrate libflashrom :p)
In any case, I don't think it makes sense to mention flashrom in the source. And ofc, it might be a good idea to consolidate the copies so the next cleanup doesn't have to do the same changes 6 times again.
Well consolidating the copies is not the scope here, the files have drifted too much to make them one. So would using coreboot proper functions score +2 here?
Yes, I guess. It just seems weird to call things flashrom-compat if that is already obsolete infor- mation. So anything that gets rid of that notion would already silence my concerns.
Hello Patrick Rudolph, David Guckian, Angel Pons, Huang Jin, York Yang, Paul Menzel, Philipp Deppenwiese, build bot (Jenkins), David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31270
to look at the new patch set (#5).
Change subject: intel/spi: Switch to native PCI config accessors ......................................................................
intel/spi: Switch to native PCI config accessors
Change-Id: If7190ac105b2a65a9576709955c3cc840b95dcdf Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/baytrail/spi.c M src/soc/intel/braswell/spi.c M src/soc/intel/broadwell/spi.c M src/soc/intel/fsp_baytrail/spi.c M src/southbridge/intel/common/spi.c M src/southbridge/intel/fsp_rangeley/spi.c 6 files changed, 22 insertions(+), 191 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/31270/5
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Switch to native PCI config accessors ......................................................................
Patch Set 5: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Switch to native PCI config accessors ......................................................................
Patch Set 5: Code-Review+2
nice diffstat
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Switch to native PCI config accessors ......................................................................
Patch Set 5: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Switch to native PCI config accessors ......................................................................
Patch Set 5: Code-Review+1
Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31270 )
Change subject: intel/spi: Switch to native PCI config accessors ......................................................................
intel/spi: Switch to native PCI config accessors
Change-Id: If7190ac105b2a65a9576709955c3cc840b95dcdf Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/31270 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Werner Zeh werner.zeh@siemens.com Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Matt DeVillier matt.devillier@gmail.com Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/baytrail/spi.c M src/soc/intel/braswell/spi.c M src/soc/intel/broadwell/spi.c M src/soc/intel/fsp_baytrail/spi.c M src/southbridge/intel/common/spi.c M src/southbridge/intel/fsp_rangeley/spi.c 6 files changed, 22 insertions(+), 191 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Werner Zeh: Looks good to me, approved Matt DeVillier: Looks good to me, but someone else must approve Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/baytrail/spi.c b/src/soc/intel/baytrail/spi.c index 497bfd5..f7472b9 100644 --- a/src/soc/intel/baytrail/spi.c +++ b/src/soc/intel/baytrail/spi.c @@ -21,42 +21,14 @@ #include <arch/io.h> #include <commonlib/helpers.h> #include <console/console.h> +#include <device/device.h> +#include <device/pci.h> #include <spi_flash.h> #include <spi-generic.h>
#include <soc/lpc.h> #include <soc/pci_devs.h>
-#ifdef __SMM__ -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#else /* !__SMM__ */ -#include <device/device.h> -#include <device/pci.h> -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#endif /* !__SMM__ */ - typedef struct spi_slave ich_spi_slave;
static int ichspi_lock = 0; @@ -269,7 +241,7 @@ #else struct device *dev = pcidev_on_root(LPC_DEV, LPC_FUNC); #endif - pci_read_config_dword(dev, SBASE, &sbase); + sbase = pci_read_config32(dev, SBASE); sbase &= ~0x1ff;
return (void *)sbase; diff --git a/src/soc/intel/braswell/spi.c b/src/soc/intel/braswell/spi.c index 66eb53a..14d1087 100644 --- a/src/soc/intel/braswell/spi.c +++ b/src/soc/intel/braswell/spi.c @@ -19,6 +19,8 @@ #include <commonlib/helpers.h> #include <console/console.h> #include <delay.h> +#include <device/device.h> +#include <device/pci.h> #include <soc/lpc.h> #include <soc/pci_devs.h> #include <spi_flash.h> @@ -27,36 +29,6 @@ #include <stdlib.h> #include <string.h>
-#if ENV_SMM -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#else /* ENV_SMM */ -#include <device/device.h> -#include <device/pci.h> -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#endif /* ENV_SMM */ - typedef struct spi_slave ich_spi_slave;
static int ichspi_lock = 0; @@ -242,7 +214,7 @@ return NULL; }
- pci_read_config_dword(dev, SBASE, &sbase); + sbase = pci_read_config32(dev, SBASE); sbase &= ~0x1ff;
return (void *)sbase; diff --git a/src/soc/intel/broadwell/spi.c b/src/soc/intel/broadwell/spi.c index de3d061..5280967 100644 --- a/src/soc/intel/broadwell/spi.c +++ b/src/soc/intel/broadwell/spi.c @@ -20,42 +20,14 @@ #include <delay.h> #include <arch/io.h> #include <console/console.h> +#include <device/device.h> +#include <device/pci.h> #include <spi_flash.h> #include <spi-generic.h> #include <soc/pci_devs.h> #include <soc/rcba.h> #include <soc/spi.h>
-#ifdef __SMM__ -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#else /* !__SMM__ */ -#include <device/device.h> -#include <device/pci.h> -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#endif /* !__SMM__ */ - typedef struct spi_slave ich_spi_slave;
static int ichspi_lock = 0; @@ -271,7 +243,7 @@ #endif ich9_spi_regs *ich9_spi;
- pci_read_config_dword(dev, 0xf0, &rcba); + rcba = pci_read_config32(dev, 0xf0); /* Bits 31-14 are the base address, 13-1 are reserved, 0 is enable. */ rcrb = (uint8_t *)(rcba & 0xffffc000); ich9_spi = (ich9_spi_regs *)(rcrb + 0x3800); @@ -289,9 +261,9 @@ ich_set_bbar(0);
/* Disable the BIOS write protect so write commands are allowed. */ - pci_read_config_byte(dev, 0xdc, &bios_cntl); + bios_cntl = pci_read_config8(dev, 0xdc); bios_cntl &= ~(1 << 5); - pci_write_config_byte(dev, 0xdc, bios_cntl | 0x1); + pci_write_config8(dev, 0xdc, bios_cntl | 0x1); }
static void spi_init_cb(void *unused) diff --git a/src/soc/intel/fsp_baytrail/spi.c b/src/soc/intel/fsp_baytrail/spi.c index a9c0454..275b038 100644 --- a/src/soc/intel/fsp_baytrail/spi.c +++ b/src/soc/intel/fsp_baytrail/spi.c @@ -22,42 +22,14 @@ #include <delay.h> #include <arch/io.h> #include <console/console.h> +#include <device/device.h> +#include <device/pci.h> #include <spi_flash.h> #include <spi-generic.h>
#include <soc/lpc.h> #include <soc/pci_devs.h>
-#ifdef __SMM__ -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#else /* !__SMM__ */ -#include <device/device.h> -#include <device/pci.h> -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#endif /* !__SMM__ */ - typedef struct spi_slave ich_spi_slave;
static int ichspi_lock = 0; @@ -258,7 +230,7 @@ #else struct device *dev = pcidev_on_root(LPC_DEV, LPC_FUNC); #endif - pci_read_config_dword(dev, SBASE, &sbase); + sbase = pci_read_config32(dev, SBASE); sbase &= ~0x1ff;
return (void *)sbase; diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c index 60c0b8d..1d871d2 100644 --- a/src/southbridge/intel/common/spi.c +++ b/src/southbridge/intel/common/spi.c @@ -24,6 +24,7 @@ #include <delay.h> #include <arch/io.h> #include <console/console.h> +#include <device/device.h> #include <device/pci.h> #include <spi_flash.h>
@@ -34,36 +35,6 @@ #define HSFC_FDBC_OFF 8 /* 8-13: Flash Data Byte Count */ #define HSFC_FDBC (0x3f << HSFC_FDBC_OFF)
- -#ifdef __SMM__ -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#else /* !__SMM__ */ -#include <device/device.h> -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#endif /* !__SMM__ */ - static int spi_is_multichip(void);
typedef struct spi_slave ich_spi_slave; @@ -308,7 +279,7 @@ struct device *dev = pcidev_on_root(31, 0); #endif
- pci_read_config_dword(dev, 0xf0, &rcba); + rcba = pci_read_config32(dev, 0xf0); /* Bits 31-14 are the base address, 13-1 are reserved, 0 is enable. */ rcrb = (uint8_t *)(rcba & 0xffffc000); if (IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_I82801GX)) { @@ -356,10 +327,10 @@ ich_set_bbar(0);
/* Disable the BIOS write protect so write commands are allowed. */ - pci_read_config_byte(dev, 0xdc, &bios_cntl); + bios_cntl = pci_read_config8(dev, 0xdc); /* Deassert SMM BIOS Write Protect Disable. */ bios_cntl &= ~(1 << 5); - pci_write_config_byte(dev, 0xdc, bios_cntl | 0x1); + pci_write_config8(dev, 0xdc, bios_cntl | 0x1); }
static void spi_init_cb(void *unused) diff --git a/src/southbridge/intel/fsp_rangeley/spi.c b/src/southbridge/intel/fsp_rangeley/spi.c index e10072a..227422b 100644 --- a/src/southbridge/intel/fsp_rangeley/spi.c +++ b/src/southbridge/intel/fsp_rangeley/spi.c @@ -21,6 +21,8 @@ #include <delay.h> #include <arch/io.h> #include <console/console.h> +#include <device/device.h> +#include <device/pci.h> #include <device/pci_ids.h>
#include <spi_flash.h> @@ -28,36 +30,6 @@
static int ich_status_poll(u16 bitmask, int wait_til_set);
-#ifdef __SMM__ -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#else /* !__SMM__ */ -#include <device/device.h> -#include <device/pci.h> -#define pci_read_config_byte(dev, reg, targ)\ - *(targ) = pci_read_config8(dev, reg) -#define pci_read_config_word(dev, reg, targ)\ - *(targ) = pci_read_config16(dev, reg) -#define pci_read_config_dword(dev, reg, targ)\ - *(targ) = pci_read_config32(dev, reg) -#define pci_write_config_byte(dev, reg, val)\ - pci_write_config8(dev, reg, val) -#define pci_write_config_word(dev, reg, val)\ - pci_write_config16(dev, reg, val) -#define pci_write_config_dword(dev, reg, val)\ - pci_write_config32(dev, reg, val) -#endif /* !__SMM__ */ - typedef struct spi_slave ich_spi_slave;
static int ichspi_lock = 0; @@ -349,7 +321,7 @@ #else struct device *dev = pcidev_on_root(31, 0); #endif - pci_read_config_dword(dev, 0, &ids); + ids = pci_read_config32(dev, 0); vendor_id = ids; device_id = (ids >> 16);
@@ -370,7 +342,7 @@ { uint8_t *spibase; /* SPI Base Address */ uint32_t sbase; /* SPI Base Address Register */ - pci_read_config_dword(dev, 0x54, &sbase); + sbase = pci_read_config32(dev, 0x54); /* Bits 31-9 are the base address, 8-4 are reserved, 3-0 are used. */ spibase = (uint8_t *)(sbase & 0xffffff00); ich10_spi_regs *ich10_spi =
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
David Guckian (Code Review)
-
Kyösti Mälkki (Code Review)
-
Matt DeVillier (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)
-
Werner Zeh (Code Review)