Skip to contentSkip to conversation

mmc: pxamci: cleanups and minor DT tweaks

PATCH Applied Daniel Mack 3 participants 14 messages Jun 30, 2018 – Jul 3, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover mmc: pxamci: cleanups and minor DT tweaks

Here is a series of small patches for pxamci that do some cleanups.

The pxa tree has a patch for 4.19 [1] that this series builds upon, so if possible, we'd like to let this series go through Robert's tree after it has been reviewed. If that's not okay for the mmc folks, teh series has to wait until 4.20-rc1.

[1] https://github.com/rjarzmik/linux/commit/6b3348f9e6e

Thanks,
Daniel

Daniel Mack (7):
mmc: pxamci: remove irq from private context
mmc: pxamci: remove dma resources from private context
mmc: pxamci: remove dead code from pxamci_remove()
mmc: pxamci: fix indenting
mmc: pxamci: call mmc_of_parse()
mmc: pxamci: remove pxa-mmc,gpio-power from devicetree bindings
mmc: pxamci: let mmc core handle regulators

 .../devicetree/bindings/mmc/pxa-mmc.txt       |   6 +-
 drivers/mmc/host/pxamci.c                     | 116 +++++++-----------
 2 files changed, 50 insertions(+), 72 deletions(-)
Signature · 1 line
2.17.1
#1 lore raw

PATCH v2 1/7 mmc: pxamci: remove irq from private context

This seems to be a left-over from times before the IRQ was handled by devm functions. Remove it.

---
 drivers/mmc/host/pxamci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/mmc/host/pxamci.c +1 -3
index 6c94474e36f4..f0968882dbd3 100644
@@ -58,7 +58,6 @@ struct pxamci_host {
 	void __iomem		*base;
 	struct clk		*clk;
 	unsigned long		clkrate;
-	int			irq;
 	unsigned int		clkrt;
 	unsigned int		cmdat;
 	unsigned int		imask;
@@ -711,7 +710,6 @@ static int pxamci_probe(struct platform_device *pdev)
 
 	spin_lock_init(&host->lock);
 	host->res = r;
-	host->irq = irq;
 	host->imask = MMC_I_MASK_ALL;
 
 	host->base = devm_ioremap_resource(&pdev->dev, r);
@@ -729,7 +727,7 @@ static int pxamci_probe(struct platform_device *pdev)
 	writel(64, host->base + MMC_RESTO);
 	writel(host->imask, host->base + MMC_I_MASK);
 
-	ret = devm_request_irq(&pdev->dev, host->irq, pxamci_irq, 0,
+	ret = devm_request_irq(&pdev->dev, irq, pxamci_irq, 0,
 			       DRIVER_NAME, host);
 	if (ret)
 		goto out;
Signature · 1 line
2.17.1
#2 lore raw

PATCH v2 2/7 mmc: pxamci: remove dma resources from private context

In reply to Daniel Mack

These members are no longer in use, so let's remove them.

---
 drivers/mmc/host/pxamci.c | 4 ----
 1 file changed, 4 deletions(-)
drivers/mmc/host/pxamci.c +0 -4
index f0968882dbd3..a826c0e8a096 100644
@@ -71,12 +71,8 @@ struct pxamci_host {
 	struct dma_chan		*dma_chan_rx;
 	struct dma_chan		*dma_chan_tx;
 	dma_cookie_t		dma_cookie;
-	dma_addr_t		sg_dma;
 	unsigned int		dma_len;
-
 	unsigned int		dma_dir;
-	unsigned int		dma_drcmrrx;
-	unsigned int		dma_drcmrtx;
 
 	struct regulator	*vcc;
 };
Signature · 1 line
2.17.1
#3 lore raw

PATCH v2 3/7 mmc: pxamci: remove dead code from pxamci_remove()

In reply to Daniel Mack

These gpio assignments don't make sense, as they are not used anywhere. Remove the dead code.

---
 drivers/mmc/host/pxamci.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)
drivers/mmc/host/pxamci.c +1 -6
index a826c0e8a096..ee1c32862ef8 100644
@@ -806,18 +806,12 @@ static int pxamci_probe(struct platform_device *pdev)
 static int pxamci_remove(struct platform_device *pdev)
 {
 	struct mmc_host *mmc = platform_get_drvdata(pdev);
-	int gpio_cd = -1, gpio_ro = -1, gpio_power = -1;
 
 	if (mmc) {
 		struct pxamci_host *host = mmc_priv(mmc);
 
 		mmc_remove_host(mmc);
 
-		if (host->pdata) {
-			gpio_cd = host->pdata->gpio_card_detect;
-			gpio_ro = host->pdata->gpio_card_ro;
-			gpio_power = host->pdata->gpio_power;
-		}
 		if (host->pdata && host->pdata->exit)
 			host->pdata->exit(&pdev->dev, mmc);
 
@@ -833,6 +827,7 @@ static int pxamci_remove(struct platform_device *pdev)
 
 		mmc_free_host(mmc);
 	}
+
 	return 0;
 }
 
Signature · 1 line
2.17.1
#4 lore raw

PATCH v2 4/7 mmc: pxamci: fix indenting

In reply to Daniel Mack

pxamci_of_init() had some weird indenting.

---
 drivers/mmc/host/pxamci.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
drivers/mmc/host/pxamci.c +10 -10
index ee1c32862ef8..3df60c7babce 100644
@@ -593,16 +593,16 @@ MODULE_DEVICE_TABLE(of, pxa_mmc_dt_ids);
 
 static int pxamci_of_init(struct platform_device *pdev)
 {
-        struct device_node *np = pdev->dev.of_node;
-        struct pxamci_platform_data *pdata;
-        u32 tmp;
+	struct device_node *np = pdev->dev.of_node;
+	struct pxamci_platform_data *pdata;
+	u32 tmp;
 
-        if (!np)
-                return 0;
+	if (!np)
+		return 0;
 
-        pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
-        if (!pdata)
-                return -ENOMEM;
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
 
 	pdata->gpio_card_detect =
 		of_get_named_gpio(np, "cd-gpios", 0);
@@ -616,9 +616,9 @@ static int pxamci_of_init(struct platform_device *pdev)
 	if (of_property_read_u32(np, "pxa-mmc,detect-delay-ms", &tmp) == 0)
 		pdata->detect_delay_ms = tmp;
 
-        pdev->dev.platform_data = pdata;
+	pdev->dev.platform_data = pdata;
 
-        return 0;
+	return 0;
 }
 #else
 static int pxamci_of_init(struct platform_device *pdev)
Signature · 1 line
2.17.1
#5 lore raw

PATCH v2 5/7 mmc: pxamci: call mmc_of_parse()

In reply to Daniel Mack

Call into mmc_of_parse() from pxamci_of_init(). As it needs a pointer to a struct mmc_host, refactor the code a bit.

This allows all generic MMC properties to be set that are described in Documentation/devicetree/bindings/mmc/mmc.txt. Reword the documentation a bit to make that clear.

The "cd" and "wp" gpio lookups are removed as the lookup will now be done by mmc_of_parse().

---
 .../devicetree/bindings/mmc/pxa-mmc.txt       |  4 ++--
 drivers/mmc/host/pxamci.c                     | 24 ++++++++++---------
 2 files changed, 15 insertions(+), 13 deletions(-)
Documentation/devicetree/bindings/mmc/pxa-mmc.txt +2 -2
index b7025de7dced..f2687752597e 100644
@@ -10,8 +10,8 @@ Optional properties:
 - marvell,detect-delay-ms: sets the detection delay timeout in ms.
 - marvell,gpio-power: GPIO spec for the card power enable pin
 
-This file documents differences between the core properties in mmc.txt
-and the properties used by the pxa-mmc driver.
+In addition to the properties described in this docuent, the details
+described in mmc.txt are supported.
 
 Examples:
 
drivers/mmc/host/pxamci.c +13 -11
index 3df60c7babce..e046e684b3f9 100644
@@ -591,11 +591,13 @@ static const struct of_device_id pxa_mmc_dt_ids[] = {
 
 MODULE_DEVICE_TABLE(of, pxa_mmc_dt_ids);
 
-static int pxamci_of_init(struct platform_device *pdev)
+static int pxamci_of_init(struct platform_device *pdev,
+			  struct mmc_host *mmc)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct pxamci_platform_data *pdata;
 	u32 tmp;
+	int ret;
 
 	if (!np)
 		return 0;
@@ -604,11 +606,6 @@ static int pxamci_of_init(struct platform_device *pdev)
 	if (!pdata)
 		return -ENOMEM;
 
-	pdata->gpio_card_detect =
-		of_get_named_gpio(np, "cd-gpios", 0);
-	pdata->gpio_card_ro =
-		of_get_named_gpio(np, "wp-gpios", 0);
-
 	/* pxa-mmc specific */
 	pdata->gpio_power =
 		of_get_named_gpio(np, "pxa-mmc,gpio-power", 0);
@@ -616,12 +613,17 @@ static int pxamci_of_init(struct platform_device *pdev)
 	if (of_property_read_u32(np, "pxa-mmc,detect-delay-ms", &tmp) == 0)
 		pdata->detect_delay_ms = tmp;
 
+	ret = mmc_of_parse(mmc);
+	if (ret < 0)
+		return ret;
+
 	pdev->dev.platform_data = pdata;
 
 	return 0;
 }
 #else
-static int pxamci_of_init(struct platform_device *pdev)
+static int pxamci_of_init(struct platform_device *pdev,
+			  struct mmc_host *mmc)
 {
         return 0;
 }
@@ -634,10 +636,6 @@ static int pxamci_probe(struct platform_device *pdev)
 	struct resource *r;
 	int ret, irq, gpio_cd = -1, gpio_ro = -1, gpio_power = -1;
 
-	ret = pxamci_of_init(pdev);
-	if (ret)
-		return ret;
-
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
@@ -672,6 +670,10 @@ static int pxamci_probe(struct platform_device *pdev)
 	 */
 	mmc->max_blk_count = 65535;
 
+	ret = pxamci_of_init(pdev, mmc);
+	if (ret)
+		return ret;
+
 	host = mmc_priv(mmc);
 	host->mmc = mmc;
 	host->pdata = pdev->dev.platform_data;
Signature · 1 line
2.17.1
#6 lore raw

PATCH v2 6/7 mmc: pxamci: remove pxa-mmc,gpio-power from devicetree bindings

In reply to Daniel Mack

Devicetree-enabled boards should use proper regulators to control the power of cards, not GPIOs, so let's remove this property. The regulator properties are supported by the MMC core and are described in the generic MMC document:

Documentation/devicetree/bindings/mmc/mmc.txt

Note that devicetree support for PXA platforms hasn't fully landed yet, so this binding does not have any users at this point.

---
 Documentation/devicetree/bindings/mmc/pxa-mmc.txt | 2 +-
 drivers/mmc/host/pxamci.c                         | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)
