| Message ID | 20190427183008.27111-1-zajec5@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | brcmfmac: print firmware messages after a firmware crash | expand |
On 4/27/2019 8:30 PM, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > Normally firmware messages are printed with debugging enabled only. It's > a good idea as firmware may print a lot of messages that normal users > don't need to care about. > > However, on firmware crash, it may be very helpful to log all recent > messages. There is almost always a backtrace available as well as rought > info on the latest actions/state. nice... there is one minor nit below, but other than that... Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > --- > .../broadcom/brcm80211/brcmfmac/pcie.c | 24 ++++++++++++++----- > 1 file changed, 18 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c > index 637973fe8928..f519b050aff3 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c > @@ -764,15 +764,22 @@ static void brcmf_pcie_bus_console_init(struct brcmf_pciedev_info *devinfo) > console->base_addr, console->buf_addr, console->bufsize); > } > > - > -static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo) > +/** > + * brcmf_pcie_bus_console_read - reads firmware messages > + * > + * @error: specifies if error has occurred (prints messages unconditionally) > + */ > +static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo, > + int error) Given how it is called I would say 'bool error' makes a bit more sense. Gr. AvS
On 2019-04-28 23:06, Arend Van Spriel wrote: > On 4/27/2019 8:30 PM, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@milecki.pl> >> >> Normally firmware messages are printed with debugging enabled only. >> It's >> a good idea as firmware may print a lot of messages that normal users >> don't need to care about. >> >> However, on firmware crash, it may be very helpful to log all recent >> messages. There is almost always a backtrace available as well as >> rought >> info on the latest actions/state. > > nice... there is one minor nit below, but other than that... > > Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >> --- >> .../broadcom/brcm80211/brcmfmac/pcie.c | 24 >> ++++++++++++++----- >> 1 file changed, 18 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c >> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c >> index 637973fe8928..f519b050aff3 100644 >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c >> @@ -764,15 +764,22 @@ static void brcmf_pcie_bus_console_init(struct >> brcmf_pciedev_info *devinfo) >> console->base_addr, console->buf_addr, console->bufsize); >> } >> - >> -static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info >> *devinfo) >> +/** >> + * brcmf_pcie_bus_console_read - reads firmware messages >> + * >> + * @error: specifies if error has occurred (prints messages >> unconditionally) >> + */ >> +static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info >> *devinfo, >> + int error) > > Given how it is called I would say 'bool error' makes a bit more sense. I even call that function passing "true" or "false" as argument. Nice catch!
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 637973fe8928..f519b050aff3 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -764,15 +764,22 @@ static void brcmf_pcie_bus_console_init(struct brcmf_pciedev_info *devinfo) console->base_addr, console->buf_addr, console->bufsize); } - -static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo) +/** + * brcmf_pcie_bus_console_read - reads firmware messages + * + * @error: specifies if error has occurred (prints messages unconditionally) + */ +static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo, + int error) { + struct pci_dev *pdev = devinfo->pdev; + struct brcmf_bus *bus = dev_get_drvdata(&pdev->dev); struct brcmf_pcie_console *console; u32 addr; u8 ch; u32 newidx; - if (!BRCMF_FWCON_ON()) + if (!error && !BRCMF_FWCON_ON()) return; console = &devinfo->shared.console; @@ -796,7 +803,10 @@ static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo) } if (ch == '\n') { console->log_str[console->log_idx] = 0; - pr_debug("CONSOLE: %s", console->log_str); + if (error) + brcmf_err(bus, "CONSOLE: %s", console->log_str); + else + pr_debug("CONSOLE: %s", console->log_str); console->log_idx = 0; } } @@ -857,7 +867,7 @@ static irqreturn_t brcmf_pcie_isr_thread(int irq, void *arg) &devinfo->pdev->dev); } } - brcmf_pcie_bus_console_read(devinfo); + brcmf_pcie_bus_console_read(devinfo, false); if (devinfo->state == BRCMFMAC_PCIE_STATE_UP) brcmf_pcie_intr_enable(devinfo); devinfo->in_irq = false; @@ -1426,6 +1436,8 @@ static int brcmf_pcie_reset(struct device *dev) struct brcmf_fw_request *fwreq; int err; + brcmf_pcie_bus_console_read(devinfo, true); + brcmf_detach(dev); brcmf_pcie_release_irq(devinfo); @@ -1824,7 +1836,7 @@ static void brcmf_pcie_setup(struct device *dev, int ret, if (brcmf_attach(&devinfo->pdev->dev, devinfo->settings) == 0) return; - brcmf_pcie_bus_console_read(devinfo); + brcmf_pcie_bus_console_read(devinfo, false); fail: device_release_driver(dev);