| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1175818158.31131.63.camel@daplas>
Date: Fri, 06 Apr 2007 08:09:18 +0800
From: "Antonino A. Daplas" <adaplas@...il.com>
To: Alan Hourihane <alanh@...rlite.demon.co.uk>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/01] New FBDev driver for Intel Vermilion Range
On Thu, 2007-04-05 at 11:44 +0100, Alan Hourihane wrote:
> Attached is a patch against 2.6.21-rc5 which adds the Intel Vermilion
> Range support.
>
> Intel funded Tungsten Graphics to do this work.
>
> If there's any problems or updates needed to be done to get accepted,
> please let me know.
>
Preferably, add sparse annotations and compile with make C=1. I've
included possible sparse annotations (the only ones I can see) below.
>
> +
> +struct cr_sys {
> + struct vml_sys sys;
> + struct pci_dev *mch_dev;
> + struct pci_dev *lpc_dev;
> + __u32 mch_bar;
> + __u8 *mch_regs_base;
void __iomem *mch_regs_base; (sparse)
> + __u32 gpio_bar;
> + __u32 saved_panel_state;
> + __u32 saved_clock;
> +};
> +
>
> +static void crvml_panel_on(const struct vml_sys *sys)
> +{
> + const struct cr_sys *crsys = container_of(sys, struct cr_sys, sys);
> + __u32 addr = crsys->gpio_bar + CRVML_PANEL_PORT;
> + __u32 cur = inl(addr);
> +
> + if (!(cur & CRVML_PANEL_ON)) {
> + /* Make sure LVDS controller is down. */
> + if (cur & 0x00000001) {
> + cur &= ~CRVML_LVDS_ON;
> + outl(cur, addr);
> + }
> + /* Power up Panel */
> + schedule_timeout(HZ / 10);
> + cur |= CRVML_PANEL_ON;
> + outl(cur, addr);
> + }
> +
> + /* Power up LVDS controller */
> +
> + if (!(cur & CRVML_LVDS_ON)) {
> + schedule_timeout(HZ / 10);
> + outl(cur | CRVML_LVDS_ON, addr);
> + }
> +}
> +
> +static void crvml_panel_off(const struct vml_sys *sys)
> +{
> + const struct cr_sys *crsys = container_of(sys, struct cr_sys, sys);
> +
> + __u32 addr = crsys->gpio_bar + CRVML_PANEL_PORT;
> + __u32 cur = inl(addr);
> +
> + /* Power down LVDS controller first to avoid high currents */
> + if (cur & CRVML_LVDS_ON) {
> + cur &= ~CRVML_LVDS_ON;
> + outl(cur, addr);
> + }
> + if (cur & CRVML_PANEL_ON) {
> + schedule_timeout(HZ / 10);
> + outl(cur & ~CRVML_PANEL_ON, addr);
> + }
> +}
> +
> +static void crvml_backlight_on(const struct vml_sys *sys)
> +{
> + const struct cr_sys *crsys = container_of(sys, struct cr_sys, sys);
> + __u32 addr = crsys->gpio_bar + CRVML_PANEL_PORT;
> + __u32 cur = inl(addr);
> +
> + if (cur & CRVML_BACKLIGHT_OFF) {
> + cur &= ~CRVML_BACKLIGHT_OFF;
> + outl(cur, addr);
> + }
> +}
> +
> +static void crvml_backlight_off(const struct vml_sys *sys)
> +{
> + const struct cr_sys *crsys = container_of(sys, struct cr_sys, sys);
> + __u32 addr = crsys->gpio_bar + CRVML_PANEL_PORT;
> + __u32 cur = inl(addr);
> +
> + if (!(cur & CRVML_BACKLIGHT_OFF)) {
> + cur |= CRVML_BACKLIGHT_OFF;
> + outl(cur, addr);
> + }
> +}
>
Perhaps backling_on/off and panel_on/off can be moved to the backlight
subsystem?
> +
>
> +static int crvml_sys_restore(struct vml_sys *sys)
> +{
> + struct cr_sys *crsys = container_of(sys, struct cr_sys, sys);
> + __u32 *clock_reg = (__u32 *) (crsys->mch_regs_base + CRVML_REG_CLOCK);
__u32 __iomem *clock_reg = crsys->mch_regs_base + CRVML_REG_CLOCK; (sparse)
> + __u32 cur = crsys->saved_panel_state;
> +
> + if (cur & CRVML_BACKLIGHT_OFF) {
> + crvml_backlight_off(sys);
> + } else {
> + crvml_backlight_on(sys);
> + }
> +
> + if (cur & CRVML_PANEL_ON) {
> + crvml_panel_on(sys);
> + } else {
> + crvml_panel_off(sys);
> + if (cur & CRVML_LVDS_ON) {
> + ;
> + /* Will not power up LVDS controller while panel is off */
> + }
> + }
> + iowrite32(crsys->saved_clock, clock_reg);
> + ioread32(clock_reg);
> +
> + return 0;
> +}
> +
> +static int crvml_sys_save(struct vml_sys *sys)
> +{
> + struct cr_sys *crsys = container_of(sys, struct cr_sys, sys);
> + __u32 *clock_reg = (__u32 *) (crsys->mch_regs_base + CRVML_REG_CLOCK);
> +
__u32 __iomem *clock_reg = crsys->mch_regs_base + CRVML_REG_CLOCK; (sparse)
> + crsys->saved_panel_state = inl(crsys->gpio_bar + CRVML_PANEL_PORT);
> + crsys->saved_clock = ioread32(clock_reg);
> +
> + return 0;
> +}
> +
> +static int crvml_nearest_index(const struct vml_sys *sys, int clock)
> +{
> +
> + int i;
> + int cur_index;
> + int cur_diff;
> + int diff;
> +
> + cur_index = 0;
> + cur_diff = clock - crvml_clocks[0];
> + cur_diff = (cur_diff < 0) ? -cur_diff : cur_diff;
> + for (i = 1; i < crvml_num_clocks; ++i) {
> + diff = clock - crvml_clocks[i];
> + diff = (diff < 0) ? -diff : diff;
> + if (diff < cur_diff) {
> + cur_index = i;
> + cur_diff = diff;
> + }
> + }
> + return cur_index;
> +}
> +
> +static int crvml_nearest_clock(const struct vml_sys *sys, int clock)
> +{
> + return crvml_clocks[crvml_nearest_index(sys, clock)];
> +}
> +
> +static int crvml_set_clock(struct vml_sys *sys, int clock)
> +{
> + struct cr_sys *crsys = container_of(sys, struct cr_sys, sys);
> + __u32 *clock_reg = (__u32 *) (crsys->mch_regs_base + CRVML_REG_CLOCK);
>
__u32 __iomem *clock_reg = crsys->mch_regs_base + CRVML_REG_CLOCK; (sparse)
> + int index;
> + __u32 clock_val;
> +
> + index = crvml_nearest_index(sys, clock);
> +
> + if (crvml_clocks[index] != clock)
> + return -EINVAL;
> +
> + clock_val = ioread32(clock_reg) & ~CRVML_CLOCK_MASK;
> + clock_val = crvml_clock_bits[index] << CRVML_CLOCK_SHIFT;
> + iowrite32(clock_val, clock_reg);
> + ioread32(clock_reg);
> +
> + return 0;
> +}
> +
> +
> +static int __init crvml_init(void)
> +{
> + int err = 0;
> + struct vml_sys *sys;
> + struct cr_sys *crsys;
> +
> + crsys = (struct cr_sys *)kmalloc(sizeof(*crsys), GFP_KERNEL);
> +
> + if (!crsys)
> + return -ENOMEM;
> +
> + sys = &crsys->sys;
> + err = crvml_sysinit(crsys);
> + if (err) {
> + kfree(crsys);
> + return err;
> + }
> +
> + sys->destroy = crvml_sys_destroy;
> + sys->subsys = crvml_subsys;
> + sys->save = crvml_sys_save;
> + sys->restore = crvml_sys_restore;
> + sys->prog_clock = crvml_false;
> + sys->set_clock = crvml_set_clock;
> + sys->has_panel = crvml_true;
Hmm...
> +
> +#ifndef FB_BLANK_UNBLANK
> +#define FB_BLANK_UNBLANK VESA_NO_BLANKING
> +#endif
> +#ifndef FB_BLANK_NORMAL
> +#define FB_BLANK_NORMAL VESA_NO_BLANKING + 1
> +#endif
> +#ifndef FB_BLANK_VSYNC_SUSPEND
> +#define FB_BLANK_VSYNC_SUSPEND VESA_VSYNC_SUSPEND + 1
> +#endif
> +#ifndef FB_BLANK_HSYNC_SUSPEND
> +#define FB_BLANK_HSYNC_SUSPEND VESA_HSYNC_SUSPEND + 1
> +#endif
> +#ifndef FB_BLANK_POWERDOWN
> +#define FB_BLANK_POWERDOWN VESA_POWERDOWN + 1
> +#endif
Since this driver going to be part of the kernel, the above is redundant,
you don't need it.
>
> +static int __devinit vml_pci_probe(struct pci_dev *dev,
> + const struct pci_device_id *id)
> +{
> + struct vml_info *vinfo;
> + struct fb_info *info;
> + struct vml_par *par;
> + int err = 0;
> +
> + par = kmalloc(sizeof(*par), GFP_KERNEL);
> + if (par == NULL)
> + return -ENOMEM;
> +
> + vinfo = kmalloc(sizeof(*vinfo), GFP_KERNEL);
You can use kzalloc instead of kmalloc.
> + if (vinfo == NULL) {
> + err = -ENOMEM;
> + goto out_err_0;
> + }
> +
> + memset(vinfo, 0, sizeof(*vinfo));
> + memset(par, 0, sizeof(*par));
> +
You can also use framebuffer_alloc()/framebuffer_release() for
struct fb_info and for par. But if it is not possible,
set info->device = dev->dev so your driver can show up in sysfs.
> + vinfo->par = par;
> + vinfo->pipe = 0;
> + par->mutex = &global_mutex;
> + INIT_LIST_HEAD(&par->gpu.head);
> + par->gpu_list = &global_gpu_list;
> + par->vdc = dev;
> + atomic_set(&par->refcount, 1);
> +
> + switch (id->device) {
> + case VML_DEVICE_VDC:
> + if ((err = vmlfb_get_gpu(par)))
> + goto out_err_1;
> + if ((err = pci_enable_device(dev)))
> + goto out_err_1;
> + pci_set_drvdata(dev, &vinfo->info);
> + break;
> + default:
> + err = -ENODEV;
> + goto out_err_1;
> + break;
> + }
> +
> + info = &vinfo->info;
> + info->flags = FBINFO_PARTIAL_PAN_OK;
Since your driver is tristate, do
info->flags = FBINFO_DEFAULT | FBINFO_PARTIAL_PAN_OK;
I will probably use this flag later to differentiate driver modules
to fix a bug in the logo drawing code.
You may also wish to add FBINFO_HW_ACCEL_YPAN for faster text scrolling.
>
> +
> +static void vml_wait_vblank(struct vml_info *vinfo)
> +{
> + schedule_timeout(HZ / 40);
>
This is not really wait for vblank, is it?
> +
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,16)
> +static int vmlfb_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
> + unsigned long arg, struct fb_info *info)
> +#else
> +static int vmlfb_ioctl(struct fb_info *info, unsigned int cmd,
> + unsigned long arg)
> +#endif
>
Similarly, you can remove the above version checks.
Tony
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists