W1: ds2490.c optimize ds_set_pullup

Optimize the ds_set_pullup function. For a strong pullup to be sent the ds2490 has to have both the strong pullup mode enabled, and the specific write operation has to have the SPU bit enabled. Previously the write always had the SPU bit enabled and both the duration and model was set when a strong pullup was requested. Now the strong pullup mode is enabled at initialization time, the delay is updated only when the value changes, and the write SPU bit is set only when a strong pullup is required. This removes two or three bus transactions per strong pullup request. Signed-off-by: David Fries <david@fries.net> Signed-off-by: Evgeniy Polyakov <johnpol@2ka.mipt.ru> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

W1: ds2490.c optimize ds_set_pullup
Optimize the ds_set_pullup function. For a strong pullup to be sent the ds2490 has to have both the strong pullup mode enabled, and the specific write operation has to have the SPU bit enabled. Previously the write always had the SPU bit enabled and both the duration and model was set when a strong pullup was requested. Now the strong pullup mode is enabled at initialization time, the delay is updated only when the value changes, and the write SPU bit is set only when a strong pullup is required. This removes two or three bus transactions per strong pullup request. Signed-off-by: David Fries <david@fries.net> Signed-off-by: Evgeniy Polyakov <johnpol@2ka.mipt.ru> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
David Fries · Linus Torvalds
1 parent 3823ee44cf
Showing 1 changed file with 45 additions and 19 deletions Side-by-side Diff
drivers/w1/masters/ds2490.c
@@ -141,6 +141,10 @@
 	 * 0: pullup not active, else duration in milliseconds
 	 */
 	int			spu_sleep;
+	/* spu_bit contains COMM_SPU or 0 depending on if the strong pullup
+	 * should be active or not for writes.
+	 */
+	u16			spu_bit;
  
 	struct w1_bus_master	master;
 };
@@ -311,6 +315,25 @@
 	}
 }
  
+static void ds_reset_device(struct ds_device *dev)
+{
+	ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0);
+	/* Always allow strong pullup which allow individual writes to use
+	 * the strong pullup.
+	 */
+	if (ds_send_control_mode(dev, MOD_PULSE_EN, PULSE_SPUE))
+		printk(KERN_ERR "ds_reset_device: "
+			"Error allowing strong pullup\n");
+	/* Chip strong pullup time was cleared. */
+	if (dev->spu_sleep) {
+		/* lower 4 bits are 0, see ds_set_pullup */
+		u8 del = dev->spu_sleep>>4;
+		if (ds_send_control(dev, COMM_SET_DURATION | COMM_IM, del))
+			printk(KERN_ERR "ds_reset_device: "
+				"Error setting duration\n");
+	}
+}
+
 static int ds_recv_data(struct ds_device *dev, unsigned char *buf, int size)
 {
 	int count, err;
@@ -444,7 +467,7 @@
  
 	if (err >= 16 && st->status & ST_EPOF) {
 		printk(KERN_INFO "Resetting device after ST_EPOF.\n");
-		ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0);
+		ds_reset_device(dev);
 		/* Always dump the device status. */
 		count = 101;
 	}
  
  
  
  
@@ -509,25 +532,27 @@
  
 static int ds_set_pullup(struct ds_device *dev, int delay)
 {
-	int err;
+	int err = 0;
 	u8 del = 1 + (u8)(delay >> 4);
+	/* Just storing delay would not get the trunication and roundup. */
+	int ms = del<<4;
  
-	dev->spu_sleep = 0;
-	err = ds_send_control_mode(dev, MOD_PULSE_EN, delay ? PULSE_SPUE : 0);
+	/* Enable spu_bit if a delay is set. */
+	dev->spu_bit = delay ? COMM_SPU : 0;
+	/* If delay is zero, it has already been disabled, if the time is
+	 * the same as the hardware was last programmed to, there is also
+	 * nothing more to do.  Compare with the recalculated value ms
+	 * rather than del or delay which can have a different value.
+	 */
+	if (delay == 0 || ms == dev->spu_sleep)
+		return err;
+
+	err = ds_send_control(dev, COMM_SET_DURATION | COMM_IM, del);
 	if (err)
 		return err;
  
-	if (delay) {
-		err = ds_send_control(dev, COMM_SET_DURATION | COMM_IM, del);
-		if (err)
-			return err;
+	dev->spu_sleep = ms;
  
-		/* Just storing delay would not get the trunication and
-		 * roundup.
-		 */
-		dev->spu_sleep = del<<4;
-	}
-
 	return err;
 }
  
  
@@ -577,11 +602,11 @@
 	struct ds_status st;
 	u8 rbyte;
  
-	err = ds_send_control(dev, COMM_BYTE_IO | COMM_IM | COMM_SPU, byte);
+	err = ds_send_control(dev, COMM_BYTE_IO | COMM_IM | dev->spu_bit, byte);
 	if (err)
 		return err;
  
-	if (dev->spu_sleep)
+	if (dev->spu_bit)
 		msleep(dev->spu_sleep);
  
 	err = ds_wait_status(dev, &st);
  
@@ -648,11 +673,11 @@
 	if (err < 0)
 		return err;
  
-	err = ds_send_control(dev, COMM_BLOCK_IO | COMM_IM | COMM_SPU, len);
+	err = ds_send_control(dev, COMM_BLOCK_IO | COMM_IM | dev->spu_bit, len);
 	if (err)
 		return err;
  
-	if (dev->spu_sleep)
+	if (dev->spu_bit)
 		msleep(dev->spu_sleep);
  
 	ds_wait_status(dev, &st);
@@ -849,7 +874,7 @@
 	 * the input buffer.  This will cause the next read to fail
 	 * see the note in ds_recv_data.
 	 */
-	ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0);
+	ds_reset_device(dev);
  
 	dev->master.data	= dev;
 	dev->master.touch_bit	= &ds9490r_touch_bit;
@@ -892,6 +917,7 @@
 		return -ENOMEM;
 	}
 	dev->spu_sleep = 0;
+	dev->spu_bit = 0;
 	dev->udev = usb_get_dev(udev);
 	if (!dev->udev) {
 		err = -ENOMEM;
...	...	@@ -141,6 +141,10 @@
141	141	* 0: pullup not active, else duration in milliseconds
142	142	*/
143	143	int spu_sleep;
	144	+ /* spu_bit contains COMM_SPU or 0 depending on if the strong pullup
	145	+ * should be active or not for writes.
	146	+ */
	147	+ u16 spu_bit;
144	148
145	149	struct w1_bus_master master;
146	150	};
...	...	@@ -311,6 +315,25 @@
311	315	}
312	316	}
313	317
	318	+static void ds_reset_device(struct ds_device *dev)
	319	+{
	320	+ ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0);
	321	+ /* Always allow strong pullup which allow individual writes to use
	322	+ * the strong pullup.
	323	+ */
	324	+ if (ds_send_control_mode(dev, MOD_PULSE_EN, PULSE_SPUE))
	325	+ printk(KERN_ERR "ds_reset_device: "
	326	+ "Error allowing strong pullup\n");
	327	+ /* Chip strong pullup time was cleared. */
	328	+ if (dev->spu_sleep) {
	329	+ /* lower 4 bits are 0, see ds_set_pullup */
	330	+ u8 del = dev->spu_sleep>>4;
	331	+ if (ds_send_control(dev, COMM_SET_DURATION \| COMM_IM, del))
	332	+ printk(KERN_ERR "ds_reset_device: "
	333	+ "Error setting duration\n");
	334	+ }
	335	+}
	336	+
