[PATCH] mmc: dw_mmc: fix dw_mci_get_cd

[Russell King - ARM Linux]

On Wed, Jan 15, 2014 at 05:07:49PM +0100, Arnd Bergmann wrote:
> On Wednesday 15 January 2014 16:01:46 Russell King - ARM Linux wrote:
> > On Wed, Jan 15, 2014 at 02:59:59PM +0100, Arnd Bergmann wrote:
> > > On Wednesday 15 January 2014 21:56:26 zhangfei wrote:
> > > > However, in our board cd =0 when card is deteced while cd=1 when card is
> > > > removed.
> > > > In order to mmc_gpio_get_cd return 1, MMC_CAP2_CD_ACTIVE_HIGH has to be
> > > > set, as well as new property "caps2-mmc-cd-active-low".
> > > > 
> > > > --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> > > > +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> > > > @@ -73,6 +73,8 @@ Optional properties:
> > > > +* caps2-mmc-cd-active-low: cd pin is low when card active
> > > > +
> > > > 
> > > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> > > > +       if (of_find_property(np, "caps2-mmc-cd-active-low", NULL))
> > > > +               pdata->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
> > > > +
> > > 
> > > The MMC_CAP2_CD_ACTIVE_HIGH flag should only be required for
> > > legacy platforms. With DT parsing, you can normally specify
> > > the polarity of the GPIO line in the GPIO specifier in DT.
> > 
> > Since when?
> 
> I just looked through the bindings and found that about half
> of them allow passing polarity in the gpio specifier. I thought
> it was more than that and IIRC the outcome of a previous discussion
> was that when the gpio controller allows passing polarity, you
> should use that rather than a separate flag.
> 
> Not a problem though, since we can use the cd-inverted and
> wp-inverted properties.

Well, having this in the gpio level as well as in subsystems like MMC
is going to create confusion - and from the results of this patch _IS_
causing confusion.  You've just encouraged one implementation to have
things setup such that mmc_gpio_get_cd() returns false when a card is
inserted, rather than it correctly returning true.

The issue here is that we'll potentially now end up with _three_
inversions.  One in the GPIO layers.  One in mmc_gpio_get_cd(), and
another in every driver which uses the GPIO layer inversion.  This
is hardly the right approach as it leads to multiple points where
confusion about the inverted status can occur.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".