> On Sep 30, 2015, at 6:50 PM, Daniel Axtens <dja@xxxxxxxxxx> wrote: > (resending to the list this time, apologies!) > >>> 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. > > OK. That makes more sense - it wasn't clear to me how it could be > correct to proceed if the links were down but now I understand how that > works. I think that explanation should go in the commit message. > >>>> 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); > > As an aside, should this be a bit noisier? It seems like something > a user would probably want to know - especially in the case where > something has actually gone wrong so there's no link state change > interrupt forthcoming regardless of how long you wait. You bring up a good point. There is another place where we are noisier with respect to the link being down, so we'll do the same here. I'll include this in v5 along with an updated commit message as requested. -- 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