Documentation/devicetree/bindings/mmc/pxa-mmc.txt +1 -1
index f2687752597e..5f5c2bec2b8c 100644
@@ -8,7 +8,6 @@ Required properties:
 
 Optional properties:
 - marvell,detect-delay-ms: sets the detection delay timeout in ms.
-- marvell,gpio-power: GPIO spec for the card power enable pin
 
 In addition to the properties described in this docuent, the details
 described in mmc.txt are supported.
@@ -19,6 +18,7 @@ mmc0: mmc@41100000 {
 	compatible = "marvell,pxa-mmc";
 	reg = <0x41100000 0x1000>;
 	interrupts = <23>;
+	vmmc-supply = <&mmc_regulator>;
 	cd-gpios = <&gpio 23 0>;
 	wp-gpios = <&gpio 24 0>;
 };
drivers/mmc/host/pxamci.c +0 -3
index e046e684b3f9..1e7703116763 100644
@@ -607,9 +607,6 @@ static int pxamci_of_init(struct platform_device *pdev,
 		return -ENOMEM;
 
 	/* pxa-mmc specific */
-	pdata->gpio_power =
-		of_get_named_gpio(np, "pxa-mmc,gpio-power", 0);
-
 	if (of_property_read_u32(np, "pxa-mmc,detect-delay-ms", &tmp) == 0)
 		pdata->detect_delay_ms = tmp;
 
Signature · 1 line
2.17.1
#7 lore raw

PATCH v2 7/7 mmc: pxamci: let mmc core handle regulators

In reply to Daniel Mack

Strip some code by letting the mmc core handle the regulators. The old .gpio_power pdata handling is kept around for now.

This also set the voltage on the regulator and handles -EPROBE_DEFER correctly.

---
 drivers/mmc/host/pxamci.c | 54 ++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 32 deletions(-)
drivers/mmc/host/pxamci.c +22 -32
index 1e7703116763..ed576809fae8 100644
@@ -73,58 +73,46 @@ struct pxamci_host {
 	dma_cookie_t		dma_cookie;
 	unsigned int		dma_len;
 	unsigned int		dma_dir;
-
-	struct regulator	*vcc;
 };
 
-static inline void pxamci_init_ocr(struct pxamci_host *host)
+static int pxamci_init_ocr(struct pxamci_host *host)
 {
-#ifdef CONFIG_REGULATOR
-	host->vcc = devm_regulator_get_optional(mmc_dev(host->mmc), "vmmc");
-
-	if (IS_ERR(host->vcc))
-		host->vcc = NULL;
-	else {
-		host->mmc->ocr_avail = mmc_regulator_get_ocrmask(host->vcc);
-		if (host->pdata && host->pdata->ocr_mask)
-			dev_warn(mmc_dev(host->mmc),
-				"ocr_mask/setpower will not be used\n");
-	}
-#endif
-	if (host->vcc == NULL) {
+	struct mmc_host *mmc = host->mmc;
+	int ret;
+
+	ret = mmc_regulator_get_supply(mmc);
+	if (ret < 0)
+		return ret;
+
+	if (IS_ERR(mmc->supply.vmmc)) {
 		/* fall-back to platform data */
-		host->mmc->ocr_avail = host->pdata ?
+		mmc->ocr_avail = host->pdata ?
 			host->pdata->ocr_mask :
 			MMC_VDD_32_33 | MMC_VDD_33_34;
 	}
+
+	return 0;
 }
 
 static inline int pxamci_set_power(struct pxamci_host *host,
 				    unsigned char power_mode,
 				    unsigned int vdd)
 {
+	struct mmc_host *mmc = host->mmc;
+	struct regulator *supply = mmc->supply.vmmc;
 	int on;
 
-	if (host->vcc) {
-		int ret;
+	if (!IS_ERR(supply))
+		return mmc_regulator_set_ocr(mmc, supply, vdd);
 
-		if (power_mode == MMC_POWER_UP) {
-			ret = mmc_regulator_set_ocr(host->mmc, host->vcc, vdd);
-			if (ret)
-				return ret;
-		} else if (power_mode == MMC_POWER_OFF) {
-			ret = mmc_regulator_set_ocr(host->mmc, host->vcc, 0);
-			if (ret)
-				return ret;
-		}
-	}
-	if (!host->vcc && host->pdata &&
+	if (host->pdata &&
 	    gpio_is_valid(host->pdata->gpio_power)) {
 		on = ((1 << vdd) & host->pdata->ocr_mask);
 		gpio_set_value(host->pdata->gpio_power,
 			       !!on ^ host->pdata->gpio_power_invert);
 	}
-	if (!host->vcc && host->pdata && host->pdata->setpower)
+
+	if (host->pdata && host->pdata->setpower)
 		return host->pdata->setpower(mmc_dev(host->mmc), vdd);
 
 	return 0;
@@ -691,7 +679,9 @@ static int pxamci_probe(struct platform_device *pdev)
 	mmc->f_min = (host->clkrate + 63) / 64;
 	mmc->f_max = (mmc_has_26MHz()) ? 26000000 : host->clkrate;
 
-	pxamci_init_ocr(host);
+	ret = pxamci_init_ocr(host);
+	if (ret < 0)
+		return ret;
 
 	mmc->caps = 0;
 	host->cmdat = 0;
Signature · 1 line
2.17.1
Applied
#8 lore raw
In reply to Daniel Mack
Quoted 22 lines · Daniel Mack

Here is a series of small patches for pxamci that do some cleanups.

The pxa tree has a patch for 4.19 [1] that this series builds upon, so if possible, we'd like to let this series go through Robert's tree after it has been reviewed. If that's not okay for the mmc folks, teh series has to wait until 4.20-rc1.

[1] https://github.com/rjarzmik/linux/commit/6b3348f9e6e

Thanks,
Daniel

Daniel Mack (7):
mmc: pxamci: remove irq from private context
mmc: pxamci: remove dma resources from private context
mmc: pxamci: remove dead code from pxamci_remove()
mmc: pxamci: fix indenting
mmc: pxamci: call mmc_of_parse()
mmc: pxamci: remove pxa-mmc,gpio-power from devicetree bindings
mmc: pxamci: let mmc core handle regulators

Quoted 7 lines
 .../devicetree/bindings/mmc/pxa-mmc.txt       |   6 +-
 drivers/mmc/host/pxamci.c                     | 116 +++++++-----------
 2 files changed, 50 insertions(+), 72 deletions(-)

--
2.17.1

Thanks, applied for next!

Kind regards
Uffe

#9 lore raw
Quoted 27 lines · Ulf Hansson
Quoted 26 lines · Daniel Mack

Here is a series of small patches for pxamci that do some cleanups.

The pxa tree has a patch for 4.19 [1] that this series builds upon, so if possible, we'd like to let this series go through Robert's tree after it has been reviewed. If that's not okay for the mmc folks, teh series has to wait until 4.20-rc1.

[1] https://github.com/rjarzmik/linux/commit/6b3348f9e6e

Thanks,
Daniel

Daniel Mack (7):
mmc: pxamci: remove irq from private context
mmc: pxamci: remove dma resources from private context
mmc: pxamci: remove dead code from pxamci_remove()
mmc: pxamci: fix indenting
mmc: pxamci: call mmc_of_parse()
mmc: pxamci: remove pxa-mmc,gpio-power from devicetree bindings
mmc: pxamci: let mmc core handle regulators

  .../devicetree/bindings/mmc/pxa-mmc.txt       |   6 +-
  drivers/mmc/host/pxamci.c                     | 116 +++++++-----------
  2 files changed, 50 insertions(+), 72 deletions(-)
Quoted 5 lines
Quoted 3 lines

--
2.17.1

Thanks, applied for next!

Hmm, applied on top of Robert's patch in [1] (see above)? Without that patch, I expect build breakage with this series.

Thanks,
Daniel

#10 lore raw
Quoted 34 lines · Daniel Mack
Quoted 33 lines · Ulf Hansson
Quoted 30 lines · Daniel Mack

Here is a series of small patches for pxamci that do some cleanups.

The pxa tree has a patch for 4.19 [1] that this series builds upon, so if possible, we'd like to let this series go through Robert's tree after it has been reviewed. If that's not okay for the mmc folks, teh series has to wait until 4.20-rc1.

[1] https://github.com/rjarzmik/linux/commit/6b3348f9e6e

Thanks,
Daniel

Daniel Mack (7):
mmc: pxamci: remove irq from private context
mmc: pxamci: remove dma resources from private context
mmc: pxamci: remove dead code from pxamci_remove()
mmc: pxamci: fix indenting
mmc: pxamci: call mmc_of_parse()
mmc: pxamci: remove pxa-mmc,gpio-power from devicetree bindings
mmc: pxamci: let mmc core handle regulators

  .../devicetree/bindings/mmc/pxa-mmc.txt       |   6 +-
  drivers/mmc/host/pxamci.c                     | 116 +++++++-----------
  2 files changed, 50 insertions(+), 72 deletions(-)

--
2.17.1

Quoted 5 lines
Quoted 1 line

Thanks, applied for next!

Hmm, applied on top of Robert's patch in [1] (see above)? Without that patch, I expect build breakage with this series.

Right, I drop the series. Thanks for letting me know!

I think [1] is available from an immutable branch somewhere. I look it up and merge it before applying this again.

Kind regards
Uffe

#11 lore raw
Quoted 11 lines · Ulf Hansson
Quoted 5 lines · Daniel Mack
Quoted 2 lines · Ulf Hansson

On 30 June 2018 at 20:13, Daniel Mack <[email protected]> wrote:

Hmm, applied on top of Robert's patch in [1] (see above)? Without that patch, I expect build breakage with this series.

Right, I drop the series. Thanks for letting me know!

I think [1] is available from an immutable branch somewhere. I look it up and merge it before applying this again.

Yup, it's here :
git fetch https://github.com/rjarzmik/linux.git tags/pxa-for-4.19-dma_slave_map

Let it spare a bit of your maintainer time :)

Cheers.

Signature · 1 line
Robert
#12 lore raw
Quoted 9 lines · Robert Jarzmik
Quoted 7 lines · Ulf Hansson
Quoted 5 lines · Daniel Mack
Quoted 2 lines · Ulf Hansson

On 30 June 2018 at 20:13, Daniel Mack <[email protected]> wrote:

Hmm, applied on top of Robert's patch in [1] (see above)? Without that patch, I expect build breakage with this series.

Quoted 8 lines
Quoted 4 lines

Right, I drop the series. Thanks for letting me know!

I think [1] is available from an immutable branch somewhere. I look it up and merge it before applying this again.

Yup, it's here :
git fetch https://github.com/rjarzmik/linux.git tags/pxa-for-4.19-dma_slave_map

Let it spare a bit of your maintainer time :)

Thanks. I have pulled the branch and re-applied the series from Daniel.

Kind regards
Uffe

#13 lore raw
Quoted 12 lines · Ulf Hansson
Quoted 11 lines · Robert Jarzmik
Quoted 9 lines · Ulf Hansson
Quoted 5 lines · Daniel Mack
Quoted 2 lines · Ulf Hansson

On 30 June 2018 at 20:13, Daniel Mack <[email protected]> wrote:

Hmm, applied on top of Robert's patch in [1] (see above)? Without that patch, I expect build breakage with this series.

Right, I drop the series. Thanks for letting me know!

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

I think [1] is available from an immutable branch somewhere. I look it up and merge it before applying this again.

Yup, it's here :
git fetch https://github.com/rjarzmik/linux.git tags/pxa-for-4.19-dma_slave_map

Let it spare a bit of your maintainer time :)

Thanks. I have pulled the branch and re-applied the series from Daniel.

Great, thank you!

Daniel