[Skiboot] [PATCH 2/5] phb3: Load CAPP ucode to both CAPP units on Naples
Philippe Bergheaud
felix at linux.vnet.ibm.com
Wed Feb 10 23:18:38 AEDT 2016
Michael Neuling wrote:
> On Mon, 2016-02-08 at 16:30 +0100, Philippe Bergheaud wrote:
>
>
>>In capp_load_ucode, check for phb3 revision and load capp unit 0 if phb0,
>>or capp unit 1 if phb1. Write to the scoms registers of the targeted capp.
>
>
> Needs a longer description of what you're changing here.
>
Yes.
>
>>Signed-off-by: Philippe Bergheaud <felix at linux.vnet.ibm.com>
>>---
>> hw/phb3.c | 43 +++++++++++++++++++++++++++----------------
>> include/capp.h | 10 ++++++++++
>> include/chip.h | 2 +-
>> 3 files changed, 38 insertions(+), 17 deletions(-)
>>
>>diff --git a/hw/phb3.c b/hw/phb3.c
>>index adff5bc..e4f131d 100644
>>--- a/hw/phb3.c
>>+++ b/hw/phb3.c
>>@@ -2314,6 +2314,10 @@ static struct {
>>
>> #define CAPP_UCODE_MAX_SIZE 0x20000
>>
>>+#define CAPP_UCODE_LOADED(chip, p) \
>>+ ((p->rev < PHB3_REV_NAPLES_DD10) ? chip->capp_ucode_loaded : \
>>+ (chip->capp_ucode_loaded & (1 << p->index)))
>>+
>> static int64_t capp_lid_download(void)
>> {
>> int64_t ret;
>>@@ -2346,12 +2350,18 @@ static int64_t capp_load_ucode(struct phb3 *p)
>> struct capp_ucode_data *data;
>> struct capp_lid_hdr *lid;
>> uint64_t rc, val, addr;
>>- uint32_t chunk_count, offset;
>>+ uint32_t chunk_count, offset, reg_offset;
>> int i;
>>
>>- if (chip->capp_ucode_loaded)
>>+ if (CAPP_UCODE_LOADED(chip, p))
>> return OPAL_SUCCESS;
>>
>>+ /* Return if PHB not attached to a CAPP unit */
>>+ if ((p->rev < PHB3_REV_NAPLES_DD10) && (p->index > 2))
>>+ return OPAL_HARDWARE;
>>+ else if (p->index > 1)
>>+ return OPAL_HARDWARE;
>
>
> Isn't the else statement going to break running phb anything other
> than phb = 0 on venice and murano? We do this a lot for open power
> boxes.
>
> I think this is wrong
>
Yes, you are right. I will fix this broken logic.
>
>>+
>> rc = capp_lid_download();
>> if (rc)
>> return rc;
>>@@ -2392,26 +2402,27 @@ static int64_t capp_load_ucode(struct phb3 *p)
>> return OPAL_HARDWARE;
>> }
>>
>>+ reg_offset = CAPP_REG_OFFSET(p->rev, p->index);
>> switch (data->hdr.reg) {
>> case apc_master_cresp:
>>- xscom_write(p->chip_id, CAPP_APC_MASTER_ARRAY_ADDR_REG,
>>- 0);
>>- addr = CAPP_APC_MASTER_ARRAY_WRITE_REG;
>>+ addr = CAPP_APC_MASTER_ARRAY_ADDR_REG + reg_offset;
>>+ xscom_write(p->chip_id, addr, 0);
>
>
> I'd probably just do
> xscom_write(p->chip_id, CAPP_APC_MASTER_ARRAY_ADDR_REG + off, 0);
> rather than setting addr = all the time.
>
> but that's just a style thing
>
I chose the adr = style to avoid folding each long xscom_write() line.
I will try the alternative. Both options require two lines anyway. :)
>
>
>>+ addr = CAPP_APC_MASTER_ARRAY_WRITE_REG + reg_offset;
>> break;
>> case apc_master_uop_table:
>>- xscom_write(p->chip_id, CAPP_APC_MASTER_ARRAY_ADDR_REG,
>>- 0x180ULL << 52);
>>- addr = CAPP_APC_MASTER_ARRAY_WRITE_REG;
>>+ addr = CAPP_APC_MASTER_ARRAY_ADDR_REG + reg_offset;
>>+ xscom_write(p->chip_id, addr, 0x180ULL << 52);
>>+ addr = CAPP_APC_MASTER_ARRAY_WRITE_REG + reg_offset;
>> break;
>> case snp_ttype:
>>- xscom_write(p->chip_id, CAPP_SNP_ARRAY_ADDR_REG,
>>- 0x5000ULL << 48);
>>- addr = CAPP_SNP_ARRAY_WRITE_REG;
>>+ addr = CAPP_SNP_ARRAY_ADDR_REG + reg_offset;
>>+ xscom_write(p->chip_id, addr, 0x5000ULL << 48);
>>+ addr = CAPP_SNP_ARRAY_WRITE_REG + reg_offset;
>> break;
>> case snp_uop_table:
>>- xscom_write(p->chip_id, CAPP_SNP_ARRAY_ADDR_REG,
>>- 0x4000ULL << 48);
>>- addr = CAPP_SNP_ARRAY_WRITE_REG;
>>+ addr = CAPP_SNP_ARRAY_ADDR_REG + reg_offset;
>>+ xscom_write(p->chip_id, addr, 0x4000ULL << 48);
>>+ addr = CAPP_SNP_ARRAY_WRITE_REG + reg_offset;
>> break;
>> default:
>> continue;
>>@@ -2423,7 +2434,7 @@ static int64_t capp_load_ucode(struct phb3 *p)
>> }
>> }
>>
>>- chip->capp_ucode_loaded = true;
>>+ chip->capp_ucode_loaded |= (1 << p->index);
>> return OPAL_SUCCESS;
>> }
>>
>>@@ -3343,7 +3354,7 @@ static int64_t phb3_set_capi_mode(struct phb *phb, uint64_t mode,
>> int i;
>> u8 mask;
>>
>>- if (!chip->capp_ucode_loaded) {
>>+ if (!CAPP_UCODE_LOADED(chip, p)) {
>> PHBERR(p, "CAPP: ucode not loaded\n");
>> return OPAL_RESOURCE;
>> }
>>diff --git a/include/capp.h b/include/capp.h
>>index d9275ec..984bc5a 100644
>>--- a/include/capp.h
>>+++ b/include/capp.h
>>@@ -80,3 +80,13 @@ enum capp_reg {
>> #define FLUSH_UOP_CONFIG1 0x2013803
>> #define FLUSH_UOP_CONFIG2 0x2013804
>> #define SNOOP_CAPI_CONFIG 0x201301A
>>+
>>+/*
>>+ * Naples has two CAPP units, statically mapped:
>>+ * CAPP0 attached to PHB0, and CAPP1 attached to PHB1.
>>+ * The addresses of CAPP1 XSCOMS registers are 0x180 away.
>>+ */
>>+#define CAPP1_REG_OFFSET 0x180
>>+
>>+#define CAPP_REG_OFFSET(rev, index) \
>>+ ((index && rev >= PHB3_REV_NAPLES_DD10) ? CAPP1_REG_OFFSET : 0x0)
>
>
> Needs more braces here. ie
>
> #define CAPP_REG_OFFSET(rev, index) \
> (((index) && (rev) >= PHB3_REV_NAPLES_DD10) ? CAPP1_REG_OFFSET : 0x0)
>
Yes, thank you.
Philippe
>>diff --git a/include/chip.h b/include/chip.h
>>index 5109e25..5225095 100644
>>--- a/include/chip.h
>>+++ b/include/chip.h
>>@@ -159,7 +159,7 @@ struct proc_chip {
>>
>> /* Must hold capi_lock to change */
>> u8 capp_phb3_attached_mask;
>>- bool capp_ucode_loaded;
>>+ u8 capp_ucode_loaded;
>>
>> /* Used by hw/centaur.c */
>> struct centaur_chip *centaurs
>
> ;
More information about the Skiboot
mailing list