staging: rtl8723bs: Fix coding style issues in the hal_pwr_seq.h

[PATCH v2] staging: rtl8723bs: Fix coding style issues in the hal_pwr_seq.h

Posted by abid-sayyad 2 months, 2 weeks ago

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@gmail.com>
---
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@gmail.com/
 .../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

Re: [PATCH v2] staging: rtl8723bs: Fix coding style issues in the hal_pwr_seq.h

Posted by Philipp Hortmann 2 months, 2 weeks ago

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@gmail.com>
> ---

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

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.

You can find accepted examples in the git repo.

Thanks for your support.

Bye Philipp



> 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@gmail.com/
>   .../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
>

Re: [PATCH v2] staging: rtl8723bs: Fix coding style issues in the hal_pwr_seq.h

Posted by Sayyad Abid 2 months, 2 weeks ago

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@gmail.com>
> > ---
>
> 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.
> 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@gmail.com/
> >   .../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
> >
>

Re: [PATCH v2] staging: rtl8723bs: Fix coding style issues in the hal_pwr_seq.h

Posted by Philipp Hortmann 2 months, 2 weeks ago

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@gmail.com>
>>> ---
>>
>> 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@gmail.com/
>>>    .../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
>>>
>>