Re: [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call
Em Fri, 5 Oct 2018 13:06:04 +0300 Sakari Ailus escreveu: > > > > - unsigned int i; > > > > > > I'd like to keep this here. > > > > Why? IMHO, it makes harder to read (yet, if you insist, I'm ok with > > both ways). > > Generally loop, temporary, return etc. variables are nice to declare as > last. That is the practice in this file and generally in kernel code, > albeit with variable degree of application. I've seen more than one practice of ordering arguments at the Kernel :-) Anyway, I kept it there on the version I just sent. Thanks, Mauro
Re: [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call
On Fri, Oct 05, 2018 at 06:54:49AM -0300, Mauro Carvalho Chehab wrote:
> Em Fri, 5 Oct 2018 11:03:10 +0300
> Sakari Ailus escreveu:
>
> > Hi Mauro,
> >
> > On Thu, Oct 04, 2018 at 06:13:48PM -0400, Mauro Carvalho Chehab wrote:
> > > The v4l2_fwnode_reference_parse_int_props() has a big name, causing
> > > it to cause coding style warnings. Also, it depends on a const
> > > struct embedded indide a function.
> > >
> > > Rearrange the logic in order to move the struct declaration out
> > > of such function and use it inside this function.
> > >
> > > That cleans up some coding style issues.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab
> > > ---
> > > drivers/media/v4l2-core/v4l2-fwnode.c | 25 +
> > > 1 file changed, 13 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> > > b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > index a7c2487154a4..e0cd119d6f5c 100644
> > > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > @@ -1006,6 +1006,12 @@ v4l2_fwnode_reference_get_int_prop(struct
> > > fwnode_handle *fwnode,
> > > return fwnode;
> > > }
> > >
> > > +struct v4l2_fwnode_int_props {
> > > + const char *name;
> > > + const char * const *props;
> > > + unsigned int nprops;
> > > +};
> > > +
> > > /*
> > > * v4l2_fwnode_reference_parse_int_props - parse references for async
> > > * sub-devices
> > > @@ -1032,13 +1038,14 @@ v4l2_fwnode_reference_get_int_prop(struct
> > > fwnode_handle *fwnode,
> > > static int
> > > v4l2_fwnode_reference_parse_int_props(struct device *dev,
> > > struct v4l2_async_notifier *notifier,
> > > - const char *prop,
> > > - const char * const *props,
> > > - unsigned int nprops)
> > > + const struct v4l2_fwnode_int_props *p)
> > > {
> > > struct fwnode_handle *fwnode;
> > > unsigned int index;
> > > int ret;
> > > + const char *prop = p->name;
> > > + const char * const *props = p->props;
> > > + unsigned int nprops = p->nprops;
> > >
> > > index = 0;
> > > do {
> > > @@ -1092,16 +1099,12 @@ v4l2_fwnode_reference_parse_int_props(struct
> > > device *dev,
> > > int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
> > > struct v4l2_async_notifier
> > > *notifier)
> > > {
> > > + unsigned int i;
> > > static const char * const led_props[] = { "led" };
> > > - static const struct {
> > > - const char *name;
> > > - const char * const *props;
> > > - unsigned int nprops;
> > > - } props[] = {
> > > + static const struct v4l2_fwnode_int_props props[] = {
> > > { "flash-leds", led_props, ARRAY_SIZE(led_props) },
> > > { "lens-focus", NULL, 0 },
> > > };
> > > - unsigned int i;
> >
> > I'd like to keep this here.
>
> Why? IMHO, it makes harder to read (yet, if you insist, I'm ok with
> both ways).
Generally loop, temporary, return etc. variables are nice to declare as
last. That is the practice in this file and generally in kernel code,
albeit with variable degree of application.
See e.g. drivers/media/v4l2-core/v4l2-ctrls.c .
>
> > Apart from that,
> >
> > Acked-by: Sakari Ailus
> >
> > >
> > > for (i = 0; i < ARRAY_SIZE(props); i++) {
> > > int ret;
> > > @@ -1109,9 +1112,7 @@ int
> > > v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
> > > if (props[i].props && is_acpi_node(dev_fwnode(dev)))
> > > ret = v4l2_fwnode_reference_parse_int_props(dev,
> > > notifier,
> > > -
> > > props[i].name,
> > > -
> > > props[i].props,
> > > -
> > > props[i].nprops);
> > > + [i]);
> > > else
> > > ret = v4l2_fwnode_reference_parse(dev, notifier,
> > > props[i].name);
> >
>
>
>
> Thanks,
> Mauro
--
Sakari Ailus
sakari.ai...@linux.intel.com
Re: [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call
Em Fri, 5 Oct 2018 11:03:10 +0300
Sakari Ailus escreveu:
> Hi Mauro,
>
> On Thu, Oct 04, 2018 at 06:13:48PM -0400, Mauro Carvalho Chehab wrote:
> > The v4l2_fwnode_reference_parse_int_props() has a big name, causing
> > it to cause coding style warnings. Also, it depends on a const
> > struct embedded indide a function.
> >
> > Rearrange the logic in order to move the struct declaration out
> > of such function and use it inside this function.
> >
> > That cleans up some coding style issues.
> >
> > Signed-off-by: Mauro Carvalho Chehab
> > ---
> > drivers/media/v4l2-core/v4l2-fwnode.c | 25 +
> > 1 file changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> > b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index a7c2487154a4..e0cd119d6f5c 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -1006,6 +1006,12 @@ v4l2_fwnode_reference_get_int_prop(struct
> > fwnode_handle *fwnode,
> > return fwnode;
> > }
> >
> > +struct v4l2_fwnode_int_props {
> > + const char *name;
> > + const char * const *props;
> > + unsigned int nprops;
> > +};
> > +
> > /*
> > * v4l2_fwnode_reference_parse_int_props - parse references for async
> > *sub-devices
> > @@ -1032,13 +1038,14 @@ v4l2_fwnode_reference_get_int_prop(struct
> > fwnode_handle *fwnode,
> > static int
> > v4l2_fwnode_reference_parse_int_props(struct device *dev,
> > struct v4l2_async_notifier *notifier,
> > - const char *prop,
> > - const char * const *props,
> > - unsigned int nprops)
> > + const struct v4l2_fwnode_int_props *p)
> > {
> > struct fwnode_handle *fwnode;
> > unsigned int index;
> > int ret;
> > + const char *prop = p->name;
> > + const char * const *props = p->props;
> > + unsigned int nprops = p->nprops;
> >
> > index = 0;
> > do {
> > @@ -1092,16 +1099,12 @@ v4l2_fwnode_reference_parse_int_props(struct device
> > *dev,
> > int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
> >struct v4l2_async_notifier
> > *notifier)
> > {
> > + unsigned int i;
> > static const char * const led_props[] = { "led" };
> > - static const struct {
> > - const char *name;
> > - const char * const *props;
> > - unsigned int nprops;
> > - } props[] = {
> > + static const struct v4l2_fwnode_int_props props[] = {
> > { "flash-leds", led_props, ARRAY_SIZE(led_props) },
> > { "lens-focus", NULL, 0 },
> > };
> > - unsigned int i;
>
> I'd like to keep this here.
Why? IMHO, it makes harder to read (yet, if you insist, I'm ok with
both ways).
> Apart from that,
>
> Acked-by: Sakari Ailus
>
> >
> > for (i = 0; i < ARRAY_SIZE(props); i++) {
> > int ret;
> > @@ -1109,9 +1112,7 @@ int
> > v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
> > if (props[i].props && is_acpi_node(dev_fwnode(dev)))
> > ret = v4l2_fwnode_reference_parse_int_props(dev,
> > notifier,
> > -
> > props[i].name,
> > -
> > props[i].props,
> > -
> > props[i].nprops);
> > + [i]);
> > else
> > ret = v4l2_fwnode_reference_parse(dev, notifier,
> > props[i].name);
>
Thanks,
Mauro
Re: [PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call
Hi Mauro,
On Thu, Oct 04, 2018 at 06:13:48PM -0400, Mauro Carvalho Chehab wrote:
> The v4l2_fwnode_reference_parse_int_props() has a big name, causing
> it to cause coding style warnings. Also, it depends on a const
> struct embedded indide a function.
>
> Rearrange the logic in order to move the struct declaration out
> of such function and use it inside this function.
>
> That cleans up some coding style issues.
>
> Signed-off-by: Mauro Carvalho Chehab
> ---
> drivers/media/v4l2-core/v4l2-fwnode.c | 25 +
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index a7c2487154a4..e0cd119d6f5c 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -1006,6 +1006,12 @@ v4l2_fwnode_reference_get_int_prop(struct
> fwnode_handle *fwnode,
> return fwnode;
> }
>
> +struct v4l2_fwnode_int_props {
> + const char *name;
> + const char * const *props;
> + unsigned int nprops;
> +};
> +
> /*
> * v4l2_fwnode_reference_parse_int_props - parse references for async
> * sub-devices
> @@ -1032,13 +1038,14 @@ v4l2_fwnode_reference_get_int_prop(struct
> fwnode_handle *fwnode,
> static int
> v4l2_fwnode_reference_parse_int_props(struct device *dev,
> struct v4l2_async_notifier *notifier,
> - const char *prop,
> - const char * const *props,
> - unsigned int nprops)
> + const struct v4l2_fwnode_int_props *p)
> {
> struct fwnode_handle *fwnode;
> unsigned int index;
> int ret;
> + const char *prop = p->name;
> + const char * const *props = p->props;
> + unsigned int nprops = p->nprops;
>
> index = 0;
> do {
> @@ -1092,16 +1099,12 @@ v4l2_fwnode_reference_parse_int_props(struct device
> *dev,
> int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
> struct v4l2_async_notifier
> *notifier)
> {
> + unsigned int i;
> static const char * const led_props[] = { "led" };
> - static const struct {
> - const char *name;
> - const char * const *props;
> - unsigned int nprops;
> - } props[] = {
> + static const struct v4l2_fwnode_int_props props[] = {
> { "flash-leds", led_props, ARRAY_SIZE(led_props) },
> { "lens-focus", NULL, 0 },
> };
> - unsigned int i;
I'd like to keep this here.
Apart from that,
Acked-by: Sakari Ailus
>
> for (i = 0; i < ARRAY_SIZE(props); i++) {
> int ret;
> @@ -1109,9 +1112,7 @@ int
> v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
> if (props[i].props && is_acpi_node(dev_fwnode(dev)))
> ret = v4l2_fwnode_reference_parse_int_props(dev,
> notifier,
> -
> props[i].name,
> -
> props[i].props,
> -
> props[i].nprops);
> + [i]);
> else
> ret = v4l2_fwnode_reference_parse(dev, notifier,
> props[i].name);
--
Regards,
Sakari Ailus
sakari.ai...@linux.intel.com
[PATCH 3/3] media: v4l2-fwnode: simplify v4l2_fwnode_reference_parse_int_props() call
The v4l2_fwnode_reference_parse_int_props() has a big name, causing
it to cause coding style warnings. Also, it depends on a const
struct embedded indide a function.
Rearrange the logic in order to move the struct declaration out
of such function and use it inside this function.
That cleans up some coding style issues.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/v4l2-core/v4l2-fwnode.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
b/drivers/media/v4l2-core/v4l2-fwnode.c
index a7c2487154a4..e0cd119d6f5c 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -1006,6 +1006,12 @@ v4l2_fwnode_reference_get_int_prop(struct fwnode_handle
*fwnode,
return fwnode;
}
+struct v4l2_fwnode_int_props {
+ const char *name;
+ const char * const *props;
+ unsigned int nprops;
+};
+
/*
* v4l2_fwnode_reference_parse_int_props - parse references for async
*sub-devices
@@ -1032,13 +1038,14 @@ v4l2_fwnode_reference_get_int_prop(struct fwnode_handle
*fwnode,
static int
v4l2_fwnode_reference_parse_int_props(struct device *dev,
struct v4l2_async_notifier *notifier,
- const char *prop,
- const char * const *props,
- unsigned int nprops)
+ const struct v4l2_fwnode_int_props *p)
{
struct fwnode_handle *fwnode;
unsigned int index;
int ret;
+ const char *prop = p->name;
+ const char * const *props = p->props;
+ unsigned int nprops = p->nprops;
index = 0;
do {
@@ -1092,16 +1099,12 @@ v4l2_fwnode_reference_parse_int_props(struct device
*dev,
int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
struct v4l2_async_notifier
*notifier)
{
+ unsigned int i;
static const char * const led_props[] = { "led" };
- static const struct {
- const char *name;
- const char * const *props;
- unsigned int nprops;
- } props[] = {
+ static const struct v4l2_fwnode_int_props props[] = {
{ "flash-leds", led_props, ARRAY_SIZE(led_props) },
{ "lens-focus", NULL, 0 },
};
- unsigned int i;
for (i = 0; i < ARRAY_SIZE(props); i++) {
int ret;
@@ -1109,9 +1112,7 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(struct
device *dev,
if (props[i].props && is_acpi_node(dev_fwnode(dev)))
ret = v4l2_fwnode_reference_parse_int_props(dev,
notifier,
-
props[i].name,
-
props[i].props,
-
props[i].nprops);
+ [i]);
else
ret = v4l2_fwnode_reference_parse(dev, notifier,
props[i].name);
--
2.17.1
