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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100115000115.GA27344@opensource.wolfsonmicro.com>
Date:	Fri, 15 Jan 2010 00:01:16 +0000
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Ben Dooks <ben-linux@...ff.org>, linux-kernel@...r.kernel.org,
	stable@...nel.org, InKi Dae <inki.dae@...sung.com>,
	Kyungmin Park <kmpark@...radead.org>,
	Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [PATCH 2.6.33] s3c-fb: Fix divide by zero and broken output

On Thu, Jan 14, 2010 at 03:06:58PM -0800, Andrew Morton wrote:

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

Through oversight, sorry - the original patch was an incremental update
to the driver rather than a reversion so I CCed the relevant maintainers
then and maintained the same CC list for the followup.

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

It's not entirely clear that there is any breakage to fix.  Certainly
the SMDK6410 I have works perfectly fine with the reversion, and I've
had a report that the hct board does too (which makes two out of the
three mainline users AFAICT).  Since none of the boards in mainline
supply a refresh parameter they'll all be failing with the current code.

Ben's analysis, which seemed reasonable to me, was that the patch is
based on a misunderstanding of the units in which the pixclk parameter
is specified.  He wrote:

| Thirdly he's changed the code to calculate the divider to assume pixclk
| is a frequency, and not a time-period. It clearly stats in the docs
| that pixclk is in picoseconds. Our clock rate is in Hz.

My understanding is that the breakage that was believed to exist was
based on the driver misbehaving if you supply the pixclk as a frequency,
which isn't surprising since that's what it's looking for.  The effect
of the patch is to change the interpretation of pixclk value so that a
refresh rate must also be specified but either way it should be possible
to specify a working configuration.

There's possibly a case to be made for changing the platform data over
to specify things differently, I'm not sufficiently familiar with the
area to say one way or another, but given that the original method works 
and existing machine are broken by the change it seems safer to revert
at least for -rc.
--
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/