Re: [PATCH v2] staging: rtl8723bs: Fix coding style issues in the hal_pwr_seq.h
From: Philipp Hortmann
Date: Wed Sep 11 2024 - 12:36:54 EST
On 9/10/24 21:07, Sayyad Abid wrote:
On Tue, Sep 10, 2024 at 08:47:54PM +0200, Philipp Hortmann wrote:
On 9/10/24 14:11, abid-sayyad wrote:
Improving the code readability and coding style compliance of the code.
Running checkpatch.pl on the file raised coding style warnings:
-The comment block needs "*" on all lines of the block
from line 8 to 26
-Use tabs for indent
on line 103 and 115
Applying the patch fixes these coding style issues and makes the code more
readable/developer friendly.
Signed-off-by: abid-sayyad <sayyad.abid16@xxxxxxxxx>
---
Hi Abid,
I cannot apply your patch. Are you using the right git repo?
git remote show origin
* remote origin
Fetch URL:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
...
git branch -a
my branch: staging-testing
kernel@matrix-ESPRIMO-P710:~/Documents/git/kernels/staging$ mutt
Applying: staging: rtl8723bs: Fix coding style issues in the hal_pwr_seq.h
error: patch failed: drivers/staging/rtl8723bs/include/hal_pwr_seq.h:101
error: drivers/staging/rtl8723bs/include/hal_pwr_seq.h: patch does not apply
Patch failed at 0001 staging: rtl8723bs: Fix coding style issues in the
hal_pwr_seq.h
I found the issue, I have been amending these changes on the mainline repo,
$ git remote show origin
* remote origin
Fetch URL: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
I need to clone the staging branch.
Please also change your Description. Concentrate on why this patch makes
sense. Do not describe the patch. What is changed can be seen below. Please
also use the correct time.
I did not understand the correct time here. could you please
elaborate a little on this please.
Hi Abid,
please visit:
https://kernelnewbies.org/PatchPhilosophy#Patch_description_format_for_checkpatch_fixes
Look for this clause:
"In patch descriptions and in the subject, it is common and preferable
to use present-tense, imperative language. Write as if you are telling
git what to do with your patch. For example, instead of:"
This is about the time that should be used in patches.
Bye Philipp
You can find accepted examples in the git repo.
I'll look into the accepted patches right now. Meanwhile I have this question
, descriptions addressing only the coding style issue fixes are acceptable or
it needs to be having somethingelse too? (apologies in advance for
this illy question)
Thanks for your support.
Bye Philipp
Thank Youy for your feedbacks
changes since v1:
v2: Fix the email body, amke it more informative
link to v1:
https://lore.kernel.org/all/ca1908f3-74aa-45e7-a389-3995aba2660c@xxxxxxxxx/
.../staging/rtl8723bs/include/hal_pwr_seq.h | 46 +++++++++----------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/staging/rtl8723bs/include/hal_pwr_seq.h b/drivers/staging/rtl8723bs/include/hal_pwr_seq.h
index 5e43cc89f535..10fef1b3f393 100644
--- a/drivers/staging/rtl8723bs/include/hal_pwr_seq.h
+++ b/drivers/staging/rtl8723bs/include/hal_pwr_seq.h
@@ -5,26 +5,26 @@
#include "HalPwrSeqCmd.h"
/*
- Check document WM-20130815-JackieLau-RTL8723B_Power_Architecture v08.vsd
- There are 6 HW Power States:
- 0: POFF--Power Off
- 1: PDN--Power Down
- 2: CARDEMU--Card Emulation
- 3: ACT--Active Mode
- 4: LPS--Low Power State
- 5: SUS--Suspend
-
- The transition from different states are defined below
- TRANS_CARDEMU_TO_ACT
- TRANS_ACT_TO_CARDEMU
- TRANS_CARDEMU_TO_SUS
- TRANS_SUS_TO_CARDEMU
- TRANS_CARDEMU_TO_PDN
- TRANS_ACT_TO_LPS
- TRANS_LPS_TO_ACT
-
- TRANS_END
-*/
+ * Check document WM-20130815-JackieLau-RTL8723B_Power_Architecture v08.vsd
+ * There are 6 HW Power States:
+ * 0: POFF--Power Off
+ * 1: PDN--Power Down
+ * 2: CARDEMU--Card Emulation
+ * 3: ACT--Active Mode
+ * 4: LPS--Low Power State
+ * 5: SUS--Suspend
+ *
+ * The transition from different states are defined below
+ * TRANS_CARDEMU_TO_ACT
+ * TRANS_ACT_TO_CARDEMU
+ * TRANS_CARDEMU_TO_SUS
+ * TRANS_SUS_TO_CARDEMU
+ * TRANS_CARDEMU_TO_PDN
+ * TRANS_ACT_TO_LPS
+ * TRANS_LPS_TO_ACT
+ *
+ * TRANS_END
+ */
#define RTL8723B_TRANS_CARDEMU_TO_ACT_STEPS 26
#define RTL8723B_TRANS_ACT_TO_CARDEMU_STEPS 15
#define RTL8723B_TRANS_CARDEMU_TO_SUS_STEPS 15
@@ -101,7 +101,7 @@
{0x0007, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_SDIO_MSK, PWR_BASEADDR_MAC, PWR_CMD_WRITE, 0xFF, 0x20}, /*0x07 = 0x20 , SOP option to disable BG/MB*/ \
{0x0005, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_USB_MSK|PWR_INTF_SDIO_MSK, PWR_BASEADDR_MAC, PWR_CMD_WRITE, BIT3|BIT4, BIT3}, /*0x04[12:11] = 2b'01 enable WL suspend*/ \
{0x0005, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_PCI_MSK, PWR_BASEADDR_MAC, PWR_CMD_WRITE, BIT2, BIT2}, /*0x04[10] = 1, enable SW LPS*/ \
- {0x004A, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_USB_MSK, PWR_BASEADDR_MAC, PWR_CMD_WRITE, BIT0, 1}, /*0x48[16] = 1 to enable GPIO9 as EXT WAKEUP*/ \
+ {0x004A, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_USB_MSK, PWR_BASEADDR_MAC, PWR_CMD_WRITE, BIT0, 1}, /*0x48[16] = 1 to enable GPIO9 as EXT WAKEUP*/ \
{0x0023, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_SDIO_MSK, PWR_BASEADDR_MAC, PWR_CMD_WRITE, BIT4, BIT4}, /*0x23[4] = 1b'1 12H LDO enter sleep mode*/ \
{0x0086, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_SDIO_MSK, PWR_BASEADDR_SDIO, PWR_CMD_WRITE, BIT0, BIT0}, /*Set SDIO suspend local register*/ \
{0x0086, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_SDIO_MSK, PWR_BASEADDR_SDIO, PWR_CMD_POLLING, BIT1, 0}, /*wait power state to suspend*/
@@ -112,7 +112,7 @@
{0x0005, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_ALL_MSK, PWR_BASEADDR_MAC, PWR_CMD_WRITE, BIT3 | BIT7, 0}, /*clear suspend enable and power down enable*/ \
{0x0086, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_SDIO_MSK, PWR_BASEADDR_SDIO, PWR_CMD_WRITE, BIT0, 0}, /*Set SDIO suspend local register*/ \
{0x0086, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_SDIO_MSK, PWR_BASEADDR_SDIO, PWR_CMD_POLLING, BIT1, BIT1}, /*wait power state to suspend*/\
- {0x004A, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_USB_MSK, PWR_BASEADDR_MAC, PWR_CMD_WRITE, BIT0, 0}, /*0x48[16] = 0 to disable GPIO9 as EXT WAKEUP*/ \
+ {0x004A, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_USB_MSK, PWR_BASEADDR_MAC, PWR_CMD_WRITE, BIT0, 0}, /*0x48[16] = 0 to disable GPIO9 as EXT WAKEUP*/ \
{0x0005, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_ALL_MSK, PWR_BASEADDR_MAC, PWR_CMD_WRITE, BIT3|BIT4, 0}, /*0x04[12:11] = 2b'01enable WL suspend*/\
{0x0023, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_SDIO_MSK, PWR_BASEADDR_MAC, PWR_CMD_WRITE, BIT4, 0}, /*0x23[4] = 1b'0 12H LDO enter normal mode*/ \
{0x0301, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_PCI_MSK, PWR_BASEADDR_MAC, PWR_CMD_WRITE, 0xFF, 0},/*PCIe DMA start*/
@@ -209,7 +209,7 @@
#define RTL8723B_TRANS_END \
/* format */ \
/* { offset, cut_msk, fab_msk|interface_msk, base|cmd, msk, value }, comments here*/ \
- {0xFFFF, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_ALL_MSK, 0, PWR_CMD_END, 0, 0},
+ {0xFFFF, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_ALL_MSK, 0, PWR_CMD_END, 0, 0},
extern struct wlan_pwr_cfg rtl8723B_power_on_flow[RTL8723B_TRANS_CARDEMU_TO_ACT_STEPS+RTL8723B_TRANS_END_STEPS];
--
2.39.2