diff mbox

[V2,3/6] mwifiex: support sysfs initiated device coredump

Message ID 1526472723-15758-4-git-send-email-arend.vanspriel@broadcom.com (mailing list archive)
State Accepted
Commit 21c5c83ce83359f0ab930824c67984575c051550
Delegated to: Kalle Valo
Headers show

Commit Message

Arend van Spriel May 16, 2018, 12:12 p.m. UTC
Since commit 3c47d19ff4dc ("drivers: base: add coredump driver ops")
it is possible to initiate a device coredump from user-space. This
patch adds support for it adding the .coredump() driver callback.
As there is no longer a need to initiate it through debugfs remove
that code.

Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 V2:
  - export mwifiex_send_cmd() needed by mwifiex_usb.ko.
---
 drivers/net/wireless/marvell/mwifiex/cmdevt.c  |  1 +
 drivers/net/wireless/marvell/mwifiex/debugfs.c | 31 +-------------------------
 drivers/net/wireless/marvell/mwifiex/pcie.c    | 18 +++++++++++++--
 drivers/net/wireless/marvell/mwifiex/sdio.c    | 12 ++++++++++
 drivers/net/wireless/marvell/mwifiex/usb.c     | 13 +++++++++++
 5 files changed, 43 insertions(+), 32 deletions(-)

Comments

Kalle Valo May 16, 2018, 1:50 p.m. UTC | #1
Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> Since commit 3c47d19ff4dc ("drivers: base: add coredump driver ops")
> it is possible to initiate a device coredump from user-space. This
> patch adds support for it adding the .coredump() driver callback.
> As there is no longer a need to initiate it through debugfs remove
> that code.
>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> ---
>  V2:
>   - export mwifiex_send_cmd() needed by mwifiex_usb.ko.
> ---
>  drivers/net/wireless/marvell/mwifiex/cmdevt.c  |  1 +
>  drivers/net/wireless/marvell/mwifiex/debugfs.c | 31 +-------------------------
>  drivers/net/wireless/marvell/mwifiex/pcie.c    | 18 +++++++++++++--
>  drivers/net/wireless/marvell/mwifiex/sdio.c    | 12 ++++++++++
>  drivers/net/wireless/marvell/mwifiex/usb.c     | 13 +++++++++++
>  5 files changed, 43 insertions(+), 32 deletions(-)

You forgot to CC mwifiex maintainers, doing it now. Full patch here:

https://patchwork.kernel.org/patch/10403717/
Arend van Spriel May 16, 2018, 6:52 p.m. UTC | #2
On 5/16/2018 3:50 PM, Kalle Valo wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> Since commit 3c47d19ff4dc ("drivers: base: add coredump driver ops")
>> it is possible to initiate a device coredump from user-space. This
>> patch adds support for it adding the .coredump() driver callback.
>> As there is no longer a need to initiate it through debugfs remove
>> that code.
>>
>> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>> ---
>>   V2:
>>    - export mwifiex_send_cmd() needed by mwifiex_usb.ko.
>> ---
>>   drivers/net/wireless/marvell/mwifiex/cmdevt.c  |  1 +
>>   drivers/net/wireless/marvell/mwifiex/debugfs.c | 31 +-------------------------
>>   drivers/net/wireless/marvell/mwifiex/pcie.c    | 18 +++++++++++++--
>>   drivers/net/wireless/marvell/mwifiex/sdio.c    | 12 ++++++++++
>>   drivers/net/wireless/marvell/mwifiex/usb.c     | 13 +++++++++++
>>   5 files changed, 43 insertions(+), 32 deletions(-)
>
> You forgot to CC mwifiex maintainers, doing it now. Full patch here:
>
> https://patchwork.kernel.org/patch/10403717/

Thanks, Kalle

You would almost think I am not used to submit patches for drivers I do 
not actively maintain ;-)

Gr. AvS
Ganapathi Bhat May 21, 2018, 8:19 a.m. UTC | #3
Hi arend.vanspriel,
>
> ----------------------------------------------------------------------
> On 5/16/2018 3:50 PM, Kalle Valo wrote:
> > Arend van Spriel <arend.vanspriel@broadcom.com> writes:
> >
> >> Since commit 3c47d19ff4dc ("drivers: base: add coredump driver ops")
> >> it is possible to initiate a device coredump from user-space. This
> >> patch adds support for it adding the .coredump() driver callback.
> >> As there is no longer a need to initiate it through debugfs remove
> >> that code.
> >>
> >> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> >> ---
> >>   V2:
> >>    - export mwifiex_send_cmd() needed by mwifiex_usb.ko.
> >> ---
> >>   drivers/net/wireless/marvell/mwifiex/cmdevt.c  |  1 +
> >>   drivers/net/wireless/marvell/mwifiex/debugfs.c | 31 +----------------------
> ---
> >>   drivers/net/wireless/marvell/mwifiex/pcie.c    | 18 +++++++++++++--
> >>   drivers/net/wireless/marvell/mwifiex/sdio.c    | 12 ++++++++++
> >>   drivers/net/wireless/marvell/mwifiex/usb.c     | 13 +++++++++++
> >>   5 files changed, 43 insertions(+), 32 deletions(-)
> >
> > You forgot to CC mwifiex maintainers, doing it now. Full patch here:
> >
> > https://patchwork.kernel.org/patch/10403717/
>
> Thanks, Kalle
>
> You would almost think I am not used to submit patches for drivers I do not
> actively maintain ;-)
>
> Gr. AvS
In V2:

        return ret;
 }
+EXPORT_SYMBOL_GPL(mwifiex_send_cmd);

Instead of exporting 'mwifiex_send_cmd'  we can prepare a wrapper for the command 'MWIFIEX_IFACE_WORK_DEVICE_DUMP' in mwifiex module. Let me know if we can send a follow up.

Thanks,
Ganapathi
Kalle Valo May 23, 2018, 7:54 a.m. UTC | #4
Ganapathi Bhat <gbhat@marvell.com> writes:

> In V2:
>
>         return ret;
>  }
> +EXPORT_SYMBOL_GPL(mwifiex_send_cmd);
>
> Instead of exporting 'mwifiex_send_cmd' we can prepare a wrapper for
> the command 'MWIFIEX_IFACE_WORK_DEVICE_DUMP' in mwifiex module. Let me
> know if we can send a follow up.

So what's the plan? Can I apply this patch or should I drop it?
Arend van Spriel May 23, 2018, 8:11 a.m. UTC | #5
On 5/23/2018 9:54 AM, Kalle Valo wrote:
> Ganapathi Bhat <gbhat@marvell.com> writes:
>
>> In V2:
>>
>>          return ret;
>>   }
>> +EXPORT_SYMBOL_GPL(mwifiex_send_cmd);
>>
>> Instead of exporting 'mwifiex_send_cmd' we can prepare a wrapper for
>> the command 'MWIFIEX_IFACE_WORK_DEVICE_DUMP' in mwifiex module. Let me
>> know if we can send a follow up.
>
> So what's the plan? Can I apply this patch or should I drop it?

Sorry. Had to reply that earlier. When adding the export I figured it 
might not be desirable. If Ganapathi can fix it with a follow up patch 
that would ok, right? Than you can apply it, I would say.

Regards,
Arend
Kalle Valo May 23, 2018, 8:17 a.m. UTC | #6
Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 5/23/2018 9:54 AM, Kalle Valo wrote:
>> Ganapathi Bhat <gbhat@marvell.com> writes:
>>
>>> In V2:
>>>
>>>          return ret;
>>>   }
>>> +EXPORT_SYMBOL_GPL(mwifiex_send_cmd);
>>>
>>> Instead of exporting 'mwifiex_send_cmd' we can prepare a wrapper for
>>> the command 'MWIFIEX_IFACE_WORK_DEVICE_DUMP' in mwifiex module. Let me
>>> know if we can send a follow up.
>>
>> So what's the plan? Can I apply this patch or should I drop it?
>
> Sorry. Had to reply that earlier. When adding the export I figured it
> might not be desirable. If Ganapathi can fix it with a follow up patch
> that would ok, right?

A follow up patch sounds good to me.

> Than you can apply it, I would say.

Good, thanks.
Ganapathi Bhat May 23, 2018, 11:24 a.m. UTC | #7
Hi Kalle,

> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>
> > On 5/23/2018 9:54 AM, Kalle Valo wrote:
> >> Ganapathi Bhat <gbhat@marvell.com> writes:
> >>
> >>> In V2:
> >>>
> >>>          return ret;
> >>>   }
> >>> +EXPORT_SYMBOL_GPL(mwifiex_send_cmd);
> >>>
> >>> Instead of exporting 'mwifiex_send_cmd' we can prepare a wrapper for
> >>> the command 'MWIFIEX_IFACE_WORK_DEVICE_DUMP' in mwifiex
> module. Let
> >>> me know if we can send a follow up.
> >>
> >> So what's the plan? Can I apply this patch or should I drop it?
> >
> > Sorry. Had to reply that earlier. When adding the export I figured it
> > might not be desirable. If Ganapathi can fix it with a follow up patch
> > that would ok, right?
>
> A follow up patch sounds good to me.
Sorry for confusions. I assume you will be margining the original patch and want me to send the follow-up right?
>
Regards,
Ganapathi
Kalle Valo May 23, 2018, 3:56 p.m. UTC | #8
Ganapathi Bhat <gbhat@marvell.com> writes:

> Hi Kalle,
>
>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>> > On 5/23/2018 9:54 AM, Kalle Valo wrote:
>> >> Ganapathi Bhat <gbhat@marvell.com> writes:
>> >>
>> >>> In V2:
>> >>>
>> >>>          return ret;
>> >>>   }
>> >>> +EXPORT_SYMBOL_GPL(mwifiex_send_cmd);
>> >>>
>> >>> Instead of exporting 'mwifiex_send_cmd' we can prepare a wrapper for
>> >>> the command 'MWIFIEX_IFACE_WORK_DEVICE_DUMP' in mwifiex
>> module. Let
>> >>> me know if we can send a follow up.
>> >>
>> >> So what's the plan? Can I apply this patch or should I drop it?
>> >
>> > Sorry. Had to reply that earlier. When adding the export I figured it
>> > might not be desirable. If Ganapathi can fix it with a follow up patch
>> > that would ok, right?
>>
>> A follow up patch sounds good to me.
>
> Sorry for confusions. I assume you will be margining the original
> patch and want me to send the follow-up right?

Correct. I applied Arend's patch now so you can send your follow-up
patch.
diff mbox

