On 8/12/2022 9:09 AM, Kalle Valo wrote:
From: Kalle Valo <quic_kvalo@xxxxxxxxxxx>
(Patches split into one patch per file for easier review, but the final
commit will be one big patch. See the cover letter for more info.)
Signed-off-by: Kalle Valo <quic_kvalo@xxxxxxxxxxx>
---
drivers/net/wireless/ath/ath12k/wmi.c | 6558 +++++++++++++++++++++++++++++++++
[...]
I notice an inconsistency in logging, and wonder if there should be
consistency. Some commands have a debug log after ath12k_wmi_cmd_send()
and some have a debug log before:
+int ath12k_wmi_vdev_create(struct ath12k *ar, u8 *macaddr,
+ struct ath12k_wmi_vdev_create_arg *args)
+{
[... after case ...]
+ ret = ath12k_wmi_cmd_send(wmi, skb, WMI_VDEV_CREATE_CMDID);
+ if (ret) {
+ ath12k_warn(ar->ab,
+ "failed to submit WMI_VDEV_CREATE_CMDID\n");
+ dev_kfree_skb(skb);
+ }
+
+ ath12k_dbg(ar->ab, ATH12K_DBG_WMI,
+ "WMI vdev create: id %d type %d subtype %d macaddr %pM pdevid %d\n",
+ args->if_id, args->type, args->subtype,
+ macaddr, args->pdev_id);
+
+ return ret;
+}
[...]
+int ath12k_wmi_send_peer_delete_cmd(struct ath12k *ar,
+ const u8 *peer_addr, u8 vdev_id)
+{
[... before case ...]
+ ath12k_dbg(ar->ab, ATH12K_DBG_WMI,
+ "WMI peer delete vdev_id %d peer_addr %pM\n",
+ vdev_id, peer_addr);
+
+ ret = ath12k_wmi_cmd_send(wmi, skb, WMI_PEER_DELETE_CMDID);
+ if (ret) {
+ ath12k_warn(ar->ab, "failed to send WMI_PEER_DELETE cmd\n");
+ dev_kfree_skb(skb);
+ }
+
+ return ret;
+}
in the case where an error is reported I think it would make more sense
to have the error log after the debug log.
For the "after" case above, in the error path, you'd have logs:
failed to submit WMI_VDEV_CREATE_CMDID
WMI vdev create: id %d type %d subtype %d macaddr %pM pdevid %d
IMO this is confusing since it tells you it failed, but then seems to be
telling you it did somethig
For the "before" case above, in the error path, you'd have:
WMI peer delete vdev_id %d peer_addr %pM
failed to send WMI_PEER_DELETE cmd
This seems to make more sense since it tells you it is doing something
and then tells you that what it was trying to do failed. no ambiguity.
