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

[Sayyad Abid <sayyad.abid16@gmail.com>]

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
> >
>