mirror of
				https://github.com/Ysurac/openmptcprouter.git
				synced 2025-03-09 15:40:20 +00:00 
			
		
		
		
	
		
			
				
	
	
		
			145 lines
		
	
	
	
		
			4.4 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
			
		
		
	
	
			145 lines
		
	
	
	
		
			4.4 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
| From 646f633466ac539a329dd3694c3ec74d55297855 Mon Sep 17 00:00:00 2001
 | |
| From: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
 | |
| Date: Thu, 28 Mar 2019 12:41:11 -0400
 | |
| Subject: [PATCH 500/515] w1: ds2408: reset on output_write retry with readback
 | |
| 
 | |
| commit 49695ac46861180baf2b2b92c62da8619b6bf28f upstream.
 | |
| 
 | |
| When we have success in 'Channel Access Write' but reading back latch
 | |
| states fails, a write is retried without doing a proper slave reset.
 | |
| This leads to protocol errors as the slave treats the next 'Channel
 | |
| Access Write' as the continuation of previous command.
 | |
| 
 | |
| This commit is fixing this by making sure if the retry loop re-runs, a
 | |
| reset is performed, whatever the failure (CONFIRM_BYTE or the read
 | |
| back).
 | |
| 
 | |
| The loop was quite due for a cleanup and this change mandated it. By
 | |
| isolating the CONFIG_W1_SLAVE_DS2408_READBACK case into it's own
 | |
| function, we vastly reduce the visual and branching(runtime and
 | |
| compile-time) noise.
 | |
| 
 | |
| Reported-by: Mariusz Bialonczyk <manio@skyboo.net>
 | |
| Tested-by: Mariusz Bialonczyk <manio@skyboo.net>
 | |
| Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
 | |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
 | |
| ---
 | |
|  drivers/w1/slaves/w1_ds2408.c | 76 ++++++++++++++++++-----------------
 | |
|  1 file changed, 39 insertions(+), 37 deletions(-)
 | |
| 
 | |
| diff --git a/drivers/w1/slaves/w1_ds2408.c b/drivers/w1/slaves/w1_ds2408.c
 | |
| index b535d5ec35b6..92e8f0755b9a 100644
 | |
| --- a/drivers/w1/slaves/w1_ds2408.c
 | |
| +++ b/drivers/w1/slaves/w1_ds2408.c
 | |
| @@ -138,14 +138,37 @@ static ssize_t status_control_read(struct file *filp, struct kobject *kobj,
 | |
|  		W1_F29_REG_CONTROL_AND_STATUS, buf);
 | |
|  }
 | |
|  
 | |
| +#ifdef fCONFIG_W1_SLAVE_DS2408_READBACK
 | |
| +static bool optional_read_back_valid(struct w1_slave *sl, u8 expected)
 | |
| +{
 | |
| +	u8 w1_buf[3];
 | |
| +
 | |
| +	if (w1_reset_resume_command(sl->master))
 | |
| +		return false;
 | |
| +
 | |
| +	w1_buf[0] = W1_F29_FUNC_READ_PIO_REGS;
 | |
| +	w1_buf[1] = W1_F29_REG_OUTPUT_LATCH_STATE;
 | |
| +	w1_buf[2] = 0;
 | |
| +
 | |
| +	w1_write_block(sl->master, w1_buf, 3);
 | |
| +
 | |
| +	return (w1_read_8(sl->master) == expected);
 | |
| +}
 | |
| +#else
 | |
| +static bool optional_read_back_valid(struct w1_slave *sl, u8 expected)
 | |
| +{
 | |
| +	return true;
 | |
| +}
 | |
| +#endif
 | |
| +
 | |
|  static ssize_t output_write(struct file *filp, struct kobject *kobj,
 | |
|  			    struct bin_attribute *bin_attr, char *buf,
 | |
|  			    loff_t off, size_t count)
 | |
