Re: [PATCH] zr36067: Debugging cleanups (updated again)

Hi,

[ oops, forgot to reply-all, setting back all CCs here ]

On 5/28/07, Trent Piepho <[email protected]> wrote:
>
> You can set the frame buffer memory address to anything you want, that
> could be bad.  You can mmap /dev/kmem and do all kinds of bad things.
> There's a million bad things you can do.


For a framebuffer, there's a reason it's not just w, but sysadmin()
protected. Per setting, I'd prefer to be as correct and safe as possible.
Let's go through all of them:

- card, encoder, decoder, vidmem, video_nr:: pointless, b/c only used on
module init (specifically zr36057_init(), which is therefore marked as
__devinit). Exporting it would make people expect that it's settable, and it
isn't.

- default_input/norm could be exported as 0444, but anything else would not
be correct, because maybe the variable is 0644, the _setting_ isn't, and
that's really what matters. Changing the variable won't do a thing. Unless
sysfs has any setting to mark variables as init-only (is it possible to
restore sysfs properties on module-init?).

- pass_through: sort of 0644, although it's only after a device-close, not
instant-apply. If we make this 0644, on the long-run it should be instant
apply using a callback from sysfs.

{jpg,v4l}_{bufsize,nbufs}: I think v4l_bufsize/nbufs are per device
open-close, so same "objection" as above, although I'm ok with it with some
documentation that it is not instant-apply. jpg-*, as said, introduces
races. If possible, this would have to be sysadmin protected, w-protection
alone is not enough since most people _need_ w access to the device to be
able to write data to it (e.g. using lavplay).

debug: should obviously be in sysfs and is already instant-apply.

> I was able to change the driver's kmalloc() limit from 128k to 1024k, and
> it
> > > appeared to be able to allocate the larger memory region fine.  This
> is an
> > > old driver, and the rules wrt large kmalloc()s may have changed since
> the
> > > 128k limit was put in.
> >
> >
> > I think recent hardware (e.g. AMD, Intel probably has something similar
> > since they always do the same thing) can allocate larger pieces of
> memory
> > contiguously. For other stuff, you may want to ensure the memory is
> actually
>
> Recent hardware has an iommu unit.  I think this might be able to make
> non-contiguous memory look like it is to a PCI device.  My motherboard is
> from 2001, so I don't think I have any such fancy hardware.  While it
> appeared to work fine and not crash anything, you're right that is
> possible
> that the memory wasn't contiguous.
>

If it works, it's good enough for me. If your computer was about to crash,
you'd notice soon enough (plus that the device-memory wouldn't match the
pointer in your kernel for the later parts of the area, which means you'd
get a garbled picture at the bottom)... :-). Again, if the error paths are
tested and working, it's fine with me. :-).

Ronald

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

_______________________________________________
Mjpeg-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mjpeg-developer