Hi Jacopo,
Thank you for the patch.
This is a partial review, as some of the comments will result in large changes
that would make review of some of the current code pointless. There is however
enough comments to keep you busy for a bit :-)
By the way, you might want to keep your development h
Hi Laurent,
thanks for the extensive review
On Thu, Apr 27, 2017 at 02:47:14PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> This is a partial review, as some of the comments will result in large changes
> that would make review of some of the current code pointle
Hi Jacopo,
On Mon, May 1, 2017 at 4:37 PM, jmondi wrote:
>> > + struct sh_ceu_fmt **active_fmts;
>> > + struct sh_ceu_fmt *current_fmt;
>>
>> These two should be const.
>>
>
> Why both of them? I understand the list of active formats is built
> during driver initilization, but the "current_fm
Hi Laurent,
On Thu, Apr 27, 2017 at 02:47:14PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
>
[snip]
> > + pcdev = devm_kzalloc(&pdev->dev, sizeof(*pcdev), GFP_KERNEL);
>
> devm_kzalloc() is harmful here. You're embedding a struct video_device instead
> struct sh_ceu_dev, and the video device
Hi Jacopo,
On Monday 01 May 2017 16:37:54 jmondi wrote:
> On Thu, Apr 27, 2017 at 02:47:14PM +0300, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > Thank you for the patch.
> >
> > This is a partial review, as some of the comments will result in large
> > changes that would make review of some of
Hi Jacopo,
On Tuesday 02 May 2017 11:48:42 jmondi wrote:
> On Thu, Apr 27, 2017 at 02:47:14PM +0300, Laurent Pinchart wrote:
> > Hi Jacopo,
>
> [snip]
>
> > > + pcdev = devm_kzalloc(&pdev->dev, sizeof(*pcdev), GFP_KERNEL);
> >
> > devm_kzalloc() is harmful here. You're embedding a struct video_
Hi Laurent,
On Thu, Apr 27, 2017 at 02:47:14PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
[snip]
> > +/**
> > + * Collect formats supported by CEU and sensor subdevice
> > + */
> > +static int sh_ceu_init_active_formats(struct sh_ceu_dev *pcdev)
> > +{
> > + struc
Hi Jacopo,
On Wednesday 03 May 2017 11:52:36 jmondi wrote:
> On Thu, Apr 27, 2017 at 02:47:14PM +0300, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > Thank you for the patch.
>
> [snip]
>
> >> +/**
> >> + * Collect formats supported by CEU and sensor subdevice
> >> + */
> >> +static int sh_ceu_
Hi Laurent,
On Wed, May 03, 2017 at 06:06:19PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Wednesday 03 May 2017 11:52:36 jmondi wrote:
> > On Thu, Apr 27, 2017 at 02:47:14PM +0300, Laurent Pinchart wrote:
> > > Hi Jacopo,
> > >
> > > Thank you for the patch.
> >
> > [snip]
> >
> > >> +/**
Hi Jacopo,
On Wednesday 03 May 2017 18:14:03 jmondi wrote:
> On Wed, May 03, 2017 at 06:06:19PM +0300, Laurent Pinchart wrote:
> > On Wednesday 03 May 2017 11:52:36 jmondi wrote:
> >> On Thu, Apr 27, 2017 at 02:47:14PM +0300, Laurent Pinchart wrote:
> >>> Hi Jacopo,
> >>>
> >>> Thank you for the
Hi Jacopo,
On Wednesday, May 03, 2017, jmondi wrote:
> I have a proposal here, as the original driver only supported "image fetch
> mode" (ie. It accepts data in YUYV with components ordering arbitrary
> swapped) as a first step we may want to replicate this, ignoring data
> synch fetch mode (Chri
Hi Chris,
On Thursday 04 May 2017 12:23:32 Chris Brandt wrote:
> On Wednesday, May 03, 2017, jmondi wrote:
> > I have a proposal here, as the original driver only supported "image
> > fetch mode" (ie. It accepts data in YUYV with components ordering
> > arbitrary swapped) as a first step we may wa
12 matches
Mail list logo