> On Sep 28, 2015, at 6:37 PM, Daniel Axtens <dja@xxxxxxxxxx> wrote: > > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA512 > > Hi, > >> static int afu_set_wwpn(struct afu *afu, int port, u64 *fc_regs, u64 wwpn) >> { >> - int ret = 0; >> + int rc = 0; > I realise it's nice to have things consistent, but making this change > now makes the rest of the patch quite difficult to follow. Next time I will try to separate out a change like this. > >> >> set_port_offline(fc_regs); >> >> @@ -1038,33 +1038,26 @@ static int afu_set_wwpn(struct afu *afu, int port, u64 *fc_regs, u64 wwpn) >> FC_PORT_STATUS_RETRY_CNT)) { >> pr_debug("%s: wait on port %d to go offline timed out\n", >> __func__, port); >> - ret = -1; /* but continue on to leave the port back online */ >> + rc = -1; /* but continue on to leave the port back online */ >> } >> >> - if (ret == 0) >> + if (rc == 0) >> writeq_be(wwpn, &fc_regs[FC_PNAME / 8]); >> >> + /* Always return success after programming WWPN */ >> + rc = 0; >> + >> set_port_online(fc_regs); >> >> if (!wait_port_online(fc_regs, FC_PORT_STATUS_RETRY_INTERVAL_US, >> FC_PORT_STATUS_RETRY_CNT)) { >> pr_debug("%s: wait on port %d to go online timed out\n", >> __func__, port); >> - ret = -1; >> - >> - /* >> - * Override for internal lun!!! >> - */ >> - if (afu->internal_lun) { >> - pr_debug("%s: Overriding port %d online timeout!!!\n", >> - __func__, port); >> - ret = 0; >> - } >> } >> >> - pr_debug("%s: returning rc=%d\n", __func__, ret); >> + pr_debug("%s: returning rc=%d\n", __func__, rc); > I'm not sure I fully understand the flow of this function, but it looks > like you set rc=0 regardless of how things actually go: is this ever > going to print a return value other than zero? Correct, this function behaves more like a void for the time being. The overall goal of this is to allow a card to configure even when the link is down. At some later point when the link is transitioned to 'up', a link state change interrupt will trigger the port configuration. I left this with a return code for right now in case we need to alter the behavior again (based upon testing) and actually return a value other than 0. -- 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