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

[Kun-Fa Lin]

> > 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.

OK, I'll do that.


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

I get these debug messages after VNC viewer connected:

npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: VCD mode = 0x1000b, Hi Res mode
npcm-video f0810000.video: Digital mode: 1600 x 1200 x 2, pixelclock
161666000, bytesperline 4096
npcm-video f0810000.video: ECE open: client 1
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: VCD irq status 0x84b00ff1
npcm-video f0810000.video: offset = 0, ed_size = 1540907, gap = 12
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: VCD irq status 0x84b00ff1
npcm-video f0810000.video: offset = 0, ed_size = 1539638, gap = 12
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: VCD irq status 0x84b00ff1
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: offset = 0, ed_size = 1539638, gap = 12
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: VCD irq status 0x84b00ff1
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: offset = 0, ed_size = 1539638, gap = 12
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: VCD irq status 0x84b00ff1
npcm-video f0810000.video: offset = 0, ed_size = 1539638, gap = 12
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: VCD irq status 0x84b00ff1
npcm-video f0810000.video: offset = 0, ed_size = 1539638, gap = 12
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: VCD irq status 0x84b00ff1
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: VCD irq status 0x84b00ff1
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: VCD irq status 0x84b00ff1
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: VCD irq status 0x84b00ff1
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
npcm-video f0810000.video: VCD irq status 0x84b00ff1
npcm-video f0810000.video: Got resolution[1600x1200] -> [1600x1200], status 0
[...]


> > 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.

> > 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.

 Thank you for your feedback.

Regards,
Marvin