After a recent change in clang to expose uninitialized warnings from const variables [1], there is a warning in cxacru_heavy_init():
drivers/usb/atm/cxacru.c:1104:6: error: variable 'bp' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] 1104 | if (instance->modem_type->boot_rom_patch) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/usb/atm/cxacru.c:1113:39: note: uninitialized use occurs here 1113 | cxacru_upload_firmware(instance, fw, bp); | ^~ drivers/usb/atm/cxacru.c:1104:2: note: remove the 'if' if its condition is always true 1104 | if (instance->modem_type->boot_rom_patch) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/usb/atm/cxacru.c:1095:32: note: initialize the variable 'bp' to silence this warning 1095 | const struct firmware *fw, *bp; | ^ | = NULL
This warning occurs in clang's frontend before inlining occurs, so it cannot notice that bp is only used within cxacru_upload_firmware() under the same condition that initializes it in cxacru_heavy_init(). Just initialize bp to NULL to silence the warning without functionally changing the code, which is what happens with modern compilers when they support '-ftrivial-auto-var-init=zero' (CONFIG_INIT_STACK_ALL_ZERO=y).
Cc: stable@vger.kernel.org Fixes: 1b0e61465234 ("[PATCH] USB ATM: driver for the Conexant AccessRunner chipset cxacru") Closes: https://github.com/ClangBuiltLinux/linux/issues/2102 Link: https://github.com/llvm/llvm-project/commit/2464313eef01c5b1edf0eccf57a32cde... [1] Signed-off-by: Nathan Chancellor nathan@kernel.org --- drivers/usb/atm/cxacru.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c index a12ab90b3db7..b7c3b224a759 100644 --- a/drivers/usb/atm/cxacru.c +++ b/drivers/usb/atm/cxacru.c @@ -1092,7 +1092,7 @@ static int cxacru_find_firmware(struct cxacru_data *instance, static int cxacru_heavy_init(struct usbatm_data *usbatm_instance, struct usb_interface *usb_intf) { - const struct firmware *fw, *bp; + const struct firmware *fw, *bp = NULL; struct cxacru_data *instance = usbatm_instance->driver_data; int ret = cxacru_find_firmware(instance, "fw", &fw);
--- base-commit: fdfa018c6962c86d2faa183187669569be4d513f change-id: 20250715-usb-cxacru-fix-clang-21-uninit-warning-9430d96c6bc1
Best regards, -- Nathan Chancellor nathan@kernel.org
On Tue, Jul 15, 2025 at 01:33:32PM -0700, Nathan Chancellor wrote:
After a recent change in clang to expose uninitialized warnings from const variables [1], there is a warning in cxacru_heavy_init():
drivers/usb/atm/cxacru.c:1104:6: error: variable 'bp' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] 1104 | if (instance->modem_type->boot_rom_patch) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/usb/atm/cxacru.c:1113:39: note: uninitialized use occurs here 1113 | cxacru_upload_firmware(instance, fw, bp); | ^~ drivers/usb/atm/cxacru.c:1104:2: note: remove the 'if' if its condition is always true 1104 | if (instance->modem_type->boot_rom_patch) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/usb/atm/cxacru.c:1095:32: note: initialize the variable 'bp' to silence this warning 1095 | const struct firmware *fw, *bp; | ^ | = NULL
This warning occurs in clang's frontend before inlining occurs, so it cannot notice that bp is only used within cxacru_upload_firmware() under the same condition that initializes it in cxacru_heavy_init(). Just initialize bp to NULL to silence the warning without functionally changing the code, which is what happens with modern compilers when they support '-ftrivial-auto-var-init=zero' (CONFIG_INIT_STACK_ALL_ZERO=y).
We generally do not want to paper over compiler bugs, when our code is correct, so why should we do that here? Why not fix clang instead?
thanks,
greg k-h
On Wed, Jul 16, 2025 at 07:06:50AM +0200, Greg Kroah-Hartman wrote:
On Tue, Jul 15, 2025 at 01:33:32PM -0700, Nathan Chancellor wrote:
After a recent change in clang to expose uninitialized warnings from const variables [1], there is a warning in cxacru_heavy_init():
drivers/usb/atm/cxacru.c:1104:6: error: variable 'bp' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] 1104 | if (instance->modem_type->boot_rom_patch) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/usb/atm/cxacru.c:1113:39: note: uninitialized use occurs here 1113 | cxacru_upload_firmware(instance, fw, bp); | ^~ drivers/usb/atm/cxacru.c:1104:2: note: remove the 'if' if its condition is always true 1104 | if (instance->modem_type->boot_rom_patch) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/usb/atm/cxacru.c:1095:32: note: initialize the variable 'bp' to silence this warning 1095 | const struct firmware *fw, *bp; | ^ | = NULL
This warning occurs in clang's frontend before inlining occurs, so it cannot notice that bp is only used within cxacru_upload_firmware() under the same condition that initializes it in cxacru_heavy_init(). Just initialize bp to NULL to silence the warning without functionally changing the code, which is what happens with modern compilers when they support '-ftrivial-auto-var-init=zero' (CONFIG_INIT_STACK_ALL_ZERO=y).
We generally do not want to paper over compiler bugs, when our code is correct, so why should we do that here? Why not fix clang instead?
I would not really call this a compiler bug. It IS passed uninitialized to this function and while the uninitialized value is not actually used, clang has no way of knowing that at this point in its pipeline, so I don't think warning in this case is unreasonable. This type of warning is off for GCC because of how unreliable it was when it is done in the middle end with optimizations. Furthermore, it is my understanding based on [1] that just the passing of an uninitialized variable in this manner is UB.
[1]: https://lore.kernel.org/20220614214039.GA25951@gate.crashing.org/
Cheers, Nathan
On Tue, Jul 15, 2025 at 10:24:50PM -0700, Nathan Chancellor wrote:
On Wed, Jul 16, 2025 at 07:06:50AM +0200, Greg Kroah-Hartman wrote:
On Tue, Jul 15, 2025 at 01:33:32PM -0700, Nathan Chancellor wrote:
After a recent change in clang to expose uninitialized warnings from const variables [1], there is a warning in cxacru_heavy_init():
drivers/usb/atm/cxacru.c:1104:6: error: variable 'bp' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] 1104 | if (instance->modem_type->boot_rom_patch) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/usb/atm/cxacru.c:1113:39: note: uninitialized use occurs here 1113 | cxacru_upload_firmware(instance, fw, bp); | ^~ drivers/usb/atm/cxacru.c:1104:2: note: remove the 'if' if its condition is always true 1104 | if (instance->modem_type->boot_rom_patch) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/usb/atm/cxacru.c:1095:32: note: initialize the variable 'bp' to silence this warning 1095 | const struct firmware *fw, *bp; | ^ | = NULL
This warning occurs in clang's frontend before inlining occurs, so it cannot notice that bp is only used within cxacru_upload_firmware() under the same condition that initializes it in cxacru_heavy_init(). Just initialize bp to NULL to silence the warning without functionally changing the code, which is what happens with modern compilers when they support '-ftrivial-auto-var-init=zero' (CONFIG_INIT_STACK_ALL_ZERO=y).
We generally do not want to paper over compiler bugs, when our code is correct, so why should we do that here? Why not fix clang instead?
I would not really call this a compiler bug. It IS passed uninitialized to this function and while the uninitialized value is not actually used, clang has no way of knowing that at this point in its pipeline, so I don't think warning in this case is unreasonable. This type of warning is off for GCC because of how unreliable it was when it is done in the middle end with optimizations. Furthermore, it is my understanding based on [1] that just the passing of an uninitialized variable in this manner is UB.
Ah, I see now what you are referring to, sorry. I'll go queue this up now, thanks.
greg k-h
On Tue, Jul 15, 2025 at 10:24:50PM -0700, Nathan Chancellor wrote:
On Wed, Jul 16, 2025 at 07:06:50AM +0200, Greg Kroah-Hartman wrote:
On Tue, Jul 15, 2025 at 01:33:32PM -0700, Nathan Chancellor wrote:
After a recent change in clang to expose uninitialized warnings from const variables [1], there is a warning in cxacru_heavy_init():
drivers/usb/atm/cxacru.c:1104:6: error: variable 'bp' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] 1104 | if (instance->modem_type->boot_rom_patch) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/usb/atm/cxacru.c:1113:39: note: uninitialized use occurs here 1113 | cxacru_upload_firmware(instance, fw, bp); | ^~ drivers/usb/atm/cxacru.c:1104:2: note: remove the 'if' if its condition is always true 1104 | if (instance->modem_type->boot_rom_patch) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/usb/atm/cxacru.c:1095:32: note: initialize the variable 'bp' to silence this warning 1095 | const struct firmware *fw, *bp; | ^ | = NULL
This warning occurs in clang's frontend before inlining occurs, so it cannot notice that bp is only used within cxacru_upload_firmware() under the same condition that initializes it in cxacru_heavy_init(). Just initialize bp to NULL to silence the warning without functionally changing the code, which is what happens with modern compilers when they support '-ftrivial-auto-var-init=zero' (CONFIG_INIT_STACK_ALL_ZERO=y).
We generally do not want to paper over compiler bugs, when our code is correct, so why should we do that here? Why not fix clang instead?
I would not really call this a compiler bug. It IS passed uninitialized to this function and while the uninitialized value is not actually used, clang has no way of knowing that at this point in its pipeline, so I don't think warning in this case is unreasonable.
No, I take it back, it is unreasonable :)
At runtime, there never is a uninitialized use of this pointer, the first time it is used, it is intended to be filled in if this is a "boot rom patch": ret = cxacru_find_firmware(instance, "bp", &bp);
Then if that call fails, the function exits, great.
Then later on, this is called: cxacru_upload_firmware(instance, fw, bp); so either bp IS valid, or it's still uninitialized, fair enough.
But then cxacru_upload_firmware() does the same check for "is this a boot rom patch" and only then does it reference the variable.
And when it references it, it does NOT check if it is valid or not, so even if you do pre-initialize this to NULL, surely some other static checker is going to come along and say "Hey, you just dereferenced a NULL pointer, this needs to be fixed!" when that too is just not true at all.
So the logic here is all "safe" for now, and if you set this to NULL, you are just papering over the fact that it is right, AND setting us up to get another patch that actually does nothing, while feeling like the submitter just fixed a security bug, demanding a CVE for an impossible code path :)
So let's leave this for now because:
This type of warning is off for GCC because of how unreliable it was when it is done in the middle end with optimizations. Furthermore, it is my understanding based on [1] that just the passing of an uninitialized variable in this manner is UB.
As gcc can't handle this either, it seems that clang also can't handle it. So turning this on for the kernel surely is going to trip it up in other places than just this one driver.
If you _really_ want to fix this, refactor the code to be more sane and obvious from a C parsing standpoint, but really, it isn't that complex for a human to read and understand, and I see why it was written this way.
As for the UB argument, bah, I don't care, sane compilers will do the right thing, i.e. pass in the uninitialized value, or if we turned on the 0-fill stack option, will be NULL anyway, otherwise why do we have that option if not to "solve" the UB issue?).
thanks,
greg k-h
On Wed, Jul 16, 2025 at 10:00:10AM +0200, Greg Kroah-Hartman wrote:
No, I take it back, it is unreasonable :)
At runtime, there never is a uninitialized use of this pointer, the first time it is used, it is intended to be filled in if this is a "boot rom patch": ret = cxacru_find_firmware(instance, "bp", &bp);
Then if that call fails, the function exits, great.
Then later on, this is called: cxacru_upload_firmware(instance, fw, bp); so either bp IS valid, or it's still uninitialized, fair enough.
But then cxacru_upload_firmware() does the same check for "is this a boot rom patch" and only then does it reference the variable.
Right but how would you know this if you were unable to look at what's inside cxacru_upload_firmware()? That's basically what is happening with clang, it is only able to look at cxacru_heavy_init().
And when it references it, it does NOT check if it is valid or not, so even if you do pre-initialize this to NULL, surely some other static checker is going to come along and say "Hey, you just dereferenced a NULL pointer, this needs to be fixed!" when that too is just not true at all.
If a static checker has the ability to see the NULL passed to cxacru_upload_firmware() from cxacru_heavy_init(), I would expect it to notice the identical conditions but point taken :)
So the logic here is all "safe" for now, and if you set this to NULL, you are just papering over the fact that it is right, AND setting us up to get another patch that actually does nothing, while feeling like the submitter just fixed a security bug, demanding a CVE for an impossible code path :)
Wouldn this be sufficient to avoid such a situation?
diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c index b7c3b224a759..fcff092fe826 100644 --- a/drivers/usb/atm/cxacru.c +++ b/drivers/usb/atm/cxacru.c @@ -1026,7 +1026,7 @@ static void cxacru_upload_firmware(struct cxacru_data *instance, }
/* Boot ROM patch */ - if (instance->modem_type->boot_rom_patch) { + if (instance->modem_type->boot_rom_patch && bp) { usb_info(usbatm, "loading boot ROM patch\n"); ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, BR_ADDR, bp->data, bp->size); if (ret) {
So let's leave this for now because:
This type of warning is off for GCC because of how unreliable it was when it is done in the middle end with optimizations. Furthermore, it is my understanding based on [1] that just the passing of an uninitialized variable in this manner is UB.
As gcc can't handle this either, it seems that clang also can't handle it. So turning this on for the kernel surely is going to trip it up in other places than just this one driver.
I turned this warning on in 5.3 in commit 3a61925e91ba ("kbuild: Enable -Wuninitialized"), so it is already enabled and it has found many, many legitimate instances in doing so, just go run 'git log --grep=Wuninitialized' or 'git log --grep=Wsometimes-uninitialized' in the kernel sources. While there have been other places in the kernel where this warning has been falsely triggered such as here, I have rarely received pushback from maintainers on fixes to silence them because the majority of them are legitimate (and the false positive fixes usually result in more robust code). For example, the strengthening of the warning in clang-21 resulted in what I would consider legitimate fixes:
https://lore.kernel.org/20250715-mt7996-fix-uninit-const-pointer-v1-1-b5d8d1... https://lore.kernel.org/20250715-net-phonet-fix-uninit-const-pointer-v1-1-8e... https://lore.kernel.org/20250715-drm-msm-fix-const-uninit-warning-v1-1-d6a36... https://lore.kernel.org/20250715-riscv-uaccess-fix-self-init-val-v1-1-82b8e9... https://lore.kernel.org/20250715-trace_probe-fix-const-uninit-warning-v1-1-9... https://lore.kernel.org/20250715-sdca_interrupts-fix-const-uninit-warning-v1...
If you _really_ want to fix this, refactor the code to be more sane and obvious from a C parsing standpoint, but really, it isn't that complex for a human to read and understand, and I see why it was written this way.
Yes, I agree that it is not complex or hard to understand, so I would rather not refactor it, but I do need this fixed so that allmodconfig builds (which enable -Werror by default) with clang-21 do not break. Won't Android eventually hit this when they get a newer compiler?
As for the UB argument, bah, I don't care, sane compilers will do the right thing, i.e. pass in the uninitialized value, or if we turned on the 0-fill stack option, will be NULL anyway, otherwise why do we have that option if not to "solve" the UB issue?).
As far as I understand it, clang adds "noundef" to function parameters when lowering to LLVM IR, which codifies that passing an uninitialized value is UB. I suspect that cxacru_upload_firmware() gets inlined so that ends up not mattering in this case but it could in others.
While '-ftrivial-auto-var-init=zero' does "solve" the UB issue, I see it more of a mitigation against missed initializations, not as a replacement for ensuring variables are consistently initialized, as zero might not be the expected initialization. Since that is the default for the kernel when compilers support it, why not just take this patch with that fixup above to make it consistent? I would be happy to send a v2 if you would be okay with it.
Cheers, Nathan
On Wed, Jul 16, 2025 at 08:43:04AM -0700, Nathan Chancellor wrote:
On Wed, Jul 16, 2025 at 10:00:10AM +0200, Greg Kroah-Hartman wrote:
No, I take it back, it is unreasonable :)
At runtime, there never is a uninitialized use of this pointer, the first time it is used, it is intended to be filled in if this is a "boot rom patch": ret = cxacru_find_firmware(instance, "bp", &bp);
Then if that call fails, the function exits, great.
Then later on, this is called: cxacru_upload_firmware(instance, fw, bp); so either bp IS valid, or it's still uninitialized, fair enough.
But then cxacru_upload_firmware() does the same check for "is this a boot rom patch" and only then does it reference the variable.
Right but how would you know this if you were unable to look at what's inside cxacru_upload_firmware()? That's basically what is happening with clang, it is only able to look at cxacru_heavy_init().
True, and it's also unable to look into cxacru_upload_firmware() :)
And when it references it, it does NOT check if it is valid or not, so even if you do pre-initialize this to NULL, surely some other static checker is going to come along and say "Hey, you just dereferenced a NULL pointer, this needs to be fixed!" when that too is just not true at all.
If a static checker has the ability to see the NULL passed to cxacru_upload_firmware() from cxacru_heavy_init(), I would expect it to notice the identical conditions but point taken :)
So the logic here is all "safe" for now, and if you set this to NULL, you are just papering over the fact that it is right, AND setting us up to get another patch that actually does nothing, while feeling like the submitter just fixed a security bug, demanding a CVE for an impossible code path :)
Wouldn this be sufficient to avoid such a situation?
diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c index b7c3b224a759..fcff092fe826 100644 --- a/drivers/usb/atm/cxacru.c +++ b/drivers/usb/atm/cxacru.c @@ -1026,7 +1026,7 @@ static void cxacru_upload_firmware(struct cxacru_data *instance, } /* Boot ROM patch */
- if (instance->modem_type->boot_rom_patch) {
- if (instance->modem_type->boot_rom_patch && bp) { usb_info(usbatm, "loading boot ROM patch\n"); ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, BR_ADDR, bp->data, bp->size); if (ret) {
That's what a follow-on patch would generate thinking they were actually fixing a bug, but again, that's a pointless check!
So let's leave this for now because:
This type of warning is off for GCC because of how unreliable it was when it is done in the middle end with optimizations. Furthermore, it is my understanding based on [1] that just the passing of an uninitialized variable in this manner is UB.
As gcc can't handle this either, it seems that clang also can't handle it. So turning this on for the kernel surely is going to trip it up in other places than just this one driver.
I turned this warning on in 5.3 in commit 3a61925e91ba ("kbuild: Enable -Wuninitialized"), so it is already enabled and it has found many, many legitimate instances in doing so, just go run 'git log --grep=Wuninitialized' or 'git log --grep=Wsometimes-uninitialized' in the kernel sources. While there have been other places in the kernel where this warning has been falsely triggered such as here, I have rarely received pushback from maintainers on fixes to silence them because the majority of them are legitimate (and the false positive fixes usually result in more robust code). For example, the strengthening of the warning in clang-21 resulted in what I would consider legitimate fixes:
https://lore.kernel.org/20250715-mt7996-fix-uninit-const-pointer-v1-1-b5d8d1... https://lore.kernel.org/20250715-net-phonet-fix-uninit-const-pointer-v1-1-8e... https://lore.kernel.org/20250715-drm-msm-fix-const-uninit-warning-v1-1-d6a36... https://lore.kernel.org/20250715-riscv-uaccess-fix-self-init-val-v1-1-82b8e9... https://lore.kernel.org/20250715-trace_probe-fix-const-uninit-warning-v1-1-9... https://lore.kernel.org/20250715-sdca_interrupts-fix-const-uninit-warning-v1...
If you _really_ want to fix this, refactor the code to be more sane and obvious from a C parsing standpoint, but really, it isn't that complex for a human to read and understand, and I see why it was written this way.
Yes, I agree that it is not complex or hard to understand, so I would rather not refactor it, but I do need this fixed so that allmodconfig builds (which enable -Werror by default) with clang-21 do not break. Won't Android eventually hit this when they get a newer compiler?
I have no idea what Android uses for their compiler. Usually when they run into issues like this, for their 'allmodconfig' builds, they just apply a "CONFIG_BROKEN" patch to disable the old/unneeded driver entirely.
As for the UB argument, bah, I don't care, sane compilers will do the right thing, i.e. pass in the uninitialized value, or if we turned on the 0-fill stack option, will be NULL anyway, otherwise why do we have that option if not to "solve" the UB issue?).
As far as I understand it, clang adds "noundef" to function parameters when lowering to LLVM IR, which codifies that passing an uninitialized value is UB. I suspect that cxacru_upload_firmware() gets inlined so that ends up not mattering in this case but it could in others.
While '-ftrivial-auto-var-init=zero' does "solve" the UB issue, I see it more of a mitigation against missed initializations, not as a replacement for ensuring variables are consistently initialized, as zero might not be the expected initialization. Since that is the default for the kernel when compilers support it, why not just take this patch with that fixup above to make it consistent? I would be happy to send a v2 if you would be okay with it.
I'm really loath to take it, sorry. I'd prefer that if the compiler can't figure it out, we should rewrite it to make it more "obvious" as to what is going on here so that both people, and the compiler, can understand it easier.
Just setting the variable to NULL does neither of those things, except to shut up a false-positive, not making it more obvious to the compiler as to what really is going on.
thanks,
greg k-h
On Wed, Jul 16, 2025 at 06:08:47PM +0200, Greg Kroah-Hartman wrote:
I have no idea what Android uses for their compiler. Usually when they run into issues like this, for their 'allmodconfig' builds, they just apply a "CONFIG_BROKEN" patch to disable the old/unneeded driver entirely.
Well I can say for certain that they do use clang but if that's their preferred solution to situations such as this, so be it.
I'm really loath to take it, sorry. I'd prefer that if the compiler can't figure it out, we should rewrite it to make it more "obvious" as to what is going on here so that both people, and the compiler, can understand it easier.
Just setting the variable to NULL does neither of those things, except to shut up a false-positive, not making it more obvious to the compiler as to what really is going on.
Alright. We could just manually inline cxacru_upload_firmware() into cxacru_heavy_init() since that is the only place where it is called so that clang can see via the control flow graph that bp is only initialized and used under the same condition. This resolves the warning for me as well.
diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c index a12ab90b3db7..68a8e9de8b4f 100644 --- a/drivers/usb/atm/cxacru.c +++ b/drivers/usb/atm/cxacru.c @@ -980,25 +980,60 @@ static int cxacru_fw(struct usb_device *usb_dev, enum cxacru_fw_request fw, return ret; }
-static void cxacru_upload_firmware(struct cxacru_data *instance, - const struct firmware *fw, - const struct firmware *bp) + +static int cxacru_find_firmware(struct cxacru_data *instance, + char *phase, const struct firmware **fw_p) { - int ret; + struct usbatm_data *usbatm = instance->usbatm; + struct device *dev = &usbatm->usb_intf->dev; + char buf[16]; + + sprintf(buf, "cxacru-%s.bin", phase); + usb_dbg(usbatm, "cxacru_find_firmware: looking for %s\n", buf); + + if (request_firmware(fw_p, buf, dev)) { + usb_dbg(usbatm, "no stage %s firmware found\n", phase); + return -ENOENT; + } + + usb_info(usbatm, "found firmware %s\n", buf); + + return 0; +} + +static int cxacru_heavy_init(struct usbatm_data *usbatm_instance, + struct usb_interface *usb_intf) +{ + const struct firmware *fw, *bp; + struct cxacru_data *instance = usbatm_instance->driver_data; struct usbatm_data *usbatm = instance->usbatm; struct usb_device *usb_dev = usbatm->usb_dev; __le16 signature[] = { usb_dev->descriptor.idVendor, usb_dev->descriptor.idProduct }; __le32 val; + int ret;
- usb_dbg(usbatm, "%s\n", __func__); + ret = cxacru_find_firmware(instance, "fw", &fw); + if (ret) { + usb_warn(usbatm_instance, "firmware (cxacru-fw.bin) unavailable (system misconfigured?)\n"); + return ret; + } + + if (instance->modem_type->boot_rom_patch) { + ret = cxacru_find_firmware(instance, "bp", &bp); + if (ret) { + usb_warn(usbatm_instance, "boot ROM patch (cxacru-bp.bin) unavailable (system misconfigured?)\n"); + release_firmware(fw); + return ret; + } + }
/* FirmwarePllFClkValue */ val = cpu_to_le32(instance->modem_type->pll_f_clk); ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, PLLFCLK_ADDR, (u8 *) &val, 4); if (ret) { usb_err(usbatm, "FirmwarePllFClkValue failed: %d\n", ret); - return; + goto done; }
/* FirmwarePllBClkValue */ @@ -1006,7 +1041,7 @@ static void cxacru_upload_firmware(struct cxacru_data *instance, ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, PLLBCLK_ADDR, (u8 *) &val, 4); if (ret) { usb_err(usbatm, "FirmwarePllBClkValue failed: %d\n", ret); - return; + goto done; }
/* Enable SDRAM */ @@ -1014,7 +1049,7 @@ static void cxacru_upload_firmware(struct cxacru_data *instance, ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, SDRAMEN_ADDR, (u8 *) &val, 4); if (ret) { usb_err(usbatm, "Enable SDRAM failed: %d\n", ret); - return; + goto done; }
/* Firmware */ @@ -1022,7 +1057,7 @@ static void cxacru_upload_firmware(struct cxacru_data *instance, ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, FW_ADDR, fw->data, fw->size); if (ret) { usb_err(usbatm, "Firmware upload failed: %d\n", ret); - return; + goto done; }
/* Boot ROM patch */ @@ -1031,7 +1066,7 @@ static void cxacru_upload_firmware(struct cxacru_data *instance, ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, BR_ADDR, bp->data, bp->size); if (ret) { usb_err(usbatm, "Boot ROM patching failed: %d\n", ret); - return; + goto done; } }
@@ -1039,7 +1074,7 @@ static void cxacru_upload_firmware(struct cxacru_data *instance, ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, SIG_ADDR, (u8 *) signature, 4); if (ret) { usb_err(usbatm, "Signature storing failed: %d\n", ret); - return; + goto done; }
usb_info(usbatm, "starting device\n"); @@ -1051,7 +1086,7 @@ static void cxacru_upload_firmware(struct cxacru_data *instance, } if (ret) { usb_err(usbatm, "Passing control to firmware failed: %d\n", ret); - return; + goto done; }
/* Delay to allow firmware to start up. */ @@ -1065,53 +1100,10 @@ static void cxacru_upload_firmware(struct cxacru_data *instance, ret = cxacru_cm(instance, CM_REQUEST_CARD_GET_STATUS, NULL, 0, NULL, 0); if (ret < 0) { usb_err(usbatm, "modem failed to initialize: %d\n", ret); - return; - } -} - -static int cxacru_find_firmware(struct cxacru_data *instance, - char *phase, const struct firmware **fw_p) -{ - struct usbatm_data *usbatm = instance->usbatm; - struct device *dev = &usbatm->usb_intf->dev; - char buf[16]; - - sprintf(buf, "cxacru-%s.bin", phase); - usb_dbg(usbatm, "cxacru_find_firmware: looking for %s\n", buf); - - if (request_firmware(fw_p, buf, dev)) { - usb_dbg(usbatm, "no stage %s firmware found\n", phase); - return -ENOENT; - } - - usb_info(usbatm, "found firmware %s\n", buf); - - return 0; -} - -static int cxacru_heavy_init(struct usbatm_data *usbatm_instance, - struct usb_interface *usb_intf) -{ - const struct firmware *fw, *bp; - struct cxacru_data *instance = usbatm_instance->driver_data; - int ret = cxacru_find_firmware(instance, "fw", &fw); - - if (ret) { - usb_warn(usbatm_instance, "firmware (cxacru-fw.bin) unavailable (system misconfigured?)\n"); - return ret; + goto done; }
- if (instance->modem_type->boot_rom_patch) { - ret = cxacru_find_firmware(instance, "bp", &bp); - if (ret) { - usb_warn(usbatm_instance, "boot ROM patch (cxacru-bp.bin) unavailable (system misconfigured?)\n"); - release_firmware(fw); - return ret; - } - } - - cxacru_upload_firmware(instance, fw, bp); - +done: if (instance->modem_type->boot_rom_patch) release_firmware(bp); release_firmware(fw);
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Nathan Chancellor