| Message ID | c6c3af60a84f7111e4e5a018c6c4b8867771f883.1534495426.git-series.andrew.donnellan@au1.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | OpenCAPI support for Witherspoon | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | master/apply_patch Successfully applied |
On Fri, Aug 17, 2018 at 6:44 PM, Andrew Donnellan <andrew.donnellan@au1.ibm.com> wrote: > In platform_ocapi, we define i2c_{reset,presence}_odl{0,1} to specify the > appropriate reset/presence GPIO pins for devices connected to ODL0 and ODL1 > respectively. > > This is obviously wrong, because a device connected to brick 2 and a device > connected to brick 4 are going to be different devices connected to > different I2C pins, but rather conveniently we haven't had to deal with > systems that can use the full 4 bricks as yet. Now that we're adding > OpenCAPI support for Witherspoon, we should change this to specify pins > separately for all 4 bricks. > > Replace i2c_{reset,presence}_odl{0,1} with > i2c_{reset,presence}_pin{2,3,4,5} and update the presence detection code, That naming convention is amazingly bad. Can't you call it brick2_presence or something? > device reset code, and existing platforms accordingly. > > Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> > --- > core/platform.c | 12 ++++++++---- > hw/npu2-common.c | 9 +++++++-- > hw/npu2-opencapi.c | 10 +++++++--- > include/platform.h | 12 ++++++++---- > platforms/astbmc/zaius.c | 12 ++++++++---- > platforms/ibm-fsp/zz.c | 12 ++++++++---- > 6 files changed, 46 insertions(+), 21 deletions(-) > > diff --git a/core/platform.c b/core/platform.c > index b32cbf5ce9d1..85b2cd8f72ee 100644 > --- a/core/platform.c > +++ b/core/platform.c > @@ -174,11 +174,15 @@ const struct platform_ocapi generic_ocapi = { > .i2c_engine = 1, > .i2c_port = 4, > .i2c_reset_addr = 0x20, > - .i2c_reset_odl0 = (1 << 1), > - .i2c_reset_odl1 = (1 << 6), > + .i2c_reset_pin2 = (1 << 1), > + .i2c_reset_pin3 = (1 << 6), > + .i2c_reset_pin4 = 0, /* unused */ > + .i2c_reset_pin5 = 0, /* unused */ > .i2c_presence_addr = 0x20, > - .i2c_presence_odl0 = (1 << 2), /* bottom connector */ > - .i2c_presence_odl1 = (1 << 7), /* top connector */ > + .i2c_presence_pin2 = (1 << 2), /* bottom connector */ > + .i2c_presence_pin3 = (1 << 7), /* top connector */ > + .i2c_presence_pin4 = 0, /* unused */ > + .i2c_presence_pin5 = 0, /* unused */ > /* > * The ZZs we typically use for BML/generic platform tend to > * have old planars and presence detection is broken there, so > diff --git a/hw/npu2-common.c b/hw/npu2-common.c > index cceb88706981..8151df3d9ba5 100644 > --- a/hw/npu2-common.c > +++ b/hw/npu2-common.c > @@ -115,11 +115,16 @@ static bool _i2c_presence_detect(struct npu2_dev *dev) > > switch (dev->link_index) { > case 2: > - data = platform.ocapi->i2c_presence_odl0; > + data = platform.ocapi->i2c_presence_pin2; > break; > case 3: > - data = platform.ocapi->i2c_presence_odl1; > + data = platform.ocapi->i2c_presence_pin3; > break; > + case 4: > + data = platform.ocapi->i2c_presence_pin4; > + break; > + case 5: > + data = platform.ocapi->i2c_presence_pin5; Can we make it an array or something rather than having switch statements all over the place? That's probably something for another patch though. > default: > OCAPIERR(dev, "presence detection on invalid link\n"); > return true; > diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c > index fa28633bb150..ce5f99114463 100644 > --- a/hw/npu2-opencapi.c > +++ b/hw/npu2-opencapi.c > @@ -816,12 +816,16 @@ static void assert_reset(struct npu2_dev *dev) > > switch (dev->brick_index) { > case 2: > - case 4: > - pin = platform.ocapi->i2c_reset_odl0; > + pin = platform.ocapi->i2c_reset_pin2; > break; > case 3: > + pin = platform.ocapi->i2c_reset_pin3; > + break; > + case 4: > + pin = platform.ocapi->i2c_reset_pin4; > + break; > case 5: > - pin = platform.ocapi->i2c_reset_odl1; > + pin = platform.ocapi->i2c_reset_pin5; > break; > default: > assert(false); > diff --git a/include/platform.h b/include/platform.h > index efafbd2239b9..76d001547d05 100644 > --- a/include/platform.h > +++ b/include/platform.h > @@ -50,11 +50,15 @@ struct platform_ocapi { > uint8_t i2c_engine; /* I2C engine number */ > uint8_t i2c_port; /* I2C port number */ > uint8_t i2c_reset_addr; /* I2C address for reset */ > - uint8_t i2c_reset_odl0; /* I2C pin to write to reset ODL0 */ > - uint8_t i2c_reset_odl1; /* I2C pin to write to reset ODL1 */ > + uint8_t i2c_reset_pin2; /* I2C pin to write to reset brick 2 */ > + uint8_t i2c_reset_pin3; /* I2C pin to write to reset brick 3 */ > + uint8_t i2c_reset_pin4; /* I2C pin to write to reset brick 4 */ > + uint8_t i2c_reset_pin5; /* I2C pin to write to reset brick 5 */ > uint8_t i2c_presence_addr; /* I2C address for presence detection */ > - uint8_t i2c_presence_odl0; /* I2C mask for detection on ODL0 */ > - uint8_t i2c_presence_odl1; /* I2C mask for detection on ODL1 */ > + uint8_t i2c_presence_pin2; /* I2C mask for detection on brick 2 */ > + uint8_t i2c_presence_pin3; /* I2C mask for detection on brick 3 */ > + uint8_t i2c_presence_pin4; /* I2C mask for detection on brick 4 */ > + uint8_t i2c_presence_pin5; /* I2C mask for detection on brick 5 */ What is an I2C mask even? > bool force_presence; /* don't use i2c detection */ > bool odl_phy_swap; /* Swap ODL1 to use brick 2 rather than > * brick 1 lanes */ If we support all four bricks is this still meaningful? > diff --git a/platforms/astbmc/zaius.c b/platforms/astbmc/zaius.c > index 069ce5751550..95d59e8feee9 100644 > --- a/platforms/astbmc/zaius.c > +++ b/platforms/astbmc/zaius.c > @@ -28,11 +28,15 @@ const struct platform_ocapi zaius_ocapi = { > .i2c_engine = 1, > .i2c_port = 4, > .i2c_reset_addr = 0x20, > - .i2c_reset_odl0 = (1 << 1), > - .i2c_reset_odl1 = (1 << 6), > + .i2c_reset_pin2 = (1 << 1), > + .i2c_reset_pin3 = (1 << 6), > + .i2c_reset_pin4 = 0, /* unused */ > + .i2c_reset_pin5 = 0, /* unused */ > .i2c_presence_addr = 0x20, > - .i2c_presence_odl0 = (1 << 2), /* bottom connector */ > - .i2c_presence_odl1 = (1 << 7), /* top connector */ > + .i2c_presence_pin2 = (1 << 2), /* bottom connector */ > + .i2c_presence_pin3 = (1 << 7), /* top connector */ > + .i2c_presence_pin4 = 0, /* unused */ > + .i2c_presence_pin5 = 0, /* unused */ > .odl_phy_swap = true, > }; > > diff --git a/platforms/ibm-fsp/zz.c b/platforms/ibm-fsp/zz.c > index dc48768b8a96..800ad024314f 100644 > --- a/platforms/ibm-fsp/zz.c > +++ b/platforms/ibm-fsp/zz.c > @@ -32,11 +32,15 @@ const struct platform_ocapi zz_ocapi = { > .i2c_engine = 1, > .i2c_port = 4, > .i2c_reset_addr = 0x20, > - .i2c_reset_odl0 = (1 << 1), > - .i2c_reset_odl1 = (1 << 6), > + .i2c_reset_pin2 = (1 << 1), > + .i2c_reset_pin3 = (1 << 6), > + .i2c_reset_pin4 = 0, /* unused */ > + .i2c_reset_pin5 = 0, /* unused */ > .i2c_presence_addr = 0x20, > - .i2c_presence_odl0 = (1 << 2), /* bottom connector */ > - .i2c_presence_odl1 = (1 << 7), /* top connector */ > + .i2c_presence_pin2 = (1 << 2), /* bottom connector */ > + .i2c_presence_pin3 = (1 << 7), /* top connector */ > + .i2c_presence_pin4 = 0, /* unused */ > + .i2c_presence_pin5 = 0, /* unused */ > /* > * i2c presence detection is broken on ZZ planar < v4 so we > * force the presence until all our systems are upgraded > -- > git-series 0.9.1 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
On 23/08/18 11:28, Oliver wrote: > On Fri, Aug 17, 2018 at 6:44 PM, Andrew Donnellan > <andrew.donnellan@au1.ibm.com> wrote: >> In platform_ocapi, we define i2c_{reset,presence}_odl{0,1} to specify the >> appropriate reset/presence GPIO pins for devices connected to ODL0 and ODL1 >> respectively. >> >> This is obviously wrong, because a device connected to brick 2 and a device >> connected to brick 4 are going to be different devices connected to >> different I2C pins, but rather conveniently we haven't had to deal with >> systems that can use the full 4 bricks as yet. Now that we're adding >> OpenCAPI support for Witherspoon, we should change this to specify pins >> separately for all 4 bricks. >> >> Replace i2c_{reset,presence}_odl{0,1} with >> i2c_{reset,presence}_pin{2,3,4,5} and update the presence detection code, > > That naming convention is amazingly bad. Can't you call it > brick2_presence or something? ACK > >> device reset code, and existing platforms accordingly. >> >> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> >> --- >> core/platform.c | 12 ++++++++---- >> hw/npu2-common.c | 9 +++++++-- >> hw/npu2-opencapi.c | 10 +++++++--- >> include/platform.h | 12 ++++++++---- >> platforms/astbmc/zaius.c | 12 ++++++++---- >> platforms/ibm-fsp/zz.c | 12 ++++++++---- >> 6 files changed, 46 insertions(+), 21 deletions(-) >> >> diff --git a/core/platform.c b/core/platform.c >> index b32cbf5ce9d1..85b2cd8f72ee 100644 >> --- a/core/platform.c >> +++ b/core/platform.c >> @@ -174,11 +174,15 @@ const struct platform_ocapi generic_ocapi = { >> .i2c_engine = 1, >> .i2c_port = 4, >> .i2c_reset_addr = 0x20, >> - .i2c_reset_odl0 = (1 << 1), >> - .i2c_reset_odl1 = (1 << 6), >> + .i2c_reset_pin2 = (1 << 1), >> + .i2c_reset_pin3 = (1 << 6), >> + .i2c_reset_pin4 = 0, /* unused */ >> + .i2c_reset_pin5 = 0, /* unused */ >> .i2c_presence_addr = 0x20, >> - .i2c_presence_odl0 = (1 << 2), /* bottom connector */ >> - .i2c_presence_odl1 = (1 << 7), /* top connector */ >> + .i2c_presence_pin2 = (1 << 2), /* bottom connector */ >> + .i2c_presence_pin3 = (1 << 7), /* top connector */ >> + .i2c_presence_pin4 = 0, /* unused */ >> + .i2c_presence_pin5 = 0, /* unused */ >> /* >> * The ZZs we typically use for BML/generic platform tend to >> * have old planars and presence detection is broken there, so >> diff --git a/hw/npu2-common.c b/hw/npu2-common.c >> index cceb88706981..8151df3d9ba5 100644 >> --- a/hw/npu2-common.c >> +++ b/hw/npu2-common.c >> @@ -115,11 +115,16 @@ static bool _i2c_presence_detect(struct npu2_dev *dev) >> >> switch (dev->link_index) { >> case 2: >> - data = platform.ocapi->i2c_presence_odl0; >> + data = platform.ocapi->i2c_presence_pin2; >> break; >> case 3: >> - data = platform.ocapi->i2c_presence_odl1; >> + data = platform.ocapi->i2c_presence_pin3; >> break; >> + case 4: >> + data = platform.ocapi->i2c_presence_pin4; >> + break; >> + case 5: >> + data = platform.ocapi->i2c_presence_pin5; > > Can we make it an array or something rather than having switch > statements all over the place? That's probably something for another > patch though. We could, this mostly follows tradition in npu2-opencapi.c in dealing with register addresses but this could be made into an array if people would rather it be one, it would be a bit cleaner > >> default: >> OCAPIERR(dev, "presence detection on invalid link\n"); >> return true; >> diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c >> index fa28633bb150..ce5f99114463 100644 >> --- a/hw/npu2-opencapi.c >> +++ b/hw/npu2-opencapi.c >> @@ -816,12 +816,16 @@ static void assert_reset(struct npu2_dev *dev) >> >> switch (dev->brick_index) { >> case 2: >> - case 4: >> - pin = platform.ocapi->i2c_reset_odl0; >> + pin = platform.ocapi->i2c_reset_pin2; >> break; >> case 3: >> + pin = platform.ocapi->i2c_reset_pin3; >> + break; >> + case 4: >> + pin = platform.ocapi->i2c_reset_pin4; >> + break; >> case 5: >> - pin = platform.ocapi->i2c_reset_odl1; >> + pin = platform.ocapi->i2c_reset_pin5; >> break; >> default: >> assert(false); >> diff --git a/include/platform.h b/include/platform.h >> index efafbd2239b9..76d001547d05 100644 >> --- a/include/platform.h >> +++ b/include/platform.h >> @@ -50,11 +50,15 @@ struct platform_ocapi { >> uint8_t i2c_engine; /* I2C engine number */ >> uint8_t i2c_port; /* I2C port number */ >> uint8_t i2c_reset_addr; /* I2C address for reset */ >> - uint8_t i2c_reset_odl0; /* I2C pin to write to reset ODL0 */ >> - uint8_t i2c_reset_odl1; /* I2C pin to write to reset ODL1 */ >> + uint8_t i2c_reset_pin2; /* I2C pin to write to reset brick 2 */ >> + uint8_t i2c_reset_pin3; /* I2C pin to write to reset brick 3 */ >> + uint8_t i2c_reset_pin4; /* I2C pin to write to reset brick 4 */ >> + uint8_t i2c_reset_pin5; /* I2C pin to write to reset brick 5 */ >> uint8_t i2c_presence_addr; /* I2C address for presence detection */ >> - uint8_t i2c_presence_odl0; /* I2C mask for detection on ODL0 */ >> - uint8_t i2c_presence_odl1; /* I2C mask for detection on ODL1 */ >> + uint8_t i2c_presence_pin2; /* I2C mask for detection on brick 2 */ >> + uint8_t i2c_presence_pin3; /* I2C mask for detection on brick 3 */ >> + uint8_t i2c_presence_pin4; /* I2C mask for detection on brick 4 */ >> + uint8_t i2c_presence_pin5; /* I2C mask for detection on brick 5 */ > > What is an I2C mask even? The bitmask against which we AND the value we read via i2c. It's not actually any different from the reset pins described above so I'm not sure why I didn't use consistent comment terminology there to begin with... > >> bool force_presence; /* don't use i2c detection */ > >> bool odl_phy_swap; /* Swap ODL1 to use brick 2 rather than >> * brick 1 lanes */ > > If we support all four bricks is this still meaningful? Yeah that could be worded less confusingly I guess. But we do, at present, apply this to both obus0 and obus3 so the lanes for ODL1 on each obus are swapped. > >> diff --git a/platforms/astbmc/zaius.c b/platforms/astbmc/zaius.c >> index 069ce5751550..95d59e8feee9 100644 >> --- a/platforms/astbmc/zaius.c >> +++ b/platforms/astbmc/zaius.c >> @@ -28,11 +28,15 @@ const struct platform_ocapi zaius_ocapi = { >> .i2c_engine = 1, >> .i2c_port = 4, >> .i2c_reset_addr = 0x20, >> - .i2c_reset_odl0 = (1 << 1), >> - .i2c_reset_odl1 = (1 << 6), >> + .i2c_reset_pin2 = (1 << 1), >> + .i2c_reset_pin3 = (1 << 6), >> + .i2c_reset_pin4 = 0, /* unused */ >> + .i2c_reset_pin5 = 0, /* unused */ >> .i2c_presence_addr = 0x20, >> - .i2c_presence_odl0 = (1 << 2), /* bottom connector */ >> - .i2c_presence_odl1 = (1 << 7), /* top connector */ >> + .i2c_presence_pin2 = (1 << 2), /* bottom connector */ >> + .i2c_presence_pin3 = (1 << 7), /* top connector */ >> + .i2c_presence_pin4 = 0, /* unused */ >> + .i2c_presence_pin5 = 0, /* unused */ >> .odl_phy_swap = true, >> }; >> >> diff --git a/platforms/ibm-fsp/zz.c b/platforms/ibm-fsp/zz.c >> index dc48768b8a96..800ad024314f 100644 >> --- a/platforms/ibm-fsp/zz.c >> +++ b/platforms/ibm-fsp/zz.c >> @@ -32,11 +32,15 @@ const struct platform_ocapi zz_ocapi = { >> .i2c_engine = 1, >> .i2c_port = 4, >> .i2c_reset_addr = 0x20, >> - .i2c_reset_odl0 = (1 << 1), >> - .i2c_reset_odl1 = (1 << 6), >> + .i2c_reset_pin2 = (1 << 1), >> + .i2c_reset_pin3 = (1 << 6), >> + .i2c_reset_pin4 = 0, /* unused */ >> + .i2c_reset_pin5 = 0, /* unused */ >> .i2c_presence_addr = 0x20, >> - .i2c_presence_odl0 = (1 << 2), /* bottom connector */ >> - .i2c_presence_odl1 = (1 << 7), /* top connector */ >> + .i2c_presence_pin2 = (1 << 2), /* bottom connector */ >> + .i2c_presence_pin3 = (1 << 7), /* top connector */ >> + .i2c_presence_pin4 = 0, /* unused */ >> + .i2c_presence_pin5 = 0, /* unused */ >> /* >> * i2c presence detection is broken on ZZ planar < v4 so we >> * force the presence until all our systems are upgraded >> -- >> git-series 0.9.1 >> >> _______________________________________________ >> Skiboot mailing list >> Skiboot@lists.ozlabs.org >> https://lists.ozlabs.org/listinfo/skiboot >
diff --git a/core/platform.c b/core/platform.c index b32cbf5ce9d1..85b2cd8f72ee 100644 --- a/core/platform.c +++ b/core/platform.c @@ -174,11 +174,15 @@ const struct platform_ocapi generic_ocapi = { .i2c_engine = 1, .i2c_port = 4, .i2c_reset_addr = 0x20, - .i2c_reset_odl0 = (1 << 1), - .i2c_reset_odl1 = (1 << 6), + .i2c_reset_pin2 = (1 << 1), + .i2c_reset_pin3 = (1 << 6), + .i2c_reset_pin4 = 0, /* unused */ + .i2c_reset_pin5 = 0, /* unused */ .i2c_presence_addr = 0x20, - .i2c_presence_odl0 = (1 << 2), /* bottom connector */ - .i2c_presence_odl1 = (1 << 7), /* top connector */ + .i2c_presence_pin2 = (1 << 2), /* bottom connector */ + .i2c_presence_pin3 = (1 << 7), /* top connector */ + .i2c_presence_pin4 = 0, /* unused */ + .i2c_presence_pin5 = 0, /* unused */ /* * The ZZs we typically use for BML/generic platform tend to * have old planars and presence detection is broken there, so diff --git a/hw/npu2-common.c b/hw/npu2-common.c index cceb88706981..8151df3d9ba5 100644 --- a/hw/npu2-common.c +++ b/hw/npu2-common.c @@ -115,11 +115,16 @@ static bool _i2c_presence_detect(struct npu2_dev *dev) switch (dev->link_index) { case 2: - data = platform.ocapi->i2c_presence_odl0; + data = platform.ocapi->i2c_presence_pin2; break; case 3: - data = platform.ocapi->i2c_presence_odl1; + data = platform.ocapi->i2c_presence_pin3; break; + case 4: + data = platform.ocapi->i2c_presence_pin4; + break; + case 5: + data = platform.ocapi->i2c_presence_pin5; default: OCAPIERR(dev, "presence detection on invalid link\n"); return true; diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c index fa28633bb150..ce5f99114463 100644 --- a/hw/npu2-opencapi.c +++ b/hw/npu2-opencapi.c @@ -816,12 +816,16 @@ static void assert_reset(struct npu2_dev *dev) switch (dev->brick_index) { case 2: - case 4: - pin = platform.ocapi->i2c_reset_odl0; + pin = platform.ocapi->i2c_reset_pin2; break; case 3: + pin = platform.ocapi->i2c_reset_pin3; + break; + case 4: + pin = platform.ocapi->i2c_reset_pin4; + break; case 5: - pin = platform.ocapi->i2c_reset_odl1; + pin = platform.ocapi->i2c_reset_pin5; break; default: assert(false); diff --git a/include/platform.h b/include/platform.h index efafbd2239b9..76d001547d05 100644 --- a/include/platform.h +++ b/include/platform.h @@ -50,11 +50,15 @@ struct platform_ocapi { uint8_t i2c_engine; /* I2C engine number */ uint8_t i2c_port; /* I2C port number */ uint8_t i2c_reset_addr; /* I2C address for reset */ - uint8_t i2c_reset_odl0; /* I2C pin to write to reset ODL0 */ - uint8_t i2c_reset_odl1; /* I2C pin to write to reset ODL1 */ + uint8_t i2c_reset_pin2; /* I2C pin to write to reset brick 2 */ + uint8_t i2c_reset_pin3; /* I2C pin to write to reset brick 3 */ + uint8_t i2c_reset_pin4; /* I2C pin to write to reset brick 4 */ + uint8_t i2c_reset_pin5; /* I2C pin to write to reset brick 5 */ uint8_t i2c_presence_addr; /* I2C address for presence detection */ - uint8_t i2c_presence_odl0; /* I2C mask for detection on ODL0 */ - uint8_t i2c_presence_odl1; /* I2C mask for detection on ODL1 */ + uint8_t i2c_presence_pin2; /* I2C mask for detection on brick 2 */ + uint8_t i2c_presence_pin3; /* I2C mask for detection on brick 3 */ + uint8_t i2c_presence_pin4; /* I2C mask for detection on brick 4 */ + uint8_t i2c_presence_pin5; /* I2C mask for detection on brick 5 */ bool force_presence; /* don't use i2c detection */ bool odl_phy_swap; /* Swap ODL1 to use brick 2 rather than * brick 1 lanes */ diff --git a/platforms/astbmc/zaius.c b/platforms/astbmc/zaius.c index 069ce5751550..95d59e8feee9 100644 --- a/platforms/astbmc/zaius.c +++ b/platforms/astbmc/zaius.c @@ -28,11 +28,15 @@ const struct platform_ocapi zaius_ocapi = { .i2c_engine = 1, .i2c_port = 4, .i2c_reset_addr = 0x20, - .i2c_reset_odl0 = (1 << 1), - .i2c_reset_odl1 = (1 << 6), + .i2c_reset_pin2 = (1 << 1), + .i2c_reset_pin3 = (1 << 6), + .i2c_reset_pin4 = 0, /* unused */ + .i2c_reset_pin5 = 0, /* unused */ .i2c_presence_addr = 0x20, - .i2c_presence_odl0 = (1 << 2), /* bottom connector */ - .i2c_presence_odl1 = (1 << 7), /* top connector */ + .i2c_presence_pin2 = (1 << 2), /* bottom connector */ + .i2c_presence_pin3 = (1 << 7), /* top connector */ + .i2c_presence_pin4 = 0, /* unused */ + .i2c_presence_pin5 = 0, /* unused */ .odl_phy_swap = true, }; diff --git a/platforms/ibm-fsp/zz.c b/platforms/ibm-fsp/zz.c index dc48768b8a96..800ad024314f 100644 --- a/platforms/ibm-fsp/zz.c +++ b/platforms/ibm-fsp/zz.c @@ -32,11 +32,15 @@ const struct platform_ocapi zz_ocapi = { .i2c_engine = 1, .i2c_port = 4, .i2c_reset_addr = 0x20, - .i2c_reset_odl0 = (1 << 1), - .i2c_reset_odl1 = (1 << 6), + .i2c_reset_pin2 = (1 << 1), + .i2c_reset_pin3 = (1 << 6), + .i2c_reset_pin4 = 0, /* unused */ + .i2c_reset_pin5 = 0, /* unused */ .i2c_presence_addr = 0x20, - .i2c_presence_odl0 = (1 << 2), /* bottom connector */ - .i2c_presence_odl1 = (1 << 7), /* top connector */ + .i2c_presence_pin2 = (1 << 2), /* bottom connector */ + .i2c_presence_pin3 = (1 << 7), /* top connector */ + .i2c_presence_pin4 = 0, /* unused */ + .i2c_presence_pin5 = 0, /* unused */ /* * i2c presence detection is broken on ZZ planar < v4 so we * force the presence until all our systems are upgraded
In platform_ocapi, we define i2c_{reset,presence}_odl{0,1} to specify the appropriate reset/presence GPIO pins for devices connected to ODL0 and ODL1 respectively. This is obviously wrong, because a device connected to brick 2 and a device connected to brick 4 are going to be different devices connected to different I2C pins, but rather conveniently we haven't had to deal with systems that can use the full 4 bricks as yet. Now that we're adding OpenCAPI support for Witherspoon, we should change this to specify pins separately for all 4 bricks. Replace i2c_{reset,presence}_odl{0,1} with i2c_{reset,presence}_pin{2,3,4,5} and update the presence detection code, device reset code, and existing platforms accordingly. Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> --- core/platform.c | 12 ++++++++---- hw/npu2-common.c | 9 +++++++-- hw/npu2-opencapi.c | 10 +++++++--- include/platform.h | 12 ++++++++---- platforms/astbmc/zaius.c | 12 ++++++++---- platforms/ibm-fsp/zz.c | 12 ++++++++---- 6 files changed, 46 insertions(+), 21 deletions(-)