Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add ABAR helpers ......................................................................
sb/intel/common: Add ABAR helpers
These can be used instead of bit twiddling for clarity.
Change-Id: Iffca4533f418340462fc773c2432e30a5abed6ac Signed-off-by: Angel Pons th3fanbus@gmail.com --- A src/southbridge/intel/common/abar.h 1 file changed, 119 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/47023/1
diff --git a/src/southbridge/intel/common/abar.h b/src/southbridge/intel/common/abar.h new file mode 100644 index 0000000..ccfaa01 --- /dev/null +++ b/src/southbridge/intel/common/abar.h @@ -0,0 +1,119 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __SERIAL_ATA_ABAR_H__ +#define __SERIAL_ATA_ABAR_H__ + +#include <arch/mmio.h> +#include <types.h> + +/* Per-port register offset helper */ +#define _ABAR_PORT_OFFSET(x) (0x100 + (x) * 0x80) + +/* Register definitions */ + +#define ABAR_REG_CAP 0x00 +#define ABAR_REG_GHC 0x04 +#define ABAR_REG_IS 0x08 +#define ABAR_REG_PI 0x0c +#define ABAR_REG_AHCI_VER 0x10 +#define ABAR_REG_CCC_CTL 0x14 +#define ABAR_REG_CCC_PORTS 0x18 +#define ABAR_REG_EM_LOC 0x1c +#define ABAR_REG_EM_CTL 0x20 +#define ABAR_REG_CAP_2 0x24 +#define ABAR_REG_BOHC 0x28 + +#define ABAR_REG_PxCLB(x) (0x00 + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxCLBU(x) (0x04 + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxFB(x) (0x08 + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxFBU(x) (0x0c + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxIS(x) (0x10 + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxIE(x) (0x14 + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxCMD(x) (0x18 + _ABAR_PORT_OFFSET(x)) +/* Note that this port register (0x1c + _ABAR_PORT_OFFSET(x)) is not in the AHCI spec */ +#define ABAR_REG_PxTFD(x) (0x20 + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxSIG(x) (0x24 + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxSSTS(x) (0x28 + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxSCTL(x) (0x2c + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxSERR(x) (0x30 + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxSACT(x) (0x34 + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxCI(x) (0x38 + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxSNTF(x) (0x3c + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxFBS(x) (0x40 + _ABAR_PORT_OFFSET(x)) +#define ABAR_REG_PxDEVSLP(x) (0x44 + _ABAR_PORT_OFFSET(x)) + +/* Bitfield definitions */ +union abar_reg_cap { + struct { + uint32_t number_of_ports : 5; // Bits 4: 0 + uint32_t external_sata : 1; // Bits 5: 5 + uint32_t enclosure_management : 1; // Bits 6: 6 + uint32_t cmd_cmpl_coalescing : 1; // Bits 7: 7 + uint32_t num_command_slots : 5; // Bits 12: 8 + uint32_t partial_state : 1; // Bits 13:13 + uint32_t slumber_state : 1; // Bits 14:14 + uint32_t pio_multiple_drq : 1; // Bits 15:15 + uint32_t fis_based_switching : 1; // Bits 16:16 + uint32_t port_multiplier : 1; // Bits 17:17 + uint32_t ahci_mode_only : 1; // Bits 18:18 + uint32_t : 1; // Bits 19:19 + uint32_t interface_speed : 4; // Bits 23:20 + uint32_t cmd_list_override : 1; // Bits 24:24 + uint32_t activity_led : 1; // Bits 25:25 + uint32_t aggressive_link_pm : 1; // Bits 26:26 + uint32_t staggered_spinup : 1; // Bits 27:27 + uint32_t interlock_switch : 1; // Bits 28:28 + uint32_t snotification_reg : 1; // Bits 29:29 + uint32_t native_cmd_queuing : 1; // Bits 30:30 + uint32_t addressing_64bit : 1; // Bits 31:31 + }; + uint32_t raw; +}; + +union abar_reg_cap_2 { + struct { + uint32_t bios_os_handoff : 1; // Bits 0: 0 + uint32_t nvm_hci_present : 1; // Bits 1: 1 + uint32_t auto_partial_to_slumber : 1; // Bits 2: 2 + uint32_t supports_device_sleep : 1; // Bits 3: 3 + uint32_t aggressive_devslp_mgmt : 1; // Bits 4: 4 + uint32_t devslp_entry_slumber_only : 1; // Bits 5: 5 + uint32_t : 26; // Bits 31: 6 + }; + uint32_t raw; +}; + +union abar_reg_px_devslp { + struct { + uint32_t aggressive_devslp_enable : 1; // Bits 0: 0 + uint32_t device_sleep_present : 1; // Bits 1: 1 + uint32_t device_sleep_exit_timeout : 8; // Bits 9: 2 + uint32_t min_devslp_assertion_time : 5; // Bits 14:10 + uint32_t device_sleep_idle_timeout : 10; // Bits 24:15 + uint32_t dito_multiplier : 4; // Bits 28:25 + uint32_t : 3; // Bits 31:29 + }; + uint32_t raw; +}; + +/* Accessors */ +static inline uint32_t abar_read32(const uintptr_t abar, const size_t offset) +{ + return read32((void *)(abar + offset)); +} + +static inline void abar_write32(const uintptr_t abar, const size_t offset, const uint32_t value) +{ + write32((void *)(abar + offset), value); +} + +static inline void abar_write_ports_implemented(const uintptr_t abar, const uint32_t port_map) +{ + abar_write32(abar, ABAR_REG_PI, port_map); + + /* Read back twice */ + abar_read32(abar, ABAR_REG_PI); + abar_read32(abar, ABAR_REG_PI); +} + +#endif /* __SERIAL_ATA_ABAR_H__ */
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add ABAR helpers ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47023/1/src/southbridge/intel/commo... File src/southbridge/intel/common/abar.h:
https://review.coreboot.org/c/coreboot/+/47023/1/src/southbridge/intel/commo... PS1, Line 59: uint32_t : 1; // Bits 19:19 space prohibited before that ':' (ctx:WxW)
https://review.coreboot.org/c/coreboot/+/47023/1/src/southbridge/intel/commo... PS1, Line 81: uint32_t : 26; // Bits 31: 6 space prohibited before that ':' (ctx:WxW)
https://review.coreboot.org/c/coreboot/+/47023/1/src/southbridge/intel/commo... PS1, Line 94: uint32_t : 3; // Bits 31:29 space prohibited before that ':' (ctx:WxW)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add ABAR helpers ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47023/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47023/1//COMMIT_MSG@7 PS1, Line 7: ABAR AHCI (everywhere)
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47023
to look at the new patch set (#2).
Change subject: sb/intel/common: Add AHCI library ......................................................................
sb/intel/common: Add AHCI library
These can be used instead of bit twiddling for clarity.
Change-Id: Iffca4533f418340462fc773c2432e30a5abed6ac Signed-off-by: Angel Pons th3fanbus@gmail.com --- A src/southbridge/intel/common/ahci.h 1 file changed, 118 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/47023/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add AHCI library ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47023/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47023/1//COMMIT_MSG@7 PS1, Line 7: ABAR
AHCI (everywhere)
Done
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add AHCI library ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47023/2/src/southbridge/intel/commo... File src/southbridge/intel/common/ahci.h:
https://review.coreboot.org/c/coreboot/+/47023/2/src/southbridge/intel/commo... PS2, Line 58: uint32_t : 1; // Bits 19:19 space prohibited before that ':' (ctx:WxW)
https://review.coreboot.org/c/coreboot/+/47023/2/src/southbridge/intel/commo... PS2, Line 80: uint32_t : 26; // Bits 31: 6 space prohibited before that ':' (ctx:WxW)
https://review.coreboot.org/c/coreboot/+/47023/2/src/southbridge/intel/commo... PS2, Line 93: uint32_t : 3; // Bits 31:29 space prohibited before that ':' (ctx:WxW)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add AHCI library ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47023/3/src/southbridge/intel/commo... File src/southbridge/intel/common/ahci.h:
https://review.coreboot.org/c/coreboot/+/47023/3/src/southbridge/intel/commo... PS3, Line 58: uint32_t : 1; // Bits 19:19 space prohibited before that ':' (ctx:WxW)
https://review.coreboot.org/c/coreboot/+/47023/3/src/southbridge/intel/commo... PS3, Line 80: uint32_t : 26; // Bits 31: 6 space prohibited before that ':' (ctx:WxW)
https://review.coreboot.org/c/coreboot/+/47023/3/src/southbridge/intel/commo... PS3, Line 93: uint32_t : 3; // Bits 31:29 space prohibited before that ':' (ctx:WxW)
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add AHCI library ......................................................................
Patch Set 3: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/47023/3/src/southbridge/intel/commo... File src/southbridge/intel/common/ahci.h:
https://review.coreboot.org/c/coreboot/+/47023/3/src/southbridge/intel/commo... PS3, Line 32: /* Note that this port register (0x1c + _AHCI_PORT_OFFSET(x)) is not in the AHCI spec */ why add it (even as comment) when it's not there?
https://review.coreboot.org/c/coreboot/+/47023/3/src/southbridge/intel/commo... PS3, Line 47: // don't mix comment style
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add AHCI library ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47023/3/src/southbridge/intel/commo... File src/southbridge/intel/common/ahci.h:
https://review.coreboot.org/c/coreboot/+/47023/3/src/southbridge/intel/commo... PS3, Line 32: /* Note that this port register (0x1c + _AHCI_PORT_OFFSET(x)) is not in the AHCI spec */
why add it (even as comment) when it's not there?
I didn't notice it was missing at first, and since the other registers are contiguous, I placed this comment to fill in the gap.
Hello build bot (Jenkins), Michael Niewöhner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47023
to look at the new patch set (#4).
Change subject: sb/intel/common: Add AHCI library ......................................................................
sb/intel/common: Add AHCI library
These can be used instead of bit twiddling for clarity.
Change-Id: Iffca4533f418340462fc773c2432e30a5abed6ac Signed-off-by: Angel Pons th3fanbus@gmail.com --- A src/southbridge/intel/common/ahci.h 1 file changed, 118 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/47023/4
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add AHCI library ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47023/3/src/southbridge/intel/commo... File src/southbridge/intel/common/ahci.h:
https://review.coreboot.org/c/coreboot/+/47023/3/src/southbridge/intel/commo... PS3, Line 32: /* Note that this port register (0x1c + _AHCI_PORT_OFFSET(x)) is not in the AHCI spec */
I didn't notice it was missing at first, and since the other registers are contiguous, I placed this […]
Ack
https://review.coreboot.org/c/coreboot/+/47023/3/src/southbridge/intel/commo... PS3, Line 47: //
don't mix comment style
Done
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add AHCI library ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47023/4/src/southbridge/intel/commo... File src/southbridge/intel/common/ahci.h:
https://review.coreboot.org/c/coreboot/+/47023/4/src/southbridge/intel/commo... PS4, Line 58: uint32_t : 1; space prohibited before that ':' (ctx:WxW)
https://review.coreboot.org/c/coreboot/+/47023/4/src/southbridge/intel/commo... PS4, Line 80: uint32_t : 26; space prohibited before that ':' (ctx:WxW)
https://review.coreboot.org/c/coreboot/+/47023/4/src/southbridge/intel/commo... PS4, Line 93: uint32_t : 3; space prohibited before that ':' (ctx:WxW)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add AHCI library ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47023/5/src/southbridge/intel/commo... File src/southbridge/intel/common/ahci.h:
https://review.coreboot.org/c/coreboot/+/47023/5/src/southbridge/intel/commo... PS5, Line 58: uint32_t : 1; space prohibited before that ':' (ctx:WxW)
https://review.coreboot.org/c/coreboot/+/47023/5/src/southbridge/intel/commo... PS5, Line 80: uint32_t : 26; space prohibited before that ':' (ctx:WxW)
https://review.coreboot.org/c/coreboot/+/47023/5/src/southbridge/intel/commo... PS5, Line 93: uint32_t : 3; space prohibited before that ':' (ctx:WxW)
Attention is currently required from: Angel Pons. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add AHCI library ......................................................................
Patch Set 5: Code-Review+1
(3 comments)
File src/southbridge/intel/common/ahci.h:
https://review.coreboot.org/c/coreboot/+/47023/comment/bdd32ddd_3a7e4321 PS5, Line 3: #ifndef __SOUTHBRIDGE_INTEL_COMMON_AHCI_H__ besides 0x1c is there anything intel specific here? This should be moved to src/include
https://review.coreboot.org/c/coreboot/+/47023/comment/2e3131b2_ed2650f0 PS5, Line 46: struct { once in src/include/ this and the other structs must also support big endian.
https://review.coreboot.org/c/coreboot/+/47023/comment/4c93f0e7_c026eec6 PS5, Line 118: __SERIAL_ATA_AHCI_H__ header guard doesn't match
Attention is currently required from: Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47023 )
Change subject: sb/intel/common: Add AHCI library ......................................................................
Patch Set 5:
(2 comments)
File src/southbridge/intel/common/ahci.h:
https://review.coreboot.org/c/coreboot/+/47023/comment/d8168ab2_96a0a496 PS5, Line 3: #ifndef __SOUTHBRIDGE_INTEL_COMMON_AHCI_H__
besides 0x1c is there anything intel specific here? […]
Which 0x1c?
I don't know if there's any Intel-specific stuff in here.
https://review.coreboot.org/c/coreboot/+/47023/comment/4269f279_f8b19f94 PS5, Line 46: struct {
once in src/include/ this and the other structs must also support big endian.
Agreed. If this ends up in src/include, the bitfields will have to go.
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/47023?usp=email )
Change subject: sb/intel/common: Add AHCI library ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Martin L Roth (Code Review)
-
Michael Niewöhner (Code Review)
-
Patrick Rudolph (Code Review)