lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