Re: [GIT PULL 1/2] SOCFPGA updates for 3.18

Re: [GIT PULL 1/2] SOCFPGA updates for 3.18

[Arnd Bergmann]

On Tuesday 09 September 2014 16:17:56 Pavel Machek wrote:
> > Jaehoon Chung (1):
> >       ARM: dts: socfpga: unuse the slot-node and deprecate the supports-highspeed for dw-mmc
> > 
> 
> This patch is a bad idea. It removes support for two mmc cards on a
> single controller -- configuration hardware supports and configuration
> that allows using u-SD card on mcvevk board.
> 

Your objection comes too late, and to the wrong patch, since the
driver and all other users have already been changed. We had a long
discussion about this when the issue first came up, and we could
not find any example of dw-mmc actually being used in a scenario
with one controller that has multiple devices attached.

Apparently every user out there instead uses multiple controller
instances instead. Are you sure that the socfpga implementation
is an exception from this?

	Arnd

Re: [GIT PULL 1/2] SOCFPGA updates for 3.18

[Pavel Machek]

On Tue 2014-09-09 17:02:34, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 16:17:56 Pavel Machek wrote:
> > > Jaehoon Chung (1):
> > >       ARM: dts: socfpga: unuse the slot-node and deprecate the supports-highspeed for dw-mmc
> > > 
> > 
> > This patch is a bad idea. It removes support for two mmc cards on a
> > single controller -- configuration hardware supports and configuration
> > that allows using u-SD card on mcvevk board.
> > 
> 
> Your objection comes too late, and to the wrong patch, since the
> driver and all other users have already been changed. We had a long
> discussion about this when the issue first came up, and we could
> not find any example of dw-mmc actually being used in a scenario
> with one controller that has multiple devices attached.

Well, this is not first time I raised this. 3.17 is not yet out, so we
still have chance to fix regressions without major fuss.

> Apparently every user out there instead uses multiple controller
> instances instead. Are you sure that the socfpga implementation
> is an exception from this?

Marek Vasut has the hardware. His board apprently has uSD and eMMC,
and I believe it has just one controller. I'll try to get details.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [GIT PULL 1/2] SOCFPGA updates for 3.18

[Ulf Hansson]

On 9 September 2014 23:02, Pavel Machek <pavel@denx.de> wrote:
> On Tue 2014-09-09 17:02:34, Arnd Bergmann wrote:
>> On Tuesday 09 September 2014 16:17:56 Pavel Machek wrote:
>> > > Jaehoon Chung (1):
>> > >       ARM: dts: socfpga: unuse the slot-node and deprecate the supports-highspeed for dw-mmc
>> > >
>> >
>> > This patch is a bad idea. It removes support for two mmc cards on a
>> > single controller -- configuration hardware supports and configuration
>> > that allows using u-SD card on mcvevk board.
>> >
>>
>> Your objection comes too late, and to the wrong patch, since the
>> driver and all other users have already been changed. We had a long
>> discussion about this when the issue first came up, and we could
>> not find any example of dw-mmc actually being used in a scenario
>> with one controller that has multiple devices attached.
>
> Well, this is not first time I raised this. 3.17 is not yet out, so we
> still have chance to fix regressions without major fuss.
>
>> Apparently every user out there instead uses multiple controller
>> instances instead. Are you sure that the socfpga implementation
>> is an exception from this?
>
> Marek Vasut has the hardware. His board apprently has uSD and eMMC,
> and I believe it has just one controller. I'll try to get details.
>

Just wanted to highlight some of the reasons to why the earlier
discussion we have had, came to the conclusion to remove the slot
node.

1) The mmc core don't support multiple cards attached to the same
host, it never has. Also, I am not aware of any requests that
suggested us to add it. Due to obvious reasons from performance
perspective it's simply a really bad idea, that's likely also why
there haven't been any requests for it.

2) On the host level, the support for handle multiple slots in DT for
dw-mmc has been broken. While dw-mmc parsed the DT nodes for slots, it
screwed up configurations. Thus the support for slots have never
worked as expected from DT point of view.

Kind regards
Uffe

Re: [GIT PULL 1/2] SOCFPGA updates for 3.18

[Pavel Machek]

