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

From: Sayyad Abid
Date: Tue Sep 10 2024 - 15:08:35 EST

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