[PATCH RFC 07/31] mmc: sdhci: push card_tasklet into threaded irq handler

[Viresh Kumar]

Adding ST people as well who have access to boards and are working on SPEAr.

On 18 February 2014 23:27, Russell King - ARM Linux
<linux at arm.linux.org.uk> wrote:
> On Tue, Feb 18, 2014 at 03:09:38PM +0000, Russell King wrote:
>> There's no requirement to have the card tasklet separate now that we
>> have a threaded interrupt handler, so kill this and move the called
>> code into the threaded part of the handler.
>
> This patch breaks sdhci-spear's build due to this:
>
> /* gpio card detection interrupt handler */
> static irqreturn_t sdhci_gpio_irq(int irq, void *dev_id)
> {
>         struct platform_device *pdev = dev_id;
>         struct sdhci_host *host = platform_get_drvdata(pdev);
>         struct spear_sdhci *sdhci = dev_get_platdata(&pdev->dev);
>         unsigned long gpio_irq_type;
>         int val;
>
>         val = gpio_get_value(sdhci->data->card_int_gpio);
>
>         /* val == 1 -> card removed, val == 0 -> card inserted */
>         /* if card removed - set irq for low level, else vice versa */
>         gpio_irq_type = val ? IRQF_TRIGGER_LOW : IRQF_TRIGGER_HIGH;
>         irq_set_irq_type(irq, gpio_irq_type);
>
>         if (sdhci->data->card_power_gpio >= 0) {
>                 if (!sdhci->data->power_always_enb) {
>                         /* if card inserted, give power, otherwise remove it */
>                         val = sdhci->data->power_active_high ? !val : val ;
>                         gpio_set_value(sdhci->data->card_power_gpio, val);
>                 }
>         }
>
>         /* inform sdhci driver about card insertion/removal */
>         tasklet_schedule(&host->card_tasklet);
>
>         return IRQ_HANDLED;
> }
>
> which is really nice code.  WTF is this doing?

:)

> 1) You're controlling the card power GPIO via the insertion/removal
>    interrupt handler?  What's wrong with using a vmmc regulator for
>    that, which will be controlled via the MMC subsystem as required?

Haven't explored that earlier.. Maybe its possible but no idea.
The requirements were like this:
- We may or maynot need a GPIO for controlling power
- If we need it, we may need to disable/enable power with card removal/
insertion Or we may need to keep it enabled for ever..

Don't know if this can be done with existing stuff..

> 2) card detection GPIO is handled natively by sdhci.c, there's really
>    no need for sdhci users to implement this themselves.

Probably it was done to take care of power stuff, so that we know
when card is inserted/removed..

> Viresh, can this custom code be killed?

Not sure. Somebody from inside ST has to work on this, use already
present common stuff from sdhci/regulator/etc..

> Lastly, you do realise that using sdhci-spear with DT results in it
> always claiming GPIO 0 for power control, since you use devm_kzalloc()
> to allocate the private data and never set card_power_gpio to a
> negative number.

Hmm.. No I didn't at that time. Its a bug then.. Actually while leaving ST
I pushed the DT patch upstream and it had power binding there but was
then asked to use regulators etc for it and so dropped it at last time. Never
had bandwidth and hardware after that to do it..

http://www.spinics.net/lists/linux-mmc/msg16685.html

@Pratyush/Mohit: Can you guys help Russell here?

--
viresh