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

2015-08-07 Thread Rafael J. Wysocki 
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.kroge...@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 netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2015-08-06 Thread Heikki Krogerus 
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.kroge...@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,

-- 
heikki
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2015-08-05 Thread Andy Shevchenko 
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.kroge...@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 andriy.shevche...@linux.intel.com
Intel Finland Oy
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2015-08-05 Thread Shevchenko, Andriy 
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.shevche...@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.