| Message ID | 1456090575-28354-2-git-send-email-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Sun, 21 Feb 2016 23:36:12 +0200 Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > From: Sakari Ailus <sakari.ailus@iki.fi> > > Align them up to a power of two. Looks OK to me. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > include/uapi/linux/media.h | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h > index 6aac2f0..008d077 100644 > --- a/include/uapi/linux/media.h > +++ b/include/uapi/linux/media.h > @@ -302,7 +302,7 @@ struct media_v2_entity { > __u32 id; > char name[64]; /* FIXME: move to a property? (RFC says so) */ > __u32 function; /* Main function of the entity */ > - __u16 reserved[12]; > + __u32 reserved[14]; > }; > > /* Should match the specific fields at media_intf_devnode */ > @@ -315,7 +315,7 @@ struct media_v2_interface { > __u32 id; > __u32 intf_type; > __u32 flags; > - __u32 reserved[9]; > + __u32 reserved[13]; > > union { > struct media_v2_intf_devnode devnode; > @@ -327,7 +327,7 @@ struct media_v2_pad { > __u32 id; > __u32 entity_id; > __u32 flags; > - __u16 reserved[9]; > + __u32 reserved[5]; > }; > > struct media_v2_link { > @@ -335,7 +335,7 @@ struct media_v2_link { > __u32 source_id; > __u32 sink_id; > __u32 flags; > - __u32 reserved[5]; > + __u32 reserved[4]; > }; > > struct media_v2_topology {
Em Sun, 21 Feb 2016 23:36:12 +0200 Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > From: Sakari Ailus <sakari.ailus@iki.fi> > > Align them up to a power of two. Looks OK to me, but I would comment that the structs are aligned to 2^n for those structs. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > include/uapi/linux/media.h | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h > index 6aac2f0..008d077 100644 > --- a/include/uapi/linux/media.h > +++ b/include/uapi/linux/media.h > @@ -302,7 +302,7 @@ struct media_v2_entity { > __u32 id; > char name[64]; /* FIXME: move to a property? (RFC says so) */ > __u32 function; /* Main function of the entity */ > - __u16 reserved[12]; > + __u32 reserved[14]; > }; > > /* Should match the specific fields at media_intf_devnode */ > @@ -315,7 +315,7 @@ struct media_v2_interface { > __u32 id; > __u32 intf_type; > __u32 flags; > - __u32 reserved[9]; > + __u32 reserved[13]; > > union { > struct media_v2_intf_devnode devnode; > @@ -327,7 +327,7 @@ struct media_v2_pad { > __u32 id; > __u32 entity_id; > __u32 flags; > - __u16 reserved[9]; > + __u32 reserved[5]; > }; > > struct media_v2_link { > @@ -335,7 +335,7 @@ struct media_v2_link { > __u32 source_id; > __u32 sink_id; > __u32 flags; > - __u32 reserved[5]; > + __u32 reserved[4]; > }; > > struct media_v2_topology {
Em Mon, 22 Feb 2016 07:00:47 -0300 Mauro Carvalho Chehab <mchehab@osg.samsung.com> escreveu: > Em Sun, 21 Feb 2016 23:36:12 +0200 > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > From: Sakari Ailus <sakari.ailus@iki.fi> > > > > Align them up to a power of two. > > Looks OK to me, but I would comment that the structs are aligned to > 2^n for those structs. Hmm... on a second tought, I don't think this patch makes any sense. As those structs will be part of an array at media_v2_topology, this won't be aligned to a power of two, as we don't require that the number of links, entities, etc.. to be a aligned. Regards, Mauro > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > include/uapi/linux/media.h | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h > > index 6aac2f0..008d077 100644 > > --- a/include/uapi/linux/media.h > > +++ b/include/uapi/linux/media.h > > @@ -302,7 +302,7 @@ struct media_v2_entity { > > __u32 id; > > char name[64]; /* FIXME: move to a property? (RFC says so) */ > > __u32 function; /* Main function of the entity */ > > - __u16 reserved[12]; > > + __u32 reserved[14]; > > }; > > > > /* Should match the specific fields at media_intf_devnode */ > > @@ -315,7 +315,7 @@ struct media_v2_interface { > > __u32 id; > > __u32 intf_type; > > __u32 flags; > > - __u32 reserved[9]; > > + __u32 reserved[13]; > > > > union { > > struct media_v2_intf_devnode devnode; > > @@ -327,7 +327,7 @@ struct media_v2_pad { > > __u32 id; > > __u32 entity_id; > > __u32 flags; > > - __u16 reserved[9]; > > + __u32 reserved[5]; > > }; > > > > struct media_v2_link { > > @@ -335,7 +335,7 @@ struct media_v2_link { > > __u32 source_id; > > __u32 sink_id; > > __u32 flags; > > - __u32 reserved[5]; > > + __u32 reserved[4]; > > }; > > > > struct media_v2_topology { > >
Hi Mauro, On Mon, Feb 22, 2016 at 07:23:21AM -0300, Mauro Carvalho Chehab wrote: > Em Mon, 22 Feb 2016 07:00:47 -0300 > Mauro Carvalho Chehab <mchehab@osg.samsung.com> escreveu: > > > Em Sun, 21 Feb 2016 23:36:12 +0200 > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > > From: Sakari Ailus <sakari.ailus@iki.fi> > > > > > > Align them up to a power of two. > > > > Looks OK to me, but I would comment that the structs are aligned to > > 2^n for those structs. > > Hmm... on a second tought, I don't think this patch makes any sense. > As those structs will be part of an array at media_v2_topology, > this won't be aligned to a power of two, as we don't require that > the number of links, entities, etc.. to be a aligned. Well... that's a valid point indeed. Still I think the reserved fields do need some changes, at least aligning the size to the common ABI alignment (e.g. 8 bytes) rather than relying on the compiler to do it.
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h index 6aac2f0..008d077 100644 --- a/include/uapi/linux/media.h +++ b/include/uapi/linux/media.h @@ -302,7 +302,7 @@ struct media_v2_entity { __u32 id; char name[64]; /* FIXME: move to a property? (RFC says so) */ __u32 function; /* Main function of the entity */ - __u16 reserved[12]; + __u32 reserved[14]; }; /* Should match the specific fields at media_intf_devnode */ @@ -315,7 +315,7 @@ struct media_v2_interface { __u32 id; __u32 intf_type; __u32 flags; - __u32 reserved[9]; + __u32 reserved[13]; union { struct media_v2_intf_devnode devnode; @@ -327,7 +327,7 @@ struct media_v2_pad { __u32 id; __u32 entity_id; __u32 flags; - __u16 reserved[9]; + __u32 reserved[5]; }; struct media_v2_link { @@ -335,7 +335,7 @@ struct media_v2_link { __u32 source_id; __u32 sink_id; __u32 flags; - __u32 reserved[5]; + __u32 reserved[4]; }; struct media_v2_topology {