Re: [PATCH] staging: greybus: power_supply fix alignment

[Rui Miguel Silva <rmfrfs@gmail.com>]

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@disroot.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@linaro.org>

Cheers,
    Rui

>
> thanks,
>
> greg k-h