Chuck Tuffli wrote: > This fixes the issue I was seeing. Thanks! > > -----Original Message----- > From: Jack Wang [mailto:jack_wang@xxxxxxxxx] > Sent: Sunday, September 19, 2010 10:52 PM > To: 'Jack Wang'; Chuck Tuffli; 'James Bottomley'; linux-scsi@xxxxxxxxxxxxxxx > Cc: 'lindar_liu'; 'roy' > Subject: [PATCH v2] libsas: fix bug for vacant phy > > > Hi, James > > Please drop the previous patch and apply this new one. > Attached patch fix following bugs reported by Chuck. > > Chuck > Could you test if this solve your problem ? > > Jack - > Signed-off-by: Jack Wang <jack_wang@xxxxxxxxx> > > Signed-off-by: Lindar <lindar_liu@xxxxxxxxx> > --- > sas_expander.c | 7 ++++--- > 1 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/sas_expander.c b/sas_expander.c > index d1d86a6..f63dbea 100644 > --- a/sas_expander.c > +++ b/sas_expander.c > @@ -175,10 +175,10 @@ static void sas_set_ex_phy(struct domain_device *dev, > int phy_id, > switch (resp->result) { > case SMP_RESP_PHY_VACANT: > phy->phy_state = PHY_VACANT; > - return; > + break; > default: > phy->phy_state = PHY_NOT_PRESENT; > - return; > + break; > case SMP_RESP_FUNC_ACC: > phy->phy_state = PHY_EMPTY; /* do not know yet */ > break; > @@ -209,7 +209,8 @@ static void sas_set_ex_phy(struct domain_device *dev, > int phy_id, > phy->phy->negotiated_linkrate = phy->linkrate; > > if (!rediscover) > - sas_phy_add(phy->phy); > + if (sas_phy_add(phy->phy)) > + sas_phy_free(phy->phy); > > SAS_DPRINTK("ex %016llx phy%02d:%c attached: %016llx\n", > SAS_ADDR(dev->sas_addr), phy->phy_id, Hmm. Shouldn't you avoid the message here if you entered the error path? I guess the SAS phy isn't actually attached then. So maybe the above statement should read: if (!rediscover) if (sas_phy_add(phy->phy)) { sas_phy_free(phy->phy); return; } Cheers, Hannes [Jack] Yes, you're right. I'll update the patch soon. Thanks for review. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
