linux-kernel - Re: [PATCH] staging: greybus: power

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: <af2103a0-f9d0-4980-9ec7-e4ce512ea59a@disroot.org>
Date: Tue, 15 Jul 2025 13:38:02 +0200
From: Akhil <akhilvarkey@...root.org>
To: Rui Miguel Silva <rmfrfs@...il.com>, Greg KH <gregkh@...uxfoundation.org>
Cc: greybus-dev@...ts.linaro.org, linux-staging@...ts.linux.dev,
 linux-kernel@...r.kernel.org, johan@...nel.org, elder@...nel.org,
 ~lkcamp/patches@...ts.sr.ht, koike@...lia.com
Subject: Re: [PATCH] staging: greybus: power_supply fix alignment

Hi Rui and Greg,

On 15/07/25 11:10, Rui Miguel Silva wrote:
> Hey Greg,
> On Tue Jul 15, 2025 at 9:05 AM WEST, Greg KH wrote:
> 
>> On Mon, Jul 14, 2025 at 05:38:31PM +0100, Rui Miguel Silva wrote:
>>> Hey Akhil,
>>> Thanks for your patch.
>>>
>>> All looks good with the exception of a small nit...
>>>
>>> On Mon Jul 14, 2025 at 2:56 PM WEST, Akhil Varkey wrote:
>>>
>>>> Fix checkpatch check "CHECK:Alignment should match open parenthesis"
>>>>
>>>> Signed-off-by: Akhil Varkey <akhilvarkey@...root.org>
>>>> ---
>>>>
>>>> Hello, This is my first patch, I appreciate any feedbacks. Thanks!!
>>>
>>> Welcome, and continue...
>>>
>>>> ---
>>>>   drivers/staging/greybus/power_supply.c | 14 +++++++-------
>>>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/greybus/power_supply.c b/drivers/staging/greybus/power_supply.c
>>>> index 2ef46822f676..a484c0ca058d 100644
>>>> --- a/drivers/staging/greybus/power_supply.c
>>>> +++ b/drivers/staging/greybus/power_supply.c
>>>> @@ -324,7 +324,7 @@ static struct gb_power_supply_prop *get_psy_prop(struct gb_power_supply *gbpsy,
>>>>   }
>>>>   
>>>>   static int is_psy_prop_writeable(struct gb_power_supply *gbpsy,
>>>> -				     enum power_supply_property psp)
>>>> +				 enum power_supply_property psp)
>>>>   {
>>>>   	struct gb_power_supply_prop *prop;
>>>>   
>>>> @@ -493,7 +493,7 @@ static int gb_power_supply_description_get(struct gb_power_supply *gbpsy)
>>>>   	if (!gbpsy->model_name)
>>>>   		return -ENOMEM;
>>>>   	gbpsy->serial_number = kstrndup(resp.serial_number, PROP_MAX,
>>>> -				       GFP_KERNEL);
>>>> +					GFP_KERNEL);
>>>>   	if (!gbpsy->serial_number)
>>>>   		return -ENOMEM;
>>>>   
>>>> @@ -546,7 +546,7 @@ static int gb_power_supply_prop_descriptors_get(struct gb_power_supply *gbpsy)
>>>>   	}
>>>>   
>>>>   	gbpsy->props = kcalloc(gbpsy->properties_count, sizeof(*gbpsy->props),
>>>> -			      GFP_KERNEL);
>>>> +			       GFP_KERNEL);
>>>>   	if (!gbpsy->props) {
>>>>   		ret = -ENOMEM;
>>>>   		goto out_put_operation;
>>>> @@ -634,8 +634,8 @@ static int __gb_power_supply_property_get(struct gb_power_supply *gbpsy,
>>>>   }
>>>>   
>>>>   static int __gb_power_supply_property_strval_get(struct gb_power_supply *gbpsy,
>>>> -						enum power_supply_property psp,
>>>> -						union power_supply_propval *val)
>>>> +						 enum power_supply_property psp,
>>>> +						 union power_supply_propval *val)
>>>
>>> Here you fix the alignment, but the last line goes over column 81, even
>>> though 80 is not really one hard requirement anymore, all code
>>> (strings is ok to go over to be easier to grep for messages) is on that
>>> register.
>>>
>>> So, to be coherent, if you could please send a V2 without this specific change
>>> would be great, Or even better, if you could get rid of all the _ and __
>>> prefixes in functions names that would be great, and will give more
>>> space for function paramethers.
>>> Your call.
>>
>> Nah, this is fine as-is, we can go over the limit to 100 for tiny stuff
>> like this.
>>
>> And the __ prefixes should be there to show no locking, or "internal"
>> functions, right?  So changing the name needs to happen very carefully.
> 
> Yup, we can go either way here. I do not have strong feelings about
> this.
> 
> So, LGTM, Thanks Akhil.
> Reviewed-by: Rui Miguel Silva <rui.silva@...aro.org>
> 
> Cheers,
>      Rui
> 
>>
>> thanks,
>>
>> greg k-h
> 
> 
> 

Thanks accepting my patches and for the suggestions on what could be 
done better.

Best Regards,
Akhil Varkey