linux-kernel - Re: [PATCH v1 07/11] mmc: mmci: move ST specific register extensions access under condition.

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: <CACRpkdbgScfdLUc64a0r4Jxq9UHCbmMiAwD_G1-LZq8B58sX7g@mail.gmail.com>
Date:	Tue, 13 May 2014 10:08:40 +0200
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc:	Russell King <linux@....linux.org.uk>,
	"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
	Chris Ball <chris@...ntf.net>,
	Ulf Hansson <ulf.hansson@...aro.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	agross@...cinc.com,
	"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>
Subject: Re: [PATCH v1 07/11] mmc: mmci: move ST specific register extensions
 access under condition.

On Tue, Apr 29, 2014 at 10:20 AM,  <srinivas.kandagatla@...aro.org> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
>
> This patch moves some of the ST specific register extensions access under
> condition, so that other SOCs like Qualcomm or ARM would not a side effect of
> writing to those reserved/different purpose bits.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
(...)

>         /* Keep ST Micro busy mode if enabled */
> -       datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
> +       if (host->hw_designer == AMBA_VENDOR_ST)
> +               datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;

Do not hard-check the hw_designer everywhere, follow the pattern
if storing special stuff in the variant data.

struct variant_data {
        u32 datactrl_mask_busymode;
(...)

static struct variant_data variant_u300 = {
        .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
static struct variant_data variant_nomadik = {
        .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
static struct variant_data variant_ux500 = {
        .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
static struct variant_data variant_ux500v2 = {
        .datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)

Then end up like this:

>         /* Keep ST Micro busy mode if enabled */
> -       datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
> +      datactrl |= host->datactrl_reg & host->vendor->datactrl_mask_busymode;

OK we know this should have been done like this from the beginning
but that is not an excuse not to fix it up now.

> -       if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
> +       if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 &&
> +           host->hw_designer == AMBA_VENDOR_ST)
>                 datactrl |= MCI_ST_DPSM_DDRMODE;

Same pattern here.

Actually I think this is only available on Ux500v2 (Ulf? Can you verify
this) so it is probably plain wrong to do it for other variants.

struct variant_data {
        u32 datactrl_ddrmode;
(...)
static struct variant_data variant_ux500v2 = {
        .datactrl_ddrmode = MCI_ST_DPSM_DDRMODE,
(...)

Then end up like this:

>        if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50)
> -                datactrl |= MCI_ST_DPSM_DDRMODE;
> +               datactrl |= host->vendor->datactrl_ddrmode;

Yours,
Linus Walleij
--
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/