linux-kernel - Re: [PATCH v2] Bluetooth: Add hci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 31 Dec 2013 23:12:02 +0100
From:	Pavel Machek <pavel@....cz>
To:	Marcel Holtmann <marcel@...tmann.org>
Cc:	Pali Rohár <pali.rohar@...il.com>,
	Ивайло Димитров 
	<freemangordon@....bg>, "Gustavo F. Padovan" <gustavo@...ovan.org>,
	Johan Hedberg <johan.hedberg@...il.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	"linux-bluetooth@...r.kernel.org development" 
	<linux-bluetooth@...r.kernel.org>,
	Ville Tervo <ville.tervo@...ia.com>,
	Sebastian Reichel <sre@...g0.de>
Subject: Re: [PATCH v2] Bluetooth: Add hci_h4p driver

Hi!

> > +static struct task_struct *h4p_thread;
> 
> Can’t this be done using a work queue. You are looking at a 3.14
> kernel the earliest. We have way better primitives these days.

I tried to convert it to work queue, but was not too
succesfull. Workqueue is not really good match for what this is trying
to do... Nokia code relies on sleeping, than timing those sleeps for
signaling. I'm still trying to wrap my head around it.

Ok, I guess I could convert it to one big workqueue task, and leave
the logic alone. Was that what you wanted?

In the meantime, while trying to untangle it:

commit b02dc19d9269f22baa705d16d1019b86acf15758
Author: Pavel <pavel@....cz>
Date:   Tue Dec 31 23:01:23 2013 +0100

    De-obfuscate power management code. Someone please carefully review
    this.
    
    Signed-off-by: Pavel Machek <pavel@....cz>

diff --git a/drivers/bluetooth/nokia_core.c b/drivers/bluetooth/nokia_core.c
index 85dd106..07761a3 100644
--- a/drivers/bluetooth/nokia_core.c
+++ b/drivers/bluetooth/nokia_core.c
@@ -705,28 +705,22 @@ static irqreturn_t hci_h4p_wakeup_interrupt(int irq, void *dev_inst)
 static inline void hci_h4p_set_pm_limits(struct hci_h4p_info *info, bool set)
 {
 	struct hci_h4p_platform_data *bt_plat_data = info->dev->platform_data;
+	char *sset = set ? "set" : "clear";
 
 	if (unlikely(!bt_plat_data || !bt_plat_data->set_pm_limits))
 		return;
 
-	if (set && !test_bit(H4P_ACTIVE_MODE, &info->pm_flags)) {
+	if (set != !!test_bit(H4P_ACTIVE_MODE, &info->pm_flags)) {
 		bt_plat_data->set_pm_limits(info->dev, set);
-		set_bit(H4P_ACTIVE_MODE, &info->pm_flags);
-		BT_DBG("Change pm constraints to: %s", set ?
-				"set" : "clear");
+		if (set)
+			set_bit(H4P_ACTIVE_MODE, &info->pm_flags);
+		else
+			clear_bit(H4P_ACTIVE_MODE, &info->pm_flags);
+		BT_DBG("Change pm constraints to: %s", sset);
 		return;
 	}
 
-	if (!set && test_bit(H4P_ACTIVE_MODE, &info->pm_flags)) {
-		bt_plat_data->set_pm_limits(info->dev, set);
-		clear_bit(H4P_ACTIVE_MODE, &info->pm_flags);
-		BT_DBG("Change pm constraints to: %s",
-				set ? "set" : "clear");
-		return;
-	}
-
-	BT_DBG("pm constraints remains: %s",
-			set ? "set" : "clear");
+	BT_DBG("pm constraints remains: %s", sset);
 }
 
 static int h4p_run(void *data)
@@ -762,16 +756,17 @@ static int h4p_run(void *data)
 		BT_DBG("Timeout before calculation = %u",
 				jiffies_to_msecs(timeout));
 
-		/* Empiric analyzer  :-) */
 		if (elapsed < TIMEOUT_MIN) {
 			timeout <<= 1;
-			timeout = (timeout > TIMEOUT_MAX) ?
-				TIMEOUT_MAX : timeout;
 		} else {
-			timeout = (elapsed > timeout - TIMEOUT_MIN) ?
-				TIMEOUT_MIN : timeout - elapsed;
+			timeout -= elapsed;
 		}
 
+		if (timeout > TIMEOUT_MAX)
+			timeout = TIMEOUT_MAX;
+		if (timeout < TIMEOUT_MIN)
+			timeout = TIMEOUT_MIN;
+
 		BT_DBG("Timeout after calculation = %u",
 				jiffies_to_msecs(timeout));
 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/