On Tue, 11 Feb 2014, Bryan Wu wrote:
> On Mon, Feb 10, 2014 at 10:37 PM, Guennadi Liakhovetski
> <g.liakhovetski@xxxxxx> wrote:
> > Hi Bryan,
> >
> > On Mon, 10 Feb 2014, Bryan Wu wrote:
> >
> >> On Sun, Feb 9, 2014 at 2:20 PM, Guennadi Liakhovetski
> >> <g.liakhovetski@xxxxxx> wrote:
> >> > Hi Bryan,
> >> >
> >> > Thanks for reiterating this patch!
> >> >
> >>
> >> Sure, my pleasure. I basically assembled your patches together and
> >> change them to use latest V4L2 soc_camera API.
> >>
> >> > On Fri, 7 Feb 2014, Bryan Wu wrote:
> >
> > [snip]
> >
> >> >> @@ -67,6 +81,8 @@ struct soc_camera_async_client {
> >> >>
> >> >> static int soc_camera_video_start(struct soc_camera_device *icd);
> >> >> static int video_dev_create(struct soc_camera_device *icd);
> >> >> +static void soc_camera_of_i2c_info(struct device_node *node,
> >> >> + struct soc_camera_of_client *sofc);
> >> >
> >> > If you have to resubmit this patch, plase, make sure the second line of
> >> > the above declaration is aligned af usual - under the first character
> >> > _after_ the opening bracket.
> >> >
> >>
> >> No problem, I will update this.
> >> Hmmm, something weird on my side. I did put the second line starting
> >> under the first character after the opening bracket. But in git show
> >> and git format-patch I got this
> >> ---
> >> static int soc_camera_video_start(struct soc_camera_device *icd);
> >> static int video_dev_create(struct soc_camera_device *icd);
> >> +static void soc_camera_of_i2c_info(struct device_node *node,
> >> + struct soc_camera_of_client *sofc);
> >> ---
> >>
> >> But I think that's what you want, right?
> >
> > Don't know - now aöö TABs above are replaced with spaces, so, cannot say.
> >
> > [snip]
> >
> >> >> +{
> >> >> + struct soc_camera_of_client *sofc;
> >> >> + struct soc_camera_desc *sdesc;
> >> >
> >> > I'm really grateful, that you decided to use my original patch and
> >> > preserve my authorship! But then, I think, it'd be also better to avoid
> >> > unnecessary changes to it. What was wrong with allocation of *sofc in the
> >> > definition line?
> >> >
> >>
> >> Oh, this is really I want to bring up. It's a very subtle bug here.
> >>
> >> If we use local variable sofc instead of zalloc, fields of sofc have
> >> undetermined None NULL value.
> >
> > No. If you initialise some members of a struct in its definition line, the
> > rest will be initialised to 0 / NULL. I.e. in
> >
> > struct foo y = {.x = 1,};
> >
> > all other fields of y will be initialised to 0.
>
> I see, but original one is soc_camera_link which is simple in this
> case. right now we move to soc_camera_desc. I think following line is
> not very straight forward in a local function.
>
> struct soc_camera_desc sdesc = { .host_desc = { .host_wait = true,},};
I usually do
struct soc_camera_desc sdesc = {.host_desc.host_wait = true,};
> What about a) struct soc_camera_desc sdesc and use memset to all 0. b)
> use kzalloc() and kfree() in this function.
>
> I think b) is more straight forward and easy to understand.
With error handling for a failed kzalloc() - don't think so.
Thanks
Guennadi
>
> Thanks,
> -Bryan
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
