linux-kernel - Re: [PATCH V6 06/24] mmc: core: Support UHS-II card control and access

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: <CAPDyKFquNQ4GpanTDpbgheYVL9N9GJCa60p0iqN6R7ydjYy3fw@mail.gmail.com>
Date:   Mon, 27 Feb 2023 16:19:54 +0100
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Victor Shih <victorshihgli@...il.com>
Cc:     adrian.hunter@...el.com, linux-mmc@...r.kernel.org,
        linux-kernel@...r.kernel.org, benchuanggli@...il.com,
        HL.Liu@...esyslogic.com.tw, Greg.tu@...esyslogic.com.tw,
        takahiro.akashi@...aro.org, dlunev@...omium.org,
        Victor Shih <victor.shih@...esyslogic.com.tw>,
        Jason Lai <jason.lai@...esyslogic.com.tw>
Subject: Re: [PATCH V6 06/24] mmc: core: Support UHS-II card control and access

[...]

> > > +}
> > > +
> > > +/*
> > > + * Mask off any voltages we don't support and select
> > > + * the lowest voltage
> > > + */
> > > +u32 sd_uhs2_select_voltage(struct mmc_host *host, u32 ocr)
> > > +{
> > > +       int bit;
> > > +       int err;
> > > +
> > > +       /*
> > > +        * Sanity check the voltages that the card claims to
> > > +        * support.
> > > +        */
> > > +       if (ocr & 0x7F) {
> > > +               dev_warn(mmc_dev(host), "card claims to support voltages below defined range\n");
> > > +               ocr &= ~0x7F;
> > > +       }
> > > +
> > > +       ocr &= host->ocr_avail;
> > > +       if (!ocr) {
> > > +               dev_warn(mmc_dev(host), "no support for card's volts\n");
> > > +               return 0;
> > > +       }
> > > +
> > > +       if (host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) {
> > > +               bit = ffs(ocr) - 1;
> > > +               ocr &= 3 << bit;
> > > +               /* Power cycle */
> > > +               err = sd_uhs2_power_off(host);
> > > +               if (err)
> > > +                       return 0;
> > > +               err = sd_uhs2_reinit(host);
> >
> > As also pointed out by Adrian, this path becomes circular and doesn't
> > work as expected.
> >
> > At this point, I would prefer to keep things as simple as possible, so
> > I suggest that we drop this path entirely for UHS2.
> >
>
> Can we use sd_uhs2_power and sd_uhs2_power_up instead of the
> sd_uhs2_reinit after I reference the mmc_power_cycle function?

Sorry, but I don't quite understand how that would help to make this work!?

Anyway, please have a look at the corresponding comment below too. The
point is, I think it's better if we can update (or refactor) and make
use of mmc_select_voltage() instead.

>
> > > +               if (err)
> > > +                       return 0;
> > > +       } else {
> > > +               bit = fls(ocr) - 1;
> > > +               ocr &= 3 << bit;
> >
> > Note that, in mmc_select_voltage() the corresponding code has been
> > updated in commit 39a72dbfe188 ("mmc: core: properly select voltage
> > range without power cycle") that fixes a real problem for us.
> >
> > By taking the above observations into consideration, it looks like it
> > may be better to reuse mmc_select_voltage() for UHS-II after all. To
> > prevent the power cycle path for UHS-II, we can just add a check for
> > the ios->timings in there. That should work, I think.
> >
> > > +               if (bit != host->ios.vdd)
> > > +                       dev_warn(mmc_dev(host), "exceeding card's volts\n");
> > > +       }
> > > +
> > > +       return ocr;
> > > +}
> > > +

[...]

Kind regards
Uffe