| Message ID | 20150120043052.25425.17775.stgit@localhost.localdomain |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Jan 20, 2015 at 10:01:28AM +0530, Neelesh Gupta wrote: > There could be some processor chips not populated in the system. > So, if we are not able to get chip data of given chip id, we should > just move on to the next or return instead of aborting (bz 120562). > > Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com> > --- > platforms/ibm-fsp/firenze.c | 42 +++++++++++++++++++++++++----------------- > 1 file changed, 25 insertions(+), 17 deletions(-) > > diff --git a/platforms/ibm-fsp/firenze.c b/platforms/ibm-fsp/firenze.c > index d971d57..258a6b3 100644 > --- a/platforms/ibm-fsp/firenze.c > +++ b/platforms/ibm-fsp/firenze.c > @@ -130,29 +130,37 @@ static void firenze_dt_fixup_i2cm(void) > case LX_VPD_SHARK_BACKPLANE: /* XXX confirm ? */ > /* i2c nodes on chip 0x10 */ > c = get_chip(0x10); > - assert(c); > - > - /* Engine 1 */ > - master = dt_create_i2c_master(c->devnode, 1); > - assert(master); > - snprintf(name, sizeof(name), "p8_%08x_e%dp%d", c->id, 1, 0); > - bus = dt_create_i2c_bus(master, name, 0); > - assert(bus); > - dev = dt_create_i2c_device(bus, 0x39, "power-control", > - "maxim,5961", "pcie-hotplug"); > - assert(dev); > - dt_add_property_strings(dev, "target-list", "slot-C4", "slot-C5"); > + if (c) { > + /* Engine 1 */ > + master = dt_create_i2c_master(c->devnode, 1); > + assert(master); > + snprintf(name, sizeof(name), "p8_%08x_e%dp%d", c->id, 1, 0); > + bus = dt_create_i2c_bus(master, name, 0); > + assert(bus); > + dev = dt_create_i2c_device(bus, 0x39, "power-control", > + "maxim,5961", "pcie-hotplug"); > + assert(dev); > + dt_add_property_strings(dev, "target-list", "slot-C4", > + "slot-C5"); > + > + dev = dt_create_i2c_device(bus, 0x3a, "power-control", > + "maxim,5961", "pcie-hotplug"); > + assert(dev); > + dt_add_property_strings(dev, "target-list", "slot-C2", > + "slot-C3"); Should we fall through even if we find a chip? > + } else { > + prlog(PR_INFO, "PLAT: Chip not found for the id 0x10\n"); > + } > > - dev = dt_create_i2c_device(bus, 0x3a, "power-control", > - "maxim,5961", "pcie-hotplug"); > - assert(dev); > - dt_add_property_strings(dev, "target-list", "slot-C2", "slot-C3"); > /* Fall through */ > case LX_VPD_1S4U_BACKPLANE: > case LX_VPD_1S2U_BACKPLANE: > /* i2c nodes on chip 0 */ > c = get_chip(0); > - assert(c); > + if (!c) { > + prlog(PR_INFO, "PLAT: Chip not found for the id 0x0\n"); Should we continue if we don't find a chip on either of the chips? Ananth > + break; > + } > > /* Engine 1*/ > master = dt_create_i2c_master(c->devnode, 1); > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
On 01/20/2015 12:27 PM, Ananth N Mavinakayanahalli wrote: > On Tue, Jan 20, 2015 at 10:01:28AM +0530, Neelesh Gupta wrote: >> There could be some processor chips not populated in the system. >> So, if we are not able to get chip data of given chip id, we should >> just move on to the next or return instead of aborting (bz 120562). >> >> Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com> >> --- >> platforms/ibm-fsp/firenze.c | 42 +++++++++++++++++++++++++----------------- >> 1 file changed, 25 insertions(+), 17 deletions(-) >> >> diff --git a/platforms/ibm-fsp/firenze.c b/platforms/ibm-fsp/firenze.c >> index d971d57..258a6b3 100644 >> --- a/platforms/ibm-fsp/firenze.c >> +++ b/platforms/ibm-fsp/firenze.c >> @@ -130,29 +130,37 @@ static void firenze_dt_fixup_i2cm(void) >> case LX_VPD_SHARK_BACKPLANE: /* XXX confirm ? */ >> /* i2c nodes on chip 0x10 */ >> c = get_chip(0x10); >> - assert(c); >> - >> - /* Engine 1 */ >> - master = dt_create_i2c_master(c->devnode, 1); >> - assert(master); >> - snprintf(name, sizeof(name), "p8_%08x_e%dp%d", c->id, 1, 0); >> - bus = dt_create_i2c_bus(master, name, 0); >> - assert(bus); >> - dev = dt_create_i2c_device(bus, 0x39, "power-control", >> - "maxim,5961", "pcie-hotplug"); >> - assert(dev); >> - dt_add_property_strings(dev, "target-list", "slot-C4", "slot-C5"); >> + if (c) { >> + /* Engine 1 */ >> + master = dt_create_i2c_master(c->devnode, 1); >> + assert(master); >> + snprintf(name, sizeof(name), "p8_%08x_e%dp%d", c->id, 1, 0); >> + bus = dt_create_i2c_bus(master, name, 0); >> + assert(bus); >> + dev = dt_create_i2c_device(bus, 0x39, "power-control", >> + "maxim,5961", "pcie-hotplug"); >> + assert(dev); >> + dt_add_property_strings(dev, "target-list", "slot-C4", >> + "slot-C5"); >> + >> + dev = dt_create_i2c_device(bus, 0x3a, "power-control", >> + "maxim,5961", "pcie-hotplug"); >> + assert(dev); >> + dt_add_property_strings(dev, "target-list", "slot-C2", >> + "slot-C3"); > Should we fall through even if we find a chip? Yes, it is happening. In either case, found/not-found (if/else) we fall through. > >> + } else { >> + prlog(PR_INFO, "PLAT: Chip not found for the id 0x10\n"); >> + } >> >> - dev = dt_create_i2c_device(bus, 0x3a, "power-control", >> - "maxim,5961", "pcie-hotplug"); >> - assert(dev); >> - dt_add_property_strings(dev, "target-list", "slot-C2", "slot-C3"); >> /* Fall through */ >> case LX_VPD_1S4U_BACKPLANE: >> case LX_VPD_1S2U_BACKPLANE: >> /* i2c nodes on chip 0 */ >> c = get_chip(0); >> - assert(c); >> + if (!c) { >> + prlog(PR_INFO, "PLAT: Chip not found for the id 0x0\n"); > Should we continue if we don't find a chip on either of the chips? No, the function is about creating i2c nodes under a chip node (c->devnode), if found. - Neelesh > > Ananth > >> + break; >> + } >> >> /* Engine 1*/ >> master = dt_create_i2c_master(c->devnode, 1); >> >> _______________________________________________ >> Skiboot mailing list >> Skiboot@lists.ozlabs.org >> https://lists.ozlabs.org/listinfo/skiboot
On Tue, Jan 20, 2015 at 12:27:37PM +0530, Ananth N Mavinakayanahalli wrote: > On Tue, Jan 20, 2015 at 10:01:28AM +0530, Neelesh Gupta wrote: ... > Should we fall through even if we find a chip? > > > + } else { > > + prlog(PR_INFO, "PLAT: Chip not found for the id 0x10\n"); > > + } > > > > - dev = dt_create_i2c_device(bus, 0x3a, "power-control", > > - "maxim,5961", "pcie-hotplug"); > > - assert(dev); > > - dt_add_property_strings(dev, "target-list", "slot-C2", "slot-C3"); > > /* Fall through */ > > case LX_VPD_1S4U_BACKPLANE: > > case LX_VPD_1S2U_BACKPLANE: > > /* i2c nodes on chip 0 */ > > c = get_chip(0); > > - assert(c); > > + if (!c) { > > + prlog(PR_INFO, "PLAT: Chip not found for the id 0x0\n"); > > Should we continue if we don't find a chip on either of the chips? I wasn't sure of the I2C master topology and it being on the chip.. ignore my comments... Ananth
diff --git a/platforms/ibm-fsp/firenze.c b/platforms/ibm-fsp/firenze.c index d971d57..258a6b3 100644 --- a/platforms/ibm-fsp/firenze.c +++ b/platforms/ibm-fsp/firenze.c @@ -130,29 +130,37 @@ static void firenze_dt_fixup_i2cm(void) case LX_VPD_SHARK_BACKPLANE: /* XXX confirm ? */ /* i2c nodes on chip 0x10 */ c = get_chip(0x10); - assert(c); - - /* Engine 1 */ - master = dt_create_i2c_master(c->devnode, 1); - assert(master); - snprintf(name, sizeof(name), "p8_%08x_e%dp%d", c->id, 1, 0); - bus = dt_create_i2c_bus(master, name, 0); - assert(bus); - dev = dt_create_i2c_device(bus, 0x39, "power-control", - "maxim,5961", "pcie-hotplug"); - assert(dev); - dt_add_property_strings(dev, "target-list", "slot-C4", "slot-C5"); + if (c) { + /* Engine 1 */ + master = dt_create_i2c_master(c->devnode, 1); + assert(master); + snprintf(name, sizeof(name), "p8_%08x_e%dp%d", c->id, 1, 0); + bus = dt_create_i2c_bus(master, name, 0); + assert(bus); + dev = dt_create_i2c_device(bus, 0x39, "power-control", + "maxim,5961", "pcie-hotplug"); + assert(dev); + dt_add_property_strings(dev, "target-list", "slot-C4", + "slot-C5"); + + dev = dt_create_i2c_device(bus, 0x3a, "power-control", + "maxim,5961", "pcie-hotplug"); + assert(dev); + dt_add_property_strings(dev, "target-list", "slot-C2", + "slot-C3"); + } else { + prlog(PR_INFO, "PLAT: Chip not found for the id 0x10\n"); + } - dev = dt_create_i2c_device(bus, 0x3a, "power-control", - "maxim,5961", "pcie-hotplug"); - assert(dev); - dt_add_property_strings(dev, "target-list", "slot-C2", "slot-C3"); /* Fall through */ case LX_VPD_1S4U_BACKPLANE: case LX_VPD_1S2U_BACKPLANE: /* i2c nodes on chip 0 */ c = get_chip(0); - assert(c); + if (!c) { + prlog(PR_INFO, "PLAT: Chip not found for the id 0x0\n"); + break; + } /* Engine 1*/ master = dt_create_i2c_master(c->devnode, 1);
There could be some processor chips not populated in the system. So, if we are not able to get chip data of given chip id, we should just move on to the next or return instead of aborting (bz 120562). Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com> --- platforms/ibm-fsp/firenze.c | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-)