| 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
| ||
|
Date: Wed, 1 Jan 2014 21:09:27 +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?
>
> the Bluetooth subsystem moved away from tasklets and uses workqueues for everything. So this should be just fine for this driver as well. I do not know about what timings are required, but they should only matter during initial device setup. The HCI traffic is actually driven by the Bluetooth core.
>
I actually tried to convert it to workqueue... result is below but I
was not successful.
h4p driver actually has one long-running, mostly sleeping, thread and
communicates with it by sending it wakeups; the thread toggles some
power management options based on activity. (No, I don't like that
design). That is not something that can be easily converted to
workqueue, AFAICT.
I don't think other Bluetooth drivers do anything similar.
Pavel
diff --git a/drivers/bluetooth/hci_h4p.h b/drivers/bluetooth/hci_h4p.h
index a2174ea..5dcbaa1 100644
--- a/drivers/bluetooth/hci_h4p.h
+++ b/drivers/bluetooth/hci_h4p.h
@@ -19,12 +19,13 @@
*
*/
+#ifndef __DRIVERS_BLUETOOTH_HCI_H4P_H
+#define __DRIVERS_BLUETOOTH_HCI_H4P_H
+
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
#include <net/bluetooth/hci.h>
-
-#ifndef __DRIVERS_BLUETOOTH_HCI_H4P_H
-#define __DRIVERS_BLUETOOTH_HCI_H4P_H
+#include <linux/workqueue.h>
#define FW_NAME_TI1271_PRELE "ti1273_prele.bin"
#define FW_NAME_TI1271_LE "ti1273_le.bin"
@@ -103,6 +104,13 @@ struct hci_h4p_info {
u16 ier;
u16 mdr1;
u16 efr;
+
+#if 1
+ struct work_struct work;
+ struct workqueue_struct work_queue; /* FIXME: init me */
+ unsigned long last_transfer_jiffies;
+ unsigned long transfer_timeout;
+#endif
};
struct hci_h4p_radio_hdr {
diff --git a/drivers/bluetooth/nokia_core.c b/drivers/bluetooth/nokia_core.c
index 07761a3..6e37866 100644
--- a/drivers/bluetooth/nokia_core.c
+++ b/drivers/bluetooth/nokia_core.c
@@ -35,7 +35,6 @@
#include <linux/interrupt.h>
#include <linux/gpio.h>
#include <linux/timer.h>
-#include <linux/kthread.h>
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -45,8 +44,6 @@
#include "hci_h4p.h"
-static struct task_struct *h4p_thread;
-
/* This should be used in function that cannot release clocks */
static void hci_h4p_set_clk(struct hci_h4p_info *info, int *clock, int enable)
{
@@ -103,13 +100,11 @@ void hci_h4p_smart_idle(struct hci_h4p_info *info, bool enable)
static inline void h4p_schedule_pm(struct hci_h4p_info *info)
{
- if (unlikely(!h4p_thread))
- return;
-
set_bit(H4P_SCHED_TRANSFER_MODE, &info->pm_flags);
if (unlikely(!test_bit(H4P_TRANSFER_MODE, &info->pm_flags)))
- wake_up_process(h4p_thread);
+ /* FIXME */
+ /* wake_up_process(h4p_thread) */ ;
}
static void hci_h4p_disable_tx(struct hci_h4p_info *info)
@@ -723,18 +718,18 @@ static inline void hci_h4p_set_pm_limits(struct hci_h4p_info *info, bool set)
BT_DBG("pm constraints remains: %s", sset);
}
-static int h4p_run(void *data)
+static int h4p_run(struct work_struct *work)
{
#define TIMEOUT_MIN msecs_to_jiffies(100)
#define TIMEOUT_MAX msecs_to_jiffies(2000)
- struct hci_h4p_info *info = data;
+ struct hci_h4p_info *info = container_of(work, struct hci_h4p_info, work);
unsigned long last_jiffies = jiffies;
unsigned long timeout = TIMEOUT_MIN;
unsigned long elapsed;
BT_DBG("");
set_user_nice(current, -10);
- while (!kthread_should_stop()) {
+ while (1) {
set_current_state(TASK_INTERRUPTIBLE);
if (!test_bit(H4P_SCHED_TRANSFER_MODE, &info->pm_flags)) {
if (timeout != TIMEOUT_MIN) {
@@ -998,7 +993,8 @@ static int hci_h4p_hci_close(struct hci_dev *hdev)
return 0;
/* Wake up h4p_thread which removes pm constraints */
- wake_up_process(h4p_thread);
+ /* FIXME */
+// wake_up_process(h4p_thread);
hci_h4p_hci_flush(hdev);
hci_h4p_set_clk(info, &info->tx_clocks_en, 1);
@@ -1272,12 +1268,11 @@ static int hci_h4p_probe(struct platform_device *pdev)
goto cleanup_irq;
}
- h4p_thread = kthread_run(h4p_run, info, "h4p_pm");
- if (IS_ERR(h4p_thread)) {
- err = PTR_ERR(h4p_thread);
- goto cleanup_irq;
- }
+ INIT_WORK(&info->work, h4p_run);
+// schedule_work(&info->work);
+// info->work_queue = init_work
+ queue_work(info->work_queue, &info->work);
return 0;
cleanup_irq:
@@ -1300,7 +1295,7 @@ static int hci_h4p_remove(struct platform_device *pdev)
info = platform_get_drvdata(pdev);
- kthread_stop(h4p_thread);
+ cancel_work_sync(&info->work);
hci_h4p_sysfs_remove_files(info->dev);
hci_h4p_hci_close(info->hdev);
--
(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/
Powered by blists - more mailing lists