Re: [PATCH 2.6.33] s3c-fb: Fix divide by zero and broken output

From: Andrew Morton
Date: Thu Jan 14 2010 - 18:08:37 EST


On Thu, 14 Jan 2010 10:26:23 +0000
Mark Brown <broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:

> Commit 600ce1a0faafeed1ce6bcfd421bc040b941cbbc1. ("fix clock setting
> for Samsung SoC Framebuffer") introduced a mandatory refresh parameter
> to the platform data for the S3C framebuffer but did not introduce any
> validation code, causing existing platforms (none of which have refresh
> set) to divide by zero whenever the framebuffer is configured,
> generating warnings and unusable output.
>
> Ben Dooks noted several problems with the patch:
>
> - The platform data supplies the pixclk directly and should already
> have taken care of the refresh rate.
> - The addition of a window ID parameter doesn't help since only the
> root framebuffer can control the pixclk.
> - pixclk is specified in picoseconds (rather than Hz) as the patch
> assumed.
>
> and suggests reverting the commit so do that. Without fixing this no
> mainline user of the driver will produce output.
>

Why on earth would you revert someone's patch and not tell them that
you're doing it?

This reversion will presumably break the Samsung SoC Framebuffer again.
How about we not do that?

> @@ -358,6 +359,8 @@ static int s3c_fb_set_par(struct fb_info *info)
>
> data = var->xres * var->yres;
>
> + u32 osdc_data = 0;
> +
> osdc_data = VIDISD14C_ALPHA1_R(0xf) |
> VIDISD14C_ALPHA1_G(0xf) |
> VIDISD14C_ALPHA1_B(0xf);

The reversion restores this incorrect c99-style declaration.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/