linux-kernel - Re: [PATCH] mmc: dw_mmc: rewrite CLKDIV computation

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: <CAD=FV=WRJhr1+OS5Be2Vz6J=Rgk3UQAzwjY3FuNSjvMbeR3uUg@mail.gmail.com>
Date:	Wed, 27 Mar 2013 08:07:32 -0700
From:	Doug Anderson <dianders@...omium.org>
To:	Grant Grundler <grundler@...omium.org>
Cc:	Chris Ball <cjb@...top.org>, Will Newton <will.newton@...il.com>,
	Seungwon Jeon <tgih.jun@...sung.com>,
	Bing Zhao <bzhao@...vell.com>,
	Jaehoon Chung <jh80.chung@...sung.com>,
	Ashok Nagarajan <asnagarajan@...omium.org>,
	Olof Johansson <olof@...om.net>, linux-mmc@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mmc: dw_mmc: rewrite CLKDIV computation

Grant,

Thanks for posting!  See below...

On Tue, Mar 26, 2013 at 3:50 PM, Grant Grundler <grundler@...omium.org> wrote:
> Last year Seungwon Jeon (Samsung) fixed a bug in CLKDIV computation.
> But when debugging a related issue (http://crbug.com/221828) I found
> the code unreadable.  This rewrite simplifies the computation and
> explains each step.

The fact that you mention a bug here is confusing.  I think this patch
has nothing to do with fixing that bug and is just a readability
patch.  Is that correct?  Please add to the description if so and
maybe remove unrelated comment about the bug.


> +                       /* don't overclock due to resolution of HW */
> +                       if (div & 1)
> +                               div++;
> +
> +                       /* See 6.2.3 CLKDIV in "Mobile Storage Host Databook"
> +                        * Look for dwc_mobile_storage_db.pdf from Synopsys.
> +                        * CLKDIV value 0 means divisor 1, val 1 -> 2, ...

You are quoting exynos5250 docs here.  This driver is used for more
than just exynos and so this could be confusing to users on other
platforms.


>                          */
> -                       div += 1;
> -
> -               div = (host->bus_hz != slot->clock) ? DIV_ROUND_UP(div, 2) : 0;
> +                       div /= 2;

It does look like you're re-implementing DIV_ROUND_UP.  Maybe replace
your "if" test and division with just a DIV_ROUND_UP?


-Doug
--
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/