| Message ID | 1383133346-8135-13-git-send-email-michal.kazior@tieto.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, 2013-10-30 at 12:42 +0100, Michal Kazior wrote: > Some errors were handled too silently. These aren't really debug prints. > diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c [] > @@ -1860,8 +1862,10 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar) > ath10k_do_pci_wake(ar); > > ret = ath10k_pci_ce_init(ar); > - if (ret) > + if (ret) { > + ath10k_err("could not initialize CE (%d)\n", ret); Rather than try to reinterpret the function name, perhaps it's better to simply emit the function name as is done most other places like: ath10k_err("ath10k_pci_ce_init failed: (%d)\n", ret); > @@ -1876,16 +1880,22 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar) > } > > ret = ath10k_pci_wait_for_target_init(ar); > - if (ret) > + if (ret) { > + ath10k_err("failed to wait for target to init (%d)\n", ret); > goto err_irq; > + } Like this one, because the function did wait, but the init was unsuccessful. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior <michal.kazior@tieto.com> writes: > Some errors were handled too silently. Also add a > print indicating BMI is booted. > > Signed-off-by: Michal Kazior <michal.kazior@tieto.com> [...] > static int ath10k_pci_hif_power_up(struct ath10k *ar) > @@ -1860,8 +1862,10 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar) > ath10k_do_pci_wake(ar); > > ret = ath10k_pci_ce_init(ar); > - if (ret) > + if (ret) { > + ath10k_err("could not initialize CE (%d)\n", ret); "could not initialize CE: %d\n" That comment applies to all the error codes printed.
Joe Perches <joe@perches.com> writes: > On Wed, 2013-10-30 at 12:42 +0100, Michal Kazior wrote: >> Some errors were handled too silently. > > These aren't really debug prints. Yeah, the patch title needs work. >> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c > [] >> @@ -1860,8 +1862,10 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar) >> ath10k_do_pci_wake(ar); >> >> ret = ath10k_pci_ce_init(ar); >> - if (ret) >> + if (ret) { >> + ath10k_err("could not initialize CE (%d)\n", ret); > > Rather than try to reinterpret the function name, > perhaps it's better to simply emit the function name > as is done most other places like: > > ath10k_err("ath10k_pci_ce_init failed: (%d)\n", ret); I don't think that's any better. When function names change but people forget to change the error message and then it's even more confusing. And I prefer that a developer puts a bit more effort to the warning message by writing it in english instead of just copying the function name.
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c index 7b606d0..42dd0b7 100644 --- a/drivers/net/wireless/ath/ath10k/pci.c +++ b/drivers/net/wireless/ath/ath10k/pci.c @@ -1832,6 +1832,8 @@ static void ath10k_pci_start_bmi(struct ath10k *ar) pipe = &ar_pci->pipe_info[BMI_CE_NUM_TO_HOST]; ath10k_ce_recv_cb_register(pipe->ce_hdl, ath10k_pci_bmi_recv_data); + + ath10k_dbg(ATH10K_DBG_BOOT, "boot start bmi\n"); } static int ath10k_pci_hif_power_up(struct ath10k *ar) @@ -1860,8 +1862,10 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar) ath10k_do_pci_wake(ar); ret = ath10k_pci_ce_init(ar); - if (ret) + if (ret) { + ath10k_err("could not initialize CE (%d)\n", ret); goto err_ps; + } ret = ath10k_ce_disable_interrupts(ar); if (ret) { @@ -1876,16 +1880,22 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar) } ret = ath10k_pci_wait_for_target_init(ar); - if (ret) + if (ret) { + ath10k_err("failed to wait for target to init (%d)\n", ret); goto err_irq; + } ret = ath10k_ce_enable_err_irq(ar); - if (ret) + if (ret) { + ath10k_err("failed to enable CE error irq (%d)\n", ret); goto err_irq; + } ret = ath10k_pci_init_config(ar); - if (ret) + if (ret) { + ath10k_err("failed to setup init config (%d)\n", ret); goto err_irq; + } ret = ath10k_pci_wake_target_cpu(ar); if (ret) {
Some errors were handled too silently. Also add a print indicating BMI is booted. Signed-off-by: Michal Kazior <michal.kazior@tieto.com> --- drivers/net/wireless/ath/ath10k/pci.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)