314	337	static int ds_recv_data(struct ds_device dev, unsigned char buf, int size)
315	338	{
316	339	int count, err;
...	...	@@ -444,7 +467,7 @@
444	467
445	468	if (err >= 16 && st->status & ST_EPOF) {
446	469	printk(KERN_INFO "Resetting device after ST_EPOF.\n");
447		- ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0);
	470	+ ds_reset_device(dev);
448	471	/* Always dump the device status. */
449	472	count = 101;
450	473	}
451	474
452	475
453	476
454	477
...	...	@@ -509,25 +532,27 @@
509	532
510	533	static int ds_set_pullup(struct ds_device *dev, int delay)
511	534	{
512		- int err;
	535	+ int err = 0;
513	536	u8 del = 1 + (u8)(delay >> 4);
	537	+ /* Just storing delay would not get the trunication and roundup. */
	538	+ int ms = del<<4;
514	539
515		- dev->spu_sleep = 0;
516		- err = ds_send_control_mode(dev, MOD_PULSE_EN, delay ? PULSE_SPUE : 0);
	540	+ /* Enable spu_bit if a delay is set. */
	541	+ dev->spu_bit = delay ? COMM_SPU : 0;
	542	+ /* If delay is zero, it has already been disabled, if the time is
	543	+ * the same as the hardware was last programmed to, there is also
	544	+ * nothing more to do. Compare with the recalculated value ms
	545	+ * rather than del or delay which can have a different value.
	546	+ */
	547	+ if (delay == 0 \|\| ms == dev->spu_sleep)
	548	+ return err;
	549	+
	550	+ err = ds_send_control(dev, COMM_SET_DURATION \| COMM_IM, del);
517	551	if (err)
518	552	return err;
519	553
520		- if (delay) {
521		- err = ds_send_control(dev, COMM_SET_DURATION \| COMM_IM, del);
522		- if (err)
523		- return err;
	554	+ dev->spu_sleep = ms;
524	555
525		- /* Just storing delay would not get the trunication and
526		- * roundup.
527		- */
528		- dev->spu_sleep = del<<4;
529		- }
530		-
531	556	return err;
532	557	}
533	558
534	559
...	...	@@ -577,11 +602,11 @@
577	602	struct ds_status st;
578	603	u8 rbyte;
579	604
580		- err = ds_send_control(dev, COMM_BYTE_IO \| COMM_IM \| COMM_SPU, byte);
	605	+ err = ds_send_control(dev, COMM_BYTE_IO \| COMM_IM \| dev->spu_bit, byte);
581	606	if (err)
582	607	return err;
583	608
584		- if (dev->spu_sleep)
	609	+ if (dev->spu_bit)
585	610	msleep(dev->spu_sleep);
586	611
587	612	err = ds_wait_status(dev, &st);
588	613
...	...	@@ -648,11 +673,11 @@
648	673	if (err < 0)
649	674	return err;
650	675
651		- err = ds_send_control(dev, COMM_BLOCK_IO \| COMM_IM \| COMM_SPU, len);
	676	+ err = ds_send_control(dev, COMM_BLOCK_IO \| COMM_IM \| dev->spu_bit, len);
652	677	if (err)
653	678	return err;
654	679
655		- if (dev->spu_sleep)
	680	+ if (dev->spu_bit)
656	681	msleep(dev->spu_sleep);
657	682
658	683	ds_wait_status(dev, &st);
...	...	@@ -849,7 +874,7 @@
849	874	* the input buffer. This will cause the next read to fail
850	875	* see the note in ds_recv_data.
851	876	*/
852		- ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0);
	877	+ ds_reset_device(dev);
853	878
854	879	dev->master.data = dev;
855	880	dev->master.touch_bit = &ds9490r_touch_bit;
...	...	@@ -892,6 +917,7 @@
892	917	return -ENOMEM;
893	918	}
894	919	dev->spu_sleep = 0;
	920	+ dev->spu_bit = 0;
895	921	dev->udev = usb_get_dev(udev);
896	922	if (!dev->udev) {
897	923	err = -ENOMEM;