diff mbox

[1/5] device property: helper macros for property entry creation

Message ID 1438781947-7952-2-git-send-email-heikki.krogerus@linux.intel.com
State Deferred
Headers show

Commit Message

Heikki Krogerus Aug. 5, 2015, 1:39 p.m. UTC
Marcos for easier creation of build-in property entries.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/property.h | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Comments

Andy Shevchenko Aug. 5, 2015, 2:02 p.m. UTC | #1
On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:
> Marcos for easier creation of build-in property entries.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  include/linux/property.h | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 76ebde9..204d899 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -152,6 +152,41 @@ struct property_entry {
>  	} value;
>  };
>  
> +#define PROP_ENTRY_U8(_name_, _val_) { \

PROP_ prefix is too generic.
Maybe DEVPROP_ ? At least for the latter no records in the current
sources.

> +	.name = _name_, \
> +	.type = DEV_PROP_U8, \
> +	.nval = 1, \
> +	.value.u8_data = _val_, \
> +}
> +
> +#define PROP_ENTRY_U16(_name_, _val_) { \
> +	.name = _name_, \
> +	.type = DEV_PROP_U16, \
> +	.nval = 1, \
> +	.value.u16_data = _val_, \
> +}
> +
> +#define PROP_ENTRY_U32(_name_, _val_) { \
> +	.name = _name_, \
> +	.type = DEV_PROP_U32, \
> +	.nval = 1, \
> +	.value.u32_data = _val_, \
> +}
> +
> +#define PROP_ENTRY_U64(_name_, _val_) { \
> +	.name = _name_, \
> +	.type = DEV_PROP_U64, \
> +	.nval = 1, \
> +	.value.u64_data = _val_, \
> +}
> +
> +#define PROP_ENTRY_STRING(_name_, _val_) { \

…_STRING_ARRAY I can notice.

> +	.name = _name_, \
> +	.type = DEV_PROP_STRING, \
> +	.nval = 1, \
> +	.value.str = (const char **)_val_, \
> +}
> +
>  /**
>   * struct property_set - Collection of "built-in" device properties.
>   * @fwnode: Handle to be pointed to by the fwnode field of struct 
> device.
Andy Shevchenko Aug. 5, 2015, 2:12 p.m. UTC | #2
On Wed, 2015-08-05 at 17:02 +0300, Andy Shevchenko wrote:
> On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:


[]

> > +#define PROP_ENTRY_STRING(_name_, _val_) { \

> 

> …_STRING_ARRAY I can notice.


s / can / can't /

> 

> > +	.name = _name_, \

> > +	.type = DEV_PROP_STRING, \

> > +	.nval = 1, \

> > +	.value.str = (const char **)_val_, \

> > +}

> > +

> >  /**

> >   * struct property_set - Collection of "built-in" device 

> > properties.

> >   * @fwnode: Handle to be pointed to by the fwnode field of struct 

> > device.

> 


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Heikki Krogerus Aug. 6, 2015, 7:48 a.m. UTC | #3
On Wed, Aug 05, 2015 at 05:02:18PM +0300, Andy Shevchenko wrote:
> On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:
> > Marcos for easier creation of build-in property entries.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  include/linux/property.h | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/include/linux/property.h b/include/linux/property.h
> > index 76ebde9..204d899 100644
> > --- a/include/linux/property.h
> > +++ b/include/linux/property.h
> > @@ -152,6 +152,41 @@ struct property_entry {
> >  	} value;
> >  };
> >  
> > +#define PROP_ENTRY_U8(_name_, _val_) { \
> 
> PROP_ prefix is too generic.
> Maybe DEVPROP_ ? At least for the latter no records in the current
> sources.

I disagree with that. IMO this kind of macros should ideally resemble
the structure name they are used to fill (struct property_entry in
this case). And there are already definitions for DEV_PROP_* to
describe the types, so using something like DEVPROP_* here is just
confusing.

If PROP_ENTRY_* is really not good enough, we can change them
PROPERTY_ENTRY_*. But is PROP_ENTRY_* really so bad?

Rafael, what is your opinion?


Thanks,
Rafael J. Wysocki Aug. 7, 2015, 10:08 p.m. UTC | #4
On Thursday, August 06, 2015 10:48:48 AM Heikki Krogerus wrote:
> On Wed, Aug 05, 2015 at 05:02:18PM +0300, Andy Shevchenko wrote:
> > On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:
> > > Marcos for easier creation of build-in property entries.
> > > 
> > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > ---
> > >  include/linux/property.h | 35 +++++++++++++++++++++++++++++++++++
> > >  1 file changed, 35 insertions(+)
> > > 
> > > diff --git a/include/linux/property.h b/include/linux/property.h
> > > index 76ebde9..204d899 100644
> > > --- a/include/linux/property.h
> > > +++ b/include/linux/property.h
> > > @@ -152,6 +152,41 @@ struct property_entry {
> > >  	} value;
> > >  };
> > >  
> > > +#define PROP_ENTRY_U8(_name_, _val_) { \
> > 
> > PROP_ prefix is too generic.
> > Maybe DEVPROP_ ? At least for the latter no records in the current
> > sources.
> 
> I disagree with that. IMO this kind of macros should ideally resemble
> the structure name they are used to fill (struct property_entry in
> this case). And there are already definitions for DEV_PROP_* to
> describe the types, so using something like DEVPROP_* here is just
> confusing.
> 
> If PROP_ENTRY_* is really not good enough, we can change them
> PROPERTY_ENTRY_*. But is PROP_ENTRY_* really so bad?
> 
> Rafael, what is your opinion?

I would prefer PROPERTY_ENTRY_ to be honest.  It's not like we need to save
characters here.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/property.h b/include/linux/property.h
index 76ebde9..204d899 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -152,6 +152,41 @@  struct property_entry {
 	} value;
 };
 
+#define PROP_ENTRY_U8(_name_, _val_) { \
+	.name = _name_, \
+	.type = DEV_PROP_U8, \
+	.nval = 1, \
+	.value.u8_data = _val_, \
+}
+
+#define PROP_ENTRY_U16(_name_, _val_) { \
+	.name = _name_, \
+	.type = DEV_PROP_U16, \
+	.nval = 1, \
+	.value.u16_data = _val_, \
+}
+
+#define PROP_ENTRY_U32(_name_, _val_) { \
+	.name = _name_, \
+	.type = DEV_PROP_U32, \
+	.nval = 1, \
+	.value.u32_data = _val_, \
+}
+
+#define PROP_ENTRY_U64(_name_, _val_) { \
+	.name = _name_, \
+	.type = DEV_PROP_U64, \
+	.nval = 1, \
+	.value.u64_data = _val_, \
+}
+
+#define PROP_ENTRY_STRING(_name_, _val_) { \
+	.name = _name_, \
+	.type = DEV_PROP_STRING, \
+	.nval = 1, \
+	.value.str = (const char **)_val_, \
+}
+
 /**
  * struct property_set - Collection of "built-in" device properties.
  * @fwnode: Handle to be pointed to by the fwnode field of struct device.