[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <93afc2fd-7aa2-41d5-bd08-2afc72540abb@gmail.com>
Date: Wed, 11 Sep 2024 18:36:44 +0200
From: Philipp Hortmann <philipp.g.hortmann@...il.com>
To: Sayyad Abid <sayyad.abid16@...il.com>
Cc: linux-staging@...ts.linux.dev, gregkh@...uxfoundation.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] staging: rtl8723bs: Fix coding style issues in the
hal_pwr_seq.h
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@...il.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@...rix-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
>>>
>>
Powered by blists - more mailing lists