On Wed 2014-09-10 09:13:27, Ulf Hansson wrote:
> On 9 September 2014 23:02, Pavel Machek <pavel@denx.de> wrote:
> > On Tue 2014-09-09 17:02:34, Arnd Bergmann wrote:
> >> On Tuesday 09 September 2014 16:17:56 Pavel Machek wrote:
> >> > > Jaehoon Chung (1):
> >> > >       ARM: dts: socfpga: unuse the slot-node and deprecate the supports-highspeed for dw-mmc
> >> > >
> >> >
> >> > This patch is a bad idea. It removes support for two mmc cards on a
> >> > single controller -- configuration hardware supports and configuration
> >> > that allows using u-SD card on mcvevk board.
> >> >
> >>
> >> Your objection comes too late, and to the wrong patch, since the
> >> driver and all other users have already been changed. We had a long
> >> discussion about this when the issue first came up, and we could
> >> not find any example of dw-mmc actually being used in a scenario
> >> with one controller that has multiple devices attached.
> >
> > Well, this is not first time I raised this. 3.17 is not yet out, so we
> > still have chance to fix regressions without major fuss.
> >
> >> Apparently every user out there instead uses multiple controller
> >> instances instead. Are you sure that the socfpga implementation
> >> is an exception from this?
> >
> > Marek Vasut has the hardware. His board apprently has uSD and eMMC,
> > and I believe it has just one controller. I'll try to get details.
> >
> 
> Just wanted to highlight some of the reasons to why the earlier
> discussion we have had, came to the conclusion to remove the slot
> node.
> 
> 1) The mmc core don't support multiple cards attached to the same
> host, it never has. Also, I am not aware of any requests that
> suggested us to add it. Due to obvious reasons from performance
> perspective it's simply a really bad idea, that's likely also why
> there haven't been any requests for it.

Well, that's Linux problem, right? (And why is it bad idea, btw? The
bandwidth will be shared between the controllers, ok, that does not
sound too bad.)

> 2) On the host level, the support for handle multiple slots in DT for
> dw-mmc has been broken. While dw-mmc parsed the DT nodes for slots, it
> screwed up configurations. Thus the support for slots have never
> worked as expected from DT point of view.

Well, DT is supposed to describe the hardware. From your description,
it seems that linux does not support two slots on one controller and
DT parsing code basically ignores the slots. (Logical, if it can't
support two slots).

So now we are breaking DT description due to Linux limitations. Which

a) makes it hard for any other os not having same limitation

b) makes it hard for people to fix the limitation

c) does not really solve anything

d) breaks backward compatibility with old dts

If nothing, d) should be argument not to do this.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [GIT PULL 1/2] SOCFPGA updates for 3.18

[Ulf Hansson]

On 10 September 2014 10:33, Pavel Machek <pavel@denx.de> wrote:
> On Wed 2014-09-10 09:13:27, Ulf Hansson wrote:
>> On 9 September 2014 23:02, Pavel Machek <pavel@denx.de> wrote:
>> > On Tue 2014-09-09 17:02:34, Arnd Bergmann wrote:
>> >> On Tuesday 09 September 2014 16:17:56 Pavel Machek wrote:
>> >> > > Jaehoon Chung (1):
>> >> > >       ARM: dts: socfpga: unuse the slot-node and deprecate the supports-highspeed for dw-mmc
>> >> > >
>> >> >
>> >> > This patch is a bad idea. It removes support for two mmc cards on a
>> >> > single controller -- configuration hardware supports and configuration
>> >> > that allows using u-SD card on mcvevk board.
>> >> >
>> >>
>> >> Your objection comes too late, and to the wrong patch, since the
>> >> driver and all other users have already been changed. We had a long
>> >> discussion about this when the issue first came up, and we could
>> >> not find any example of dw-mmc actually being used in a scenario
>> >> with one controller that has multiple devices attached.
>> >
>> > Well, this is not first time I raised this. 3.17 is not yet out, so we
>> > still have chance to fix regressions without major fuss.
>> >
>> >> Apparently every user out there instead uses multiple controller
>> >> instances instead. Are you sure that the socfpga implementation
>> >> is an exception from this?
>> >
>> > Marek Vasut has the hardware. His board apprently has uSD and eMMC,
>> > and I believe it has just one controller. I'll try to get details.
>> >
>>
>> Just wanted to highlight some of the reasons to why the earlier
>> discussion we have had, came to the conclusion to remove the slot
>> node.
>>
>> 1) The mmc core don't support multiple cards attached to the same
>> host, it never has. Also, I am not aware of any requests that
>> suggested us to add it. Due to obvious reasons from performance
>> perspective it's simply a really bad idea, that's likely also why
>> there haven't been any requests for it.
>
> Well, that's Linux problem, right? (And why is it bad idea, btw? The
> bandwidth will be shared between the controllers, ok, that does not
> sound too bad.)

Bandwidth is just one issue. Latency is yet another, which is probably
far worse to handle than bandwidth.

>
>> 2) On the host level, the support for handle multiple slots in DT for
>> dw-mmc has been broken. While dw-mmc parsed the DT nodes for slots, it
>> screwed up configurations. Thus the support for slots have never
>> worked as expected from DT point of view.
>
> Well, DT is supposed to describe the hardware. From your description,
> it seems that linux does not support two slots on one controller and
> DT parsing code basically ignores the slots. (Logical, if it can't
> support two slots).
>
> So now we are breaking DT description due to Linux limitations. Which
>
> a) makes it hard for any other os not having same limitation
>
> b) makes it hard for people to fix the limitation
>
> c) does not really solve anything

Yes it does, the problem in 2) gets fixed.

>
> d) breaks backward compatibility with old dts

According to 2), it has never worked - so we don't break anything.

>
> If nothing, d) should be argument not to do this.
>                                                                         Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Kind regards
Uffe

Re: [GIT PULL 1/2] SOCFPGA updates for 3.18

[Pavel Machek]

Hi!

> >> 2) On the host level, the support for handle multiple slots in DT for
> >> dw-mmc has been broken. While dw-mmc parsed the DT nodes for slots, it
> >> screwed up configurations. Thus the support for slots have never
> >> worked as expected from DT point of view.
> >
> > Well, DT is supposed to describe the hardware. From your description,
> > it seems that linux does not support two slots on one controller and
> > DT parsing code basically ignores the slots. (Logical, if it can't
> > support two slots).
> >
> > So now we are breaking DT description due to Linux limitations. Which
> >
> > a) makes it hard for any other os not having same limitation
> >
> > b) makes it hard for people to fix the limitation
> >
> > c) does not really solve anything
> 
> Yes it does, the problem in 2) gets fixed.
> 
> >
> > d) breaks backward compatibility with old dts
> 
> According to 2), it has never worked - so we don't break anything.

Umm? u-SD worked for me in 3.14, 3.15 and 3.16 with the device tree,
and with single-slot described in the device tree. Now I have to
change dtb-s to keep working configuration, which is something device
tree should never ever do.

Is it so hard to just fix the multiple slot parsing?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [GIT PULL 1/2] SOCFPGA updates for 3.18

[Dinh Nguyen]

On 9/10/14, 6:00 AM, Pavel Machek wrote:
> Hi!
> 
>>>> 2) On the host level, the support for handle multiple slots in DT for
>>>> dw-mmc has been broken. While dw-mmc parsed the DT nodes for slots, it
>>>> screwed up configurations. Thus the support for slots have never
>>>> worked as expected from DT point of view.
>>>
>>> Well, DT is supposed to describe the hardware. From your description,
>>> it seems that linux does not support two slots on one controller and
>>> DT parsing code basically ignores the slots. (Logical, if it can't
>>> support two slots).
>>>
>>> So now we are breaking DT description due to Linux limitations. Which
>>>
>>> a) makes it hard for any other os not having same limitation
>>>
>>> b) makes it hard for people to fix the limitation
>>>
>>> c) does not really solve anything
>>
>> Yes it does, the problem in 2) gets fixed.
>>
>>>
>>> d) breaks backward compatibility with old dts
>>
>> According to 2), it has never worked - so we don't break anything.
> 
> Umm? u-SD worked for me in 3.14, 3.15 and 3.16 with the device tree,
> and with single-slot described in the device tree. Now I have to
> change dtb-s to keep working configuration, which is something device
> tree should never ever do.

I tested the DTB with and without the patch, and it all works fine.

Dinh
> 
> Is it so hard to just fix the multiple slot parsing?
> 
> 									Pavel
> 

Re: [GIT PULL 1/2] SOCFPGA updates for 3.18

[Marek Vasut]

On Tuesday, September 09, 2014 at 11:02:32 PM, Pavel Machek wrote:
> On Tue 2014-09-09 17:02:34, Arnd Bergmann wrote:
> > On Tuesday 09 September 2014 16:17:56 Pavel Machek wrote:
> > > > Jaehoon Chung (1):
> > > >       ARM: dts: socfpga: unuse the slot-node and deprecate the
> > > >       supports-highspeed for dw-mmc
> > > 
> > > This patch is a bad idea. It removes support for two mmc cards on a
> > > single controller -- configuration hardware supports and configuration
> > > that allows using u-SD card on mcvevk board.

This is not true, the MCVEVK does _NOT_ have two cards on a single controller.

> > Your objection comes too late, and to the wrong patch, since the
> > driver and all other users have already been changed. We had a long
> > discussion about this when the issue first came up, and we could
> > not find any example of dw-mmc actually being used in a scenario
> > with one controller that has multiple devices attached.
> 
> Well, this is not first time I raised this. 3.17 is not yet out, so we
> still have chance to fix regressions without major fuss.
> 
> > Apparently every user out there instead uses multiple controller
> > instances instead. Are you sure that the socfpga implementation
> > is an exception from this?
> 
> Marek Vasut has the hardware. His board apprently has uSD and eMMC,
> and I believe it has just one controller. I'll try to get details.

So please go ahead with this PR, sorry for blocking it.

Best regards,
Marek Vasut