[coreboot-gerrit] Change in coreboot[master]: soc/intel/broadwell/sata.c: Simplify RMW operations

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47025 )

Change subject: soc/intel/broadwell/sata.c: Simplify RMW operations
......................................................................

soc/intel/broadwell/sata.c: Simplify RMW operations

Introduce the `sir_unset_and_set_mask` helper and update the one PCI
read-modify-write operation that is somehow not reproducible.

Change-Id: I334623ab19537a6c16fad2ca293522c71c624eee
Signed-off-by: Angel Pons 
---
M src/soc/intel/broadwell/pch/sata.c
1 file changed, 13 insertions(+), 18 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/47025/1

diff --git a/src/soc/intel/broadwell/pch/sata.c b/src/soc/intel/broadwell/pch/sata.c
index 3ed136b..ab73dbd 100644
--- a/src/soc/intel/broadwell/pch/sata.c
+++ b/src/soc/intel/broadwell/pch/sata.c
@@ -27,12 +27,19 @@
    pci_write_config32(dev, SATA_SIRD, value);
 }

+static inline void sir_unset_and_set_mask(struct device *dev, int idx, u32 unset, u32 set)
+{
+	pci_write_config32(dev, SATA_SIRI, idx);
+
+	const u32 value = pci_read_config32(dev, SATA_SIRD) & ~unset;
+	pci_write_config32(dev, SATA_SIRD, value | set);
+}
+
 static void sata_init(struct device *dev)
 {
    const struct soc_intel_broadwell_pch_config *config = config_of(dev);
    u32 reg32;
    u8 *abar;
-	u16 reg16;
    int port;

printk(BIOS_DEBUG, "SATA: Initializing controller in AHCI mode.\n");
@@ -49,10 +56,7 @@
    pci_write_config16(dev, IDE_TIM_SEC, IDE_DECODE_ENABLE);

/* for AHCI, Port Enable is managed in memory mapped space */
-	reg16 = pci_read_config16(dev, 0x92);
-	reg16 &= ~SATA_PORT_MASK;
-	reg16 |= 0x8000 | config->sata_port_map;
-	pci_write_config16(dev, 0x92, reg16);
+	pci_update_config16(dev, 0x92, ~SATA_PORT_MASK, 0x8000 | config->sata_port_map);
    udelay(2);

/* Setup register 98h */
@@ -208,25 +212,16 @@
    sir_write(dev, 0x64, 0x883c9003);

/* Step 2: SIR 68h[15:0] = 880Ah */
-	reg32 = sir_read(dev, 0x68);
-	reg32 &= 0xffff0000;
-	reg32 |= 0x880a;
-	sir_write(dev, 0x68, reg32);
+	sir_unset_and_set_mask(dev, 0x68, 0xffff, 0x880a);

/* Step 3: SIR 60h[3] = 1 */
-	reg32 = sir_read(dev, 0x60);
-	reg32 |= (1 << 3);
-	sir_write(dev, 0x60, reg32);
+	sir_unset_and_set_mask(dev, 0x60, 0, 1 << 3);

/* Step 4: SIR 60h[0] = 1 */
-	reg32 = sir_read(dev, 0x60);
-	reg32 |= (1 << 0);
-	sir_write(dev, 0x60, reg32);
+	sir_unset_and_set_mask(dev, 0x60, 0, 1 << 0);

/* Step 5: SIR 60h[1] = 1 */
-	reg32 = sir_read(dev, 0x60);
-	reg32 |= (1 << 1);
-	sir_write(dev, 0x60, reg32);
+	sir_unset_and_set_mask(dev, 0x60, 0, 1 << 1);

/* Clock Gating */
    sir_write(dev, 0x70, 0x3f00bf1f);