On Mon, 2018-04-23 at 08:47 +0200, Ulf Hansson wrote:
> On 20 April 2018 at 17:53, Eugeniy Paltsev <Eugeniy.Paltsev@xxxxxxxxxxxx> wrote:
> > Hi Ulf,
> >
> > On Fri, 2018-04-20 at 09:35 +0200, Ulf Hansson wrote:
> > > [...]
> > >
> > > >
> > > > 2. Add missing stuff to support multislot mode in DesignWare MMC driver.
> > > > * Add missing slot switch to __dw_mci_start_request() function.
> > > > * Refactor set_ios function:
> > > > a) Calculate common clock which is
> > > > suitable for all slots instead of directly use clock value
> > > > provided by mmc core. We calculate common clock as the minimum
> > > > among each used slot clocks. This clock is calculated in
> > > > dw_mci_calc_common_clock() function which is called
> > > > from set_ios()
> > > > b) Disable clock only if no other slots are ON.
> > > > c) Setup clock directly in set_ios() only if no other slots
> > > > are ON. Otherwise adjust clock in __dw_mci_start_request()
> > > > function before slot switch.
> > > > d) Move timings and bus_width setup to separate funcions.
> > > > * Use timing field in each slot structure instead of common field in
> > > > host structure.
> > > > * Add locks to serialize access to registers.
> > >
> > > Sorry, but this is a hack to *try* to make multi-slot work and this
> > > isn't sufficient. There were good reasons to why the earlier
> > > non-working multi slot support was removed from dw_mmc.
> >
> > Previous multi slot implementation was removed as nobody used it and
> > nobody tested it. There are lots of mistakes in previous implementation
> > which are not related to request serialization
> > like lack of slot switch / lack of adding slot id to CIU commands / ets...
> > So obviously it was never tested and never used at real multi slot hardware.
> >
> > > Let me elaborate a bit for your understanding. The core uses a host
> > > lock (mmc_claim|release_host()) to serialize operations and commands,
> > > as to confirm to the SD/SDIO/(e)MMC specs. The above changes gives no
> > > guarantees for this. To make that work, we would need a "mmc bus lock"
> > > to be managed by the core.
> >
> > In current implementation data transfers and commands to different
> > hosts (slots) are serialized internally in the dw_mmc driver. We have
> > request queue and when .request() is called we add new request to the
> > queue. We take new request from the queue only if the previous one
> > has already finished.
>
> That isn't sufficient. The core expects all calls to *any* of the host
> ops to be serialized for one host. It does so to conform to the specs.
>
> For example it may call:
> ->set_ios()
> ->request()
> ->set_ios()
> ->request()
> ->request()
>
A bit remark for better understanding:
All card settings change are serialized too. These settings are applied
after slot switch before execution of new request for this slot.
So situations like calling any host_0 ops while another host (host_1) is active
are handled by current code.
This is example of simultaneous ops calls for both slots:
host (slot) 0 | host (slot) 1
-----------------------------------
h0->set_ios() | h1->set_ios()
h0->request() | h1->request()
h0->set_ios() | h1->set_ios()
h0->request() | h1->request()
h0->request() |
h0->request() |
h0->request() |
How it will be serialized in the mmc driver:
h0->set_ios() // h0 settings save
h1->set_ios() // h1 settings save
h0->request() // apply settings for h0 and do request
------ slot switch to h1 ------
h1->request() // apply settings for h1 and do request
h0->set_ios() // h0 settings save
h1->set_ios() // h1 settings save
------ slot switch to h0 ------
h0->request() // apply settings for h0 and do request
------ slot switch to h1 ------
h1->request() // apply settings for h1 and do request
------ slot switch to h0 ------
h0->request() // do request (no new settings to apply)
h0->request() // do request (no new settings to apply)
h0->request() // do request (no new settings to apply)
--
Eugeniy Paltsev��.n��������+%������w��{.n�����{��i��)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
