[PATCH v4 17/32] cxlflash: Remove dual port online dependency

[Matthew R. Ochs]

> On Sep 28, 2015, at 6:37 PM, Daniel Axtens <dja at axtens.net> 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.