MMC is *still* broken...

MMC is *still* broken...

[Russell King - ARM Linux]

drivers/mmc/core/core.c: In function 'mmc_card_power_up':
drivers/mmc/core/core.c:1517:4: error: implicit declaration of function 'gpiod_set_value'

I reported this back in April, and it's still present.  This makes me
wonder if MMC is maintained.... and makes me wonder what the point
of reporting bugs is if they just get ignored.

The config file which produced that is:

http://www.arm.linux.org.uk/developer/build/file.php?lid=8820

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

MMC is *still* broken...

[Chris Ball]

Hi Russell, adding Ulf and Olof,

On Thu, Jun 05 2014, Russell King - ARM Linux wrote:
> drivers/mmc/core/core.c: In function 'mmc_card_power_up':
> drivers/mmc/core/core.c:1517:4: error: implicit declaration of
> function 'gpiod_set_value'
>
> I reported this back in April, and it's still present.  This makes me
> wonder if MMC is maintained.... and makes me wonder what the point
> of reporting bugs is if they just get ignored.

I don't recall your report.  I might have just failed to read it, but
doing a quick mail search only finds "Subject: randconfig failures -
MMC + trusted foundations" -- which was sent to linux-arm-kernel@, but
not CC'd to linux-mmc@ or to me.

Furthermore, this error does not exist in mmc-next or linux-next,
so it's unclear why or how the MMC maintainers would fix it.
drivers/mmc/core/core.c does not use gpiod_set_value() in these trees.

Presumably you're testing Olof's unmerged patch "mmc: add support for
power-on sequencing through DT".  If it needs to include more gpio
headers, that should be reported to Olof for a respin.

The patch has not been merged because we're still discussing the best
way forward on linux-mmc@, in the thread "RFC: representing sdio
devices oob interrupt, clks, etc. in device tree".  The most recent
message in the thread is from yesterday.

- Chris.
-- 
Chris Ball   <http://printf.net/>

MMC is *still* broken...

[Olof Johansson]

[Adding Hans]

On Thu, Jun 5, 2014 at 12:31 PM, Chris Ball <chris@printf.net> wrote:
> Hi Russell, adding Ulf and Olof,
>
> On Thu, Jun 05 2014, Russell King - ARM Linux wrote:
>> drivers/mmc/core/core.c: In function 'mmc_card_power_up':
>> drivers/mmc/core/core.c:1517:4: error: implicit declaration of
>> function 'gpiod_set_value'
>>
>> I reported this back in April, and it's still present.  This makes me
>> wonder if MMC is maintained.... and makes me wonder what the point
>> of reporting bugs is if they just get ignored.
>
> I don't recall your report.  I might have just failed to read it, but
> doing a quick mail search only finds "Subject: randconfig failures -
> MMC + trusted foundations" -- which was sent to linux-arm-kernel@, but
> not CC'd to linux-mmc@ or to me.
>
> Furthermore, this error does not exist in mmc-next or linux-next,
> so it's unclear why or how the MMC maintainers would fix it.
> drivers/mmc/core/core.c does not use gpiod_set_value() in these trees.
>
> Presumably you're testing Olof's unmerged patch "mmc: add support for
> power-on sequencing through DT".  If it needs to include more gpio
> headers, that should be reported to Olof for a respin.
>
> The patch has not been merged because we're still discussing the best
> way forward on linux-mmc@, in the thread "RFC: representing sdio
> devices oob interrupt, clks, etc. in device tree".  The most recent
> message in the thread is from yesterday.

Right, Hans has taken over the patch set and he should revise it as
needed. Hans?


-Olof

MMC is *still* broken...

[Hans de Goede]

Hi,

On 06/06/2014 12:21 AM, Olof Johansson wrote:
> [Adding Hans]
> 
> On Thu, Jun 5, 2014 at 12:31 PM, Chris Ball <chris@printf.net> wrote:
>> Hi Russell, adding Ulf and Olof,
>>
>> On Thu, Jun 05 2014, Russell King - ARM Linux wrote:
>>> drivers/mmc/core/core.c: In function 'mmc_card_power_up':
>>> drivers/mmc/core/core.c:1517:4: error: implicit declaration of
>>> function 'gpiod_set_value'
>>>
>>> I reported this back in April, and it's still present.  This makes me
>>> wonder if MMC is maintained.... and makes me wonder what the point
>>> of reporting bugs is if they just get ignored.
>>
>> I don't recall your report.  I might have just failed to read it, but
>> doing a quick mail search only finds "Subject: randconfig failures -
>> MMC + trusted foundations" -- which was sent to linux-arm-kernel@, but
>> not CC'd to linux-mmc@ or to me.
>>
>> Furthermore, this error does not exist in mmc-next or linux-next,
>> so it's unclear why or how the MMC maintainers would fix it.
>> drivers/mmc/core/core.c does not use gpiod_set_value() in these trees.
>>
>> Presumably you're testing Olof's unmerged patch "mmc: add support for
>> power-on sequencing through DT".  If it needs to include more gpio
>> headers, that should be reported to Olof for a respin.
>>
>> The patch has not been merged because we're still discussing the best
>> way forward on linux-mmc@, in the thread "RFC: representing sdio
>> devices oob interrupt, clks, etc. in device tree".  The most recent
>> message in the thread is from yesterday.
> 
> Right, Hans has taken over the patch set and he should revise it as
> needed. Hans?

Erm, that was not really my intent. My interest in mmc powerup
sequencing is only peripheral. As a hobby project I work on support for
Allwinner ARM SoCs, some of their boards use Broadcom sdio wifi IC-s,
and as such I've been testing (and fixing) a patch-set from
Arend van Spriel to add oob irq support to the brcmfmac driver.

In its original incarnation this set also added some mmc powerup sequencing
stuff. Since this was broken I decided to start a discussion on how to deal
with this, hoping that we could reach a consensus on this quickly...

Besides me being only peripheral interested, and this only being
a hobby project, there also is the problem that people who are interested
in this seem to stop responding to any proposals about 3 mails in to
the thread. So just when it seems we are getting somewhere everyone
involved seems to go quiet, which I find very frustrating.

Combining all of the above with the fact that I'm currently really
trying to keep to much balls up in the air at once, means that I've
decided to throw in the towel wrt mmc powerup sequencing.

I've just dropped powerup sequencing from the brcmfmac patch-set, only
adding oob irq support for now, which is all I need for the Allwinner
boards.

I'm sorry if I gave the impression that I was going to fix this all,
but I simply don't have the time nor energy to work at this atm.

For whomever does pick this up, let me try to summarize the direction
the discussion seems to be heading in (but I still have not yet heard
any maintainer clearly state yes this is how we're going to do this) :

1) The necessary resources for powerup should live in a subnode of
the mmc-host.

2) As described in this patch: http://patchwork.ozlabs.org/patch/355235/
mmc-host subnodes will be addressed by sdio-function number with number
0 addressing the card itself. Since the sdio-function subnodes will have
things like oob-irq info and compatible strings to indicate how this info
should be interpreted, I believe that the info from 1). would best be
stored in the subnode with reg = <0>, so the node describing the card
itself, this also makes sense since some of the resources used for powerup
may well be shared between sdio functions.

3) Storing all the resources needed for powerup in a subnode with reg = <0>,
means that we can also out a compatible string there to indicate how these
resources should be used. I can see your (Olof's) original powerup patch
be turned into a generic powerup-driver using a compatible e.g. "generic-powerup".

4) Implementation wise I think we all agree that we want to use a platform
device + driver. So the mmc-core would check for a subnode with reg == 0 and if
there is one instantiate a platform device from this, using the subnode as the
platform devices of_node, and the platform driver for the powerup will be bound
using the compatible string of the subnode.

5) mmc_add_host will do a platform_device_register() for this platform device,
and afterwards check if a driver was bound, if no driver was bound,
the platform device will be unregistered and mmc_add_host will return -EPROBE_DEFER,
so that the whole probe can be tried again later. This is necessary since some
of the resources needed to powerup may not be available yet (or the powerup driver
may be a module which is not available yet).

6) power-management will use runtime-pm. If more fine grained power-management is
added later, then device specific callbacks can be added.

Regards,

Hans

MMC is *still* broken...

[Russell King - ARM Linux]

On Sun, Jun 08, 2014 at 11:55:19AM +0200, Hans de Goede wrote:
> 4) Implementation wise I think we all agree that we want to use a platform
> device + driver. So the mmc-core would check for a subnode with reg == 0 and if
> there is one instantiate a platform device from this, using the subnode as the
> platform devices of_node, and the platform driver for the powerup will be bound
> using the compatible string of the subnode.

I really don't think so on the platform device/driver issue.  Greg keeps
saying that he wants to kill these off, so really we need to stop inventing
new use cases for them.  We're just making more work for ourselves for
when Greg decides to put a block on any further platform drivers - at that
point, we will be forced to rewrite it not to use platform drivers.

So, we need to take account of that today, and *not* use a platform device/
driver for this.

What's also disappointing is that we're close to six months on this issue,
and we still don't have a proper solution to the problem, meanwhile we have
devices out there in the wild where mainline kernels can't be used with
wifi/bt because of our lack of progress on this issue.

While it is important to get interfaces correct, we also can't sit around
being indecisive like this - because the result is we just end up driving
people away from mainline to vendor kernels.  That's exactly what has
been happening.

99% of people run Jon Nettleton's kernels on the Cubox-i because that's
the only modern kernel which supports most of the hardware... why should
anyone bother with mainline given that even 9 months down the line (it's
been 9 months since developers had the initial hardware), mainline kernels
remain mostly crippled on this hardware... anything beyond MMC or NFS boot
and basic GUI... forget it with mainline... not even the eSATA port works
with mainline kernels.

This rather makes me wonder why I'm even bothering _trying_ to get support
for this hardware into mainline - I'm probably completely wasting my time
on that.

(Note: I'm not blaming you, Hans, for this - I'm merely pointing out the
amount of time that has been collectively wasted on this topic and we
*still* seem to be no closer to any kind of solution to it.  Much of that
is because we can't decide on a reasonable DT description.  Board files
with platform data was /loads/ easier because we could come up with
solutions to these kinds of problems much faster there...)

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.