All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shuah Khan <skhan@linuxfoundation.org>
To: Yunke Cao <yunkec@google.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Kieran Bingham <kieran.bingham@ideasonboard.com>,
	Tomasz Figa <tfiga@chromium.org>,
	linux-media@vger.kernel.org,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH v2 1/2] media: vimc: add ancillary lens
Date: Tue, 21 Jun 2022 09:57:10 -0600	[thread overview]
Message-ID: <610bc1b9-c013-46d7-4446-5a5eab4c2569@linuxfoundation.org> (raw)
In-Reply-To: <CANqU6Fed=E1ogvR1ccwJA9bR_9VxU4QPd6PjvBAyRrMeQuRWfg@mail.gmail.com>

On 6/19/22 7:57 PM, Yunke Cao wrote:
> Hi Shuah,
> 
> Thanks for the review.
> 
> On Sat, Jun 18, 2022 at 5:01 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 6/16/22 7:57 PM, Yunke Cao wrote:
>>> Add a basic version of vimc lens.
>>> The lens supports V4L2_CID_FOCUS_ABSOLUTE control.
>>> Link the lens with vimc sensors using media-controller
>>> ancillary links.
>>>
>>
>> Commit log lines are usually ~75 charracters long. Make it easier
>> to read.
> That's good to know. Thanks!
> Should I send v3 and trim the commit log?
> I'm thinking something like this:
> 
> The lens supports FOCUS_ABSOLUTE control.
> Link the lens with sensors using ancillary links.
> 

Why is this necessary? How did you test this change? How could
one use this feature?

>>
>>> This change can be used to test the recently added ancillary
>>> links.
>>>
>>
>> Care to add instructions on how one would test ancillary with
>> this feature?
> 
> The lens shows up in the media topology. I documented it in 2/2.
> Not sure what else is necessary here.
> 

Why is this necessary? How did you test this change? How could
one use this feature?

Take a look at the some of the other commit messages e.g:
4a2e0a806cb58a4d3106add079488e0b56a221b6
5f3fb5c54d67670fa6743d2434a5bd43a97c01de

This one is a good example of what would a commit log adding a
new feature should include.

commit 9b4a9b31b9aeef262b4fa211f2083c30c4391df7
Author: Pedro Terra <pedro@terraco.de>
Date:   Tue Aug 31 19:48:22 2021 +0200

     media: vimc: Enable set resolution at the scaler src pad

thanks,
-- Shuah

  reply	other threads:[~2022-06-21 15:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-17  1:57 [PATCH v2 0/2] media: vimc: add ancillary lens Yunke Cao
2022-06-17  1:57 ` [PATCH v2 1/2] " Yunke Cao
2022-06-17 20:01   ` Shuah Khan
2022-06-20  1:57     ` Yunke Cao
2022-06-21 15:57       ` Shuah Khan [this message]
2022-06-27  1:30         ` Yunke Cao
2022-06-27 15:43           ` Shuah Khan
2022-06-28  0:54             ` Yunke Cao
2022-06-17  1:57 ` [PATCH v2 2/2] media: vimc: documentation for lens Yunke Cao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=610bc1b9-c013-46d7-4446-5a5eab4c2569@linuxfoundation.org \
    --to=skhan@linuxfoundation.org \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=tfiga@chromium.org \
    --cc=yunkec@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.