Patch

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 9cfcdf6..8be1e69 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -674,6 +674,7 @@  int mwifiex_send_cmd(struct mwifiex_private *priv, u16 cmd_no,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(mwifiex_send_cmd);
 
 /*
  * This function queues a command to the command pending queue.
diff --git a/drivers/net/wireless/marvell/mwifiex/debugfs.c b/drivers/net/wireless/marvell/mwifiex/debugfs.c
index db2872d..0745393 100644
--- a/drivers/net/wireless/marvell/mwifiex/debugfs.c
+++ b/drivers/net/wireless/marvell/mwifiex/debugfs.c
@@ -154,34 +154,6 @@ 
 }
 
 /*
- * Proc device dump read handler.
- *
- * This function is called when the 'device_dump' file is opened for
- * reading.
- * This function dumps driver information and firmware memory segments
- * (ex. DTCM, ITCM, SQRAM etc.) for
- * debugging.
- */
-static ssize_t
-mwifiex_device_dump_read(struct file *file, char __user *ubuf,
-			 size_t count, loff_t *ppos)
-{
-	struct mwifiex_private *priv = file->private_data;
-
-	/* For command timeouts, USB firmware will automatically emit
-	 * firmware dump events, so we don't implement device_dump().
-	 * For user-initiated dumps, we trigger it ourselves.
-	 */
-	if (priv->adapter->iface_type == MWIFIEX_USB)
-		mwifiex_send_cmd(priv, HostCmd_CMD_FW_DUMP_EVENT,
-				 HostCmd_ACT_GEN_SET, 0, NULL, true);
-	else
-		priv->adapter->if_ops.device_dump(priv->adapter);
-
-	return 0;
-}
-
-/*
  * Proc getlog file read handler.
  *
  * This function is called when the 'getlog' file is opened for reading
@@ -980,7 +952,6 @@ 
 MWIFIEX_DFS_FILE_READ_OPS(info);
 MWIFIEX_DFS_FILE_READ_OPS(debug);
 MWIFIEX_DFS_FILE_READ_OPS(getlog);
-MWIFIEX_DFS_FILE_READ_OPS(device_dump);
 MWIFIEX_DFS_FILE_OPS(regrdwr);
 MWIFIEX_DFS_FILE_OPS(rdeeprom);
 MWIFIEX_DFS_FILE_OPS(memrw);
@@ -1011,7 +982,7 @@ 
 	MWIFIEX_DFS_ADD_FILE(getlog);
 	MWIFIEX_DFS_ADD_FILE(regrdwr);
 	MWIFIEX_DFS_ADD_FILE(rdeeprom);
-	MWIFIEX_DFS_ADD_FILE(device_dump);
+
 	MWIFIEX_DFS_ADD_FILE(memrw);
 	MWIFIEX_DFS_ADD_FILE(hscfg);
 	MWIFIEX_DFS_ADD_FILE(histogram);
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 7538543..0c42b72 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -320,6 +320,19 @@  static void mwifiex_pcie_shutdown(struct pci_dev *pdev)
 	return;
 }
 
+static void mwifiex_pcie_coredump(struct device *dev)
+{
+	struct pci_dev *pdev;
+	struct pcie_service_card *card;
+
+	pdev = container_of(dev, struct pci_dev, dev);
+	card = pci_get_drvdata(pdev);
+
+	if (!test_and_set_bit(MWIFIEX_IFACE_WORK_DEVICE_DUMP,
+			      &card->work_flags))
+		schedule_work(&card->work);
+}
+
 static const struct pci_device_id mwifiex_ids[] = {
 	{
 		PCIE_VENDOR_ID_MARVELL, PCIE_DEVICE_ID_MARVELL_88W8766P,
@@ -415,11 +428,12 @@  static SIMPLE_DEV_PM_OPS(mwifiex_pcie_pm_ops, mwifiex_pcie_suspend,
 	.id_table = mwifiex_ids,
 	.probe    = mwifiex_pcie_probe,
 	.remove   = mwifiex_pcie_remove,
-#ifdef CONFIG_PM_SLEEP
 	.driver   = {
+		.coredump = mwifiex_pcie_coredump,
+#ifdef CONFIG_PM_SLEEP
 		.pm = &mwifiex_pcie_pm_ops,
-	},
 #endif
+	},
 	.shutdown = mwifiex_pcie_shutdown,
 	.err_handler = &mwifiex_pcie_err_handler,
 };
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index a828801..47d2dcc 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -466,6 +466,17 @@  static int mwifiex_sdio_suspend(struct device *dev)
 	return ret;
 }
 
+static void mwifiex_sdio_coredump(struct device *dev)
+{
+	struct sdio_func *func = dev_to_sdio_func(dev);
+	struct sdio_mmc_card *card;
+
+	card = sdio_get_drvdata(func);
+	if (!test_and_set_bit(MWIFIEX_IFACE_WORK_DEVICE_DUMP,
+			      &card->work_flags))
+		schedule_work(&card->work);
+}
+
 /* Device ID for SD8786 */
 #define SDIO_DEVICE_ID_MARVELL_8786   (0x9116)
 /* Device ID for SD8787 */
@@ -515,6 +526,7 @@  static int mwifiex_sdio_suspend(struct device *dev)
 	.remove = mwifiex_sdio_remove,
 	.drv = {
 		.owner = THIS_MODULE,
+		.coredump = mwifiex_sdio_coredump,
 		.pm = &mwifiex_sdio_pm_ops,
 	}
 };
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 4bc2448..7aa39878 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -653,6 +653,16 @@  static void mwifiex_usb_disconnect(struct usb_interface *intf)
 	usb_put_dev(interface_to_usbdev(intf));
 }
 
+static void mwifiex_usb_coredump(struct device *dev)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usb_card_rec *card = usb_get_intfdata(intf);
+
+	mwifiex_send_cmd(mwifiex_get_priv(card->adapter, MWIFIEX_BSS_ROLE_ANY),
+			 HostCmd_CMD_FW_DUMP_EVENT, HostCmd_ACT_GEN_SET, 0,
+			 NULL, true);
+}
+
 static struct usb_driver mwifiex_usb_driver = {
 	.name = "mwifiex_usb",
 	.probe = mwifiex_usb_probe,
@@ -661,6 +671,9 @@  static void mwifiex_usb_disconnect(struct usb_interface *intf)
 	.suspend = mwifiex_usb_suspend,
 	.resume = mwifiex_usb_resume,
 	.soft_unbind = 1,
+	.drvwrap.driver = {
+		.coredump = mwifiex_usb_coredump,
+	},
 };
 
 static int mwifiex_write_data_sync(struct mwifiex_adapter *adapter, u8 *pbuf,