Re: [RFC 4/7] mmc: tmio: Add UHS-I mode support

From: Wolfram Sang <wsa@the-dreams.de>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: linux-renesas-soc@vger.kernel.org,
	linux-mmc <linux-mmc@vger.kernel.org>,
	Ben Hutchings <ben.hutchings@codethink.co.uk>,
	Dirk Behme <dirk.behme@gmail.com>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>
Subject: Re: [RFC 4/7] mmc: tmio: Add UHS-I mode support
Date: Wed, 9 Mar 2016 17:17:35 +0100	[thread overview]
Message-ID: <20160309161735.GB9443@katana> (raw)
In-Reply-To: <CAPDyKFpxE3XU8ocRLTyMvBo-FPOECY7zeJAr7O1oLwKXEZ20zQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

> > +
> > +       int (*start_signal_voltage_switch)(struct tmio_mmc_host *host,
> > +                                          unsigned char signal_voltage);
> 
> Do you really need to add a new tmio specific callback for this?
> 
> Why can't you instead have the tmio variant driver assign the
> ->start_signal_voltage_switch() callback into the struct mmc_host_ops
> instead?

Just to see how it looks, I tried your suggestion. However, mmc_ops is
consted all the way into the core (rightfully, I'd say), so this makes
this approach quite clumsy. I kept the above callback now, just changed
the second paramater to pass the whole ios so I can use it with
mmc_regulator_set_vqmmc().

> > +static int tmio_mmc_card_busy(struct mmc_host *mmc)
> > +{
> > +       struct tmio_mmc_host *host = mmc_priv(mmc);
> > +       u32 status;
> > +
> > +       pm_runtime_get_sync(mmc_dev(mmc));
> 
> This isn't needed as the mmc core already deal with runtime PM of the
> host device via mmc_claim|release_host().

sdhci.c does it, too - missing cleanup?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]