|  {
 | |
|  	struct w1_slave *sl = kobj_to_w1_slave(kobj);
 | |
|  	u8 w1_buf[3];
 | |
| -	u8 readBack;
 | |
|  	unsigned int retries = W1_F29_RETRIES;
 | |
| +	ssize_t bytes_written = -EIO;
 | |
|  
 | |
|  	if (count != 1 || off != 0)
 | |
|  		return -EFAULT;
 | |
| @@ -155,54 +178,33 @@ static ssize_t output_write(struct file *filp, struct kobject *kobj,
 | |
|  	dev_dbg(&sl->dev, "mutex locked");
 | |
|  
 | |
|  	if (w1_reset_select_slave(sl))
 | |
| -		goto error;
 | |
| +		goto out;
 | |
|  
 | |
| -	while (retries--) {
 | |
| +	do {
 | |
|  		w1_buf[0] = W1_F29_FUNC_CHANN_ACCESS_WRITE;
 | |
|  		w1_buf[1] = *buf;
 | |
|  		w1_buf[2] = ~(*buf);
 | |
| -		w1_write_block(sl->master, w1_buf, 3);
 | |
|  
 | |
| -		readBack = w1_read_8(sl->master);
 | |
| +		w1_write_block(sl->master, w1_buf, 3);
 | |
|  
 | |
| -		if (readBack != W1_F29_SUCCESS_CONFIRM_BYTE) {
 | |
| -			if (w1_reset_resume_command(sl->master))
 | |
| -				goto error;
 | |
| -			/* try again, the slave is ready for a command */
 | |
| -			continue;
 | |
| +		if (w1_read_8(sl->master) == W1_F29_SUCCESS_CONFIRM_BYTE &&
 | |
| +		    optional_read_back_valid(sl, *buf)) {
 | |
| +			bytes_written = 1;
 | |
| +			goto out;
 | |
|  		}
 | |
|  
 | |
| -#ifdef CONFIG_W1_SLAVE_DS2408_READBACK
 | |
| -		/* here the master could read another byte which
 | |
| -		   would be the PIO reg (the actual pin logic state)
 | |
| -		   since in this driver we don't know which pins are
 | |
| -		   in and outs, there's no value to read the state and
 | |
| -		   compare. with (*buf) so end this command abruptly: */
 | |
|  		if (w1_reset_resume_command(sl->master))
 | |
| -			goto error;
 | |
| +			goto out; /* unrecoverable error */
 | |
| +		/* try again, the slave is ready for a command */
 | |
| +	} while (--retries);
 | |
|  
 | |
| -		/* go read back the output latches */
 | |
| -		/* (the direct effect of the write above) */
 | |
| -		w1_buf[0] = W1_F29_FUNC_READ_PIO_REGS;
 | |
| -		w1_buf[1] = W1_F29_REG_OUTPUT_LATCH_STATE;
 | |
| -		w1_buf[2] = 0;
 | |
| -		w1_write_block(sl->master, w1_buf, 3);
 | |
| -		/* read the result of the READ_PIO_REGS command */
 | |
| -		if (w1_read_8(sl->master) == *buf)
 | |
| -#endif
 | |
| -		{
 | |
| -			/* success! */
 | |
| -			mutex_unlock(&sl->master->bus_mutex);
 | |
| -			dev_dbg(&sl->dev,
 | |
| -				"mutex unlocked, retries:%d", retries);
 | |
| -			return 1;
 | |
| -		}
 | |
| -	}
 | |
| -error:
 | |
| +out:
 | |
|  	mutex_unlock(&sl->master->bus_mutex);
 | |
| -	dev_dbg(&sl->dev, "mutex unlocked in error, retries:%d", retries);
 | |
|  
 | |
| -	return -EIO;
 | |
| +	dev_dbg(&sl->dev, "%s, mutex unlocked retries:%d\n",
 | |
| +		(bytes_written > 0) ? "succeeded" : "error", retries);
 | |
| +
 | |
| +	return bytes_written;
 | |
|  }
 | |
|  
 | |
|  
 | |
| -- 
 | |
| 2.19.1
 | |
| 
 |