Date: Thu, 9 May 2013 11:03:33 -0700
From: Andrew Morton <akpm@linux-foundation.org>
To: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
Cc: zbr@ioremap.net, linux-kernel@vger.kernel.org, Greg KH <greg@kroah.com>
Subject: Re: [PATCH V2] w1: ds2408: add magic sequence to disable P0 test
 mode
Message-Id: <20130509110333.f6ba3b4cc22e87c7adb30993@linux-foundation.org>
In-Reply-To: <1367935248-12118-1-git-send-email-jeff.dagenais@gmail.com>
References: <1367930422-25972-2-git-send-email-jeff.dagenais@gmail.com>
	<1367935248-12118-1-git-send-email-jeff.dagenais@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2914
Lines: 87

On Tue,  7 May 2013 10:00:48 -0400 Jean-Francois Dagenais <jeff.dagenais@gmail.com> wrote:

> V2: use the new bus_mutex
> 
> extract from http://datasheets.maximintegrated.com/en/ds/DS2408.pdf:
> 
> Power-up timing
> The DS2408 is sensitive to the power-on slew rate and can inadvertently
> power up with a test mode feature enabled. When this occurs, the P0 port
> does not respond to the Channel Access Write command.
> For most reliable operation, it is recommended to disable the test mode
> after every power-on reset using the Disable Test Mode sequence shown
> below. The 64-bit ROM code must be transmitted in the same bit sequence
> as with the Match ROM command, i.e., least significant bit first. This
> precaution is recommended in parasite power mode (VCC pin connected to
> GND) as well as with VCC power.
> 
> Disable Test Mode:
> RST,PD,96h,<64-bit DS2408 ROM Code>,3Ch,RST,PD
> 
> ...
>
> --- a/drivers/w1/slaves/w1_ds2408.c
> +++ b/drivers/w1/slaves/w1_ds2408.c
> @@ -301,7 +301,32 @@ error:
>  	return -EIO;
>  }
>  
> +/**

/** is used to introduce kerneldoc comments, but this isn't a kerneldoc
comment.  Just use /* here.

> + * This is a special sequence we must do to ensure the P0 output is not stuck
> + * in test mode. This is described in rev 2 of the ds2408's datasheet
> + * (http://datasheets.maximintegrated.com/en/ds/DS2408.pdf) under
> + * "APPLICATION INFORMATION/Power-up timing".
> + */
> +static int w1_f29_disable_test_mode(struct w1_slave *sl)
> +{
> +	int res;
> +	u8 magic[10] = {0x96, };
> +	u64 rn = le64_to_cpu(*((u64*)&sl->reg_num));
> +	memcpy(&magic[1], &rn, 8);
> +	magic[9] = 0x3C;

(please put a blank line between end-of-locals and start-of-code)

The casting looks decidedly dodgy.  I guess it won't cause an unalignned
exception due to reg_num's alignment, but it appears to defeat the
endianness handling in the definotion of `struct w1_reg_num'.

Are you sure this will work OK with all architectures and both
__LITTLE_ENDIAN_BITFIELD and __BIG_ENDIAN_BITFIELD?

> +	mutex_lock(&sl->master->bus_mutex);
>  
> +	res = w1_reset_bus(sl->master);
> +	if (res)
> +		goto out;
> +	w1_write_block(sl->master, magic, ARRAY_SIZE(magic));
> +
> +	res = w1_reset_bus(sl->master);
> +out:
> +	mutex_unlock(&sl->master->bus_mutex);
> +	return res;
> +}
>  
>  static struct bin_attribute w1_f29_sysfs_bin_files[] = {
>  	{
> @@ -362,6 +387,10 @@ static int w1_f29_add_slave(struct w1_slave *sl)
>  	int err = 0;
>  	int i;
>  
> +	err = w1_f29_disable_test_mode(sl);
> +	if (err)
> +		return err;
> +
>  	for (i = 0; i < ARRAY_SIZE(w1_f29_sysfs_bin_files) && !err; ++i)
>  		err = sysfs_create_bin_file(
>  			&sl->dev.kobj,

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/