[Skiboot] [PATCH 2/5] phb3: Load CAPP ucode to both CAPP units on Naples

[Michael Neuling]

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.


> 
> 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


> +
>          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



> +			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)


> 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;