All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
To: "ulf.hansson@linaro.org" <ulf.hansson@linaro.org>
Cc: "jh80.chung@samsung.com" <jh80.chung@samsung.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"krzk@kernel.org" <krzk@kernel.org>,
	"Eugeniy.Paltsev@synopsys.com" <Eugeniy.Paltsev@synopsys.com>,
	"linux-snps-arc@lists.infradead.org"
	<linux-snps-arc@lists.infradead.org>,
	"kgene@kernel.org" <kgene@kernel.org>,
	"shawn.lin@rock-chips.com" <shawn.lin@rock-chips.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-samsung-soc@vger.kernel.org"
	<linux-samsung-soc@vger.kernel.org>
Subject: Re: [RFC 0/2] dw_mmc: add multislot support
Date: Fri, 20 Apr 2018 07:42:13 +0000	[thread overview]
Message-ID: <1524210132.4614.4.camel@synopsys.com> (raw)
In-Reply-To: <CAPDyKFqxZLtx_=0UU9NgBnW0Vmw=456q2FFbMBK4LKEaF-ua7A@mail.gmail.com>

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.
> 
> 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.
> 
> However, inventing a "mmc bus lock" would lead to other problems
> related to I/O scheduling for upper layers - it simply breaks. For
> example, I/O requests for one card/slot can then starve I/O requests
> reaching another card/slot.

So are you saying that multi-slot support is a no go in general or it is only
applicable to DW MMC (I really doubt that's a case)?

BTW there're other controllers that seem to support multi-slot like Atmel etc.

-Alexey

WARNING: multiple messages have this Message-ID (diff)
From: Alexey.Brodkin@synopsys.com (Alexey Brodkin)
To: linux-snps-arc@lists.infradead.org
Subject: [RFC 0/2] dw_mmc: add multislot support
Date: Fri, 20 Apr 2018 07:42:13 +0000	[thread overview]
Message-ID: <1524210132.4614.4.camel@synopsys.com> (raw)
In-Reply-To: <CAPDyKFqxZLtx_=0UU9NgBnW0Vmw=456q2FFbMBK4LKEaF-ua7A@mail.gmail.com>

Hi Ulf,

On Fri, 2018-04-20@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.
> 
> 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.
> 
> However, inventing a "mmc bus lock" would lead to other problems
> related to I/O scheduling for upper layers - it simply breaks. For
> example, I/O requests for one card/slot can then starve I/O requests
> reaching another card/slot.

So are you saying that multi-slot support is a no go in general or it is only
applicable to DW MMC (I really doubt that's a case)?

BTW there're other controllers that seem to support multi-slot like Atmel etc.

-Alexey

WARNING: multiple messages have this Message-ID (diff)
From: Alexey.Brodkin@synopsys.com (Alexey Brodkin)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 0/2] dw_mmc: add multislot support
Date: Fri, 20 Apr 2018 07:42:13 +0000	[thread overview]
Message-ID: <1524210132.4614.4.camel@synopsys.com> (raw)
In-Reply-To: <CAPDyKFqxZLtx_=0UU9NgBnW0Vmw=456q2FFbMBK4LKEaF-ua7A@mail.gmail.com>

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.
> 
> 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.
> 
> However, inventing a "mmc bus lock" would lead to other problems
> related to I/O scheduling for upper layers - it simply breaks. For
> example, I/O requests for one card/slot can then starve I/O requests
> reaching another card/slot.

So are you saying that multi-slot support is a no go in general or it is only
applicable to DW MMC (I really doubt that's a case)?

BTW there're other controllers that seem to support multi-slot like Atmel etc.

-Alexey

  reply	other threads:[~2018-04-20  7:42 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20180417121325epcas2p319c672feee4203d8b90131da461f4bd6@epcas2p3.samsung.com>
2018-04-17 12:11 ` [RFC 0/2] dw_mmc: add multislot support Eugeniy Paltsev
2018-04-17 12:11   ` Eugeniy Paltsev
2018-04-17 12:11   ` Eugeniy Paltsev
2018-04-17 12:11   ` Eugeniy Paltsev
2018-04-17 12:11   ` [RFC 1/2] dw_mmc: revert removal " Eugeniy Paltsev
2018-04-17 12:11     ` Eugeniy Paltsev
2018-04-17 12:11     ` Eugeniy Paltsev
2018-04-17 12:11     ` Eugeniy Paltsev
2018-04-17 12:11   ` [RFC 2/2] dw_mmc: add " Eugeniy Paltsev
2018-04-17 12:11     ` Eugeniy Paltsev
2018-04-17 12:11     ` Eugeniy Paltsev
2018-04-17 12:11     ` Eugeniy Paltsev
2018-04-20  7:35   ` [RFC 0/2] " Ulf Hansson
2018-04-20  7:35     ` Ulf Hansson
2018-04-20  7:35     ` Ulf Hansson
2018-04-20  7:42     ` Alexey Brodkin [this message]
2018-04-20  7:42       ` Alexey Brodkin
2018-04-20  7:42       ` Alexey Brodkin
2018-04-20  8:56       ` Ulf Hansson
2018-04-20  8:56         ` Ulf Hansson
2018-04-20  8:56         ` Ulf Hansson
2018-04-20 15:53     ` Eugeniy Paltsev
2018-04-20 15:53       ` Eugeniy Paltsev
2018-04-20 15:53       ` Eugeniy Paltsev
2018-04-23  6:47       ` Ulf Hansson
2018-04-23  6:47         ` Ulf Hansson
2018-04-23  6:47         ` Ulf Hansson
2018-04-25 13:53         ` Eugeniy Paltsev
2018-04-25 13:53           ` Eugeniy Paltsev
2018-04-25 13:53           ` Eugeniy Paltsev
2018-04-26  6:28           ` Ulf Hansson
2018-04-26  6:28             ` Ulf Hansson
2018-04-26  6:28             ` Ulf Hansson
2018-04-26 10:30   ` Jaehoon Chung
2018-04-26 10:30     ` Jaehoon Chung
2018-04-26 10:30     ` Jaehoon Chung

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1524210132.4614.4.camel@synopsys.com \
    --to=alexey.brodkin@synopsys.com \
    --cc=Eugeniy.Paltsev@synopsys.com \
    --cc=jh80.chung@samsung.com \
    --cc=kgene@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux-snps-arc@lists.infradead.org \
    --cc=shawn.lin@rock-chips.com \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.