View Change

8 comments:

• File src/soc/amd/picasso/data_fabric.c:

  • Patch Set #2, Line 176: return pci_read_config32(_SOC_DEV(DF_DEV, function), reg);

    it might be useful to add a comment that the macros will apply a bit mask to the values, so no bit m […]

    Done

  • Patch Set #2, Line 180: * requested offset.*/

    Remove leading asterisk and add space in front of */ to match the short multi-line style. […]

    Done

  • Patch Set #2, Line 181: reg

    reg is the byte address and the two last bits get masked off, since the access is always dword/qword […]

    Yes, this is always aligned so they omit the last bits

• File src/soc/amd/picasso/include/soc/data_fabric.h:

  • Patch Set #2, Line 10: 0xFF

    Use lower case for hex numbers here, line 31, 37, etc.

    Done

  • Patch Set #2, Line 28: #define D18F0_DRAM_HOLE_CTL 0x104

    Unless I'm mistaken, lines 28-56 are unrelated to the "add data fabric helper". […]

    Done

  • Patch Set #2, Line 28:

    #define D18F0_DRAM_HOLE_CTL		0x104
    #define   DRAM_HOLE_CTL_VALID		BIT(0)
    #define   DRAM_HOLE_CTL_BASE_SHFT	24
    #define   DRAM_HOLE_CTL_BASE		(0xFF << DRAM_HOLE_CTL_BASE_SHFT)
    
    #define D18F0_DRAM_BASE0		0x110
    #define   DRAM_BASE_REG_VALID		BIT(0)
    #define   DRAM_BASE_HOLE_EN		BIT(1)
    #define   DRAM_BASE_INTLV_CH_SHFT	4
    #define   DRAM_BASE_INTLV_CH		(0xF << DRAM_BASE_INTLV_CH_SHFT)
    #define   DRAM_BASE_INTLV_SEL_SHFT	8
    #define   DRAM_BASE_INTLV_SEL		(0x7 << DRAM_BASE_INTLV_SEL_SHFT)
    #define   DRAM_BASE_ADDR_SHFT		12
    #define   DRAM_BASE_ADDR		(0xFFFFF << DRAM_BASE_ADDR_SHFT)
    
    #define D18F0_DRAM_LIMIT0			0x114
    #define   DRAM_LIMIT_DST_ID_SHFT		0
    #define   DRAM_LIMIT_DST_ID			(0xFF << DRAM_LIMIT_DST_ID_SHFT)
    #define   DRAM_LIMIT_INTLV_NUM_SOCK_SHFT	8
    #define   DRAM_LIMIT_INTLV_NUM_SOCK		(0x1 << DRAM_LIMIT_INTLV_NUM_SOCK_SHFT)
    #define   DRAM_LIMIT_INTLV_NUM_DIE_SHFT		10
    #define   DRAM_LIMIT_INTLV_NUM_DIE		(0x3 << DRAM_LIMIT_INTLV_NUM_DIE_SHFT)
    #define   DRAM_LIMIT_ADDR_SHFT			12
    #define   DRAM_LIMIT_ADDR			(0xFFFFF << DRAM_LIMIT_ADDR_SHFT)
    
    #define DF_DRAM_BASE(dram_map_pair)	((dram_map_pair) * 2 * sizeof(uint32_t) \
    						+ D18F0_DRAM_BASE0)
    #define DF_DRAM_LIMIT(dram_map_pair)	((dram_map_pair) * 2 * sizeof(uint32_t) \
    						+ D18F0_DRAM_LIMIT0)
    

    i'd add those in the next commit, since they aren't needed or used for what this patch adds

    Done

  • Patch Set #2, Line 62: BIT(0)

    i'd prefer (1 << 0) over BIT(0), but both will work. your call.

    Done

  • Patch Set #2, Line 70: DF_IND_CFG_INST_ID

    is this a mask? if so, i'dd ad _MASK as suffix

    Done

To view, visit change 47726. To unsubscribe, or for help writing mail filters, visit settings.