On Wed, Jun 08, 2011 at 02:54:40PM +0300, Kalle Valo wrote:
> From: Kalle Valo <kalle.valo@xxxxxxxxxxx>
>
> Drivers should not request firmware during resume. Fix ath6kl to
> cache the firmware instead.
>
> Signed-off-by: Kalle Valo <kalle.valo@xxxxxxxxxxx>
> ---
> drivers/staging/ath6kl/os/linux/ar6000_drv.c | 53 ++++++++++++++------
> .../staging/ath6kl/os/linux/include/ar6000_drv.h | 9 +++
> 2 files changed, 47 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/staging/ath6kl/os/linux/ar6000_drv.c b/drivers/staging/ath6kl/os/linux/ar6000_drv.c
> index 30ae63b..74aa343 100644
> --- a/drivers/staging/ath6kl/os/linux/ar6000_drv.c
> +++ b/drivers/staging/ath6kl/os/linux/ar6000_drv.c
> @@ -954,9 +954,13 @@ ar6000_transfer_bin_file(struct ar6_softc *ar, AR6K_BIN_FILE file, u32 address,
> const char *filename;
> const struct firmware *fw_entry;
> u32 fw_entry_size;
> + u8 **buf;
> + size_t *buf_len;
Don't make buf_len a pointer that just makes the code messier.
[snip]
> +
> + if (WARN_ON((buf == NULL) || (buf_len == NULL)))
> + return A_ERROR;
This test is a nonsense. "buf" and buf_len are always valid
pointers here. Generally, I'd almost prefer a oops with a stack
trace instead of adding in bogus error handling code for stuff that
can't happen.
Also "buf" is never initialized to NULL, so if there were a
programming bug introduced, it would generate an uninitialized
variable warning even without the superflous test. So lets remove
it.
> +
> + if (*buf == NULL) {
> + if ((A_REQUEST_FIRMWARE(&fw_entry, filename, ((struct device *)ar->osDevInfo.pOSDevice))) != 0)
> + {
^
This is on the wrong line.
> + AR_DEBUG_PRINTF(ATH_DEBUG_ERR, ("Failed to get %s\n", filename));
> + return A_ENOENT;
> + }
> +
> + *buf = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
> + *buf_len = fw_entry->size;
> + A_RELEASE_FIRMWARE(fw_entry);
> }
>
> #ifdef SOFTMAC_FILE_USED
> - if (file==AR6K_BOARD_DATA_FILE && fw_entry->data) {
> - ar6000_softmac_update(ar, (u8 *)fw_entry->data, fw_entry->size);
> + if (file==AR6K_BOARD_DATA_FILE && *buf_len) {
^^^^
White space.
> + ar6000_softmac_update(ar, *buf, *buf_len);
> }
> #endif
>
>
> - fw_entry_size = fw_entry->size;
> + fw_entry_size = *buf_len;
>
> /* Load extended board data for AR6003 */
> - if ((file==AR6K_BOARD_DATA_FILE) && (fw_entry->data)) {
> + if ((file==AR6K_BOARD_DATA_FILE) && *buf) {
^^^^
White space is wrong here.
> u32 board_ext_address;
> u32 board_ext_data_size;
> u32 board_data_size;
> @@ -1089,14 +1109,13 @@ ar6000_transfer_bin_file(struct ar6_softc *ar, AR6K_BIN_FILE file, u32 address,
> AR_DEBUG_PRINTF(ATH_DEBUG_INFO, ("Board extended Data download address: 0x%x\n", board_ext_address));
>
> /* check whether the target has allocated memory for extended board data and file contains extended board data */
> - if ((board_ext_address) && (fw_entry->size == (board_data_size + board_ext_data_size))) {
> + if ((board_ext_address) && (*buf_len == (board_data_size + board_ext_data_size))) {
^ ^
Those parenthesis are not needed.
> u32 param;
>
> - status = BMIWriteMemory(ar->arHifDevice, board_ext_address, (u8 *)(fw_entry->data + board_data_size), board_ext_data_size);
> + status = BMIWriteMemory(ar->arHifDevice, board_ext_address, (u8 *)(*buf + board_data_size), board_ext_data_size);
^^^^^^
This cast isn't needed by the way.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
