* Re: [PATCH v3 00/27] kill devm_ioremap_nocache
From: Greg KH @ 2017-12-23 13:48 UTC (permalink / raw)
To: Yisheng Xie
Cc: linux-mips, ulf.hansson, jakub.kicinski, platform-driver-x86,
airlied, linux-wireless, linus.walleij, alsa-devel, dri-devel,
linux-kernel, linux-ide, linux-mtd, daniel.vetter, dan.j.williams,
jason, linux-rtc, boris.brezillon, mchehab, dmaengine, vinod.koul,
richard, marek.vasut, industrypack-devel, linux-pci, dvhart,
linux, linux-media, seanpaul, devel, linux-watchdog, arnd,
b.zolnierkie, marc.zyngier, jslaby
In-Reply-To: <1514026525-32538-1-git-send-email-xieyisheng1@huawei.com>
On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
> Hi all,
>
> When I tried to use devm_ioremap function and review related code, I found
> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
> except one use ioremap while the other use ioremap_nocache.
For all arches? Really? Look at MIPS, and x86, they have different
functions.
> While ioremap's
> default function is ioremap_nocache, so devm_ioremap_nocache also have the
> same function with devm_ioremap, which can just be killed to reduce the size
> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>
> I have posted two versions, which use macro instead of function for
> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
> devm_ioremap_nocache for no need to keep a macro around for the duplicate
> thing. So here comes v3 and please help to review.
I don't think this can be done, what am I missing? These functions are
not identical, sorry for missing that before.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 27/27] devres: kill devm_ioremap_nocache
From: Greg KH @ 2017-12-23 13:45 UTC (permalink / raw)
To: Yisheng Xie
Cc: linux-mips, ulf.hansson, jakub.kicinski, lgirdwood, airlied,
linux-pci, alsa-devel, dri-devel, platform-driver-x86, linux-ide,
linux-mtd, daniel.vetter, tglx, linux-watchdog, linux-rtc,
boris.brezillon, andriy.shevchenko, vinod.koul, richard,
alexandre.belloni, marek.vasut, industrypack-devel, jslaby,
dvhart, linux, linux-media, devel, jason, arnd, b.zolnierkie,
marc.zyngier, linux-mmc, linux-can, linux-gp
In-Reply-To: <1514026979-33838-1-git-send-email-xieyisheng1@huawei.com>
On Sat, Dec 23, 2017 at 07:02:59PM +0800, Yisheng Xie wrote:
> --- a/lib/devres.c
> +++ b/lib/devres.c
> @@ -44,35 +44,6 @@ void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
> EXPORT_SYMBOL(devm_ioremap);
>
> /**
> - * devm_ioremap_nocache - Managed ioremap_nocache()
> - * @dev: Generic device to remap IO address for
> - * @offset: Resource address to map
> - * @size: Size of map
> - *
> - * Managed ioremap_nocache(). Map is automatically unmapped on driver
> - * detach.
> - */
> -void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
> - resource_size_t size)
> -{
> - void __iomem **ptr, *addr;
> -
> - ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
> - if (!ptr)
> - return NULL;
> -
> - addr = ioremap_nocache(offset, size);
Wait, devm_ioremap() calls ioremap(), not ioremap_nocache(), are you
_SURE_ that these are all identical? For all arches? If so, then
ioremap_nocache() can also be removed, right?
In my quick glance, I don't think you can do this series at all :(
greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH v3 07/27] mmc: replace devm_ioremap_nocache with devm_ioremap
From: Yisheng Xie @ 2017-12-23 10:58 UTC (permalink / raw)
To: linux-kernel, gregkh; +Cc: ysxie, Yisheng Xie, Ulf Hansson, linux-mmc
Default ioremap is ioremap_nocache, so devm_ioremap has the same
function with devm_ioremap_nocache, which can just be killed to
save the size of devres.o
This patch is to use use devm_ioremap instead of devm_ioremap_nocache,
which should not have any function change but prepare for killing
devm_ioremap_nocache.
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: linux-mmc@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
drivers/mmc/host/sdhci-acpi.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index b988997..b1aa909 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -567,8 +567,7 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
host->ops = &sdhci_acpi_ops_dflt;
host->irq = platform_get_irq(pdev, 0);
- host->ioaddr = devm_ioremap_nocache(dev, iomem->start,
- resource_size(iomem));
+ host->ioaddr = devm_ioremap(dev, iomem->start, resource_size(iomem));
if (host->ioaddr == NULL) {
err = -ENOMEM;
goto err_free;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 27/27] devres: kill devm_ioremap_nocache
From: Yisheng Xie @ 2017-12-23 11:02 UTC (permalink / raw)
To: linux-kernel, gregkh
Cc: ysxie, ulf.hansson, linux-mmc, boris.brezillon, richard,
marek.vasut, cyrille.pitchen, linux-mtd, alsa-devel, wim, linux,
linux-watchdog, b.zolnierkie, linux-fbdev, linus.walleij,
linux-gpio, ralf, linux-mips, lgirdwood, broonie, tglx, jason,
marc.zyngier, arnd, andriy.shevchenko, industrypack-devel, wg,
mkl, linux-can, mcheh
Now, nobody use devm_ioremap_nocache anymore, can it can just be
removed. After this patch the size of devres.o will be reduced from
20304 bytes to 18992 bytes.
Suggested-by: Greg KH <gregkh@linuxfoundation.org>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
Documentation/driver-model/devres.txt | 1 -
include/linux/io.h | 2 --
lib/devres.c | 29 -----------------------------
scripts/coccinelle/free/devm_free.cocci | 2 --
4 files changed, 34 deletions(-)
diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index c180045..c3fddb5 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -292,7 +292,6 @@ IOMAP
devm_ioport_map()
devm_ioport_unmap()
devm_ioremap()
- devm_ioremap_nocache()
devm_ioremap_wc()
devm_ioremap_resource() : checks resource, requests memory region, ioremaps
devm_iounmap()
diff --git a/include/linux/io.h b/include/linux/io.h
index 32e30e8..a9c7270 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -75,8 +75,6 @@ static inline void devm_ioport_unmap(struct device *dev, void __iomem *addr)
void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
resource_size_t size);
-void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
- resource_size_t size);
void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
resource_size_t size);
void devm_iounmap(struct device *dev, void __iomem *addr);
diff --git a/lib/devres.c b/lib/devres.c
index 5f2aedd..f818fcf 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -44,35 +44,6 @@ void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
EXPORT_SYMBOL(devm_ioremap);
/**
- * devm_ioremap_nocache - Managed ioremap_nocache()
- * @dev: Generic device to remap IO address for
- * @offset: Resource address to map
- * @size: Size of map
- *
- * Managed ioremap_nocache(). Map is automatically unmapped on driver
- * detach.
- */
-void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
- resource_size_t size)
-{
- void __iomem **ptr, *addr;
-
- ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return NULL;
-
- addr = ioremap_nocache(offset, size);
- if (addr) {
- *ptr = addr;
- devres_add(dev, ptr);
- } else
- devres_free(ptr);
-
- return addr;
-}
-EXPORT_SYMBOL(devm_ioremap_nocache);
-
-/**
* devm_ioremap_wc - Managed ioremap_wc()
* @dev: Generic device to remap IO address for
* @offset: Resource address to map
diff --git a/scripts/coccinelle/free/devm_free.cocci b/scripts/coccinelle/free/devm_free.cocci
index c990d2c..36b8752 100644
--- a/scripts/coccinelle/free/devm_free.cocci
+++ b/scripts/coccinelle/free/devm_free.cocci
@@ -51,8 +51,6 @@ expression x;
|
x = devm_ioremap(...)
|
- x = devm_ioremap_nocache(...)
-|
x = devm_ioport_map(...)
)
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 00/27] kill devm_ioremap_nocache
From: Yisheng Xie @ 2017-12-23 10:55 UTC (permalink / raw)
To: linux-kernel, gregkh
Cc: ysxie, ulf.hansson, linux-mmc, boris.brezillon, richard,
marek.vasut, cyrille.pitchen, linux-mtd, alsa-devel, wim, linux,
linux-watchdog, b.zolnierkie, linux-fbdev, linus.walleij,
linux-gpio, ralf, linux-mips, lgirdwood, broonie, tglx, jason,
marc.zyngier, arnd, andriy.shevchenko, industrypack-devel, wg,
mkl, linux-can, mcheh
Hi all,
When I tried to use devm_ioremap function and review related code, I found
devm_ioremap and devm_ioremap_nocache is almost the same with each other,
except one use ioremap while the other use ioremap_nocache. While ioremap's
default function is ioremap_nocache, so devm_ioremap_nocache also have the
same function with devm_ioremap, which can just be killed to reduce the size
of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
I have posted two versions, which use macro instead of function for
devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
devm_ioremap_nocache for no need to keep a macro around for the duplicate
thing. So here comes v3 and please help to review.
Thanks so much!
Yisheng Xie
[1] https://lkml.org/lkml/2017/11/20/135
[2] https://lkml.org/lkml/2017/11/25/21
Yisheng Xie (27):
ASOC: replace devm_ioremap_nocache with devm_ioremap
spi: replace devm_ioremap_nocache with devm_ioremap
staging: replace devm_ioremap_nocache with devm_ioremap
ipack: replace devm_ioremap_nocache with devm_ioremap
media: replace devm_ioremap_nocache with devm_ioremap
gpio: replace devm_ioremap_nocache with devm_ioremap
mmc: replace devm_ioremap_nocache with devm_ioremap
PCI: replace devm_ioremap_nocache with devm_ioremap
platform/x86: replace devm_ioremap_nocache with devm_ioremap
tty: replace devm_ioremap_nocache with devm_ioremap
video: replace devm_ioremap_nocache with devm_ioremap
rtc: replace devm_ioremap_nocache with devm_ioremap
char: replace devm_ioremap_nocache with devm_ioremap
mtd: nand: replace devm_ioremap_nocache with devm_ioremap
dmaengine: replace devm_ioremap_nocache with devm_ioremap
ata: replace devm_ioremap_nocache with devm_ioremap
irqchip: replace devm_ioremap_nocache with devm_ioremap
pinctrl: replace devm_ioremap_nocache with devm_ioremap
drm: replace devm_ioremap_nocache with devm_ioremap
regulator: replace devm_ioremap_nocache with devm_ioremap
watchdog: replace devm_ioremap_nocache with devm_ioremap
tools/testing/nvdimm: replace devm_ioremap_nocache with devm_ioremap
MIPS: pci: replace devm_ioremap_nocache with devm_ioremap
can: replace devm_ioremap_nocache with devm_ioremap
wireless: replace devm_ioremap_nocache with devm_ioremap
ethernet: replace devm_ioremap_nocache with devm_ioremap
devres: kill devm_ioremap_nocache
Documentation/driver-model/devres.txt | 1 -
arch/mips/pci/pci-ar2315.c | 3 +--
drivers/ata/pata_arasan_cf.c | 3 +--
drivers/ata/pata_octeon_cf.c | 9 ++++----
drivers/ata/pata_rb532_cf.c | 2 +-
drivers/char/hw_random/bcm63xx-rng.c | 3 +--
drivers/char/hw_random/octeon-rng.c | 10 ++++-----
drivers/dma/altera-msgdma.c | 3 +--
drivers/dma/sprd-dma.c | 4 ++--
drivers/gpio/gpio-ath79.c | 3 +--
drivers/gpio/gpio-em.c | 6 ++---
drivers/gpio/gpio-htc-egpio.c | 4 ++--
drivers/gpio/gpio-xgene.c | 3 +--
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +-
drivers/gpu/drm/msm/msm_drv.c | 2 +-
drivers/gpu/drm/sti/sti_dvo.c | 3 +--
drivers/gpu/drm/sti/sti_hda.c | 4 ++--
drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
drivers/gpu/drm/sti/sti_tvout.c | 2 +-
drivers/gpu/drm/sti/sti_vtg.c | 2 +-
drivers/ipack/devices/ipoctal.c | 13 +++++------
drivers/irqchip/irq-renesas-intc-irqpin.c | 4 ++--
drivers/media/platform/tegra-cec/tegra_cec.c | 4 ++--
drivers/mmc/host/sdhci-acpi.c | 3 +--
drivers/mtd/nand/fsl_upm.c | 4 ++--
drivers/net/can/sja1000/sja1000_platform.c | 4 ++--
drivers/net/ethernet/altera/altera_tse_main.c | 3 +--
drivers/net/ethernet/ethoc.c | 8 +++----
drivers/net/ethernet/lantiq_etop.c | 4 ++--
drivers/net/ethernet/ti/netcp_core.c | 2 +-
drivers/net/wireless/ath/ath9k/ahb.c | 2 +-
drivers/pci/dwc/pci-dra7xx.c | 2 +-
drivers/pinctrl/bcm/pinctrl-ns2-mux.c | 2 +-
drivers/pinctrl/bcm/pinctrl-nsp-mux.c | 4 ++--
drivers/pinctrl/freescale/pinctrl-imx1-core.c | 2 +-
drivers/pinctrl/pinctrl-amd.c | 4 ++--
drivers/platform/x86/intel_pmc_core.c | 5 ++---
drivers/regulator/ti-abb-regulator.c | 6 ++---
drivers/rtc/rtc-sh.c | 4 ++--
drivers/spi/spi-jcore.c | 3 +--
drivers/staging/fsl-mc/bus/mc-io.c | 8 +++----
drivers/tty/mips_ejtag_fdc.c | 4 ++--
drivers/tty/serial/8250/8250_omap.c | 3 +--
drivers/tty/serial/lantiq.c | 3 +--
drivers/tty/serial/meson_uart.c | 3 +--
drivers/tty/serial/owl-uart.c | 2 +-
drivers/tty/serial/pic32_uart.c | 4 ++--
drivers/video/fbdev/mbx/mbxfb.c | 9 ++++----
drivers/video/fbdev/mmp/hw/mmp_ctrl.c | 2 +-
drivers/video/fbdev/pxa168fb.c | 4 ++--
drivers/watchdog/bcm63xx_wdt.c | 4 ++--
drivers/watchdog/rc32434_wdt.c | 4 ++--
include/linux/io.h | 2 --
lib/devres.c | 29 -------------------------
scripts/coccinelle/free/devm_free.cocci | 2 --
sound/soc/au1x/ac97c.c | 4 ++--
sound/soc/au1x/i2sc.c | 4 ++--
sound/soc/intel/atom/sst/sst_acpi.c | 20 ++++++++---------
sound/soc/intel/boards/mfld_machine.c | 4 ++--
sound/soc/sh/fsi.c | 4 ++--
tools/testing/nvdimm/Kbuild | 2 +-
tools/testing/nvdimm/test/iomap.c | 2 +-
62 files changed, 109 insertions(+), 168 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH] mmc: Don't reference Linux-specific OF_GPIO_ACTIVE_LOW flag in DT binding
From: Tuomas Tynkkynen @ 2017-12-23 0:34 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rob Herring, Mark Rutland, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tuomas Tynkkynen
OF_GPIO_ACTIVE_LOW is a Linux implementation detail. The binding
document should be referring to GPIO_ACTIVE_LOW found in
include/dt-bindings/gpio/gpio.h.
Signed-off-by: Tuomas Tynkkynen <tuomas-yrGDUoBaLx3QT0dZR+AlfA@public.gmane.org>
---
Documentation/devicetree/bindings/mmc/mmc.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index fb11ae8b3b72..467cd7b147ce 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -67,10 +67,10 @@ logic applies to the "wp-inverted" property.
CD and WP lines can be implemented on the hardware in one of two ways: as GPIOs,
specified in cd-gpios and wp-gpios properties, or as dedicated pins. Polarity of
dedicated pins can be specified, using *-inverted properties. GPIO polarity can
-also be specified using the OF_GPIO_ACTIVE_LOW flag. This creates an ambiguity
+also be specified using the GPIO_ACTIVE_LOW flag. This creates an ambiguity
in the latter case. We choose to use the XOR logic for GPIO CD and WP lines.
This means, the two properties are "superimposed," for example leaving the
-OF_GPIO_ACTIVE_LOW flag clear and specifying the respective *-inverted
+GPIO_ACTIVE_LOW flag clear and specifying the respective *-inverted property
property results in a double-inversion and actually means the "normal" line
polarity is in effect.
--
2.15.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* RE: [PATCH v1] mmc: sdhci-of-esdhc: Workaround for reducing the maximum speed on ls1021atwr
From: Y.b. Lu @ 2017-12-22 8:13 UTC (permalink / raw)
To: Ulf Hansson, Yinbo Zhu; +Cc: Adrian Hunter, linux-mmc@vger.kernel.org
In-Reply-To: <CAPDyKFotZ9eZkSCj603BGuHOTCXitd7GmaKk0XFsj_7=CnoDgg@mail.gmail.com>
Hi Uffe,
> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> Sent: 2017年11月16日 15:34
> To: Yinbo Zhu <yinbo.zhu@nxp.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>; Y.b. Lu <yangbo.lu@nxp.com>;
> linux-mmc@vger.kernel.org
> Subject: Re: [PATCH v1] mmc: sdhci-of-esdhc: Workaround for reducing the
> maximum speed on ls1021atwr
>
> On 15 November 2017 at 10:01, <yinbo.zhu@nxp.com> wrote:
> > From: "yinbo.zhu" <yinbo.zhu@nxp.com>
> >
> > In SDHC high speed AC timing, the tshivkh parameter is defined as
> > input setup times:SDHC_CMD, SDHC_DATx, to SDHC_CLK. The value of the
> > tshivkh should be 2.5 ns considering the round trip delay, board/data
> > skew.
> > However, because of this erratum, it needs at least 4.1 ns.
> >
> > eSDHC cannot run at the maximum clock speed for the high speed mode,
> > or there is a limit on the length of the trace on the board for data,
> > command, and clock lines of the SDHC.
> >
> > Signed-off-by: yinbo.zhu <yinbo.zhu@nxp.com>
> > ---
> > drivers/mmc/host/sdhci-of-esdhc.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
> > b/drivers/mmc/host/sdhci-of-esdhc.c
> > index 023c24bd0d94..2744dd58a573 100644
> > --- a/drivers/mmc/host/sdhci-of-esdhc.c
> > +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> > @@ -498,6 +498,12 @@ static void esdhc_of_set_clock(struct sdhci_host
> *host, unsigned int clock)
> > clock -= 5000000;
> > }
> >
> > + /* Workaround to reduce the clock frequency for ls1021a esdhc */
> > + if (of_find_compatible_node(NULL, NULL, "fsl,ls1021a-esdhc"))
> > + {
>
> It's better to use the ->data pointer in the struct of_device_id for this kind of
> variant data.
>
> In ->probe() you then call of_match_device() and pick up the ->data pointer
> and assign it to your driver private data.
>
> Many drivers already do like this. Have a look at dw_mmc-exynos.c for
> example.
>
[Y.b. Lu] How about using soc_device_match()? There is also .data member could be used in soc_device_attribute structure.
Use soc_device_match() in esdhc_init and get .data information into sdhci_esdhc structure which is private data.
Current incorrect host version register value fix-up for some platforms is using this method.
Thanks a lot:)
> > + if (clock == 50000000)
> > + clock = 46500000;
> > + }
> > +
> > temp = sdhci_readl(host, ESDHC_SYSTEM_CONTROL);
> > temp &= ~(ESDHC_CLOCK_SDCLKEN | ESDHC_CLOCK_IPGEN |
> ESDHC_CLOCK_HCKEN |
> > ESDHC_CLOCK_PEREN | ESDHC_CLOCK_MASK);
>
> Kind regards
> Uffe
^ permalink raw reply
* Re: [PATCH 1/3] mmc: sdhci-pci-o2micro: Add hardware tuning for eMMC
From: Craig Bergstrom @ 2017-12-21 17:18 UTC (permalink / raw)
To: Adrian Hunter
Cc: LinuxPatchCommit, ulf.hansson@linaro.org,
linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
Shirley Her (SC)
In-Reply-To: <64a8b6a8-ba1b-a298-5115-d8379f121cab@intel.com>
Thanks for sending these patches out, I would be very excited to see
these devices supported by Linux.
I'm trying them out with a SEABIRD device (pci device 1217:8620).
When I boot up and load the driver, it don't see any indication of the
existence of block device and I see these messages repeatedly spammed
to the kernel logs:
[ 0.855163] mmc1: Unknown controller version (3). You may
experience problems.
[ 0.862519] mmc1: SDHCI controller on PCI [0000:01:00.0] using ADMA
[ 93.152163] mmc1: Timeout waiting for hardware cmd interrupt.
[ 93.157911] mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
[ 93.164347] mmc1: sdhci: Sys addr: 0x00000000 | Version: 0x00000603
[ 93.170782] mmc1: sdhci: Blk size: 0x00000000 | Blk cnt: 0x00000000
[ 93.177218] mmc1: sdhci: Argument: 0x00000000 | Trn mode: 0x00000000
[ 93.183652] mmc1: sdhci: Present: 0x01ff0000 | Host ctl: 0x00000001
[ 93.190088] mmc1: sdhci: Power: 0x0000000f | Blk gap: 0x00000000
[ 93.196523] mmc1: sdhci: Wake-up: 0x00000000 | Clock: 0x0000fa07
[ 93.202956] mmc1: sdhci: Timeout: 0x00000000 | Int stat: 0x00018000
[ 93.209392] mmc1: sdhci: Int enab: 0x00ff0083 | Sig enab: 0x00ff0083
[ 93.215826] mmc1: sdhci: AC12 err: 0x00000000 | Slot int: 0x00000001
[ 93.222260] mmc1: sdhci: Caps: 0x25fcc8bf | Caps_1: 0x00002077
[ 93.228694] mmc1: sdhci: Cmd: 0x0000371a | Max curr: 0x005800c8
[ 93.235127] mmc1: sdhci: Resp[0]: 0x00000000 | Resp[1]: 0x00000000
[ 93.241562] mmc1: sdhci: Resp[2]: 0x00000000 | Resp[3]: 0x00000000
[ 93.247994] mmc1: sdhci: Host ctl2: 0x00000000
[ 93.252435] mmc1: sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0x00000000
[ 93.258863] mmc1: sdhci: ============================================
[ 103.392164] mmc1: Timeout waiting for hardware cmd interrupt.
[ 103.397913] mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
[ 103.404350] mmc1: sdhci: Sys addr: 0x00000000 | Version: 0x00000603
[ 103.410786] mmc1: sdhci: Blk size: 0x00000000 | Blk cnt: 0x00000000
[ 103.417221] mmc1: sdhci: Argument: 0x00000000 | Trn mode: 0x00000000
[ 103.423654] mmc1: sdhci: Present: 0x01ff0000 | Host ctl: 0x00000001
[ 103.430089] mmc1: sdhci: Power: 0x0000000f | Blk gap: 0x00000000
[ 103.436523] mmc1: sdhci: Wake-up: 0x00000000 | Clock: 0x0000fa07
[ 103.442955] mmc1: sdhci: Timeout: 0x00000000 | Int stat: 0x00018000
[ 103.449390] mmc1: sdhci: Int enab: 0x00ff0083 | Sig enab: 0x00ff0083
[ 103.455823] mmc1: sdhci: AC12 err: 0x00000000 | Slot int: 0x00000001
[ 103.462255] mmc1: sdhci: Caps: 0x25fcc8bf | Caps_1: 0x00002077
[ 103.468689] mmc1: sdhci: Cmd: 0x0000371a | Max curr: 0x005800c8
[ 103.475126] mmc1: sdhci: Resp[0]: 0x00000000 | Resp[1]: 0x00000000
[ 103.481560] mmc1: sdhci: Resp[2]: 0x00000000 | Resp[3]: 0x00000000
[ 103.487994] mmc1: sdhci: Host ctl2: 0x00000000
[ 103.492435] mmc1: sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0x00000000
[ 103.498867] mmc1: sdhci: ============================================
Is it expected that this should "just work", or am I doing something
wrong? My patched kernel is based on 4.14.2.
On Tue, Dec 19, 2017 at 2:05 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 04/12/17 11:39, LinuxPatchCommit wrote:
>> Dear All,
>>
>> For O2micro/Bayhubtech SD Host DeviceID 8620, eMMC HS200 mode is working at 1.8v and it uses hardware tuning. The hardware tuning only needs to send one tuning command instead of multiple tuning commands with software tuning.
>>
>> Signed-off-by: ernest.zhang <ernest.zhang@bayhubtech.com>
>> ---
>> drivers/mmc/host/sdhci-pci-o2micro.c | 276 ++++++++++++++++++++++++++++++-----
>> 1 file changed, 239 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pci-o2micro.c b/drivers/mmc/host/sdhci-pci-o2micro.c
>> index 14273ca00641..2a7ffd240497 100644
>> --- a/drivers/mmc/host/sdhci-pci-o2micro.c
>> +++ b/drivers/mmc/host/sdhci-pci-o2micro.c
>> @@ -16,22 +16,223 @@
>> */
>>
>> #include <linux/pci.h>
>> -
>> +#include <linux/platform_device.h>
>> +#include <linux/regulator/fixed.h>
>> +#include <linux/regulator/machine.h>
>> +#include <linux/mmc/host.h>
>> +#include <linux/mmc/mmc.h>
>> +#include <linux/delay.h>
>> #include "sdhci.h"
>> #include "sdhci-pci.h"
>> #include "sdhci-pci-o2micro.h"
>>
>> +static void sdhci_o2_start_tuning(struct sdhci_host *host) {
>> + u16 ctrl;
>> +
>> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> + ctrl |= SDHCI_CTRL_EXEC_TUNING;
>> + if (host->quirks2 & SDHCI_QUIRK2_TUNING_WORK_AROUND)
>> + ctrl |= SDHCI_CTRL_TUNED_CLK;
>> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>> +
>> + /*
>> + * As per the Host Controller spec v3.00, tuning command
>> + * generates Buffer Read Ready interrupt, so enable that.
>> + *
>> + * Note: The spec clearly says that when tuning sequence
>> + * is being performed, the controller does not generate
>> + * interrupts other than Buffer Read Ready interrupt. But
>> + * to make sure we don't hit a controller bug, we _only_
>> + * enable Buffer Read Ready interrupt here.
>> + */
>> + sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE);
>> + sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE); }
>> +
>> +static void sdhci_o2_end_tuning(struct sdhci_host *host) {
>> + sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>> + sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE); }
>> +
>> +static inline bool sdhci_data_line_cmd(struct mmc_command *cmd) {
>> + return cmd->data || cmd->flags & MMC_RSP_BUSY; }
>> +
>> +static void sdhci_del_timer(struct sdhci_host *host, struct mmc_request
>> +*mrq) {
>> + if (sdhci_data_line_cmd(mrq->cmd))
>> + del_timer(&host->data_timer);
>> + else
>> + del_timer(&host->timer);
>> +}
>> +
>> +static void sdhci_o2_set_tuning_mode(struct sdhci_host *host, bool hw)
>> +{
>> + u16 reg;
>> +
>> + if (hw) {
>> + // enable hardware tuning
>> + reg = sdhci_readw(host, O2_SD_VENDOR_SETTING);
>> + reg &= (~O2_SD_HW_TUNING_ENABLE);
>> + sdhci_writew(host, reg, O2_SD_VENDOR_SETTING);
>> + } else {
>> + reg = sdhci_readw(host, O2_SD_VENDOR_SETTING);
>> + reg |= O2_SD_HW_TUNING_ENABLE;
>> + sdhci_writew(host, reg, O2_SD_VENDOR_SETTING);
>> + }
>> +}
>> +
>> +static u8 data_buf[64];
>> +
>> +static void sdhci_o2_send_tuning(struct sdhci_host *host, u32 opcode) {
>> + struct mmc_command cmd = { };
>> + struct mmc_data data = { };
>> + struct scatterlist sg;
>> + struct mmc_request mrq = { };
>> + unsigned long flags;
>> + u32 b = host->sdma_boundary;
>> + int size = sizeof(data_buf);
>> +
>> + cmd.opcode = opcode;
>> + cmd.flags = MMC_RSP_PRESENT | MMC_RSP_OPCODE | MMC_RSP_CRC;
>> + cmd.mrq = &mrq;
>> + mrq.cmd = &cmd;
>> + mrq.data = &data;
>> + data.blksz = size;
>> + data.blocks = 1;
>> + data.flags = MMC_DATA_READ;
>> +
>> + data.timeout_ns = 150 * NSEC_PER_MSEC;
>> +
>> + data.sg = &sg;
>> + data.sg_len = 1;
>> + sg_init_one(&sg, data_buf, size);
>> +
>> + spin_lock_irqsave(&host->lock, flags);
>> +
>> + sdhci_writew(host, SDHCI_MAKE_BLKSZ(b, 64), SDHCI_BLOCK_SIZE);
>> +
>> + /*
>> + * The tuning block is sent by the card to the host controller.
>> + * So we set the TRNS_READ bit in the Transfer Mode register.
>> + * This also takes care of setting DMA Enable and Multi Block
>> + * Select in the same register to 0.
>> + */
>> + sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
>> +
>> + sdhci_send_command(host, &cmd);
>> +
>> + host->cmd = NULL;
>> +
>> + sdhci_del_timer(host, &mrq);
>> +
>> + host->tuning_done = 0;
>> +
>> + mmiowb();
>> + spin_unlock_irqrestore(&host->lock, flags);
>> +
>> + /* Wait for Buffer Read Ready interrupt */
>> + wait_event_timeout(host->buf_ready_int, (host->tuning_done == 1),
>> + msecs_to_jiffies(50));
>> +
>> +}
>> +
>> +static void sdhci_o2_reset_tuning(struct sdhci_host *host) {
>> + u16 ctrl;
>> +
>> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> + ctrl &= ~SDHCI_CTRL_TUNED_CLK;
>> + ctrl &= ~SDHCI_CTRL_EXEC_TUNING;
>> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); }
>> +
>> +static void __sdhci_o2_execute_tuning(struct sdhci_host *host, u32
>> +opcode) {
>> +
>> + int i;
>> +
>> + sdhci_o2_send_tuning(host, MMC_SEND_TUNING_BLOCK_HS200);
>
> So it looks like you send the tuning command only once. Is that right?
> Maybe you could briefly outline what the tuning procedure is, and how it
> differs from sdhci_execute_tuning().
>
>> +
>> + for (i = 0; i < 150; i++) {
>> + u16 ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> +
>> + if (!(ctrl & SDHCI_CTRL_EXEC_TUNING)) {
>> + if (ctrl & SDHCI_CTRL_TUNED_CLK) {
>> + pr_info("%s: HW tuning ok !\n",
>> + mmc_hostname(host->mmc));
>> + host->tuning_done = true;
>> + return;
>> + }
>> + pr_warn("%s: HW tuning failed !\n",
>> + mmc_hostname(host->mmc));
>> +
>> + break;
>> + }
>> +
>> + mdelay(1);
>> + }
>> +
>> + pr_info("%s: Tuning failed, falling back to fixed sampling clock\n",
>> + mmc_hostname(host->mmc));
>> + sdhci_o2_reset_tuning(host);
>> +
>> +}
>> +
>> +static int sdhci_o2_execute_tuning(struct mmc_host *mmc, u32 opcode) {
>> + struct sdhci_host *host = mmc_priv(mmc);
>> + int current_bus_width = 0;
>> +
>> + /*
>> + * This handler only implements the eMMC tuning that is specific to
>> + * this controller. Fall back to the standard method for other TINMING.
>> + */
>> + if (host->timing != MMC_TIMING_MMC_HS200)
>> + return sdhci_execute_tuning(mmc, opcode);
>> +
>> + if (WARN_ON(opcode != MMC_SEND_TUNING_BLOCK_HS200))
>> + return -EINVAL;
>> +
>> + /*
>> + * o2 sdhci host didn't support 8bit emmc tuning
>> + */
>> + if (mmc->ios.bus_width == MMC_BUS_WIDTH_8) {
>> + current_bus_width = mmc->ios.bus_width;
>> + mmc->ios.bus_width = MMC_BUS_WIDTH_4;
>> + mmc->ops->set_ios(mmc, &mmc->ios);
>> + }
>> +
>> + sdhci_o2_set_tuning_mode(host, true);
>> +
>> + sdhci_o2_start_tuning(host);
>> +
>> + __sdhci_o2_execute_tuning(host, opcode);
>> +
>> + sdhci_o2_end_tuning(host);
>> +
>> + if (current_bus_width == MMC_BUS_WIDTH_8) {
>> + mmc->ios.bus_width = current_bus_width;
>> + mmc->ops->set_ios(mmc, &mmc->ios);
>> + }
>> +
>> + host->flags &= ~SDHCI_HS400_TUNING;
>> + return 0;
>> +}
>> +
>> static void o2_pci_set_baseclk(struct sdhci_pci_chip *chip, u32 value) {
>> u32 scratch_32;
>> - pci_read_config_dword(chip->pdev,
>> - O2_SD_PLL_SETTING, &scratch_32);
>> + pci_read_config_dword(chip->pdev, O2_SD_PLL_SETTING, &scratch_32);
>>
>> scratch_32 &= 0x0000FFFF;
>> scratch_32 |= value;
>>
>> - pci_write_config_dword(chip->pdev,
>> - O2_SD_PLL_SETTING, scratch_32);
>> + pci_write_config_dword(chip->pdev, O2_SD_PLL_SETTING, scratch_32);
>> }
>>
>> static void o2_pci_led_enable(struct sdhci_pci_chip *chip) @@ -40,23 +241,19 @@ static void o2_pci_led_enable(struct sdhci_pci_chip *chip)
>> u32 scratch_32;
>>
>> /* Set led of SD host function enable */
>> - ret = pci_read_config_dword(chip->pdev,
>> - O2_SD_FUNC_REG0, &scratch_32);
>> + ret = pci_read_config_dword(chip->pdev, O2_SD_FUNC_REG0, &scratch_32);
>> if (ret)
>> return;
>>
>> scratch_32 &= ~O2_SD_FREG0_LEDOFF;
>> - pci_write_config_dword(chip->pdev,
>> - O2_SD_FUNC_REG0, scratch_32);
>> + pci_write_config_dword(chip->pdev, O2_SD_FUNC_REG0, scratch_32);
>>
>> - ret = pci_read_config_dword(chip->pdev,
>> - O2_SD_TEST_REG, &scratch_32);
>> + ret = pci_read_config_dword(chip->pdev, O2_SD_TEST_REG, &scratch_32);
>> if (ret)
>> return;
>>
>> scratch_32 |= O2_SD_LED_ENABLE;
>> - pci_write_config_dword(chip->pdev,
>> - O2_SD_TEST_REG, scratch_32);
>> + pci_write_config_dword(chip->pdev, O2_SD_TEST_REG, scratch_32);
>>
>> }
>>
>> @@ -104,8 +301,7 @@ static void sdhci_pci_o2_fujin2_pci_init(struct sdhci_pci_chip *chip)
>> scratch_32 |= 0x00CC;
>> pci_write_config_dword(chip->pdev, O2_SD_CAP_REG0, scratch_32);
>> /* Set DLL Tuning Window */
>> - ret = pci_read_config_dword(chip->pdev,
>> - O2_SD_TUNING_CTRL, &scratch_32);
>> + ret = pci_read_config_dword(chip->pdev, O2_SD_TUNING_CTRL,
>> +&scratch_32);
>> if (ret)
>> return;
>> scratch_32 &= ~(0x000000FF);
>> @@ -137,8 +333,7 @@ static void sdhci_pci_o2_fujin2_pci_init(struct sdhci_pci_chip *chip)
>> scratch_32 |= 0x30000000;
>> pci_write_config_dword(chip->pdev, O2_SD_CAPS, scratch_32);
>>
>> - ret = pci_read_config_dword(chip->pdev,
>> - O2_SD_MISC_CTRL4, &scratch_32);
>> + ret = pci_read_config_dword(chip->pdev, O2_SD_MISC_CTRL4,
>> +&scratch_32);
>> if (ret)
>> return;
>> scratch_32 &= ~(0x000f0000);
>> @@ -151,6 +346,7 @@ int sdhci_pci_o2_probe_slot(struct sdhci_pci_slot *slot)
>> struct sdhci_pci_chip *chip;
>> struct sdhci_host *host;
>> u32 reg;
>> + int ret;
>>
>> chip = slot->chip;
>> host = slot->host;
>> @@ -164,6 +360,22 @@ int sdhci_pci_o2_probe_slot(struct sdhci_pci_slot *slot)
>> if (reg & 0x1)
>> host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>>
>> + if (chip->pdev->device == PCI_DEVICE_ID_O2_SEABIRD0) {
>> + ret = pci_read_config_dword(chip->pdev,
>> + O2_SD_MISC_SETTING, ®);
>> + if (ret)
>> + return -EIO;
>> + if (reg & (1 << 4)) {
>> + pr_info("%s: emmc 1.8v flag is set,
>> + force 1.8v signaling voltage\n",
>> + mmc_hostname(host->mmc));
>> + host->flags &= ~(SDHCI_SIGNALING_330);
>> + host->flags |= SDHCI_SIGNALING_180;
>> + }
>> + }
>> +
>> + host->mmc_host_ops.execute_tuning = sdhci_o2_execute_tuning;
>> +
>> if (chip->pdev->device != PCI_DEVICE_ID_O2_FUJIN2)
>> break;
>> /* set dll watch dog timer */
>> @@ -191,8 +403,7 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
>> case PCI_DEVICE_ID_O2_8320:
>> case PCI_DEVICE_ID_O2_8321:
>> /* This extra setup is required due to broken ADMA. */
>> - ret = pci_read_config_byte(chip->pdev,
>> - O2_SD_LOCK_WP, &scratch);
>> + ret = pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch);
>> if (ret)
>> return ret;
>> scratch &= 0x7f;
>> @@ -202,8 +413,7 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
>> pci_write_config_byte(chip->pdev, O2_SD_MULTI_VCC3V, 0x08);
>>
>> /* Disable CLK_REQ# support after media DET */
>> - ret = pci_read_config_byte(chip->pdev,
>> - O2_SD_CLKREQ, &scratch);
>> + ret = pci_read_config_byte(chip->pdev, O2_SD_CLKREQ, &scratch);
>> if (ret)
>> return ret;
>> scratch |= 0x20;
>> @@ -224,16 +434,14 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
>> pci_write_config_byte(chip->pdev, O2_SD_ADMA2, 0x08);
>>
>> /* Disable the infinite transfer mode */
>> - ret = pci_read_config_byte(chip->pdev,
>> - O2_SD_INF_MOD, &scratch);
>> + ret = pci_read_config_byte(chip->pdev, O2_SD_INF_MOD, &scratch);
>> if (ret)
>> return ret;
>> scratch |= 0x08;
>> pci_write_config_byte(chip->pdev, O2_SD_INF_MOD, scratch);
>>
>> /* Lock WP */
>> - ret = pci_read_config_byte(chip->pdev,
>> - O2_SD_LOCK_WP, &scratch);
>> + ret = pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch);
>> if (ret)
>> return ret;
>> scratch |= 0x80;
>> @@ -243,8 +451,7 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
>> case PCI_DEVICE_ID_O2_SDS1:
>> case PCI_DEVICE_ID_O2_FUJIN2:
>> /* UnLock WP */
>> - ret = pci_read_config_byte(chip->pdev,
>> - O2_SD_LOCK_WP, &scratch);
>> + ret = pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch);
>> if (ret)
>> return ret;
>>
>> @@ -319,15 +526,13 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
>> if (ret)
>> return ret;
>> scratch_32 &= ~(0xE0);
>> - pci_write_config_dword(chip->pdev,
>> - O2_SD_CAP_REG2, scratch_32);
>> + pci_write_config_dword(chip->pdev, O2_SD_CAP_REG2, scratch_32);
>>
>> if (chip->pdev->device == PCI_DEVICE_ID_O2_FUJIN2)
>> sdhci_pci_o2_fujin2_pci_init(chip);
>>
>> /* Lock WP */
>> - ret = pci_read_config_byte(chip->pdev,
>> - O2_SD_LOCK_WP, &scratch);
>> + ret = pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch);
>> if (ret)
>> return ret;
>> scratch |= 0x80;
>> @@ -336,8 +541,7 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
>> case PCI_DEVICE_ID_O2_SEABIRD0:
>> case PCI_DEVICE_ID_O2_SEABIRD1:
>> /* UnLock WP */
>> - ret = pci_read_config_byte(chip->pdev,
>> - O2_SD_LOCK_WP, &scratch);
>> + ret = pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch);
>> if (ret)
>> return ret;
>>
>> @@ -369,11 +573,9 @@ int sdhci_pci_o2_probe(struct sdhci_pci_chip *chip)
>> }
>>
>> /* Set Tuning Windows to 5 */
>> - pci_write_config_byte(chip->pdev,
>> - O2_SD_TUNING_CTRL, 0x55);
>> + pci_write_config_byte(chip->pdev, O2_SD_TUNING_CTRL, 0x55);
>> /* Lock WP */
>> - ret = pci_read_config_byte(chip->pdev,
>> - O2_SD_LOCK_WP, &scratch);
>> + ret = pci_read_config_byte(chip->pdev, O2_SD_LOCK_WP, &scratch);
>> if (ret)
>> return ret;
>> scratch |= 0x80;
>> --
>> 2.14.1
>>
>
^ permalink raw reply
* Re: [PATCH 1/9] mmc: sdhci-pci: Stop calling sdhci_enable_irq_wakeups()
From: Ulf Hansson @ 2017-12-21 15:33 UTC (permalink / raw)
To: Adrian Hunter
Cc: linux-mmc, Haridhar Kalvala, Linux PM, Rafael J. Wysocki,
Geert Uytterhoeven
In-Reply-To: <b9801ecc-eaf4-7efc-fff0-3cbd40f29c9a@intel.com>
+ linux-pm, Rafael, Geert
On 21 December 2017 at 15:25, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 21/12/17 16:15, Ulf Hansson wrote:
>> On 20 December 2017 at 09:18, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> sdhci_enable_irq_wakeups() is already called by sdhci_suspend_host() so
>>> sdhci-pci should not need to call it. However sdhci_suspend_host() only
>>> calls it if wakeups are enabled, and sdhci-pci does not enable them until
>>> after calling sdhci_suspend_host(). So move the calls to
>>> sdhci_pci_init_wakeup() before calling sdhci_suspend_host(), and
>>> stop calling sdhci_enable_irq_wakeups(). That results in some
>>> simplification because sdhci_pci_suspend_host() and
>>> __sdhci_pci_suspend_host() no longer need to be separate functions.
>>>
>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>> ---
>>> drivers/mmc/host/sdhci-pci-core.c | 58 ++++++++++++++-------------------------
>>> 1 file changed, 21 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
>>> index 110c634cfb43..2ffc09f088ee 100644
>>> --- a/drivers/mmc/host/sdhci-pci-core.c
>>> +++ b/drivers/mmc/host/sdhci-pci-core.c
>>> @@ -39,10 +39,29 @@
>>> static void sdhci_pci_hw_reset(struct sdhci_host *host);
>>>
>>> #ifdef CONFIG_PM_SLEEP
>>> -static int __sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
>>> +static int sdhci_pci_init_wakeup(struct sdhci_pci_chip *chip)
>>> +{
>>> + mmc_pm_flag_t pm_flags = 0;
>>> + int i;
>>> +
>>> + for (i = 0; i < chip->num_slots; i++) {
>>> + struct sdhci_pci_slot *slot = chip->slots[i];
>>> +
>>> + if (slot)
>>> + pm_flags |= slot->host->mmc->pm_flags;
>>> + }
>>> +
>>> + return device_init_wakeup(&chip->pdev->dev,
>>> + (pm_flags & MMC_PM_KEEP_POWER) &&
>>> + (pm_flags & MMC_PM_WAKE_SDIO_IRQ));
>>
>> A couple of comments here.
>>
>> Wakeup settings shouldn't be changed during system suspend. That means
>> we shouldn't call device_init_wakeup() (or device_set_wakeup_enable()
>> for that matter) here.
>>
>> Instead, device_init_wakeup() should be called during ->probe(), while
>> device_set_wakeup_enable() should normally *not* have to be called at
>> all by drivers. There are a exceptions for device_set_wakeup_enable(),
>> however it should not have to be called during system suspend, at
>> least.
>>
>> Of course, I realize that you are not changing the behavior in this
>> regards in this patch, so I am fine this anyway.
>>
>> My point is, that the end result while doing this improvements in this
>> series, should strive to that device_init_wakeup() and
>> device_set_wakeup_enable() becomes used correctly. I don't think that
>> is the case, or is it?
>
> Unfortunately we don't find out what the SDIO driver wants to do (i.e
> pm_flags & MMC_PM_WAKE_SDIO_IRQ) until suspend.
That's true! Of course, we need to be able to act on this, somehow.
>
> I considered enabling wakeups by default but wasn't sure that would not
> increase power consumption in devices not expecting it.
I understand the problem, believe me. However, I would rather like to
try to find a generic solution to these problems, else we will keep
abusing existing wakeups APIs.
For your information, I have been working on several issues on how to
handle the "wakeup path" correctly, which is linked to this problem.
See a few quite small series for this below [1], [2].
I think the general problems can be described liked this:
1) The dev->power.wakeup_path flag doesn't get set for the device by
the PM core, in case device_set_wakeup_enable() is called from a
->suspend() callback. That also means, that the parent device don't
get its >power.wakeup_path flag set in __device_suspend() by the PM
core, while this may be expected by upper layers (PM domains, middle
layers).
2) device_may_wakeup() doesn't tell us the hole story about the
wakeup. Only that is *may* be configured. :-)
So in cases when device_may_wakeup() returns true, we still have these
three conditions to consider, which we currently can't distinguish
between:
*) The wakeup is configured and enabled, so the device should be
enabled in the wakeup path.
**) The wakeup is configured and enabled, but as an out-band-wakeup
signal (like a GPIO IRQ). This means the device shouldn't be enabled
in the wakeup path.
***) The wakeup isn't configured, thus disabled, because there are no
consumers of the wakeup IRQ registered (as may be the case for SDIO
IRQ). This also means the device shouldn't be enabled in the wakeup
path.
Potentially, one could consider **) and ***) being treated in the same
way, via using an opt-out method, avoiding the wakeup path to be
enabled. Currently I have been thinking of adding an "OUT_BAND_WAKEUP"
driver PM flag, to deal with this, however there may be other
preferred options.
Kind regards
Uffe
[1]
https://www.spinics.net/lists/linux-renesas-soc/msg21421.html
[2]
https://www.spinics.net/lists/linux-renesas-soc/msg20122.html
(Actually the discussions there concluded that we may need an
"OUT_BAND_WAKEUP" flag instead)
^ permalink raw reply
* Re: [PATCH] mmc: sdhci-esdhc-imx: Enable/Disable mmc clock during runtime suspend
From: Michael Nazzareno Trimarchi @ 2017-12-21 14:43 UTC (permalink / raw)
To: Adrian Hunter; +Cc: Ulf Hansson, linux-mmc
In-Reply-To: <1513862546-20221-1-git-send-email-michael@amarulasolutions.com>
Hi
I have some questions:
On Thu, Dec 21, 2017 at 2:22 PM, Michael Trimarchi
<michael@amarulasolutions.com> wrote:
> mmc clock can be stopped during runtime suspend and restart during runtime
> resume. This let us know to not have any clock running and this reduce
> the EMI of the device when the bus is not in use
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 7123ef9..9a5e96f 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -196,6 +196,7 @@ struct pltfm_imx_data {
> struct clk *clk_ipg;
> struct clk *clk_ahb;
> struct clk *clk_per;
> + unsigned int actual_clock;
> enum {
> NO_CMD_PENDING, /* no multiblock command pending*/
> MULTIBLK_IN_PROCESS, /* exact multiblock cmd in process */
> @@ -1346,6 +1347,9 @@ static int sdhci_esdhc_runtime_suspend(struct device *dev)
>
> ret = sdhci_runtime_suspend_host(host);
>
> + imx_data->actual_clock = host->mmc->actual_clock;
> + esdhc_pltfm_set_clock(host, 0);
> +
> if (!sdhci_sdio_irq_enabled(host)) {
> clk_disable_unprepare(imx_data->clk_per);
> clk_disable_unprepare(imx_data->clk_ipg);
What if the runtime suspend fail in the sdhci_runtime_suspend_host? Is
the runtime resume called?
Because in the old code the ret is not taken in account to unprepare
and disable the clock so I did not
take in account too. Is this correct?
Michael
> @@ -1366,6 +1370,7 @@ static int sdhci_esdhc_runtime_resume(struct device *dev)
> clk_prepare_enable(imx_data->clk_ipg);
> }
> clk_prepare_enable(imx_data->clk_ahb);
> + esdhc_pltfm_set_clock(host, imx_data->actual_clock);
>
> return sdhci_runtime_resume_host(host);
> }
> --
> 2.7.4
>
--
| Michael Nazzareno Trimarchi Amarula Solutions BV |
| COO - Founder Cruquiuskade 47 |
| +31(0)851119172 Amsterdam 1018 AM NL |
| [`as] http://www.amarulasolutions.com |
^ permalink raw reply
* Re: [PATCH 1/9] mmc: sdhci-pci: Stop calling sdhci_enable_irq_wakeups()
From: Adrian Hunter @ 2017-12-21 14:25 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Haridhar Kalvala
In-Reply-To: <CAPDyKFoHPdYaorR38LaS5Hzs8CC99Smki8uZW8aDanboLRV3QA@mail.gmail.com>
On 21/12/17 16:15, Ulf Hansson wrote:
> On 20 December 2017 at 09:18, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> sdhci_enable_irq_wakeups() is already called by sdhci_suspend_host() so
>> sdhci-pci should not need to call it. However sdhci_suspend_host() only
>> calls it if wakeups are enabled, and sdhci-pci does not enable them until
>> after calling sdhci_suspend_host(). So move the calls to
>> sdhci_pci_init_wakeup() before calling sdhci_suspend_host(), and
>> stop calling sdhci_enable_irq_wakeups(). That results in some
>> simplification because sdhci_pci_suspend_host() and
>> __sdhci_pci_suspend_host() no longer need to be separate functions.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>> drivers/mmc/host/sdhci-pci-core.c | 58 ++++++++++++++-------------------------
>> 1 file changed, 21 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
>> index 110c634cfb43..2ffc09f088ee 100644
>> --- a/drivers/mmc/host/sdhci-pci-core.c
>> +++ b/drivers/mmc/host/sdhci-pci-core.c
>> @@ -39,10 +39,29 @@
>> static void sdhci_pci_hw_reset(struct sdhci_host *host);
>>
>> #ifdef CONFIG_PM_SLEEP
>> -static int __sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
>> +static int sdhci_pci_init_wakeup(struct sdhci_pci_chip *chip)
>> +{
>> + mmc_pm_flag_t pm_flags = 0;
>> + int i;
>> +
>> + for (i = 0; i < chip->num_slots; i++) {
>> + struct sdhci_pci_slot *slot = chip->slots[i];
>> +
>> + if (slot)
>> + pm_flags |= slot->host->mmc->pm_flags;
>> + }
>> +
>> + return device_init_wakeup(&chip->pdev->dev,
>> + (pm_flags & MMC_PM_KEEP_POWER) &&
>> + (pm_flags & MMC_PM_WAKE_SDIO_IRQ));
>
> A couple of comments here.
>
> Wakeup settings shouldn't be changed during system suspend. That means
> we shouldn't call device_init_wakeup() (or device_set_wakeup_enable()
> for that matter) here.
>
> Instead, device_init_wakeup() should be called during ->probe(), while
> device_set_wakeup_enable() should normally *not* have to be called at
> all by drivers. There are a exceptions for device_set_wakeup_enable(),
> however it should not have to be called during system suspend, at
> least.
>
> Of course, I realize that you are not changing the behavior in this
> regards in this patch, so I am fine this anyway.
>
> My point is, that the end result while doing this improvements in this
> series, should strive to that device_init_wakeup() and
> device_set_wakeup_enable() becomes used correctly. I don't think that
> is the case, or is it?
Unfortunately we don't find out what the SDIO driver wants to do (i.e
pm_flags & MMC_PM_WAKE_SDIO_IRQ) until suspend.
I considered enabling wakeups by default but wasn't sure that would not
increase power consumption in devices not expecting it.
^ permalink raw reply
* Re: [PATCH] mmc: tmio: use io* accessors consistently
From: Ulf Hansson @ 2017-12-21 14:22 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-mmc@vger.kernel.org, Linux-Renesas, Masahiro Yamada,
Simon Horman
In-Reply-To: <20171219133403.788-1-wsa+renesas@sang-engineering.com>
On 19 December 2017 at 14:34, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Because we started using io*_rep accessors previously because they are
> more widely defined across architectures, let's be consistent and use
> this family for all accessor wrappers.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> And here is the follow-up patch. Tested on a R-Car M3-W; still could checksum a
> huge file from SD without performance regressions.
>
> drivers/mmc/host/tmio_mmc.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 03519c4ca0aa1a..e7d651352dc90f 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -227,7 +227,7 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
>
> static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
> {
> - return readw(host->ctl + (addr << host->bus_shift));
> + return ioread16(host->ctl + (addr << host->bus_shift));
> }
>
> static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
> @@ -239,8 +239,8 @@ static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
> static inline u32 sd_ctrl_read16_and_16_as_32(struct tmio_mmc_host *host,
> int addr)
> {
> - return readw(host->ctl + (addr << host->bus_shift)) |
> - readw(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
> + return ioread16(host->ctl + (addr << host->bus_shift)) |
> + ioread16(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
> }
>
> static inline void sd_ctrl_read32_rep(struct tmio_mmc_host *host, int addr,
> @@ -257,7 +257,7 @@ static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr,
> */
> if (host->write16_hook && host->write16_hook(host, addr))
> return;
> - writew(val, host->ctl + (addr << host->bus_shift));
> + iowrite16(val, host->ctl + (addr << host->bus_shift));
> }
>
> static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
> @@ -269,8 +269,8 @@ static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
> static inline void sd_ctrl_write32_as_16_and_16(struct tmio_mmc_host *host,
> int addr, u32 val)
> {
> - writew(val & 0xffff, host->ctl + (addr << host->bus_shift));
> - writew(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
> + iowrite16(val & 0xffff, host->ctl + (addr << host->bus_shift));
> + iowrite16(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
> }
>
> static inline void sd_ctrl_write32_rep(struct tmio_mmc_host *host, int addr,
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH mmc-utils 0/6] mmc-utils: various cleanups
From: Ulf Hansson @ 2017-12-21 14:21 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: Chris Ball, linux-mmc@vger.kernel.org, Sascha Hauer
In-Reply-To: <20171221133316.19466-1-u.kleine-koenig@pengutronix.de>
On 21 December 2017 at 14:33, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> Hello,
>
> while trying to teach barebox to do what can be achived in Linux with
>
> mmc enh_area set ...
>
> (not only by reading but also by stealing code from mmc-utils) I noticed
> a few things that when fixed make it easier for me to copy code. These
> (and a few more) are fixed in this series.
>
> I hope I sent this series to the right people and list. If not, please
> excuse me not knowing it better. I'm thankful for hints where to send
> patches like this instead in the future.
This is the right place!
Chris maintains mmc-utils, however, I haven't heard from him for quite
a while, let's hope that he sees this!
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 1/9] mmc: sdhci-pci: Stop calling sdhci_enable_irq_wakeups()
From: Ulf Hansson @ 2017-12-21 14:15 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, Haridhar Kalvala
In-Reply-To: <1513757933-11232-2-git-send-email-adrian.hunter@intel.com>
On 20 December 2017 at 09:18, Adrian Hunter <adrian.hunter@intel.com> wrote:
> sdhci_enable_irq_wakeups() is already called by sdhci_suspend_host() so
> sdhci-pci should not need to call it. However sdhci_suspend_host() only
> calls it if wakeups are enabled, and sdhci-pci does not enable them until
> after calling sdhci_suspend_host(). So move the calls to
> sdhci_pci_init_wakeup() before calling sdhci_suspend_host(), and
> stop calling sdhci_enable_irq_wakeups(). That results in some
> simplification because sdhci_pci_suspend_host() and
> __sdhci_pci_suspend_host() no longer need to be separate functions.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/sdhci-pci-core.c | 58 ++++++++++++++-------------------------
> 1 file changed, 21 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 110c634cfb43..2ffc09f088ee 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -39,10 +39,29 @@
> static void sdhci_pci_hw_reset(struct sdhci_host *host);
>
> #ifdef CONFIG_PM_SLEEP
> -static int __sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
> +static int sdhci_pci_init_wakeup(struct sdhci_pci_chip *chip)
> +{
> + mmc_pm_flag_t pm_flags = 0;
> + int i;
> +
> + for (i = 0; i < chip->num_slots; i++) {
> + struct sdhci_pci_slot *slot = chip->slots[i];
> +
> + if (slot)
> + pm_flags |= slot->host->mmc->pm_flags;
> + }
> +
> + return device_init_wakeup(&chip->pdev->dev,
> + (pm_flags & MMC_PM_KEEP_POWER) &&
> + (pm_flags & MMC_PM_WAKE_SDIO_IRQ));
A couple of comments here.
Wakeup settings shouldn't be changed during system suspend. That means
we shouldn't call device_init_wakeup() (or device_set_wakeup_enable()
for that matter) here.
Instead, device_init_wakeup() should be called during ->probe(), while
device_set_wakeup_enable() should normally *not* have to be called at
all by drivers. There are a exceptions for device_set_wakeup_enable(),
however it should not have to be called during system suspend, at
least.
Of course, I realize that you are not changing the behavior in this
regards in this patch, so I am fine this anyway.
My point is, that the end result while doing this improvements in this
series, should strive to that device_init_wakeup() and
device_set_wakeup_enable() becomes used correctly. I don't think that
is the case, or is it?
> +}
> +
> +static int sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
> {
> int i, ret;
>
> + sdhci_pci_init_wakeup(chip);
> +
> for (i = 0; i < chip->num_slots; i++) {
> struct sdhci_pci_slot *slot = chip->slots[i];
> struct sdhci_host *host;
> @@ -58,9 +77,6 @@ static int __sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
> ret = sdhci_suspend_host(host);
> if (ret)
> goto err_pci_suspend;
> -
> - if (host->mmc->pm_flags & MMC_PM_WAKE_SDIO_IRQ)
> - sdhci_enable_irq_wakeups(host);
> }
>
> return 0;
> @@ -71,36 +87,6 @@ static int __sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
> return ret;
> }
>
> -static int sdhci_pci_init_wakeup(struct sdhci_pci_chip *chip)
> -{
> - mmc_pm_flag_t pm_flags = 0;
> - int i;
> -
> - for (i = 0; i < chip->num_slots; i++) {
> - struct sdhci_pci_slot *slot = chip->slots[i];
> -
> - if (slot)
> - pm_flags |= slot->host->mmc->pm_flags;
> - }
> -
> - return device_init_wakeup(&chip->pdev->dev,
> - (pm_flags & MMC_PM_KEEP_POWER) &&
> - (pm_flags & MMC_PM_WAKE_SDIO_IRQ));
> -}
> -
> -static int sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
> -{
> - int ret;
> -
> - ret = __sdhci_pci_suspend_host(chip);
> - if (ret)
> - return ret;
> -
> - sdhci_pci_init_wakeup(chip);
> -
> - return 0;
> -}
> -
> int sdhci_pci_resume_host(struct sdhci_pci_chip *chip)
> {
> struct sdhci_pci_slot *slot;
> @@ -1108,7 +1094,7 @@ static int jmicron_suspend(struct sdhci_pci_chip *chip)
> {
> int i, ret;
>
> - ret = __sdhci_pci_suspend_host(chip);
> + ret = sdhci_pci_suspend_host(chip);
> if (ret)
> return ret;
>
> @@ -1118,8 +1104,6 @@ static int jmicron_suspend(struct sdhci_pci_chip *chip)
> jmicron_enable_mmc(chip->slots[i]->host, 0);
> }
>
> - sdhci_pci_init_wakeup(chip);
> -
> return 0;
> }
>
> --
> 1.9.1
>
Kind regards
Uffe
^ permalink raw reply
* [PATCH mmc-utils 2/6] mmc-utils: drop unused header
From: Uwe Kleine-König @ 2017-12-21 13:33 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc, kernel
In-Reply-To: <20171221133316.19466-1-u.kleine-koenig@pengutronix.de>
---
mmc.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/mmc.h b/mmc.h
index 4beb4fd1a819..d01a55f18805 100644
--- a/mmc.h
+++ b/mmc.h
@@ -17,7 +17,6 @@
* those modifications are Copyright (c) 2016 SanDisk Corp.
*/
-#include <asm-generic/int-ll64.h>
#include <linux/mmc/ioctl.h>
#ifndef offsetof
--
2.15.1
^ permalink raw reply related
* [PATCH mmc-utils 1/6] mmc-utils: drop macro CHECK
From: Uwe Kleine-König @ 2017-12-21 13:33 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc, kernel
In-Reply-To: <20171221133316.19466-1-u.kleine-koenig@pengutronix.de>
This macro saves a few lines of code but is harder to read than the
equivalent code spelled out. IMHO the latter is more important, so
expand the macro everywhere and drop it.
While touching this also unbreak the strings used there for better
grepability.
---
lsmmc.c | 18 ++++++---
mmc.h | 3 --
mmc_cmds.c | 124 ++++++++++++++++++++++++++++++++++++++++---------------------
3 files changed, 93 insertions(+), 52 deletions(-)
diff --git a/lsmmc.c b/lsmmc.c
index 71e6a58d26fc..c4faa002e780 100644
--- a/lsmmc.c
+++ b/lsmmc.c
@@ -2348,8 +2348,10 @@ int do_read_csd(int argc, char **argv)
struct config config;
int ret;
- CHECK(argc != 2 && argc != 3, "Usage: Print CSD data from <device path>.\n",
- exit(1));
+ if (argc != 2 && argc != 3) {
+ fprintf(stderr, "Usage: Print CSD data from <device path>.\n");
+ exit(1);
+ }
ret = lsmmc_main(&config, argc, argv);
if (ret)
@@ -2369,8 +2371,10 @@ int do_read_cid(int argc, char **argv)
struct config config;
int ret;
- CHECK(argc != 2 && argc != 3, "Usage: Print CID data from <device path>.\n",
- exit(1));
+ if (argc != 2 && argc != 3) {
+ fprintf(stderr, "Usage: Print CID data from <device path>.\n");
+ exit(1);
+ }
ret = lsmmc_main(&config, argc, argv);
if (ret)
@@ -2390,8 +2394,10 @@ int do_read_scr(int argc, char **argv)
struct config config;
int ret;
- CHECK(argc != 2 && argc != 3, "Usage: Print SCR data from <device path>.\n",
- exit(1));
+ if (argc != 2 && argc != 3) {
+ fprintf(stderr, "Usage: Print SCR data from <device path>.\n");
+ exit(1);
+ }
ret = lsmmc_main(&config, argc, argv);
if (ret)
diff --git a/mmc.h b/mmc.h
index fa49df6f79f8..4beb4fd1a819 100644
--- a/mmc.h
+++ b/mmc.h
@@ -19,9 +19,6 @@
#include <asm-generic/int-ll64.h>
#include <linux/mmc/ioctl.h>
-#include <stdio.h>
-
-#define CHECK(expr, msg, err_stmt) { if (expr) { fprintf(stderr, msg); err_stmt; } }
#ifndef offsetof
#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
diff --git a/mmc_cmds.c b/mmc_cmds.c
index d7215bb8921f..f1c70fb5fe55 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -231,9 +231,10 @@ int do_writeprotect_boot_get(int nargs, char **argv)
int fd, ret;
char *device;
- CHECK(nargs != 2,
- "Usage: mmc writeprotect boot get </path/to/mmcblkX>\n",
- exit(1));
+ if (nargs != 2) {
+ fprintf(stderr, "Usage: mmc writeprotect boot get </path/to/mmcblkX>\n");
+ exit(1);
+ }
device = argv[1];
@@ -260,9 +261,10 @@ int do_writeprotect_boot_set(int nargs, char **argv)
int fd, ret;
char *device;
- CHECK(nargs != 2,
- "Usage: mmc writeprotect boot set </path/to/mmcblkX>\n",
- exit(1));
+ if (nargs != 2) {
+ fprintf(stderr, "Usage: mmc writeprotect boot set </path/to/mmcblkX>\n");
+ exit(1);
+ }
device = argv[1];
@@ -325,9 +327,10 @@ int do_writeprotect_user_get(int nargs, char **argv)
__u32 last_prot = -1;
int remain;
- CHECK(nargs != 2,
- "Usage: mmc writeprotect user get </path/to/mmcblkX>\n",
- exit(1));
+ if (nargs != 2) {
+ fprintf(stderr, "Usage: mmc writeprotect user get </path/to/mmcblkX>\n");
+ exit(1);
+ }
device = argv[1];
@@ -482,7 +485,11 @@ int do_disable_512B_emulation(int nargs, char **argv)
int fd, ret;
char *device;
- CHECK(nargs != 2, "Usage: mmc disable 512B emulation </path/to/mmcblkX>\n", exit(1));
+ if (nargs != 2) {
+ fprintf(stderr, "Usage: mmc disable 512B emulation </path/to/mmcblkX>\n");
+ exit(1);
+ }
+
device = argv[1];
fd = open(device, O_RDWR);
@@ -528,8 +535,10 @@ int do_write_boot_en(int nargs, char **argv)
char *device;
int boot_area, send_ack;
- CHECK(nargs != 4, "Usage: mmc bootpart enable <partition_number> "
- "<send_ack> </path/to/mmcblkX>\n", exit(1));
+ if (nargs != 4) {
+ fprintf(stderr, "Usage: mmc bootpart enable <partition_number> <send_ack> </path/to/mmcblkX>\n");
+ exit(1);
+ }
/*
* If <send_ack> is 1, the device will send acknowledgment
@@ -596,9 +605,10 @@ int do_boot_bus_conditions_set(int nargs, char **argv)
int fd, ret;
char *device;
- CHECK(nargs != 5, "Usage: mmc: bootbus set <boot_mode> "
- "<reset_boot_bus_conditions> <boot_bus_width> <device>\n",
- exit(1));
+ if (nargs != 5) {
+ fprintf(stderr, "Usage: mmc: bootbus set <boot_mode> <reset_boot_bus_conditions> <boot_bus_width> <device>\n");
+ exit(1);
+ }
if (strcmp(argv[1], "single_backward") == 0)
value |= 0;
@@ -664,8 +674,10 @@ int do_hwreset(int value, int nargs, char **argv)
int fd, ret;
char *device;
- CHECK(nargs != 2, "Usage: mmc hwreset enable </path/to/mmcblkX>\n",
- exit(1));
+ if (nargs != 2) {
+ fprintf(stderr, "Usage: mmc hwreset enable </path/to/mmcblkX>\n");
+ exit(1);
+ }
device = argv[1];
@@ -723,8 +735,10 @@ int do_write_bkops_en(int nargs, char **argv)
int fd, ret;
char *device;
- CHECK(nargs != 2, "Usage: mmc bkops enable </path/to/mmcblkX>\n",
- exit(1));
+ if (nargs != 2) {
+ fprintf(stderr, "Usage: mmc bkops enable </path/to/mmcblkX>\n");
+ exit(1);
+ }
device = argv[1];
@@ -761,8 +775,10 @@ int do_status_get(int nargs, char **argv)
int fd, ret;
char *device;
- CHECK(nargs != 2, "Usage: mmc status get </path/to/mmcblkX>\n",
- exit(1));
+ if (nargs != 2) {
+ fprintf(stderr, "Usage: mmc status get </path/to/mmcblkX>\n");
+ exit(1);
+ }
device = argv[1];
@@ -963,8 +979,10 @@ int do_create_gp_partition(int nargs, char **argv)
unsigned int length_kib, gp_size_mult;
unsigned long align;
- CHECK(nargs != 7, "Usage: mmc gp create <-y|-n|-c> <length KiB> "
- "<partition> <enh_attr> <ext_attr> </path/to/mmcblkX>\n", exit(1));
+ if (nargs != 7) {
+ fprintf(stderr, "Usage: mmc gp create <-y|-n|-c> <length KiB> <partition> <enh_attr> <ext_attr> </path/to/mmcblkX>\n");
+ exit(1);
+ }
if (!strcmp("-y", argv[1])) {
dry_run = 0;
@@ -1089,8 +1107,10 @@ int do_enh_area_set(int nargs, char **argv)
unsigned int start_kib, length_kib, enh_start_addr, enh_size_mult;
unsigned long align;
- CHECK(nargs != 5, "Usage: mmc enh_area set <-y|-n|-c> <start KiB> <length KiB> "
- "</path/to/mmcblkX>\n", exit(1));
+ if (nargs != 5) {
+ fprintf(stderr, "Usage: mmc enh_area set <-y|-n|-c> <start KiB> <length KiB> </path/to/mmcblkX>\n");
+ exit(1);
+ }
if (!strcmp("-y", argv[1])) {
dry_run = 0;
@@ -1234,8 +1254,10 @@ int do_write_reliability_set(int nargs, char **argv)
int partition;
char *device;
- CHECK(nargs != 4, "Usage: mmc write_reliability set <-y|-n|-c> "
- "<partition> </path/to/mmcblkX>\n", exit(1));
+ if (nargs != 4) {
+ fprintf(stderr,"Usage: mmc write_reliability set <-y|-n|-c> <partition> </path/to/mmcblkX>\n");
+ exit(1);
+ }
if (!strcmp("-y", argv[1])) {
dry_run = 0;
@@ -1297,8 +1319,10 @@ int do_read_extcsd(int nargs, char **argv)
char *device;
const char *str;
- CHECK(nargs != 2, "Usage: mmc extcsd read </path/to/mmcblkX>\n",
- exit(1));
+ if (nargs != 2) {
+ fprintf(stderr, "Usage: mmc extcsd read </path/to/mmcblkX>\n");
+ exit(1);
+ }
device = argv[1];
@@ -1761,8 +1785,10 @@ int do_sanitize(int nargs, char **argv)
int fd, ret;
char *device;
- CHECK(nargs != 2, "Usage: mmc sanitize </path/to/mmcblkX>\n",
- exit(1));
+ if (nargs != 2) {
+ fprintf(stderr, "Usage: mmc sanitize </path/to/mmcblkX>\n");
+ exit(1);
+ }
device = argv[1];
@@ -1933,8 +1959,10 @@ int do_rpmb_write_key(int nargs, char **argv)
.req_resp = htobe16(MMC_RPMB_WRITE_KEY)
}, frame_out;
- CHECK(nargs != 3, "Usage: mmc rpmb write-key </path/to/mmcblkXrpmb> </path/to/key>\n",
- exit(1));
+ if (nargs != 3) {
+ fprintf(stderr, "Usage: mmc rpmb write-key </path/to/mmcblkXrpmb> </path/to/key>\n");
+ exit(1);
+ }
dev_fd = open(argv[1], O_RDWR);
if (dev_fd < 0) {
@@ -2013,8 +2041,10 @@ int do_rpmb_read_counter(int nargs, char **argv)
int ret, dev_fd;
unsigned int cnt;
- CHECK(nargs != 2, "Usage: mmc rpmb read-counter </path/to/mmcblkXrpmb>\n",
- exit(1));
+ if (nargs != 2) {
+ fprintf(stderr, "Usage: mmc rpmb read-counter </path/to/mmcblkXrpmb>\n");
+ exit(1);
+ }
dev_fd = open(argv[1], O_RDWR);
if (dev_fd < 0) {
@@ -2046,8 +2076,10 @@ int do_rpmb_read_block(int nargs, char **argv)
.req_resp = htobe16(MMC_RPMB_READ),
}, *frame_out_p;
- CHECK(nargs != 5 && nargs != 6, "Usage: mmc rpmb read-block </path/to/mmcblkXrpmb> <address> <blocks count> </path/to/output_file> [/path/to/key]\n",
- exit(1));
+ if (nargs != 5 && nargs != 6) {
+ fprintf(stderr, "Usage: mmc rpmb read-block </path/to/mmcblkXrpmb> <address> <blocks count> </path/to/output_file> [/path/to/key]\n");
+ exit(1);
+ }
dev_fd = open(argv[1], O_RDWR);
if (dev_fd < 0) {
@@ -2195,8 +2227,10 @@ int do_rpmb_write_block(int nargs, char **argv)
.block_count = htobe16(1)
}, frame_out;
- CHECK(nargs != 5, "Usage: mmc rpmb write-block </path/to/mmcblkXrpmb> <address> </path/to/input_file> </path/to/key>\n",
- exit(1));
+ if (nargs != 5) {
+ fprintf(stderr, "Usage: mmc rpmb write-block </path/to/mmcblkXrpmb> <address> </path/to/input_file> </path/to/key>\n");
+ exit(1);
+ }
dev_fd = open(argv[1], O_RDWR);
if (dev_fd < 0) {
@@ -2300,8 +2334,10 @@ int do_cache_ctrl(int value, int nargs, char **argv)
int fd, ret;
char *device;
- CHECK(nargs != 2, "Usage: mmc cache enable </path/to/mmcblkX>\n",
- exit(1));
+ if (nargs != 2) {
+ fprintf(stderr, "Usage: mmc cache enable </path/to/mmcblkX>\n");
+ exit(1);
+ }
device = argv[1];
@@ -2373,8 +2409,10 @@ int do_ffu(int nargs, char **argv)
char *device;
struct mmc_ioc_multi_cmd *multi_cmd;
- CHECK(nargs != 3, "Usage: ffu <image name> </path/to/mmcblkX> \n",
- exit(1));
+ if (nargs != 3) {
+ fprintf(stderr, "Usage: ffu <image name> </path/to/mmcblkX> \n");
+ exit(1);
+ }
device = argv[2];
dev_fd = open(device, O_RDWR);
--
2.15.1
^ permalink raw reply related
* [PATCH mmc-utils 4/6] mmc-utils: expand .gitignore
From: Uwe Kleine-König @ 2017-12-21 13:33 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc, kernel
In-Reply-To: <20171221133316.19466-1-u.kleine-koenig@pengutronix.de>
Instead of adding .o and .o.d files for the files that are not
explicitly mentioned, add a pattern matching also all future files of
these types. This makes git status stop to mention
3rdparty/hmac_sha/.hmac_sha2.o.d
3rdparty/hmac_sha/.sha2.o.d
3rdparty/hmac_sha/hmac_sha2.o
3rdparty/hmac_sha/sha2.o
.lsmmc.o.d
lsmmc.o
---
.gitignore | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/.gitignore b/.gitignore
index 5a94d4731f72..2a0cb3034cdb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,3 @@
-.mmc.o.d
-.mmc_cmds.o.d
-mmc
-mmc.o
-mmc_cmds.o
+.*.o.d
+*.o
+/mmc
--
2.15.1
^ permalink raw reply related
* [PATCH mmc-utils 0/6] mmc-utils: various cleanups
From: Uwe Kleine-König @ 2017-12-21 13:33 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc, kernel
Hello,
while trying to teach barebox to do what can be achived in Linux with
mmc enh_area set ...
(not only by reading but also by stealing code from mmc-utils) I noticed
a few things that when fixed make it easier for me to copy code. These
(and a few more) are fixed in this series.
I hope I sent this series to the right people and list. If not, please
excuse me not knowing it better. I'm thankful for hints where to send
patches like this instead in the future.
Best regards
Uwe
Uwe Kleine-König (6):
mmc-utils: drop macro CHECK
mmc-utils: drop unused header
mmc-utils: make use of dependency information generated by gcc
mmc-utils: expand .gitignore
mmc-utils: move offsetof from mmc.h to only user
mmc-utils: remove unused #includes
.gitignore | 8 ++--
Makefile | 2 +
lsmmc.c | 18 ++++++---
mmc.h | 8 ----
mmc_cmds.c | 134 +++++++++++++++++++++++++++++++++++++++----------------------
5 files changed, 103 insertions(+), 67 deletions(-)
--
2.15.1
^ permalink raw reply
* [PATCH mmc-utils 5/6] mmc-utils: move offsetof from mmc.h to only user
From: Uwe Kleine-König @ 2017-12-21 13:33 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc, kernel
In-Reply-To: <20171221133316.19466-1-u.kleine-koenig@pengutronix.de>
offsetof isn't mmc specific, so remove it from mmc.h. As there is only a single user
define it there.
---
mmc.h | 4 ----
mmc_cmds.c | 4 ++++
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/mmc.h b/mmc.h
index d01a55f18805..285c1f1c9423 100644
--- a/mmc.h
+++ b/mmc.h
@@ -19,10 +19,6 @@
#include <linux/mmc/ioctl.h>
-#ifndef offsetof
-#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
-#endif
-
/* From kernel linux/major.h */
#define MMC_BLOCK_MAJOR 179
diff --git a/mmc_cmds.c b/mmc_cmds.c
index f1c70fb5fe55..cec947da1d4a 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -38,6 +38,10 @@
#include "mmc_cmds.h"
#include "3rdparty/hmac_sha/hmac_sha2.h"
+#ifndef offsetof
+#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
+#endif
+
#define WP_BLKS_PER_QUERY 32
#define USER_WP_PERM_PSWD_DIS 0x80
--
2.15.1
^ permalink raw reply related
* [PATCH mmc-utils 3/6] mmc-utils: make use of dependency information generated by gcc
From: Uwe Kleine-König @ 2017-12-21 13:33 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc, kernel
In-Reply-To: <20171221133316.19466-1-u.kleine-koenig@pengutronix.de>
While these dependency information was generated from the start of mmc-utils
it was never used.
Using these informations results in mmc being rebuild when e.g. mmc.h
was touched.
---
Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Makefile b/Makefile
index 5e4eb1cb0153..aa27ff2b7707 100644
--- a/Makefile
+++ b/Makefile
@@ -52,4 +52,6 @@ install: $(progs) install-man
$(INSTALL) -m755 -d $(DESTDIR)$(bindir)
$(INSTALL) $(progs) $(DESTDIR)$(bindir)
+-include $(foreach obj,$(objects), $(dir $(obj))/.$(notdir $(obj)).d)
+
.PHONY: all clean install manpages install-man
--
2.15.1
^ permalink raw reply related
* [PATCH mmc-utils 6/6] mmc-utils: remove unused #includes
From: Uwe Kleine-König @ 2017-12-21 13:33 UTC (permalink / raw)
To: Chris Ball; +Cc: linux-mmc, kernel
In-Reply-To: <20171221133316.19466-1-u.kleine-koenig@pengutronix.de>
I didn't find any functions used for which the documentation specifies to
use (at least) one of the dropped headers.
The only Linux specific header <linux/fs.h> is needed for BLKGETSIZE,
document that.
---
mmc_cmds.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/mmc_cmds.c b/mmc_cmds.c
index cec947da1d4a..038dbd42e6bf 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -22,17 +22,13 @@
#include <string.h>
#include <sys/ioctl.h>
#include <sys/types.h>
-#include <dirent.h>
#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
-#include <libgen.h>
-#include <limits.h>
-#include <ctype.h>
#include <errno.h>
#include <stdint.h>
#include <assert.h>
-#include <linux/fs.h>
+#include <linux/fs.h> /* for BLKGETSIZE */
#include "mmc.h"
#include "mmc_cmds.h"
--
2.15.1
^ permalink raw reply related
* [PATCH] mmc: sdhci-esdhc-imx: Enable/Disable mmc clock during runtime suspend
From: Michael Trimarchi @ 2017-12-21 13:22 UTC (permalink / raw)
To: Adrian Hunter; +Cc: Ulf Hansson, linux-mmc
mmc clock can be stopped during runtime suspend and restart during runtime
resume. This let us know to not have any clock running and this reduce
the EMI of the device when the bus is not in use
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
drivers/mmc/host/sdhci-esdhc-imx.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 7123ef9..9a5e96f 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -196,6 +196,7 @@ struct pltfm_imx_data {
struct clk *clk_ipg;
struct clk *clk_ahb;
struct clk *clk_per;
+ unsigned int actual_clock;
enum {
NO_CMD_PENDING, /* no multiblock command pending*/
MULTIBLK_IN_PROCESS, /* exact multiblock cmd in process */
@@ -1346,6 +1347,9 @@ static int sdhci_esdhc_runtime_suspend(struct device *dev)
ret = sdhci_runtime_suspend_host(host);
+ imx_data->actual_clock = host->mmc->actual_clock;
+ esdhc_pltfm_set_clock(host, 0);
+
if (!sdhci_sdio_irq_enabled(host)) {
clk_disable_unprepare(imx_data->clk_per);
clk_disable_unprepare(imx_data->clk_ipg);
@@ -1366,6 +1370,7 @@ static int sdhci_esdhc_runtime_resume(struct device *dev)
clk_prepare_enable(imx_data->clk_ipg);
}
clk_prepare_enable(imx_data->clk_ahb);
+ esdhc_pltfm_set_clock(host, imx_data->actual_clock);
return sdhci_runtime_resume_host(host);
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 2/2] mmc: sunxi: Add runtime_pm support
From: Ulf Hansson @ 2017-12-21 12:21 UTC (permalink / raw)
To: Maxime Ripard; +Cc: Chen-Yu Tsai, linux-arm-kernel, linux-mmc@vger.kernel.org
In-Reply-To: <412579f183c42f6fe2ea04f294dfb788be5e4875.1513766964.git-series.maxime.ripard@free-electrons.com>
On 20 December 2017 at 11:50, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> So far, even if our card was not in use, we didn't shut down our main
> clock, which meant that it was still output on the MMC bus.
>
> While this obviously means that we could save some power there, it also
> created issues when it comes with EMC control since we'll have a perfect
> peak at the card clock rate.
>
> Let's implement runtime_pm with autosuspend so that we will shut down the
> clock when it's not been in use for quite some time.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/mmc/host/sunxi-mmc.c | 90 ++++++++++++++++++++++++-------------
> 1 file changed, 60 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index 3ce46ebd3488..c6a0bd0e0476 100644
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -35,6 +35,7 @@
> #include <linux/of_gpio.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> #include <linux/regulator/consumer.h>
> #include <linux/reset.h>
> #include <linux/scatterlist.h>
> @@ -1217,29 +1218,11 @@ static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host,
> return ret;
> }
>
> - ret = clk_prepare_enable(host->clk_mmc);
> - if (ret) {
> - dev_err(&pdev->dev, "Enable mmc clk err %d\n", ret);
> - goto error_disable_clk_ahb;
> - }
> -
> - ret = clk_prepare_enable(host->clk_output);
> - if (ret) {
> - dev_err(&pdev->dev, "Enable output clk err %d\n", ret);
> - goto error_disable_clk_mmc;
> - }
> -
> - ret = clk_prepare_enable(host->clk_sample);
> - if (ret) {
> - dev_err(&pdev->dev, "Enable sample clk err %d\n", ret);
> - goto error_disable_clk_output;
> - }
> -
Actually, I think you should keep all the above. Perhaps you may want
to move it to a separate helper function though, which the
->runtime_resume() callbacks can invoke as well.
More reasons to why, see the comment in the ->probe() function.
> if (!IS_ERR(host->reset)) {
> ret = reset_control_reset(host->reset);
> if (ret) {
> dev_err(&pdev->dev, "reset err %d\n", ret);
> - goto error_disable_clk_sample;
> + goto error_disable_clk_ahb;
> }
> }
>
> @@ -1258,12 +1241,6 @@ static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host,
> error_assert_reset:
> if (!IS_ERR(host->reset))
> reset_control_assert(host->reset);
> -error_disable_clk_sample:
> - clk_disable_unprepare(host->clk_sample);
> -error_disable_clk_output:
> - clk_disable_unprepare(host->clk_output);
> -error_disable_clk_mmc:
> - clk_disable_unprepare(host->clk_mmc);
> error_disable_clk_ahb:
> clk_disable_unprepare(host->clk_ahb);
> return ret;
> @@ -1280,6 +1257,7 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
> dev_err(&pdev->dev, "mmc alloc host failed\n");
> return -ENOMEM;
> }
> + platform_set_drvdata(pdev, mmc);
>
> host = mmc_priv(mmc);
> host->mmc = mmc;
> @@ -1340,12 +1318,16 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
> if (ret)
> goto error_free_dma;
>
> + pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> + pm_runtime_use_autosuspend(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
> +
This means in case you don't have CONFIG_PM set when compiling this
driver, the clocks will never be enabled using runtime PM.
I think it's good practice to deal with this, therefore I think you
should enable the clocks as before, and instead indicate that the
device is already runtime resumed.
In other words, before you call pm_runtime_enable(), invoke
pm_runtime_set_active().
> ret = mmc_add_host(mmc);
> if (ret)
> goto error_free_dma;
>
> dev_info(&pdev->dev, "base:0x%p irq:%u\n", host->reg_base, host->irq);
> - platform_set_drvdata(pdev, mmc);
> +
> return 0;
>
> error_free_dma:
> @@ -1361,27 +1343,75 @@ static int sunxi_mmc_remove(struct platform_device *pdev)
> struct sunxi_mmc_host *host = mmc_priv(mmc);
>
> mmc_remove_host(mmc);
> + pm_runtime_force_suspend(&pdev->dev);
Do you need the clocks to be enabled, while calling disable_irq() and
sunxi_mmc_reset_host()?
In such case you need to call pm_runtime_get_sync() here. Then move
pm_runtime_force_suspend() a few lines below, and later call
pm_runtime_put_noidle().
> disable_irq(host->irq);
> sunxi_mmc_reset_host(host);
>
> if (!IS_ERR(host->reset))
> reset_control_assert(host->reset);
>
> - clk_disable_unprepare(host->clk_sample);
> + dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma);
> + mmc_free_host(mmc);
> +
> + return 0;
> +}
> +
> +static int sunxi_mmc_runtime_resume(struct device *dev)
> +{
> + struct mmc_host *mmc = dev_get_drvdata(dev);
> + struct sunxi_mmc_host *host = mmc_priv(mmc);
> + int ret;
> +
> + ret = clk_prepare_enable(host->clk_mmc);
> + if (ret) {
> + dev_err(dev, "Enable mmc clk err %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(host->clk_output);
> + if (ret) {
> + dev_err(dev, "Enable output clk err %d\n", ret);
> + goto error_disable_clk_mmc;
> + }
> +
> + ret = clk_prepare_enable(host->clk_sample);
> + if (ret) {
> + dev_err(dev, "Enable sample clk err %d\n", ret);
> + goto error_disable_clk_output;
> + }
> +
> + return 0;
> +
> +error_disable_clk_output:
> clk_disable_unprepare(host->clk_output);
> +error_disable_clk_mmc:
> clk_disable_unprepare(host->clk_mmc);
> - clk_disable_unprepare(host->clk_ahb);
> + return ret;
> +}
>
> - dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma);
> - mmc_free_host(mmc);
> +static int sunxi_mmc_runtime_suspend(struct device *dev)
> +{
> + struct mmc_host *mmc = dev_get_drvdata(dev);
> + struct sunxi_mmc_host *host = mmc_priv(mmc);
> +
> + clk_disable_unprepare(host->clk_sample);
> + clk_disable_unprepare(host->clk_output);
> + clk_disable_unprepare(host->clk_mmc);
>
> return 0;
> }
>
> +static const struct dev_pm_ops sunxi_mmc_pm_ops = {
> + SET_RUNTIME_PM_OPS(sunxi_mmc_runtime_suspend,
> + sunxi_mmc_runtime_resume,
> + NULL)
> +};
> +
> static struct platform_driver sunxi_mmc_driver = {
> .driver = {
> .name = "sunxi-mmc",
> .of_match_table = of_match_ptr(sunxi_mmc_of_match),
> + .pm = &sunxi_mmc_pm_ops,
> },
> .probe = sunxi_mmc_probe,
> .remove = sunxi_mmc_remove,
> --
> git-series 0.9.1
Otherwise this looks good to me!
BTW, isn't system wide suspend/resume() also important to save power
for? That's rather simple to implement, after you have enabled runtime
PM support.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 11/12] mmc: sdhci-omap: Add support for MMC/SD controller in k2g SoC
From: Adrian Hunter @ 2017-12-21 9:15 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Ulf Hansson, Rob Herring, Tony Lindgren
Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
linux-omap, linux-arm-kernel, nsekhar
In-Reply-To: <20171214130941.26666-12-kishon@ti.com>
On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
> Add support for the new compatible added specifically to support
> k2g's MMC/SD controller.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/sdhci-omap.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index cddc3ad1331f..5e81e29383d9 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -767,6 +767,10 @@ static const struct sdhci_pltfm_data sdhci_omap_pdata = {
> .ops = &sdhci_omap_ops,
> };
>
> +static const struct sdhci_omap_data k2g_data = {
> + .offset = 0x200,
> +};
> +
> static const struct sdhci_omap_data dra7_data = {
> .offset = 0x200,
> .flags = SDHCI_OMAP_REQUIRE_IODELAY,
> @@ -774,6 +778,7 @@ static const struct sdhci_omap_data dra7_data = {
>
> static const struct of_device_id omap_sdhci_match[] = {
> { .compatible = "ti,dra7-sdhci", .data = &dra7_data },
> + { .compatible = "ti,k2g-sdhci", .data = &k2g_data },
> {},
> };
> MODULE_DEVICE_TABLE(of, omap_sdhci_match);
> @@ -882,6 +887,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> int ret;
> u32 offset;
> struct device *dev = &pdev->dev;
> + struct device_node *node = dev->of_node;
> struct sdhci_host *host;
> struct sdhci_pltfm_host *pltfm_host;
> struct sdhci_omap_host *omap_host;
> @@ -908,6 +914,9 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> return PTR_ERR(host);
> }
>
> + if (of_device_is_compatible(node, "ti,k2g-sdhci"))
> + host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
> +
> pltfm_host = sdhci_priv(host);
> omap_host = sdhci_pltfm_priv(pltfm_host);
> omap_host->host = host;
>
^ permalink raw reply
* Re: [PATCH 08/12] mmc: sdhci-omap: Add support to override f_max and iodelay from pdata
From: Adrian Hunter @ 2017-12-21 9:13 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Ulf Hansson, Rob Herring, Tony Lindgren
Cc: Mark Rutland, Russell King, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
nsekhar-l0cyMroinI0
In-Reply-To: <20171214130941.26666-9-kishon-l0cyMroinI0@public.gmane.org>
On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
> DRA74x EVM Rev H EVM comes with revision 2.0 silicon. However, earlier
> versions of EVM can come with either revision 1.1 or revision 1.0 of
> silicon.
>
> The device-tree file is written to support rev 2.0 of silicon.
> pdata-quirks are used to then override the settings needed for
> PG 1.1 silicon.
>
> PG 1.1 silicon has limitations w.r.t frequencies at which MMC1/2/3
> can operate as well as different IOdelay numbers.
>
> Add support in sdhci-omap driver to get platform data if available
> (added using pdata quirks) and override the data (max-frequency and
> iodelay data) obtained from device tree.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
> ---
> drivers/mmc/host/sdhci-omap.c | 17 ++++++++++++++++
> include/linux/platform_data/sdhci-omap.h | 35 ++++++++++++++++++++++++++++++++
> 2 files changed, 52 insertions(+)
> create mode 100644 include/linux/platform_data/sdhci-omap.h
>
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index 6dee275b2e57..cddc3ad1331f 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -22,6 +22,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/platform_data/sdhci-omap.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/regulator/consumer.h>
> @@ -102,6 +103,7 @@ struct sdhci_omap_data {
> };
>
> struct sdhci_omap_host {
> + char *version;
> void __iomem *base;
> struct device *dev;
> struct regulator *pbias;
> @@ -781,11 +783,18 @@ static struct pinctrl_state
> u32 *caps, u32 capmask)
> {
> struct device *dev = omap_host->dev;
> + char *version = omap_host->version;
> struct pinctrl_state *pinctrl_state = ERR_PTR(-ENODEV);
> + char str[20];
>
> if (!(*caps & capmask))
> goto ret;
>
> + if (version) {
> + sprintf(str, "%s-%s", mode, version);
snprintf please
> + pinctrl_state = pinctrl_lookup_state(omap_host->pinctrl, str);
Doesn't look like this 'pinctrl_state' is used?
> + }
> +
> pinctrl_state = pinctrl_lookup_state(omap_host->pinctrl, mode);
> if (IS_ERR(pinctrl_state)) {
> dev_err(dev, "no pinctrl state for %s mode", mode);
> @@ -879,6 +888,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> struct mmc_host *mmc;
> const struct of_device_id *match;
> struct sdhci_omap_data *data;
> + struct sdhci_omap_platform_data *platform_data;
>
> match = of_match_device(omap_sdhci_match, dev);
> if (!match)
> @@ -913,6 +923,13 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> if (ret)
> goto err_pltfm_free;
>
> + platform_data = dev_get_platdata(dev);
> + if (platform_data) {
> + omap_host->version = platform_data->version;
> + if (platform_data->max_freq)
> + mmc->f_max = platform_data->max_freq;
> + }
> +
> pltfm_host->clk = devm_clk_get(dev, "fck");
> if (IS_ERR(pltfm_host->clk)) {
> ret = PTR_ERR(pltfm_host->clk);
> diff --git a/include/linux/platform_data/sdhci-omap.h b/include/linux/platform_data/sdhci-omap.h
> new file mode 100644
> index 000000000000..a46e1240956a
> --- /dev/null
> +++ b/include/linux/platform_data/sdhci-omap.h
> @@ -0,0 +1,35 @@
> +/**
> + * SDHCI Controller Platform Data for TI's OMAP SoCs
> + *
> + * Copyright (C) 2017 Texas Instruments
> + * Author: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 of
> + * the License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef __SDHCI_OMAP_PDATA_H__
> +#define __SDHCI_OMAP_PDATA_H__
> +
> +struct sdhci_omap_platform_data {
> + const char *name;
> +
> + /*
> + * set if your board has components or wiring that limits the
> + * maximum frequency on the MMC bus
> + */
> + unsigned int max_freq;
> +
> + /* string specifying a particular variant of hardware */
> + char *version;
> +};
> +
> +#endif
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox