* [RFC PATCH v2 01/12] mtd: rawnand: ams-delta: Assign mtd->dev.parent, not mtd->owner
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
@ 2018-08-06 22:29 ` Janusz Krzysztofik
2018-08-06 23:54 ` Marek Vasut
2018-08-07 16:57 ` Boris Brezillon
2018-08-06 22:29 ` [RFC PATCH v2 02/12] mtd: rawnand: ams-delta: Use private structure Janusz Krzysztofik
` (11 subsequent siblings)
12 siblings, 2 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-06 22:29 UTC (permalink / raw)
To: Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
Fix missing mtd->dev.parent assignment and drop useless mtd->owner.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
drivers/mtd/nand/raw/ams-delta.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 2a8872ebd14a..af313c620264 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -162,7 +162,7 @@ static int ams_delta_init(struct platform_device *pdev)
}
ams_delta_mtd = nand_to_mtd(this);
- ams_delta_mtd->owner = THIS_MODULE;
+ ams_delta_mtd->dev.parent = &pdev->dev;
/*
* Don't try to request the memory region from here,
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 01/12] mtd: rawnand: ams-delta: Assign mtd->dev.parent, not mtd->owner
2018-08-06 22:29 ` [RFC PATCH v2 01/12] mtd: rawnand: ams-delta: Assign mtd->dev.parent, not mtd->owner Janusz Krzysztofik
@ 2018-08-06 23:54 ` Marek Vasut
2018-08-07 21:55 ` Janusz Krzysztofik
2018-08-07 16:57 ` Boris Brezillon
1 sibling, 1 reply; 93+ messages in thread
From: Marek Vasut @ 2018-08-06 23:54 UTC (permalink / raw)
To: Janusz Krzysztofik, Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Tony Lindgren, Aaro Koskinen,
linux-arm-kernel, linux-omap, linux-mtd, linux-doc, linux-gpio,
linux-kernel
On 08/07/2018 12:29 AM, Janusz Krzysztofik wrote:
> Fix missing mtd->dev.parent assignment and drop useless mtd->owner.
You fail to explain why this fix is required.
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> ---
> drivers/mtd/nand/raw/ams-delta.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
> index 2a8872ebd14a..af313c620264 100644
> --- a/drivers/mtd/nand/raw/ams-delta.c
> +++ b/drivers/mtd/nand/raw/ams-delta.c
> @@ -162,7 +162,7 @@ static int ams_delta_init(struct platform_device *pdev)
> }
>
> ams_delta_mtd = nand_to_mtd(this);
> - ams_delta_mtd->owner = THIS_MODULE;
> + ams_delta_mtd->dev.parent = &pdev->dev;
>
> /*
> * Don't try to request the memory region from here,
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 01/12] mtd: rawnand: ams-delta: Assign mtd->dev.parent, not mtd->owner
2018-08-06 23:54 ` Marek Vasut
@ 2018-08-07 21:55 ` Janusz Krzysztofik
0 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-07 21:55 UTC (permalink / raw)
To: Marek Vasut
Cc: Boris Brezillon, Linus Walleij, Jonathan Corbet, Miquel Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
Hi Marek,
On Tuesday, August 7, 2018 1:54:10 AM CEST Marek Vasut wrote:
> On 08/07/2018 12:29 AM, Janusz Krzysztofik wrote:
> > Fix missing mtd->dev.parent assignment and drop useless mtd->owner.
>
> You fail to explain why this fix is required.
OK, I'll have a look at similar patches from the past and add an explanation.
Thanks,
Janusz
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 01/12] mtd: rawnand: ams-delta: Assign mtd->dev.parent, not mtd->owner
2018-08-06 22:29 ` [RFC PATCH v2 01/12] mtd: rawnand: ams-delta: Assign mtd->dev.parent, not mtd->owner Janusz Krzysztofik
2018-08-06 23:54 ` Marek Vasut
@ 2018-08-07 16:57 ` Boris Brezillon
1 sibling, 0 replies; 93+ messages in thread
From: Boris Brezillon @ 2018-08-07 16:57 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Linus Walleij, Jonathan Corbet, Miquel Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, linux-arm-kernel, linux-omap,
linux-mtd, linux-doc, linux-gpio, linux-kernel
On Tue, 7 Aug 2018 00:29:07 +0200
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Fix missing mtd->dev.parent assignment and drop useless mtd->owner.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
> drivers/mtd/nand/raw/ams-delta.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
> index 2a8872ebd14a..af313c620264 100644
> --- a/drivers/mtd/nand/raw/ams-delta.c
> +++ b/drivers/mtd/nand/raw/ams-delta.c
> @@ -162,7 +162,7 @@ static int ams_delta_init(struct platform_device *pdev)
> }
>
> ams_delta_mtd = nand_to_mtd(this);
> - ams_delta_mtd->owner = THIS_MODULE;
> + ams_delta_mtd->dev.parent = &pdev->dev;
>
> /*
> * Don't try to request the memory region from here,
^ permalink raw reply [flat|nested] 93+ messages in thread
* [RFC PATCH v2 02/12] mtd: rawnand: ams-delta: Use private structure
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
2018-08-06 22:29 ` [RFC PATCH v2 01/12] mtd: rawnand: ams-delta: Assign mtd->dev.parent, not mtd->owner Janusz Krzysztofik
@ 2018-08-06 22:29 ` Janusz Krzysztofik
2018-08-07 16:59 ` Boris Brezillon
2018-08-06 22:29 ` [RFC PATCH v2 03/12] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port Janusz Krzysztofik
` (10 subsequent siblings)
12 siblings, 1 reply; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-06 22:29 UTC (permalink / raw)
To: Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
Introduce a driver private structure and allocate it on device probe.
Use it for storing nand_chip structure, GPIO descriptors prevoiusly
stored in static variables as well as io_base pointer previously passed
as nand controller data or platform driver data. Subsequent patches
may populate the structure with more members as needed.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
drivers/mtd/nand/raw/ams-delta.c | 126 +++++++++++++++++++++------------------
1 file changed, 69 insertions(+), 57 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index af313c620264..48233d638d2a 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -34,14 +34,18 @@
/*
* MTD structure for E3 (Delta)
*/
-static struct mtd_info *ams_delta_mtd = NULL;
-static struct gpio_desc *gpiod_rdy;
-static struct gpio_desc *gpiod_nce;
-static struct gpio_desc *gpiod_nre;
-static struct gpio_desc *gpiod_nwp;
-static struct gpio_desc *gpiod_nwe;
-static struct gpio_desc *gpiod_ale;
-static struct gpio_desc *gpiod_cle;
+
+struct ams_delta_nand {
+ struct nand_chip nand_chip;
+ struct gpio_desc *gpiod_rdy;
+ struct gpio_desc *gpiod_nce;
+ struct gpio_desc *gpiod_nre;
+ struct gpio_desc *gpiod_nwp;
+ struct gpio_desc *gpiod_nwe;
+ struct gpio_desc *gpiod_ale;
+ struct gpio_desc *gpiod_cle;
+ void __iomem *io_base;
+};
/*
* Define partitions for flash devices
@@ -71,26 +75,28 @@ static const struct mtd_partition partition_info[] = {
static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
{
struct nand_chip *this = mtd_to_nand(mtd);
- void __iomem *io_base = (void __iomem *)nand_get_controller_data(this);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
+ void __iomem *io_base = priv->io_base;
writew(0, io_base + OMAP_MPUIO_IO_CNTL);
writew(byte, this->IO_ADDR_W);
- gpiod_set_value(gpiod_nwe, 0);
+ gpiod_set_value(priv->gpiod_nwe, 0);
ndelay(40);
- gpiod_set_value(gpiod_nwe, 1);
+ gpiod_set_value(priv->gpiod_nwe, 1);
}
static u_char ams_delta_read_byte(struct mtd_info *mtd)
{
u_char res;
struct nand_chip *this = mtd_to_nand(mtd);
- void __iomem *io_base = (void __iomem *)nand_get_controller_data(this);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
+ void __iomem *io_base = priv->io_base;
- gpiod_set_value(gpiod_nre, 0);
+ gpiod_set_value(priv->gpiod_nre, 0);
ndelay(40);
writew(~0, io_base + OMAP_MPUIO_IO_CNTL);
res = readw(this->IO_ADDR_R);
- gpiod_set_value(gpiod_nre, 1);
+ gpiod_set_value(priv->gpiod_nre, 1);
return res;
}
@@ -123,11 +129,13 @@ static void ams_delta_read_buf(struct mtd_info *mtd, u_char *buf, int len)
static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
unsigned int ctrl)
{
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
if (ctrl & NAND_CTRL_CHANGE) {
- gpiod_set_value(gpiod_nce, !(ctrl & NAND_NCE));
- gpiod_set_value(gpiod_cle, !!(ctrl & NAND_CLE));
- gpiod_set_value(gpiod_ale, !!(ctrl & NAND_ALE));
+ gpiod_set_value(priv->gpiod_nce, !(ctrl & NAND_NCE));
+ gpiod_set_value(priv->gpiod_cle, !!(ctrl & NAND_CLE));
+ gpiod_set_value(priv->gpiod_ale, !!(ctrl & NAND_ALE));
}
if (cmd != NAND_CMD_NONE)
@@ -136,7 +144,10 @@ static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
static int ams_delta_nand_ready(struct mtd_info *mtd)
{
- return gpiod_get_value(gpiod_rdy);
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
+
+ return gpiod_get_value(priv->gpiod_rdy);
}
@@ -145,7 +156,9 @@ static int ams_delta_nand_ready(struct mtd_info *mtd)
*/
static int ams_delta_init(struct platform_device *pdev)
{
+ struct ams_delta_nand *priv;
struct nand_chip *this;
+ struct mtd_info *mtd;
struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
void __iomem *io_base;
int err = 0;
@@ -154,15 +167,16 @@ static int ams_delta_init(struct platform_device *pdev)
return -ENXIO;
/* Allocate memory for MTD device structure and private data */
- this = kzalloc(sizeof(struct nand_chip), GFP_KERNEL);
- if (!this) {
+ priv = devm_kzalloc(&pdev->dev, sizeof(struct ams_delta_nand),
+ GFP_KERNEL);
+ if (!priv) {
pr_warn("Unable to allocate E3 NAND MTD device structure.\n");
- err = -ENOMEM;
- goto out;
+ return -ENOMEM;
}
+ this = &priv->nand_chip;
- ams_delta_mtd = nand_to_mtd(this);
- ams_delta_mtd->dev.parent = &pdev->dev;
+ mtd = nand_to_mtd(this);
+ mtd->dev.parent = &pdev->dev;
/*
* Don't try to request the memory region from here,
@@ -177,7 +191,8 @@ static int ams_delta_init(struct platform_device *pdev)
goto out_free;
}
- nand_set_controller_data(this, (void *)io_base);
+ priv->io_base = io_base;
+ nand_set_controller_data(this, priv);
/* Set address of NAND IO lines */
this->IO_ADDR_R = io_base + OMAP_MPUIO_INPUT_LATCH;
@@ -187,14 +202,14 @@ static int ams_delta_init(struct platform_device *pdev)
this->read_buf = ams_delta_read_buf;
this->cmd_ctrl = ams_delta_hwcontrol;
- gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
- if (IS_ERR(gpiod_rdy)) {
- err = PTR_ERR(gpiod_rdy);
+ priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+ if (IS_ERR(priv->gpiod_rdy)) {
+ err = PTR_ERR(priv->gpiod_rdy);
dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
goto out_mtd;
}
- if (gpiod_rdy)
+ if (priv->gpiod_rdy)
this->dev_ready = ams_delta_nand_ready;
/* 25 us command delay time */
@@ -202,66 +217,64 @@ static int ams_delta_init(struct platform_device *pdev)
this->ecc.mode = NAND_ECC_SOFT;
this->ecc.algo = NAND_ECC_HAMMING;
- platform_set_drvdata(pdev, io_base);
+ platform_set_drvdata(pdev, priv);
/* Set chip enabled, but */
- gpiod_nwp = devm_gpiod_get(&pdev->dev, "nwp", GPIOD_OUT_HIGH);
- if (IS_ERR(gpiod_nwp)) {
- err = PTR_ERR(gpiod_nwp);
+ priv->gpiod_nwp = devm_gpiod_get(&pdev->dev, "nwp", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->gpiod_nwp)) {
+ err = PTR_ERR(priv->gpiod_nwp);
dev_err(&pdev->dev, "NWP GPIO request failed (%d)\n", err);
goto out_mtd;
}
- gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
- if (IS_ERR(gpiod_nce)) {
- err = PTR_ERR(gpiod_nce);
+ priv->gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->gpiod_nce)) {
+ err = PTR_ERR(priv->gpiod_nce);
dev_err(&pdev->dev, "NCE GPIO request failed (%d)\n", err);
goto out_mtd;
}
- gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
- if (IS_ERR(gpiod_nre)) {
- err = PTR_ERR(gpiod_nre);
+ priv->gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->gpiod_nre)) {
+ err = PTR_ERR(priv->gpiod_nre);
dev_err(&pdev->dev, "NRE GPIO request failed (%d)\n", err);
goto out_mtd;
}
- gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
- if (IS_ERR(gpiod_nwe)) {
- err = PTR_ERR(gpiod_nwe);
+ priv->gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->gpiod_nwe)) {
+ err = PTR_ERR(priv->gpiod_nwe);
dev_err(&pdev->dev, "NWE GPIO request failed (%d)\n", err);
goto out_mtd;
}
- gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
- if (IS_ERR(gpiod_ale)) {
- err = PTR_ERR(gpiod_ale);
+ priv->gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
+ if (IS_ERR(priv->gpiod_ale)) {
+ err = PTR_ERR(priv->gpiod_ale);
dev_err(&pdev->dev, "ALE GPIO request failed (%d)\n", err);
goto out_mtd;
}
- gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
- if (IS_ERR(gpiod_cle)) {
- err = PTR_ERR(gpiod_cle);
+ priv->gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
+ if (IS_ERR(priv->gpiod_cle)) {
+ err = PTR_ERR(priv->gpiod_cle);
dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
goto out_mtd;
}
/* Scan to find existence of the device */
- err = nand_scan(ams_delta_mtd, 1);
+ err = nand_scan(mtd, 1);
if (err)
goto out_mtd;
/* Register the partitions */
- mtd_device_register(ams_delta_mtd, partition_info,
- ARRAY_SIZE(partition_info));
+ mtd_device_register(mtd, partition_info, ARRAY_SIZE(partition_info));
goto out;
out_mtd:
iounmap(io_base);
out_free:
- kfree(this);
out:
return err;
}
@@ -271,16 +284,15 @@ static int ams_delta_init(struct platform_device *pdev)
*/
static int ams_delta_cleanup(struct platform_device *pdev)
{
- void __iomem *io_base = platform_get_drvdata(pdev);
+ struct ams_delta_nand *priv = platform_get_drvdata(pdev);
+ struct mtd_info *mtd = nand_to_mtd(&priv->nand_chip);
+ void __iomem *io_base = priv->io_base;
/* Release resources, unregister device */
- nand_release(ams_delta_mtd);
+ nand_release(mtd);
iounmap(io_base);
- /* Free the MTD device structure */
- kfree(mtd_to_nand(ams_delta_mtd));
-
return 0;
}
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 02/12] mtd: rawnand: ams-delta: Use private structure
2018-08-06 22:29 ` [RFC PATCH v2 02/12] mtd: rawnand: ams-delta: Use private structure Janusz Krzysztofik
@ 2018-08-07 16:59 ` Boris Brezillon
0 siblings, 0 replies; 93+ messages in thread
From: Boris Brezillon @ 2018-08-07 16:59 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Linus Walleij, Jonathan Corbet, Miquel Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, linux-arm-kernel, linux-omap,
linux-mtd, linux-doc, linux-gpio, linux-kernel
On Tue, 7 Aug 2018 00:29:08 +0200
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Introduce a driver private structure and allocate it on device probe.
> Use it for storing nand_chip structure, GPIO descriptors prevoiusly
> stored in static variables as well as io_base pointer previously passed
> as nand controller data or platform driver data. Subsequent patches
> may populate the structure with more members as needed.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
> drivers/mtd/nand/raw/ams-delta.c | 126 +++++++++++++++++++++------------------
> 1 file changed, 69 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
> index af313c620264..48233d638d2a 100644
> --- a/drivers/mtd/nand/raw/ams-delta.c
> +++ b/drivers/mtd/nand/raw/ams-delta.c
> @@ -34,14 +34,18 @@
> /*
> * MTD structure for E3 (Delta)
> */
> -static struct mtd_info *ams_delta_mtd = NULL;
> -static struct gpio_desc *gpiod_rdy;
> -static struct gpio_desc *gpiod_nce;
> -static struct gpio_desc *gpiod_nre;
> -static struct gpio_desc *gpiod_nwp;
> -static struct gpio_desc *gpiod_nwe;
> -static struct gpio_desc *gpiod_ale;
> -static struct gpio_desc *gpiod_cle;
> +
> +struct ams_delta_nand {
> + struct nand_chip nand_chip;
> + struct gpio_desc *gpiod_rdy;
> + struct gpio_desc *gpiod_nce;
> + struct gpio_desc *gpiod_nre;
> + struct gpio_desc *gpiod_nwp;
> + struct gpio_desc *gpiod_nwe;
> + struct gpio_desc *gpiod_ale;
> + struct gpio_desc *gpiod_cle;
> + void __iomem *io_base;
> +};
>
> /*
> * Define partitions for flash devices
> @@ -71,26 +75,28 @@ static const struct mtd_partition partition_info[] = {
> static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
> {
> struct nand_chip *this = mtd_to_nand(mtd);
> - void __iomem *io_base = (void __iomem *)nand_get_controller_data(this);
> + struct ams_delta_nand *priv = nand_get_controller_data(this);
> + void __iomem *io_base = priv->io_base;
>
> writew(0, io_base + OMAP_MPUIO_IO_CNTL);
> writew(byte, this->IO_ADDR_W);
> - gpiod_set_value(gpiod_nwe, 0);
> + gpiod_set_value(priv->gpiod_nwe, 0);
> ndelay(40);
> - gpiod_set_value(gpiod_nwe, 1);
> + gpiod_set_value(priv->gpiod_nwe, 1);
> }
>
> static u_char ams_delta_read_byte(struct mtd_info *mtd)
> {
> u_char res;
> struct nand_chip *this = mtd_to_nand(mtd);
> - void __iomem *io_base = (void __iomem *)nand_get_controller_data(this);
> + struct ams_delta_nand *priv = nand_get_controller_data(this);
> + void __iomem *io_base = priv->io_base;
>
> - gpiod_set_value(gpiod_nre, 0);
> + gpiod_set_value(priv->gpiod_nre, 0);
> ndelay(40);
> writew(~0, io_base + OMAP_MPUIO_IO_CNTL);
> res = readw(this->IO_ADDR_R);
> - gpiod_set_value(gpiod_nre, 1);
> + gpiod_set_value(priv->gpiod_nre, 1);
>
> return res;
> }
> @@ -123,11 +129,13 @@ static void ams_delta_read_buf(struct mtd_info *mtd, u_char *buf, int len)
> static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
> unsigned int ctrl)
> {
> + struct nand_chip *this = mtd_to_nand(mtd);
> + struct ams_delta_nand *priv = nand_get_controller_data(this);
>
> if (ctrl & NAND_CTRL_CHANGE) {
> - gpiod_set_value(gpiod_nce, !(ctrl & NAND_NCE));
> - gpiod_set_value(gpiod_cle, !!(ctrl & NAND_CLE));
> - gpiod_set_value(gpiod_ale, !!(ctrl & NAND_ALE));
> + gpiod_set_value(priv->gpiod_nce, !(ctrl & NAND_NCE));
> + gpiod_set_value(priv->gpiod_cle, !!(ctrl & NAND_CLE));
> + gpiod_set_value(priv->gpiod_ale, !!(ctrl & NAND_ALE));
> }
>
> if (cmd != NAND_CMD_NONE)
> @@ -136,7 +144,10 @@ static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
>
> static int ams_delta_nand_ready(struct mtd_info *mtd)
> {
> - return gpiod_get_value(gpiod_rdy);
> + struct nand_chip *this = mtd_to_nand(mtd);
> + struct ams_delta_nand *priv = nand_get_controller_data(this);
> +
> + return gpiod_get_value(priv->gpiod_rdy);
> }
>
>
> @@ -145,7 +156,9 @@ static int ams_delta_nand_ready(struct mtd_info *mtd)
> */
> static int ams_delta_init(struct platform_device *pdev)
> {
> + struct ams_delta_nand *priv;
> struct nand_chip *this;
> + struct mtd_info *mtd;
> struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> void __iomem *io_base;
> int err = 0;
> @@ -154,15 +167,16 @@ static int ams_delta_init(struct platform_device *pdev)
> return -ENXIO;
>
> /* Allocate memory for MTD device structure and private data */
> - this = kzalloc(sizeof(struct nand_chip), GFP_KERNEL);
> - if (!this) {
> + priv = devm_kzalloc(&pdev->dev, sizeof(struct ams_delta_nand),
> + GFP_KERNEL);
> + if (!priv) {
> pr_warn("Unable to allocate E3 NAND MTD device structure.\n");
> - err = -ENOMEM;
> - goto out;
> + return -ENOMEM;
> }
> + this = &priv->nand_chip;
>
> - ams_delta_mtd = nand_to_mtd(this);
> - ams_delta_mtd->dev.parent = &pdev->dev;
> + mtd = nand_to_mtd(this);
> + mtd->dev.parent = &pdev->dev;
>
> /*
> * Don't try to request the memory region from here,
> @@ -177,7 +191,8 @@ static int ams_delta_init(struct platform_device *pdev)
> goto out_free;
> }
>
> - nand_set_controller_data(this, (void *)io_base);
> + priv->io_base = io_base;
> + nand_set_controller_data(this, priv);
>
> /* Set address of NAND IO lines */
> this->IO_ADDR_R = io_base + OMAP_MPUIO_INPUT_LATCH;
> @@ -187,14 +202,14 @@ static int ams_delta_init(struct platform_device *pdev)
> this->read_buf = ams_delta_read_buf;
> this->cmd_ctrl = ams_delta_hwcontrol;
>
> - gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> - if (IS_ERR(gpiod_rdy)) {
> - err = PTR_ERR(gpiod_rdy);
> + priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> + if (IS_ERR(priv->gpiod_rdy)) {
> + err = PTR_ERR(priv->gpiod_rdy);
> dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
> goto out_mtd;
> }
>
> - if (gpiod_rdy)
> + if (priv->gpiod_rdy)
> this->dev_ready = ams_delta_nand_ready;
>
> /* 25 us command delay time */
> @@ -202,66 +217,64 @@ static int ams_delta_init(struct platform_device *pdev)
> this->ecc.mode = NAND_ECC_SOFT;
> this->ecc.algo = NAND_ECC_HAMMING;
>
> - platform_set_drvdata(pdev, io_base);
> + platform_set_drvdata(pdev, priv);
>
> /* Set chip enabled, but */
> - gpiod_nwp = devm_gpiod_get(&pdev->dev, "nwp", GPIOD_OUT_HIGH);
> - if (IS_ERR(gpiod_nwp)) {
> - err = PTR_ERR(gpiod_nwp);
> + priv->gpiod_nwp = devm_gpiod_get(&pdev->dev, "nwp", GPIOD_OUT_HIGH);
> + if (IS_ERR(priv->gpiod_nwp)) {
> + err = PTR_ERR(priv->gpiod_nwp);
> dev_err(&pdev->dev, "NWP GPIO request failed (%d)\n", err);
> goto out_mtd;
> }
>
> - gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
> - if (IS_ERR(gpiod_nce)) {
> - err = PTR_ERR(gpiod_nce);
> + priv->gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
> + if (IS_ERR(priv->gpiod_nce)) {
> + err = PTR_ERR(priv->gpiod_nce);
> dev_err(&pdev->dev, "NCE GPIO request failed (%d)\n", err);
> goto out_mtd;
> }
>
> - gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
> - if (IS_ERR(gpiod_nre)) {
> - err = PTR_ERR(gpiod_nre);
> + priv->gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
> + if (IS_ERR(priv->gpiod_nre)) {
> + err = PTR_ERR(priv->gpiod_nre);
> dev_err(&pdev->dev, "NRE GPIO request failed (%d)\n", err);
> goto out_mtd;
> }
>
> - gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
> - if (IS_ERR(gpiod_nwe)) {
> - err = PTR_ERR(gpiod_nwe);
> + priv->gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
> + if (IS_ERR(priv->gpiod_nwe)) {
> + err = PTR_ERR(priv->gpiod_nwe);
> dev_err(&pdev->dev, "NWE GPIO request failed (%d)\n", err);
> goto out_mtd;
> }
>
> - gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
> - if (IS_ERR(gpiod_ale)) {
> - err = PTR_ERR(gpiod_ale);
> + priv->gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
> + if (IS_ERR(priv->gpiod_ale)) {
> + err = PTR_ERR(priv->gpiod_ale);
> dev_err(&pdev->dev, "ALE GPIO request failed (%d)\n", err);
> goto out_mtd;
> }
>
> - gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
> - if (IS_ERR(gpiod_cle)) {
> - err = PTR_ERR(gpiod_cle);
> + priv->gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
> + if (IS_ERR(priv->gpiod_cle)) {
> + err = PTR_ERR(priv->gpiod_cle);
> dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
> goto out_mtd;
> }
>
> /* Scan to find existence of the device */
> - err = nand_scan(ams_delta_mtd, 1);
> + err = nand_scan(mtd, 1);
> if (err)
> goto out_mtd;
>
> /* Register the partitions */
> - mtd_device_register(ams_delta_mtd, partition_info,
> - ARRAY_SIZE(partition_info));
> + mtd_device_register(mtd, partition_info, ARRAY_SIZE(partition_info));
>
> goto out;
>
> out_mtd:
> iounmap(io_base);
> out_free:
> - kfree(this);
> out:
> return err;
> }
> @@ -271,16 +284,15 @@ static int ams_delta_init(struct platform_device *pdev)
> */
> static int ams_delta_cleanup(struct platform_device *pdev)
> {
> - void __iomem *io_base = platform_get_drvdata(pdev);
> + struct ams_delta_nand *priv = platform_get_drvdata(pdev);
> + struct mtd_info *mtd = nand_to_mtd(&priv->nand_chip);
> + void __iomem *io_base = priv->io_base;
>
> /* Release resources, unregister device */
> - nand_release(ams_delta_mtd);
> + nand_release(mtd);
>
> iounmap(io_base);
>
> - /* Free the MTD device structure */
> - kfree(mtd_to_nand(ams_delta_mtd));
> -
> return 0;
> }
>
^ permalink raw reply [flat|nested] 93+ messages in thread
* [RFC PATCH v2 03/12] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
2018-08-06 22:29 ` [RFC PATCH v2 01/12] mtd: rawnand: ams-delta: Assign mtd->dev.parent, not mtd->owner Janusz Krzysztofik
2018-08-06 22:29 ` [RFC PATCH v2 02/12] mtd: rawnand: ams-delta: Use private structure Janusz Krzysztofik
@ 2018-08-06 22:29 ` Janusz Krzysztofik
2018-08-07 16:59 ` Boris Brezillon
2018-08-10 10:10 ` Linus Walleij
2018-08-06 22:29 ` [RFC PATCH v2 04/12] mtd: rawnand: ams-delta: request data port GPIO resource Janusz Krzysztofik
` (9 subsequent siblings)
12 siblings, 2 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-06 22:29 UTC (permalink / raw)
To: Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
Data port used by Amstrad Delta NAND driver is actually an OMAP MPUIO
device, already under control of gpio-omap driver. The NAND driver
gets access to the port by ioremapping it and performs read/write
operations. That is done without any proteciton from other users
legally manipulating the port pins over GPIO API.
The plan is to convert the driver to access the port over GPIO consumer
API. Before that is implemented, the driver can already obtain
exclusive access to the port by requesting an array of its GPIO
descriptors.
Add respective entries to the NAND GPIO lookup table.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
arch/arm/mach-omap1/board-ams-delta.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index eedacdfe9725..16f7bbe47607 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -337,7 +337,8 @@ static struct platform_device ams_delta_nand_device = {
.resource = ams_delta_nand_resources,
};
-#define OMAP_GPIO_LABEL "gpio-0-15"
+#define OMAP_GPIO_LABEL "gpio-0-15"
+#define OMAP_MPUIO_LABEL "mpuio"
static struct gpiod_lookup_table ams_delta_nand_gpio_table = {
.table = {
@@ -349,6 +350,14 @@ static struct gpiod_lookup_table ams_delta_nand_gpio_table = {
GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NWE, "nwe", 0),
GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_ALE, "ale", 0),
GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_CLE, "cle", 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 0, "data", 0, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 1, "data", 1, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 2, "data", 2, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 3, "data", 3, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 4, "data", 4, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 5, "data", 5, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 6, "data", 6, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 7, "data", 7, 0),
{ },
},
};
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 03/12] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
2018-08-06 22:29 ` [RFC PATCH v2 03/12] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port Janusz Krzysztofik
@ 2018-08-07 16:59 ` Boris Brezillon
2018-08-10 10:10 ` Linus Walleij
1 sibling, 0 replies; 93+ messages in thread
From: Boris Brezillon @ 2018-08-07 16:59 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Linus Walleij, Jonathan Corbet, Miquel Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, linux-arm-kernel, linux-omap,
linux-mtd, linux-doc, linux-gpio, linux-kernel
On Tue, 7 Aug 2018 00:29:09 +0200
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Data port used by Amstrad Delta NAND driver is actually an OMAP MPUIO
> device, already under control of gpio-omap driver. The NAND driver
> gets access to the port by ioremapping it and performs read/write
> operations. That is done without any proteciton from other users
> legally manipulating the port pins over GPIO API.
>
> The plan is to convert the driver to access the port over GPIO consumer
> API. Before that is implemented, the driver can already obtain
> exclusive access to the port by requesting an array of its GPIO
> descriptors.
>
> Add respective entries to the NAND GPIO lookup table.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
> arch/arm/mach-omap1/board-ams-delta.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
> index eedacdfe9725..16f7bbe47607 100644
> --- a/arch/arm/mach-omap1/board-ams-delta.c
> +++ b/arch/arm/mach-omap1/board-ams-delta.c
> @@ -337,7 +337,8 @@ static struct platform_device ams_delta_nand_device = {
> .resource = ams_delta_nand_resources,
> };
>
> -#define OMAP_GPIO_LABEL "gpio-0-15"
> +#define OMAP_GPIO_LABEL "gpio-0-15"
> +#define OMAP_MPUIO_LABEL "mpuio"
>
> static struct gpiod_lookup_table ams_delta_nand_gpio_table = {
> .table = {
> @@ -349,6 +350,14 @@ static struct gpiod_lookup_table ams_delta_nand_gpio_table = {
> GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NWE, "nwe", 0),
> GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_ALE, "ale", 0),
> GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_CLE, "cle", 0),
> + GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 0, "data", 0, 0),
> + GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 1, "data", 1, 0),
> + GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 2, "data", 2, 0),
> + GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 3, "data", 3, 0),
> + GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 4, "data", 4, 0),
> + GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 5, "data", 5, 0),
> + GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 6, "data", 6, 0),
> + GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 7, "data", 7, 0),
> { },
> },
> };
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 03/12] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
2018-08-06 22:29 ` [RFC PATCH v2 03/12] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port Janusz Krzysztofik
2018-08-07 16:59 ` Boris Brezillon
@ 2018-08-10 10:10 ` Linus Walleij
1 sibling, 0 replies; 93+ messages in thread
From: Linus Walleij @ 2018-08-10 10:10 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Boris Brezillon, Jonathan Corbet, Miquèl Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Mark Vasut,
ext Tony Lindgren, Aaro Koskinen, Linux ARM, Linux-OMAP,
linux-mtd, linux-doc, open list:GPIO SUBSYSTEM, linux-kernel
On Tue, Aug 7, 2018 at 12:29 AM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Data port used by Amstrad Delta NAND driver is actually an OMAP MPUIO
> device, already under control of gpio-omap driver. The NAND driver
> gets access to the port by ioremapping it and performs read/write
> operations. That is done without any proteciton from other users
> legally manipulating the port pins over GPIO API.
>
> The plan is to convert the driver to access the port over GPIO consumer
> API. Before that is implemented, the driver can already obtain
> exclusive access to the port by requesting an array of its GPIO
> descriptors.
>
> Add respective entries to the NAND GPIO lookup table.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 93+ messages in thread
* [RFC PATCH v2 04/12] mtd: rawnand: ams-delta: request data port GPIO resource
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
` (2 preceding siblings ...)
2018-08-06 22:29 ` [RFC PATCH v2 03/12] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port Janusz Krzysztofik
@ 2018-08-06 22:29 ` Janusz Krzysztofik
2018-08-07 17:00 ` Boris Brezillon
2018-08-10 10:11 ` Linus Walleij
2018-08-06 22:29 ` [RFC PATCH v2 05/12] mtd: rawnand: ams-delta: use GPIO API for data read/write Janusz Krzysztofik
` (8 subsequent siblings)
12 siblings, 2 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-06 22:29 UTC (permalink / raw)
To: Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
Data port used by the driver is actually an OMAP MPUIO device, already
under control of gpio-omap driver. For that reason we used to not
request the memory region of the port as that would fail because the
region is already busy. Despite that, we are still accessing the port
by just ioremapping it and performing read/write operations. Moreover,
we are doing that without any proteciton from other users legally
manipulating the port pins over GPIO API.
The plan is to convert the driver to access the port over functions
exposed by the gpio-omap driver. Before that happens, already prevent
from other users accessing the port pins by requesting an array of its
GPIO descriptors.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
drivers/mtd/nand/raw/ams-delta.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 48233d638d2a..09d6901fc94d 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -161,6 +161,7 @@ static int ams_delta_init(struct platform_device *pdev)
struct mtd_info *mtd;
struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
void __iomem *io_base;
+ struct gpio_descs *data_gpiods;
int err = 0;
if (!res)
@@ -261,6 +262,13 @@ static int ams_delta_init(struct platform_device *pdev)
dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
goto out_mtd;
}
+ /* Request array of data pins, initialize them as input */
+ data_gpiods = devm_gpiod_get_array(&pdev->dev, "data", GPIOD_IN);
+ if (IS_ERR(data_gpiods)) {
+ err = PTR_ERR(data_gpiods);
+ dev_err(&pdev->dev, "data GPIO request failed: %d\n", err);
+ goto out_mtd;
+ }
/* Scan to find existence of the device */
err = nand_scan(mtd, 1);
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 04/12] mtd: rawnand: ams-delta: request data port GPIO resource
2018-08-06 22:29 ` [RFC PATCH v2 04/12] mtd: rawnand: ams-delta: request data port GPIO resource Janusz Krzysztofik
@ 2018-08-07 17:00 ` Boris Brezillon
2018-08-10 10:11 ` Linus Walleij
1 sibling, 0 replies; 93+ messages in thread
From: Boris Brezillon @ 2018-08-07 17:00 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Linus Walleij, Jonathan Corbet, Miquel Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, linux-arm-kernel, linux-omap,
linux-mtd, linux-doc, linux-gpio, linux-kernel
On Tue, 7 Aug 2018 00:29:10 +0200
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Data port used by the driver is actually an OMAP MPUIO device, already
> under control of gpio-omap driver. For that reason we used to not
> request the memory region of the port as that would fail because the
> region is already busy. Despite that, we are still accessing the port
> by just ioremapping it and performing read/write operations. Moreover,
> we are doing that without any proteciton from other users legally
> manipulating the port pins over GPIO API.
>
> The plan is to convert the driver to access the port over functions
> exposed by the gpio-omap driver. Before that happens, already prevent
> from other users accessing the port pins by requesting an array of its
> GPIO descriptors.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
> drivers/mtd/nand/raw/ams-delta.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
> index 48233d638d2a..09d6901fc94d 100644
> --- a/drivers/mtd/nand/raw/ams-delta.c
> +++ b/drivers/mtd/nand/raw/ams-delta.c
> @@ -161,6 +161,7 @@ static int ams_delta_init(struct platform_device *pdev)
> struct mtd_info *mtd;
> struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> void __iomem *io_base;
> + struct gpio_descs *data_gpiods;
> int err = 0;
>
> if (!res)
> @@ -261,6 +262,13 @@ static int ams_delta_init(struct platform_device *pdev)
> dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
> goto out_mtd;
> }
> + /* Request array of data pins, initialize them as input */
> + data_gpiods = devm_gpiod_get_array(&pdev->dev, "data", GPIOD_IN);
> + if (IS_ERR(data_gpiods)) {
> + err = PTR_ERR(data_gpiods);
> + dev_err(&pdev->dev, "data GPIO request failed: %d\n", err);
> + goto out_mtd;
> + }
>
> /* Scan to find existence of the device */
> err = nand_scan(mtd, 1);
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 04/12] mtd: rawnand: ams-delta: request data port GPIO resource
2018-08-06 22:29 ` [RFC PATCH v2 04/12] mtd: rawnand: ams-delta: request data port GPIO resource Janusz Krzysztofik
2018-08-07 17:00 ` Boris Brezillon
@ 2018-08-10 10:11 ` Linus Walleij
1 sibling, 0 replies; 93+ messages in thread
From: Linus Walleij @ 2018-08-10 10:11 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Boris Brezillon, Jonathan Corbet, Miquèl Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Mark Vasut,
ext Tony Lindgren, Aaro Koskinen, Linux ARM, Linux-OMAP,
linux-mtd, linux-doc, open list:GPIO SUBSYSTEM, linux-kernel
On Tue, Aug 7, 2018 at 12:29 AM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Data port used by the driver is actually an OMAP MPUIO device, already
> under control of gpio-omap driver. For that reason we used to not
> request the memory region of the port as that would fail because the
> region is already busy. Despite that, we are still accessing the port
> by just ioremapping it and performing read/write operations. Moreover,
> we are doing that without any proteciton from other users legally
> manipulating the port pins over GPIO API.
>
> The plan is to convert the driver to access the port over functions
> exposed by the gpio-omap driver. Before that happens, already prevent
> from other users accessing the port pins by requesting an array of its
> GPIO descriptors.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 93+ messages in thread
* [RFC PATCH v2 05/12] mtd: rawnand: ams-delta: use GPIO API for data read/write
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
` (3 preceding siblings ...)
2018-08-06 22:29 ` [RFC PATCH v2 04/12] mtd: rawnand: ams-delta: request data port GPIO resource Janusz Krzysztofik
@ 2018-08-06 22:29 ` Janusz Krzysztofik
2018-08-06 23:57 ` Marek Vasut
` (2 more replies)
2018-08-06 22:29 ` [RFC PATCH v2 06/12] ARM: OMAP1: ams-delta: drop obsolete NAND resources Janusz Krzysztofik
` (7 subsequent siblings)
12 siblings, 3 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-06 22:29 UTC (permalink / raw)
To: Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
Don't readw()/writew() data directly from/to GPIO port which is under
control of gpio-omap driver, use GPIO API instead.
Degrade of performance on Amstrad Delta is completely not acceptable.
The driver should work with any 8+-bit bidirectional GPIO port, not
only OMAP.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
drivers/mtd/nand/raw/ams-delta.c | 97 ++++++++++++++++------------------------
1 file changed, 38 insertions(+), 59 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 09d6901fc94d..78996ddf82e0 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -24,13 +24,10 @@
#include <linux/mtd/mtd.h>
#include <linux/mtd/rawnand.h>
#include <linux/mtd/partitions.h>
-#include <linux/platform_data/gpio-omap.h>
+#include <linux/platform_device.h>
-#include <asm/io.h>
#include <asm/sizes.h>
-#include <mach/hardware.h>
-
/*
* MTD structure for E3 (Delta)
*/
@@ -44,7 +41,7 @@ struct ams_delta_nand {
struct gpio_desc *gpiod_nwe;
struct gpio_desc *gpiod_ale;
struct gpio_desc *gpiod_cle;
- void __iomem *io_base;
+ struct gpio_descs *data_gpiods;
};
/*
@@ -76,10 +73,14 @@ static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
{
struct nand_chip *this = mtd_to_nand(mtd);
struct ams_delta_nand *priv = nand_get_controller_data(this);
- void __iomem *io_base = priv->io_base;
+ struct gpio_descs *data_gpiods = priv->data_gpiods;
+ unsigned long bits = byte;
+ int i;
+
+ for (i = 0; i < data_gpiods->ndescs; i++)
+ gpiod_direction_output_raw(data_gpiods->desc[i],
+ test_bit(i, &bits));
- writew(0, io_base + OMAP_MPUIO_IO_CNTL);
- writew(byte, this->IO_ADDR_W);
gpiod_set_value(priv->gpiod_nwe, 0);
ndelay(40);
gpiod_set_value(priv->gpiod_nwe, 1);
@@ -87,18 +88,28 @@ static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
static u_char ams_delta_read_byte(struct mtd_info *mtd)
{
- u_char res;
struct nand_chip *this = mtd_to_nand(mtd);
struct ams_delta_nand *priv = nand_get_controller_data(this);
- void __iomem *io_base = priv->io_base;
+ struct gpio_descs *data_gpiods = priv->data_gpiods;
+ unsigned long bits = 0;
+ int i, value_array[data_gpiods->ndescs];
+
+ for (i = 0; i < data_gpiods->ndescs; i++)
+ gpiod_direction_input(data_gpiods->desc[i]);
gpiod_set_value(priv->gpiod_nre, 0);
ndelay(40);
- writew(~0, io_base + OMAP_MPUIO_IO_CNTL);
- res = readw(this->IO_ADDR_R);
+
+ gpiod_get_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
+ value_array);
+
gpiod_set_value(priv->gpiod_nre, 1);
- return res;
+ for (i = 0; i < data_gpiods->ndescs; i++)
+ if (value_array[i])
+ __set_bit(i, &bits);
+
+ return bits;
}
static void ams_delta_write_buf(struct mtd_info *mtd, const u_char *buf,
@@ -159,14 +170,8 @@ static int ams_delta_init(struct platform_device *pdev)
struct ams_delta_nand *priv;
struct nand_chip *this;
struct mtd_info *mtd;
- struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- void __iomem *io_base;
- struct gpio_descs *data_gpiods;
int err = 0;
- if (!res)
- return -ENXIO;
-
/* Allocate memory for MTD device structure and private data */
priv = devm_kzalloc(&pdev->dev, sizeof(struct ams_delta_nand),
GFP_KERNEL);
@@ -179,25 +184,8 @@ static int ams_delta_init(struct platform_device *pdev)
mtd = nand_to_mtd(this);
mtd->dev.parent = &pdev->dev;
- /*
- * Don't try to request the memory region from here,
- * it should have been already requested from the
- * gpio-omap driver and requesting it again would fail.
- */
-
- io_base = ioremap(res->start, resource_size(res));
- if (io_base == NULL) {
- dev_err(&pdev->dev, "ioremap failed\n");
- err = -EIO;
- goto out_free;
- }
-
- priv->io_base = io_base;
nand_set_controller_data(this, priv);
- /* Set address of NAND IO lines */
- this->IO_ADDR_R = io_base + OMAP_MPUIO_INPUT_LATCH;
- this->IO_ADDR_W = io_base + OMAP_MPUIO_OUTPUT;
this->read_byte = ams_delta_read_byte;
this->write_buf = ams_delta_write_buf;
this->read_buf = ams_delta_read_buf;
@@ -207,7 +195,7 @@ static int ams_delta_init(struct platform_device *pdev)
if (IS_ERR(priv->gpiod_rdy)) {
err = PTR_ERR(priv->gpiod_rdy);
dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
if (priv->gpiod_rdy)
@@ -225,66 +213,60 @@ static int ams_delta_init(struct platform_device *pdev)
if (IS_ERR(priv->gpiod_nwp)) {
err = PTR_ERR(priv->gpiod_nwp);
dev_err(&pdev->dev, "NWP GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
priv->gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
if (IS_ERR(priv->gpiod_nce)) {
err = PTR_ERR(priv->gpiod_nce);
dev_err(&pdev->dev, "NCE GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
priv->gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
if (IS_ERR(priv->gpiod_nre)) {
err = PTR_ERR(priv->gpiod_nre);
dev_err(&pdev->dev, "NRE GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
priv->gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
if (IS_ERR(priv->gpiod_nwe)) {
err = PTR_ERR(priv->gpiod_nwe);
dev_err(&pdev->dev, "NWE GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
priv->gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
if (IS_ERR(priv->gpiod_ale)) {
err = PTR_ERR(priv->gpiod_ale);
dev_err(&pdev->dev, "ALE GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
priv->gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
if (IS_ERR(priv->gpiod_cle)) {
err = PTR_ERR(priv->gpiod_cle);
dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
/* Request array of data pins, initialize them as input */
- data_gpiods = devm_gpiod_get_array(&pdev->dev, "data", GPIOD_IN);
- if (IS_ERR(data_gpiods)) {
- err = PTR_ERR(data_gpiods);
+ priv->data_gpiods = devm_gpiod_get_array(&pdev->dev, "data", GPIOD_IN);
+ if (IS_ERR(priv->data_gpiods)) {
+ err = PTR_ERR(priv->data_gpiods);
dev_err(&pdev->dev, "data GPIO request failed: %d\n", err);
- goto out_mtd;
+ return err;
}
/* Scan to find existence of the device */
err = nand_scan(mtd, 1);
if (err)
- goto out_mtd;
+ return err;
/* Register the partitions */
mtd_device_register(mtd, partition_info, ARRAY_SIZE(partition_info));
- goto out;
-
- out_mtd:
- iounmap(io_base);
-out_free:
- out:
- return err;
+ return 0;
}
/*
@@ -294,13 +276,10 @@ static int ams_delta_cleanup(struct platform_device *pdev)
{
struct ams_delta_nand *priv = platform_get_drvdata(pdev);
struct mtd_info *mtd = nand_to_mtd(&priv->nand_chip);
- void __iomem *io_base = priv->io_base;
- /* Release resources, unregister device */
+ /* Unregister device */
nand_release(mtd);
- iounmap(io_base);
-
return 0;
}
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 05/12] mtd: rawnand: ams-delta: use GPIO API for data read/write
2018-08-06 22:29 ` [RFC PATCH v2 05/12] mtd: rawnand: ams-delta: use GPIO API for data read/write Janusz Krzysztofik
@ 2018-08-06 23:57 ` Marek Vasut
2018-08-07 17:06 ` Boris Brezillon
2018-08-10 10:25 ` Linus Walleij
2 siblings, 0 replies; 93+ messages in thread
From: Marek Vasut @ 2018-08-06 23:57 UTC (permalink / raw)
To: Janusz Krzysztofik, Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Tony Lindgren, Aaro Koskinen,
linux-arm-kernel, linux-omap, linux-mtd, linux-doc, linux-gpio,
linux-kernel
On 08/07/2018 12:29 AM, Janusz Krzysztofik wrote:
> Don't readw()/writew() data directly from/to GPIO port which is under
> control of gpio-omap driver, use GPIO API instead.
>
> Degrade of performance on Amstrad Delta is completely not acceptable.
I'd expect that changing from direct PIO to access through GPIO API
would degrade the performance. Maybe I misunderstood something ?
> The driver should work with any 8+-bit bidirectional GPIO port, not
> only OMAP.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> ---
> drivers/mtd/nand/raw/ams-delta.c | 97 ++++++++++++++++------------------------
> 1 file changed, 38 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
> index 09d6901fc94d..78996ddf82e0 100644
> --- a/drivers/mtd/nand/raw/ams-delta.c
> +++ b/drivers/mtd/nand/raw/ams-delta.c
> @@ -24,13 +24,10 @@
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/rawnand.h>
> #include <linux/mtd/partitions.h>
> -#include <linux/platform_data/gpio-omap.h>
> +#include <linux/platform_device.h>
>
> -#include <asm/io.h>
> #include <asm/sizes.h>
>
> -#include <mach/hardware.h>
> -
> /*
> * MTD structure for E3 (Delta)
> */
> @@ -44,7 +41,7 @@ struct ams_delta_nand {
> struct gpio_desc *gpiod_nwe;
> struct gpio_desc *gpiod_ale;
> struct gpio_desc *gpiod_cle;
> - void __iomem *io_base;
> + struct gpio_descs *data_gpiods;
> };
>
> /*
> @@ -76,10 +73,14 @@ static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
> {
> struct nand_chip *this = mtd_to_nand(mtd);
> struct ams_delta_nand *priv = nand_get_controller_data(this);
> - void __iomem *io_base = priv->io_base;
> + struct gpio_descs *data_gpiods = priv->data_gpiods;
> + unsigned long bits = byte;
> + int i;
> +
> + for (i = 0; i < data_gpiods->ndescs; i++)
> + gpiod_direction_output_raw(data_gpiods->desc[i],
> + test_bit(i, &bits));
>
> - writew(0, io_base + OMAP_MPUIO_IO_CNTL);
> - writew(byte, this->IO_ADDR_W);
> gpiod_set_value(priv->gpiod_nwe, 0);
> ndelay(40);
> gpiod_set_value(priv->gpiod_nwe, 1);
> @@ -87,18 +88,28 @@ static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
>
> static u_char ams_delta_read_byte(struct mtd_info *mtd)
> {
> - u_char res;
> struct nand_chip *this = mtd_to_nand(mtd);
> struct ams_delta_nand *priv = nand_get_controller_data(this);
> - void __iomem *io_base = priv->io_base;
> + struct gpio_descs *data_gpiods = priv->data_gpiods;
> + unsigned long bits = 0;
> + int i, value_array[data_gpiods->ndescs];
> +
> + for (i = 0; i < data_gpiods->ndescs; i++)
> + gpiod_direction_input(data_gpiods->desc[i]);
>
> gpiod_set_value(priv->gpiod_nre, 0);
> ndelay(40);
> - writew(~0, io_base + OMAP_MPUIO_IO_CNTL);
> - res = readw(this->IO_ADDR_R);
> +
> + gpiod_get_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
> + value_array);
> +
> gpiod_set_value(priv->gpiod_nre, 1);
>
> - return res;
> + for (i = 0; i < data_gpiods->ndescs; i++)
> + if (value_array[i])
> + __set_bit(i, &bits);
> +
> + return bits;
> }
>
> static void ams_delta_write_buf(struct mtd_info *mtd, const u_char *buf,
> @@ -159,14 +170,8 @@ static int ams_delta_init(struct platform_device *pdev)
> struct ams_delta_nand *priv;
> struct nand_chip *this;
> struct mtd_info *mtd;
> - struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - void __iomem *io_base;
> - struct gpio_descs *data_gpiods;
> int err = 0;
>
> - if (!res)
> - return -ENXIO;
> -
> /* Allocate memory for MTD device structure and private data */
> priv = devm_kzalloc(&pdev->dev, sizeof(struct ams_delta_nand),
> GFP_KERNEL);
> @@ -179,25 +184,8 @@ static int ams_delta_init(struct platform_device *pdev)
> mtd = nand_to_mtd(this);
> mtd->dev.parent = &pdev->dev;
>
> - /*
> - * Don't try to request the memory region from here,
> - * it should have been already requested from the
> - * gpio-omap driver and requesting it again would fail.
> - */
> -
> - io_base = ioremap(res->start, resource_size(res));
> - if (io_base == NULL) {
> - dev_err(&pdev->dev, "ioremap failed\n");
> - err = -EIO;
> - goto out_free;
> - }
> -
> - priv->io_base = io_base;
> nand_set_controller_data(this, priv);
>
> - /* Set address of NAND IO lines */
> - this->IO_ADDR_R = io_base + OMAP_MPUIO_INPUT_LATCH;
> - this->IO_ADDR_W = io_base + OMAP_MPUIO_OUTPUT;
> this->read_byte = ams_delta_read_byte;
> this->write_buf = ams_delta_write_buf;
> this->read_buf = ams_delta_read_buf;
> @@ -207,7 +195,7 @@ static int ams_delta_init(struct platform_device *pdev)
> if (IS_ERR(priv->gpiod_rdy)) {
> err = PTR_ERR(priv->gpiod_rdy);
> dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
> - goto out_mtd;
> + return err;
> }
>
> if (priv->gpiod_rdy)
> @@ -225,66 +213,60 @@ static int ams_delta_init(struct platform_device *pdev)
> if (IS_ERR(priv->gpiod_nwp)) {
> err = PTR_ERR(priv->gpiod_nwp);
> dev_err(&pdev->dev, "NWP GPIO request failed (%d)\n", err);
> - goto out_mtd;
> + return err;
> }
>
> priv->gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
> if (IS_ERR(priv->gpiod_nce)) {
> err = PTR_ERR(priv->gpiod_nce);
> dev_err(&pdev->dev, "NCE GPIO request failed (%d)\n", err);
> - goto out_mtd;
> + return err;
> }
>
> priv->gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
> if (IS_ERR(priv->gpiod_nre)) {
> err = PTR_ERR(priv->gpiod_nre);
> dev_err(&pdev->dev, "NRE GPIO request failed (%d)\n", err);
> - goto out_mtd;
> + return err;
> }
>
> priv->gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
> if (IS_ERR(priv->gpiod_nwe)) {
> err = PTR_ERR(priv->gpiod_nwe);
> dev_err(&pdev->dev, "NWE GPIO request failed (%d)\n", err);
> - goto out_mtd;
> + return err;
> }
>
> priv->gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
> if (IS_ERR(priv->gpiod_ale)) {
> err = PTR_ERR(priv->gpiod_ale);
> dev_err(&pdev->dev, "ALE GPIO request failed (%d)\n", err);
> - goto out_mtd;
> + return err;
> }
>
> priv->gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
> if (IS_ERR(priv->gpiod_cle)) {
> err = PTR_ERR(priv->gpiod_cle);
> dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
> - goto out_mtd;
> + return err;
> }
> /* Request array of data pins, initialize them as input */
> - data_gpiods = devm_gpiod_get_array(&pdev->dev, "data", GPIOD_IN);
> - if (IS_ERR(data_gpiods)) {
> - err = PTR_ERR(data_gpiods);
> + priv->data_gpiods = devm_gpiod_get_array(&pdev->dev, "data", GPIOD_IN);
> + if (IS_ERR(priv->data_gpiods)) {
> + err = PTR_ERR(priv->data_gpiods);
> dev_err(&pdev->dev, "data GPIO request failed: %d\n", err);
> - goto out_mtd;
> + return err;
> }
>
> /* Scan to find existence of the device */
> err = nand_scan(mtd, 1);
> if (err)
> - goto out_mtd;
> + return err;
>
> /* Register the partitions */
> mtd_device_register(mtd, partition_info, ARRAY_SIZE(partition_info));
>
> - goto out;
> -
> - out_mtd:
> - iounmap(io_base);
> -out_free:
> - out:
> - return err;
> + return 0;
> }
>
> /*
> @@ -294,13 +276,10 @@ static int ams_delta_cleanup(struct platform_device *pdev)
> {
> struct ams_delta_nand *priv = platform_get_drvdata(pdev);
> struct mtd_info *mtd = nand_to_mtd(&priv->nand_chip);
> - void __iomem *io_base = priv->io_base;
>
> - /* Release resources, unregister device */
> + /* Unregister device */
> nand_release(mtd);
>
> - iounmap(io_base);
> -
> return 0;
> }
>
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 05/12] mtd: rawnand: ams-delta: use GPIO API for data read/write
2018-08-06 22:29 ` [RFC PATCH v2 05/12] mtd: rawnand: ams-delta: use GPIO API for data read/write Janusz Krzysztofik
2018-08-06 23:57 ` Marek Vasut
@ 2018-08-07 17:06 ` Boris Brezillon
2018-08-07 17:11 ` Janusz Krzysztofik
2018-08-10 10:25 ` Linus Walleij
2 siblings, 1 reply; 93+ messages in thread
From: Boris Brezillon @ 2018-08-07 17:06 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Linus Walleij, Jonathan Corbet, Miquel Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, linux-arm-kernel, linux-omap,
linux-mtd, linux-doc, linux-gpio, linux-kernel
On Tue, 7 Aug 2018 00:29:11 +0200
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Don't readw()/writew() data directly from/to GPIO port which is under
> control of gpio-omap driver, use GPIO API instead.
>
> Degrade of performance on Amstrad Delta is completely not acceptable.
Can we have numbers along with information about where the overhead is
when using gpiod_{get,set}_raw_array_value()?
>
> The driver should work with any 8+-bit bidirectional GPIO port, not
> only OMAP.
That's cool!
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 05/12] mtd: rawnand: ams-delta: use GPIO API for data read/write
2018-08-07 17:06 ` Boris Brezillon
@ 2018-08-07 17:11 ` Janusz Krzysztofik
0 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-07 17:11 UTC (permalink / raw)
To: Boris Brezillon
Cc: Linus Walleij, Jonathan Corbet, Miquel Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, linux-arm-kernel, linux-omap,
linux-mtd, linux-doc, linux-gpio, linux-kernel,
Janusz Krzysztofik
Hi Boris,
On Tuesday, August 7, 2018 7:06:27 PM CEST Boris Brezillon wrote:
> On Tue, 7 Aug 2018 00:29:11 +0200
> Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
>
> > Don't readw()/writew() data directly from/to GPIO port which is under
> > control of gpio-omap driver, use GPIO API instead.
> >
> > Degrade of performance on Amstrad Delta is completely not acceptable.
>
> Can we have numbers along with information about where the overhead is
> when using gpiod_{get,set}_raw_array_value()?
Yes, as soon as I get physical access to the device, probably this or next
weekend.
Thanks,
Janusz
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 05/12] mtd: rawnand: ams-delta: use GPIO API for data read/write
2018-08-06 22:29 ` [RFC PATCH v2 05/12] mtd: rawnand: ams-delta: use GPIO API for data read/write Janusz Krzysztofik
2018-08-06 23:57 ` Marek Vasut
2018-08-07 17:06 ` Boris Brezillon
@ 2018-08-10 10:25 ` Linus Walleij
2 siblings, 0 replies; 93+ messages in thread
From: Linus Walleij @ 2018-08-10 10:25 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Boris Brezillon, Jonathan Corbet, Miquèl Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Mark Vasut,
ext Tony Lindgren, Aaro Koskinen, Linux ARM, Linux-OMAP,
linux-mtd, linux-doc, open list:GPIO SUBSYSTEM, linux-kernel
On Tue, Aug 7, 2018 at 12:29 AM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Don't readw()/writew() data directly from/to GPIO port which is under
> control of gpio-omap driver, use GPIO API instead.
>
> Degrade of performance on Amstrad Delta is completely not acceptable.
>
> The driver should work with any 8+-bit bidirectional GPIO port, not
> only OMAP.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 93+ messages in thread
* [RFC PATCH v2 06/12] ARM: OMAP1: ams-delta: drop obsolete NAND resources
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
` (4 preceding siblings ...)
2018-08-06 22:29 ` [RFC PATCH v2 05/12] mtd: rawnand: ams-delta: use GPIO API for data read/write Janusz Krzysztofik
@ 2018-08-06 22:29 ` Janusz Krzysztofik
2018-08-06 22:29 ` [RFC PATCH v2 07/12] mtd: rawnand: ams-delta: Set port direction once per transfer Janusz Krzysztofik
` (6 subsequent siblings)
12 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-06 22:29 UTC (permalink / raw)
To: Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
Amstrad Delta NAND device now uses GPIO API for data I/O so there is no
need to assign memory I/O resource to the device any longer. Drop it.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
arch/arm/mach-omap1/board-ams-delta.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 16f7bbe47607..08e732bc1cd2 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -321,20 +321,9 @@ struct modem_private_data {
static struct modem_private_data modem_priv;
-static struct resource ams_delta_nand_resources[] = {
- [0] = {
- .start = OMAP1_MPUIO_BASE,
- .end = OMAP1_MPUIO_BASE +
- OMAP_MPUIO_IO_CNTL + sizeof(u32) - 1,
- .flags = IORESOURCE_MEM,
- },
-};
-
static struct platform_device ams_delta_nand_device = {
.name = "ams-delta-nand",
.id = -1,
- .num_resources = ARRAY_SIZE(ams_delta_nand_resources),
- .resource = ams_delta_nand_resources,
};
#define OMAP_GPIO_LABEL "gpio-0-15"
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* [RFC PATCH v2 07/12] mtd: rawnand: ams-delta: Set port direction once per transfer
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
` (5 preceding siblings ...)
2018-08-06 22:29 ` [RFC PATCH v2 06/12] ARM: OMAP1: ams-delta: drop obsolete NAND resources Janusz Krzysztofik
@ 2018-08-06 22:29 ` Janusz Krzysztofik
2018-08-07 18:57 ` Boris Brezillon
2018-08-06 22:29 ` [RFC PATCH v2 08/12] mtd: rawnand: ams-delta: Simplify pointer resolution on read/write Janusz Krzysztofik
` (5 subsequent siblings)
12 siblings, 1 reply; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-06 22:29 UTC (permalink / raw)
To: Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
In its current shape, the driver sets data port direction before each
byte read/write operation, even during multi-byte transfers. Since
performance of the driver is completely not acceptable on Amstrad Delta
after it has been converted to GPIO bitbang, try to improve things a
bit by setting the port direction only on first byte of each transfer.
Resulting performance on Amstrad Delta is still far from acceptable.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
drivers/mtd/nand/raw/ams-delta.c | 58 ++++++++++++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 11 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 78996ddf82e0..d02c48c013e8 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -69,6 +69,30 @@ static const struct mtd_partition partition_info[] = {
.size = 3 * SZ_256K },
};
+static void ams_delta_write_commit(struct ams_delta_nand *priv)
+{
+ gpiod_set_value(priv->gpiod_nwe, 0);
+ ndelay(40);
+ gpiod_set_value(priv->gpiod_nwe, 1);
+}
+
+static void ams_delta_write_next_byte(struct mtd_info *mtd, u_char byte)
+{
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
+ struct gpio_descs *data_gpiods = priv->data_gpiods;
+ unsigned long bits = byte;
+ int i, value_array[data_gpiods->ndescs];
+
+ for (i = 0; i < data_gpiods->ndescs; i++)
+ value_array[i] = test_bit(i, &bits);
+
+ gpiod_set_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
+ value_array);
+
+ ams_delta_write_commit(priv);
+}
+
static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
{
struct nand_chip *this = mtd_to_nand(mtd);
@@ -81,12 +105,10 @@ static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
gpiod_direction_output_raw(data_gpiods->desc[i],
test_bit(i, &bits));
- gpiod_set_value(priv->gpiod_nwe, 0);
- ndelay(40);
- gpiod_set_value(priv->gpiod_nwe, 1);
+ ams_delta_write_commit(priv);
}
-static u_char ams_delta_read_byte(struct mtd_info *mtd)
+static u_char ams_delta_read_next_byte(struct mtd_info *mtd)
{
struct nand_chip *this = mtd_to_nand(mtd);
struct ams_delta_nand *priv = nand_get_controller_data(this);
@@ -94,9 +116,6 @@ static u_char ams_delta_read_byte(struct mtd_info *mtd)
unsigned long bits = 0;
int i, value_array[data_gpiods->ndescs];
- for (i = 0; i < data_gpiods->ndescs; i++)
- gpiod_direction_input(data_gpiods->desc[i]);
-
gpiod_set_value(priv->gpiod_nre, 0);
ndelay(40);
@@ -112,21 +131,38 @@ static u_char ams_delta_read_byte(struct mtd_info *mtd)
return bits;
}
+static u_char ams_delta_read_byte(struct mtd_info *mtd)
+{
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
+ struct gpio_descs *data_gpiods = priv->data_gpiods;
+ int i;
+
+ for (i = 0; i < data_gpiods->ndescs; i++)
+ gpiod_direction_input(data_gpiods->desc[i]);
+
+ return ams_delta_read_next_byte(mtd);
+}
+
static void ams_delta_write_buf(struct mtd_info *mtd, const u_char *buf,
int len)
{
int i;
- for (i=0; i<len; i++)
- ams_delta_write_byte(mtd, buf[i]);
+ if (len > 0)
+ ams_delta_write_byte(mtd, buf[0]);
+ for (i = 1; i < len; i++)
+ ams_delta_write_next_byte(mtd, buf[i]);
}
static void ams_delta_read_buf(struct mtd_info *mtd, u_char *buf, int len)
{
int i;
- for (i=0; i<len; i++)
- buf[i] = ams_delta_read_byte(mtd);
+ if (len > 0)
+ buf[0] = ams_delta_read_byte(mtd);
+ for (i = 1; i < len; i++)
+ buf[i] = ams_delta_read_next_byte(mtd);
}
/*
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 07/12] mtd: rawnand: ams-delta: Set port direction once per transfer
2018-08-06 22:29 ` [RFC PATCH v2 07/12] mtd: rawnand: ams-delta: Set port direction once per transfer Janusz Krzysztofik
@ 2018-08-07 18:57 ` Boris Brezillon
2018-08-08 16:55 ` Janusz Krzysztofik
0 siblings, 1 reply; 93+ messages in thread
From: Boris Brezillon @ 2018-08-07 18:57 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Brian Norris, Jonathan Corbet, Tony Lindgren, Richard Weinberger,
Linus Walleij, Aaro Koskinen, linux-doc, linux-kernel,
linux-gpio, linux-mtd, Miquel Raynal, linux-omap,
David Woodhouse, Marek Vasut, linux-arm-kernel
On Tue, 7 Aug 2018 00:29:13 +0200
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> In its current shape, the driver sets data port direction before each
> byte read/write operation, even during multi-byte transfers. Since
> performance of the driver is completely not acceptable on Amstrad Delta
> after it has been converted to GPIO bitbang, try to improve things a
> bit by setting the port direction only on first byte of each transfer.
>
> Resulting performance on Amstrad Delta is still far from acceptable.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> ---
> drivers/mtd/nand/raw/ams-delta.c | 58 ++++++++++++++++++++++++++++++++--------
> 1 file changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
> index 78996ddf82e0..d02c48c013e8 100644
> --- a/drivers/mtd/nand/raw/ams-delta.c
> +++ b/drivers/mtd/nand/raw/ams-delta.c
> @@ -69,6 +69,30 @@ static const struct mtd_partition partition_info[] = {
> .size = 3 * SZ_256K },
> };
>
> +static void ams_delta_write_commit(struct ams_delta_nand *priv)
> +{
> + gpiod_set_value(priv->gpiod_nwe, 0);
> + ndelay(40);
> + gpiod_set_value(priv->gpiod_nwe, 1);
> +}
> +
> +static void ams_delta_write_next_byte(struct mtd_info *mtd, u_char byte)
> +{
> + struct nand_chip *this = mtd_to_nand(mtd);
> + struct ams_delta_nand *priv = nand_get_controller_data(this);
> + struct gpio_descs *data_gpiods = priv->data_gpiods;
> + unsigned long bits = byte;
> + int i, value_array[data_gpiods->ndescs];
> +
> + for (i = 0; i < data_gpiods->ndescs; i++)
> + value_array[i] = test_bit(i, &bits);
> +
> + gpiod_set_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
> + value_array);
> +
> + ams_delta_write_commit(priv);
> +}
> +
> static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
> {
> struct nand_chip *this = mtd_to_nand(mtd);
> @@ -81,12 +105,10 @@ static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
> gpiod_direction_output_raw(data_gpiods->desc[i],
> test_bit(i, &bits));
>
> - gpiod_set_value(priv->gpiod_nwe, 0);
> - ndelay(40);
> - gpiod_set_value(priv->gpiod_nwe, 1);
> + ams_delta_write_commit(priv);
> }
>
> -static u_char ams_delta_read_byte(struct mtd_info *mtd)
> +static u_char ams_delta_read_next_byte(struct mtd_info *mtd)
> {
> struct nand_chip *this = mtd_to_nand(mtd);
> struct ams_delta_nand *priv = nand_get_controller_data(this);
> @@ -94,9 +116,6 @@ static u_char ams_delta_read_byte(struct mtd_info *mtd)
> unsigned long bits = 0;
> int i, value_array[data_gpiods->ndescs];
>
> - for (i = 0; i < data_gpiods->ndescs; i++)
> - gpiod_direction_input(data_gpiods->desc[i]);
> -
> gpiod_set_value(priv->gpiod_nre, 0);
> ndelay(40);
>
> @@ -112,21 +131,38 @@ static u_char ams_delta_read_byte(struct mtd_info *mtd)
> return bits;
> }
>
> +static u_char ams_delta_read_byte(struct mtd_info *mtd)
> +{
> + struct nand_chip *this = mtd_to_nand(mtd);
> + struct ams_delta_nand *priv = nand_get_controller_data(this);
> + struct gpio_descs *data_gpiods = priv->data_gpiods;
> + int i;
> +
> + for (i = 0; i < data_gpiods->ndescs; i++)
> + gpiod_direction_input(data_gpiods->desc[i]);
> +
> + return ams_delta_read_next_byte(mtd);
> +}
> +
> static void ams_delta_write_buf(struct mtd_info *mtd, const u_char *buf,
> int len)
> {
> int i;
>
> - for (i=0; i<len; i++)
> - ams_delta_write_byte(mtd, buf[i]);
> + if (len > 0)
> + ams_delta_write_byte(mtd, buf[0]);
> + for (i = 1; i < len; i++)
> + ams_delta_write_next_byte(mtd, buf[i]);
> }
>
> static void ams_delta_read_buf(struct mtd_info *mtd, u_char *buf, int len)
> {
> int i;
>
> - for (i=0; i<len; i++)
> - buf[i] = ams_delta_read_byte(mtd);
> + if (len > 0)
> + buf[0] = ams_delta_read_byte(mtd);
> + for (i = 1; i < len; i++)
> + buf[i] = ams_delta_read_next_byte(mtd);
> }
I'd suggest a slightly different approach where the data pins
direction state is stored in the the priv struct and only changed when
required. This way you just have to add a test in
ams_delta_read/write_byte().
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 07/12] mtd: rawnand: ams-delta: Set port direction once per transfer
2018-08-07 18:57 ` Boris Brezillon
@ 2018-08-08 16:55 ` Janusz Krzysztofik
2018-08-08 17:42 ` Miquel Raynal
0 siblings, 1 reply; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-08 16:55 UTC (permalink / raw)
To: Boris Brezillon
Cc: Linus Walleij, linux-omap, Jonathan Corbet, Tony Lindgren,
Richard Weinberger, linux-gpio, Aaro Koskinen, linux-doc,
linux-kernel, Marek Vasut, linux-mtd, Miquel Raynal,
Brian Norris, David Woodhouse, linux-arm-kernel
Hi Boris,
On Tuesday, August 7, 2018 8:57:52 PM CEST Boris Brezillon wrote:
> On Tue, 7 Aug 2018 00:29:13 +0200
> Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
>
> > In its current shape, the driver sets data port direction before each
> > byte read/write operation, even during multi-byte transfers. Since
> > performance of the driver is completely not acceptable on Amstrad Delta
> > after it has been converted to GPIO bitbang, try to improve things a
> > bit by setting the port direction only on first byte of each transfer.
> ...
> I'd suggest a slightly different approach where the data pins
> direction state is stored in the the priv struct and only changed when
> required. This way you just have to add a test in
> ams_delta_read/write_byte().
Good idea, I'm going to use it, thanks.
Once done, may I also move that one earlier in the series so that it can be
applied while our discussion on GPIO bitmap changes still continues?
Thanks,
Janusz
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 07/12] mtd: rawnand: ams-delta: Set port direction once per transfer
2018-08-08 16:55 ` Janusz Krzysztofik
@ 2018-08-08 17:42 ` Miquel Raynal
0 siblings, 0 replies; 93+ messages in thread
From: Miquel Raynal @ 2018-08-08 17:42 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Boris Brezillon, Linus Walleij, linux-omap, Jonathan Corbet,
Tony Lindgren, Richard Weinberger, linux-gpio, Aaro Koskinen,
linux-doc, linux-kernel, Marek Vasut, linux-mtd, Brian Norris,
David Woodhouse, linux-arm-kernel
Hi Janusz,
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote on Wed, 08 Aug 2018
18:55:35 +0200:
> Hi Boris,
>
> On Tuesday, August 7, 2018 8:57:52 PM CEST Boris Brezillon wrote:
> > On Tue, 7 Aug 2018 00:29:13 +0200
> > Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> >
> > > In its current shape, the driver sets data port direction before each
> > > byte read/write operation, even during multi-byte transfers. Since
> > > performance of the driver is completely not acceptable on Amstrad Delta
> > > after it has been converted to GPIO bitbang, try to improve things a
> > > bit by setting the port direction only on first byte of each transfer.
> > ...
> > I'd suggest a slightly different approach where the data pins
> > direction state is stored in the the priv struct and only changed when
> > required. This way you just have to add a test in
> > ams_delta_read/write_byte().
>
> Good idea, I'm going to use it, thanks.
>
> Once done, may I also move that one earlier in the series so that it can be
> applied while our discussion on GPIO bitmap changes still continues?
I think I may answer on his behalf: yes! You can move the GPIO bitmap
changes at the end of the series, checking that you never break the
bisectability. Then I could apply the major changes and let us iterate
on the GPIO bitmap stuff only.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 93+ messages in thread
* [RFC PATCH v2 08/12] mtd: rawnand: ams-delta: Simplify pointer resolution on read/write
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
` (6 preceding siblings ...)
2018-08-06 22:29 ` [RFC PATCH v2 07/12] mtd: rawnand: ams-delta: Set port direction once per transfer Janusz Krzysztofik
@ 2018-08-06 22:29 ` Janusz Krzysztofik
2018-08-07 17:02 ` Boris Brezillon
2018-08-06 22:29 ` [RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct mapping Janusz Krzysztofik
` (4 subsequent siblings)
12 siblings, 1 reply; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-06 22:29 UTC (permalink / raw)
To: Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
Simplify data read/write sub-functions by changing their APIs so they
accept driver private structure pointer instead of mtd_info.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
drivers/mtd/nand/raw/ams-delta.c | 40 ++++++++++++++++++++++------------------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index d02c48c013e8..30c461138195 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -76,10 +76,8 @@ static void ams_delta_write_commit(struct ams_delta_nand *priv)
gpiod_set_value(priv->gpiod_nwe, 1);
}
-static void ams_delta_write_next_byte(struct mtd_info *mtd, u_char byte)
+static void ams_delta_write_next_byte(struct ams_delta_nand *priv, u_char byte)
{
- struct nand_chip *this = mtd_to_nand(mtd);
- struct ams_delta_nand *priv = nand_get_controller_data(this);
struct gpio_descs *data_gpiods = priv->data_gpiods;
unsigned long bits = byte;
int i, value_array[data_gpiods->ndescs];
@@ -93,10 +91,8 @@ static void ams_delta_write_next_byte(struct mtd_info *mtd, u_char byte)
ams_delta_write_commit(priv);
}
-static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
+static void ams_delta_write_first_byte(struct ams_delta_nand *priv, u_char byte)
{
- struct nand_chip *this = mtd_to_nand(mtd);
- struct ams_delta_nand *priv = nand_get_controller_data(this);
struct gpio_descs *data_gpiods = priv->data_gpiods;
unsigned long bits = byte;
int i;
@@ -108,10 +104,8 @@ static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
ams_delta_write_commit(priv);
}
-static u_char ams_delta_read_next_byte(struct mtd_info *mtd)
+static u_char ams_delta_read_next_byte(struct ams_delta_nand *priv)
{
- struct nand_chip *this = mtd_to_nand(mtd);
- struct ams_delta_nand *priv = nand_get_controller_data(this);
struct gpio_descs *data_gpiods = priv->data_gpiods;
unsigned long bits = 0;
int i, value_array[data_gpiods->ndescs];
@@ -131,38 +125,48 @@ static u_char ams_delta_read_next_byte(struct mtd_info *mtd)
return bits;
}
-static u_char ams_delta_read_byte(struct mtd_info *mtd)
+static u_char ams_delta_read_first_byte(struct ams_delta_nand *priv)
{
- struct nand_chip *this = mtd_to_nand(mtd);
- struct ams_delta_nand *priv = nand_get_controller_data(this);
struct gpio_descs *data_gpiods = priv->data_gpiods;
int i;
for (i = 0; i < data_gpiods->ndescs; i++)
gpiod_direction_input(data_gpiods->desc[i]);
- return ams_delta_read_next_byte(mtd);
+ return ams_delta_read_next_byte(priv);
+}
+
+static u_char ams_delta_read_byte(struct mtd_info *mtd)
+{
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
+
+ return ams_delta_read_first_byte(priv);
}
static void ams_delta_write_buf(struct mtd_info *mtd, const u_char *buf,
int len)
{
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
int i;
if (len > 0)
- ams_delta_write_byte(mtd, buf[0]);
+ ams_delta_write_first_byte(priv, buf[0]);
for (i = 1; i < len; i++)
- ams_delta_write_next_byte(mtd, buf[i]);
+ ams_delta_write_next_byte(priv, buf[i]);
}
static void ams_delta_read_buf(struct mtd_info *mtd, u_char *buf, int len)
{
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
int i;
if (len > 0)
- buf[0] = ams_delta_read_byte(mtd);
+ buf[0] = ams_delta_read_first_byte(priv);
for (i = 1; i < len; i++)
- buf[i] = ams_delta_read_next_byte(mtd);
+ buf[i] = ams_delta_read_next_byte(priv);
}
/*
@@ -186,7 +190,7 @@ static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
}
if (cmd != NAND_CMD_NONE)
- ams_delta_write_byte(mtd, cmd);
+ ams_delta_write_first_byte(priv, cmd);
}
static int ams_delta_nand_ready(struct mtd_info *mtd)
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 08/12] mtd: rawnand: ams-delta: Simplify pointer resolution on read/write
2018-08-06 22:29 ` [RFC PATCH v2 08/12] mtd: rawnand: ams-delta: Simplify pointer resolution on read/write Janusz Krzysztofik
@ 2018-08-07 17:02 ` Boris Brezillon
2018-08-07 17:15 ` Janusz Krzysztofik
0 siblings, 1 reply; 93+ messages in thread
From: Boris Brezillon @ 2018-08-07 17:02 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Linus Walleij, Jonathan Corbet, Miquel Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, linux-arm-kernel, linux-omap,
linux-mtd, linux-doc, linux-gpio, linux-kernel
On Tue, 7 Aug 2018 00:29:14 +0200
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Simplify data read/write sub-functions by changing their APIs so they
> accept driver private structure pointer instead of mtd_info.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
Can you move that one earlier in the series so that it can be applied
even if we're still discussing the GPIO bitmap changes?
> ---
> drivers/mtd/nand/raw/ams-delta.c | 40 ++++++++++++++++++++++------------------
> 1 file changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
> index d02c48c013e8..30c461138195 100644
> --- a/drivers/mtd/nand/raw/ams-delta.c
> +++ b/drivers/mtd/nand/raw/ams-delta.c
> @@ -76,10 +76,8 @@ static void ams_delta_write_commit(struct ams_delta_nand *priv)
> gpiod_set_value(priv->gpiod_nwe, 1);
> }
>
> -static void ams_delta_write_next_byte(struct mtd_info *mtd, u_char byte)
> +static void ams_delta_write_next_byte(struct ams_delta_nand *priv, u_char byte)
> {
> - struct nand_chip *this = mtd_to_nand(mtd);
> - struct ams_delta_nand *priv = nand_get_controller_data(this);
> struct gpio_descs *data_gpiods = priv->data_gpiods;
> unsigned long bits = byte;
> int i, value_array[data_gpiods->ndescs];
> @@ -93,10 +91,8 @@ static void ams_delta_write_next_byte(struct mtd_info *mtd, u_char byte)
> ams_delta_write_commit(priv);
> }
>
> -static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
> +static void ams_delta_write_first_byte(struct ams_delta_nand *priv, u_char byte)
> {
> - struct nand_chip *this = mtd_to_nand(mtd);
> - struct ams_delta_nand *priv = nand_get_controller_data(this);
> struct gpio_descs *data_gpiods = priv->data_gpiods;
> unsigned long bits = byte;
> int i;
> @@ -108,10 +104,8 @@ static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
> ams_delta_write_commit(priv);
> }
>
> -static u_char ams_delta_read_next_byte(struct mtd_info *mtd)
> +static u_char ams_delta_read_next_byte(struct ams_delta_nand *priv)
> {
> - struct nand_chip *this = mtd_to_nand(mtd);
> - struct ams_delta_nand *priv = nand_get_controller_data(this);
> struct gpio_descs *data_gpiods = priv->data_gpiods;
> unsigned long bits = 0;
> int i, value_array[data_gpiods->ndescs];
> @@ -131,38 +125,48 @@ static u_char ams_delta_read_next_byte(struct mtd_info *mtd)
> return bits;
> }
>
> -static u_char ams_delta_read_byte(struct mtd_info *mtd)
> +static u_char ams_delta_read_first_byte(struct ams_delta_nand *priv)
> {
> - struct nand_chip *this = mtd_to_nand(mtd);
> - struct ams_delta_nand *priv = nand_get_controller_data(this);
> struct gpio_descs *data_gpiods = priv->data_gpiods;
> int i;
>
> for (i = 0; i < data_gpiods->ndescs; i++)
> gpiod_direction_input(data_gpiods->desc[i]);
>
> - return ams_delta_read_next_byte(mtd);
> + return ams_delta_read_next_byte(priv);
> +}
> +
> +static u_char ams_delta_read_byte(struct mtd_info *mtd)
> +{
> + struct nand_chip *this = mtd_to_nand(mtd);
> + struct ams_delta_nand *priv = nand_get_controller_data(this);
> +
> + return ams_delta_read_first_byte(priv);
> }
>
> static void ams_delta_write_buf(struct mtd_info *mtd, const u_char *buf,
> int len)
> {
> + struct nand_chip *this = mtd_to_nand(mtd);
> + struct ams_delta_nand *priv = nand_get_controller_data(this);
> int i;
>
> if (len > 0)
> - ams_delta_write_byte(mtd, buf[0]);
> + ams_delta_write_first_byte(priv, buf[0]);
> for (i = 1; i < len; i++)
> - ams_delta_write_next_byte(mtd, buf[i]);
> + ams_delta_write_next_byte(priv, buf[i]);
> }
>
> static void ams_delta_read_buf(struct mtd_info *mtd, u_char *buf, int len)
> {
> + struct nand_chip *this = mtd_to_nand(mtd);
> + struct ams_delta_nand *priv = nand_get_controller_data(this);
> int i;
>
> if (len > 0)
> - buf[0] = ams_delta_read_byte(mtd);
> + buf[0] = ams_delta_read_first_byte(priv);
> for (i = 1; i < len; i++)
> - buf[i] = ams_delta_read_next_byte(mtd);
> + buf[i] = ams_delta_read_next_byte(priv);
> }
>
> /*
> @@ -186,7 +190,7 @@ static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
> }
>
> if (cmd != NAND_CMD_NONE)
> - ams_delta_write_byte(mtd, cmd);
> + ams_delta_write_first_byte(priv, cmd);
> }
>
> static int ams_delta_nand_ready(struct mtd_info *mtd)
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 08/12] mtd: rawnand: ams-delta: Simplify pointer resolution on read/write
2018-08-07 17:02 ` Boris Brezillon
@ 2018-08-07 17:15 ` Janusz Krzysztofik
0 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-07 17:15 UTC (permalink / raw)
To: Boris Brezillon
Cc: Linus Walleij, Jonathan Corbet, Miquel Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, linux-arm-kernel, linux-omap,
linux-mtd, linux-doc, linux-gpio, linux-kernel
On Tuesday, August 7, 2018 7:02:56 PM CEST Boris Brezillon wrote:
> On Tue, 7 Aug 2018 00:29:14 +0200
> Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
>
> > Simplify data read/write sub-functions by changing their APIs so they
> > accept driver private structure pointer instead of mtd_info.
> >
> > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
>
> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
>
> Can you move that one earlier in the series so that it can be applied
> even if we're still discussing the GPIO bitmap changes?
Sure, I will, and I would be still more happy if you agreed on me doing the
same with [RFC PATCH v2 07/12] mtd: rawnand: ams-delta: Set port direction
once per transfer.
Thanks,
Janusz
^ permalink raw reply [flat|nested] 93+ messages in thread
* [RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct mapping
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
` (7 preceding siblings ...)
2018-08-06 22:29 ` [RFC PATCH v2 08/12] mtd: rawnand: ams-delta: Simplify pointer resolution on read/write Janusz Krzysztofik
@ 2018-08-06 22:29 ` Janusz Krzysztofik
2018-08-06 23:29 ` Linus Walleij
2018-08-07 17:14 ` Boris Brezillon
2018-08-06 22:29 ` [RFC PATCH v2 10/12] gpiolib: Introduce bitmap get/set array API extension Janusz Krzysztofik
` (3 subsequent siblings)
12 siblings, 2 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-06 22:29 UTC (permalink / raw)
To: Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
Certain GPIO array lookup results may map directly to GPIO pins of a
single GPIO chip in hardware order. If that condition is recognized
and handled efficiently, significant performance gain of get/set array
functions may be possible.
While processing a request for an array of GPIO descriptors, verify if
the descriptors just collected represent consecutive pins of a single
GPIO chip. Pass that information with the array to the caller so it
can benefit from enhanced performance as soon as bitmap based get/set
array functions which can make efficient use of that are available.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Documentation/driver-api/gpio/consumer.rst | 4 +++-
drivers/gpio/gpiolib.c | 14 ++++++++++++++
include/linux/gpio/consumer.h | 1 +
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index aa03f389d41d..38a990b5f3b6 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -109,11 +109,13 @@ For a function using multiple GPIOs all of those can be obtained with one call::
enum gpiod_flags flags)
This function returns a struct gpio_descs which contains an array of
-descriptors::
+descriptors. It may also contain a valid descriptor of a single GPIO chip in
+case the array strictly matches pin hardware layout of the chip::
struct gpio_descs {
unsigned int ndescs;
struct gpio_desc *desc[];
+ struct gpio_chip *chip;
}
The following function returns NULL instead of -ENOENT if no GPIOs have been
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index bdbfc95793e7..c50bcec6e2d7 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4161,6 +4161,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
{
struct gpio_desc *desc;
struct gpio_descs *descs;
+ struct gpio_chip *chip;
int count;
count = gpiod_count(dev, con_id);
@@ -4177,6 +4178,19 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
gpiod_put_array(descs);
return ERR_CAST(desc);
}
+
+ /*
+ * Verify if the array qualifies for fast bitmap operations
+ * (single chip, pins in hardware order starting from 0)
+ * and mark the array with the chip descriptor if true.
+ */
+ chip = gpiod_to_chip(desc);
+ if (descs->chip == NULL)
+ descs->chip = chip;
+ if (!IS_ERR(descs->chip) && (chip != descs->chip ||
+ gpio_chip_hwgpio(desc) != descs->ndescs))
+ descs->chip = ERR_PTR(-EINVAL);
+
descs->desc[descs->ndescs] = desc;
descs->ndescs++;
}
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 21ddbe440030..862ee027a02f 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -22,6 +22,7 @@ struct gpio_desc;
* gpiod_get_array().
*/
struct gpio_descs {
+ struct gpio_chip *chip;
unsigned int ndescs;
struct gpio_desc *desc[];
};
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct mapping
2018-08-06 22:29 ` [RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct mapping Janusz Krzysztofik
@ 2018-08-06 23:29 ` Linus Walleij
2018-08-07 16:50 ` Janusz Krzysztofik
2018-08-07 17:14 ` Boris Brezillon
1 sibling, 1 reply; 93+ messages in thread
From: Linus Walleij @ 2018-08-06 23:29 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Boris Brezillon, Jonathan Corbet, Miquèl Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Mark Vasut,
ext Tony Lindgren, Aaro Koskinen, Linux ARM, Linux-OMAP,
linux-mtd, linux-doc, open list:GPIO SUBSYSTEM, linux-kernel
Hi Janusz!
On Tue, Aug 7, 2018 at 12:29 AM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Certain GPIO array lookup results may map directly to GPIO pins of a
> single GPIO chip in hardware order. If that condition is recognized
> and handled efficiently, significant performance gain of get/set array
> functions may be possible.
>
> While processing a request for an array of GPIO descriptors, verify if
> the descriptors just collected represent consecutive pins of a single
> GPIO chip. Pass that information with the array to the caller so it
> can benefit from enhanced performance as soon as bitmap based get/set
> array functions which can make efficient use of that are available.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
(...)
> This function returns a struct gpio_descs which contains an array of
> -descriptors::
> +descriptors. It may also contain a valid descriptor of a single GPIO chip in
> +case the array strictly matches pin hardware layout of the chip::
>
> struct gpio_descs {
> unsigned int ndescs;
> struct gpio_desc *desc[];
> + struct gpio_chip *chip;
This must be motivated: if the only purpose is to indicate to the consumer that
all GPIOs are on the same chip, why not just have a
bool all_on_same_chip;
That you set to true if these are all on the same chip?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct mapping
2018-08-06 23:29 ` Linus Walleij
@ 2018-08-07 16:50 ` Janusz Krzysztofik
2018-08-07 17:10 ` Boris Brezillon
0 siblings, 1 reply; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-07 16:50 UTC (permalink / raw)
To: Linus Walleij
Cc: Boris Brezillon, Jonathan Corbet, Miquèl Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Mark Vasut,
ext Tony Lindgren, Aaro Koskinen, Linux ARM, Linux-OMAP,
linux-mtd, linux-doc, open list:GPIO SUBSYSTEM, linux-kernel,
Janusz Krzysztofik
Hi Linus,
On Tuesday, August 7, 2018 1:29:43 AM CEST Linus Walleij wrote:
> Hi Janusz!
>
> On Tue, Aug 7, 2018 at 12:29 AM Janusz Krzysztofik <jmkrzyszt@gmail.com>
wrote:
>
> > Certain GPIO array lookup results may map directly to GPIO pins of a
> > single GPIO chip in hardware order. If that condition is recognized
> > and handled efficiently, significant performance gain of get/set array
> > functions may be possible.
> >
> > While processing a request for an array of GPIO descriptors, verify if
> > the descriptors just collected represent consecutive pins of a single
> > GPIO chip. Pass that information with the array to the caller so it
> > can benefit from enhanced performance as soon as bitmap based get/set
> > array functions which can make efficient use of that are available.
> >
> > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> (...)
> > This function returns a struct gpio_descs which contains an array of
> > -descriptors::
> > +descriptors. It may also contain a valid descriptor of a single GPIO
chip in
> > +case the array strictly matches pin hardware layout of the chip::
> >
> > struct gpio_descs {
> > unsigned int ndescs;
> > struct gpio_desc *desc[];
> > + struct gpio_chip *chip;
>
> This must be motivated: if the only purpose is to indicate to the consumer
that
> all GPIOs are on the same chip, why not just have a
>
> bool all_on_same_chip;
>
> That you set to true if these are all on the same chip?
My approach would probably save one or two instructions per get/set call, but
I'm not stuck to it and will be happy to find a better solution.
How about folding the chip descriptor inside an additional structure, private
to drivers, with internals not revealed to consumers?
Thanks,
Janusz
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct mapping
2018-08-07 16:50 ` Janusz Krzysztofik
@ 2018-08-07 17:10 ` Boris Brezillon
0 siblings, 0 replies; 93+ messages in thread
From: Boris Brezillon @ 2018-08-07 17:10 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Linus Walleij, Jonathan Corbet, Miquèl Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Mark Vasut,
ext Tony Lindgren, Aaro Koskinen, Linux ARM, Linux-OMAP,
linux-mtd, linux-doc, open list:GPIO SUBSYSTEM, linux-kernel
On Tue, 07 Aug 2018 18:50:22 +0200
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Hi Linus,
>
> On Tuesday, August 7, 2018 1:29:43 AM CEST Linus Walleij wrote:
> > Hi Janusz!
> >
> > On Tue, Aug 7, 2018 at 12:29 AM Janusz Krzysztofik <jmkrzyszt@gmail.com>
> wrote:
> >
> > > Certain GPIO array lookup results may map directly to GPIO pins of a
> > > single GPIO chip in hardware order. If that condition is recognized
> > > and handled efficiently, significant performance gain of get/set array
> > > functions may be possible.
> > >
> > > While processing a request for an array of GPIO descriptors, verify if
> > > the descriptors just collected represent consecutive pins of a single
> > > GPIO chip. Pass that information with the array to the caller so it
> > > can benefit from enhanced performance as soon as bitmap based get/set
> > > array functions which can make efficient use of that are available.
> > >
> > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> > (...)
> > > This function returns a struct gpio_descs which contains an array of
> > > -descriptors::
> > > +descriptors. It may also contain a valid descriptor of a single GPIO
> chip in
> > > +case the array strictly matches pin hardware layout of the chip::
> > >
> > > struct gpio_descs {
> > > unsigned int ndescs;
> > > struct gpio_desc *desc[];
> > > + struct gpio_chip *chip;
> >
> > This must be motivated: if the only purpose is to indicate to the consumer
> that
> > all GPIOs are on the same chip, why not just have a
> >
> > bool all_on_same_chip;
> >
> > That you set to true if these are all on the same chip?
>
> My approach would probably save one or two instructions per get/set call, but
> I'm not stuck to it and will be happy to find a better solution.
>
> How about folding the chip descriptor inside an additional structure, private
> to drivers, with internals not revealed to consumers?
Or just get the chip from gpio_descs->desc[0]->gdev->chip when
->all_on_same_chip is true...
That adds 2 dereferencing though.
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct mapping
2018-08-06 22:29 ` [RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct mapping Janusz Krzysztofik
2018-08-06 23:29 ` Linus Walleij
@ 2018-08-07 17:14 ` Boris Brezillon
2018-08-07 17:19 ` Janusz Krzysztofik
1 sibling, 1 reply; 93+ messages in thread
From: Boris Brezillon @ 2018-08-07 17:14 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Linus Walleij, Jonathan Corbet, Miquel Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, linux-arm-kernel, linux-omap,
linux-mtd, linux-doc, linux-gpio, linux-kernel
On Tue, 7 Aug 2018 00:29:15 +0200
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Certain GPIO array lookup results may map directly to GPIO pins of a
> single GPIO chip in hardware order. If that condition is recognized
> and handled efficiently, significant performance gain of get/set array
> functions may be possible.
>
> While processing a request for an array of GPIO descriptors, verify if
> the descriptors just collected represent consecutive pins of a single
> GPIO chip. Pass that information with the array to the caller so it
> can benefit from enhanced performance as soon as bitmap based get/set
> array functions which can make efficient use of that are available.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> ---
> Documentation/driver-api/gpio/consumer.rst | 4 +++-
> drivers/gpio/gpiolib.c | 14 ++++++++++++++
> include/linux/gpio/consumer.h | 1 +
> 3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
> index aa03f389d41d..38a990b5f3b6 100644
> --- a/Documentation/driver-api/gpio/consumer.rst
> +++ b/Documentation/driver-api/gpio/consumer.rst
> @@ -109,11 +109,13 @@ For a function using multiple GPIOs all of those can be obtained with one call::
> enum gpiod_flags flags)
>
> This function returns a struct gpio_descs which contains an array of
> -descriptors::
> +descriptors. It may also contain a valid descriptor of a single GPIO chip in
> +case the array strictly matches pin hardware layout of the chip::
>
> struct gpio_descs {
> unsigned int ndescs;
> struct gpio_desc *desc[];
> + struct gpio_chip *chip;
chip is placed at the beginning of the struct in the real code, which
is expected since putting it at the end won't work because of the
desc[] declaration.
...
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index 21ddbe440030..862ee027a02f 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -22,6 +22,7 @@ struct gpio_desc;
> * gpiod_get_array().
> */
> struct gpio_descs {
> + struct gpio_chip *chip;
> unsigned int ndescs;
> struct gpio_desc *desc[];
> };
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct mapping
2018-08-07 17:14 ` Boris Brezillon
@ 2018-08-07 17:19 ` Janusz Krzysztofik
0 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-07 17:19 UTC (permalink / raw)
To: Boris Brezillon
Cc: Linus Walleij, Jonathan Corbet, Miquel Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, linux-arm-kernel, linux-omap,
linux-mtd, linux-doc, linux-gpio, linux-kernel,
Janusz Krzysztofik
On Tuesday, August 7, 2018 7:14:20 PM CEST Boris Brezillon wrote:
> On Tue, 7 Aug 2018 00:29:15 +0200
> Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
>
> > Certain GPIO array lookup results may map directly to GPIO pins of a
> > single GPIO chip in hardware order. If that condition is recognized
> > and handled efficiently, significant performance gain of get/set array
> > functions may be possible.
> >
> > While processing a request for an array of GPIO descriptors, verify if
> > the descriptors just collected represent consecutive pins of a single
> > GPIO chip. Pass that information with the array to the caller so it
> > can benefit from enhanced performance as soon as bitmap based get/set
> > array functions which can make efficient use of that are available.
> >
> > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> > ---
> > Documentation/driver-api/gpio/consumer.rst | 4 +++-
> > drivers/gpio/gpiolib.c | 14 ++++++++++++++
> > include/linux/gpio/consumer.h | 1 +
> > 3 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
> > index aa03f389d41d..38a990b5f3b6 100644
> > --- a/Documentation/driver-api/gpio/consumer.rst
> > +++ b/Documentation/driver-api/gpio/consumer.rst
> > @@ -109,11 +109,13 @@ For a function using multiple GPIOs all of those can be obtained with one call::
> > enum gpiod_flags flags)
> >
> > This function returns a struct gpio_descs which contains an array of
> > -descriptors::
> > +descriptors. It may also contain a valid descriptor of a single GPIO chip in
> > +case the array strictly matches pin hardware layout of the chip::
> >
> > struct gpio_descs {
> > unsigned int ndescs;
> > struct gpio_desc *desc[];
> > + struct gpio_chip *chip;
>
> chip is placed at the beginning of the struct in the real code, which
> is expected since putting it at the end won't work because of the
> desc[] declaration.
Yes, I've already noticed that and will fix on next iteration, thanks.
Janusz
^ permalink raw reply [flat|nested] 93+ messages in thread
* [RFC PATCH v2 10/12] gpiolib: Introduce bitmap get/set array API extension
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
` (8 preceding siblings ...)
2018-08-06 22:29 ` [RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct mapping Janusz Krzysztofik
@ 2018-08-06 22:29 ` Janusz Krzysztofik
2018-08-06 22:29 ` [RFC PATCH v2 11/12] mtd: rawnand: ams-delta: Use GPIO API bitmap extension Janusz Krzysztofik
` (2 subsequent siblings)
12 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-06 22:29 UTC (permalink / raw)
To: Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
Certain GPIO array lookups may return arrays marked as applicable for
fast get/set array operations. In order to make use of that
information, a new API extension which allows passing it to get/set
functions is needed.
Create a set of frontends to get/set array functions which accept
strict descriptor array structures returned by gpiod_get_array()
instead of arbitrary descriptor arrays.
Since the intended purpose of the new API extension is to speed up
get/set array operations, also replace array of integer values argument
with their bitmap representation, ready for being passed directly to
chip callback functions, without iterating them.
Applicability of the new API is limited to arrays not exceeding bit
length of type unsigned long (32 pins).
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Documentation/driver-api/gpio/consumer.rst | 26 ++++
drivers/gpio/gpiolib.c | 209 +++++++++++++++++++++++++++++
include/linux/gpio/consumer.h | 14 ++
3 files changed, 249 insertions(+)
diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index 38a990b5f3b6..bec4eab3b87c 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -383,6 +383,32 @@ or negative on error. Note the difference to gpiod_get_value(), which returns
0 or 1 on success to convey the GPIO value. With the array functions, the GPIO
values are stored in value_array rather than passed back as return value.
+Additionally, the following variants of the above functions exist which operate
+on bitmaps of values instead of arrays of values::
+
+ int gpiod_get_array_bitmap(struct gpio_descs *desc_array,
+ unsigned long *bits);
+ int gpiod_get_raw_array_bitmap(struct gpio_descs *desc_array,
+ unsigned long *bits);
+ int gpiod_get_array_bitmap_cansleep(struct gpio_desc *desc_array,
+ unsigned long *bits);
+ int gpiod_get_raw_array_bitmap_cansleep(struct gpio_desc **desc_array,
+ unsigned long *bits)
+
+ void gpiod_set_array_bitmap(struct gpio_descs *desc_array,
+ unsigned long *bits)
+ void gpiod_set_raw_array_bitmap(struct gpio_descs *desc_array,
+ unsigned long *bitmap)
+ void gpiod_set_array_bitmap_cansleep(struct gpio_descs *desc_array,
+ unsigned long *bits)
+ void gpiod_set_raw_array_bitmap_cansleep(struct gpio_descs *desc_array,
+ unsigned long *bits)
+
+Unlike their counterparts, these functions don't accept arbitrary GPIO
+descriptor arrays, only those of type struct gpio_descs returned by
+gpiod_get_array() and its variants. Supported array size is limited to the size
+of the bitmap, i.e., sizeof(unsigned long).
+
GPIOs mapped to IRQs
--------------------
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index c50bcec6e2d7..5b541364dee0 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2836,6 +2836,27 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
return 0;
}
+int gpiod_get_array_bitmap_complex(bool raw, bool can_sleep,
+ struct gpio_descs *array,
+ unsigned long *bits)
+{
+ int value_array[sizeof(*bits)];
+ int i;
+
+ if (array->ndescs > sizeof(*bits))
+ return -EINVAL;
+
+ i = gpiod_get_array_value_complex(raw, can_sleep, array->ndescs,
+ array->desc, value_array);
+ if (i)
+ return i;
+
+ for (i = 0; i < array->ndescs; i++)
+ __assign_bit(i, bits, value_array[i]);
+
+ return 0;
+}
+
/**
* gpiod_get_raw_value() - return a gpio's raw value
* @desc: gpio whose value will be returned
@@ -2929,6 +2950,50 @@ int gpiod_get_array_value(unsigned int array_size,
}
EXPORT_SYMBOL_GPL(gpiod_get_array_value);
+/**
+ * gpiod_get_raw_array_bitmap() - read raw values from an array of GPIOs
+ * @desc_array: array of GPIO descriptors whose values will be read, obtained
+ * with gpiod_get_array(),
+ * @bits: bitmap to store the read values
+ *
+ * Read the raw values of the GPIOs, i.e. the values of the physical lines
+ * without regard for their ACTIVE_LOW status. Return 0 in case of success,
+ * else an error code.
+ *
+ * This function should be called from contexts where we cannot sleep,
+ * and it will complain if the GPIO chip functions potentially sleep.
+ */
+int gpiod_get_raw_array_bitmap(struct gpio_descs *desc_array,
+ unsigned long *bits)
+{
+ if (!desc_array)
+ return -EINVAL;
+ return gpiod_get_array_bitmap_complex(true, false, desc_array, bits);
+}
+EXPORT_SYMBOL_GPL(gpiod_get_raw_array_bitmap);
+
+/**
+ * gpiod_get_array_bitmap() - read values from an array of GPIOs
+ * @desc_array: array of GPIO descriptors whose values will be read, obtained
+ * with gpiod_get_array(),
+ * @bits: bitmap to store the read values
+ *
+ * Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
+ * into account. Return 0 in case of success, else an error code.
+ *
+ * This function should be called from contexts where we cannot sleep,
+ * and it will complain if the GPIO chip functions potentially sleep.
+ */
+
+int gpiod_get_array_bitmap(struct gpio_descs *desc_array,
+ unsigned long *bits)
+{
+ if (!desc_array)
+ return -EINVAL;
+ return gpiod_get_array_bitmap_complex(false, false, desc_array, bits);
+}
+EXPORT_SYMBOL_GPL(gpiod_get_raw_array_bitmap);
+
/*
* gpio_set_open_drain_value_commit() - Set the open drain gpio's value.
* @desc: gpio descriptor whose state need to be set.
@@ -3081,6 +3146,23 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
return 0;
}
+int gpiod_set_array_bitmap_complex(bool raw, bool can_sleep,
+ struct gpio_descs *array,
+ unsigned long *bits)
+{
+ int value_array[sizeof(*bits)];
+ int i;
+
+ if (array->ndescs > sizeof(*bits))
+ return -EINVAL;
+
+ for (i = 0; i < array->ndescs; i++)
+ value_array[i] = test_bit(i, bits);
+
+ return gpiod_set_array_value_complex(raw, can_sleep, array->ndescs,
+ array->desc, value_array);
+}
+
/**
* gpiod_set_raw_value() - assign a gpio's raw value
* @desc: gpio whose value will be assigned
@@ -3185,6 +3267,48 @@ void gpiod_set_array_value(unsigned int array_size,
}
EXPORT_SYMBOL_GPL(gpiod_set_array_value);
+/**
+ * gpiod_set_raw_array_bitmap() - assign values to an array of GPIOs
+ * @desc_array: array of GPIO descriptors whose values will be assigned,
+ * obtained with gpiod_get_array(),
+ * @bits: bitmap of values to assign
+ *
+ * Set the raw values of the GPIOs, i.e. the values of the physical lines
+ * without regard for their ACTIVE_LOW status.
+ *
+ * This function should be called from contexts where we cannot sleep, and will
+ * complain if the GPIO chip functions potentially sleep.
+ */
+int gpiod_set_raw_array_bitmap(struct gpio_descs *desc_array,
+ unsigned long *bits)
+{
+ if (!desc_array)
+ return -EINVAL;
+ return gpiod_set_array_bitmap_complex(true, false, desc_array, bits);
+}
+EXPORT_SYMBOL_GPL(gpiod_set_raw_array_bitmap);
+
+/**
+ * gpiod_set_array_bitmap() - assign values to an array of GPIOs
+ * @array_size: number of elements in the descriptor / value arrays
+ * @desc_array: array of GPIO descriptors whose values will be assigned
+ * @bits: bitmap of values to assign
+ *
+ * Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
+ * into account.
+ *
+ * This function should be called from contexts where we cannot sleep, and will
+ * complain if the GPIO chip functions potentially sleep.
+ */
+void gpiod_set_array_bitmap(struct gpio_descs *desc_array,
+ unsigned long *bits)
+{
+ if (!desc_array)
+ return;
+ gpiod_set_array_bitmap_complex(false, false, desc_array, bits);
+}
+EXPORT_SYMBOL_GPL(gpiod_set_array_bitmap);
+
/**
* gpiod_cansleep() - report whether gpio value access may sleep
* @desc: gpio to check
@@ -3446,6 +3570,49 @@ int gpiod_get_array_value_cansleep(unsigned int array_size,
}
EXPORT_SYMBOL_GPL(gpiod_get_array_value_cansleep);
+/**
+ * gpiod_get_raw_array_bitmap_cansleep() - read raw values from array of GPIOs
+ * @desc_array: array of GPIO descriptors whose values will be read, obtained
+ * with gpiod_get_array(),
+ * @bits: bitmap to store the read values
+ *
+ * Read the raw values of the GPIOs, i.e. the values of the physical lines
+ * without regard for their ACTIVE_LOW status. Return 0 in case of success,
+ * else an error code.
+ *
+ * This function is to be called from contexts that can sleep.
+ */
+int gpiod_get_raw_array_bitmap_cansleep(struct gpio_descs *desc_array,
+ unsigned long *bits)
+{
+ might_sleep_if(extra_checks);
+ if (!desc_array)
+ return -EINVAL;
+ return gpiod_get_array_bitmap_complex(true, true, desc_array, bits);
+}
+EXPORT_SYMBOL_GPL(gpiod_get_raw_array_bitmap_cansleep);
+
+/**
+ * gpiod_get_array_bitmap_cansleep() - read values from an array of GPIOs
+ * @desc_array: array of GPIO descriptors whose values will be read, obtained
+ * with gpiod_get_array(),
+ * @bits: bitmap to store the read values
+ *
+ * Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
+ * into account. Return 0 in case of success, else an error code.
+ *
+ * This function is to be called from contexts that can sleep.
+ */
+int gpiod_get_array_bitmap_cansleep(struct gpio_descs *desc_array,
+ unsigned long *bits)
+{
+ might_sleep_if(extra_checks);
+ if (!desc_array)
+ return -EINVAL;
+ return gpiod_get_array_bitmap_complex(false, true, desc_array, bits);
+}
+EXPORT_SYMBOL_GPL(gpiod_get_array_bitmap_cansleep);
+
/**
* gpiod_set_raw_value_cansleep() - assign a gpio's raw value
* @desc: gpio whose value will be assigned
@@ -3545,6 +3712,48 @@ void gpiod_set_array_value_cansleep(unsigned int array_size,
}
EXPORT_SYMBOL_GPL(gpiod_set_array_value_cansleep);
+/**
+ * gpiod_set_raw_array_bitmap_cansleep() - assign values to an array of GPIOs
+ * @desc_array: array of GPIO descriptors whose values will be assigned,
+ * obtained with gpiod_get_array(),
+ * @bits: bitmap of values to assign
+ *
+ * Set the raw values of the GPIOs, i.e. the values of the physical lines
+ * without regard for their ACTIVE_LOW status.
+ *
+ * This function is to be called from contexts that can sleep.
+ */
+int gpiod_set_raw_array_bitmap_cansleep(struct gpio_descs *desc_array,
+ unsigned long *bits)
+{
+ might_sleep_if(extra_checks);
+ if (!desc_array)
+ return -EINVAL;
+ return gpiod_set_array_bitmap_complex(true, true, desc_array, bits);
+}
+EXPORT_SYMBOL_GPL(gpiod_set_raw_array_bitmap_cansleep);
+
+/**
+ * gpiod_set_array_bitmap_cansleep() - assign values to an array of GPIOs
+ * @desc_array: array of GPIO descriptors whose values will be assigned,
+ * obtained with gpiod_get_array(),
+ * @bits: bitmap of values to assign
+ *
+ * Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
+ * into account.
+ *
+ * This function is to be called from contexts that can sleep.
+ */
+void gpiod_set_array_bitmap_cansleep(struct gpio_descs *desc_array,
+ unsigned long *bits)
+{
+ might_sleep_if(extra_checks);
+ if (!desc_array)
+ return;
+ gpiod_set_array_bitmap_complex(false, true, desc_array, bits);
+}
+EXPORT_SYMBOL_GPL(gpiod_set_array_bitmap_cansleep);
+
/**
* gpiod_add_lookup_table() - register GPIO device consumers
* @table: table of consumers to register
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 862ee027a02f..1eabce4fc6c5 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -106,35 +106,49 @@ int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
int gpiod_get_value(const struct gpio_desc *desc);
int gpiod_get_array_value(unsigned int array_size,
struct gpio_desc **desc_array, int *value_array);
+int gpiod_get_array_bitmap(struct gpio_descs *desc_array, unsigned long *bits);
void gpiod_set_value(struct gpio_desc *desc, int value);
void gpiod_set_array_value(unsigned int array_size,
struct gpio_desc **desc_array, int *value_array);
+void gpiod_set_array_bitmap(struct gpio_descs *desc_array, unsigned long *bits);
int gpiod_get_raw_value(const struct gpio_desc *desc);
int gpiod_get_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
int *value_array);
+int gpiod_get_raw_array_bitmap(struct gpio_descs *desc_array,
+ unsigned long *bits);
void gpiod_set_raw_value(struct gpio_desc *desc, int value);
int gpiod_set_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
int *value_array);
+int gpiod_set_raw_array_bitmap(struct gpio_descs *desc_array,
+ unsigned long *bits);
/* Value get/set from sleeping context */
int gpiod_get_value_cansleep(const struct gpio_desc *desc);
int gpiod_get_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
int *value_array);
+int gpiod_get_array_bitmap_cansleep(struct gpio_descs *desc_array,
+ unsigned long *bits);
void gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
void gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
int *value_array);
+void gpiod_set_array_bitmap_cansleep(struct gpio_descs *desc_array,
+ unsigned long *bits);
int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc);
int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
int *value_array);
+int gpiod_get_raw_array_bitmap_cansleep(struct gpio_descs *desc_array,
+ unsigned long *bits);
void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value);
int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
int *value_array);
+int gpiod_set_raw_array_bitmap_cansleep(struct gpio_descs *desc_array,
+ unsigned long *bits);
int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
int gpiod_set_transitory(struct gpio_desc *desc, bool transitory);
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* [RFC PATCH v2 11/12] mtd: rawnand: ams-delta: Use GPIO API bitmap extension
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
` (9 preceding siblings ...)
2018-08-06 22:29 ` [RFC PATCH v2 10/12] gpiolib: Introduce bitmap get/set array API extension Janusz Krzysztofik
@ 2018-08-06 22:29 ` Janusz Krzysztofik
2018-08-06 22:29 ` [RFC PATCH v2 12/12] gpiolib: Add fast processing path to bitmap API functions Janusz Krzysztofik
2018-08-13 22:34 ` [PATCH v3 0/7] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
12 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-06 22:29 UTC (permalink / raw)
To: Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
Try to address the driver performance issues by replacing traditional
get/set array function calls with their bitmap based equivalents.
As long as fast bitmap processing path is not implemented in the new
API extension, performance of the driver remains unchanged.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
drivers/mtd/nand/raw/ams-delta.c | 19 +++----------------
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 30c461138195..7b08b2c441d3 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -78,15 +78,9 @@ static void ams_delta_write_commit(struct ams_delta_nand *priv)
static void ams_delta_write_next_byte(struct ams_delta_nand *priv, u_char byte)
{
- struct gpio_descs *data_gpiods = priv->data_gpiods;
unsigned long bits = byte;
- int i, value_array[data_gpiods->ndescs];
-
- for (i = 0; i < data_gpiods->ndescs; i++)
- value_array[i] = test_bit(i, &bits);
- gpiod_set_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
- value_array);
+ gpiod_set_raw_array_bitmap(priv->data_gpiods, &bits);
ams_delta_write_commit(priv);
}
@@ -106,22 +100,15 @@ static void ams_delta_write_first_byte(struct ams_delta_nand *priv, u_char byte)
static u_char ams_delta_read_next_byte(struct ams_delta_nand *priv)
{
- struct gpio_descs *data_gpiods = priv->data_gpiods;
- unsigned long bits = 0;
- int i, value_array[data_gpiods->ndescs];
+ unsigned long bits;
gpiod_set_value(priv->gpiod_nre, 0);
ndelay(40);
- gpiod_get_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
- value_array);
+ gpiod_get_raw_array_bitmap(priv->data_gpiods, &bits);
gpiod_set_value(priv->gpiod_nre, 1);
- for (i = 0; i < data_gpiods->ndescs; i++)
- if (value_array[i])
- __set_bit(i, &bits);
-
return bits;
}
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* [RFC PATCH v2 12/12] gpiolib: Add fast processing path to bitmap API functions
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
` (10 preceding siblings ...)
2018-08-06 22:29 ` [RFC PATCH v2 11/12] mtd: rawnand: ams-delta: Use GPIO API bitmap extension Janusz Krzysztofik
@ 2018-08-06 22:29 ` Janusz Krzysztofik
2018-08-06 23:43 ` Linus Walleij
2018-08-13 22:34 ` [PATCH v3 0/7] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
12 siblings, 1 reply; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-06 22:29 UTC (permalink / raw)
To: Boris Brezillon, Linus Walleij
Cc: Jonathan Corbet, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Tony Lindgren,
Aaro Koskinen, linux-arm-kernel, linux-omap, linux-mtd,
linux-doc, linux-gpio, linux-kernel, Janusz Krzysztofik
Certain GPIO descriptor arrays returned by gpio_get_array() may contain
information on a single GPIO chip driving array member pins in hardware
order. In such cases, bitmaps of values can be passed directly to the
chip callback functions without wasting time on iterations.
Add respective code to gpiod_get/set_array_bitmap_complex() functions.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Documentation/driver-api/gpio/consumer.rst | 6 ++++++
drivers/gpio/gpiolib.c | 14 ++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index bec4eab3b87c..b82f134dc352 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -409,6 +409,12 @@ descriptor arrays, only those of type struct gpio_descs returned by
gpiod_get_array() and its variants. Supported array size is limited to the size
of the bitmap, i.e., sizeof(unsigned long).
+If the .chip member of the array structure, filled in by gpiod_get_array() in
+certain circumstances, contains a valid GPIO chip descriptor, the raw variants
+of the functions can take fast processing paths, passing bitmap arguments
+directly to the chip callback functions. That allows for utilization of GPIO
+banks as data I/O ports without much loss of performance.
+
GPIOs mapped to IRQs
--------------------
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5b541364dee0..bf95f2964bc5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2846,6 +2846,12 @@ int gpiod_get_array_bitmap_complex(bool raw, bool can_sleep,
if (array->ndescs > sizeof(*bits))
return -EINVAL;
+ if (raw && !IS_ERR_OR_NULL(array->chip)) {
+ unsigned long mask = (1ULL << array->ndescs) - 1;
+
+ return gpio_chip_get_multiple(array->chip, &mask, bits);
+ }
+
i = gpiod_get_array_value_complex(raw, can_sleep, array->ndescs,
array->desc, value_array);
if (i)
@@ -3156,6 +3162,14 @@ int gpiod_set_array_bitmap_complex(bool raw, bool can_sleep,
if (array->ndescs > sizeof(*bits))
return -EINVAL;
+ if (raw && !IS_ERR_OR_NULL(array->chip)) {
+ unsigned long mask = (1ULL << array->ndescs) - 1;
+
+ gpio_chip_set_multiple(array->chip, &mask, bits);
+
+ return 0;
+ }
+
for (i = 0; i < array->ndescs; i++)
value_array[i] = test_bit(i, bits);
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 12/12] gpiolib: Add fast processing path to bitmap API functions
2018-08-06 22:29 ` [RFC PATCH v2 12/12] gpiolib: Add fast processing path to bitmap API functions Janusz Krzysztofik
@ 2018-08-06 23:43 ` Linus Walleij
2018-08-07 17:29 ` Janusz Krzysztofik
0 siblings, 1 reply; 93+ messages in thread
From: Linus Walleij @ 2018-08-06 23:43 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Boris Brezillon, Jonathan Corbet, Miquèl Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Mark Vasut,
ext Tony Lindgren, Aaro Koskinen, Linux ARM, Linux-OMAP,
linux-mtd, linux-doc, open list:GPIO SUBSYSTEM, linux-kernel
On Tue, Aug 7, 2018 at 12:29 AM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
Hi Janusz!
> Certain GPIO descriptor arrays returned by gpio_get_array() may contain
> information on a single GPIO chip driving array member pins in hardware
> order. In such cases, bitmaps of values can be passed directly to the
> chip callback functions without wasting time on iterations.
>
> Add respective code to gpiod_get/set_array_bitmap_complex() functions.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
I think it would be disappointing to leave all the users of the old
array API without the performance improvement. I think we need to
deal with this in a way such that everyone can benefit from it.
Also it is kludgy if users (consumers) would need to handle the case
where all lines are on the same chip separately, through the bitmap
API.
What we need is an API that:
- All drivers handling arrays can use (including current users).
- Enables speed-up if the lines are all on the same chip/register.
- Doesn't require consumers to know if they are all on the same
chip or not.
This means a deep API with a small surface.
How do we achieve this the best way?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 12/12] gpiolib: Add fast processing path to bitmap API functions
2018-08-06 23:43 ` Linus Walleij
@ 2018-08-07 17:29 ` Janusz Krzysztofik
2018-08-07 17:47 ` Boris Brezillon
0 siblings, 1 reply; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-07 17:29 UTC (permalink / raw)
To: Linus Walleij
Cc: Boris Brezillon, Jonathan Corbet, Miquèl Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Mark Vasut,
ext Tony Lindgren, Aaro Koskinen, Linux ARM, Linux-OMAP,
linux-mtd, linux-doc, open list:GPIO SUBSYSTEM, linux-kernel,
Janusz Krzysztofik
On Tuesday, August 7, 2018 1:43:56 AM CEST Linus Walleij wrote:
> On Tue, Aug 7, 2018 at 12:29 AM Janusz Krzysztofik <jmkrzyszt@gmail.com>
wrote:
>
> Hi Janusz!
>
> > Certain GPIO descriptor arrays returned by gpio_get_array() may contain
> > information on a single GPIO chip driving array member pins in hardware
> > order. In such cases, bitmaps of values can be passed directly to the
> > chip callback functions without wasting time on iterations.
> >
> > Add respective code to gpiod_get/set_array_bitmap_complex() functions.
> >
> > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
>
> I think it would be disappointing to leave all the users of the old
> array API without the performance improvement. I think we need to
> deal with this in a way such that everyone can benefit from it.
There are a few issues to be resolved:
1) array size limited by bitmap size:
- are we ready to limit array size to a single bitmap for all users?
- if not, how can we pass a bitmap of an arbitrary size?
- if as an array of bitmaps, is that still clear enough and easy to use?
- other ideas?
2) arbitrary array support:
- are we ready to drop that?
- if not, do we agree to require all users to pack their arbitrary arrays
inside the gpio_descs structure?
Maybe more.
> Also it is kludgy if users (consumers) would need to handle the case
> where all lines are on the same chip separately, through the bitmap
> API.
Not true as long as array size fits (arbitrary arrays can be packed by users),
but I see your point.
> What we need is an API that:
>
> - All drivers handling arrays can use (including current users).
>
> - Enables speed-up if the lines are all on the same chip/register.
>
> - Doesn't require consumers to know if they are all on the same
> chip or not.
>
> This means a deep API with a small surface.
>
> How do we achieve this the best way?
I think widely accepted solutions to those two issues I've mentioned above can
give the answer.
Thanks,
Janusz
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 12/12] gpiolib: Add fast processing path to bitmap API functions
2018-08-07 17:29 ` Janusz Krzysztofik
@ 2018-08-07 17:47 ` Boris Brezillon
2018-08-10 10:55 ` Linus Walleij
0 siblings, 1 reply; 93+ messages in thread
From: Boris Brezillon @ 2018-08-07 17:47 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Linus Walleij, Jonathan Corbet, Miquèl Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Mark Vasut,
ext Tony Lindgren, Aaro Koskinen, Linux ARM, Linux-OMAP,
linux-mtd, linux-doc, open list:GPIO SUBSYSTEM, linux-kernel
Hi Janusz,
On Tue, 07 Aug 2018 19:29:53 +0200
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> On Tuesday, August 7, 2018 1:43:56 AM CEST Linus Walleij wrote:
> > On Tue, Aug 7, 2018 at 12:29 AM Janusz Krzysztofik <jmkrzyszt@gmail.com>
> wrote:
> >
> > Hi Janusz!
> >
> > > Certain GPIO descriptor arrays returned by gpio_get_array() may contain
> > > information on a single GPIO chip driving array member pins in hardware
> > > order. In such cases, bitmaps of values can be passed directly to the
> > > chip callback functions without wasting time on iterations.
> > >
> > > Add respective code to gpiod_get/set_array_bitmap_complex() functions.
> > >
> > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> >
> > I think it would be disappointing to leave all the users of the old
> > array API without the performance improvement. I think we need to
> > deal with this in a way such that everyone can benefit from it.
I agree with Linus on that one. When I initially proposed the gpio
bitbanging API I had something more advanced in mind where the GPIO
framework would be responsible for toggling the GPIOs on its own when
it's given an array of bytes to transmit (this way you avoid going
back and forth between the GPIO user and the GPIO framework). But this
approach would clearly be more invasive than what you propose
here (turning the int array into a bitmap and optimizing). So, if we go
for the "int array -> bitmap" approach I think all users should be
converted so that we end up with a single API.
>
> There are a few issues to be resolved:
>
> 1) array size limited by bitmap size:
> - are we ready to limit array size to a single bitmap for all users?
> - if not, how can we pass a bitmap of an arbitrary size?
> - if as an array of bitmaps, is that still clear enough and easy to use?
> - other ideas?
What we call a bitmap is an array of unsigned longs each entry
containing NBITS_PER_LONG bits, so yes, it's an arbitrary size (see the
bitmap API here [1]).
>
> 2) arbitrary array support:
> - are we ready to drop that?
> - if not, do we agree to require all users to pack their arbitrary arrays
> inside the gpio_descs structure?
I could only find one user, and it's the core itself (for the ioctl),
so that shouldn't be too hard to convert all users. Did you find more.
>
> Maybe more.
>
> > Also it is kludgy if users (consumers) would need to handle the case
> > where all lines are on the same chip separately, through the bitmap
> > API.
>
> Not true as long as array size fits (arbitrary arrays can be packed by users),
> but I see your point.
I think the API should be the same and the framework should decide to
take the fast path if all gpios belong to the same chip (which AFAICT
is already the case, except it's putting the result in an int array
instead of a bitmap)
>
> > What we need is an API that:
> >
> > - All drivers handling arrays can use (including current users).
> >
> > - Enables speed-up if the lines are all on the same chip/register.
> >
> > - Doesn't require consumers to know if they are all on the same
> > chip or not.
> >
> > This means a deep API with a small surface.
> >
> > How do we achieve this the best way?
>
> I think widely accepted solutions to those two issues I've mentioned above can
> give the answer.
I'd still like to see how far we are from the initial perfs (the one
poking the GPIO regs directly) with this approach, and what's the
improvement compared to the int array solution we already have in place.
Regards,
Boris
[1]https://elixir.bootlin.com/linux/v4.18-rc8/source/include/linux/bitmap.h
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: [RFC PATCH v2 12/12] gpiolib: Add fast processing path to bitmap API functions
2018-08-07 17:47 ` Boris Brezillon
@ 2018-08-10 10:55 ` Linus Walleij
0 siblings, 0 replies; 93+ messages in thread
From: Linus Walleij @ 2018-08-10 10:55 UTC (permalink / raw)
To: Boris Brezillon
Cc: Janusz Krzysztofik, Jonathan Corbet, Miquèl Raynal,
Richard Weinberger, David Woodhouse, Brian Norris, Mark Vasut,
ext Tony Lindgren, Aaro Koskinen, Linux ARM, Linux-OMAP,
linux-mtd, linux-doc, open list:GPIO SUBSYSTEM, linux-kernel
On Tue, Aug 7, 2018 at 7:47 PM Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
>
> > On Tuesday, August 7, 2018 1:43:56 AM CEST Linus Walleij wrote:
> > > On Tue, Aug 7, 2018 at 12:29 AM Janusz Krzysztofik <jmkrzyszt@gmail.com>
> > wrote:
> > >
> > > Hi Janusz!
> > >
> > > > Certain GPIO descriptor arrays returned by gpio_get_array() may contain
> > > > information on a single GPIO chip driving array member pins in hardware
> > > > order. In such cases, bitmaps of values can be passed directly to the
> > > > chip callback functions without wasting time on iterations.
> > > >
> > > > Add respective code to gpiod_get/set_array_bitmap_complex() functions.
> > > >
> > > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> > >
> > > I think it would be disappointing to leave all the users of the old
> > > array API without the performance improvement. I think we need to
> > > deal with this in a way such that everyone can benefit from it.
>
> I agree with Linus on that one. When I initially proposed the gpio
> bitbanging API I had something more advanced in mind where the GPIO
> framework would be responsible for toggling the GPIOs on its own when
> it's given an array of bytes to transmit (this way you avoid going
> back and forth between the GPIO user and the GPIO framework). But this
> approach would clearly be more invasive than what you propose
> here (turning the int array into a bitmap and optimizing). So, if we go
> for the "int array -> bitmap" approach I think all users should be
> converted so that we end up with a single API.
I thought about this the recent days and something must have gone
wrong in the development process of the array API because this
was the (mine atleast) intention all the time.
If we look at the GPIOchip driver API it looks like this:
int (*get_multiple)(struct gpio_chip *chip,
unsigned long *mask,
unsigned long *bits);
void (*set_multiple)(struct gpio_chip *chip,
unsigned long *mask,
unsigned long *bits);
So there is nothing hindering the drivers from optimizing a call
here into a single register write, which is what e.g. the gpio-mmio.c
driver does: if the hardware has a dedicated register for clearing
and setting lines, it will simply just write the register with
whatever is passed in, also cache the current value so it doesn't
need to read back the register every time.
When an array comes down to gpiod_set_array_value_complex()
it loops over the descriptors in order to handle e.g. open drain
settings separately. Then the remainder (lines that should just
be set 1/0) is pushed to the .set_multiple() callback if they
are on the same chip.
This is assuming:
1. The CPU is not the bottleneck so we can do a bit
of complex looping over structs etc in each write.
2. We want to perform as much in a single register write
as possible to avoid I/O and glitches as all lines (e.g.
clock and data) get written at the same time, if possible.
(No skew.)
It seems Janusz has problems with assumption (1) and therefore
is trying to optimize the read/write path. This can be done if all
descs are on the same chip and none of them is using open drain
or open source.
To keep track of "quick path" the array needs to have a state.
So a magic "cookie" or something like that needs to be passed
to the array API.
I would suggest that struct gpiod_descs contain a magic cookie
returned from [devm_]gpiod_get_array[_optional]() that can be
passed along to get/set array operations or left as NULL to just
fall back to the default:
void gpiod_set_array_value(unsigned int array_size,
struct gpio_desc **desc_array, int *value_array,
struct gpiod_array_cookie *cookie);
Cookie is just a dummy name, I don't know what makes most
sense. It reflects a state for the entire array.
If this cookie exist in some struct gpio_chip state variable, it
informs gpiolib that this array:
- Has all descriptors in the same gpiochip
- Has no open drain or open source-flagged lines
It can thenbypass the complex check and just write the values
directly by calling down into .set_multiple().
Maybe this cookie could just be a bool that is true when the above
is fulfilled. But it's best if that is hidden from the consumers
I guess, they shouldn't try to half-guess if the criteria is true,
gpiolib should do that.
The current users would have to be augmented to store the
cookie and pass it along when getting/setting arrays, but it would
be pretty simple and straight-forward compared to adding a new
API.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 93+ messages in thread
* [PATCH v3 0/7] mtd: rawnand: ams-delta: Use GPIO API for data I/O
2018-08-06 22:29 ` [RFC PATCH v2] mtd: rawnand: ams-delta: Use GPIO API " Janusz Krzysztofik
` (11 preceding siblings ...)
2018-08-06 22:29 ` [RFC PATCH v2 12/12] gpiolib: Add fast processing path to bitmap API functions Janusz Krzysztofik
@ 2018-08-13 22:34 ` Janusz Krzysztofik
2018-08-13 22:34 ` [PATCH v3 1/7] mtd: rawnand: ams-delta: show parent device in sysfs Janusz Krzysztofik
` (7 more replies)
12 siblings, 8 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-13 22:34 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, Linus Walleij, linux-arm-kernel,
linux-omap, linux-mtd, linux-gpio, linux-kernel,
Janusz Krzysztofik
Implement the idea suggested by Artem Bityutskiy and Tony Lindgren,
described in commit b027274d2e3a ("mtd: ams-delta: fix
request_mem_region() failure"). Use pure GPIO API as suggested by
Boris Brezillon.
Janusz Krzysztofik (7):
mtd: rawnand: ams-delta: show parent device in sysfs
mtd: rawnand: ams-delta: Use private structure
ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
mtd: rawnand: ams-delta: request data port GPIO resource
mtd: rawnand: ams-delta: Set port direction when needed
mtd: rawnand: ams-delta: Simplify pointer resolution on read/write
mtd: rawnand: ams-delta: use GPIO API for data I/O
Changelog:
v3:
[PATCH v3 1/7] mtd: rawnand: ams-delta: show parent device in sysfs
- renamed and an explanation added based on other similar patches on
Marek Vasut request, thanks.
[PATCH v3 2/7] mtd: rawnand: ams-delta: Use private structure
- no changes.
[PATCH v3 3/7] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND
data port
- no changes.
[PATCH v3 4/7] mtd: rawnand: ams-delta: request data port GPIO resource
- no changes.
[PATCH v3 5/7] mtd: rawnand: ams-delta: Set port direction when needed
- modified to set port direction only when needed instead of on each
transfer as suggested by Boris, thanks, though I kept separate
*_next_byte() functions to maximize performance as much as possible,
- moved back in front of "mtd: rawnand: ams-delta: use GPIO API for data
I/O" with a comment added referring to the planned switch to GPIO API.
[PATCH v3 6/7] mtd: rawnand: ams-delta: Simplify pointer resolution
- moved back in front of "mtd: rawnand: ams-delta: use GPIO API for data
I/O" on Boris request, thanks.
[RFC PATCH v3 7/7] mtd: rawnand: ams-delta: use GPIO API for data I/O
- rebased back on top of the two mentioned above,
- not intended to apply it yet due to performance issues on Amstrad Delta.
Removed from the series:
[RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct
mapping
[RFC PATCH v2 10/12] gpiolib: Introduce bitmap get/set array API extension
[RFC PATCH v2 11/12] mtd: rawnand: ams-delta: Use GPIO API bitmap extension
[RFC PATCH v2 12/12] gpiolib: Add fast processing path to bitmap API functions
- intended to be still iterated in a follow up series until performance
issues are resolved.
[RFC PATCH v2 06/12] ARM: OMAP1: ams-delta: drop obsolete NAND resources
- postponed until acceptable performance on Amstrad Delta is achieved.
v2:
[RFC PATCH v2 00/12] mtd: rawnand: ams-delta: Use GPIO API for data I/O
- renamed from former [RFC PATCH 0/8] mtd: rawnand: ams-delta: Use
gpio-omap accessors for data I/O
[RFC PATCH v2 01/12] mtd: rawnand: ams-delta: Assign mtd->dev.parent,
not mtd->owner
- split out from former [RFC PATCH 1/8] on Boris request, thanks.
[RFC PATCH v2 02/12] mtd: rawnand: ams-delta: Use private structure
- remaining part of the former [RFC PATCH 1/8].
[RFC PATCH v2 03/12] ARM: OMAP1: ams-delta: Provide GPIO lookup table
for NAND data port
- split out from former [RFC PATCH 5/8] on Boris requesst, thanks,
[RFC PATCH v2 04/12] mtd: rawnand: ams-delta: request data port GPIO resource
- remaining part of the former [RFC PATCH 5/8],
[RFC PATCH v2 05/12] mtd: rawnand: ams-delta: use GPIO API for data read/write
- reworked from former [RFC PATCH 8/8] on Boris requesst to use pure
GPIO API, thanks,
- moved up in front of former [RFC PATCH 3/8] on Boris request, thanks.
[RFC PATCH v2 06/12] ARM: OMAP1: ams-delta: drop obsolete NAND resources
- split out from former [RFC PATCH 8/8].
[RFC PATCH v2 07/12] mtd: rawnand: ams-delta: Set port direction once per
transfer
- reworked from former [RFC PATCH 3/8] on top of [RFC PATCH v2 05/12].
[RFC PATCH v2 08/12] mtd: rawnand: ams-delta: Simplify pointer resolution
on read/write
- reworked from former [RFC PATCH 4/8] on top of [RFC PATCH v2 08/12],
- renamed from 'Optimize' to 'Simplify' on Boris request, thanks.
[RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct
mapping
- new patch.
[RFC PATCH v2 10/12] gpiolib: Introduce bitmap get/set array API extension
- new patch.
[RFC PATCH v2 11/12] mtd: rawnand: ams-delta: Use GPIO API bitmap extension
- new patch.
[RFC PATCH v2 12/12] gpiolib: Add fast processing path to bitmap API functions
- new patch.
Removed from the series:
[RFC PATCH 2/8] mtd: rawnand: ams-delta: Write protect device during probe
- postponed on Boris request, thanks.
[RFC PATCH 6/8] gpio: omap: Add get/set_multiple() callbacks
- already applied by Linux in linux-gpio tree, thanks.
[RFC PATCH 7/8] mtd: rawnand: ams-delta: Check sanity of data GPIO resource
- most controversial one, no longer needed after switching to GPIO API.
diffstat
arch/arm/mach-omap1/board-ams-delta.c | 11 -
drivers/mtd/nand/raw/ams-delta.c | 351 +++++++++++++++++++---------------
2 files changed, 209 insertions(+), 153 deletions(-)
Thanks,
Janusz
^ permalink raw reply [flat|nested] 93+ messages in thread
* [PATCH v3 1/7] mtd: rawnand: ams-delta: show parent device in sysfs
2018-08-13 22:34 ` [PATCH v3 0/7] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
@ 2018-08-13 22:34 ` Janusz Krzysztofik
2018-08-13 22:34 ` [PATCH v3 2/7] mtd: rawnand: ams-delta: Use private structure Janusz Krzysztofik
` (6 subsequent siblings)
7 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-13 22:34 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, Linus Walleij, linux-arm-kernel,
linux-omap, linux-mtd, linux-gpio, linux-kernel,
Janusz Krzysztofik
Fix a bug where parent device symlinks aren't shown in sysfs.
While at it, make use of the default owner set by mtdcore.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
drivers/mtd/nand/raw/ams-delta.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 2a8872ebd14a..af313c620264 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -162,7 +162,7 @@ static int ams_delta_init(struct platform_device *pdev)
}
ams_delta_mtd = nand_to_mtd(this);
- ams_delta_mtd->owner = THIS_MODULE;
+ ams_delta_mtd->dev.parent = &pdev->dev;
/*
* Don't try to request the memory region from here,
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* [PATCH v3 2/7] mtd: rawnand: ams-delta: Use private structure
2018-08-13 22:34 ` [PATCH v3 0/7] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
2018-08-13 22:34 ` [PATCH v3 1/7] mtd: rawnand: ams-delta: show parent device in sysfs Janusz Krzysztofik
@ 2018-08-13 22:34 ` Janusz Krzysztofik
2018-08-13 22:34 ` [PATCH v3 3/7] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port Janusz Krzysztofik
` (5 subsequent siblings)
7 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-13 22:34 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, Linus Walleij, linux-arm-kernel,
linux-omap, linux-mtd, linux-gpio, linux-kernel,
Janusz Krzysztofik
Introduce a driver private structure and allocate it on device probe.
Use it for storing nand_chip structure, GPIO descriptors prevoiusly
stored in static variables as well as io_base pointer previously passed
as nand controller data or platform driver data. Subsequent patches
may populate the structure with more members as needed.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
drivers/mtd/nand/raw/ams-delta.c | 126 +++++++++++++++++++++------------------
1 file changed, 69 insertions(+), 57 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index af313c620264..48233d638d2a 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -34,14 +34,18 @@
/*
* MTD structure for E3 (Delta)
*/
-static struct mtd_info *ams_delta_mtd = NULL;
-static struct gpio_desc *gpiod_rdy;
-static struct gpio_desc *gpiod_nce;
-static struct gpio_desc *gpiod_nre;
-static struct gpio_desc *gpiod_nwp;
-static struct gpio_desc *gpiod_nwe;
-static struct gpio_desc *gpiod_ale;
-static struct gpio_desc *gpiod_cle;
+
+struct ams_delta_nand {
+ struct nand_chip nand_chip;
+ struct gpio_desc *gpiod_rdy;
+ struct gpio_desc *gpiod_nce;
+ struct gpio_desc *gpiod_nre;
+ struct gpio_desc *gpiod_nwp;
+ struct gpio_desc *gpiod_nwe;
+ struct gpio_desc *gpiod_ale;
+ struct gpio_desc *gpiod_cle;
+ void __iomem *io_base;
+};
/*
* Define partitions for flash devices
@@ -71,26 +75,28 @@ static const struct mtd_partition partition_info[] = {
static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
{
struct nand_chip *this = mtd_to_nand(mtd);
- void __iomem *io_base = (void __iomem *)nand_get_controller_data(this);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
+ void __iomem *io_base = priv->io_base;
writew(0, io_base + OMAP_MPUIO_IO_CNTL);
writew(byte, this->IO_ADDR_W);
- gpiod_set_value(gpiod_nwe, 0);
+ gpiod_set_value(priv->gpiod_nwe, 0);
ndelay(40);
- gpiod_set_value(gpiod_nwe, 1);
+ gpiod_set_value(priv->gpiod_nwe, 1);
}
static u_char ams_delta_read_byte(struct mtd_info *mtd)
{
u_char res;
struct nand_chip *this = mtd_to_nand(mtd);
- void __iomem *io_base = (void __iomem *)nand_get_controller_data(this);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
+ void __iomem *io_base = priv->io_base;
- gpiod_set_value(gpiod_nre, 0);
+ gpiod_set_value(priv->gpiod_nre, 0);
ndelay(40);
writew(~0, io_base + OMAP_MPUIO_IO_CNTL);
res = readw(this->IO_ADDR_R);
- gpiod_set_value(gpiod_nre, 1);
+ gpiod_set_value(priv->gpiod_nre, 1);
return res;
}
@@ -123,11 +129,13 @@ static void ams_delta_read_buf(struct mtd_info *mtd, u_char *buf, int len)
static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
unsigned int ctrl)
{
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
if (ctrl & NAND_CTRL_CHANGE) {
- gpiod_set_value(gpiod_nce, !(ctrl & NAND_NCE));
- gpiod_set_value(gpiod_cle, !!(ctrl & NAND_CLE));
- gpiod_set_value(gpiod_ale, !!(ctrl & NAND_ALE));
+ gpiod_set_value(priv->gpiod_nce, !(ctrl & NAND_NCE));
+ gpiod_set_value(priv->gpiod_cle, !!(ctrl & NAND_CLE));
+ gpiod_set_value(priv->gpiod_ale, !!(ctrl & NAND_ALE));
}
if (cmd != NAND_CMD_NONE)
@@ -136,7 +144,10 @@ static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
static int ams_delta_nand_ready(struct mtd_info *mtd)
{
- return gpiod_get_value(gpiod_rdy);
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
+
+ return gpiod_get_value(priv->gpiod_rdy);
}
@@ -145,7 +156,9 @@ static int ams_delta_nand_ready(struct mtd_info *mtd)
*/
static int ams_delta_init(struct platform_device *pdev)
{
+ struct ams_delta_nand *priv;
struct nand_chip *this;
+ struct mtd_info *mtd;
struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
void __iomem *io_base;
int err = 0;
@@ -154,15 +167,16 @@ static int ams_delta_init(struct platform_device *pdev)
return -ENXIO;
/* Allocate memory for MTD device structure and private data */
- this = kzalloc(sizeof(struct nand_chip), GFP_KERNEL);
- if (!this) {
+ priv = devm_kzalloc(&pdev->dev, sizeof(struct ams_delta_nand),
+ GFP_KERNEL);
+ if (!priv) {
pr_warn("Unable to allocate E3 NAND MTD device structure.\n");
- err = -ENOMEM;
- goto out;
+ return -ENOMEM;
}
+ this = &priv->nand_chip;
- ams_delta_mtd = nand_to_mtd(this);
- ams_delta_mtd->dev.parent = &pdev->dev;
+ mtd = nand_to_mtd(this);
+ mtd->dev.parent = &pdev->dev;
/*
* Don't try to request the memory region from here,
@@ -177,7 +191,8 @@ static int ams_delta_init(struct platform_device *pdev)
goto out_free;
}
- nand_set_controller_data(this, (void *)io_base);
+ priv->io_base = io_base;
+ nand_set_controller_data(this, priv);
/* Set address of NAND IO lines */
this->IO_ADDR_R = io_base + OMAP_MPUIO_INPUT_LATCH;
@@ -187,14 +202,14 @@ static int ams_delta_init(struct platform_device *pdev)
this->read_buf = ams_delta_read_buf;
this->cmd_ctrl = ams_delta_hwcontrol;
- gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
- if (IS_ERR(gpiod_rdy)) {
- err = PTR_ERR(gpiod_rdy);
+ priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+ if (IS_ERR(priv->gpiod_rdy)) {
+ err = PTR_ERR(priv->gpiod_rdy);
dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
goto out_mtd;
}
- if (gpiod_rdy)
+ if (priv->gpiod_rdy)
this->dev_ready = ams_delta_nand_ready;
/* 25 us command delay time */
@@ -202,66 +217,64 @@ static int ams_delta_init(struct platform_device *pdev)
this->ecc.mode = NAND_ECC_SOFT;
this->ecc.algo = NAND_ECC_HAMMING;
- platform_set_drvdata(pdev, io_base);
+ platform_set_drvdata(pdev, priv);
/* Set chip enabled, but */
- gpiod_nwp = devm_gpiod_get(&pdev->dev, "nwp", GPIOD_OUT_HIGH);
- if (IS_ERR(gpiod_nwp)) {
- err = PTR_ERR(gpiod_nwp);
+ priv->gpiod_nwp = devm_gpiod_get(&pdev->dev, "nwp", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->gpiod_nwp)) {
+ err = PTR_ERR(priv->gpiod_nwp);
dev_err(&pdev->dev, "NWP GPIO request failed (%d)\n", err);
goto out_mtd;
}
- gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
- if (IS_ERR(gpiod_nce)) {
- err = PTR_ERR(gpiod_nce);
+ priv->gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->gpiod_nce)) {
+ err = PTR_ERR(priv->gpiod_nce);
dev_err(&pdev->dev, "NCE GPIO request failed (%d)\n", err);
goto out_mtd;
}
- gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
- if (IS_ERR(gpiod_nre)) {
- err = PTR_ERR(gpiod_nre);
+ priv->gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->gpiod_nre)) {
+ err = PTR_ERR(priv->gpiod_nre);
dev_err(&pdev->dev, "NRE GPIO request failed (%d)\n", err);
goto out_mtd;
}
- gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
- if (IS_ERR(gpiod_nwe)) {
- err = PTR_ERR(gpiod_nwe);
+ priv->gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->gpiod_nwe)) {
+ err = PTR_ERR(priv->gpiod_nwe);
dev_err(&pdev->dev, "NWE GPIO request failed (%d)\n", err);
goto out_mtd;
}
- gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
- if (IS_ERR(gpiod_ale)) {
- err = PTR_ERR(gpiod_ale);
+ priv->gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
+ if (IS_ERR(priv->gpiod_ale)) {
+ err = PTR_ERR(priv->gpiod_ale);
dev_err(&pdev->dev, "ALE GPIO request failed (%d)\n", err);
goto out_mtd;
}
- gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
- if (IS_ERR(gpiod_cle)) {
- err = PTR_ERR(gpiod_cle);
+ priv->gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
+ if (IS_ERR(priv->gpiod_cle)) {
+ err = PTR_ERR(priv->gpiod_cle);
dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
goto out_mtd;
}
/* Scan to find existence of the device */
- err = nand_scan(ams_delta_mtd, 1);
+ err = nand_scan(mtd, 1);
if (err)
goto out_mtd;
/* Register the partitions */
- mtd_device_register(ams_delta_mtd, partition_info,
- ARRAY_SIZE(partition_info));
+ mtd_device_register(mtd, partition_info, ARRAY_SIZE(partition_info));
goto out;
out_mtd:
iounmap(io_base);
out_free:
- kfree(this);
out:
return err;
}
@@ -271,16 +284,15 @@ static int ams_delta_init(struct platform_device *pdev)
*/
static int ams_delta_cleanup(struct platform_device *pdev)
{
- void __iomem *io_base = platform_get_drvdata(pdev);
+ struct ams_delta_nand *priv = platform_get_drvdata(pdev);
+ struct mtd_info *mtd = nand_to_mtd(&priv->nand_chip);
+ void __iomem *io_base = priv->io_base;
/* Release resources, unregister device */
- nand_release(ams_delta_mtd);
+ nand_release(mtd);
iounmap(io_base);
- /* Free the MTD device structure */
- kfree(mtd_to_nand(ams_delta_mtd));
-
return 0;
}
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* [PATCH v3 3/7] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
2018-08-13 22:34 ` [PATCH v3 0/7] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
2018-08-13 22:34 ` [PATCH v3 1/7] mtd: rawnand: ams-delta: show parent device in sysfs Janusz Krzysztofik
2018-08-13 22:34 ` [PATCH v3 2/7] mtd: rawnand: ams-delta: Use private structure Janusz Krzysztofik
@ 2018-08-13 22:34 ` Janusz Krzysztofik
2018-08-13 22:34 ` [PATCH v3 4/7] mtd: rawnand: ams-delta: request data port GPIO resource Janusz Krzysztofik
` (4 subsequent siblings)
7 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-13 22:34 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, Linus Walleij, linux-arm-kernel,
linux-omap, linux-mtd, linux-gpio, linux-kernel,
Janusz Krzysztofik
Data port used by Amstrad Delta NAND driver is actually an OMAP MPUIO
device, already under control of gpio-omap driver. The NAND driver
gets access to the port by ioremapping it and performs read/write
operations. That is done without any proteciton from other users
legally manipulating the port pins over GPIO API.
The plan is to convert the driver to access the port over GPIO consumer
API. Before that is implemented, the driver can already obtain
exclusive access to the port by requesting an array of its GPIO
descriptors.
Add respective entries to the NAND GPIO lookup table.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
arch/arm/mach-omap1/board-ams-delta.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index eedacdfe9725..16f7bbe47607 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -337,7 +337,8 @@ static struct platform_device ams_delta_nand_device = {
.resource = ams_delta_nand_resources,
};
-#define OMAP_GPIO_LABEL "gpio-0-15"
+#define OMAP_GPIO_LABEL "gpio-0-15"
+#define OMAP_MPUIO_LABEL "mpuio"
static struct gpiod_lookup_table ams_delta_nand_gpio_table = {
.table = {
@@ -349,6 +350,14 @@ static struct gpiod_lookup_table ams_delta_nand_gpio_table = {
GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NWE, "nwe", 0),
GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_ALE, "ale", 0),
GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_CLE, "cle", 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 0, "data", 0, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 1, "data", 1, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 2, "data", 2, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 3, "data", 3, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 4, "data", 4, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 5, "data", 5, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 6, "data", 6, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 7, "data", 7, 0),
{ },
},
};
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* [PATCH v3 4/7] mtd: rawnand: ams-delta: request data port GPIO resource
2018-08-13 22:34 ` [PATCH v3 0/7] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
` (2 preceding siblings ...)
2018-08-13 22:34 ` [PATCH v3 3/7] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port Janusz Krzysztofik
@ 2018-08-13 22:34 ` Janusz Krzysztofik
2018-08-13 22:34 ` [PATCH v3 5/7] mtd: rawnand: ams-delta: Set port direction when needed Janusz Krzysztofik
` (3 subsequent siblings)
7 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-13 22:34 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, Linus Walleij, linux-arm-kernel,
linux-omap, linux-mtd, linux-gpio, linux-kernel,
Janusz Krzysztofik
Data port used by the driver is actually an OMAP MPUIO device, already
under control of gpio-omap driver. For that reason we used to not
request the memory region of the port as that would fail because the
region is already busy. Despite that, we are still accessing the port
by just ioremapping it and performing read/write operations. Moreover,
we are doing that without any proteciton from other users legally
manipulating the port pins over GPIO API.
The plan is to convert the driver to access the port over functions
exposed by the gpio-omap driver. Before that happens, already prevent
from other users accessing the port pins by requesting an array of its
GPIO descriptors.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mtd/nand/raw/ams-delta.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 48233d638d2a..09d6901fc94d 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -161,6 +161,7 @@ static int ams_delta_init(struct platform_device *pdev)
struct mtd_info *mtd;
struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
void __iomem *io_base;
+ struct gpio_descs *data_gpiods;
int err = 0;
if (!res)
@@ -261,6 +262,13 @@ static int ams_delta_init(struct platform_device *pdev)
dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
goto out_mtd;
}
+ /* Request array of data pins, initialize them as input */
+ data_gpiods = devm_gpiod_get_array(&pdev->dev, "data", GPIOD_IN);
+ if (IS_ERR(data_gpiods)) {
+ err = PTR_ERR(data_gpiods);
+ dev_err(&pdev->dev, "data GPIO request failed: %d\n", err);
+ goto out_mtd;
+ }
/* Scan to find existence of the device */
err = nand_scan(mtd, 1);
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* [PATCH v3 5/7] mtd: rawnand: ams-delta: Set port direction when needed
2018-08-13 22:34 ` [PATCH v3 0/7] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
` (3 preceding siblings ...)
2018-08-13 22:34 ` [PATCH v3 4/7] mtd: rawnand: ams-delta: request data port GPIO resource Janusz Krzysztofik
@ 2018-08-13 22:34 ` Janusz Krzysztofik
2018-08-16 7:30 ` Boris Brezillon
2018-08-13 22:34 ` [PATCH v3 6/7] mtd: rawnand: ams-delta: Simplify pointer resolution on read/write Janusz Krzysztofik
` (2 subsequent siblings)
7 siblings, 1 reply; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-13 22:34 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, Linus Walleij, linux-arm-kernel,
linux-omap, linux-mtd, linux-gpio, linux-kernel,
Janusz Krzysztofik
In its current shape, the driver sets data port direction before each
byte read/write operation, even during multi-byte transfers. Improve
performance of the driver by setting the port direction only when
needed.
This optimisation will become particularly important as soon as
planned conversion of the driver to GPIO API for data I/O will be
implemented.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
drivers/mtd/nand/raw/ams-delta.c | 59 ++++++++++++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 09d6901fc94d..5f9180fe4f8b 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -45,6 +45,7 @@ struct ams_delta_nand {
struct gpio_desc *gpiod_ale;
struct gpio_desc *gpiod_cle;
void __iomem *io_base;
+ bool data_in;
};
/*
@@ -72,50 +73,83 @@ static const struct mtd_partition partition_info[] = {
.size = 3 * SZ_256K },
};
-static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
+static void ams_delta_write_next_byte(struct mtd_info *mtd, u_char byte)
{
struct nand_chip *this = mtd_to_nand(mtd);
struct ams_delta_nand *priv = nand_get_controller_data(this);
- void __iomem *io_base = priv->io_base;
- writew(0, io_base + OMAP_MPUIO_IO_CNTL);
writew(byte, this->IO_ADDR_W);
+
gpiod_set_value(priv->gpiod_nwe, 0);
ndelay(40);
gpiod_set_value(priv->gpiod_nwe, 1);
}
-static u_char ams_delta_read_byte(struct mtd_info *mtd)
+static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
{
- u_char res;
struct nand_chip *this = mtd_to_nand(mtd);
struct ams_delta_nand *priv = nand_get_controller_data(this);
void __iomem *io_base = priv->io_base;
+ if (priv->data_in) {
+ writew(0, io_base + OMAP_MPUIO_IO_CNTL);
+ priv->data_in = false;
+ }
+
+ ams_delta_write_next_byte(mtd, byte);
+}
+
+static u_char ams_delta_read_next_byte(struct mtd_info *mtd)
+{
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
+ u_char res;
+
gpiod_set_value(priv->gpiod_nre, 0);
ndelay(40);
- writew(~0, io_base + OMAP_MPUIO_IO_CNTL);
+
res = readw(this->IO_ADDR_R);
+
gpiod_set_value(priv->gpiod_nre, 1);
return res;
}
+static u_char ams_delta_read_byte(struct mtd_info *mtd)
+{
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
+ void __iomem *io_base = priv->io_base;
+
+ if (!priv->data_in) {
+ writew(~0, io_base + OMAP_MPUIO_IO_CNTL);
+ priv->data_in = true;
+ }
+
+ return ams_delta_read_next_byte(mtd);
+}
+
static void ams_delta_write_buf(struct mtd_info *mtd, const u_char *buf,
int len)
{
- int i;
+ int i = 0;
+
+ if (len > 0)
+ ams_delta_write_byte(mtd, buf[i++]);
- for (i=0; i<len; i++)
- ams_delta_write_byte(mtd, buf[i]);
+ while (i < len)
+ ams_delta_write_next_byte(mtd, buf[i++]);
}
static void ams_delta_read_buf(struct mtd_info *mtd, u_char *buf, int len)
{
- int i;
+ int i = 0;
+
+ if (len > 0)
+ buf[i++] = ams_delta_read_byte(mtd);
- for (i=0; i<len; i++)
- buf[i] = ams_delta_read_byte(mtd);
+ while (i < len)
+ buf[i++] = ams_delta_read_next_byte(mtd);
}
/*
@@ -269,6 +303,7 @@ static int ams_delta_init(struct platform_device *pdev)
dev_err(&pdev->dev, "data GPIO request failed: %d\n", err);
goto out_mtd;
}
+ priv->data_in = true;
/* Scan to find existence of the device */
err = nand_scan(mtd, 1);
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [PATCH v3 5/7] mtd: rawnand: ams-delta: Set port direction when needed
2018-08-13 22:34 ` [PATCH v3 5/7] mtd: rawnand: ams-delta: Set port direction when needed Janusz Krzysztofik
@ 2018-08-16 7:30 ` Boris Brezillon
0 siblings, 0 replies; 93+ messages in thread
From: Boris Brezillon @ 2018-08-16 7:30 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Miquel Raynal, Richard Weinberger, David Woodhouse, Brian Norris,
Marek Vasut, Tony Lindgren, Aaro Koskinen, Linus Walleij,
linux-arm-kernel, linux-omap, linux-mtd, linux-gpio,
linux-kernel
On Tue, 14 Aug 2018 00:34:46 +0200
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> In its current shape, the driver sets data port direction before each
> byte read/write operation, even during multi-byte transfers. Improve
> performance of the driver by setting the port direction only when
> needed.
>
> This optimisation will become particularly important as soon as
> planned conversion of the driver to GPIO API for data I/O will be
> implemented.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> ---
> drivers/mtd/nand/raw/ams-delta.c | 59 ++++++++++++++++++++++++++++++++--------
> 1 file changed, 47 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
> index 09d6901fc94d..5f9180fe4f8b 100644
> --- a/drivers/mtd/nand/raw/ams-delta.c
> +++ b/drivers/mtd/nand/raw/ams-delta.c
> @@ -45,6 +45,7 @@ struct ams_delta_nand {
> struct gpio_desc *gpiod_ale;
> struct gpio_desc *gpiod_cle;
> void __iomem *io_base;
> + bool data_in;
> };
>
> /*
> @@ -72,50 +73,83 @@ static const struct mtd_partition partition_info[] = {
> .size = 3 * SZ_256K },
> };
>
> -static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
> +static void ams_delta_write_next_byte(struct mtd_info *mtd, u_char byte)
> {
> struct nand_chip *this = mtd_to_nand(mtd);
> struct ams_delta_nand *priv = nand_get_controller_data(this);
> - void __iomem *io_base = priv->io_base;
>
> - writew(0, io_base + OMAP_MPUIO_IO_CNTL);
> writew(byte, this->IO_ADDR_W);
> +
> gpiod_set_value(priv->gpiod_nwe, 0);
> ndelay(40);
> gpiod_set_value(priv->gpiod_nwe, 1);
> }
>
> -static u_char ams_delta_read_byte(struct mtd_info *mtd)
> +static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
> {
> - u_char res;
> struct nand_chip *this = mtd_to_nand(mtd);
> struct ams_delta_nand *priv = nand_get_controller_data(this);
> void __iomem *io_base = priv->io_base;
>
> + if (priv->data_in) {
> + writew(0, io_base + OMAP_MPUIO_IO_CNTL);
> + priv->data_in = false;
> + }
> +
> + ams_delta_write_next_byte(mtd, byte);
> +}
I'm not a big fan of this {read,write}_byte/next_byte() approach.
Can't we do something like:
static void ams_delta_io_write(struct ams_delta_nand *priv, u8 data)
{
writew(byte, priv->nand_chip.IO_ADDR_W);
gpiod_set_value(priv->gpiod_nwe, 0);
ndelay(40);
gpiod_set_value(priv->gpiod_nwe, 1);
}
static u8 ams_delta_io_read(struct ams_delta_nand *priv)
{
u8 res;
gpiod_set_value(priv->gpiod_nre, 0);
ndelay(40);
res = readw(priv->nand_chip.IO_ADDR_R);
gpiod_set_value(priv->gpiod_nre, 1);
return res;
}
static void ams_delta_set_io_dir(struct ams_delta_nand *priv, bool in)
{
if (in == priv->data_in)
return;
writew(in ? ~0 : 0, priv->io_base + OMAP_MPUIO_IO_CNTL);
priv->data_in = in;
}
static void ams_delta_write_buf(struct mtd_info *mtd, const u8 *buf,
int len)
{
struct nand_chip *this = mtd_to_nand(mtd);
struct ams_delta_nand *priv = nand_get_controller_data(this);
int i;
ams_delta_set_io_dir(priv, false);
for (i =0; i < len; i++)
ams_delta_io_write(priv, buf[i]);
}
static void ams_delta_write_byte(struct mtd_info *mtd, u8 byte)
{
ams_delta_write_buf(mtd, &byte, 1);
}
static void ams_delta_read_buf(struct mtd_info *mtd, u8 *buf, int len)
{
struct nand_chip *this = mtd_to_nand(mtd);
struct ams_delta_nand *priv = nand_get_controller_data(this);
int i;
ams_delta_set_io_dir(priv, true);
for (i = 0; i < len; i++)
buf[i] = ams_delta_io_read(priv);
}
static u8 ams_delta_read_byte(struct mtd_info *mtd)
{
u8 res;
ams_delta_read_buf(mtd, &res, 1);
return res;
}
^ permalink raw reply [flat|nested] 93+ messages in thread
* [PATCH v3 6/7] mtd: rawnand: ams-delta: Simplify pointer resolution on read/write
2018-08-13 22:34 ` [PATCH v3 0/7] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
` (4 preceding siblings ...)
2018-08-13 22:34 ` [PATCH v3 5/7] mtd: rawnand: ams-delta: Set port direction when needed Janusz Krzysztofik
@ 2018-08-13 22:34 ` Janusz Krzysztofik
2018-08-13 22:34 ` [PATCH v3 7/7] mtd: rawnand: ams-delta: use GPIO API for data I/O Janusz Krzysztofik
2018-11-21 11:08 ` Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use " Janusz Krzysztofik
7 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-13 22:34 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, Linus Walleij, linux-arm-kernel,
linux-omap, linux-mtd, linux-gpio, linux-kernel,
Janusz Krzysztofik
Simplify data read/write sub-functions by making them accept private
structure pointer instead of resolving it again from mtd_info.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
drivers/mtd/nand/raw/ams-delta.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 5f9180fe4f8b..59fc417e8fa9 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -73,10 +73,9 @@ static const struct mtd_partition partition_info[] = {
.size = 3 * SZ_256K },
};
-static void ams_delta_write_next_byte(struct mtd_info *mtd, u_char byte)
+static void ams_delta_write_next_byte(struct ams_delta_nand *priv, u_char byte)
{
- struct nand_chip *this = mtd_to_nand(mtd);
- struct ams_delta_nand *priv = nand_get_controller_data(this);
+ struct nand_chip *this = &priv->nand_chip;
writew(byte, this->IO_ADDR_W);
@@ -85,10 +84,8 @@ static void ams_delta_write_next_byte(struct mtd_info *mtd, u_char byte)
gpiod_set_value(priv->gpiod_nwe, 1);
}
-static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
+static void ams_delta_write_byte(struct ams_delta_nand *priv, u_char byte)
{
- struct nand_chip *this = mtd_to_nand(mtd);
- struct ams_delta_nand *priv = nand_get_controller_data(this);
void __iomem *io_base = priv->io_base;
if (priv->data_in) {
@@ -96,13 +93,12 @@ static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
priv->data_in = false;
}
- ams_delta_write_next_byte(mtd, byte);
+ ams_delta_write_next_byte(priv, byte);
}
-static u_char ams_delta_read_next_byte(struct mtd_info *mtd)
+static u_char ams_delta_read_next_byte(struct ams_delta_nand *priv)
{
- struct nand_chip *this = mtd_to_nand(mtd);
- struct ams_delta_nand *priv = nand_get_controller_data(this);
+ struct nand_chip *this = &priv->nand_chip;
u_char res;
gpiod_set_value(priv->gpiod_nre, 0);
@@ -126,30 +122,34 @@ static u_char ams_delta_read_byte(struct mtd_info *mtd)
priv->data_in = true;
}
- return ams_delta_read_next_byte(mtd);
+ return ams_delta_read_next_byte(priv);
}
static void ams_delta_write_buf(struct mtd_info *mtd, const u_char *buf,
int len)
{
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
int i = 0;
if (len > 0)
- ams_delta_write_byte(mtd, buf[i++]);
+ ams_delta_write_byte(priv, buf[i++]);
while (i < len)
- ams_delta_write_next_byte(mtd, buf[i++]);
+ ams_delta_write_next_byte(priv, buf[i++]);
}
static void ams_delta_read_buf(struct mtd_info *mtd, u_char *buf, int len)
{
+ struct nand_chip *this = mtd_to_nand(mtd);
+ struct ams_delta_nand *priv = nand_get_controller_data(this);
int i = 0;
if (len > 0)
buf[i++] = ams_delta_read_byte(mtd);
while (i < len)
- buf[i++] = ams_delta_read_next_byte(mtd);
+ buf[i++] = ams_delta_read_next_byte(priv);
}
/*
@@ -173,7 +173,7 @@ static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
}
if (cmd != NAND_CMD_NONE)
- ams_delta_write_byte(mtd, cmd);
+ ams_delta_write_byte(priv, cmd);
}
static int ams_delta_nand_ready(struct mtd_info *mtd)
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* [PATCH v3 7/7] mtd: rawnand: ams-delta: use GPIO API for data I/O
2018-08-13 22:34 ` [PATCH v3 0/7] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
` (5 preceding siblings ...)
2018-08-13 22:34 ` [PATCH v3 6/7] mtd: rawnand: ams-delta: Simplify pointer resolution on read/write Janusz Krzysztofik
@ 2018-08-13 22:34 ` Janusz Krzysztofik
2018-08-16 7:39 ` Boris Brezillon
2018-11-21 11:08 ` Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use " Janusz Krzysztofik
7 siblings, 1 reply; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-08-13 22:34 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Tony Lindgren, Aaro Koskinen, Linus Walleij, linux-arm-kernel,
linux-omap, linux-mtd, linux-gpio, linux-kernel,
Janusz Krzysztofik
Don't readw()/writew() data directly from/to GPIO port which is under
control of gpio-omap driver, use GPIO API instead.
Degrade of performance on Amstrad Delta is significant, can be
recognized as a regression, that's why I'm still submitting this patch
as RFC.
The driver should work with any 8+-bit bidirectional GPIO port, not
only OMAP.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
Exceprts fro timestamped boot logs showing performance degrade.
Before the change:
[ 5.469426] Creating 6 MTD partitions on "ams-delta-nand":
[ 5.480909] 0x000000000000-0x000000380000 : "Kernel"
[ 5.502659] 0x000000380000-0x0000003c0000 : "u-boot"
[ 5.523055] 0x0000003c0000-0x000000400000 : "u-boot params"
[ 5.543612] 0x000000400000-0x000000440000 : "Amstrad LDR"
[ 5.564607] 0x000000440000-0x000001f40000 : "File system"
[ 5.601760] 0x000001f40000-0x000002000000 : "PBL reserved"
[ 5.624369] usbcore: registered new interface driver dm9601
[ 5.636233] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
[ 5.649191] ohci-omap: OHCI OMAP driver
[ 5.660713] ohci ohci: OMAP OHCI
[ 5.671299] ohci ohci: new USB bus registered, assigned bus number 1
[ 5.686862] ohci ohci: irq 54, io mem 0xfffba000
[ 5.785897] hub 1-0:1.0: USB hub found
[ 5.797856] hub 1-0:1.0: 3 ports detected
[ 5.817576] usbcore: registered new interface driver usb-storage
[ 5.832551] ams-delta-serio ams-delta-serio: incomplete constraints, dummy supplies not allowed
[ 5.858588] ams-delta-serio ams-delta-serio: regulator request failed (-19)
[ 5.879312] input: omap-keypad as /devices/platform/omap-keypad/input/input0
[ 5.902490] omap_rtc omap_rtc: already running
[ 5.922929] omap_rtc omap_rtc: registered as rtc0
[ 5.945570] softdog: initialized. soft_noboot=0 soft_margin=60 sec soft_panic=0 (nowayout=0)
[ 5.976712] usbcore: registered new interface driver btusb
[ 6.007348] cx20442-codec cx20442-codec: incomplete constraints, dummy supplies not allowed
[ 6.040575] cx20442-codec cx20442-codec: failed to get POR supply (-19)
[ 6.060916] cx20442-codec cx20442-codec: ASoC: failed to probe component -517
[ 6.083486] ams-delta-audio ams-delta-audio: ASoC: failed to instantiate card -517
[ 6.121850] ams-delta-audio ams-delta-audio: snd_soc_register_card failed (-517)
[ 6.163047] NET: Registered protocol family 17
[ 6.182770] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
[ 6.203517] Bluetooth: BNEP socket layer initialized
[ 6.283017] serial8250 serial8250.1: Linked as a consumer to regulator.1
[ 6.306113] usb 1-1: new full-speed USB device number 2 using ohci
[ 6.328825] clock: disabling unused clocks to save power
[ 6.350426] Skipping reset check for DSP domain clock "dsptim_ck"
[ 6.372272] Skipping reset check for DSP domain clock "dspxor_ck"
[ 6.393712] Skipping reset check for DSP domain clock "dspper_ck"
[ 6.428311] ams-delta-serio ams-delta-serio: Linked as a consumer to regulator.2
[ 6.467801] serio serio0: AMS DELTA keyboard adapter
[ 6.492511] cx20442-codec cx20442-codec: Linked as a consumer to regulator.1
[ 6.527382] ams-delta-audio ams-delta-audio: cx20442-voice <-> omap-mcbsp.1 mapping ok
[ 6.577387] input: AMS_DELTA hook_switch as /devices/platform/ams-delta-audio/sound/card0/input1
[ 6.627497] input: AT Raw Set 2 keyboard as /devices/platform/ams-delta-serio/serio0/input/input2
[ 6.673663] omap_rtc omap_rtc: setting system clock to 2013-02-09 07:22:13 UTC (1360394533)
[ 6.715895] modem_nreset: incomplete constraints, leaving on
[ 6.738677] ALSA device list:
[ 6.758398] #0: AMS_DELTA
[ 7.036234] dm9601 1-1:1.0 eth0: register 'dm9601' at usb-ohci-1, Davicom DM96xx USB 10/100 Ethernet, 00:60:6e:00:00:11
[ 133.860599] random: crng init done
[ 138.275853] VFS: Mounted root (jffs2 filesystem) on device 31:4.
After the change:
[ 6.261107] Creating 6 MTD partitions on "ams-delta-nand":
[ 6.272046] 0x000000000000-0x000000380000 : "Kernel"
[ 6.294436] 0x000000380000-0x0000003c0000 : "u-boot"
[ 6.314454] 0x0000003c0000-0x000000400000 : "u-boot params"
[ 6.335353] 0x000000400000-0x000000440000 : "Amstrad LDR"
[ 6.356669] 0x000000440000-0x000001f40000 : "File system"
[ 6.393713] 0x000001f40000-0x000002000000 : "PBL reserved"
[ 6.416771] usbcore: registered new interface driver dm9601
[ 6.428631] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
[ 6.441533] ohci-omap: OHCI OMAP driver
[ 6.452758] ohci ohci: OMAP OHCI
[ 6.463300] ohci ohci: new USB bus registered, assigned bus number 1
[ 6.478817] ohci ohci: irq 54, io mem 0xfffba000
[ 6.580520] hub 1-0:1.0: USB hub found
[ 6.592424] hub 1-0:1.0: 3 ports detected
[ 6.612363] usbcore: registered new interface driver usb-storage
[ 6.627358] ams-delta-serio ams-delta-serio: incomplete constraints, dummy supplies not allowed
[ 6.653296] ams-delta-serio ams-delta-serio: regulator request failed (-19)
[ 6.674219] input: omap-keypad as /devices/platform/omap-keypad/input/input0
[ 6.697910] omap_rtc omap_rtc: already running
[ 6.718376] omap_rtc omap_rtc: registered as rtc0
[ 6.740942] softdog: initialized. soft_noboot=0 soft_margin=60 sec soft_panic=0 (nowayout=0)
[ 6.772085] usbcore: registered new interface driver btusb
[ 6.803187] cx20442-codec cx20442-codec: incomplete constraints, dummy supplies not allowed
[ 6.836386] cx20442-codec cx20442-codec: failed to get POR supply (-19)
[ 6.856730] cx20442-codec cx20442-codec: ASoC: failed to probe component -517
[ 6.879234] ams-delta-audio ams-delta-audio: ASoC: failed to instantiate card -517
[ 6.917325] ams-delta-audio ams-delta-audio: snd_soc_register_card failed (-517)
[ 6.958519] NET: Registered protocol family 17
[ 6.978224] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
[ 6.998989] Bluetooth: BNEP socket layer initialized
[ 7.077593] serial8250 serial8250.1: Linked as a consumer to regulator.1
[ 7.100678] usb 1-1: new full-speed USB device number 2 using ohci
[ 7.123429] clock: disabling unused clocks to save power
[ 7.145074] Skipping reset check for DSP domain clock "dsptim_ck"
[ 7.166983] Skipping reset check for DSP domain clock "dspxor_ck"
[ 7.188434] Skipping reset check for DSP domain clock "dspper_ck"
[ 7.223321] ams-delta-serio ams-delta-serio: Linked as a consumer to regulator.2
[ 7.262882] serio serio0: AMS DELTA keyboard adapter
[ 7.287656] cx20442-codec cx20442-codec: Linked as a consumer to regulator.1
[ 7.322824] ams-delta-audio ams-delta-audio: cx20442-voice <-> omap-mcbsp.1 mapping ok
[ 7.373165] input: AMS_DELTA hook_switch as /devices/platform/ams-delta-audio/sound/card0/input1
[ 7.423520] input: AT Raw Set 2 keyboard as /devices/platform/ams-delta-serio/serio0/input/input2
[ 7.469578] omap_rtc omap_rtc: setting system clock to 2013-02-09 07:34:10 UTC (1360395250)
[ 7.511830] modem_nreset: incomplete constraints, leaving on
[ 7.534812] ALSA device list:
[ 7.554541] #0: AMS_DELTA
[ 7.971899] dm9601 1-1:1.0 eth0: register 'dm9601' at usb-ohci-1, Davicom DM96xx USB 10/100 Ethernet, 00:60:6e:00:00:11
[ 133.935226] random: crng init done
[ 320.764645] VFS: Mounted root (jffs2 filesystem) on device 31:4.
I think most of the overhead is in iterations performed both inside and
outside get/set array functions:
- building a mask for get_multiple() and transfering results to value array
in gpiod_get_array_value_complex(), then again from the array by a caller,
- building a value array by the caller, the building a mask and tranferiing
values from array to bitmap for .set_multiple() in
gpiod_set_array_value_comples().
Thanks,
Janusz
drivers/mtd/nand/raw/ams-delta.c | 126 ++++++++++++++++++---------------------
1 file changed, 59 insertions(+), 67 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 59fc417e8fa9..8bedcd7c7928 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -24,13 +24,10 @@
#include <linux/mtd/mtd.h>
#include <linux/mtd/rawnand.h>
#include <linux/mtd/partitions.h>
-#include <linux/platform_data/gpio-omap.h>
+#include <linux/platform_device.h>
-#include <asm/io.h>
#include <asm/sizes.h>
-#include <mach/hardware.h>
-
/*
* MTD structure for E3 (Delta)
*/
@@ -44,7 +41,7 @@ struct ams_delta_nand {
struct gpio_desc *gpiod_nwe;
struct gpio_desc *gpiod_ale;
struct gpio_desc *gpiod_cle;
- void __iomem *io_base;
+ struct gpio_descs *data_gpiods;
bool data_in;
};
@@ -73,52 +70,79 @@ static const struct mtd_partition partition_info[] = {
.size = 3 * SZ_256K },
};
-static void ams_delta_write_next_byte(struct ams_delta_nand *priv, u_char byte)
+static void ams_delta_write_commit(struct ams_delta_nand *priv)
{
- struct nand_chip *this = &priv->nand_chip;
-
- writew(byte, this->IO_ADDR_W);
-
gpiod_set_value(priv->gpiod_nwe, 0);
ndelay(40);
gpiod_set_value(priv->gpiod_nwe, 1);
}
-static void ams_delta_write_byte(struct ams_delta_nand *priv, u_char byte)
+static void ams_delta_write_next_byte(struct ams_delta_nand *priv, u_char byte)
{
- void __iomem *io_base = priv->io_base;
+ struct gpio_descs *data_gpiods = priv->data_gpiods;
+ unsigned long bits = byte;
+ int i, value_array[data_gpiods->ndescs];
+ for (i = 0; i < data_gpiods->ndescs; i++)
+ value_array[i] = test_bit(i, &bits);
+
+ gpiod_set_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
+ value_array);
+
+ ams_delta_write_commit(priv);
+}
+
+static void ams_delta_write_byte(struct ams_delta_nand *priv, u_char byte)
+{
if (priv->data_in) {
- writew(0, io_base + OMAP_MPUIO_IO_CNTL);
+ struct gpio_descs *data_gpiods = priv->data_gpiods;
+ unsigned long bits = byte;
+ int i;
+
+ for (i = 0; i < data_gpiods->ndescs; i++)
+ gpiod_direction_output_raw(data_gpiods->desc[i],
+ test_bit(i, &bits));
priv->data_in = false;
- }
- ams_delta_write_next_byte(priv, byte);
+ ams_delta_write_commit(priv);
+ } else {
+ ams_delta_write_next_byte(priv, byte);
+ }
}
static u_char ams_delta_read_next_byte(struct ams_delta_nand *priv)
{
- struct nand_chip *this = &priv->nand_chip;
- u_char res;
+ struct gpio_descs *data_gpiods = priv->data_gpiods;
+ unsigned long bits = 0;
+ int i, value_array[data_gpiods->ndescs];
gpiod_set_value(priv->gpiod_nre, 0);
ndelay(40);
- res = readw(this->IO_ADDR_R);
+ gpiod_get_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
+ value_array);
gpiod_set_value(priv->gpiod_nre, 1);
- return res;
+ for (i = 0; i < data_gpiods->ndescs; i++)
+ if (value_array[i])
+ __set_bit(i, &bits);
+
+ return bits;
}
static u_char ams_delta_read_byte(struct mtd_info *mtd)
{
struct nand_chip *this = mtd_to_nand(mtd);
struct ams_delta_nand *priv = nand_get_controller_data(this);
- void __iomem *io_base = priv->io_base;
if (!priv->data_in) {
- writew(~0, io_base + OMAP_MPUIO_IO_CNTL);
+ struct gpio_descs *data_gpiods = priv->data_gpiods;
+ int i;
+
+ for (i = 0; i < data_gpiods->ndescs; i++)
+ gpiod_direction_input(data_gpiods->desc[i]);
+
priv->data_in = true;
}
@@ -193,14 +217,8 @@ static int ams_delta_init(struct platform_device *pdev)
struct ams_delta_nand *priv;
struct nand_chip *this;
struct mtd_info *mtd;
- struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- void __iomem *io_base;
- struct gpio_descs *data_gpiods;
int err = 0;
- if (!res)
- return -ENXIO;
-
/* Allocate memory for MTD device structure and private data */
priv = devm_kzalloc(&pdev->dev, sizeof(struct ams_delta_nand),
GFP_KERNEL);
@@ -213,25 +231,8 @@ static int ams_delta_init(struct platform_device *pdev)
mtd = nand_to_mtd(this);
mtd->dev.parent = &pdev->dev;
- /*
- * Don't try to request the memory region from here,
- * it should have been already requested from the
- * gpio-omap driver and requesting it again would fail.
- */
-
- io_base = ioremap(res->start, resource_size(res));
- if (io_base == NULL) {
- dev_err(&pdev->dev, "ioremap failed\n");
- err = -EIO;
- goto out_free;
- }
-
- priv->io_base = io_base;
nand_set_controller_data(this, priv);
- /* Set address of NAND IO lines */
- this->IO_ADDR_R = io_base + OMAP_MPUIO_INPUT_LATCH;
- this->IO_ADDR_W = io_base + OMAP_MPUIO_OUTPUT;
this->read_byte = ams_delta_read_byte;
this->write_buf = ams_delta_write_buf;
this->read_buf = ams_delta_read_buf;
@@ -241,7 +242,7 @@ static int ams_delta_init(struct platform_device *pdev)
if (IS_ERR(priv->gpiod_rdy)) {
err = PTR_ERR(priv->gpiod_rdy);
dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
if (priv->gpiod_rdy)
@@ -259,67 +260,61 @@ static int ams_delta_init(struct platform_device *pdev)
if (IS_ERR(priv->gpiod_nwp)) {
err = PTR_ERR(priv->gpiod_nwp);
dev_err(&pdev->dev, "NWP GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
priv->gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
if (IS_ERR(priv->gpiod_nce)) {
err = PTR_ERR(priv->gpiod_nce);
dev_err(&pdev->dev, "NCE GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
priv->gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
if (IS_ERR(priv->gpiod_nre)) {
err = PTR_ERR(priv->gpiod_nre);
dev_err(&pdev->dev, "NRE GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
priv->gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
if (IS_ERR(priv->gpiod_nwe)) {
err = PTR_ERR(priv->gpiod_nwe);
dev_err(&pdev->dev, "NWE GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
priv->gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
if (IS_ERR(priv->gpiod_ale)) {
err = PTR_ERR(priv->gpiod_ale);
dev_err(&pdev->dev, "ALE GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
priv->gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
if (IS_ERR(priv->gpiod_cle)) {
err = PTR_ERR(priv->gpiod_cle);
dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
- goto out_mtd;
+ return err;
}
/* Request array of data pins, initialize them as input */
- data_gpiods = devm_gpiod_get_array(&pdev->dev, "data", GPIOD_IN);
- if (IS_ERR(data_gpiods)) {
- err = PTR_ERR(data_gpiods);
+ priv->data_gpiods = devm_gpiod_get_array(&pdev->dev, "data", GPIOD_IN);
+ if (IS_ERR(priv->data_gpiods)) {
+ err = PTR_ERR(priv->data_gpiods);
dev_err(&pdev->dev, "data GPIO request failed: %d\n", err);
- goto out_mtd;
+ return err;
}
priv->data_in = true;
/* Scan to find existence of the device */
err = nand_scan(mtd, 1);
if (err)
- goto out_mtd;
+ return err;
/* Register the partitions */
mtd_device_register(mtd, partition_info, ARRAY_SIZE(partition_info));
- goto out;
-
- out_mtd:
- iounmap(io_base);
-out_free:
- out:
- return err;
+ return 0;
}
/*
@@ -329,13 +324,10 @@ static int ams_delta_cleanup(struct platform_device *pdev)
{
struct ams_delta_nand *priv = platform_get_drvdata(pdev);
struct mtd_info *mtd = nand_to_mtd(&priv->nand_chip);
- void __iomem *io_base = priv->io_base;
- /* Release resources, unregister device */
+ /* Unregister device */
nand_release(mtd);
- iounmap(io_base);
-
return 0;
}
--
2.16.4
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [PATCH v3 7/7] mtd: rawnand: ams-delta: use GPIO API for data I/O
2018-08-13 22:34 ` [PATCH v3 7/7] mtd: rawnand: ams-delta: use GPIO API for data I/O Janusz Krzysztofik
@ 2018-08-16 7:39 ` Boris Brezillon
0 siblings, 0 replies; 93+ messages in thread
From: Boris Brezillon @ 2018-08-16 7:39 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Miquel Raynal, Richard Weinberger, David Woodhouse, Brian Norris,
Marek Vasut, Tony Lindgren, Aaro Koskinen, Linus Walleij,
linux-arm-kernel, linux-omap, linux-mtd, linux-gpio,
linux-kernel
On Tue, 14 Aug 2018 00:34:48 +0200
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Don't readw()/writew() data directly from/to GPIO port which is under
> control of gpio-omap driver, use GPIO API instead.
>
> Degrade of performance on Amstrad Delta is significant, can be
> recognized as a regression, that's why I'm still submitting this patch
> as RFC.
>
> The driver should work with any 8+-bit bidirectional GPIO port, not
> only OMAP.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Exceprts fro timestamped boot logs showing performance degrade.
> Before the change:
> [ 5.469426] Creating 6 MTD partitions on "ams-delta-nand":
> [ 5.480909] 0x000000000000-0x000000380000 : "Kernel"
> [ 5.502659] 0x000000380000-0x0000003c0000 : "u-boot"
> [ 5.523055] 0x0000003c0000-0x000000400000 : "u-boot params"
> [ 5.543612] 0x000000400000-0x000000440000 : "Amstrad LDR"
> [ 5.564607] 0x000000440000-0x000001f40000 : "File system"
> [ 5.601760] 0x000001f40000-0x000002000000 : "PBL reserved"
> [ 5.624369] usbcore: registered new interface driver dm9601
> [ 5.636233] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
> [ 5.649191] ohci-omap: OHCI OMAP driver
> [ 5.660713] ohci ohci: OMAP OHCI
> [ 5.671299] ohci ohci: new USB bus registered, assigned bus number 1
> [ 5.686862] ohci ohci: irq 54, io mem 0xfffba000
> [ 5.785897] hub 1-0:1.0: USB hub found
> [ 5.797856] hub 1-0:1.0: 3 ports detected
> [ 5.817576] usbcore: registered new interface driver usb-storage
> [ 5.832551] ams-delta-serio ams-delta-serio: incomplete constraints, dummy supplies not allowed
> [ 5.858588] ams-delta-serio ams-delta-serio: regulator request failed (-19)
> [ 5.879312] input: omap-keypad as /devices/platform/omap-keypad/input/input0
> [ 5.902490] omap_rtc omap_rtc: already running
> [ 5.922929] omap_rtc omap_rtc: registered as rtc0
> [ 5.945570] softdog: initialized. soft_noboot=0 soft_margin=60 sec soft_panic=0 (nowayout=0)
> [ 5.976712] usbcore: registered new interface driver btusb
> [ 6.007348] cx20442-codec cx20442-codec: incomplete constraints, dummy supplies not allowed
> [ 6.040575] cx20442-codec cx20442-codec: failed to get POR supply (-19)
> [ 6.060916] cx20442-codec cx20442-codec: ASoC: failed to probe component -517
> [ 6.083486] ams-delta-audio ams-delta-audio: ASoC: failed to instantiate card -517
> [ 6.121850] ams-delta-audio ams-delta-audio: snd_soc_register_card failed (-517)
> [ 6.163047] NET: Registered protocol family 17
> [ 6.182770] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
> [ 6.203517] Bluetooth: BNEP socket layer initialized
> [ 6.283017] serial8250 serial8250.1: Linked as a consumer to regulator.1
> [ 6.306113] usb 1-1: new full-speed USB device number 2 using ohci
> [ 6.328825] clock: disabling unused clocks to save power
> [ 6.350426] Skipping reset check for DSP domain clock "dsptim_ck"
> [ 6.372272] Skipping reset check for DSP domain clock "dspxor_ck"
> [ 6.393712] Skipping reset check for DSP domain clock "dspper_ck"
> [ 6.428311] ams-delta-serio ams-delta-serio: Linked as a consumer to regulator.2
> [ 6.467801] serio serio0: AMS DELTA keyboard adapter
> [ 6.492511] cx20442-codec cx20442-codec: Linked as a consumer to regulator.1
> [ 6.527382] ams-delta-audio ams-delta-audio: cx20442-voice <-> omap-mcbsp.1 mapping ok
> [ 6.577387] input: AMS_DELTA hook_switch as /devices/platform/ams-delta-audio/sound/card0/input1
> [ 6.627497] input: AT Raw Set 2 keyboard as /devices/platform/ams-delta-serio/serio0/input/input2
> [ 6.673663] omap_rtc omap_rtc: setting system clock to 2013-02-09 07:22:13 UTC (1360394533)
> [ 6.715895] modem_nreset: incomplete constraints, leaving on
> [ 6.738677] ALSA device list:
> [ 6.758398] #0: AMS_DELTA
> [ 7.036234] dm9601 1-1:1.0 eth0: register 'dm9601' at usb-ohci-1, Davicom DM96xx USB 10/100 Ethernet, 00:60:6e:00:00:11
> [ 133.860599] random: crng init done
> [ 138.275853] VFS: Mounted root (jffs2 filesystem) on device 31:4.
>
> After the change:
> [ 6.261107] Creating 6 MTD partitions on "ams-delta-nand":
> [ 6.272046] 0x000000000000-0x000000380000 : "Kernel"
> [ 6.294436] 0x000000380000-0x0000003c0000 : "u-boot"
> [ 6.314454] 0x0000003c0000-0x000000400000 : "u-boot params"
> [ 6.335353] 0x000000400000-0x000000440000 : "Amstrad LDR"
> [ 6.356669] 0x000000440000-0x000001f40000 : "File system"
> [ 6.393713] 0x000001f40000-0x000002000000 : "PBL reserved"
> [ 6.416771] usbcore: registered new interface driver dm9601
> [ 6.428631] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
> [ 6.441533] ohci-omap: OHCI OMAP driver
> [ 6.452758] ohci ohci: OMAP OHCI
> [ 6.463300] ohci ohci: new USB bus registered, assigned bus number 1
> [ 6.478817] ohci ohci: irq 54, io mem 0xfffba000
> [ 6.580520] hub 1-0:1.0: USB hub found
> [ 6.592424] hub 1-0:1.0: 3 ports detected
> [ 6.612363] usbcore: registered new interface driver usb-storage
> [ 6.627358] ams-delta-serio ams-delta-serio: incomplete constraints, dummy supplies not allowed
> [ 6.653296] ams-delta-serio ams-delta-serio: regulator request failed (-19)
> [ 6.674219] input: omap-keypad as /devices/platform/omap-keypad/input/input0
> [ 6.697910] omap_rtc omap_rtc: already running
> [ 6.718376] omap_rtc omap_rtc: registered as rtc0
> [ 6.740942] softdog: initialized. soft_noboot=0 soft_margin=60 sec soft_panic=0 (nowayout=0)
> [ 6.772085] usbcore: registered new interface driver btusb
> [ 6.803187] cx20442-codec cx20442-codec: incomplete constraints, dummy supplies not allowed
> [ 6.836386] cx20442-codec cx20442-codec: failed to get POR supply (-19)
> [ 6.856730] cx20442-codec cx20442-codec: ASoC: failed to probe component -517
> [ 6.879234] ams-delta-audio ams-delta-audio: ASoC: failed to instantiate card -517
> [ 6.917325] ams-delta-audio ams-delta-audio: snd_soc_register_card failed (-517)
> [ 6.958519] NET: Registered protocol family 17
> [ 6.978224] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
> [ 6.998989] Bluetooth: BNEP socket layer initialized
> [ 7.077593] serial8250 serial8250.1: Linked as a consumer to regulator.1
> [ 7.100678] usb 1-1: new full-speed USB device number 2 using ohci
> [ 7.123429] clock: disabling unused clocks to save power
> [ 7.145074] Skipping reset check for DSP domain clock "dsptim_ck"
> [ 7.166983] Skipping reset check for DSP domain clock "dspxor_ck"
> [ 7.188434] Skipping reset check for DSP domain clock "dspper_ck"
> [ 7.223321] ams-delta-serio ams-delta-serio: Linked as a consumer to regulator.2
> [ 7.262882] serio serio0: AMS DELTA keyboard adapter
> [ 7.287656] cx20442-codec cx20442-codec: Linked as a consumer to regulator.1
> [ 7.322824] ams-delta-audio ams-delta-audio: cx20442-voice <-> omap-mcbsp.1 mapping ok
> [ 7.373165] input: AMS_DELTA hook_switch as /devices/platform/ams-delta-audio/sound/card0/input1
> [ 7.423520] input: AT Raw Set 2 keyboard as /devices/platform/ams-delta-serio/serio0/input/input2
> [ 7.469578] omap_rtc omap_rtc: setting system clock to 2013-02-09 07:34:10 UTC (1360395250)
> [ 7.511830] modem_nreset: incomplete constraints, leaving on
> [ 7.534812] ALSA device list:
> [ 7.554541] #0: AMS_DELTA
> [ 7.971899] dm9601 1-1:1.0 eth0: register 'dm9601' at usb-ohci-1, Davicom DM96xx USB 10/100 Ethernet, 00:60:6e:00:00:11
> [ 133.935226] random: crng init done
> [ 320.764645] VFS: Mounted root (jffs2 filesystem) on device 31:4.
>
> I think most of the overhead is in iterations performed both inside and
> outside get/set array functions:
> - building a mask for get_multiple() and transfering results to value array
> in gpiod_get_array_value_complex(), then again from the array by a caller,
> - building a value array by the caller, the building a mask and tranferiing
> values from array to bitmap for .set_multiple() in
> gpiod_set_array_value_comples().
Sorry but we need more than just speculations here, and using
bootlog timestamps is clearly not enough to prove any of these
suppositions. You'll have to setup ftrace/perf and trigger read/write
requests (using nanddump/nandwrite) to figure out where the overhead
comes from.
Also, I'd recommend keeping this patch for the series changing the GPIO
API.
^ permalink raw reply [flat|nested] 93+ messages in thread
* Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O
2018-08-13 22:34 ` [PATCH v3 0/7] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
` (6 preceding siblings ...)
2018-08-13 22:34 ` [PATCH v3 7/7] mtd: rawnand: ams-delta: use GPIO API for data I/O Janusz Krzysztofik
@ 2018-11-21 11:08 ` Janusz Krzysztofik
2018-11-21 11:08 ` [PATCH v4 1/4] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port Janusz Krzysztofik
` (4 more replies)
7 siblings, 5 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-11-21 11:08 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Tony Lindgren, Aaro Koskinen, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Linus Walleij,
linux-mtd, linux-omap, linux-arm-kernel, linux-kernel,
Janusz Krzysztofik
Finalize implementation of the idea suggested by Artem Bityutskiy and
Tony Lindgren, described in commit b027274d2e3a ("mtd: ams-delta: fix
request_mem_region() failure"). Use pure GPIO consumer API, as reqested
by Boris Brezillon.
Janusz Krzysztofik (4):
ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
mtd: rawnand: ams-delta: Request data port GPIO resource
mtd: rawnand: ams-delta: Use GPIO API for data I/O
ARM: OMAP1: ams-delta: Drop obsolete NAND resources
arch/arm/mach-omap1/board-ams-delta.c | 22 ++----
drivers/mtd/nand/raw/ams-delta.c | 120 +++++++++++++++++++---------------
2 files changed, 80 insertions(+), 62 deletions(-)
Performance on Amstrad Delta is now acceptable after recent extensions
to GPIO API and rawnanad enhancements.
Series intented to be merged via mtd tree, should not conflict with
other OMAP1 patches submitted for 4.21.
Changelog:
v4:
- drop patches 1/7, 2/7, 5/7 6/7 - changes already merged,
- reintroduce postponed PATCH v2 06/12 as 4/4,
- rebase on nand-next for 4.21.
v3:
[PATCH v3 1/7] mtd: rawnand: ams-delta: show parent device in sysfs
- renamed and an explanation added based on other similar patches on
Marek Vasut request, thanks.
[PATCH v3 2/7] mtd: rawnand: ams-delta: Use private structure
- no changes.
[PATCH v3 3/7] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND
data port
- no changes.
[PATCH v3 4/7] mtd: rawnand: ams-delta: request data port GPIO resource
- no changes.
[PATCH v3 5/7] mtd: rawnand: ams-delta: Set port direction when needed
- modified to set port direction only when needed instead of on each
transfer as suggested by Boris, thanks, though I kept separate
*_next_byte() functions to maximize performance as much as possible,
- moved back in front of "mtd: rawnand: ams-delta: use GPIO API for data
I/O" with a comment added referring to the planned switch to GPIO API.
[PATCH v3 6/7] mtd: rawnand: ams-delta: Simplify pointer resolution
- moved back in front of "mtd: rawnand: ams-delta: use GPIO API for data
I/O" on Boris request, thanks.
[RFC PATCH v3 7/7] mtd: rawnand: ams-delta: use GPIO API for data I/O
- rebased back on top of the two mentioned above,
- not intended to apply it yet due to performance issues on Amstrad Delta.
Removed from the series:
[RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct
mapping
[RFC PATCH v2 10/12] gpiolib: Introduce bitmap get/set array API extension
[RFC PATCH v2 11/12] mtd: rawnand: ams-delta: Use GPIO API bitmap extension
[RFC PATCH v2 12/12] gpiolib: Add fast processing path to bitmap API functions
- intended to be still iterated in a follow up series until performance
issues are resolved.
[RFC PATCH v2 06/12] ARM: OMAP1: ams-delta: drop obsolete NAND resources
- postponed until acceptable performance on Amstrad Delta is achieved.
v2:
[RFC PATCH v2 00/12] mtd: rawnand: ams-delta: Use GPIO API for data I/O
- renamed from former [RFC PATCH 0/8] mtd: rawnand: ams-delta: Use
gpio-omap accessors for data I/O
[RFC PATCH v2 01/12] mtd: rawnand: ams-delta: Assign mtd->dev.parent,
not mtd->owner
- split out from former [RFC PATCH 1/8] on Boris request, thanks.
[RFC PATCH v2 02/12] mtd: rawnand: ams-delta: Use private structure
- remaining part of the former [RFC PATCH 1/8].
[RFC PATCH v2 03/12] ARM: OMAP1: ams-delta: Provide GPIO lookup table
for NAND data port
- split out from former [RFC PATCH 5/8] on Boris requesst, thanks,
[RFC PATCH v2 04/12] mtd: rawnand: ams-delta: request data port GPIO resource
- remaining part of the former [RFC PATCH 5/8],
[RFC PATCH v2 05/12] mtd: rawnand: ams-delta: use GPIO API for data read/write
- reworked from former [RFC PATCH 8/8] on Boris requesst to use pure
GPIO API, thanks,
- moved up in front of former [RFC PATCH 3/8] on Boris request, thanks.
[RFC PATCH v2 06/12] ARM: OMAP1: ams-delta: drop obsolete NAND resources
- split out from former [RFC PATCH 8/8].
[RFC PATCH v2 07/12] mtd: rawnand: ams-delta: Set port direction once per
transfer
- reworked from former [RFC PATCH 3/8] on top of [RFC PATCH v2 05/12].
[RFC PATCH v2 08/12] mtd: rawnand: ams-delta: Simplify pointer resolution
on read/write
- reworked from former [RFC PATCH 4/8] on top of [RFC PATCH v2 08/12],
- renamed from 'Optimize' to 'Simplify' on Boris request, thanks.
[RFC PATCH v2 09/12] gpiolib: Identify GPIO descriptor arrays with direct
mapping
- new patch.
[RFC PATCH v2 10/12] gpiolib: Introduce bitmap get/set array API extension
- new patch.
[RFC PATCH v2 11/12] mtd: rawnand: ams-delta: Use GPIO API bitmap extension
- new patch.
[RFC PATCH v2 12/12] gpiolib: Add fast processing path to bitmap API functions
- new patch.
Removed from the series:
[RFC PATCH 2/8] mtd: rawnand: ams-delta: Write protect device during probe
- postponed on Boris request, thanks.
[RFC PATCH 6/8] gpio: omap: Add get/set_multiple() callbacks
- already applied by Linux in linux-gpio tree, thanks.
[RFC PATCH 7/8] mtd: rawnand: ams-delta: Check sanity of data GPIO resource
- most controversial one, no longer needed after switching to GPIO API.
Thanks,
Janusz
^ permalink raw reply [flat|nested] 93+ messages in thread
* [PATCH v4 1/4] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
2018-11-21 11:08 ` Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use " Janusz Krzysztofik
@ 2018-11-21 11:08 ` Janusz Krzysztofik
2018-11-23 17:02 ` Tony Lindgren
2018-11-21 11:08 ` [PATCH v4 2/4] mtd: rawnand: ams-delta: Request data port GPIO resource Janusz Krzysztofik
` (3 subsequent siblings)
4 siblings, 1 reply; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-11-21 11:08 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Tony Lindgren, Aaro Koskinen, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Linus Walleij,
linux-mtd, linux-omap, linux-arm-kernel, linux-kernel,
Janusz Krzysztofik
Data port used by Amstrad Delta NAND driver is actually an OMAP MPUIO
device, already under control of gpio-omap driver. The NAND driver
gets access to the port by ioremapping it and performs read/write
operations. That is done without any proteciton from other users
legally manipulating the port pins over GPIO API.
The plan is to convert the driver to access the port over GPIO consumer
API. Before that is implemented, the driver can already obtain
exclusive access to the port by requesting an array of its GPIO
descriptors.
Add respective entries to the NAND GPIO lookup table.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
arch/arm/mach-omap1/board-ams-delta.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 3d191fd52910..30c0d18f372e 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -312,7 +312,8 @@ static struct platform_device ams_delta_nand_device = {
.resource = ams_delta_nand_resources,
};
-#define OMAP_GPIO_LABEL "gpio-0-15"
+#define OMAP_GPIO_LABEL "gpio-0-15"
+#define OMAP_MPUIO_LABEL "mpuio"
static struct gpiod_lookup_table ams_delta_nand_gpio_table = {
.table = {
@@ -324,6 +325,14 @@ static struct gpiod_lookup_table ams_delta_nand_gpio_table = {
GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NWE, "nwe", 0),
GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_ALE, "ale", 0),
GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_CLE, "cle", 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 0, "data", 0, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 1, "data", 1, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 2, "data", 2, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 3, "data", 3, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 4, "data", 4, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 5, "data", 5, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 6, "data", 6, 0),
+ GPIO_LOOKUP_IDX(OMAP_MPUIO_LABEL, 7, "data", 7, 0),
{ },
},
};
--
2.18.1
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [PATCH v4 1/4] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
2018-11-21 11:08 ` [PATCH v4 1/4] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port Janusz Krzysztofik
@ 2018-11-23 17:02 ` Tony Lindgren
0 siblings, 0 replies; 93+ messages in thread
From: Tony Lindgren @ 2018-11-23 17:02 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Boris Brezillon, Miquel Raynal, Aaro Koskinen,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Linus Walleij, linux-mtd, linux-omap, linux-arm-kernel,
linux-kernel
* Janusz Krzysztofik <jmkrzyszt@gmail.com> [181121 11:06]:
> Data port used by Amstrad Delta NAND driver is actually an OMAP MPUIO
> device, already under control of gpio-omap driver. The NAND driver
> gets access to the port by ioremapping it and performs read/write
> operations. That is done without any proteciton from other users
> legally manipulating the port pins over GPIO API.
>
> The plan is to convert the driver to access the port over GPIO consumer
> API. Before that is implemented, the driver can already obtain
> exclusive access to the port by requesting an array of its GPIO
> descriptors.
>
> Add respective entries to the NAND GPIO lookup table.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Best to merge this along with the MFD patches:
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply [flat|nested] 93+ messages in thread
* [PATCH v4 2/4] mtd: rawnand: ams-delta: Request data port GPIO resource
2018-11-21 11:08 ` Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use " Janusz Krzysztofik
2018-11-21 11:08 ` [PATCH v4 1/4] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port Janusz Krzysztofik
@ 2018-11-21 11:08 ` Janusz Krzysztofik
2018-11-21 11:08 ` [PATCH v4 3/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
` (2 subsequent siblings)
4 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-11-21 11:08 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Tony Lindgren, Aaro Koskinen, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Linus Walleij,
linux-mtd, linux-omap, linux-arm-kernel, linux-kernel,
Janusz Krzysztofik
Data port used by the driver is actually an OMAP MPUIO device, already
under control of gpio-omap driver. For that reason we used to not
request the memory region of the port as that would fail because the
region is already busy. Despite that, we are still accessing the port
by just ioremapping it and performing read/write operations. Moreover,
we are doing that without any proteciton from other users legally
manipulating the port pins over GPIO API.
The plan is to convert the driver to access the port over GPIO consumer
API. Before that happens, already prevent from other users accessing
the port pins by requesting an array of its GPIO descriptors.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mtd/nand/raw/ams-delta.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index f8eb4a419e77..bb50dda05654 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -190,6 +190,7 @@ static int ams_delta_init(struct platform_device *pdev)
struct mtd_info *mtd;
struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
void __iomem *io_base;
+ struct gpio_descs *data_gpiods;
int err = 0;
if (!res)
@@ -275,8 +276,14 @@ static int ams_delta_init(struct platform_device *pdev)
goto err_unmap;
}
- /* Initialize data port direction to a known state */
- ams_delta_dir_input(priv, true);
+ /* Request array of data pins, initialize them as input */
+ data_gpiods = devm_gpiod_get_array(&pdev->dev, "data", GPIOD_IN);
+ if (IS_ERR(data_gpiods)) {
+ err = PTR_ERR(data_gpiods);
+ dev_err(&pdev->dev, "data GPIO request failed: %d\n", err);
+ goto err_unmap;
+ }
+ priv->data_in = true;
/* Initialize the NAND controller object embedded in ams_delta_nand. */
priv->base.ops = &ams_delta_ops;
--
2.18.1
^ permalink raw reply related [flat|nested] 93+ messages in thread
* [PATCH v4 3/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O
2018-11-21 11:08 ` Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use " Janusz Krzysztofik
2018-11-21 11:08 ` [PATCH v4 1/4] ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port Janusz Krzysztofik
2018-11-21 11:08 ` [PATCH v4 2/4] mtd: rawnand: ams-delta: Request data port GPIO resource Janusz Krzysztofik
@ 2018-11-21 11:08 ` Janusz Krzysztofik
2018-11-21 14:53 ` Boris Brezillon
2018-11-21 11:08 ` [PATCH v4 4/4] ARM: OMAP1: ams-delta: Drop obsolete NAND resources Janusz Krzysztofik
2018-11-21 14:56 ` Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O Boris Brezillon
4 siblings, 1 reply; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-11-21 11:08 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Tony Lindgren, Aaro Koskinen, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Linus Walleij,
linux-mtd, linux-omap, linux-arm-kernel, linux-kernel,
Janusz Krzysztofik
Don't readw()/writew() data directly from/to GPIO port which is under
control of gpio-omap driver, use GPIO consumer API instead.
The driver should now work with any 8-bit bidirectional GPIO port, not
only OMAP.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mtd/nand/raw/ams-delta.c | 109 +++++++++++++++++--------------
1 file changed, 61 insertions(+), 48 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index bb50dda05654..8312182088c1 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -18,11 +18,10 @@
#include <linux/module.h>
#include <linux/delay.h>
#include <linux/gpio/consumer.h>
-#include <linux/io.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/rawnand.h>
#include <linux/mtd/partitions.h>
-#include <linux/platform_data/gpio-omap.h>
+#include <linux/platform_device.h>
#include <linux/sizes.h>
/*
@@ -38,7 +37,7 @@ struct ams_delta_nand {
struct gpio_desc *gpiod_nwe;
struct gpio_desc *gpiod_ale;
struct gpio_desc *gpiod_cle;
- void __iomem *io_base;
+ struct gpio_descs *data_gpiods;
bool data_in;
};
@@ -67,42 +66,78 @@ static const struct mtd_partition partition_info[] = {
.size = 3 * SZ_256K },
};
-static void ams_delta_io_write(struct ams_delta_nand *priv, u8 byte)
+static void ams_delta_write_commit(struct ams_delta_nand *priv)
{
- writew(byte, priv->io_base + OMAP_MPUIO_OUTPUT);
gpiod_set_value(priv->gpiod_nwe, 0);
ndelay(40);
gpiod_set_value(priv->gpiod_nwe, 1);
}
+static void ams_delta_io_write(struct ams_delta_nand *priv, u8 byte)
+{
+ struct gpio_descs *data_gpiods = priv->data_gpiods;
+ DECLARE_BITMAP(values, BITS_PER_TYPE(byte)) = { byte, };
+
+ gpiod_set_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
+ data_gpiods->info, values);
+
+ ams_delta_write_commit(priv);
+}
+
+static void ams_delta_dir_output(struct ams_delta_nand *priv, u8 byte)
+{
+ struct gpio_descs *data_gpiods = priv->data_gpiods;
+ DECLARE_BITMAP(values, BITS_PER_TYPE(byte)) = { byte, };
+ int i;
+
+ for (i = 0; i < data_gpiods->ndescs; i++)
+ gpiod_direction_output_raw(data_gpiods->desc[i],
+ test_bit(i, values));
+
+ ams_delta_write_commit(priv);
+
+ priv->data_in = false;
+}
+
static u8 ams_delta_io_read(struct ams_delta_nand *priv)
{
u8 res;
+ struct gpio_descs *data_gpiods = priv->data_gpiods;
+ DECLARE_BITMAP(values, BITS_PER_TYPE(res)) = { 0, };
gpiod_set_value(priv->gpiod_nre, 0);
ndelay(40);
- res = readw(priv->io_base + OMAP_MPUIO_INPUT_LATCH);
+
+ gpiod_get_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
+ data_gpiods->info, values);
+
gpiod_set_value(priv->gpiod_nre, 1);
+ res = values[0];
return res;
}
-static void ams_delta_dir_input(struct ams_delta_nand *priv, bool in)
+static void ams_delta_dir_input(struct ams_delta_nand *priv)
{
- writew(in ? ~0 : 0, priv->io_base + OMAP_MPUIO_IO_CNTL);
- priv->data_in = in;
+ struct gpio_descs *data_gpiods = priv->data_gpiods;
+ int i;
+
+ for (i = 0; i < data_gpiods->ndescs; i++)
+ gpiod_direction_input(data_gpiods->desc[i]);
+
+ priv->data_in = true;
}
static void ams_delta_write_buf(struct ams_delta_nand *priv, const u8 *buf,
int len)
{
- int i;
+ int i = 0;
- if (priv->data_in)
- ams_delta_dir_input(priv, false);
+ if (len > 0 && priv->data_in)
+ ams_delta_dir_output(priv, buf[i++]);
- for (i = 0; i < len; i++)
- ams_delta_io_write(priv, buf[i]);
+ while (i < len)
+ ams_delta_io_write(priv, buf[i++]);
}
static void ams_delta_read_buf(struct ams_delta_nand *priv, u8 *buf, int len)
@@ -110,7 +145,7 @@ static void ams_delta_read_buf(struct ams_delta_nand *priv, u8 *buf, int len)
int i;
if (!priv->data_in)
- ams_delta_dir_input(priv, true);
+ ams_delta_dir_input(priv);
for (i = 0; i < len; i++)
buf[i] = ams_delta_io_read(priv);
@@ -188,14 +223,9 @@ static int ams_delta_init(struct platform_device *pdev)
struct ams_delta_nand *priv;
struct nand_chip *this;
struct mtd_info *mtd;
- struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- void __iomem *io_base;
struct gpio_descs *data_gpiods;
int err = 0;
- if (!res)
- return -ENXIO;
-
/* Allocate memory for MTD device structure and private data */
priv = devm_kzalloc(&pdev->dev, sizeof(struct ams_delta_nand),
GFP_KERNEL);
@@ -207,25 +237,13 @@ static int ams_delta_init(struct platform_device *pdev)
mtd = nand_to_mtd(this);
mtd->dev.parent = &pdev->dev;
- /*
- * Don't try to request the memory region from here,
- * it should have been already requested from the
- * gpio-omap driver and requesting it again would fail.
- */
- io_base = ioremap(res->start, resource_size(res));
- if (!io_base) {
- dev_err(&pdev->dev, "ioremap failed\n");
- return -EIO;
- }
-
- priv->io_base = io_base;
nand_set_controller_data(this, priv);
priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
if (IS_ERR(priv->gpiod_rdy)) {
err = PTR_ERR(priv->gpiod_rdy);
dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
- goto err_unmap;
+ return err;
}
this->ecc.mode = NAND_ECC_SOFT;
@@ -238,42 +256,42 @@ static int ams_delta_init(struct platform_device *pdev)
if (IS_ERR(priv->gpiod_nwp)) {
err = PTR_ERR(priv->gpiod_nwp);
dev_err(&pdev->dev, "NWP GPIO request failed (%d)\n", err);
- goto err_unmap;
+ return err;
}
priv->gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
if (IS_ERR(priv->gpiod_nce)) {
err = PTR_ERR(priv->gpiod_nce);
dev_err(&pdev->dev, "NCE GPIO request failed (%d)\n", err);
- goto err_unmap;
+ return err;
}
priv->gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
if (IS_ERR(priv->gpiod_nre)) {
err = PTR_ERR(priv->gpiod_nre);
dev_err(&pdev->dev, "NRE GPIO request failed (%d)\n", err);
- goto err_unmap;
+ return err;
}
priv->gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
if (IS_ERR(priv->gpiod_nwe)) {
err = PTR_ERR(priv->gpiod_nwe);
dev_err(&pdev->dev, "NWE GPIO request failed (%d)\n", err);
- goto err_unmap;
+ return err;
}
priv->gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
if (IS_ERR(priv->gpiod_ale)) {
err = PTR_ERR(priv->gpiod_ale);
dev_err(&pdev->dev, "ALE GPIO request failed (%d)\n", err);
- goto err_unmap;
+ return err;
}
priv->gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
if (IS_ERR(priv->gpiod_cle)) {
err = PTR_ERR(priv->gpiod_cle);
dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
- goto err_unmap;
+ return err;
}
/* Request array of data pins, initialize them as input */
@@ -281,8 +299,9 @@ static int ams_delta_init(struct platform_device *pdev)
if (IS_ERR(data_gpiods)) {
err = PTR_ERR(data_gpiods);
dev_err(&pdev->dev, "data GPIO request failed: %d\n", err);
- goto err_unmap;
+ return err;
}
+ priv->data_gpiods = data_gpiods;
priv->data_in = true;
/* Initialize the NAND controller object embedded in ams_delta_nand. */
@@ -293,7 +312,7 @@ static int ams_delta_init(struct platform_device *pdev)
/* Scan to find existence of the device */
err = nand_scan(this, 1);
if (err)
- goto err_unmap;
+ return err;
/* Register the partitions */
err = mtd_device_register(mtd, partition_info,
@@ -306,9 +325,6 @@ static int ams_delta_init(struct platform_device *pdev)
err_nand_cleanup:
nand_cleanup(this);
-err_unmap:
- iounmap(io_base);
-
return err;
}
@@ -319,13 +335,10 @@ static int ams_delta_cleanup(struct platform_device *pdev)
{
struct ams_delta_nand *priv = platform_get_drvdata(pdev);
struct mtd_info *mtd = nand_to_mtd(&priv->nand_chip);
- void __iomem *io_base = priv->io_base;
- /* Release resources, unregister device */
+ /* Unregister device */
nand_release(mtd_to_nand(mtd));
- iounmap(io_base);
-
return 0;
}
--
2.18.1
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: [PATCH v4 3/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O
2018-11-21 11:08 ` [PATCH v4 3/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
@ 2018-11-21 14:53 ` Boris Brezillon
0 siblings, 0 replies; 93+ messages in thread
From: Boris Brezillon @ 2018-11-21 14:53 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Miquel Raynal, Tony Lindgren, Aaro Koskinen, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Linus Walleij,
linux-mtd, linux-omap, linux-arm-kernel, linux-kernel
On Wed, 21 Nov 2018 12:08:05 +0100
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Don't readw()/writew() data directly from/to GPIO port which is under
> control of gpio-omap driver, use GPIO consumer API instead.
>
> The driver should now work with any 8-bit bidirectional GPIO port, not
> only OMAP.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
And thanks a lot for keeping up with that. I like the new ams-delta
driver, and I wonder if we couldn't extend it to replace the gpio-nand
driver.
> ---
> drivers/mtd/nand/raw/ams-delta.c | 109 +++++++++++++++++--------------
> 1 file changed, 61 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
> index bb50dda05654..8312182088c1 100644
> --- a/drivers/mtd/nand/raw/ams-delta.c
> +++ b/drivers/mtd/nand/raw/ams-delta.c
> @@ -18,11 +18,10 @@
> #include <linux/module.h>
> #include <linux/delay.h>
> #include <linux/gpio/consumer.h>
> -#include <linux/io.h>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/rawnand.h>
> #include <linux/mtd/partitions.h>
> -#include <linux/platform_data/gpio-omap.h>
> +#include <linux/platform_device.h>
> #include <linux/sizes.h>
>
> /*
> @@ -38,7 +37,7 @@ struct ams_delta_nand {
> struct gpio_desc *gpiod_nwe;
> struct gpio_desc *gpiod_ale;
> struct gpio_desc *gpiod_cle;
> - void __iomem *io_base;
> + struct gpio_descs *data_gpiods;
> bool data_in;
> };
>
> @@ -67,42 +66,78 @@ static const struct mtd_partition partition_info[] = {
> .size = 3 * SZ_256K },
> };
>
> -static void ams_delta_io_write(struct ams_delta_nand *priv, u8 byte)
> +static void ams_delta_write_commit(struct ams_delta_nand *priv)
> {
> - writew(byte, priv->io_base + OMAP_MPUIO_OUTPUT);
> gpiod_set_value(priv->gpiod_nwe, 0);
> ndelay(40);
> gpiod_set_value(priv->gpiod_nwe, 1);
> }
>
> +static void ams_delta_io_write(struct ams_delta_nand *priv, u8 byte)
> +{
> + struct gpio_descs *data_gpiods = priv->data_gpiods;
> + DECLARE_BITMAP(values, BITS_PER_TYPE(byte)) = { byte, };
> +
> + gpiod_set_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
> + data_gpiods->info, values);
> +
> + ams_delta_write_commit(priv);
> +}
> +
> +static void ams_delta_dir_output(struct ams_delta_nand *priv, u8 byte)
> +{
> + struct gpio_descs *data_gpiods = priv->data_gpiods;
> + DECLARE_BITMAP(values, BITS_PER_TYPE(byte)) = { byte, };
> + int i;
> +
> + for (i = 0; i < data_gpiods->ndescs; i++)
> + gpiod_direction_output_raw(data_gpiods->desc[i],
> + test_bit(i, values));
> +
> + ams_delta_write_commit(priv);
> +
> + priv->data_in = false;
> +}
> +
> static u8 ams_delta_io_read(struct ams_delta_nand *priv)
> {
> u8 res;
> + struct gpio_descs *data_gpiods = priv->data_gpiods;
> + DECLARE_BITMAP(values, BITS_PER_TYPE(res)) = { 0, };
>
> gpiod_set_value(priv->gpiod_nre, 0);
> ndelay(40);
> - res = readw(priv->io_base + OMAP_MPUIO_INPUT_LATCH);
> +
> + gpiod_get_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
> + data_gpiods->info, values);
> +
> gpiod_set_value(priv->gpiod_nre, 1);
>
> + res = values[0];
> return res;
> }
>
> -static void ams_delta_dir_input(struct ams_delta_nand *priv, bool in)
> +static void ams_delta_dir_input(struct ams_delta_nand *priv)
> {
> - writew(in ? ~0 : 0, priv->io_base + OMAP_MPUIO_IO_CNTL);
> - priv->data_in = in;
> + struct gpio_descs *data_gpiods = priv->data_gpiods;
> + int i;
> +
> + for (i = 0; i < data_gpiods->ndescs; i++)
> + gpiod_direction_input(data_gpiods->desc[i]);
> +
> + priv->data_in = true;
> }
>
> static void ams_delta_write_buf(struct ams_delta_nand *priv, const u8 *buf,
> int len)
> {
> - int i;
> + int i = 0;
>
> - if (priv->data_in)
> - ams_delta_dir_input(priv, false);
> + if (len > 0 && priv->data_in)
> + ams_delta_dir_output(priv, buf[i++]);
>
> - for (i = 0; i < len; i++)
> - ams_delta_io_write(priv, buf[i]);
> + while (i < len)
> + ams_delta_io_write(priv, buf[i++]);
> }
>
> static void ams_delta_read_buf(struct ams_delta_nand *priv, u8 *buf, int len)
> @@ -110,7 +145,7 @@ static void ams_delta_read_buf(struct ams_delta_nand *priv, u8 *buf, int len)
> int i;
>
> if (!priv->data_in)
> - ams_delta_dir_input(priv, true);
> + ams_delta_dir_input(priv);
>
> for (i = 0; i < len; i++)
> buf[i] = ams_delta_io_read(priv);
> @@ -188,14 +223,9 @@ static int ams_delta_init(struct platform_device *pdev)
> struct ams_delta_nand *priv;
> struct nand_chip *this;
> struct mtd_info *mtd;
> - struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - void __iomem *io_base;
> struct gpio_descs *data_gpiods;
> int err = 0;
>
> - if (!res)
> - return -ENXIO;
> -
> /* Allocate memory for MTD device structure and private data */
> priv = devm_kzalloc(&pdev->dev, sizeof(struct ams_delta_nand),
> GFP_KERNEL);
> @@ -207,25 +237,13 @@ static int ams_delta_init(struct platform_device *pdev)
> mtd = nand_to_mtd(this);
> mtd->dev.parent = &pdev->dev;
>
> - /*
> - * Don't try to request the memory region from here,
> - * it should have been already requested from the
> - * gpio-omap driver and requesting it again would fail.
> - */
> - io_base = ioremap(res->start, resource_size(res));
> - if (!io_base) {
> - dev_err(&pdev->dev, "ioremap failed\n");
> - return -EIO;
> - }
> -
> - priv->io_base = io_base;
> nand_set_controller_data(this, priv);
>
> priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> if (IS_ERR(priv->gpiod_rdy)) {
> err = PTR_ERR(priv->gpiod_rdy);
> dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
> - goto err_unmap;
> + return err;
> }
>
> this->ecc.mode = NAND_ECC_SOFT;
> @@ -238,42 +256,42 @@ static int ams_delta_init(struct platform_device *pdev)
> if (IS_ERR(priv->gpiod_nwp)) {
> err = PTR_ERR(priv->gpiod_nwp);
> dev_err(&pdev->dev, "NWP GPIO request failed (%d)\n", err);
> - goto err_unmap;
> + return err;
> }
>
> priv->gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
> if (IS_ERR(priv->gpiod_nce)) {
> err = PTR_ERR(priv->gpiod_nce);
> dev_err(&pdev->dev, "NCE GPIO request failed (%d)\n", err);
> - goto err_unmap;
> + return err;
> }
>
> priv->gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
> if (IS_ERR(priv->gpiod_nre)) {
> err = PTR_ERR(priv->gpiod_nre);
> dev_err(&pdev->dev, "NRE GPIO request failed (%d)\n", err);
> - goto err_unmap;
> + return err;
> }
>
> priv->gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
> if (IS_ERR(priv->gpiod_nwe)) {
> err = PTR_ERR(priv->gpiod_nwe);
> dev_err(&pdev->dev, "NWE GPIO request failed (%d)\n", err);
> - goto err_unmap;
> + return err;
> }
>
> priv->gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
> if (IS_ERR(priv->gpiod_ale)) {
> err = PTR_ERR(priv->gpiod_ale);
> dev_err(&pdev->dev, "ALE GPIO request failed (%d)\n", err);
> - goto err_unmap;
> + return err;
> }
>
> priv->gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
> if (IS_ERR(priv->gpiod_cle)) {
> err = PTR_ERR(priv->gpiod_cle);
> dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
> - goto err_unmap;
> + return err;
> }
>
> /* Request array of data pins, initialize them as input */
> @@ -281,8 +299,9 @@ static int ams_delta_init(struct platform_device *pdev)
> if (IS_ERR(data_gpiods)) {
> err = PTR_ERR(data_gpiods);
> dev_err(&pdev->dev, "data GPIO request failed: %d\n", err);
> - goto err_unmap;
> + return err;
> }
> + priv->data_gpiods = data_gpiods;
> priv->data_in = true;
>
> /* Initialize the NAND controller object embedded in ams_delta_nand. */
> @@ -293,7 +312,7 @@ static int ams_delta_init(struct platform_device *pdev)
> /* Scan to find existence of the device */
> err = nand_scan(this, 1);
> if (err)
> - goto err_unmap;
> + return err;
>
> /* Register the partitions */
> err = mtd_device_register(mtd, partition_info,
> @@ -306,9 +325,6 @@ static int ams_delta_init(struct platform_device *pdev)
> err_nand_cleanup:
> nand_cleanup(this);
>
> -err_unmap:
> - iounmap(io_base);
> -
> return err;
> }
>
> @@ -319,13 +335,10 @@ static int ams_delta_cleanup(struct platform_device *pdev)
> {
> struct ams_delta_nand *priv = platform_get_drvdata(pdev);
> struct mtd_info *mtd = nand_to_mtd(&priv->nand_chip);
> - void __iomem *io_base = priv->io_base;
>
> - /* Release resources, unregister device */
> + /* Unregister device */
> nand_release(mtd_to_nand(mtd));
>
> - iounmap(io_base);
> -
> return 0;
> }
>
^ permalink raw reply [flat|nested] 93+ messages in thread
* [PATCH v4 4/4] ARM: OMAP1: ams-delta: Drop obsolete NAND resources
2018-11-21 11:08 ` Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use " Janusz Krzysztofik
` (2 preceding siblings ...)
2018-11-21 11:08 ` [PATCH v4 3/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O Janusz Krzysztofik
@ 2018-11-21 11:08 ` Janusz Krzysztofik
2018-11-21 14:56 ` Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O Boris Brezillon
4 siblings, 0 replies; 93+ messages in thread
From: Janusz Krzysztofik @ 2018-11-21 11:08 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Tony Lindgren, Aaro Koskinen, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Linus Walleij,
linux-mtd, linux-omap, linux-arm-kernel, linux-kernel,
Janusz Krzysztofik
Amstrad Delta NAND driver now uses GPIO API for data I/O so there is no
need to assign memory I/O resource to the device any longer. Drop it.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
arch/arm/mach-omap1/board-ams-delta.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 30c0d18f372e..d594f60c3224 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -296,20 +296,9 @@ struct modem_private_data {
static struct modem_private_data modem_priv;
-static struct resource ams_delta_nand_resources[] = {
- [0] = {
- .start = OMAP1_MPUIO_BASE,
- .end = OMAP1_MPUIO_BASE +
- OMAP_MPUIO_IO_CNTL + sizeof(u32) - 1,
- .flags = IORESOURCE_MEM,
- },
-};
-
static struct platform_device ams_delta_nand_device = {
.name = "ams-delta-nand",
.id = -1,
- .num_resources = ARRAY_SIZE(ams_delta_nand_resources),
- .resource = ams_delta_nand_resources,
};
#define OMAP_GPIO_LABEL "gpio-0-15"
--
2.18.1
^ permalink raw reply related [flat|nested] 93+ messages in thread
* Re: Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O
2018-11-21 11:08 ` Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use " Janusz Krzysztofik
` (3 preceding siblings ...)
2018-11-21 11:08 ` [PATCH v4 4/4] ARM: OMAP1: ams-delta: Drop obsolete NAND resources Janusz Krzysztofik
@ 2018-11-21 14:56 ` Boris Brezillon
2018-11-23 17:03 ` Tony Lindgren
4 siblings, 1 reply; 93+ messages in thread
From: Boris Brezillon @ 2018-11-21 14:56 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Miquel Raynal, Tony Lindgren, Aaro Koskinen, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut, Linus Walleij,
linux-mtd, linux-omap, linux-arm-kernel, linux-kernel
On Wed, 21 Nov 2018 12:08:02 +0100
Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Finalize implementation of the idea suggested by Artem Bityutskiy and
> Tony Lindgren, described in commit b027274d2e3a ("mtd: ams-delta: fix
> request_mem_region() failure"). Use pure GPIO consumer API, as reqested
> by Boris Brezillon.
>
> Janusz Krzysztofik (4):
> ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
> mtd: rawnand: ams-delta: Request data port GPIO resource
> mtd: rawnand: ams-delta: Use GPIO API for data I/O
> ARM: OMAP1: ams-delta: Drop obsolete NAND resources
>
> arch/arm/mach-omap1/board-ams-delta.c | 22 ++----
> drivers/mtd/nand/raw/ams-delta.c | 120 +++++++++++++++++++---------------
> 2 files changed, 80 insertions(+), 62 deletions(-)
>
> Performance on Amstrad Delta is now acceptable after recent extensions
> to GPIO API and rawnanad enhancements.
>
> Series intented to be merged via mtd tree, should not conflict with
> other OMAP1 patches submitted for 4.21.
We'll prepare an immutable tag, just in case.
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O
2018-11-21 14:56 ` Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O Boris Brezillon
@ 2018-11-23 17:03 ` Tony Lindgren
2018-12-07 8:04 ` Miquel Raynal
0 siblings, 1 reply; 93+ messages in thread
From: Tony Lindgren @ 2018-11-23 17:03 UTC (permalink / raw)
To: Boris Brezillon
Cc: Janusz Krzysztofik, Miquel Raynal, Aaro Koskinen,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Linus Walleij, linux-mtd, linux-omap, linux-arm-kernel,
linux-kernel
* Boris Brezillon <boris.brezillon@bootlin.com> [181121 14:56]:
> On Wed, 21 Nov 2018 12:08:02 +0100
> Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
>
> > Finalize implementation of the idea suggested by Artem Bityutskiy and
> > Tony Lindgren, described in commit b027274d2e3a ("mtd: ams-delta: fix
> > request_mem_region() failure"). Use pure GPIO consumer API, as reqested
> > by Boris Brezillon.
> >
> > Janusz Krzysztofik (4):
> > ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
> > mtd: rawnand: ams-delta: Request data port GPIO resource
> > mtd: rawnand: ams-delta: Use GPIO API for data I/O
> > ARM: OMAP1: ams-delta: Drop obsolete NAND resources
> >
> > arch/arm/mach-omap1/board-ams-delta.c | 22 ++----
> > drivers/mtd/nand/raw/ams-delta.c | 120 +++++++++++++++++++---------------
> > 2 files changed, 80 insertions(+), 62 deletions(-)
> >
> > Performance on Amstrad Delta is now acceptable after recent extensions
> > to GPIO API and rawnanad enhancements.
> >
> > Series intented to be merged via mtd tree, should not conflict with
> > other OMAP1 patches submitted for 4.21.
>
> We'll prepare an immutable tag, just in case.
Sounds good to me thanks:
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O
2018-11-23 17:03 ` Tony Lindgren
@ 2018-12-07 8:04 ` Miquel Raynal
2018-12-07 16:10 ` Tony Lindgren
0 siblings, 1 reply; 93+ messages in thread
From: Miquel Raynal @ 2018-12-07 8:04 UTC (permalink / raw)
To: Tony Lindgren
Cc: Boris Brezillon, Janusz Krzysztofik, Aaro Koskinen,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Linus Walleij, linux-mtd, linux-omap, linux-arm-kernel,
linux-kernel
Hi Tony,
Tony Lindgren <tony@atomide.com> wrote on Fri, 23 Nov 2018 09:03:33
-0800:
> * Boris Brezillon <boris.brezillon@bootlin.com> [181121 14:56]:
> > On Wed, 21 Nov 2018 12:08:02 +0100
> > Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> >
> > > Finalize implementation of the idea suggested by Artem Bityutskiy and
> > > Tony Lindgren, described in commit b027274d2e3a ("mtd: ams-delta: fix
> > > request_mem_region() failure"). Use pure GPIO consumer API, as reqested
> > > by Boris Brezillon.
> > >
> > > Janusz Krzysztofik (4):
> > > ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
> > > mtd: rawnand: ams-delta: Request data port GPIO resource
> > > mtd: rawnand: ams-delta: Use GPIO API for data I/O
> > > ARM: OMAP1: ams-delta: Drop obsolete NAND resources
> > >
> > > arch/arm/mach-omap1/board-ams-delta.c | 22 ++----
> > > drivers/mtd/nand/raw/ams-delta.c | 120 +++++++++++++++++++---------------
> > > 2 files changed, 80 insertions(+), 62 deletions(-)
> > >
> > > Performance on Amstrad Delta is now acceptable after recent extensions
> > > to GPIO API and rawnanad enhancements.
> > >
> > > Series intented to be merged via mtd tree, should not conflict with
> > > other OMAP1 patches submitted for 4.21.
> >
> > We'll prepare an immutable tag, just in case.
>
> Sounds good to me thanks:
>
> Acked-by: Tony Lindgren <tony@atomide.com>
Actually, I can't setup a topic branch with this series because it
depends on other changes in nand/next. If a conflict happens in -next,
I suppose we will have to send you a PR.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 93+ messages in thread
* Re: Subject: [PATCH v4 0/4] mtd: rawnand: ams-delta: Use GPIO API for data I/O
2018-12-07 8:04 ` Miquel Raynal
@ 2018-12-07 16:10 ` Tony Lindgren
0 siblings, 0 replies; 93+ messages in thread
From: Tony Lindgren @ 2018-12-07 16:10 UTC (permalink / raw)
To: Miquel Raynal
Cc: Boris Brezillon, Janusz Krzysztofik, Aaro Koskinen,
Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
Linus Walleij, linux-mtd, linux-omap, linux-arm-kernel,
linux-kernel
* Miquel Raynal <miquel.raynal@bootlin.com> [181207 08:04]:
> Hi Tony,
>
> Tony Lindgren <tony@atomide.com> wrote on Fri, 23 Nov 2018 09:03:33
> -0800:
>
> > * Boris Brezillon <boris.brezillon@bootlin.com> [181121 14:56]:
> > > On Wed, 21 Nov 2018 12:08:02 +0100
> > > Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> > >
> > > > Finalize implementation of the idea suggested by Artem Bityutskiy and
> > > > Tony Lindgren, described in commit b027274d2e3a ("mtd: ams-delta: fix
> > > > request_mem_region() failure"). Use pure GPIO consumer API, as reqested
> > > > by Boris Brezillon.
> > > >
> > > > Janusz Krzysztofik (4):
> > > > ARM: OMAP1: ams-delta: Provide GPIO lookup table for NAND data port
> > > > mtd: rawnand: ams-delta: Request data port GPIO resource
> > > > mtd: rawnand: ams-delta: Use GPIO API for data I/O
> > > > ARM: OMAP1: ams-delta: Drop obsolete NAND resources
> > > >
> > > > arch/arm/mach-omap1/board-ams-delta.c | 22 ++----
> > > > drivers/mtd/nand/raw/ams-delta.c | 120 +++++++++++++++++++---------------
> > > > 2 files changed, 80 insertions(+), 62 deletions(-)
> > > >
> > > > Performance on Amstrad Delta is now acceptable after recent extensions
> > > > to GPIO API and rawnanad enhancements.
> > > >
> > > > Series intented to be merged via mtd tree, should not conflict with
> > > > other OMAP1 patches submitted for 4.21.
> > >
> > > We'll prepare an immutable tag, just in case.
> >
> > Sounds good to me thanks:
> >
> > Acked-by: Tony Lindgren <tony@atomide.com>
>
> Actually, I can't setup a topic branch with this series because it
> depends on other changes in nand/next. If a conflict happens in -next,
> I suppose we will have to send you a PR.
Hmm OK I have pretty much all the stuff applied and in next
already so if it's not conflicting now it should not conflict.
Regards,
Tony
^ permalink raw reply [flat|nested] 93+ messages in thread