Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: X14JhFRFGlxO715+HiWYBlY97qarQ8gibmEDoqlevPhhrK0jjeR6zbyIwtehT2EiR3sYVgvYHy
 dSZ72H+4n8d1PpTIlBektQ7bVtSp8ZOOjzRKSRc3Gi0XhZIxlYimxaMeFpEgkqY4ANv4g+SEAv
 klHNOrpt3s9uuSXckFZU98XTSLDvIiN3ZAHB5k6S4kYrOkxcG2AYEsomjdHKjdBhg0i42KEVl1
 7RkWN3Z/9iH2BjtUIXRhk/ZVS7Gdv+HQncNVoKSygaF9XT3pNI74I6lhH+EmKoIjaA6qt565C2
 uw4=
From:   Avri Altman <Avri.Altman@wdc.com>
To:     Ulf Hansson <ulf.hansson@linaro.org>,
        "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
        Adrian Hunter <adrian.hunter@intel.com>
CC:     Linus Walleij <linus.walleij@linaro.org>,
        Wolfram Sang <wsa+renesas@sang-engineering.com>,
        Shawn Lin <shawn.lin@rock-chips.com>,
        Masami Hiramatsu <masami.hiramatsu@linaro.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 2/2] mmc: core: Add support for cache ctrl for SD cards
Thread-Topic: [PATCH 2/2] mmc: core: Add support for cache ctrl for SD cards
Thread-Index: AQHXQohQmJ5aCkIH3kCczJUde/3xEqrcbNXw
Date:   Mon, 10 May 2021 09:10:52 +0000
Message-ID: <DM6PR04MB6575E3D63982278D2F1CA022FC549@DM6PR04MB6575.namprd04.prod.outlook.com>
References: <20210506145829.198823-1-ulf.hansson@linaro.org>
 <20210506145829.198823-3-ulf.hansson@linaro.org>
In-Reply-To: <20210506145829.198823-3-ulf.hansson@linaro.org>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 60b73d47-c1c6-4350-63e2-08d9139382e4
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 May 2021 09:10:52.6388
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: aiFB0rfpOun3BM7HGnSPpTs90R1RyLLjfkON3E6Q2JIybbmn8SU3IF0XLw9SOvcMM+Blbod2MnI2T/rMwJzd6A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR04MB7003
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> +static int sd_enable_cache(struct mmc_card *card)
> +{
> +       u8 *reg_buf;
> +       int err;
> +
> +       reg_buf =3D kzalloc(512, GFP_KERNEL);
> +       if (!reg_buf)
> +               return -ENOMEM;
> +
> +       /*
> +        * Set the Cache Enable bit in the performance enhancement regist=
er at
> +        * 260 bytes offset.
> +        */
> +       err =3D sd_write_ext_reg(card, card->ext_perf.fno, card->ext_perf=
.page,
> +                              card->ext_perf.offset + 260, 0x1);
> +       if (err) {
> +               pr_warn("%s: error %d writing Cache Enable bit\n",
> +                       mmc_hostname(card->host), err);
> +               goto out;
> +       }
> +
> +       err =3D mmc_poll_for_busy(card,
> SD_WRITE_EXTR_SINGLE_TIMEOUT_MS, false,
> +                               MMC_BUSY_EXTR_SINGLE);
I think 1sec is for flush cache, but I guess it makes sense to use it here =
as well.

> +       if (!err)
> +               card->ext_perf.feature_enabled |=3D SD_EXT_PERF_CACHE;
Maybe=20
If (err)
    card->ext_perf.feature_enabled &=3D ~SD_EXT_PERF_CACHE;

and move to out: to catch the sd_write_ext_reg err ?

> +
> +out:
> +       kfree(reg_buf);
> +       return err;
> +}
> +
>  /*
>   * Handle the detection and initialisation of a card.
>   *
> @@ -1442,6 +1531,13 @@ static int mmc_sd_init_card(struct mmc_host
> *host, u32 ocr,
>                         goto free_card;
>         }
>=20
> +       /* Enable internal SD cache if supported. */
> +       if (card->ext_perf.feature_support & SD_EXT_PERF_CACHE) {
> +               err =3D sd_enable_cache(card);
> +               if (err)
> +                       goto free_card;
If cache enablement failed, is it worthwhile to bail out?
Maybe disabling the cache with the appropriate message is enough?

> +       }
> +
>         if (host->cqe_ops && !host->cqe_enabled) {
>                 err =3D host->cqe_ops->cqe_enable(host, card);
>                 if (!err) {
> @@ -1694,6 +1790,8 @@ static const struct mmc_bus_ops mmc_sd_ops =3D {
>         .alive =3D mmc_sd_alive,
>         .shutdown =3D mmc_sd_suspend,
>         .hw_reset =3D mmc_sd_hw_reset,
> +       .cache_enabled =3D sd_cache_enabled,
> +       .flush_cache =3D sd_flush_cache,
>  };

I would expect 2 more patches in this series:
 - flush cache on power down
 - cache disablement events?

Thanks,
Avri