Hi,
A quick question: does anyone know of any hardware that actually
implements multiple slots per host on the dw_mmc controller?
When working on the driver I often find myself running into questions
about how things should work on the theoretical "multiple slot" dw_mmc
implementation. ...and I often find bugs where things couldn't
possibly work for a multiple slot implementation.
As an example, recently I sent up (870556a mmc: dw_mmc: Handle late
vmmc regulators with EPROBE_DEFER). Before that patch the host-global
"host->vmmc" was being set once per slot. Said another way, if slot 0
set host->mmc to ABC, then slot 1 would come along and clobber
host->mmc to DEF. After my patch the dw_mmc code assumes one vmmc
regulator per host. As Tomasz pointed out, that might not be so good
(*), but it was better than what was there before. Maybe all
multislot implementations don't use vmmc?
Another example, though, is dw_mci_init_slot(). It calls this once per slot:
mmc_alloc_host(sizeof(struct dw_mci_slot), host->dev);
...where "host->dev" is shared among all the slots. I haven't tested
it (since I don't have multislot), but that doesn't seem wise to me.
Reading through mmc_alloc_host(), it looks like it assumes that this
host owns host->dev and it feels free to clobber variables. Sound
familiar?
I guess my overall question is: if there are no actual implementations
of multislot, shouldn't we kill it and simplify the code a whole lot?
If someone out there has a real multislot device they can step back in
and do it more correctly?
Of course we need to find someone to actually go through and do the
killing of multislot, but finding that person might be easier if there
was some agreement that it was good to do.
Cheers? Flames?
-Doug
---
(*) On the other hand, maybe my patch is OK. A reasonable way that
multi-slot power might work is with a since vmmc and then it is
multiplexed through with PWREN? All I know is that exynos says
"PWREN" is not implemented and that exynos only has one slot per host.
On Thu, Aug 8, 2013 at 5:16 PM, Doug Anderson <[email protected]> wrote:
> I guess my overall question is: if there are no actual implementations
> of multislot, shouldn't we kill it and simplify the code a whole lot?
> If someone out there has a real multislot device they can step back in
> and do it more correctly?
>
> Of course we need to find someone to actually go through and do the
> killing of multislot, but finding that person might be easier if there
> was some agreement that it was good to do.
There clearly seems to be no in-tree users of multislot. If someone
new comes in, we have the code in the history and can revert the
removal (or at least use it as reference for re-introduction).
I vote for removing it. It adds really annoying complexity for
something that nobody uses.
-Olof
Hi,
On Fri, Aug 09 2013, Olof Johansson wrote:
> On Thu, Aug 8, 2013 at 5:16 PM, Doug Anderson <[email protected]> wrote:
>
>> I guess my overall question is: if there are no actual implementations
>> of multislot, shouldn't we kill it and simplify the code a whole lot?
>> If someone out there has a real multislot device they can step back in
>> and do it more correctly?
>>
>> Of course we need to find someone to actually go through and do the
>> killing of multislot, but finding that person might be easier if there
>> was some agreement that it was good to do.
>
> There clearly seems to be no in-tree users of multislot. If someone
> new comes in, we have the code in the history and can revert the
> removal (or at least use it as reference for re-introduction).
>
> I vote for removing it. It adds really annoying complexity for
> something that nobody uses.
I agree with Olof, for what it's worth. (The maintainers of the
driver are Jaehoon and Seungwon, though.)
Thanks,
- Chris.
--
Chris Ball <[email protected]> <http://printf.net/>
On Fri, August 09, 2013, Chris Ball wrote:
> On Fri, Aug 09 2013, Olof Johansson wrote:
> > On Thu, Aug 8, 2013 at 5:16 PM, Doug Anderson <[email protected]> wrote:
> >
> >> I guess my overall question is: if there are no actual implementations
> >> of multislot, shouldn't we kill it and simplify the code a whole lot?
> >> If someone out there has a real multislot device they can step back in
> >> and do it more correctly?
> >>
> >> Of course we need to find someone to actually go through and do the
> >> killing of multislot, but finding that person might be easier if there
> >> was some agreement that it was good to do.
> >
> > There clearly seems to be no in-tree users of multislot. If someone
> > new comes in, we have the code in the history and can revert the
> > removal (or at least use it as reference for re-introduction).
> >
> > I vote for removing it. It adds really annoying complexity for
> > something that nobody uses.
>
> I agree with Olof, for what it's worth. (The maintainers of the
> driver are Jaehoon and Seungwon, though.)
I feel like there is no actual use case for that though origin Synopsys IP supports.
Multi-slot might be not useful in terms of performance because shared bus should be allowed.
(At least this is the way I see it, though)
As Exynos's host does so, other hosts which are introduced in Linux seems use one card per host.
If it's really not found now, I could agree on this topic.
Thanks,
Seungwon Jeon
Hi,
On Thu, Aug 8, 2013 at 8:18 PM, Seungwon Jeon <[email protected]> wrote:
> On Fri, August 09, 2013, Chris Ball wrote:
>> On Fri, Aug 09 2013, Olof Johansson wrote:
>> > On Thu, Aug 8, 2013 at 5:16 PM, Doug Anderson <[email protected]> wrote:
>> >
>> >> I guess my overall question is: if there are no actual implementations
>> >> of multislot, shouldn't we kill it and simplify the code a whole lot?
>> >> If someone out there has a real multislot device they can step back in
>> >> and do it more correctly?
>> >>
>> >> Of course we need to find someone to actually go through and do the
>> >> killing of multislot, but finding that person might be easier if there
>> >> was some agreement that it was good to do.
>> >
>> > There clearly seems to be no in-tree users of multislot. If someone
>> > new comes in, we have the code in the history and can revert the
>> > removal (or at least use it as reference for re-introduction).
>> >
>> > I vote for removing it. It adds really annoying complexity for
>> > something that nobody uses.
>>
>> I agree with Olof, for what it's worth. (The maintainers of the
>> driver are Jaehoon and Seungwon, though.)
>
> I feel like there is no actual use case for that though origin Synopsys IP supports.
> Multi-slot might be not useful in terms of performance because shared bus should be allowed.
> (At least this is the way I see it, though)
> As Exynos's host does so, other hosts which are introduced in Linux seems use one card per host.
> If it's really not found now, I could agree on this topic.
This all sounds very promising. Certainly we should wait a little
longer to see if others find / respond to this thread, but otherwise
we can go ahead?
It's possible to do this in somewhat small steps. I think the first
step is to remove num_slots and remove all loops over num_slots. That
actually sounds pretty easy/small, though it will touch a lot of code.
After that we can try to move things out of the separate slot
structure, I think. That might be a bit of a bigger change. I can
keep that as a back burner task, but I wouldn't object at all to
someone else doing it! ;)
The big question, though, is what to do about device tree bindings
(cringe). Really bus-width, wp-gpios, and disable-wp ought to be
promoted up and we should remove the "slot" subnode. ...but that of
course breaks the stable API.
-Doug