* [PATCH 0/2] mmc: allow mmc_alloc_host() and tmio_mmc_host_alloc()
From: Greg Kroah-Hartman @ 2016-11-10 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478784263-18777-1-git-send-email-yamada.masahiro@socionext.com>
On Thu, Nov 10, 2016 at 10:24:21PM +0900, Masahiro Yamada wrote:
>
> sdhci_alloc_host() returns an error pointer when it fails.
> but mmc_alloc_host() cannot.
>
> This series allow to propagate a proper error code
> when host-allocation fails.
Why? What can we really do about the error except give up? Why does
having a explicit error value make any difference to the caller, they
can't do anything different, right?
I suggest just leaving it as-is, it's simple, and you don't have to mess
with PTR_ERR() anywhere.
thanks,
greg k-h
^ permalink raw reply
* [PATCH 1/2] mmc: allow mmc_alloc_host() to return proper error code
From: Masahiro Yamada @ 2016-11-10 13:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478784263-18777-1-git-send-email-yamada.masahiro@socionext.com>
Currently, mmc_alloc_host() returns NULL on error, so its callers
cannot return anything but -ENOMEM when it fails, assuming the most
failure cases are due to memory shortage, but it is not true.
Allow mmc_alloc_host() to return an error pointer, then propagate
the proper error code to its callers.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/mmc/core/host.c | 11 ++++++-----
drivers/mmc/host/android-goldfish.c | 4 ++--
drivers/mmc/host/atmel-mci.c | 4 ++--
drivers/mmc/host/au1xmmc.c | 4 ++--
drivers/mmc/host/bfin_sdh.c | 4 ++--
drivers/mmc/host/cb710-mmc.c | 4 ++--
drivers/mmc/host/davinci_mmc.c | 4 ++--
drivers/mmc/host/dw_mmc.c | 4 ++--
drivers/mmc/host/jz4740_mmc.c | 4 ++--
drivers/mmc/host/mmc_spi.c | 9 ++++++---
drivers/mmc/host/mmci.c | 4 ++--
drivers/mmc/host/moxart-mmc.c | 4 ++--
drivers/mmc/host/mtk-sd.c | 4 ++--
drivers/mmc/host/mvsdio.c | 4 ++--
drivers/mmc/host/mxcmmc.c | 4 ++--
drivers/mmc/host/mxs-mmc.c | 4 ++--
drivers/mmc/host/omap.c | 4 ++--
drivers/mmc/host/omap_hsmmc.c | 4 ++--
drivers/mmc/host/pxamci.c | 4 ++--
drivers/mmc/host/rtsx_pci_sdmmc.c | 4 ++--
drivers/mmc/host/rtsx_usb_sdmmc.c | 4 ++--
drivers/mmc/host/s3cmci.c | 4 ++--
drivers/mmc/host/sdhci.c | 4 ++--
drivers/mmc/host/sdricoh_cs.c | 4 ++--
drivers/mmc/host/sh_mmcif.c | 4 ++--
drivers/mmc/host/sunxi-mmc.c | 4 ++--
drivers/mmc/host/tifm_sd.c | 4 ++--
drivers/mmc/host/tmio_mmc_pio.c | 2 +-
drivers/mmc/host/toshsd.c | 4 ++--
drivers/mmc/host/usdhi6rol0.c | 4 ++--
drivers/mmc/host/ushc.c | 4 ++--
drivers/mmc/host/via-sdmmc.c | 4 ++--
drivers/mmc/host/vub300.c | 4 ++--
drivers/mmc/host/wbsd.c | 4 ++--
drivers/mmc/host/wmt-sdmmc.c | 4 ++--
drivers/staging/greybus/sdio.c | 4 ++--
36 files changed, 79 insertions(+), 75 deletions(-)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 98f25ff..b3e13e0 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -349,7 +349,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
host = kzalloc(sizeof(struct mmc_host) + extra, GFP_KERNEL);
if (!host)
- return NULL;
+ return ERR_PTR(-ENOMEM);
/* scanning will be enabled when we're ready */
host->rescan_disable = 1;
@@ -357,7 +357,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
again:
if (!ida_pre_get(&mmc_host_ida, GFP_KERNEL)) {
kfree(host);
- return NULL;
+ return ERR_PTR(-ENOMEM);
}
spin_lock(&mmc_host_lock);
@@ -368,7 +368,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
goto again;
} else if (err) {
kfree(host);
- return NULL;
+ return ERR_PTR(err);
}
dev_set_name(&host->class_dev, "mmc%d", host->index);
@@ -379,9 +379,10 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
device_initialize(&host->class_dev);
device_enable_async_suspend(&host->class_dev);
- if (mmc_gpio_alloc(host)) {
+ err = mmc_gpio_alloc(host);
+ if (err < 0) {
put_device(&host->class_dev);
- return NULL;
+ return ERR_PTR(err);
}
spin_lock_init(&host->lock);
diff --git a/drivers/mmc/host/android-goldfish.c b/drivers/mmc/host/android-goldfish.c
index dca5518..7363663 100644
--- a/drivers/mmc/host/android-goldfish.c
+++ b/drivers/mmc/host/android-goldfish.c
@@ -467,8 +467,8 @@ static int goldfish_mmc_probe(struct platform_device *pdev)
return -ENXIO;
mmc = mmc_alloc_host(sizeof(struct goldfish_mmc_host), &pdev->dev);
- if (mmc == NULL) {
- ret = -ENOMEM;
+ if (IS_ERR(mmc)) {
+ ret = PTR_ERR(mmc);
goto err_alloc_host_failed;
}
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 0ad8ef5..d0cb112 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -2296,8 +2296,8 @@ static int atmci_init_slot(struct atmel_mci *host,
struct atmel_mci_slot *slot;
mmc = mmc_alloc_host(sizeof(struct atmel_mci_slot), &host->pdev->dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
slot = mmc_priv(mmc);
slot->mmc = mmc;
diff --git a/drivers/mmc/host/au1xmmc.c b/drivers/mmc/host/au1xmmc.c
index ed77fbfa..d97b409 100644
--- a/drivers/mmc/host/au1xmmc.c
+++ b/drivers/mmc/host/au1xmmc.c
@@ -951,9 +951,9 @@ static int au1xmmc_probe(struct platform_device *pdev)
int ret, iflag;
mmc = mmc_alloc_host(sizeof(struct au1xmmc_host), &pdev->dev);
- if (!mmc) {
+ if (IS_ERR(mmc)) {
dev_err(&pdev->dev, "no memory for mmc_host\n");
- ret = -ENOMEM;
+ ret = PTR_ERR(mmc);
goto out0;
}
diff --git a/drivers/mmc/host/bfin_sdh.c b/drivers/mmc/host/bfin_sdh.c
index 526231e..60c52d1 100644
--- a/drivers/mmc/host/bfin_sdh.c
+++ b/drivers/mmc/host/bfin_sdh.c
@@ -532,8 +532,8 @@ static int sdh_probe(struct platform_device *pdev)
}
mmc = mmc_alloc_host(sizeof(struct sdh_host), &pdev->dev);
- if (!mmc) {
- ret = -ENOMEM;
+ if (IS_ERR(mmc)) {
+ ret = PTR_ERR(mmc);
goto out;
}
diff --git a/drivers/mmc/host/cb710-mmc.c b/drivers/mmc/host/cb710-mmc.c
index 1087b4c..79ce871 100644
--- a/drivers/mmc/host/cb710-mmc.c
+++ b/drivers/mmc/host/cb710-mmc.c
@@ -692,8 +692,8 @@ static int cb710_mmc_init(struct platform_device *pdev)
u32 val;
mmc = mmc_alloc_host(sizeof(*reader), cb710_slot_dev(slot));
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
platform_set_drvdata(pdev, mmc);
diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index 8fa478c..225b9a8 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -1229,8 +1229,8 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
return -EBUSY;
mmc = mmc_alloc_host(sizeof(struct mmc_davinci_host), &pdev->dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
host = mmc_priv(mmc);
host->mmc = mmc; /* Important */
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4fcbc40..4f06528 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2598,8 +2598,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
u32 freq[2];
mmc = mmc_alloc_host(sizeof(struct dw_mci_slot), host->dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
slot = mmc_priv(mmc);
slot->id = id;
diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index 684087d..351dd68 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -998,9 +998,9 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
pdata = pdev->dev.platform_data;
mmc = mmc_alloc_host(sizeof(struct jz4740_mmc_host), &pdev->dev);
- if (!mmc) {
+ if (IS_ERR(mmc)) {
dev_err(&pdev->dev, "Failed to alloc mmc host structure\n");
- return -ENOMEM;
+ return PTR_ERR(mmc);
}
host = mmc_priv(mmc);
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index e77d79c..165f73e 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -1333,15 +1333,18 @@ static int mmc_spi_probe(struct spi_device *spi)
* NOTE if many systems use more than one MMC-over-SPI connector
* it'd save some memory to share this. That's evidently rare.
*/
- status = -ENOMEM;
ones = kmalloc(MMC_SPI_BLOCKSIZE, GFP_KERNEL);
- if (!ones)
+ if (!ones) {
+ status = -ENOMEM;
goto nomem;
+ }
memset(ones, 0xff, MMC_SPI_BLOCKSIZE);
mmc = mmc_alloc_host(sizeof(*host), &spi->dev);
- if (!mmc)
+ if (IS_ERR(mmc)) {
+ status = PTR_ERR(mmc);
goto nomem;
+ }
mmc->ops = &mmc_spi_ops;
mmc->max_blk_size = MMC_SPI_BLOCKSIZE;
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index df990bb..5779b57 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1509,8 +1509,8 @@ static int mmci_probe(struct amba_device *dev,
}
mmc = mmc_alloc_host(sizeof(struct mmci_host), &dev->dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
ret = mmci_of_parse(np, mmc);
if (ret)
diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index bbad309..f453e55 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -568,9 +568,9 @@ static int moxart_probe(struct platform_device *pdev)
u32 i;
mmc = mmc_alloc_host(sizeof(struct moxart_host), dev);
- if (!mmc) {
+ if (IS_ERR(mmc)) {
dev_err(dev, "mmc_alloc_host failed\n");
- ret = -ENOMEM;
+ ret = PTR_ERR(mmc);
goto out;
}
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 84e9afc..ef28f64 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -1494,8 +1494,8 @@ static int msdc_drv_probe(struct platform_device *pdev)
}
/* Allocate MMC host for this device */
mmc = mmc_alloc_host(sizeof(struct msdc_host), &pdev->dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
host = mmc_priv(mmc);
ret = mmc_of_parse(mmc);
diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index 42296e5..5594f58 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -711,8 +711,8 @@ static int mvsd_probe(struct platform_device *pdev)
return -ENXIO;
mmc = mmc_alloc_host(sizeof(struct mvsd_host), &pdev->dev);
- if (!mmc) {
- ret = -ENOMEM;
+ if (IS_ERR(mmc)) {
+ ret = PTR_ERR(mmc);
goto out;
}
diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index fb3ca82..5bdc644 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -1018,8 +1018,8 @@ static int mxcmci_probe(struct platform_device *pdev)
return -EINVAL;
mmc = mmc_alloc_host(sizeof(*host), &pdev->dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
host = mmc_priv(mmc);
diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index d839147..7f72bb4 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -586,8 +586,8 @@ static int mxs_mmc_probe(struct platform_device *pdev)
return irq_err;
mmc = mmc_alloc_host(sizeof(struct mxs_mmc_host), &pdev->dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
host = mmc_priv(mmc);
ssp = &host->ssp;
diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index be3c49f..b1ec63f 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -1227,8 +1227,8 @@ static int mmc_omap_new_slot(struct mmc_omap_host *host, int id)
int r;
mmc = mmc_alloc_host(sizeof(struct mmc_omap_slot), host->dev);
- if (mmc == NULL)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
slot = mmc_priv(mmc);
slot->host = host;
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 5f2f24a..6154b55 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2024,8 +2024,8 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
return PTR_ERR(base);
mmc = mmc_alloc_host(sizeof(struct omap_hsmmc_host), &pdev->dev);
- if (!mmc) {
- ret = -ENOMEM;
+ if (IS_ERR(mmc)) {
+ ret = PTR_ERR(mmc);
goto err;
}
diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index c763b40..9e9b02f 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -652,8 +652,8 @@ static int pxamci_probe(struct platform_device *pdev)
return irq;
mmc = mmc_alloc_host(sizeof(struct pxamci_host), &pdev->dev);
- if (!mmc) {
- ret = -ENOMEM;
+ if (IS_ERR(mmc)) {
+ ret = PTR_ERR(mmc);
goto out;
}
diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
index 3ccaa14..551536e 100644
--- a/drivers/mmc/host/rtsx_pci_sdmmc.c
+++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
@@ -1399,8 +1399,8 @@ static int rtsx_pci_sdmmc_drv_probe(struct platform_device *pdev)
dev_dbg(&(pdev->dev), ": Realtek PCI-E SDMMC controller found\n");
mmc = mmc_alloc_host(sizeof(*host), &pdev->dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
host = mmc_priv(mmc);
host->pcr = pcr;
diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
index 6e9c0f8..443679f 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -1363,8 +1363,8 @@ static int rtsx_usb_sdmmc_drv_probe(struct platform_device *pdev)
dev_dbg(&(pdev->dev), ": Realtek USB SD/MMC controller found\n");
mmc = mmc_alloc_host(sizeof(*host), &pdev->dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
host = mmc_priv(mmc);
host->ucr = ucr;
diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
index c531dee..aacc5cf 100644
--- a/drivers/mmc/host/s3cmci.c
+++ b/drivers/mmc/host/s3cmci.c
@@ -1556,8 +1556,8 @@ static int s3cmci_probe(struct platform_device *pdev)
is2440 = platform_get_device_id(pdev)->driver_data;
mmc = mmc_alloc_host(sizeof(struct s3cmci_host), &pdev->dev);
- if (!mmc) {
- ret = -ENOMEM;
+ if (IS_ERR(mmc)) {
+ ret = PTR_ERR(mmc);
goto probe_out;
}
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 71654b9..eb8199e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2946,8 +2946,8 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
WARN_ON(dev == NULL);
mmc = mmc_alloc_host(sizeof(struct sdhci_host) + priv_size, dev);
- if (!mmc)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(mmc))
+ return ERR_CAST(mmc);
host = mmc_priv(mmc);
host->mmc = mmc;
diff --git a/drivers/mmc/host/sdricoh_cs.c b/drivers/mmc/host/sdricoh_cs.c
index 5ff26ab..6d2f671 100644
--- a/drivers/mmc/host/sdricoh_cs.c
+++ b/drivers/mmc/host/sdricoh_cs.c
@@ -424,9 +424,9 @@ static int sdricoh_init_mmc(struct pci_dev *pci_dev,
/* allocate privdata */
mmc = pcmcia_dev->priv =
mmc_alloc_host(sizeof(struct sdricoh_host), &pcmcia_dev->dev);
- if (!mmc) {
+ if (IS_ERR(mmc)) {
dev_err(dev, "mmc_alloc_host failed\n");
- result = -ENOMEM;
+ result = PTR_ERR(mmc);
goto unmap_io;
}
host = mmc_priv(mmc);
diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 9007784..c2affc6 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1432,8 +1432,8 @@ static int sh_mmcif_probe(struct platform_device *pdev)
return PTR_ERR(reg);
mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
ret = mmc_of_parse(mmc);
if (ret < 0)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index c0a5c67..a3cb388 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1212,9 +1212,9 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
int ret;
mmc = mmc_alloc_host(sizeof(struct sunxi_mmc_host), &pdev->dev);
- if (!mmc) {
+ if (IS_ERR(mmc)) {
dev_err(&pdev->dev, "mmc alloc host failed\n");
- return -ENOMEM;
+ return PTR_ERR(mmc);
}
host = mmc_priv(mmc);
diff --git a/drivers/mmc/host/tifm_sd.c b/drivers/mmc/host/tifm_sd.c
index 93c4b40..b088cb8 100644
--- a/drivers/mmc/host/tifm_sd.c
+++ b/drivers/mmc/host/tifm_sd.c
@@ -958,8 +958,8 @@ static int tifm_sd_probe(struct tifm_dev *sock)
}
mmc = mmc_alloc_host(sizeof(struct tifm_sd), &sock->dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
host = mmc_priv(mmc);
tifm_set_drvdata(sock, mmc);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 7005676..18106fc 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -1013,7 +1013,7 @@ struct tmio_mmc_host*
struct mmc_host *mmc;
mmc = mmc_alloc_host(sizeof(struct tmio_mmc_host), &pdev->dev);
- if (!mmc)
+ if (IS_ERR(mmc))
return NULL;
host = mmc_priv(mmc);
diff --git a/drivers/mmc/host/toshsd.c b/drivers/mmc/host/toshsd.c
index 553ef41..7b086ed 100644
--- a/drivers/mmc/host/toshsd.c
+++ b/drivers/mmc/host/toshsd.c
@@ -617,8 +617,8 @@ static int toshsd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
return ret;
mmc = mmc_alloc_host(sizeof(struct toshsd_host), &pdev->dev);
- if (!mmc) {
- ret = -ENOMEM;
+ if (IS_ERR(mmc)) {
+ ret = PTR_ERR(mmc);
goto err;
}
diff --git a/drivers/mmc/host/usdhi6rol0.c b/drivers/mmc/host/usdhi6rol0.c
index 1bd5f1a..e9d0126 100644
--- a/drivers/mmc/host/usdhi6rol0.c
+++ b/drivers/mmc/host/usdhi6rol0.c
@@ -1753,8 +1753,8 @@ static int usdhi6_probe(struct platform_device *pdev)
return -ENODEV;
mmc = mmc_alloc_host(sizeof(struct usdhi6_host), dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
ret = mmc_regulator_get_supply(mmc);
if (ret == -EPROBE_DEFER)
diff --git a/drivers/mmc/host/ushc.c b/drivers/mmc/host/ushc.c
index d2c386f..6937021 100644
--- a/drivers/mmc/host/ushc.c
+++ b/drivers/mmc/host/ushc.c
@@ -427,8 +427,8 @@ static int ushc_probe(struct usb_interface *intf, const struct usb_device_id *id
int ret;
mmc = mmc_alloc_host(sizeof(struct ushc_data), &intf->dev);
- if (mmc == NULL)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
ushc = mmc_priv(mmc);
usb_set_intfdata(intf, ushc);
diff --git a/drivers/mmc/host/via-sdmmc.c b/drivers/mmc/host/via-sdmmc.c
index 63fac78..010bfdb 100644
--- a/drivers/mmc/host/via-sdmmc.c
+++ b/drivers/mmc/host/via-sdmmc.c
@@ -1108,8 +1108,8 @@ static int via_sd_probe(struct pci_dev *pcidev,
pci_write_config_byte(pcidev, VIA_CRDR_PCI_DBG_MODE, 0);
mmc = mmc_alloc_host(sizeof(struct via_crdr_mmc_host), &pcidev->dev);
- if (!mmc) {
- ret = -ENOMEM;
+ if (IS_ERR(mmc)) {
+ ret = PTR_ERR(mmc);
goto release;
}
diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index bb3e0d1..d052c23 100644
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -2125,8 +2125,8 @@ static int vub300_probe(struct usb_interface *interface,
}
/* this also allocates memory for our VUB300 mmc host device */
mmc = mmc_alloc_host(sizeof(struct vub300_mmc_host), &udev->dev);
- if (!mmc) {
- retval = -ENOMEM;
+ if (IS_ERR(mmc)) {
+ retval = PTR_ERR(mmc);
dev_err(&udev->dev, "not enough memory for the mmc_host\n");
goto error4;
}
diff --git a/drivers/mmc/host/wbsd.c b/drivers/mmc/host/wbsd.c
index c3fd16d..40f8fd6 100644
--- a/drivers/mmc/host/wbsd.c
+++ b/drivers/mmc/host/wbsd.c
@@ -1204,8 +1204,8 @@ static int wbsd_alloc_mmc(struct device *dev)
* Allocate MMC structure.
*/
mmc = mmc_alloc_host(sizeof(struct wbsd_host), dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
host = mmc_priv(mmc);
host->mmc = mmc;
diff --git a/drivers/mmc/host/wmt-sdmmc.c b/drivers/mmc/host/wmt-sdmmc.c
index 5af0055..f37f9a4cd 100644
--- a/drivers/mmc/host/wmt-sdmmc.c
+++ b/drivers/mmc/host/wmt-sdmmc.c
@@ -782,9 +782,9 @@ static int wmt_mci_probe(struct platform_device *pdev)
}
mmc = mmc_alloc_host(sizeof(struct wmt_mci_priv), &pdev->dev);
- if (!mmc) {
+ if (IS_ERR(mmc)) {
dev_err(&pdev->dev, "Failed to allocate mmc_host\n");
- ret = -ENOMEM;
+ ret = PTR_ERR(mmc);
goto fail1;
}
diff --git a/drivers/staging/greybus/sdio.c b/drivers/staging/greybus/sdio.c
index 5649ef1..01c443d 100644
--- a/drivers/staging/greybus/sdio.c
+++ b/drivers/staging/greybus/sdio.c
@@ -768,8 +768,8 @@ static int gb_sdio_probe(struct gbphy_device *gbphy_dev,
int ret = 0;
mmc = mmc_alloc_host(sizeof(*host), &gbphy_dev->dev);
- if (!mmc)
- return -ENOMEM;
+ if (IS_ERR(mmc))
+ return PTR_ERR(mmc);
connection = gb_connection_create(gbphy_dev->bundle,
le16_to_cpu(gbphy_dev->cport_desc->id),
--
1.9.1
^ permalink raw reply related
* [PATCH 0/2] mmc: allow mmc_alloc_host() and tmio_mmc_host_alloc()
From: Masahiro Yamada @ 2016-11-10 13:24 UTC (permalink / raw)
To: linux-arm-kernel
sdhci_alloc_host() returns an error pointer when it fails.
but mmc_alloc_host() cannot.
This series allow to propagate a proper error code
when host-allocation fails.
Masahiro Yamada (2):
mmc: allow mmc_alloc_host() to return proper error code
mmc: tmio: allow tmio_mmc_host_alloc() to return proper error code
drivers/mmc/core/host.c | 11 ++++++-----
drivers/mmc/host/android-goldfish.c | 4 ++--
drivers/mmc/host/atmel-mci.c | 4 ++--
drivers/mmc/host/au1xmmc.c | 4 ++--
drivers/mmc/host/bfin_sdh.c | 4 ++--
drivers/mmc/host/cb710-mmc.c | 4 ++--
drivers/mmc/host/davinci_mmc.c | 4 ++--
drivers/mmc/host/dw_mmc.c | 4 ++--
drivers/mmc/host/jz4740_mmc.c | 4 ++--
drivers/mmc/host/mmc_spi.c | 9 ++++++---
drivers/mmc/host/mmci.c | 4 ++--
drivers/mmc/host/moxart-mmc.c | 4 ++--
drivers/mmc/host/mtk-sd.c | 4 ++--
drivers/mmc/host/mvsdio.c | 4 ++--
drivers/mmc/host/mxcmmc.c | 4 ++--
drivers/mmc/host/mxs-mmc.c | 4 ++--
drivers/mmc/host/omap.c | 4 ++--
drivers/mmc/host/omap_hsmmc.c | 4 ++--
drivers/mmc/host/pxamci.c | 4 ++--
drivers/mmc/host/rtsx_pci_sdmmc.c | 4 ++--
drivers/mmc/host/rtsx_usb_sdmmc.c | 4 ++--
drivers/mmc/host/s3cmci.c | 4 ++--
drivers/mmc/host/sdhci.c | 4 ++--
drivers/mmc/host/sdricoh_cs.c | 4 ++--
drivers/mmc/host/sh_mmcif.c | 4 ++--
drivers/mmc/host/sh_mobile_sdhi.c | 4 ++--
drivers/mmc/host/sunxi-mmc.c | 4 ++--
drivers/mmc/host/tifm_sd.c | 4 ++--
drivers/mmc/host/tmio_mmc.c | 4 +++-
drivers/mmc/host/tmio_mmc_pio.c | 4 ++--
drivers/mmc/host/toshsd.c | 4 ++--
drivers/mmc/host/usdhi6rol0.c | 4 ++--
drivers/mmc/host/ushc.c | 4 ++--
drivers/mmc/host/via-sdmmc.c | 4 ++--
drivers/mmc/host/vub300.c | 4 ++--
drivers/mmc/host/wbsd.c | 4 ++--
drivers/mmc/host/wmt-sdmmc.c | 4 ++--
drivers/staging/greybus/sdio.c | 4 ++--
38 files changed, 85 insertions(+), 79 deletions(-)
--
1.9.1
^ permalink raw reply
* PM regression with LED changes in next-20161109
From: Hans de Goede @ 2016-11-10 13:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5bd5333e-0dbb-6333-0a48-ca4d3a990f9c@samsung.com>
Hi,
On 10-11-16 13:56, Jacek Anaszewski wrote:
> Hi,
>
> On 11/10/2016 09:49 AM, Hans de Goede wrote:
>> Hi,
>>
>> On 09-11-16 21:45, Jacek Anaszewski wrote:
>>> Hi Tony,
>>>
>>> On 11/09/2016 08:23 PM, Tony Lindgren wrote:
>>>> Hi,
>>>>
>>>> Looks like commit 883d32ce3385 ("leds: core: Add support for poll()ing
>>>> the sysfs brightness attr for changes.") breaks runtime PM for me.
>>>>
>>>> On my omap dm3730 based test system, idle power consumption is over 70
>>>> times higher now with this patch! It goes from about 6mW for the core
>>>> system to over 440mW during idle meaning there's some busy timer now
>>>> active.
>>>>
>>>> Reverting this patch fixes the issue. Any ideas?
>>>
>>> Thanks for the report. This is probably caused by sysfs_notify_dirent().
>>> I'm afraid that we can't keep this feature in the current shape.
>>> Hans, I'm dropping the patch. We probably will have to delegate this
>>> call to a workqueue task. Think about use cases when the LED is blinked
>>> with high frequency e.g. from ledtrig-disk.c.
>>
>> sysfs_notify_dirent() already uses a workqueue, here is the actual
>> implementation of it (from fs/kernfs/file.c) :
>>
>> void kernfs_notify(struct kernfs_node *kn)
>> {
>> static DECLARE_WORK(kernfs_notify_work, kernfs_notify_workfn);
>> unsigned long flags;
>>
>> if (WARN_ON(kernfs_type(kn) != KERNFS_FILE))
>> return;
>>
>> spin_lock_irqsave(&kernfs_notify_lock, flags);
>> if (!kn->attr.notify_next) {
>> kernfs_get(kn);
>> kn->attr.notify_next = kernfs_notify_list;
>> kernfs_notify_list = kn;
>> schedule_work(&kernfs_notify_work);
>> }
>> spin_unlock_irqrestore(&kernfs_notify_lock, flags);
>> }
>
> Indeed. As a next step of this investigation Tony could disable
> particular calls made in kernfs_notify_workfn to check what
> exactly causes excessive power consumption.
>
>> So using a workqueue is not going to help. Note that I already
>> feared this, which is why my initial implementation only called
>> sysfs_notify_dirent() for user initiated changes and not for
>> triggers / blinking.
>
> AFAIR there were no calls to led_notify_brightness_change() in
> the initial implementation and it was entirely predestined for
> being called by LED class drivers on brightness changes made
> by firmware.
>
>> I think we may need to reconsider what getting the brightness
>> sysfs atrribute actually returns. Currently when a LED is
>> blinking it will return 0 resp. the actual brightness depending
>> on when in the blink cycle the user reads the brightness
>> sysfs atrribute.
>>
>> So a user can do "echo 128 > brightness && cat brightness" and
>> get out 0, or 128, depending purely on timing.
>>
>> This seems to contradict what Documentation/ABI/testing/sysfs-class-led
>> has to say:
>>
>> What: /sys/class/leds/<led>/brightness
>> Date: March 2006
>> KernelVersion: 2.6.17
>> Contact: Richard Purdie <rpurdie@rpsys.net>
>> Description:
>> Set the brightness of the LED. Most LEDs don't
>> have hardware brightness support, so will just be turned
>> on for
>> non-zero brightness settings. The value is between 0 and
>> /sys/class/leds/<led>/max_brightness.
>>
>> Writing 0 to this file clears active trigger.
>>
>> Writing non-zero to this file while trigger is active
>> changes the
>> top brightness trigger is going to use.
>>
>> Even though it only talks about writing, the logical thing would be for
>> reading to be the exact opposite of writing, so we would get:
>>
>> Reading from this file while a trigger is active returns
>> the
>> top brightness trigger is going to use.
>>
>> The current docs say not about (sw) blinking, but that should be treated
>> just
>> like a trigger IMHO.
>
> You'r right, we should describe the semantics on reading, but it would
> have to be as follows:
>
> Reading from this file returns LED brightness at given moment, i.e.
> even though LED class device brightness setting is greater than 0, the
> momentary brightness can be 0 if the readout occurred during low phase
> of blink cycle.
Why would it need to read like this, because this is the current behavior ?
I doubt anyone is relying on this current behavior because it is really
unpredictable which value one can get.
I believe it would be better to change the read semantics to follow
the write semantics, this would be much more consistent.
Making the read behavior match the write behavior should be easy I would
be happy to write a patch for this.
>> If we can get consensus on what the read behavior for the brightness
>> attribute
>> should be, then I think that a better poll() behavior will automatically
>> follow
>> from that.
>
> It seems that we should get back to your initial approach. i.e. only
> brightness changes caused by hardware should be reported.
Ok, if you really want to keep the read behavior as is, I can provide
an updated patch for this.
Regards,
Hans
^ permalink raw reply
* PM regression with LED changes in next-20161109
From: Jacek Anaszewski @ 2016-11-10 12:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f627c778-4378-e5da-09dd-3e0fb2ea4aba@redhat.com>
Hi,
On 11/10/2016 09:49 AM, Hans de Goede wrote:
> Hi,
>
> On 09-11-16 21:45, Jacek Anaszewski wrote:
>> Hi Tony,
>>
>> On 11/09/2016 08:23 PM, Tony Lindgren wrote:
>>> Hi,
>>>
>>> Looks like commit 883d32ce3385 ("leds: core: Add support for poll()ing
>>> the sysfs brightness attr for changes.") breaks runtime PM for me.
>>>
>>> On my omap dm3730 based test system, idle power consumption is over 70
>>> times higher now with this patch! It goes from about 6mW for the core
>>> system to over 440mW during idle meaning there's some busy timer now
>>> active.
>>>
>>> Reverting this patch fixes the issue. Any ideas?
>>
>> Thanks for the report. This is probably caused by sysfs_notify_dirent().
>> I'm afraid that we can't keep this feature in the current shape.
>> Hans, I'm dropping the patch. We probably will have to delegate this
>> call to a workqueue task. Think about use cases when the LED is blinked
>> with high frequency e.g. from ledtrig-disk.c.
>
> sysfs_notify_dirent() already uses a workqueue, here is the actual
> implementation of it (from fs/kernfs/file.c) :
>
> void kernfs_notify(struct kernfs_node *kn)
> {
> static DECLARE_WORK(kernfs_notify_work, kernfs_notify_workfn);
> unsigned long flags;
>
> if (WARN_ON(kernfs_type(kn) != KERNFS_FILE))
> return;
>
> spin_lock_irqsave(&kernfs_notify_lock, flags);
> if (!kn->attr.notify_next) {
> kernfs_get(kn);
> kn->attr.notify_next = kernfs_notify_list;
> kernfs_notify_list = kn;
> schedule_work(&kernfs_notify_work);
> }
> spin_unlock_irqrestore(&kernfs_notify_lock, flags);
> }
Indeed. As a next step of this investigation Tony could disable
particular calls made in kernfs_notify_workfn to check what
exactly causes excessive power consumption.
> So using a workqueue is not going to help. Note that I already
> feared this, which is why my initial implementation only called
> sysfs_notify_dirent() for user initiated changes and not for
> triggers / blinking.
AFAIR there were no calls to led_notify_brightness_change() in
the initial implementation and it was entirely predestined for
being called by LED class drivers on brightness changes made
by firmware.
> I think we may need to reconsider what getting the brightness
> sysfs atrribute actually returns. Currently when a LED is
> blinking it will return 0 resp. the actual brightness depending
> on when in the blink cycle the user reads the brightness
> sysfs atrribute.
>
> So a user can do "echo 128 > brightness && cat brightness" and
> get out 0, or 128, depending purely on timing.
>
> This seems to contradict what Documentation/ABI/testing/sysfs-class-led
> has to say:
>
> What: /sys/class/leds/<led>/brightness
> Date: March 2006
> KernelVersion: 2.6.17
> Contact: Richard Purdie <rpurdie@rpsys.net>
> Description:
> Set the brightness of the LED. Most LEDs don't
> have hardware brightness support, so will just be turned
> on for
> non-zero brightness settings. The value is between 0 and
> /sys/class/leds/<led>/max_brightness.
>
> Writing 0 to this file clears active trigger.
>
> Writing non-zero to this file while trigger is active
> changes the
> top brightness trigger is going to use.
>
> Even though it only talks about writing, the logical thing would be for
> reading to be the exact opposite of writing, so we would get:
>
> Reading from this file while a trigger is active returns
> the
> top brightness trigger is going to use.
>
> The current docs say not about (sw) blinking, but that should be treated
> just
> like a trigger IMHO.
You'r right, we should describe the semantics on reading, but it would
have to be as follows:
Reading from this file returns LED brightness at given moment, i.e.
even though LED class device brightness setting is greater than 0, the
momentary brightness can be 0 if the readout occurred during low phase
of blink cycle.
> If we can get consensus on what the read behavior for the brightness
> attribute
> should be, then I think that a better poll() behavior will automatically
> follow
> from that.
It seems that we should get back to your initial approach. i.e. only
brightness changes caused by hardware should be reported.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* [RFC v2 4/8] iommu: Add a list of iommu_reserved_region in iommu_domain
From: Robin Murphy @ 2016-11-10 12:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6c2e1c81-e951-a284-b547-733369128e7e@redhat.com>
On 10/11/16 12:14, Auger Eric wrote:
> Hi Robin,
>
> On 10/11/2016 12:54, Robin Murphy wrote:
>> Hi Eric,
>>
>> On 10/11/16 11:22, Auger Eric wrote:
>>> Hi Robin,
>>>
>>> On 04/11/2016 15:00, Robin Murphy wrote:
>>>> Hi Eric,
>>>>
>>>> Thanks for posting this new series - the bottom-up approach is a lot
>>>> easier to reason about :)
>>>>
>>>> On 04/11/16 11:24, Eric Auger wrote:
>>>>> Introduce a new iommu_reserved_region struct. This embodies
>>>>> an IOVA reserved region that cannot be used along with the IOMMU
>>>>> API. The list is protected by a dedicated mutex.
>>>>
>>>> In the light of these patches, I think I'm settling into agreement that
>>>> the iommu_domain is the sweet spot for accessing this information - the
>>>> underlying magic address ranges might be properties of various bits of
>>>> hardware many of which aren't the IOMMU itself, but they only start to
>>>> matter at the point you start wanting to use an IOMMU domain at the
>>>> higher level. Therefore, having a callback in the domain ops to pull
>>>> everything together fits rather neatly.
>>> Using get_dm_regions could have make sense but this approach now is
>>> ruled out by sysfs API approach. If attribute file is bound to be used
>>> before iommu domains are created, we cannot rely on any iommu_domain
>>> based callback. Back to square 1?
>>
>> I think it's still OK. The thing about these reserved regions is that as
>> a property of the underlying hardware they must be common to any domain
>> for a given group, therefore without loss of generality we can simply
>> query group->domain->ops->get_dm_regions(), and can expect the reserved
>> ones will be the same regardless of what domain that points to
>> (identity-mapped IVMD/RMRR/etc.
> Are they really? P2P reserved regions depend on iommu_domain right?
Indeed. To use the SMMU example, reprogramming S2CRs to target a
different context bank (i.e. attaching to a different domain) won't
affect the fact that the transactions aren't even reaching the SMMU in
the first place. That's why we need the exact same information for DMA
domains, thus why getting it for free via the dm_regions mechanism would
be really neat.
The visibility of P2P regions, doorbells, etc. for a given device is
ultimately a property of the hardware topology, and topology happens to
be what the iommu_group already represents*. There looks to be a snag
when we try to consider the addresses of such regions, since addresses
are the business of iommu_domains, not groups, but as there is a 1:1
relationship between a group and its default domain, things end up tying
together quite neatly.
Robin.
*in fact, I've just had an idea that way we check ACS paths to determine
groups might similarly be a finer-grained way to detect what P2P
regions, if any, are actually relevant.
> Now I did not consider default_domain usability, I acknowledge. I will
> send a POC anyway.
>
> regions may not be, but we'd be
>> filtering those out anyway). The default DMA domains need this
>> information too, and since those are allocated at group creation,
>> group->domain should always be non-NULL and interrogable.
>>
>> Plus, the groups are already there in sysfs, and, being representative
>> of device topology, would seem to be an ideal place to expose the
>> addressing limitations relevant to the devices within them. This really
>> feels like it's all falling into place (on the kernel end, at least, I'm
>> sticking to the sidelines on the userspace discussion ;)).
>
> Thanks
>
> Eric
>>
>> Robin.
>>
>>>
>>> Thanks
>>>
>>> Eric
>>>>
>>>>>
>>>>> An iommu domain now owns a list of those.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>>
>>>>> ---
>>>>> ---
>>>>> drivers/iommu/iommu.c | 2 ++
>>>>> include/linux/iommu.h | 17 +++++++++++++++++
>>>>> 2 files changed, 19 insertions(+)
>>>>>
>>>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>>>> index 9a2f196..0af07492 100644
>>>>> --- a/drivers/iommu/iommu.c
>>>>> +++ b/drivers/iommu/iommu.c
>>>>> @@ -1061,6 +1061,8 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
>>>>>
>>>>> domain->ops = bus->iommu_ops;
>>>>> domain->type = type;
>>>>> + INIT_LIST_HEAD(&domain->reserved_regions);
>>>>> + mutex_init(&domain->resv_mutex);
>>>>> /* Assume all sizes by default; the driver may override this later */
>>>>> domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap;
>>>>>
>>>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>>>> index 436dc21..0f2eb64 100644
>>>>> --- a/include/linux/iommu.h
>>>>> +++ b/include/linux/iommu.h
>>>>> @@ -84,6 +84,8 @@ struct iommu_domain {
>>>>> void *handler_token;
>>>>> struct iommu_domain_geometry geometry;
>>>>> void *iova_cookie;
>>>>> + struct list_head reserved_regions;
>>>>> + struct mutex resv_mutex; /* protects the reserved region list */
>>>>> };
>>>>>
>>>>> enum iommu_cap {
>>>>> @@ -131,6 +133,21 @@ struct iommu_dm_region {
>>>>> int prot;
>>>>> };
>>>>>
>>>>> +/**
>>>>> + * struct iommu_reserved_region - descriptor for a reserved iova region
>>>>> + * @list: Linked list pointers
>>>>> + * @start: IOVA base address of the region
>>>>> + * @length: Length of the region in bytes
>>>>> + */
>>>>> +struct iommu_reserved_region {
>>>>> + struct list_head list;
>>>>> + dma_addr_t start;
>>>>> + size_t length;
>>>>> +};
>>>>
>>>> Looking at this in context with the dm_region above, though, I come to
>>>> the surprising realisation that these *are* dm_regions, even at the
>>>> fundamental level - on the one hand you've got physical addresses which
>>>> can't be remapped (because something is already using them), while on
>>>> the other you've got physical addresses which can't be remapped (because
>>>> the IOMMU is incapable). In fact for reserved regions *other* than our
>>>> faked-up MSI region there's no harm if the IOMMU were to actually
>>>> identity-map them.
>>>>
>>>> Let's just add this to the existing infrastructure, either with some
>>>> kind of IOMMU_NOMAP flag or simply prot = 0. That way it automatically
>>>> gets shared between the VFIO and DMA cases for free!
>>>>
>>>> Robin.
>>>>
>>>>> +
>>>>> +#define iommu_reserved_region_for_each(resv, d) \
>>>>> + list_for_each_entry(resv, &(d)->reserved_regions, list)
>>>>> +
>>>>> #ifdef CONFIG_IOMMU_API
>>>>>
>>>>> /**
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> linux-arm-kernel at lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>>
>>
^ permalink raw reply
* [PATCH v3 1/2] ARM: EXYNOS: Remove static mapping of SCU SFR
From: pankaj.dubey @ 2016-11-10 12:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5066943.nydi3GQ3KP@wuerfel>
Hi Arnd,
On Thursday 10 November 2016 05:24 PM, Arnd Bergmann wrote:
> On Wednesday, November 9, 2016 5:45:54 PM CET Pankaj Dubey wrote:
>> Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based
>> boards. Instead use mapping from device tree node of SCU.
>>
>> NOTE: This patch has dependency on DT file of any such CORTEX-A9 SoC
>> based boards, in the absence of SCU device node in DTS file, only single
>> CPU will boot. So if you are using OUT-OF-TREE DTS file of CORTEX-A9 based
>> Exynos SoC make sure to add SCU device node to DTS file for SMP boot.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
>
> With CONFIG_SMP disabled, I now get this build failure:
>
Sorry, I missed this part and did not check with CONFIG_SMP disabled.
> arch/arm/mach-exynos/pm.o: In function `exynos_enter_aftr':
> pm.c:(.text.exynos_enter_aftr+0xec): undefined reference to `exynos_scu_enable'
> arch/arm/mach-exynos/suspend.o: In function `exynos_pm_resume':
> suspend.c:(.text.exynos_pm_resume+0x78): undefined reference to `exynos_scu_enable'
>
> Please fix. I have applied a patch locally (see below), but don't know
> if that is the best solution. As we seem to duplicate that code across
> several platforms, I wonder why we don't just put it into the core scu
> implementation.
>
When I checked scu_enable declaration it is defined in
arch/arm/include/asm/smp_scu.h as:
#if defined(CONFIG_SMP) && defined(CONFIG_HAVE_ARM_SCU)
void scu_enable(void __iomem *scu_base);
#else
static inline void scu_enable(void __iomem *scu_base) {}
#endif
So if CONFIG_SMP is disable then there is no sense of exynos_scu_enable
as well. So wow about using below patch?
--------------------------------------------------------
Subject: [PATCH] ARM: exynos: fix build fail due to exynos_scu_enable
Build failed if we disable CONFIG_SMP as shown below:
arch/arm/mach-exynos/pm.o: In function `exynos_enter_aftr':
pm.c:(.text.exynos_enter_aftr+0xec): undefined reference to
`exynos_scu_enable'
arch/arm/mach-exynos/suspend.o: In function `exynos_pm_resume':
suspend.c:(.text.exynos_pm_resume+0x78): undefined reference to
`exynos_scu_enable'
Since scu_enable is defined only in case CONFIG_SMP and CONFIG_HAVE_ARM_SCU
lets move exynos_scu_enable also under these two macros.
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/common.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index fb12d11..03fdb79 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -156,7 +156,12 @@ extern void exynos_cpu_restore_register(void);
extern void exynos_pm_central_suspend(void);
extern int exynos_pm_central_resume(void);
extern void exynos_enter_aftr(void);
+#if defined(CONFIG_SMP) && defined(CONFIG_HAVE_ARM_SCU)
extern int exynos_scu_enable(void);
+#else
+static inline void exynos_scu_enable(void) {}
+#endif
+
------------------------------------------------------
Of-course your idea to move it in core SCU file is also good that we
lots of duplication in different architecture can be avoided.
In that case I can think of following patch:
[PATCH] ARM: scu: use SCU device node to enable SCU
Many platforms are duplicating code for enabling SCU, lets add
common code to enable SCU using SCU device node so the duplication in
each platform can be avoided.
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/include/asm/smp_scu.h | 2 ++
arch/arm/kernel/smp_scu.c | 17 +++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
index bfe163c..e5e2492 100644
--- a/arch/arm/include/asm/smp_scu.h
+++ b/arch/arm/include/asm/smp_scu.h
@@ -38,8 +38,10 @@ static inline int scu_power_mode(void __iomem
*scu_base, unsigned int mode)
#endif
#if defined(CONFIG_SMP) && defined(CONFIG_HAVE_ARM_SCU)
+int of_scu_enable(void);
void scu_enable(void __iomem *scu_base);
#else
+static inline int of_scu_enable(void) {return 0;}
static inline void scu_enable(void __iomem *scu_base) {}
#endif
diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
index 72f9241..7c16d16 100644
--- a/arch/arm/kernel/smp_scu.c
+++ b/arch/arm/kernel/smp_scu.c
@@ -34,6 +34,23 @@ unsigned int __init scu_get_core_count(void __iomem
*scu_base)
return (ncores & 0x03) + 1;
}
+int of_scu_enable(void)
+{
+ struct device_node *np;
+ void __iomem *scu_base;
+
+ np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+ scu_base = of_iomap(np, 0);
+ of_node_put(np);
+ if (!scu_base) {
+ pr_err("%s failed to map scu_base\n", __func__);
+ return -ENOMEM;
+ }
+ scu_enable(scu_base);
+ iounmap(scu_base);
+ return 0;
+}
+
/*
* Enable the SCU
*/
--
Followed by cleanup in various architecture where this piece of code is
duplicated and all of them can call directly of_scu_enable()
Please let me know which one you will prefer for fixing build issue.
@Krzysztof, please let me know if I need to resubmit SCU series again
with fix or you will accept build fix patch on top of already taken patch.
Thanks,
Pankaj Dubey
^ permalink raw reply related
* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: zhichang.yuan @ 2016-11-10 12:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <17821285.aIcTyCGn5n@wuerfel>
Hi, Arnd,
On 2016/11/10 17:12, Arnd Bergmann wrote:
> On Thursday, November 10, 2016 2:40:26 PM CET zhichang.yuan wrote:
>> On 2016/11/10 5:34, Arnd Bergmann wrote:
>>> On Wednesday, November 9, 2016 12:10:43 PM CET Gabriele Paoloni wrote:
>>>>> On Tuesday, November 8, 2016 11:47:09 AM CET zhichang.yuan wrote:
>>>>>> + /*
>>>>>> + * The first PCIBIOS_MIN_IO is reserved specifically for
>>>>> indirectIO.
>>>>>> + * It will separate indirectIO range from pci host bridge to
>>>>>> + * avoid the possible PIO conflict.
>>>>>> + * Set the indirectIO range directly here.
>>>>>> + */
>>>>>> + lpcdev->io_ops.start = 0;
>>>>>> + lpcdev->io_ops.end = PCIBIOS_MIN_IO - 1;
>>>>>> + lpcdev->io_ops.devpara = lpcdev;
>>>>>> + lpcdev->io_ops.pfin = hisilpc_comm_in;
>>>>>> + lpcdev->io_ops.pfout = hisilpc_comm_out;
>>>>>> + lpcdev->io_ops.pfins = hisilpc_comm_ins;
>>>>>> + lpcdev->io_ops.pfouts = hisilpc_comm_outs;
>>>>>
>>>>> I have to look at patch 2 in more detail again, after missing a few
>>>>> review
>>>>> rounds. I'm still a bit skeptical about hardcoding a logical I/O port
>>>>> range here, and would hope that we can just go through the same
>>>>> assignment of logical port ranges that we have for PCI buses,
>>>>> decoupling
>>>>> the bus addresses from the linux-internal ones.
>>>>
>>>> The point here is that we want to avoid any conflict/overlap between
>>>> the LPC I/O space and the PCI I/O space. With the assignment above
>>>> we make sure that LPC never interfere with PCI I/O space.
>>>
>>> But we already abstract the PCI I/O space using dynamic registration.
>>> There is no need to hardcode the logical address for ISA, though
>>> I think we can hardcode the bus address to start at zero here.
>>
>> Do you means that we can pick up the maximal I/O address from all children's
>> device resources??
>
> The driver should not look at the resources of its children, just
> register a range of addresses dynamically, as I suggested in an
> earlier review.
>
Sorry! I can't catch your idea yet:(
When to register the I/O range? Is it done just after the successfully
of_translate_address() during the children scanning?
If yes, when a child is scanning, there is no range data in arm64_extio_ops. The
addr_is_indirect_io() calling in of_get_isa_indirect_io() don't need. All we can
check is just whether the address to be translated is IO and is under a parent
device which has no 'ranges' property.
>
> Your current version has
>
> if (arm64_extio_ops->pfout) \
> arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> addr, value, sizeof(type)); \
>
> Instead, just subtract the start of the range from the logical
> port number to transform it back into a bus-local port number:
>
> if (arm64_extio_ops->pfout) \
> arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> addr - arm64_extio_ops->start, value, sizeof(type)); \
>
I think there is some information needed sync.
In the old patch-set, we don't bypass the pci_address_to_pio() after
successfully of_translate_address(). In this way, we don't need to reserve any
PIO space for our LPC since the logical port are from the same mapping
algorithm. Based on this way, the port number in the device resource is logical
one, then we need to subtract the start of the resource to get back the
bus-local port.
>From V3, we don't apply the mapping based on pci_address_to_pio(), the
of_translate_address() return the bus-local port directly and store into
relevant device resource. So, in the current arm64_extio_ops->pfout(), the
reverse translation don't need anymore. The input "addr" is bus-local port now.
Thanks,
Zhichang
> We know that the ISA/LPC bus can only have up to 65536 ports,
> so you can register all of those, or possibly limit it further to
> 1024 or 4096 ports, whichever matches the bus implementation.
>
> Arnd
>
> .
>
^ permalink raw reply
* [PATCH v2 1/2] iommu/dma: Implement dma_{map,unmap}_resource()
From: Joerg Roedel @ 2016-11-10 12:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4e84146ae49a4edd5f1226d76617be9aa92a58ee.1477503578.git.robin.murphy@arm.com>
On Wed, Oct 26, 2016 at 06:43:56PM +0100, Robin Murphy wrote:
> With the new dma_{map,unmap}_resource() functions added to the DMA API
> for the benefit of cases like slave DMA, add suitable implementations to
> the arsenal of our generic layer. Since cache maintenance should not be
> a concern, these can both be standalone callback implementations without
> the need for arch code wrappers.
>
> CC: Joerg Roedel <joro@8bytes.org>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>
> v2: Use iommu_dma_unmap_page for symmetry, instead of being sneaky.
>
> drivers/iommu/dma-iommu.c | 13 +++++++++++++
> include/linux/dma-iommu.h | 4 ++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index c5ab8667e6f2..a2fd90a6a782 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -624,6 +624,19 @@ void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
> __iommu_dma_unmap(iommu_get_domain_for_dev(dev), sg_dma_address(sg));
> }
>
> +dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys,
> + size_t size, enum dma_data_direction dir, unsigned long attrs)
> +{
> + return iommu_dma_map_page(dev, phys_to_page(phys), offset_in_page(phys),
> + size, dma_direction_to_prot(dir, false) | IOMMU_MMIO);
> +}
Isn't the whole point of map_resource that we can map regions which have
no 'struct page' associated? The use phys_to_page() will certainly not
do the right thing here.
Joerg
^ permalink raw reply
* [PATCH v3] spi: spi-fsl-dspi: Add DMA support for Vybrid
From: Sanchayan Maity @ 2016-11-10 12:19 UTC (permalink / raw)
To: linux-arm-kernel
Add DMA support for Vybrid.
Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
Changes since v2:
1. Rebase on top of Shawn's latest for-next branch
2. Make DMA mode the default for Vybrid. We no longer use the EOQ mode.
Since devtype_data has been constantified it's no longer makes sense to
change the trans_mode at run time.
Tested on Toradex Colibri Vybrid VF61 module using spidev and MCP CAN.
v1 Patch:
https://patchwork.kernel.org/patch/9360583/
v2 Patch:
https://patchwork.kernel.org/patch/9361601/
Regards,
Sanchayan.
---
drivers/spi/spi-fsl-dspi.c | 301 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 300 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 35c0dd9..bc64700 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -15,6 +15,8 @@
#include <linux/clk.h>
#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
#include <linux/err.h>
#include <linux/errno.h>
#include <linux/interrupt.h>
@@ -40,6 +42,7 @@
#define TRAN_STATE_WORD_ODD_NUM 0x04
#define DSPI_FIFO_SIZE 4
+#define DSPI_DMA_BUFSIZE (DSPI_FIFO_SIZE * 1024)
#define SPI_MCR 0x00
#define SPI_MCR_MASTER (1 << 31)
@@ -71,6 +74,11 @@
#define SPI_SR_EOQF 0x10000000
#define SPI_SR_TCFQF 0x80000000
+#define SPI_RSER_TFFFE BIT(25)
+#define SPI_RSER_TFFFD BIT(24)
+#define SPI_RSER_RFDFE BIT(17)
+#define SPI_RSER_RFDFD BIT(16)
+
#define SPI_RSER 0x30
#define SPI_RSER_EOQFE 0x10000000
#define SPI_RSER_TCFQE 0x80000000
@@ -108,6 +116,8 @@
#define SPI_TCR_TCNT_MAX 0x10000
+#define DMA_COMPLETION_TIMEOUT msecs_to_jiffies(3000)
+
struct chip_data {
u32 mcr_val;
u32 ctar_val;
@@ -117,6 +127,7 @@ struct chip_data {
enum dspi_trans_mode {
DSPI_EOQ_MODE = 0,
DSPI_TCFQ_MODE,
+ DSPI_DMA_MODE,
};
struct fsl_dspi_devtype_data {
@@ -125,7 +136,7 @@ struct fsl_dspi_devtype_data {
};
static const struct fsl_dspi_devtype_data vf610_data = {
- .trans_mode = DSPI_EOQ_MODE,
+ .trans_mode = DSPI_DMA_MODE,
.max_clock_factor = 2,
};
@@ -139,6 +150,22 @@ static const struct fsl_dspi_devtype_data ls2085a_data = {
.max_clock_factor = 8,
};
+struct fsl_dspi_dma {
+ u32 curr_xfer_len;
+
+ u32 *tx_dma_buf;
+ struct dma_chan *chan_tx;
+ dma_addr_t tx_dma_phys;
+ struct completion cmd_tx_complete;
+ struct dma_async_tx_descriptor *tx_desc;
+
+ u32 *rx_dma_buf;
+ struct dma_chan *chan_rx;
+ dma_addr_t rx_dma_phys;
+ struct completion cmd_rx_complete;
+ struct dma_async_tx_descriptor *rx_desc;
+};
+
struct fsl_dspi {
struct spi_master *master;
struct platform_device *pdev;
@@ -165,6 +192,7 @@ struct fsl_dspi {
u32 waitflags;
u32 spi_tcnt;
+ struct fsl_dspi_dma *dma;
};
static inline int is_double_byte_mode(struct fsl_dspi *dspi)
@@ -176,6 +204,263 @@ static inline int is_double_byte_mode(struct fsl_dspi *dspi)
return ((val & SPI_FRAME_BITS_MASK) == SPI_FRAME_BITS(8)) ? 0 : 1;
}
+static void dspi_tx_dma_callback(void *arg)
+{
+ struct fsl_dspi *dspi = arg;
+ struct fsl_dspi_dma *dma = dspi->dma;
+
+ complete(&dma->cmd_tx_complete);
+}
+
+static void dspi_rx_dma_callback(void *arg)
+{
+ struct fsl_dspi *dspi = arg;
+ struct fsl_dspi_dma *dma = dspi->dma;
+ int rx_word;
+ int i, len;
+ u16 d;
+
+ rx_word = is_double_byte_mode(dspi);
+
+ len = rx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
+
+ if (!(dspi->dataflags & TRAN_STATE_RX_VOID)) {
+ for (i = 0; i < len; i++) {
+ d = dspi->dma->rx_dma_buf[i];
+ rx_word ? (*(u16 *)dspi->rx = d) :
+ (*(u8 *)dspi->rx = d);
+ dspi->rx += rx_word + 1;
+ }
+ }
+
+ complete(&dma->cmd_rx_complete);
+}
+
+static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
+{
+ struct fsl_dspi_dma *dma = dspi->dma;
+ struct device *dev = &dspi->pdev->dev;
+ int time_left;
+ int tx_word;
+ int i, len;
+ u16 val;
+
+ tx_word = is_double_byte_mode(dspi);
+
+ len = tx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
+
+ for (i = 0; i < len - 1; i++) {
+ val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
+ dspi->dma->tx_dma_buf[i] =
+ SPI_PUSHR_TXDATA(val) | SPI_PUSHR_PCS(dspi->cs) |
+ SPI_PUSHR_CTAS(0) | SPI_PUSHR_CONT;
+ dspi->tx += tx_word + 1;
+ }
+
+ val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
+ dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
+ SPI_PUSHR_PCS(dspi->cs) |
+ SPI_PUSHR_CTAS(0);
+ dspi->tx += tx_word + 1;
+
+ dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
+ dma->tx_dma_phys,
+ DSPI_DMA_BUFSIZE, DMA_MEM_TO_DEV,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!dma->tx_desc) {
+ dev_err(dev, "Not able to get desc for DMA xfer\n");
+ return -EIO;
+ }
+
+ dma->tx_desc->callback = dspi_tx_dma_callback;
+ dma->tx_desc->callback_param = dspi;
+ if (dma_submit_error(dmaengine_submit(dma->tx_desc))) {
+ dev_err(dev, "DMA submit failed\n");
+ return -EINVAL;
+ }
+
+ dma->rx_desc = dmaengine_prep_slave_single(dma->chan_rx,
+ dma->rx_dma_phys,
+ DSPI_DMA_BUFSIZE, DMA_DEV_TO_MEM,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!dma->rx_desc) {
+ dev_err(dev, "Not able to get desc for DMA xfer\n");
+ return -EIO;
+ }
+
+ dma->rx_desc->callback = dspi_rx_dma_callback;
+ dma->rx_desc->callback_param = dspi;
+ if (dma_submit_error(dmaengine_submit(dma->rx_desc))) {
+ dev_err(dev, "DMA submit failed\n");
+ return -EINVAL;
+ }
+
+ reinit_completion(&dspi->dma->cmd_rx_complete);
+ reinit_completion(&dspi->dma->cmd_tx_complete);
+
+ dma_async_issue_pending(dma->chan_rx);
+ dma_async_issue_pending(dma->chan_tx);
+
+ time_left = wait_for_completion_timeout(&dspi->dma->cmd_tx_complete,
+ DMA_COMPLETION_TIMEOUT);
+ if (time_left == 0) {
+ dev_err(dev, "DMA tx timeout\n");
+ dmaengine_terminate_all(dma->chan_tx);
+ dmaengine_terminate_all(dma->chan_rx);
+ return -ETIMEDOUT;
+ }
+
+ time_left = wait_for_completion_timeout(&dspi->dma->cmd_rx_complete,
+ DMA_COMPLETION_TIMEOUT);
+ if (time_left == 0) {
+ dev_err(dev, "DMA rx timeout\n");
+ dmaengine_terminate_all(dma->chan_tx);
+ dmaengine_terminate_all(dma->chan_rx);
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static int dspi_dma_xfer(struct fsl_dspi *dspi)
+{
+ struct fsl_dspi_dma *dma = dspi->dma;
+ struct device *dev = &dspi->pdev->dev;
+ int curr_remaining_bytes;
+ int bytes_per_buffer;
+ int tx_word;
+ int ret = 0;
+
+ tx_word = is_double_byte_mode(dspi);
+ curr_remaining_bytes = dspi->len;
+ while (curr_remaining_bytes) {
+ /* Check if current transfer fits the DMA buffer */
+ dma->curr_xfer_len = curr_remaining_bytes;
+ bytes_per_buffer = DSPI_DMA_BUFSIZE /
+ (DSPI_FIFO_SIZE / (tx_word ? 2 : 1));
+ if (curr_remaining_bytes > bytes_per_buffer)
+ dma->curr_xfer_len = bytes_per_buffer;
+
+ ret = dspi_next_xfer_dma_submit(dspi);
+ if (ret) {
+ dev_err(dev, "DMA transfer failed\n");
+ goto exit;
+
+ } else {
+ curr_remaining_bytes -= dma->curr_xfer_len;
+ if (curr_remaining_bytes < 0)
+ curr_remaining_bytes = 0;
+ dspi->len = curr_remaining_bytes;
+ }
+ }
+
+exit:
+ return ret;
+}
+
+static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
+{
+ struct fsl_dspi_dma *dma;
+ struct dma_slave_config cfg;
+ struct device *dev = &dspi->pdev->dev;
+ int ret;
+
+ dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
+ if (!dma)
+ return -ENOMEM;
+
+ dma->chan_rx = dma_request_slave_channel(dev, "rx");
+ if (!dma->chan_rx) {
+ dev_err(dev, "rx dma channel not available\n");
+ ret = -ENODEV;
+ return ret;
+ }
+
+ dma->chan_tx = dma_request_slave_channel(dev, "tx");
+ if (!dma->chan_tx) {
+ dev_err(dev, "tx dma channel not available\n");
+ ret = -ENODEV;
+ goto err_tx_channel;
+ }
+
+ dma->tx_dma_buf = dma_alloc_coherent(dev, DSPI_DMA_BUFSIZE,
+ &dma->tx_dma_phys, GFP_KERNEL);
+ if (!dma->tx_dma_buf) {
+ ret = -ENOMEM;
+ goto err_tx_dma_buf;
+ }
+
+ dma->rx_dma_buf = dma_alloc_coherent(dev, DSPI_DMA_BUFSIZE,
+ &dma->rx_dma_phys, GFP_KERNEL);
+ if (!dma->rx_dma_buf) {
+ ret = -ENOMEM;
+ goto err_rx_dma_buf;
+ }
+
+ cfg.src_addr = phy_addr + SPI_POPR;
+ cfg.dst_addr = phy_addr + SPI_PUSHR;
+ cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ cfg.src_maxburst = 1;
+ cfg.dst_maxburst = 1;
+
+ cfg.direction = DMA_DEV_TO_MEM;
+ ret = dmaengine_slave_config(dma->chan_rx, &cfg);
+ if (ret) {
+ dev_err(dev, "can't configure rx dma channel\n");
+ ret = -EINVAL;
+ goto err_slave_config;
+ }
+
+ cfg.direction = DMA_MEM_TO_DEV;
+ ret = dmaengine_slave_config(dma->chan_tx, &cfg);
+ if (ret) {
+ dev_err(dev, "can't configure tx dma channel\n");
+ ret = -EINVAL;
+ goto err_slave_config;
+ }
+
+ dspi->dma = dma;
+ init_completion(&dma->cmd_tx_complete);
+ init_completion(&dma->cmd_rx_complete);
+
+ return 0;
+
+err_slave_config:
+ devm_kfree(dev, dma->rx_dma_buf);
+err_rx_dma_buf:
+ devm_kfree(dev, dma->tx_dma_buf);
+err_tx_dma_buf:
+ dma_release_channel(dma->chan_tx);
+err_tx_channel:
+ dma_release_channel(dma->chan_rx);
+
+ devm_kfree(dev, dma);
+ dspi->dma = NULL;
+
+ return ret;
+}
+
+static void dspi_release_dma(struct fsl_dspi *dspi)
+{
+ struct fsl_dspi_dma *dma = dspi->dma;
+ struct device *dev = &dspi->pdev->dev;
+
+ if (dma) {
+ if (dma->chan_tx) {
+ dma_unmap_single(dev, dma->tx_dma_phys,
+ DSPI_DMA_BUFSIZE, DMA_TO_DEVICE);
+ dma_release_channel(dma->chan_tx);
+ }
+
+ if (dma->chan_rx) {
+ dma_unmap_single(dev, dma->rx_dma_phys,
+ DSPI_DMA_BUFSIZE, DMA_FROM_DEVICE);
+ dma_release_channel(dma->chan_rx);
+ }
+ }
+}
+
static void hz_to_spi_baud(char *pbr, char *br, int speed_hz,
unsigned long clkrate)
{
@@ -424,6 +709,12 @@ static int dspi_transfer_one_message(struct spi_master *master,
regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_TCFQE);
dspi_tcfq_write(dspi);
break;
+ case DSPI_DMA_MODE:
+ regmap_write(dspi->regmap, SPI_RSER,
+ SPI_RSER_TFFFE | SPI_RSER_TFFFD |
+ SPI_RSER_RFDFE | SPI_RSER_RFDFD);
+ status = dspi_dma_xfer(dspi);
+ goto out;
default:
dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
trans_mode);
@@ -733,6 +1024,13 @@ static int dspi_probe(struct platform_device *pdev)
if (ret)
goto out_master_put;
+ if (dspi->devtype_data->trans_mode == DSPI_DMA_MODE) {
+ if (dspi_request_dma(dspi, res->start)) {
+ dev_err(&pdev->dev, "can't get dma channels\n");
+ goto out_clk_put;
+ }
+ }
+
master->max_speed_hz =
clk_get_rate(dspi->clk) / dspi->devtype_data->max_clock_factor;
@@ -761,6 +1059,7 @@ static int dspi_remove(struct platform_device *pdev)
struct fsl_dspi *dspi = spi_master_get_devdata(master);
/* Disconnect from the SPI framework */
+ dspi_release_dma(dspi);
clk_disable_unprepare(dspi->clk);
spi_unregister_master(dspi->master);
--
2.10.2
^ permalink raw reply related
* [RFC v2 4/8] iommu: Add a list of iommu_reserved_region in iommu_domain
From: Auger Eric @ 2016-11-10 12:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6851a74a-775a-bd20-cdea-4cf06f5f0289@arm.com>
Hi Robin,
On 10/11/2016 12:54, Robin Murphy wrote:
> Hi Eric,
>
> On 10/11/16 11:22, Auger Eric wrote:
>> Hi Robin,
>>
>> On 04/11/2016 15:00, Robin Murphy wrote:
>>> Hi Eric,
>>>
>>> Thanks for posting this new series - the bottom-up approach is a lot
>>> easier to reason about :)
>>>
>>> On 04/11/16 11:24, Eric Auger wrote:
>>>> Introduce a new iommu_reserved_region struct. This embodies
>>>> an IOVA reserved region that cannot be used along with the IOMMU
>>>> API. The list is protected by a dedicated mutex.
>>>
>>> In the light of these patches, I think I'm settling into agreement that
>>> the iommu_domain is the sweet spot for accessing this information - the
>>> underlying magic address ranges might be properties of various bits of
>>> hardware many of which aren't the IOMMU itself, but they only start to
>>> matter at the point you start wanting to use an IOMMU domain at the
>>> higher level. Therefore, having a callback in the domain ops to pull
>>> everything together fits rather neatly.
>> Using get_dm_regions could have make sense but this approach now is
>> ruled out by sysfs API approach. If attribute file is bound to be used
>> before iommu domains are created, we cannot rely on any iommu_domain
>> based callback. Back to square 1?
>
> I think it's still OK. The thing about these reserved regions is that as
> a property of the underlying hardware they must be common to any domain
> for a given group, therefore without loss of generality we can simply
> query group->domain->ops->get_dm_regions(), and can expect the reserved
> ones will be the same regardless of what domain that points to
> (identity-mapped IVMD/RMRR/etc.
Are they really? P2P reserved regions depend on iommu_domain right?
Now I did not consider default_domain usability, I acknowledge. I will
send a POC anyway.
regions may not be, but we'd be
> filtering those out anyway). The default DMA domains need this
> information too, and since those are allocated at group creation,
> group->domain should always be non-NULL and interrogable.
>
> Plus, the groups are already there in sysfs, and, being representative
> of device topology, would seem to be an ideal place to expose the
> addressing limitations relevant to the devices within them. This really
> feels like it's all falling into place (on the kernel end, at least, I'm
> sticking to the sidelines on the userspace discussion ;)).
Thanks
Eric
>
> Robin.
>
>>
>> Thanks
>>
>> Eric
>>>
>>>>
>>>> An iommu domain now owns a list of those.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>
>>>> ---
>>>> ---
>>>> drivers/iommu/iommu.c | 2 ++
>>>> include/linux/iommu.h | 17 +++++++++++++++++
>>>> 2 files changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>>> index 9a2f196..0af07492 100644
>>>> --- a/drivers/iommu/iommu.c
>>>> +++ b/drivers/iommu/iommu.c
>>>> @@ -1061,6 +1061,8 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
>>>>
>>>> domain->ops = bus->iommu_ops;
>>>> domain->type = type;
>>>> + INIT_LIST_HEAD(&domain->reserved_regions);
>>>> + mutex_init(&domain->resv_mutex);
>>>> /* Assume all sizes by default; the driver may override this later */
>>>> domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap;
>>>>
>>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>>> index 436dc21..0f2eb64 100644
>>>> --- a/include/linux/iommu.h
>>>> +++ b/include/linux/iommu.h
>>>> @@ -84,6 +84,8 @@ struct iommu_domain {
>>>> void *handler_token;
>>>> struct iommu_domain_geometry geometry;
>>>> void *iova_cookie;
>>>> + struct list_head reserved_regions;
>>>> + struct mutex resv_mutex; /* protects the reserved region list */
>>>> };
>>>>
>>>> enum iommu_cap {
>>>> @@ -131,6 +133,21 @@ struct iommu_dm_region {
>>>> int prot;
>>>> };
>>>>
>>>> +/**
>>>> + * struct iommu_reserved_region - descriptor for a reserved iova region
>>>> + * @list: Linked list pointers
>>>> + * @start: IOVA base address of the region
>>>> + * @length: Length of the region in bytes
>>>> + */
>>>> +struct iommu_reserved_region {
>>>> + struct list_head list;
>>>> + dma_addr_t start;
>>>> + size_t length;
>>>> +};
>>>
>>> Looking at this in context with the dm_region above, though, I come to
>>> the surprising realisation that these *are* dm_regions, even at the
>>> fundamental level - on the one hand you've got physical addresses which
>>> can't be remapped (because something is already using them), while on
>>> the other you've got physical addresses which can't be remapped (because
>>> the IOMMU is incapable). In fact for reserved regions *other* than our
>>> faked-up MSI region there's no harm if the IOMMU were to actually
>>> identity-map them.
>>>
>>> Let's just add this to the existing infrastructure, either with some
>>> kind of IOMMU_NOMAP flag or simply prot = 0. That way it automatically
>>> gets shared between the VFIO and DMA cases for free!
>>>
>>> Robin.
>>>
>>>> +
>>>> +#define iommu_reserved_region_for_each(resv, d) \
>>>> + list_for_each_entry(resv, &(d)->reserved_regions, list)
>>>> +
>>>> #ifdef CONFIG_IOMMU_API
>>>>
>>>> /**
>>>>
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>
>
^ permalink raw reply
* [PATCH v3 00/10] Add DT support for ohci-da8xx
From: Greg KH @ 2016-11-10 12:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKXjFTN04Wz5jsaHsyvDD7G8QqXQvCE4D-YbHtGT=jUqELiS_w@mail.gmail.com>
On Tue, Nov 08, 2016 at 05:37:41PM +0100, Axel Haslam wrote:
> Hi,
>
> On Mon, Nov 7, 2016 at 9:39 PM, Axel Haslam <ahaslam@baylibre.com> wrote:
> > The purpose of this patch series is to add DT support for the davinci
> > ohci driver.
> >
>
> To make it easier to review. I will split the arch/arm and driver
> patches into separate series.
I don't think it's easier, as now I have no idea what order, or what
tree it should go through :(
I'm guessing not mine, so all are now deleted from my patch queue...
greg k-h
^ permalink raw reply
* [PATCH v3 1/2] ARM: EXYNOS: Remove static mapping of SCU SFR
From: Arnd Bergmann @ 2016-11-10 11:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478693755-11953-2-git-send-email-pankaj.dubey@samsung.com>
On Wednesday, November 9, 2016 5:45:54 PM CET Pankaj Dubey wrote:
> Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based
> boards. Instead use mapping from device tree node of SCU.
>
> NOTE: This patch has dependency on DT file of any such CORTEX-A9 SoC
> based boards, in the absence of SCU device node in DTS file, only single
> CPU will boot. So if you are using OUT-OF-TREE DTS file of CORTEX-A9 based
> Exynos SoC make sure to add SCU device node to DTS file for SMP boot.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
With CONFIG_SMP disabled, I now get this build failure:
arch/arm/mach-exynos/pm.o: In function `exynos_enter_aftr':
pm.c:(.text.exynos_enter_aftr+0xec): undefined reference to `exynos_scu_enable'
arch/arm/mach-exynos/suspend.o: In function `exynos_pm_resume':
suspend.c:(.text.exynos_pm_resume+0x78): undefined reference to `exynos_scu_enable'
Please fix. I have applied a patch locally (see below), but don't know
if that is the best solution. As we seem to duplicate that code across
several platforms, I wonder why we don't just put it into the core scu
implementation.
Arnd
commit ad63b863bb78188fbe9608cfad629c86bd579dc0
Author: Arnd Bergmann <arnd@arndb.de>
Date: Thu Nov 10 12:51:43 2016 +0100
ARM: exynos: move exynos_scu_enable to main file
arch/arm/mach-exynos/pm.o: In function `exynos_enter_aftr':
pm.c:(.text.exynos_enter_aftr+0xec): undefined reference to `exynos_scu_enable'
arch/arm/mach-exynos/suspend.o: In function `exynos_pm_resume':
suspend.c:(.text.exynos_pm_resume+0x78): undefined reference to `exynos_scu_enable'
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index fa08ef99b4ad..784c6d47af17 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -39,6 +39,27 @@ static struct platform_device exynos_cpuidle = {
void __iomem *sysram_base_addr;
void __iomem *sysram_ns_base_addr;
+/**
+ * exynos_scu_enable : enables SCU for Cortex-A9 based system
+ * returns 0 on success else non-zero error code
+ */
+int exynos_scu_enable(void)
+{
+ struct device_node *np;
+ void __iomem *scu_base;
+
+ np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+ scu_base = of_iomap(np, 0);
+ of_node_put(np);
+ if (!scu_base) {
+ pr_err("%s failed to map scu_base\n", __func__);
+ return -ENOMEM;
+ }
+ scu_enable(scu_base);
+ iounmap(scu_base);
+ return 0;
+}
+
void __init exynos_sysram_init(void)
{
struct device_node *node;
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 94405c72d245..4ad376637a34 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -168,27 +168,6 @@ int exynos_cluster_power_state(int cluster)
S5P_CORE_LOCAL_PWR_EN);
}
-/**
- * exynos_scu_enable : enables SCU for Cortex-A9 based system
- * returns 0 on success else non-zero error code
- */
-int exynos_scu_enable(void)
-{
- struct device_node *np;
- void __iomem *scu_base;
-
- np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
- scu_base = of_iomap(np, 0);
- of_node_put(np);
- if (!scu_base) {
- pr_err("%s failed to map scu_base\n", __func__);
- return -ENOMEM;
- }
- scu_enable(scu_base);
- iounmap(scu_base);
- return 0;
-}
-
static void __iomem *cpu_boot_reg_base(void)
{
if (soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_1_1)
^ permalink raw reply related
* [RFC v2 4/8] iommu: Add a list of iommu_reserved_region in iommu_domain
From: Robin Murphy @ 2016-11-10 11:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <fd35f7e3-6d97-7e67-a5c5-bb633ce0e8fd@redhat.com>
Hi Eric,
On 10/11/16 11:22, Auger Eric wrote:
> Hi Robin,
>
> On 04/11/2016 15:00, Robin Murphy wrote:
>> Hi Eric,
>>
>> Thanks for posting this new series - the bottom-up approach is a lot
>> easier to reason about :)
>>
>> On 04/11/16 11:24, Eric Auger wrote:
>>> Introduce a new iommu_reserved_region struct. This embodies
>>> an IOVA reserved region that cannot be used along with the IOMMU
>>> API. The list is protected by a dedicated mutex.
>>
>> In the light of these patches, I think I'm settling into agreement that
>> the iommu_domain is the sweet spot for accessing this information - the
>> underlying magic address ranges might be properties of various bits of
>> hardware many of which aren't the IOMMU itself, but they only start to
>> matter at the point you start wanting to use an IOMMU domain at the
>> higher level. Therefore, having a callback in the domain ops to pull
>> everything together fits rather neatly.
> Using get_dm_regions could have make sense but this approach now is
> ruled out by sysfs API approach. If attribute file is bound to be used
> before iommu domains are created, we cannot rely on any iommu_domain
> based callback. Back to square 1?
I think it's still OK. The thing about these reserved regions is that as
a property of the underlying hardware they must be common to any domain
for a given group, therefore without loss of generality we can simply
query group->domain->ops->get_dm_regions(), and can expect the reserved
ones will be the same regardless of what domain that points to
(identity-mapped IVMD/RMRR/etc. regions may not be, but we'd be
filtering those out anyway). The default DMA domains need this
information too, and since those are allocated at group creation,
group->domain should always be non-NULL and interrogable.
Plus, the groups are already there in sysfs, and, being representative
of device topology, would seem to be an ideal place to expose the
addressing limitations relevant to the devices within them. This really
feels like it's all falling into place (on the kernel end, at least, I'm
sticking to the sidelines on the userspace discussion ;)).
Robin.
>
> Thanks
>
> Eric
>>
>>>
>>> An iommu domain now owns a list of those.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>
>>> ---
>>> ---
>>> drivers/iommu/iommu.c | 2 ++
>>> include/linux/iommu.h | 17 +++++++++++++++++
>>> 2 files changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>> index 9a2f196..0af07492 100644
>>> --- a/drivers/iommu/iommu.c
>>> +++ b/drivers/iommu/iommu.c
>>> @@ -1061,6 +1061,8 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
>>>
>>> domain->ops = bus->iommu_ops;
>>> domain->type = type;
>>> + INIT_LIST_HEAD(&domain->reserved_regions);
>>> + mutex_init(&domain->resv_mutex);
>>> /* Assume all sizes by default; the driver may override this later */
>>> domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap;
>>>
>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>> index 436dc21..0f2eb64 100644
>>> --- a/include/linux/iommu.h
>>> +++ b/include/linux/iommu.h
>>> @@ -84,6 +84,8 @@ struct iommu_domain {
>>> void *handler_token;
>>> struct iommu_domain_geometry geometry;
>>> void *iova_cookie;
>>> + struct list_head reserved_regions;
>>> + struct mutex resv_mutex; /* protects the reserved region list */
>>> };
>>>
>>> enum iommu_cap {
>>> @@ -131,6 +133,21 @@ struct iommu_dm_region {
>>> int prot;
>>> };
>>>
>>> +/**
>>> + * struct iommu_reserved_region - descriptor for a reserved iova region
>>> + * @list: Linked list pointers
>>> + * @start: IOVA base address of the region
>>> + * @length: Length of the region in bytes
>>> + */
>>> +struct iommu_reserved_region {
>>> + struct list_head list;
>>> + dma_addr_t start;
>>> + size_t length;
>>> +};
>>
>> Looking at this in context with the dm_region above, though, I come to
>> the surprising realisation that these *are* dm_regions, even at the
>> fundamental level - on the one hand you've got physical addresses which
>> can't be remapped (because something is already using them), while on
>> the other you've got physical addresses which can't be remapped (because
>> the IOMMU is incapable). In fact for reserved regions *other* than our
>> faked-up MSI region there's no harm if the IOMMU were to actually
>> identity-map them.
>>
>> Let's just add this to the existing infrastructure, either with some
>> kind of IOMMU_NOMAP flag or simply prot = 0. That way it automatically
>> gets shared between the VFIO and DMA cases for free!
>>
>> Robin.
>>
>>> +
>>> +#define iommu_reserved_region_for_each(resv, d) \
>>> + list_for_each_entry(resv, &(d)->reserved_regions, list)
>>> +
>>> #ifdef CONFIG_IOMMU_API
>>>
>>> /**
>>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
^ permalink raw reply
* [PATCH v8 0/3] ARM, arm64: renesas: Enable UHS-I SDR-104
From: Wolfram Sang @ 2016-11-10 11:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478185645-25556-1-git-send-email-horms+renesas@verge.net.au>
On Thu, Nov 03, 2016 at 04:07:22PM +0100, Simon Horman wrote:
> Hi,
>
> this series enables SDHI UHS-I SDR-104 on:
> * r8a7790/lager
> * r8a7791/koelsch
> * r8a7794/alt
>
> It is based on renesas-next-20161102-v4.9-rc1.
For the whole series:
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
For Lager:
Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161110/b1350af2/attachment.sig>
^ permalink raw reply
* [PATCH] ARM: dts: vfxxx: Enable DMA for DSPI2 and DSPI3
From: Sanchayan Maity @ 2016-11-10 11:45 UTC (permalink / raw)
To: linux-arm-kernel
Enable DMA for DSPI2 and DSPI3 on Vybrid.
---
arch/arm/boot/dts/vfxxx.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 000550f..e9d2847 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -573,6 +573,9 @@
clocks = <&clks VF610_CLK_DSPI2>;
clock-names = "dspi";
spi-num-chipselects = <2>;
+ dmas = <&edma1 0 10>,
+ <&edma1 0 11>;
+ dma-names = "rx", "tx";
status = "disabled";
};
@@ -585,6 +588,9 @@
clocks = <&clks VF610_CLK_DSPI3>;
clock-names = "dspi";
spi-num-chipselects = <2>;
+ dmas = <&edma1 0 12>,
+ <&edma1 0 13>;
+ dma-names = "rx", "tx";
status = "disabled";
};
--
2.10.2
^ permalink raw reply related
* [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Ziji Hu @ 2016-11-10 11:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161109182426.vfrpb4i2mfatdzz3@rob-hp-laptop>
Hi Rob,
On 2016/11/10 2:24, Rob Herring wrote:
> On Mon, Oct 31, 2016 at 12:09:54PM +0100, Gregory CLEMENT wrote:
>> From: Ziji Hu <huziji@marvell.com>
>>
>> Marvell Xenon SDHC can support eMMC/SD/SDIO.
>> Add Xenon-specific properties.
>> Also add properties for Xenon PHY setting.
>>
>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>> Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 161 +++++++-
>> MAINTAINERS | 1 +-
>> 2 files changed, 162 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>> new file mode 100644
>> index 000000000000..0d2d139494d3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>> @@ -0,0 +1,161 @@
>> +Marvell's Xenon SDHCI Controller device tree bindings
>> +This file documents differences between the core mmc properties
>> +described by mmc.txt and the properties used by the Xenon implementation.
>> +
>> +A single Xenon IP can support multiple slots.
>> +Each slot acts as an independent SDHC. It owns independent resources, such
>> +as register sets clock and PHY.
>> +Each slot should have an independent device tree node.
>> +
>> +Required Properties:
>> +- compatible: should be one of the following
>> + - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SOC.
>> + Must provide a second register area and marvell,pad-type.
>> + - "marvell,xenon-sdhci": For controllers on all the SOCs, other than
>> + Armada-3700.
>
> Need SoC specific compatible strings.
>
Xenon SDHC is a common IP for all Marvell SOCs.
It is difficult to use a single SOC specific compatible to represent Xenon SDHC.
There will be so many SOC compatible strings in list if each specific SOC owns a compatible.
Actually only few SOCs require special properties.
Any suggestion please?
>> +
>> +- clocks:
>> + Array of clocks required for SDHCI.
>> + Requires at least one for Xenon IP core.
>> + Some SOCs require additional clock for AXI bus.
>> +
>> +- clock-names:
>> + Array of names corresponding to clocks property.
>> + The input clock for Xenon IP core should be named as "core".
>> + The optional AXI clock should be named as "axi".
>
> When is AXI clock optional? This should be required for ?? compatible
> strings.
>
It is required on some SOCs.
I will double check if a suitable compatible string can be determined for those SOCs.
>> +
>> +- reg:
>> + * For "marvell,xenon-sdhci", one register area for Xenon IP.
>> +
>> + * For "marvell,armada-3700-sdhci", two register areas.
>> + The first one for Xenon IP register. The second one for the Armada 3700 SOC
>> + PHY PAD Voltage Control register.
>> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
>> + in below.
>> + Please also check property marvell,pad-type in below.
>> +
>> +Optional Properties:
>> +- marvell,xenon-slotno:
>
> Multiple slots should be represented as child nodes IMO. I think some
> other bindings already do this.
>
All the slots are entirely independent.
I prefer to consider it as multiple independent SDHCs placed in a single IP, instead of that a IP contains multiple child slots.
It is unlike the implementation which put multiple slots behind PCIe EP interface. sdhci-pci.c will handle each slot init one by one.
If Xenon SDHC slots are represented as child nodes, there should also be a main entry in Xenon driver to init each child node one by one.
In my very own opinion, it is inconvenient and unnecessary.
>> + Indicate the corresponding bit index of current Xenon SDHC slot in
>> + SDHC System Operation Control Register Bit[7:0].
>> + Set/clear the corresponding bit to enable/disable current Xenon SDHC
>> + slot.
>> + If this property is not provided, Xenon IP should contain only one
>> + slot.
>> +
>> +- marvell,xenon-phy-type:
>> + Xenon support mutilple types of PHYs.
>> + To select eMMC 5.1 PHY, set:
>> + marvell,xenon-phy-type = "emmc 5.1 phy"
>> + eMMC 5.1 PHY is the default choice if this property is not provided.
>> + To select eMMC 5.0 PHY, set:
>> + marvell,xenon-phy-type = "emmc 5.0 phy"
>> + To select SDH PHY, set:
>> + marvell,xenon-phy-type = "sdh phy"
>> + Please note that eMMC PHY is a general PHY for eMMC/SD/SDIO, other than for
>> + eMMC only.
>
> Does this vary per instance on a single SoC? If not, then an SoC
> specific compatible should determine this.
>
> Also, the " phy" part is redundant.
>
Yes. Some SOCs might have multiple Xenon PHY types.
This property is only the name/type of PHY. It doesn't stand for the entire SDHC property.
"emmc 5.1 PHY" doesn't mean that this Xenon SDHC only support eMMC 5.1.
Xenon SDHC with "sdh PHY" can also support eMMC.
>> +
>> +- marvell,xenon-phy-znr:
>> + Set PHY ZNR value.
>> + Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
>> + valid range = [0:0x1F].
>> + ZNR is set as 0xF by default if this property is not provided.
>> +
>> +- marvell,xenon-phy-zpr:
>> + Set PHY ZPR value.
>> + Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
>> + valid range = [0:0x1F].
>> + ZPR is set as 0xF by default if this property is not provided.
>> +
>> +- marvell,xenon-phy-nr-success-tun:
>> + Set the number of required consecutive successful sampling points used to
>> + identify a valid sampling window, in tuning process.
>> + Valid range = [1:7]. Set as 0x4 by default if this property is not provided.
>> +
>> +- marvell,xenon-phy-tun-step-divider:
>> + Set the divider for calculating TUN_STEP.
>> + Set as 64 by default if this property is not provided.
>> +
>> +- marvell,xenon-phy-slow-mode:
>> + Force PHY into slow mode.
>> + Only available when bus frequency lower than 50MHz in SDR mde.
>> + Disabled by default. Please do not enable it unless it is necessary.
>> +
>> +- marvell,xenon-mask-conflict-err:
>> + Mask Conflict Error alert on some SOC. Disabled by default.
>> +
>> +- marvell,xenon-tun-count:
>> + Xenon SDHC SOC usually doesn't provide re-tuning counter in
>> + Capabilities Register 3 Bit[11:8].
>> + This property provides the re-tuning counter.
>> + If this property is not set, default re-tuning counter will
>> + be set as 0x9 in driver.
>> +
>> +- marvell,pad-type:
>> + Type of Armada 3700 SOC PHY PAD Voltiage Controller register.
>> + Only valid when "marvell,armada-3700-sdhci" is selected.
>> + Two types: "sd" and "fixed-1-8v".
>> + If "sd" is slected, SOC PHY PAD is set as 3.3V at the beginning and is
>> + switched to 1.8V when SD in UHS-I.
>> + If "fixed-1-8v" is slected, SOC PHY PAD is fixed 1.8V, such as for eMMC.
>> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
>> + in below.
>> +
>> +Example:
>> +- For eMMC slot:
>> +
>> + sdhci at aa0000 {
>> + compatible = "marvell,xenon-sdhci";
>> + reg = <0xaa0000 0x1000>;
>> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
>> + clocks = <&emmc_clk>, <&axi_clock>;
>> + clock-names = "core", "axi";
>> + bus-width = <8>;
>> + marvell,xenon-emmc;
>
> Not documented. If we need to specify the type of slot/card, then we
> need to come up with a standard property. This was either already done
> or attempted IIRC.
Sorry to lost this property in above.
I will add it in above.
Thank you.
Best regards,
Hu Ziji
>
>> + marvell,xenon-slotno = <0>;
>> + marvell,xenon-phy-type = "emmc 5.1 phy";
>> + marvell,xenon-tun-count = <11>;
>> + };
>> +
>> +- For SD/SDIO slot:
>> +
>> + sdhci at ab0000 {
>> + compatible = "marvell,xenon-sdhci";
>> + reg = <0xab0000 0x1000>;
>> + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
>> + vqmmc-supply = <&sd_regulator>;
>> + clocks = <&sdclk>;
>> + clock-names = "core";
>> + bus-width = <4>;
>> + marvell,xenon-tun-count = <9>;
>> + };
>> +
>> +- For eMMC slot with compatible "marvell,armada-3700-sdhci":
>> +
>> + sdhci at aa0000 {
>> + compatible = "marvell,armada-3700-sdhci";
>> + reg = <0xaa0000 0x1000>,
>> + <phy_addr 0x4>;
>> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
>> + clocks = <&emmcclk>;
>> + clock-names = "core";
>> + bus-width = <8>;
>> + marvell,xenon-emmc;
>> +
>> + marvell,pad-type = "fixed-1-8v";
>> + };
>> +
>> +- For SD/SDIO slot with compatible "marvell,armada-3700-sdhci":
>> +
>> + sdhci at ab0000 {
>> + compatible = "marvell,armada-3700-sdhci";
>> + reg = <0xab0000 0x1000>,
>> + <phy_addr 0x4>;
>> + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
>> + vqmmc-supply = <&sd_regulator>;
>> + clocks = <&sdclk>;
>> + clock-names = "core";
>> + bus-width = <4>;
>> +
>> + marvell,pad-type = "sd";
>> + };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1a5c4c30ea24..850a0afb0c8d 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7608,6 +7608,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
>> M: Ziji Hu <huziji@marvell.com>
>> L: linux-mmc at vger.kernel.org
>> S: Supported
>> +F: Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>>
>> MATROX FRAMEBUFFER DRIVER
>> L: linux-fbdev at vger.kernel.org
>> --
>> git-series 0.8.10
^ permalink raw reply
* [alsa-devel] [PATCH 2/9] ALSA: ac97: add an ac97 bus
From: Lars-Peter Clausen @ 2016-11-10 11:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87mvh8uywm.fsf@belgarion.home>
On 11/09/2016 10:27 PM, Robert Jarzmik wrote:
[...]
>>>>> +int snd_ac97_codec_driver_register(struct ac97_codec_driver *drv)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + drv->driver.bus = &ac97_bus_type;
>>>>> +
>>>>> + ret = driver_register(&drv->driver);
>>>>> + if (!ret)
>>>>> + ac97_rescan_all_controllers();
>>>>
>>>> Rescanning the bus when a new codec driver is registered should not be
>>>> neccessary. The bus is scanned once when the controller is registered, this
>>>> creates the device. The device driver core will take care of binding the
>>>> device to the driver, if the driver is registered after thed evice.
>>> That's because you suppose the initial scanning found all the ac97 codecs.
>>> But that's an incomplete vision as a codec might be powered off at that time and
>>> not seen by the first scanning, while the new scanning will discover it.
>>
>> But why would the device become suddenly visible when the driver is
>> registered. This seems to be an as arbitrary point in time as any other.
> Because in the meantime, a regulator or something else provided power to the
> AC97 IP, or a clock, and it can answer to requests after that.
>
> And another point if that the clock of controller might be provided by the AC97
> codec, and the scan will only succeed once the codec is actually providing this
> clock, which it might do only once the module_init() is done.
>
>> Also consider that when you build a driver as a module, the module will
>> typically only be auto-loaded after the device has been detected, based on
>> the device ID. On the other hand, if the driver is built-in driver
>> registration will happen either before or shortly after the controller
>> registration.
>> If there is the expectation that the AC97 CODEC might randomly appear on the
>> bus, the core should periodically scan the bus.
> Power wise a periodical scan doesn't look good, at all.
>
> More globally, I don't see if there is an actual issue we're trying to address
> here, ie. that the rescan is a bug, or if it's more an "Occam's razor"
> discussion ?
It's a framework design discussion. In my opinion the driver being
registered and the device becoming visible on the physical bus are two
completely unrelated operations. Especially in the Linux device driver model.
You have a generic driver that calls ac97_codec_driver_register() in its
module_init() section. This generic driver does (at module_init() time) not
know anything about a device instance specific clocks, regulators or other
resources. Resources are handled on a per device instance basis, and you
won't have a device instance until the device has been detected, which
happens in ac97_bus_scan(). So you have a cyclic dependency loop here.
Maybe we can just leave the rescanning out for now and think about how to
best handle it when the need arises.
>
>>>> In my opinion this should return a handle to a ac97 controller which can
>>>> then be passed to snd_ac97_controller_unregister(). This is in my opinion
>>>> the better approach rather than looking up the controller by parent device.
>>> This is another "legacy drivers" issue.
>>>
>>> The legacy driver (the one probed by platform_data or devicetree) doesn't
>>> necessarily have a private structure to store this ac97_controller pointer.
>>
>> I might be missing something, but I'm not convinced by this. Can you point
>> me to such a legacy driver where you think this would not work?
> The first one that popped out:
> - hac_soc_platform_probe()
I think that driver should be able to use platform_set_drvdata() to store
the pointer.
^ permalink raw reply
* [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus
From: Arnd Bergmann @ 2016-11-10 11:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdX+T=Y--F6KXZwFmZwWFdfP5mL5Tar-wKwRM2p6Hh2-sg@mail.gmail.com>
On Thursday, November 10, 2016 11:19:20 AM CET Geert Uytterhoeven wrote:
> Hi Arnd,
>
> Thanks for your comments!
>
> On Wed, Nov 9, 2016 at 5:55 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday, October 31, 2016 12:30:55 PM CET Geert Uytterhoeven wrote:
> >> v2:
> >> - Drop SoC families and family names; use fixed "Renesas" instead,
> >
> > I think I'd rather have seen the family names left in there, but it's
> > not important, so up to you.
>
> They're not useful for matching, as family names may change anytime, and don't
> always say much about the hardware capabilities.
> E.g. SH-Mobile -> R-Mobile -> R-Car | RZ/A | RZ/G
> Some SH-Mobile (even some R-Car) parts are SuperH only, others have ARM and
> SuperH.
>
> At least the SoC part numbers are stable (hmm, sh73a0 == r8a73a0).
I think the marketing names are much more useful for humans looking
at the sysfs files than the kernel doing matching on, but both use
cases are important.
> >> - Use "renesas,prr" and "renesas,cccr" device nodes in DT if
> >> available, else fall back to hardcoded addresses for compatibility
> >> with existing DTBs,
> >
> > I only see patches 2, 3, 5, and 7 in my inbox, so I'm lacking the DT
> > binding for these, among other things.
>
> I understand you've received them in the mean time?
Yes, I found them in my inbox later, not sure why I didn't see them
at first.
> > It does seem wrong to have a device node for a specific register though.
> > Shouldn't the node be for the block of registers that these are inside
> > of?
>
> On R-Mobile APE6, R-Car Gen2 and Gen3, PRR is a lone register.
> On R-Car Gen1, it's not even documented (and doesn't exist on all parts).
It just seems odd to have it at address 0xff000044 when all the other
devices are at page-aligned addresses. Do you mean that accessing
0xff000040 or 0xff000048 will result in a bus-level exception for a
missing register and just 0xff000044 is actually valid for access,
or is it just the only thing that is documented?
> On SH-Mobile/R-Mobile, CCCR may be part of the HPB/APB register block, which
> we further don't touch at all.
> On R-Car Gen2, it's not documented, but does exist.
This is where the family names would come in handy ;-) I now have
no idea which chip(s) you are referring to.
If you know the name of the register block, just put it into DT with
that name. The driver can trivially add the right offset.
> >> - Don't register the SoC bus if the chip ID register is missing,
> >
> > Why? My objection was to hardcoding the register, not to registering
> > the device? I think I'd rather see the device registered with an
> > empty revision string.
>
> If there's no chip ID register, there's no reason to use soc_device_match(),
> as we can always look at a compatible value. All SoCs listed in this driver
> have a chip ID register.
But you may still have user space tools looking into sysfs, e.g. to
figure out how to install a kernel that the boot loader can find,
or which hardware specific distro packages to install.
> if you want me to register the soc_bus for those SoCs regardless, I want to
> re-add r7s72100 (RZ/A) and r8a7778 (R-Car M1A), who don't have chip ID
> registers ;-)
Right. Just don't encode too much knowledge about the SoCs into the
driver, so we are prepared for adding new ones: We should still look
for the registers in DT on all chips.
> >> +#define CCCR 0xe600101c /* Common Chip Code Register */
> >> +#define PRR 0xff000044 /* Product Register */
> >> +#define PRR3 0xfff00044 /* Product Register on R-Car Gen3 */
> >> +
> >> +static const struct of_device_id renesas_socs[] __initconst = {
> >> +#ifdef CONFIG_ARCH_R8A73A4
> >> + { .compatible = "renesas,r8a73a4", .data = (void *)PRR, },
> >> +#endif
> >> +#ifdef CONFIG_ARCH_R8A7740
> >> + { .compatible = "renesas,r8a7740", .data = (void *)CCCR, },
> >> +#endif
> >
> > My preference here would be to list the register address only for
> > SoCs that are known to need them, while also having .dtb files that
> > don't have the nodes.
>
> Even if drivers don't have to handle differences, there's been a long
> outstanding request to print SoC revision information during bootup
> (E.g. "Does it still work on ES1.0?"). Hence that covers all SoCs.
Ok, fair enough.
> >> +static int __init renesas_soc_init(void)
> >> +{
> >> + struct soc_device_attribute *soc_dev_attr;
> >> + const struct of_device_id *match;
> >> + void __iomem *chipid = NULL;
> >> + struct soc_device *soc_dev;
> >> + struct device_node *np;
> >> + unsigned int product;
> >> +
> >> + np = of_find_matching_node_and_match(NULL, renesas_socs, &match);
> >> + if (!np)
> >> + return -ENODEV;
> >> +
> >> + of_node_put(np);
> >> +
> >> + /* Try PRR first, then CCCR, then hardcoded fallback */
> >> + np = of_find_compatible_node(NULL, NULL, "renesas,prr");
> >> + if (!np)
> >> + np = of_find_compatible_node(NULL, NULL, "renesas,cccr");
> >> + if (np) {
> >> + chipid = of_iomap(np, 0);
> >> + of_node_put(np);
> >> + } else if (match->data) {
> >> + chipid = ioremap((uintptr_t)match->data, 4);
> >> + }
> >> + if (!chipid)
> >>
> >
> > Here, I'd turn the order around and look for the DT nodes of the
> > devices first. Only if they are not found, look at the compatible
> > string of the root node. No need to search for a node though,
> > you know which one it is when you look for a compatible =
> > "renesas,r8a73a4".
>
> "renesas,r8a73a4" is the root node, not the device, so it does not have the
> "reg" property for reading the chip ID?
I mean replace of_find_matching_node_and_match() with
of_match_node(renesas_socs, of_root).
It does the same thing, just more efficiently.
> There is no SoC part number in the "renesas,prr" and "renesas,cccr" nodes.
> Hence I always need to look at the root nodes.
Not sure what that would protect you from. Could you have a renesas,cccr
Arnd
^ permalink raw reply
* [PATCH] iommu: convert DT component matching to component_match_add_release()
From: Joerg Roedel @ 2016-11-10 11:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <E1bwo8o-0005LV-54@rmk-PC.armlinux.org.uk>
On Wed, Oct 19, 2016 at 11:30:34AM +0100, Russell King wrote:
> Convert DT component matching to use component_match_add_release().
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> drivers/iommu/mtk_iommu.c | 8 +++++---
> drivers/iommu/mtk_iommu.h | 5 +++++
> drivers/iommu/mtk_iommu_v1.c | 8 +++++---
> 3 files changed, 15 insertions(+), 6 deletions(-)
Applied, thanks.
^ permalink raw reply
* [RFC v2 4/8] iommu: Add a list of iommu_reserved_region in iommu_domain
From: Auger Eric @ 2016-11-10 11:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <71bfe2a5-455e-9335-ba81-097ef24399ee@arm.com>
Hi Robin,
On 04/11/2016 15:00, Robin Murphy wrote:
> Hi Eric,
>
> Thanks for posting this new series - the bottom-up approach is a lot
> easier to reason about :)
>
> On 04/11/16 11:24, Eric Auger wrote:
>> Introduce a new iommu_reserved_region struct. This embodies
>> an IOVA reserved region that cannot be used along with the IOMMU
>> API. The list is protected by a dedicated mutex.
>
> In the light of these patches, I think I'm settling into agreement that
> the iommu_domain is the sweet spot for accessing this information - the
> underlying magic address ranges might be properties of various bits of
> hardware many of which aren't the IOMMU itself, but they only start to
> matter at the point you start wanting to use an IOMMU domain at the
> higher level. Therefore, having a callback in the domain ops to pull
> everything together fits rather neatly.
Using get_dm_regions could have make sense but this approach now is
ruled out by sysfs API approach. If attribute file is bound to be used
before iommu domains are created, we cannot rely on any iommu_domain
based callback. Back to square 1?
Thanks
Eric
>
>>
>> An iommu domain now owns a list of those.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>> ---
>> drivers/iommu/iommu.c | 2 ++
>> include/linux/iommu.h | 17 +++++++++++++++++
>> 2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 9a2f196..0af07492 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -1061,6 +1061,8 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
>>
>> domain->ops = bus->iommu_ops;
>> domain->type = type;
>> + INIT_LIST_HEAD(&domain->reserved_regions);
>> + mutex_init(&domain->resv_mutex);
>> /* Assume all sizes by default; the driver may override this later */
>> domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap;
>>
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 436dc21..0f2eb64 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -84,6 +84,8 @@ struct iommu_domain {
>> void *handler_token;
>> struct iommu_domain_geometry geometry;
>> void *iova_cookie;
>> + struct list_head reserved_regions;
>> + struct mutex resv_mutex; /* protects the reserved region list */
>> };
>>
>> enum iommu_cap {
>> @@ -131,6 +133,21 @@ struct iommu_dm_region {
>> int prot;
>> };
>>
>> +/**
>> + * struct iommu_reserved_region - descriptor for a reserved iova region
>> + * @list: Linked list pointers
>> + * @start: IOVA base address of the region
>> + * @length: Length of the region in bytes
>> + */
>> +struct iommu_reserved_region {
>> + struct list_head list;
>> + dma_addr_t start;
>> + size_t length;
>> +};
>
> Looking at this in context with the dm_region above, though, I come to
> the surprising realisation that these *are* dm_regions, even at the
> fundamental level - on the one hand you've got physical addresses which
> can't be remapped (because something is already using them), while on
> the other you've got physical addresses which can't be remapped (because
> the IOMMU is incapable). In fact for reserved regions *other* than our
> faked-up MSI region there's no harm if the IOMMU were to actually
> identity-map them.
>
> Let's just add this to the existing infrastructure, either with some
> kind of IOMMU_NOMAP flag or simply prot = 0. That way it automatically
> gets shared between the VFIO and DMA cases for free!
>
> Robin.
>
>> +
>> +#define iommu_reserved_region_for_each(resv, d) \
>> + list_for_each_entry(resv, &(d)->reserved_regions, list)
>> +
>> #ifdef CONFIG_IOMMU_API
>>
>> /**
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Mark Rutland @ 2016-11-10 11:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478647002.7430.69.camel@kernel.crashing.org>
On Wed, Nov 09, 2016 at 10:16:42AM +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2016-11-08 at 12:03 +0000, Mark Rutland wrote:
> > On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
> > >
> > > For arm64, there is no I/O space as other architectural platforms, such as
> > > X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
> > > such as Hip06, when accessing some legacy ISA devices connected to LPC, those
> > > known port addresses are used to control the corresponding target devices, for
> > > example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
> > > normal MMIO mode in using.
> >
> > This has nothing to do with arm64. Hardware with this kind of indirect
> > bus access could be integrated with a variety of CPU architectures. It
> > simply hasn't been, yet.
>
> On some ppc's we also use similar indirect access methods for IOs. We
> have a generic infrastructure for re-routing some memory or IO regions
> to hooks.
>
> On POWER8, our PCIe doesn't do IO at all, but we have an LPC bus behind
> firmware calls ;-) We use that infrastructure to plumb in the LPC bus.
Just to check, do you hook that in your inb/outb/etc?
Generally, it would seem nicer if we could have higher-level
isa_{inb,outb,whatever} accessors that we could hook separately from
other IO.
We don't necessarily have to move all ISA drivers over to that if we had
a separate symbol for that interface.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v16 0/5] Mediatek MT8173 CMDQ support
From: Horng-Shyang Liao @ 2016-11-10 11:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477999698-6288-1-git-send-email-hs.liao@mediatek.com>
On Tue, 2016-11-01 at 19:28 +0800, HS Liao wrote:
> Hi,
>
> This is Mediatek MT8173 Command Queue(CMDQ) driver. The CMDQ is used
> to help write registers with critical time limitation, such as
> updating display configuration during the vblank. It controls Global
> Command Engine (GCE) hardware to achieve this requirement.
>
> These patches have a build dependency on top of v4.9-rc1.
>
> Changes since v15:
> - separate "suspend and resume" patch from "save energy" patch
> - don't stop running tasks in cmdq_suspend()
> (i.e. leave no running tasks guarantee to clients)
>
> Best regards,
> HS Liao
>
> HS Liao (5):
> dt-bindings: soc: Add documentation for the MediaTek GCE unit
> CMDQ: Mediatek CMDQ driver
> arm64: dts: mt8173: Add GCE node
> CMDQ: suspend and resume
> CMDQ: save energy
>
> .../devicetree/bindings/mailbox/mtk-gce.txt | 43 ++
> arch/arm64/boot/dts/mediatek/mt8173.dtsi | 10 +
> drivers/mailbox/Kconfig | 10 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/mtk-cmdq-mailbox.c | 632 +++++++++++++++++++++
> drivers/soc/mediatek/Kconfig | 11 +
> drivers/soc/mediatek/Makefile | 1 +
> drivers/soc/mediatek/mtk-cmdq-helper.c | 310 ++++++++++
> include/linux/mailbox/mtk-cmdq-mailbox.h | 67 +++
> include/linux/soc/mediatek/mtk-cmdq.h | 182 ++++++
> 10 files changed, 1268 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt
> create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c
> create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c
> create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h
> create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h
>
Hi Jassi, Matthias,
Sorry to disturb you.
Do you have any further comment on CMDQ v16?
Thanks.
HS
^ permalink raw reply
* Summary of LPC guest MSI discussion in Santa Fe
From: Auger Eric @ 2016-11-10 11:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161110020130.GA19108@arm.com>
Hi Will, Alex,
On 10/11/2016 03:01, Will Deacon wrote:
> On Wed, Nov 09, 2016 at 05:55:17PM -0700, Alex Williamson wrote:
>> On Thu, 10 Nov 2016 01:14:42 +0100
>> Auger Eric <eric.auger@redhat.com> wrote:
>>> On 10/11/2016 00:59, Alex Williamson wrote:
>>>> On Wed, 9 Nov 2016 23:38:50 +0000
>>>> Will Deacon <will.deacon@arm.com> wrote:
>>>>> On Wed, Nov 09, 2016 at 04:24:58PM -0700, Alex Williamson wrote:
>>>>>> The VFIO_IOMMU_MAP_DMA ioctl is a contract, the user ask to map a range
>>>>>> of IOVAs to a range of virtual addresses for a given device. If VFIO
>>>>>> cannot reasonably fulfill that contract, it must fail. It's up to QEMU
>>>>>> how to manage the hotplug and what memory regions it asks VFIO to map
>>>>>> for a device, but VFIO must reject mappings that it (or the SMMU by
>>>>>> virtue of using the IOMMU API) know to overlap reserved ranges. So I
>>>>>> still disagree with the referenced statement. Thanks,
>>>>>
>>>>> I think that's a pity. Not only does it mean that both QEMU and the kernel
>>>>> have more work to do (the former has to carve up its mapping requests,
>>>>> whilst the latter has to check that it is indeed doing this), but it also
>>>>> precludes the use of hugepage mappings on the IOMMU because of reserved
>>>>> regions. For example, a 4k hole someplace may mean we can't put down 1GB
>>>>> table entries for the guest memory in the SMMU.
>>>>>
>>>>> All this seems to do is add complexity and decrease performance. For what?
>>>>> QEMU has to go read the reserved regions from someplace anyway. It's also
>>>>> the way that VFIO works *today* on arm64 wrt reserved regions, it just has
>>>>> no way to identify those holes at present.
>>>>
>>>> Sure, that sucks, but how is the alternative even an option? The user
>>>> asked to map something, we can't, if we allow that to happen now it's a
>>>> bug. Put the MSI doorbells somewhere that this won't be an issue. If
>>>> the platform has it fixed somewhere that this is an issue, don't use
>>>> that platform. The correctness of the interface is more important than
>>>> catering to a poorly designed system layout IMO. Thanks,
>>>
>>> Besides above problematic, I started to prototype the sysfs API. A first
>>> issue I face is the reserved regions become global to the iommu instead
>>> of characterizing the iommu_domain, ie. the "reserved_regions" attribute
>>> file sits below an iommu instance (~
>>> /sys/class/iommu/dmar0/intel-iommu/reserved_regions ||
>>> /sys/class/iommu/arm-smmu0/arm-smmu/reserved_regions).
>>>
>>> MSI reserved window can be considered global to the IOMMU. However PCIe
>>> host bridge P2P regions rather are per iommu-domain.
>
> I don't think we can treat them as per-domain, given that we want to
> enumerate this stuff before we've decided to do a hotplug (and therefore
> don't have a domain).
That's the issue indeed. We need to wait for the PCIe device to be
connected to the iommu. Only on the VFIO SET_IOMMU we get the
comprehensive list of P2P regions that can impact IOVA mapping for this
iommu. This removes any advantage of sysfs API over previous VFIO
capability chain API for P2P IOVA range enumeration at early stage.
>
>>>
>>> Do you confirm the attribute file should contain both global reserved
>>> regions and all per iommu_domain reserved regions?
>>>
>>> Thoughts?
>>
>> I don't think we have any business describing IOVA addresses consumed
>> by peer devices in an IOMMU sysfs file. If it's a separate device it
>> should be exposed by examining the rest of the topology. Regions
>> consumed by PCI endpoints and interconnects are already exposed in
>> sysfs. In fact, is this perhaps a more accurate model for these MSI
>> controllers too? Perhaps they should be exposed in the bus topology
>> somewhere as consuming the IOVA range.
Currently on x86 the P2P regions are not checked when allowing
passthrough. Aren't we more papist that the pope? As Don mentioned,
shouldn't we simply consider that a platform that does not support
proper ACS is not candidate for safe passthrough, like Juno?
At least we can state the feature also is missing on x86 and it would be
nice to report the risk to the userspace and urge him to opt-in.
To me taking into account those P2P still is controversial and induce
the bulk of the complexity. Considering the migration use case discussed
at LPC while only handling the MSI problem looks much easier.
host can choose an MSI base that is QEMU mach-virt friendly, ie. non RAM
region. Problem is to satisfy all potential uses though. When migrating,
mach-virt still is being used so there should not be any collision. Am I
missing some migration weird use cases here? Of course if we take into
consideration new host PCIe P2P regions this becomes completely different.
We still have the good old v14 where the user space chose where MSI
IOVA's are put without any risk of collision ;-)
>> If DMA to an IOVA is consumed
>> by an intermediate device before it hits the IOMMU vs not being
>> translated as specified by the user at the IOMMU, I'm less inclined to
>> call that something VFIO should reject.
>
> Oh, so perhaps we've been talking past each other. In all of these cases,
> the SMMU can translate the access if it makes it that far. The issue is
> that not all accesses do make it that far, because they may be "consumed"
> by another device, such as an MSI doorbell or another endpoint. In other
> words, I don't envisage a scenario where e.g. some address range just
> bypasses the SMMU straight to memory. I realise now that that's not clear
> from the slides I presented.
>
>> However, instantiating a VM
>> with holes to account for every potential peer device seems like it
>> borders on insanity. Thanks,
>
> Ok, so rather than having a list of reserved regions under the iommu node,
> you're proposing that each region is attributed to the device that "owns"
> (consumes) it? I think that can work, but we need to make sure that:
>
> (a) The topology is walkable from userspace (where do you start?)
>
> (b) It also works for platform (non-PCI) devices, that lack much in the
> way of bus hierarchy
>
> (c) It doesn't require Linux to have a driver bound to a device in order
> for the ranges consumed by that device to be advertised (again,
> more of an issue for non-PCI).
Looks a long way ...
Thanks
Eric
>
> How is this currently advertised for PCI? I'd really like to use the same
> scheme irrespective of the bus type.
>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic
From: Lorenzo Pieralisi @ 2016-11-10 11:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c3136fae-144c-5949-3933-671b783671c8@arm.com>
On Wed, Nov 09, 2016 at 02:40:08PM +0000, Robin Murphy wrote:
[...]
> > +void fwnode_iommu_set_ops(struct fwnode_handle *fwnode,
> > + const struct iommu_ops *ops)
> > +{
> > + struct iommu_fwentry *iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
> > +
> > + if (WARN_ON(!iommu))
> > + return;
> > +
> > + if (is_of_node(fwnode))
>
> Nit: this check is actually redundant, since to_of_node() already does
> the right thing and of_node_get() is NULL-safe - iommu_fwspec_init()
> already works that way. On the other hand, though, it is perhaps more
> intuitive to see it explicitly, and since to_of_node() is inline it
> ought to result in the same object code (I've not checked, though).
I can easily fold this change in the final code and I think we
should keep this consistent so I am happy to change my code and
make the is_of_node() check implicit.
> Either way:
>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Thank you !
Lorenzo
> > + of_node_get(to_of_node(fwnode));
> > +
> > + INIT_LIST_HEAD(&iommu->list);
> > + iommu->fwnode = fwnode;
> > + iommu->ops = ops;
> > + spin_lock(&iommu_fwentry_lock);
> > + list_add_tail(&iommu->list, &iommu_fwentry_list);
> > + spin_unlock(&iommu_fwentry_lock);
> > +}
> > +
> > +const struct iommu_ops *fwnode_iommu_get_ops(struct fwnode_handle *fwnode)
> > +{
> > + struct iommu_fwentry *node;
> > + const struct iommu_ops *ops = NULL;
> > +
> > + spin_lock(&iommu_fwentry_lock);
> > + list_for_each_entry(node, &iommu_fwentry_list, list)
> > + if (node->fwnode == fwnode) {
> > + ops = node->ops;
> > + break;
> > + }
> > + spin_unlock(&iommu_fwentry_lock);
> > + return ops;
> > +}
> > +
> > int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
> > const struct iommu_ops *ops)
> > {
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index 5b82862..0f57ddc 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -96,45 +96,6 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
> > }
> > EXPORT_SYMBOL_GPL(of_get_dma_window);
> >
> > -struct of_iommu_node {
> > - struct list_head list;
> > - struct device_node *np;
> > - const struct iommu_ops *ops;
> > -};
> > -static LIST_HEAD(of_iommu_list);
> > -static DEFINE_SPINLOCK(of_iommu_lock);
> > -
> > -void of_iommu_set_ops(struct device_node *np, const struct iommu_ops *ops)
> > -{
> > - struct of_iommu_node *iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
> > -
> > - if (WARN_ON(!iommu))
> > - return;
> > -
> > - of_node_get(np);
> > - INIT_LIST_HEAD(&iommu->list);
> > - iommu->np = np;
> > - iommu->ops = ops;
> > - spin_lock(&of_iommu_lock);
> > - list_add_tail(&iommu->list, &of_iommu_list);
> > - spin_unlock(&of_iommu_lock);
> > -}
> > -
> > -const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
> > -{
> > - struct of_iommu_node *node;
> > - const struct iommu_ops *ops = NULL;
> > -
> > - spin_lock(&of_iommu_lock);
> > - list_for_each_entry(node, &of_iommu_list, list)
> > - if (node->np == np) {
> > - ops = node->ops;
> > - break;
> > - }
> > - spin_unlock(&of_iommu_lock);
> > - return ops;
> > -}
> > -
> > static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
> > {
> > struct of_phandle_args *iommu_spec = data;
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index 436dc21..15d5478 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -351,6 +351,9 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
> > const struct iommu_ops *ops);
> > void iommu_fwspec_free(struct device *dev);
> > int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
> > +void fwnode_iommu_set_ops(struct fwnode_handle *fwnode,
> > + const struct iommu_ops *ops);
> > +const struct iommu_ops *fwnode_iommu_get_ops(struct fwnode_handle *fwnode);
> >
> > #else /* CONFIG_IOMMU_API */
> >
> > @@ -580,6 +583,17 @@ static inline int iommu_fwspec_add_ids(struct device *dev, u32 *ids,
> > return -ENODEV;
> > }
> >
> > +static inline void fwnode_iommu_set_ops(struct fwnode_handle *fwnode,
> > + const struct iommu_ops *ops)
> > +{
> > +}
> > +
> > +static inline
> > +const struct iommu_ops *fwnode_iommu_get_ops(struct fwnode_handle *fwnode)
> > +{
> > + return NULL;
> > +}
> > +
> > #endif /* CONFIG_IOMMU_API */
> >
> > #endif /* __LINUX_IOMMU_H */
> > diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
> > index e80b9c7..7681007 100644
> > --- a/include/linux/of_iommu.h
> > +++ b/include/linux/of_iommu.h
> > @@ -31,8 +31,16 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
> >
> > #endif /* CONFIG_OF_IOMMU */
> >
> > -void of_iommu_set_ops(struct device_node *np, const struct iommu_ops *ops);
> > -const struct iommu_ops *of_iommu_get_ops(struct device_node *np);
> > +static inline void of_iommu_set_ops(struct device_node *np,
> > + const struct iommu_ops *ops)
> > +{
> > + fwnode_iommu_set_ops(&np->fwnode, ops);
> > +}
> > +
> > +static inline const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
> > +{
> > + return fwnode_iommu_get_ops(&np->fwnode);
> > +}
> >
> > extern struct of_device_id __iommu_of_table;
> >
> >
>
^ 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