Re: [PATCH v10 7/7] media: nuvoton: Add driver for NPCM video capture and encode engine

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Kun-Fa,


Am 29.12.22 um 09:55 schrieb Kun-Fa Lin:

Add driver for Video Capture/Differentiation Engine (VCD) and Encoding
Compression Engine (ECE) present on Nuvoton NPCM SoCs. The VCD can
capture and differentiate video data from digital or analog sources,

“differentiate video data” sounds uncommon to me. Am I just ignorant or
is there a better term?

How about "The VCD can capture a frame from digital video input and
compare two frames in memory, then the ECE will compress the frame
data into HEXTITLE format", is it better?

Yes, I prefer your suggestion.

Wich VNC viewer and version?

I used RealVNC version 6.21.1109 to test.
Do I have to add this information in the commit message?

I do not think there are rules, but I prefer to have the test environment and procedure information in the commit message in case there are problems, and you want to reproduce things.

Maybe also paste the new dev_ log messages you get from one boot.

Do you mean dev_info/dev_debug messages of the driver?
If yes, I get these messages from one boot (only dev_info will be
printed in default):

npcm-video f0810000.video: assigned reserved memory node framebuffer@0x33000000
npcm-video f0810000.video: NPCM video driver probed

Yes, that is what I meant. Maybe even the debug messages.

It’d be great if you noted the datasheet name and revision.

I can note the datasheet name and revision in the commit message but
can't provide the file link because it is not public.
Is it ok with you?

Yes, that would be ok with me.

+static unsigned int npcm_video_ece_get_ed_size(struct npcm_video *video,
+                                            u32 offset, u8 *addr)
+{
+     struct regmap *ece = video->ece.regmap;
+     u32 size, gap, val;

Using a fixed size type for variables not needing is, is actually not an
optimization [1]. It’d be great, if you went over the whole change-set
to use the non-fixed types, where possible. (You can also check the
difference with `scripts/bloat-o-meter`.

So what I have to do is replace "u8/u16/u32" with "unsigned int" for
generic local variables as much as possible.
Is my understanding correct?

Yes, I would say so.

+MODULE_AUTHOR("Joseph Liu<kwliu@xxxxxxxxxxx>");
+MODULE_AUTHOR("Marvin Lin<kflin@xxxxxxxxxxx>");

Please add a space before the <.

+MODULE_DESCRIPTION("Driver for Nuvoton NPCM Video Capture/Encode Engine");
+MODULE_LICENSE("GPL");

Not GPL v2?

I'll correct them in the next patch.

Awesome.


Kind regards,

Paul



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux