There is no need to hold the firmware in memory.
Signed-off-by: Alexander Holler <[email protected]>
---
drivers/bluetooth/ath3k.c | 75 ++++++++++++---------------------------------
1 files changed, 20 insertions(+), 55 deletions(-)
diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
index 128cae4..81cd1ed 100644
--- a/drivers/bluetooth/ath3k.c
+++ b/drivers/bluetooth/ath3k.c
@@ -43,46 +43,40 @@ MODULE_DEVICE_TABLE(usb, ath3k_table);
#define USB_REQ_DFU_DNLOAD 1
#define BULK_SIZE 4096
-struct ath3k_data {
- struct usb_device *udev;
- u8 *fw_data;
- u32 fw_size;
- u32 fw_sent;
-};
-
-static int ath3k_load_firmware(struct ath3k_data *data,
- unsigned char *firmware,
- int count)
+static int ath3k_load_firmware(struct usb_device *udev,
+ const struct firmware *firmware)
{
u8 *send_buf;
int err, pipe, len, size, sent = 0;
+ int count = firmware->size;
- BT_DBG("ath3k %p udev %p", data, data->udev);
+ BT_DBG("udev %p", udev);
- pipe = usb_sndctrlpipe(data->udev, 0);
+ pipe = usb_sndctrlpipe(udev, 0);
- if ((usb_control_msg(data->udev, pipe,
+ send_buf = kmalloc(BULK_SIZE, GFP_ATOMIC);
+ if (!send_buf) {
+ BT_ERR("Can't allocate memory chunk for firmware");
+ return -ENOMEM;
+ }
+
+ memcpy(send_buf, firmware->data, 20);
+ if ((err = usb_control_msg(udev, pipe,
USB_REQ_DFU_DNLOAD,
USB_TYPE_VENDOR, 0, 0,
- firmware, 20, USB_CTRL_SET_TIMEOUT)) < 0) {
+ send_buf, 20, USB_CTRL_SET_TIMEOUT)) < 0) {
BT_ERR("Can't change to loading configuration err");
- return -EBUSY;
+ goto error;
}
sent += 20;
count -= 20;
- send_buf = kmalloc(BULK_SIZE, GFP_ATOMIC);
- if (!send_buf) {
- BT_ERR("Can't allocate memory chunk for firmware");
- return -ENOMEM;
- }
-
while (count) {
size = min_t(uint, count, BULK_SIZE);
- pipe = usb_sndbulkpipe(data->udev, 0x02);
- memcpy(send_buf, firmware + sent, size);
+ pipe = usb_sndbulkpipe(udev, 0x02);
+ memcpy(send_buf, firmware->data + sent, size);
- err = usb_bulk_msg(data->udev, pipe, send_buf, size,
+ err = usb_bulk_msg(udev, pipe, send_buf, size,
&len, 3000);
if (err || (len != size)) {
@@ -108,57 +102,28 @@ static int ath3k_probe(struct usb_interface *intf,
{
const struct firmware *firmware;
struct usb_device *udev = interface_to_usbdev(intf);
- struct ath3k_data *data;
- int size;
BT_DBG("intf %p id %p", intf, id);
if (intf->cur_altsetting->desc.bInterfaceNumber != 0)
return -ENODEV;
- data = kzalloc(sizeof(*data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- data->udev = udev;
-
if (request_firmware(&firmware, "ath3k-1.fw", &udev->dev) < 0) {
- kfree(data);
return -EIO;
}
- size = max_t(uint, firmware->size, 4096);
- data->fw_data = kmalloc(size, GFP_KERNEL);
- if (!data->fw_data) {
+ if (ath3k_load_firmware(udev, firmware)) {
release_firmware(firmware);
- kfree(data);
- return -ENOMEM;
- }
-
- memcpy(data->fw_data, firmware->data, firmware->size);
- data->fw_size = firmware->size;
- data->fw_sent = 0;
- release_firmware(firmware);
-
- usb_set_intfdata(intf, data);
- if (ath3k_load_firmware(data, data->fw_data, data->fw_size)) {
- usb_set_intfdata(intf, NULL);
- kfree(data->fw_data);
- kfree(data);
return -EIO;
}
+ release_firmware(firmware);
return 0;
}
static void ath3k_disconnect(struct usb_interface *intf)
{
- struct ath3k_data *data = usb_get_intfdata(intf);
-
BT_DBG("ath3k_disconnect intf %p", intf);
-
- kfree(data->fw_data);
- kfree(data);
}
static struct usb_driver ath3k_driver = {
--
1.7.2.3
Hello,
Am 30.11.2010 02:52, schrieb Gustavo F. Padovan:
>> - if ((usb_control_msg(data->udev, pipe,
>> + send_buf = kmalloc(BULK_SIZE, GFP_ATOMIC);
>> + if (!send_buf) {
>> + BT_ERR("Can't allocate memory chunk for firmware");
>> + return -ENOMEM;
>> + }
>> +
>> + memcpy(send_buf, firmware->data, 20);
>> + if ((err = usb_control_msg(udev, pipe,
>> USB_REQ_DFU_DNLOAD,
>> USB_TYPE_VENDOR, 0, 0,
>> - firmware, 20, USB_CTRL_SET_TIMEOUT))< 0) {
>> + send_buf, 20, USB_CTRL_SET_TIMEOUT))< 0) {
>> BT_ERR("Can't change to loading configuration err");
>> - return -EBUSY;
>> + goto error;
>> }
>> sent += 20;
>> count -= 20;
>
> Patch looks good to me, but I have a question here: what's 20 here? I
> didn't figured out.
I don't know. I assume it's a stub which has to be send before the real
firmware. It already was there and I haven't touched that.
Regards,
Alexander
Hi Alexander,
* Alexander Holler <[email protected]> [2010-11-22 21:09:01 +0100]:
> There is no need to hold the firmware in memory.
>
> Signed-off-by: Alexander Holler <[email protected]>
> ---
> drivers/bluetooth/ath3k.c | 75 ++++++++++++---------------------------------
> 1 files changed, 20 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
> index 128cae4..81cd1ed 100644
> --- a/drivers/bluetooth/ath3k.c
> +++ b/drivers/bluetooth/ath3k.c
> @@ -43,46 +43,40 @@ MODULE_DEVICE_TABLE(usb, ath3k_table);
> #define USB_REQ_DFU_DNLOAD 1
> #define BULK_SIZE 4096
>
> -struct ath3k_data {
> - struct usb_device *udev;
> - u8 *fw_data;
> - u32 fw_size;
> - u32 fw_sent;
> -};
> -
> -static int ath3k_load_firmware(struct ath3k_data *data,
> - unsigned char *firmware,
> - int count)
> +static int ath3k_load_firmware(struct usb_device *udev,
> + const struct firmware *firmware)
> {
> u8 *send_buf;
> int err, pipe, len, size, sent = 0;
> + int count = firmware->size;
>
> - BT_DBG("ath3k %p udev %p", data, data->udev);
> + BT_DBG("udev %p", udev);
>
> - pipe = usb_sndctrlpipe(data->udev, 0);
> + pipe = usb_sndctrlpipe(udev, 0);
>
> - if ((usb_control_msg(data->udev, pipe,
> + send_buf = kmalloc(BULK_SIZE, GFP_ATOMIC);
> + if (!send_buf) {
> + BT_ERR("Can't allocate memory chunk for firmware");
> + return -ENOMEM;
> + }
> +
> + memcpy(send_buf, firmware->data, 20);
> + if ((err = usb_control_msg(udev, pipe,
> USB_REQ_DFU_DNLOAD,
> USB_TYPE_VENDOR, 0, 0,
> - firmware, 20, USB_CTRL_SET_TIMEOUT)) < 0) {
> + send_buf, 20, USB_CTRL_SET_TIMEOUT)) < 0) {
> BT_ERR("Can't change to loading configuration err");
> - return -EBUSY;
> + goto error;
> }
> sent += 20;
> count -= 20;
Patch looks good to me, but I have a question here: what's 20 here? I
didn't figured out.
Vikram, what's your opinion on this patch? Can you ack/nack it?
--
Gustavo F. Padovan
http://profusion.mobi
Hello,
Am 03.12.2010 11:51, schrieb Bala Shanmugam:
> I manually made these changes and its not working.
> I am unable to apply Alex patch taken from my office id.
> Alex can you send the patch to [email protected].
The patch applies cleanly to 2.6.36.x and the current 2.6.37 as found in
Linus tree. I'm using that patch on an armv5, a x86_64 (both 2.6.36.1)
and on an armv7 with 2.6.37-rc4.
I've sent it to the above e-mail address.
Regards,
Alexander
On 11/30/2010 2:20 PM, Alexander Holler wrote:
> Hello,
>
> Am 30.11.2010 02:52, schrieb Gustavo F. Padovan:
>>> - if ((usb_control_msg(data->udev, pipe,
>>> + send_buf = kmalloc(BULK_SIZE, GFP_ATOMIC);
>>> + if (!send_buf) {
>>> + BT_ERR("Can't allocate memory chunk for firmware");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + memcpy(send_buf, firmware->data, 20);
>>> + if ((err = usb_control_msg(udev, pipe,
>>> USB_REQ_DFU_DNLOAD,
>>> USB_TYPE_VENDOR, 0, 0,
>>> - firmware, 20, USB_CTRL_SET_TIMEOUT))< 0) {
>>> + send_buf, 20, USB_CTRL_SET_TIMEOUT))< 0) {
>>> BT_ERR("Can't change to loading configuration err");
>>> - return -EBUSY;
>>> + goto error;
>>> }
>>> sent += 20;
>>> count -= 20;
>> Patch looks good to me, but I have a question here: what's 20 here? I
>> didn't figured out.
> I don't know. I assume it's a stub which has to be send before the real
> firmware. It already was there and I haven't touched that.
>
> Regards,
>
> Alexander
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
The first 20 bytes contain information about the binary like binary
length, crc,
jump address etc.,
I manually made these changes and its not working.
I am unable to apply Alex patch taken from my office id.
Alex can you send the patch to [email protected].
Regards,
BAla.
On 1/14/2011 2:55 AM, Alexander Holler wrote:
> Hello,
>
> On 13.01.2011 20:08, Gustavo F. Padovan wrote:
>> Hi Alexander,
>>
>> * Alexander Holler<[email protected]> [2010-11-22 21:09:01 +0100]:
>>
>>> There is no need to hold the firmware in memory.
>>>
>>> Signed-off-by: Alexander Holler<[email protected]>
>>> ---
>>> drivers/bluetooth/ath3k.c | 75 ++++++++++++---------------------------------
>>> 1 files changed, 20 insertions(+), 55 deletions(-)
>> I'm assuming you tested it a lot. Patch has been applied. Thanks.
> Yes, I'm using it here haven't had a problem with that patch.
>
> Thanks,
>
> Alexander
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks Alex for the patch.
Its working.
Regards,
Bala.
Hello,
On 13.01.2011 20:08, Gustavo F. Padovan wrote:
> Hi Alexander,
>
> * Alexander Holler<[email protected]> [2010-11-22 21:09:01 +0100]:
>
>> There is no need to hold the firmware in memory.
>>
>> Signed-off-by: Alexander Holler<[email protected]>
>> ---
>> drivers/bluetooth/ath3k.c | 75 ++++++++++++---------------------------------
>> 1 files changed, 20 insertions(+), 55 deletions(-)
>
> I'm assuming you tested it a lot. Patch has been applied. Thanks.
Yes, I'm using it here haven't had a problem with that patch.
Thanks,
Alexander
Hi Alexander,
* Alexander Holler <[email protected]> [2010-11-22 21:09:01 +0100]:
> There is no need to hold the firmware in memory.
>
> Signed-off-by: Alexander Holler <[email protected]>
> ---
> drivers/bluetooth/ath3k.c | 75 ++++++++++++---------------------------------
> 1 files changed, 20 insertions(+), 55 deletions(-)
I'm assuming you tested it a lot. Patch has been applied. Thanks.
--
Gustavo F. Padovan
http://profusion.mobi
On 1/10/2011 10:24 PM, Alexander Holler wrote:
> Hello,
>
> because the merge window is currently open, should I resend that patch
> or post it on another mailing list too?
>
> I still think that driver should not waste about 250kb of memory. ;)
>
> Regards,
>
> Alexander
>
> Am 30.11.2010 02:52, schrieb Gustavo F. Padovan:
>> Hi Alexander,
>>
>> * Alexander Holler<[email protected]> [2010-11-22 21:09:01 +0100]:
>>
>>> There is no need to hold the firmware in memory.
>>>
>>> Signed-off-by: Alexander Holler<[email protected]>
>>> ---
>>> drivers/bluetooth/ath3k.c | 75 ++++++++++++---------------------------------
>>> 1 files changed, 20 insertions(+), 55 deletions(-)
>>>
>>> diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
>>> index 128cae4..81cd1ed 100644
>>> --- a/drivers/bluetooth/ath3k.c
>>> +++ b/drivers/bluetooth/ath3k.c
>>> @@ -43,46 +43,40 @@ MODULE_DEVICE_TABLE(usb, ath3k_table);
>>> #define USB_REQ_DFU_DNLOAD 1
>>> #define BULK_SIZE 4096
>>>
>>> -struct ath3k_data {
>>> - struct usb_device *udev;
>>> - u8 *fw_data;
>>> - u32 fw_size;
>>> - u32 fw_sent;
>>> -};
>>> -
>>> -static int ath3k_load_firmware(struct ath3k_data *data,
>>> - unsigned char *firmware,
>>> - int count)
>>> +static int ath3k_load_firmware(struct usb_device *udev,
>>> + const struct firmware *firmware)
>>> {
>>> u8 *send_buf;
>>> int err, pipe, len, size, sent = 0;
>>> + int count = firmware->size;
>>>
>>> - BT_DBG("ath3k %p udev %p", data, data->udev);
>>> + BT_DBG("udev %p", udev);
>>>
>>> - pipe = usb_sndctrlpipe(data->udev, 0);
>>> + pipe = usb_sndctrlpipe(udev, 0);
>>>
>>> - if ((usb_control_msg(data->udev, pipe,
>>> + send_buf = kmalloc(BULK_SIZE, GFP_ATOMIC);
>>> + if (!send_buf) {
>>> + BT_ERR("Can't allocate memory chunk for firmware");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + memcpy(send_buf, firmware->data, 20);
>>> + if ((err = usb_control_msg(udev, pipe,
>>> USB_REQ_DFU_DNLOAD,
>>> USB_TYPE_VENDOR, 0, 0,
>>> - firmware, 20, USB_CTRL_SET_TIMEOUT))< 0) {
>>> + send_buf, 20, USB_CTRL_SET_TIMEOUT))< 0) {
>>> BT_ERR("Can't change to loading configuration err");
>>> - return -EBUSY;
>>> + goto error;
>>> }
>>> sent += 20;
>>> count -= 20;
>> Patch looks good to me, but I have a question here: what's 20 here? I
>> didn't figured out.
>>
>> Vikram, what's your opinion on this patch? Can you ack/nack it?
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Alex,
Sorry for late reply.
Was held up in a work.
I will check your patch and update soon.
Can you send a fresh patch cc to [email protected]
Regards,
Bala.
Hello,
because the merge window is currently open, should I resend that patch
or post it on another mailing list too?
I still think that driver should not waste about 250kb of memory. ;)
Regards,
Alexander
Am 30.11.2010 02:52, schrieb Gustavo F. Padovan:
> Hi Alexander,
>
> * Alexander Holler<[email protected]> [2010-11-22 21:09:01 +0100]:
>
>> There is no need to hold the firmware in memory.
>>
>> Signed-off-by: Alexander Holler<[email protected]>
>> ---
>> drivers/bluetooth/ath3k.c | 75 ++++++++++++---------------------------------
>> 1 files changed, 20 insertions(+), 55 deletions(-)
>>
>> diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
>> index 128cae4..81cd1ed 100644
>> --- a/drivers/bluetooth/ath3k.c
>> +++ b/drivers/bluetooth/ath3k.c
>> @@ -43,46 +43,40 @@ MODULE_DEVICE_TABLE(usb, ath3k_table);
>> #define USB_REQ_DFU_DNLOAD 1
>> #define BULK_SIZE 4096
>>
>> -struct ath3k_data {
>> - struct usb_device *udev;
>> - u8 *fw_data;
>> - u32 fw_size;
>> - u32 fw_sent;
>> -};
>> -
>> -static int ath3k_load_firmware(struct ath3k_data *data,
>> - unsigned char *firmware,
>> - int count)
>> +static int ath3k_load_firmware(struct usb_device *udev,
>> + const struct firmware *firmware)
>> {
>> u8 *send_buf;
>> int err, pipe, len, size, sent = 0;
>> + int count = firmware->size;
>>
>> - BT_DBG("ath3k %p udev %p", data, data->udev);
>> + BT_DBG("udev %p", udev);
>>
>> - pipe = usb_sndctrlpipe(data->udev, 0);
>> + pipe = usb_sndctrlpipe(udev, 0);
>>
>> - if ((usb_control_msg(data->udev, pipe,
>> + send_buf = kmalloc(BULK_SIZE, GFP_ATOMIC);
>> + if (!send_buf) {
>> + BT_ERR("Can't allocate memory chunk for firmware");
>> + return -ENOMEM;
>> + }
>> +
>> + memcpy(send_buf, firmware->data, 20);
>> + if ((err = usb_control_msg(udev, pipe,
>> USB_REQ_DFU_DNLOAD,
>> USB_TYPE_VENDOR, 0, 0,
>> - firmware, 20, USB_CTRL_SET_TIMEOUT))< 0) {
>> + send_buf, 20, USB_CTRL_SET_TIMEOUT))< 0) {
>> BT_ERR("Can't change to loading configuration err");
>> - return -EBUSY;
>> + goto error;
>> }
>> sent += 20;
>> count -= 20;
>
> Patch looks good to me, but I have a question here: what's 20 here? I
> didn't figured out.
>
> Vikram, what's your opinion on this patch? Can you ack/nack it?
>