* [PATCH] staging: greybus: power_supply fix alignment @ 2025-07-14 13:56 Akhil Varkey 2025-07-14 16:38 ` Rui Miguel Silva 0 siblings, 1 reply; 5+ messages in thread From: Akhil Varkey @ 2025-07-14 13:56 UTC (permalink / raw) To: greybus-dev, linux-staging, linux-kernel, rmfrfs, johan, elder, gregkh Cc: ~lkcamp/patches, koike, Akhil Varkey 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!! --- 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) { switch (psp) { case POWER_SUPPLY_PROP_MODEL_NAME: @@ -943,8 +943,8 @@ static int gb_power_supplies_setup(struct gb_power_supplies *supplies) goto out; supplies->supply = kcalloc(supplies->supplies_count, - sizeof(struct gb_power_supply), - GFP_KERNEL); + sizeof(struct gb_power_supply), + GFP_KERNEL); if (!supplies->supply) { ret = -ENOMEM; -- 2.47.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: greybus: power_supply fix alignment 2025-07-14 13:56 [PATCH] staging: greybus: power_supply fix alignment Akhil Varkey @ 2025-07-14 16:38 ` Rui Miguel Silva 2025-07-15 8:05 ` Greg KH 0 siblings, 1 reply; 5+ messages in thread From: Rui Miguel Silva @ 2025-07-14 16:38 UTC (permalink / raw) To: Akhil Varkey, greybus-dev, linux-staging, linux-kernel, rmfrfs, johan, elder, gregkh Cc: ~lkcamp/patches, koike 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. Also, gives you also the chance to practice to send a new version ;). Cheers, Rui > { > switch (psp) { > case POWER_SUPPLY_PROP_MODEL_NAME: > @@ -943,8 +943,8 @@ static int gb_power_supplies_setup(struct gb_power_supplies *supplies) > goto out; > > supplies->supply = kcalloc(supplies->supplies_count, > - sizeof(struct gb_power_supply), > - GFP_KERNEL); > + sizeof(struct gb_power_supply), > + GFP_KERNEL); > > if (!supplies->supply) { > ret = -ENOMEM; > -- > 2.47.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: greybus: power_supply fix alignment 2025-07-14 16:38 ` Rui Miguel Silva @ 2025-07-15 8:05 ` Greg KH 2025-07-15 9:10 ` Rui Miguel Silva 0 siblings, 1 reply; 5+ messages in thread From: Greg KH @ 2025-07-15 8:05 UTC (permalink / raw) To: Rui Miguel Silva Cc: Akhil Varkey, greybus-dev, linux-staging, linux-kernel, johan, elder, ~lkcamp/patches, koike 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. thanks, greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: greybus: power_supply fix alignment 2025-07-15 8:05 ` Greg KH @ 2025-07-15 9:10 ` Rui Miguel Silva 2025-07-15 11:38 ` Akhil 0 siblings, 1 reply; 5+ messages in thread From: Rui Miguel Silva @ 2025-07-15 9:10 UTC (permalink / raw) To: Greg KH, Rui Miguel Silva Cc: Akhil Varkey, greybus-dev, linux-staging, linux-kernel, johan, elder, ~lkcamp/patches, koike 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: greybus: power_supply fix alignment 2025-07-15 9:10 ` Rui Miguel Silva @ 2025-07-15 11:38 ` Akhil 0 siblings, 0 replies; 5+ messages in thread From: Akhil @ 2025-07-15 11:38 UTC (permalink / raw) To: Rui Miguel Silva, Greg KH Cc: greybus-dev, linux-staging, linux-kernel, johan, elder, ~lkcamp/patches, koike 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@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 > > > Thanks accepting my patches and for the suggestions on what could be done better. Best Regards, Akhil Varkey ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-07-15 11:38 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-14 13:56 [PATCH] staging: greybus: power_supply fix alignment Akhil Varkey 2025-07-14 16:38 ` Rui Miguel Silva 2025-07-15 8:05 ` Greg KH 2025-07-15 9:10 ` Rui Miguel Silva 2025-07-15 11:38 ` Akhil
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox