[REGRESSION BUG] at91 is broken on barebox

[REGRESSION BUG] at91 is broken on barebox

[Jean-Christophe PLAGNIOL-VILLARD]

HI,

	since the switch to ERR_PTR & co

	for dev_get_resource_by_name & others the at91 is broken

	as on barebox we use a 1:1 mapping and the check of the IS_ERR is
	((unsigned long)-MAX_ERRNO)

	but on at91 the PIT and so on are at 0xfffffd30 so on always seen as
	an error

	so can we revert the commit on 

	ommit 92cc1b1f9e3363046583962859a3a8c054f4b94d

	& co

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [REGRESSION BUG] at91 is broken on barebox

[Sascha Hauer]

On Fri, Dec 26, 2014 at 10:51:43AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> HI,
> 
> 	since the switch to ERR_PTR & co
> 
> 	for dev_get_resource_by_name & others the at91 is broken
> 
> 	as on barebox we use a 1:1 mapping and the check of the IS_ERR is
> 	((unsigned long)-MAX_ERRNO)
> 
> 	but on at91 the PIT and so on are at 0xfffffd30 so on always seen as
> 	an error

Damn :(

Since dev_get_resource_by_name returns a struct resource pointer it
shouldn't be broken, right?  What's broken should be:

dev_get_mem_region_by_name
dev_request_mem_region_by_name
dev_request_mem_region

> 
> 	so can we revert the commit on 
> 
> 	ommit 92cc1b1f9e3363046583962859a3a8c054f4b94d

Since these commits touch so many files I'm not sure we can simply
revert these commits. All drivers that check for IS_ERR() from the above
functions that were introduced after the commits to revert would have to
be changed aswell.

The functions should still work, but the error check with IS_ERR() does
not, right? So as a stop gap solution we could drop the error check from
the affected drivers and as the next step change the prototype of the
affected functions to something like:

int dev_request_mem_region(struct device_d *dev, int num, void __iomem **base);

Which drivers are affected anyway?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [REGRESSION BUG] at91 is broken on barebox

[Simon Aittamaa]

Since I also ran into this issue just before the holidays, I thought
I'd provide my 0.02$.

On 5 January 2015 at 11:23, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Fri, Dec 26, 2014 at 10:51:43AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> HI,
>>
>>       since the switch to ERR_PTR & co
>>
>>       for dev_get_resource_by_name & others the at91 is broken
>>
>>       as on barebox we use a 1:1 mapping and the check of the IS_ERR is
>>       ((unsigned long)-MAX_ERRNO)
>>
>>       but on at91 the PIT and so on are at 0xfffffd30 so on always seen as
>>       an error
>
> Damn :(
>
> Since dev_get_resource_by_name returns a struct resource pointer it
> shouldn't be broken, right?  What's broken should be:
>
> dev_get_mem_region_by_name
> dev_request_mem_region_by_name
> dev_request_mem_region
>
>
>
Correct, these function returns the 'start' field of the struct
resource, which contains the offending address.

>>
>>       so can we revert the commit on
>>
>>       ommit 92cc1b1f9e3363046583962859a3a8c054f4b94d
>
> Since these commits touch so many files I'm not sure we can simply
> revert these commits. All drivers that check for IS_ERR() from the above
> functions that were introduced after the commits to revert would have to
> be changed aswell.
>
> The functions should still work, but the error check with IS_ERR() does
> not, right? So as a stop gap solution we could drop the error check from
> the affected drivers and as the next step change the prototype of the
> affected functions to something like:
>
> int dev_request_mem_region(struct device_d *dev, int num, void __iomem **base);
>
Sounds like the only sane way, and that is also the point when I
realized that I had to put the fix on my TODO-list and live with the
workaround for now, since I have a few show-stopping issues that I
have to deal with first :-(

> Which drivers are affected anyway?
>
I've had to remove the IS_ERR() check in:
arch/arm/mach-at91/at91sam926x_time.c
drivers/pinctrl/at91-pinctrl.c
drivers/serial/atmel.c

There might be more drivers affected, but the list of drivers to check
shouldn't be that long, see the memory map on page 15 in the reference
manual[1]. Since it checks the base-address the AIC (0xfffff000)
should be fine, but the rest (PIOA, PIOB, ...) could incorrectly fail.

Best,
Simon Aittamaa

[1]: http://www.atmel.com/Images/doc6221.pdf

> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 0/2] FIX AT91 support

[Jean-Christophe PLAGNIOL-VILLARD]

Hi,

	This patch series fix at91 support that is broken for more than 3 months

	please apply this on fix releases

	Not even tested by Atmel when adding now SoC and Board

The following changes since commit 631be8e6cbe003f469a7e6b54046a743b710d989:

  blspec: print error when devicetree not found (2014-12-16 14:29:23 +0100)

are available in the git repository at:

  git://git.jcrosoft.org/barebox.git delivery/at91_fix

for you to fetch changes up to 8f48b25818d29d406b3eba170910c6c24bd3a347:

  driver: workarroud resource request that conflist with errno PTR (2015-01-05 17:46:38 +0800)

----------------------------------------------------------------
Jean-Christophe PLAGNIOL-VILLARD (2):
      arm: at91: fix ecc_mode on non atmel boards
      driver: workarroud resource request that conflist with errno PTR

 arch/arm/boards/animeo_ip/init.c      |  1 +
 arch/arm/boards/pm9261/init.c         |  1 +
 arch/arm/boards/pm9263/init.c         |  1 +
 arch/arm/boards/pm9g45/init.c         |  1 +
 arch/arm/boards/qil-a926x/init.c      |  1 +
 arch/arm/boards/telit-evk-pro3/init.c |  2 ++
 arch/arm/boards/tny-a926x/init.c      |  1 +
 arch/arm/boards/usb-a926x/init.c      |  1 +
 arch/arm/mach-at91/at91sam926x_time.c |  6 +++---
 drivers/base/driver.c                 | 31 +++++++++++++++++++++++++++++--
 drivers/pinctrl/pinctrl-at91.c        |  6 +++---
 drivers/serial/atmel.c                |  6 +++---
 include/driver.h                      |  8 ++++++++
 13 files changed, 55 insertions(+), 11 deletions(-)

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/2] arm: at91: fix ecc_mode on non atmel boards

[Jean-Christophe PLAGNIOL-VILLARD]

since
commit 54bccadddd52cb4cbbecd1403b9091fe651bd0fd
Author: Raphaël Poggi <poggi.raph@gmail.com>

    mtd: atmel_nand: retrieve ecc_mode from pdata

break most of the non atmel AT91 boards that did not provide a ecc_mode
params in the nand pdata

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Raphaël Poggi <poggi.raph@gmail.com>
---
 arch/arm/boards/animeo_ip/init.c      | 1 +
 arch/arm/boards/pm9261/init.c         | 1 +
 arch/arm/boards/pm9263/init.c         | 1 +
 arch/arm/boards/pm9g45/init.c         | 1 +
 arch/arm/boards/qil-a926x/init.c      | 1 +
 arch/arm/boards/telit-evk-pro3/init.c | 2 ++
 arch/arm/boards/tny-a926x/init.c      | 1 +
 arch/arm/boards/usb-a926x/init.c      | 1 +
 8 files changed, 9 insertions(+)

diff --git a/arch/arm/boards/animeo_ip/init.c b/arch/arm/boards/animeo_ip/init.c
index ca64d6d..495dc18 100644
--- a/arch/arm/boards/animeo_ip/init.c
+++ b/arch/arm/boards/animeo_ip/init.c
@@ -93,6 +93,7 @@ static struct atmel_nand_data nand_pdata = {
 	.det_pin	= -EINVAL,
 	.rdy_pin	= AT91_PIN_PC13,
 	.enable_pin	= AT91_PIN_PC14,
+	.ecc_mode	= NAND_ECC_SOFT,
 	.bus_width_16	= 0,
 	.on_flash_bbt	= 1,
 };
diff --git a/arch/arm/boards/pm9261/init.c b/arch/arm/boards/pm9261/init.c
index 09d7f80..63f4f19 100644
--- a/arch/arm/boards/pm9261/init.c
+++ b/arch/arm/boards/pm9261/init.c
@@ -50,6 +50,7 @@ static struct atmel_nand_data nand_pdata = {
 	.det_pin	= -EINVAL,
 	.rdy_pin	= AT91_PIN_PA16,
 	.enable_pin	= AT91_PIN_PC14,
+	.ecc_mode	= NAND_ECC_SOFT,
 #if defined(CONFIG_MTD_NAND_ATMEL_BUSWIDTH_16)
 	.bus_width_16	= 1,
 #else
diff --git a/arch/arm/boards/pm9263/init.c b/arch/arm/boards/pm9263/init.c
index 3dca9e4..2b6b089 100644
--- a/arch/arm/boards/pm9263/init.c
+++ b/arch/arm/boards/pm9263/init.c
@@ -50,6 +50,7 @@ static struct atmel_nand_data nand_pdata = {
 	.det_pin	= -EINVAL,
 	.rdy_pin	= AT91_PIN_PB30,
 	.enable_pin	= AT91_PIN_PD15,
+	.ecc_mode	= NAND_ECC_SOFT,
 #if defined(CONFIG_MTD_NAND_ATMEL_BUSWIDTH_16)
 	.bus_width_16	= 1,
 #else
diff --git a/arch/arm/boards/pm9g45/init.c b/arch/arm/boards/pm9g45/init.c
index 1e43be5..524fe53 100644
--- a/arch/arm/boards/pm9g45/init.c
+++ b/arch/arm/boards/pm9g45/init.c
@@ -49,6 +49,7 @@ static struct atmel_nand_data nand_pdata = {
 	.det_pin	= -EINVAL,
 	.rdy_pin	= AT91_PIN_PD3,
 	.enable_pin	= AT91_PIN_PC14,
+	.ecc_mode	= NAND_ECC_SOFT,
 	.bus_width_16	= 0,
 	.on_flash_bbt	= 1,
 };
diff --git a/arch/arm/boards/qil-a926x/init.c b/arch/arm/boards/qil-a926x/init.c
index 99a9604..4f01e2d 100644
--- a/arch/arm/boards/qil-a926x/init.c
+++ b/arch/arm/boards/qil-a926x/init.c
@@ -43,6 +43,7 @@ static struct atmel_nand_data nand_pdata = {
 	.det_pin	= -EINVAL,
 	.rdy_pin	= AT91_PIN_PC13,
 	.enable_pin	= AT91_PIN_PC14,
+	.ecc_mode	= NAND_ECC_SOFT,
 	.on_flash_bbt	= 1,
 };
 
diff --git a/arch/arm/boards/telit-evk-pro3/init.c b/arch/arm/boards/telit-evk-pro3/init.c
index e094ea7..ea63b1a 100644
--- a/arch/arm/boards/telit-evk-pro3/init.c
+++ b/arch/arm/boards/telit-evk-pro3/init.c
@@ -18,6 +18,7 @@
 #include <gpio.h>
 #include <init.h>
 #include <linux/clk.h>
+#include <linux/mtd/nand.h>
 #include <mach/at91_rstc.h>
 #include <mach/at91sam9_smc.h>
 #include <mach/board.h>
@@ -33,6 +34,7 @@ static struct atmel_nand_data nand_pdata = {
 	.det_pin	= -EINVAL,
 	.rdy_pin	= AT91_PIN_PC13,
 	.enable_pin	= AT91_PIN_PC14,
+	.ecc_mode	= NAND_ECC_SOFT,
 	.on_flash_bbt	= 1,
 };
 
diff --git a/arch/arm/boards/tny-a926x/init.c b/arch/arm/boards/tny-a926x/init.c
index ac86457..256f8ba 100644
--- a/arch/arm/boards/tny-a926x/init.c
+++ b/arch/arm/boards/tny-a926x/init.c
@@ -55,6 +55,7 @@ static struct atmel_nand_data nand_pdata = {
 	.det_pin	= -EINVAL,
 	.rdy_pin	= AT91_PIN_PC13,
 	.enable_pin	= AT91_PIN_PC14,
+	.ecc_mode	= NAND_ECC_SOFT,
 	.on_flash_bbt	= 1,
 };
 
diff --git a/arch/arm/boards/usb-a926x/init.c b/arch/arm/boards/usb-a926x/init.c
index 4a28af0..f0e1157 100644
--- a/arch/arm/boards/usb-a926x/init.c
+++ b/arch/arm/boards/usb-a926x/init.c
@@ -59,6 +59,7 @@ static struct atmel_nand_data nand_pdata = {
 	.det_pin	= -EINVAL,
 	.rdy_pin	= AT91_PIN_PC13,
 	.enable_pin	= AT91_PIN_PC14,
+	.ecc_mode	= NAND_ECC_SOFT,
 	.on_flash_bbt	= 1,
 };
 
-- 
2.1.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] driver: workarroud resource request that conflist with errno PTR

[Jean-Christophe PLAGNIOL-VILLARD]

broken since

commit ed6e965824303255cacc1c1a195d3684caa26bce
Author:     Sascha Hauer <s.hauer@pengutronix.de>
resource: Let dev_request_mem_region return an error pointer

Introduce dev_request_mem_region_err_null

only used on platform like at91 where the Ressource address collision with

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/arm/mach-at91/at91sam926x_time.c |  6 +++---
 drivers/base/driver.c                 | 31 +++++++++++++++++++++++++++++--
 drivers/pinctrl/pinctrl-at91.c        |  6 +++---
 drivers/serial/atmel.c                |  6 +++---
 include/driver.h                      |  8 ++++++++
 5 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-at91/at91sam926x_time.c b/arch/arm/mach-at91/at91sam926x_time.c
index 789e1ec..cc7ad2f 100644
--- a/arch/arm/mach-at91/at91sam926x_time.c
+++ b/arch/arm/mach-at91/at91sam926x_time.c
@@ -89,9 +89,9 @@ static int at91_pit_probe(struct device_d *dev)
 		return ret;
 	}
 
-	pit_base = dev_request_mem_region(dev, 0);
-	if (IS_ERR(pit_base))
-		return PTR_ERR(pit_base);
+	pit_base = dev_request_mem_region_err_null(dev, 0);
+	if (!pit_base)
+		return -ENOENT;
 
 	pit_rate = clk_get_rate(clk) / 16;
 
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index e0125a1..2375004 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -242,8 +242,8 @@ int register_driver(struct driver_d *drv)
 }
 EXPORT_SYMBOL(register_driver);
 
-struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
-				  int num)
+static struct resource *__dev_get_resource(struct device_d *dev,
+		unsigned long type, int num)
 {
 	int i, n = 0;
 
@@ -256,6 +256,17 @@ struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
 		}
 	}
 
+	return NULL;
+}
+
+struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
+				  int num)
+{
+	struct resource *res = __dev_get_resource(dev, type, num);
+
+	if (res)
+		return res;
+
 	return ERR_PTR(-ENOENT);
 }
 
@@ -318,6 +329,22 @@ void __iomem *dev_request_mem_region_by_name(struct device_d *dev, const char *n
 }
 EXPORT_SYMBOL(dev_request_mem_region_by_name);
 
+void __iomem *dev_request_mem_region_err_null(struct device_d *dev, int num)
+{
+	struct resource *res;
+
+	res = __dev_get_resource(dev, IORESOURCE_MEM, num);
+	if (!res)
+		return res;
+
+	res = request_iomem_region(dev_name(dev), res->start, res->end);
+	if (IS_ERR(res))
+		return ERR_CAST(res);
+
+	return (void __force __iomem *)res->start;
+}
+EXPORT_SYMBOL(dev_request_mem_region_err_null);
+
 void __iomem *dev_request_mem_region(struct device_d *dev, int num)
 {
 	struct resource *res;
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 0d6d2e7..b803f89 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -652,9 +652,9 @@ static int at91_gpio_probe(struct device_d *dev)
 	}
 
 	gpio_banks = max(gpio_banks, alias_idx + 1);
-	at91_gpio->regbase = dev_request_mem_region(dev, 0);
-	if (IS_ERR(at91_gpio->regbase))
-		return PTR_ERR(at91_gpio->regbase);
+	at91_gpio->regbase = dev_request_mem_region_err_null(dev, 0);
+	if (!at91_gpio->regbase)
+		return -ENOENT;
 
 	at91_gpio->chip.ops = &at91_gpio_ops;
 	at91_gpio->chip.ngpio = MAX_NB_GPIO_PER_BANK;
diff --git a/drivers/serial/atmel.c b/drivers/serial/atmel.c
index 1f40692..4e4624e 100644
--- a/drivers/serial/atmel.c
+++ b/drivers/serial/atmel.c
@@ -398,9 +398,9 @@ static int atmel_serial_init_port(struct console_device *cdev)
 	struct device_d *dev = cdev->dev;
 	struct atmel_uart_port *uart = to_atmel_uart_port(cdev);
 
-	uart->base = dev_request_mem_region(dev, 0);
-	if (IS_ERR(uart->base))
-		return PTR_ERR(uart->base);
+	uart->base = dev_request_mem_region_err_null(dev, 0);
+	if (!uart->base)
+		return -ENOENT;
 
 	uart->clk = clk_get(dev, "usart");
 	clk_enable(uart->clk);
diff --git a/include/driver.h b/include/driver.h
index 53e1000..46aae4f 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -206,6 +206,14 @@ void *dev_get_mem_region(struct device_d *dev, int num);
  */
 void __iomem *dev_request_mem_region(struct device_d *dev, int num);
 
+/*
+ * exlusively request register base 'num' for a device
+ * will return NULL on error
+ * only used on platform like at91 where the Ressource address collision with
+ * PTR errno
+ */
+void __iomem *dev_request_mem_region_err_null(struct device_d *dev, int num);
+
 struct device_d *device_alloc(const char *devname, int id);
 
 int device_add_resources(struct device_d *dev, const struct resource *res, int num);
-- 
2.1.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] arm: at91: fix ecc_mode on non atmel boards

[Raphaël Poggi]

Good catch, I ACK this patch.

2015-01-06 12:37 GMT+01:00 Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com>:
> since
> commit 54bccadddd52cb4cbbecd1403b9091fe651bd0fd
> Author: Raphaël Poggi <poggi.raph@gmail.com>
>
>     mtd: atmel_nand: retrieve ecc_mode from pdata
>
> break most of the non atmel AT91 boards that did not provide a ecc_mode
> params in the nand pdata
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Raphaël Poggi <poggi.raph@gmail.com>
> ---
>  arch/arm/boards/animeo_ip/init.c      | 1 +
>  arch/arm/boards/pm9261/init.c         | 1 +
>  arch/arm/boards/pm9263/init.c         | 1 +
>  arch/arm/boards/pm9g45/init.c         | 1 +
>  arch/arm/boards/qil-a926x/init.c      | 1 +
>  arch/arm/boards/telit-evk-pro3/init.c | 2 ++
>  arch/arm/boards/tny-a926x/init.c      | 1 +
>  arch/arm/boards/usb-a926x/init.c      | 1 +
>  8 files changed, 9 insertions(+)
>
> diff --git a/arch/arm/boards/animeo_ip/init.c b/arch/arm/boards/animeo_ip/init.c
> index ca64d6d..495dc18 100644
> --- a/arch/arm/boards/animeo_ip/init.c
> +++ b/arch/arm/boards/animeo_ip/init.c
> @@ -93,6 +93,7 @@ static struct atmel_nand_data nand_pdata = {
>         .det_pin        = -EINVAL,
>         .rdy_pin        = AT91_PIN_PC13,
>         .enable_pin     = AT91_PIN_PC14,
> +       .ecc_mode       = NAND_ECC_SOFT,
>         .bus_width_16   = 0,
>         .on_flash_bbt   = 1,
>  };
> diff --git a/arch/arm/boards/pm9261/init.c b/arch/arm/boards/pm9261/init.c
> index 09d7f80..63f4f19 100644
> --- a/arch/arm/boards/pm9261/init.c
> +++ b/arch/arm/boards/pm9261/init.c
> @@ -50,6 +50,7 @@ static struct atmel_nand_data nand_pdata = {
>         .det_pin        = -EINVAL,
>         .rdy_pin        = AT91_PIN_PA16,
>         .enable_pin     = AT91_PIN_PC14,
> +       .ecc_mode       = NAND_ECC_SOFT,
>  #if defined(CONFIG_MTD_NAND_ATMEL_BUSWIDTH_16)
>         .bus_width_16   = 1,
>  #else
> diff --git a/arch/arm/boards/pm9263/init.c b/arch/arm/boards/pm9263/init.c
> index 3dca9e4..2b6b089 100644
> --- a/arch/arm/boards/pm9263/init.c
> +++ b/arch/arm/boards/pm9263/init.c
> @@ -50,6 +50,7 @@ static struct atmel_nand_data nand_pdata = {
>         .det_pin        = -EINVAL,
>         .rdy_pin        = AT91_PIN_PB30,
>         .enable_pin     = AT91_PIN_PD15,
> +       .ecc_mode       = NAND_ECC_SOFT,
>  #if defined(CONFIG_MTD_NAND_ATMEL_BUSWIDTH_16)
>         .bus_width_16   = 1,
>  #else
> diff --git a/arch/arm/boards/pm9g45/init.c b/arch/arm/boards/pm9g45/init.c
> index 1e43be5..524fe53 100644
> --- a/arch/arm/boards/pm9g45/init.c
> +++ b/arch/arm/boards/pm9g45/init.c
> @@ -49,6 +49,7 @@ static struct atmel_nand_data nand_pdata = {
>         .det_pin        = -EINVAL,
>         .rdy_pin        = AT91_PIN_PD3,
>         .enable_pin     = AT91_PIN_PC14,
> +       .ecc_mode       = NAND_ECC_SOFT,
>         .bus_width_16   = 0,
>         .on_flash_bbt   = 1,
>  };
> diff --git a/arch/arm/boards/qil-a926x/init.c b/arch/arm/boards/qil-a926x/init.c
> index 99a9604..4f01e2d 100644
> --- a/arch/arm/boards/qil-a926x/init.c
> +++ b/arch/arm/boards/qil-a926x/init.c
> @@ -43,6 +43,7 @@ static struct atmel_nand_data nand_pdata = {
>         .det_pin        = -EINVAL,
>         .rdy_pin        = AT91_PIN_PC13,
>         .enable_pin     = AT91_PIN_PC14,
> +       .ecc_mode       = NAND_ECC_SOFT,
>         .on_flash_bbt   = 1,
>  };
>
> diff --git a/arch/arm/boards/telit-evk-pro3/init.c b/arch/arm/boards/telit-evk-pro3/init.c
> index e094ea7..ea63b1a 100644
> --- a/arch/arm/boards/telit-evk-pro3/init.c
> +++ b/arch/arm/boards/telit-evk-pro3/init.c
> @@ -18,6 +18,7 @@
>  #include <gpio.h>
>  #include <init.h>
>  #include <linux/clk.h>
> +#include <linux/mtd/nand.h>
>  #include <mach/at91_rstc.h>
>  #include <mach/at91sam9_smc.h>
>  #include <mach/board.h>
> @@ -33,6 +34,7 @@ static struct atmel_nand_data nand_pdata = {
>         .det_pin        = -EINVAL,
>         .rdy_pin        = AT91_PIN_PC13,
>         .enable_pin     = AT91_PIN_PC14,
> +       .ecc_mode       = NAND_ECC_SOFT,
>         .on_flash_bbt   = 1,
>  };
>
> diff --git a/arch/arm/boards/tny-a926x/init.c b/arch/arm/boards/tny-a926x/init.c
> index ac86457..256f8ba 100644
> --- a/arch/arm/boards/tny-a926x/init.c
> +++ b/arch/arm/boards/tny-a926x/init.c
> @@ -55,6 +55,7 @@ static struct atmel_nand_data nand_pdata = {
>         .det_pin        = -EINVAL,
>         .rdy_pin        = AT91_PIN_PC13,
>         .enable_pin     = AT91_PIN_PC14,
> +       .ecc_mode       = NAND_ECC_SOFT,
>         .on_flash_bbt   = 1,
>  };
>
> diff --git a/arch/arm/boards/usb-a926x/init.c b/arch/arm/boards/usb-a926x/init.c
> index 4a28af0..f0e1157 100644
> --- a/arch/arm/boards/usb-a926x/init.c
> +++ b/arch/arm/boards/usb-a926x/init.c
> @@ -59,6 +59,7 @@ static struct atmel_nand_data nand_pdata = {
>         .det_pin        = -EINVAL,
>         .rdy_pin        = AT91_PIN_PC13,
>         .enable_pin     = AT91_PIN_PC14,
> +       .ecc_mode       = NAND_ECC_SOFT,
>         .on_flash_bbt   = 1,
>  };
>
> --
> 2.1.3
>
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] driver: workarroud resource request that conflist with errno PTR

[Sascha Hauer]

On Tue, Jan 06, 2015 at 12:37:06PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> broken since
> 
> commit ed6e965824303255cacc1c1a195d3684caa26bce
> Author:     Sascha Hauer <s.hauer@pengutronix.de>
> resource: Let dev_request_mem_region return an error pointer
> 
> Introduce dev_request_mem_region_err_null
> 
> only used on platform like at91 where the Ressource address collision with
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  arch/arm/mach-at91/at91sam926x_time.c |  6 +++---
>  drivers/base/driver.c                 | 31 +++++++++++++++++++++++++++++--
>  drivers/pinctrl/pinctrl-at91.c        |  6 +++---
>  drivers/serial/atmel.c                |  6 +++---
>  include/driver.h                      |  8 ++++++++
>  5 files changed, 46 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/at91sam926x_time.c b/arch/arm/mach-at91/at91sam926x_time.c
> index 789e1ec..cc7ad2f 100644
> --- a/arch/arm/mach-at91/at91sam926x_time.c
> +++ b/arch/arm/mach-at91/at91sam926x_time.c
> @@ -89,9 +89,9 @@ static int at91_pit_probe(struct device_d *dev)
>  		return ret;
>  	}
>  
> -	pit_base = dev_request_mem_region(dev, 0);
> -	if (IS_ERR(pit_base))
> -		return PTR_ERR(pit_base);
> +	pit_base = dev_request_mem_region_err_null(dev, 0);
> +	if (!pit_base)
> +		return -ENOENT;
>  
>  	pit_rate = clk_get_rate(clk) / 16;
>  
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index e0125a1..2375004 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -242,8 +242,8 @@ int register_driver(struct driver_d *drv)
>  }
>  EXPORT_SYMBOL(register_driver);
>  
> -struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
> -				  int num)
> +static struct resource *__dev_get_resource(struct device_d *dev,
> +		unsigned long type, int num)
>  {
>  	int i, n = 0;
>  
> @@ -256,6 +256,17 @@ struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
>  		}
>  	}
>  
> +	return NULL;
> +}
> +
> +struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
> +				  int num)
> +{
> +	struct resource *res = __dev_get_resource(dev, type, num);
> +
> +	if (res)
> +		return res;
> +
>  	return ERR_PTR(-ENOENT);
>  }

No need to change dev_get_resource(), it returns a struct resource *
which will never conflict with an err ptr.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] arm: at91: fix ecc_mode on non atmel boards

[Sascha Hauer]

On Tue, Jan 06, 2015 at 12:37:05PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> since
> commit 54bccadddd52cb4cbbecd1403b9091fe651bd0fd
> Author: Raphaël Poggi <poggi.raph@gmail.com>
> 
>     mtd: atmel_nand: retrieve ecc_mode from pdata
> 
> break most of the non atmel AT91 boards that did not provide a ecc_mode
> params in the nand pdata
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Raphaël Poggi <poggi.raph@gmail.com>

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] driver: workarroud resource request that conflist with errno PTR

[Jean-Christophe PLAGNIOL-VILLARD]

> On Jan 6, 2015, at 8:44 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> On Tue, Jan 06, 2015 at 12:37:06PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> broken since
>> 
>> commit ed6e965824303255cacc1c1a195d3684caa26bce
>> Author:     Sascha Hauer <s.hauer@pengutronix.de>
>> resource: Let dev_request_mem_region return an error pointer
>> 
>> Introduce dev_request_mem_region_err_null
>> 
>> only used on platform like at91 where the Ressource address collision with
>> 
>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>> ---
>> arch/arm/mach-at91/at91sam926x_time.c |  6 +++---
>> drivers/base/driver.c                 | 31 +++++++++++++++++++++++++++++--
>> drivers/pinctrl/pinctrl-at91.c        |  6 +++---
>> drivers/serial/atmel.c                |  6 +++---
>> include/driver.h                      |  8 ++++++++
>> 5 files changed, 46 insertions(+), 11 deletions(-)
>> 
>> diff --git a/arch/arm/mach-at91/at91sam926x_time.c b/arch/arm/mach-at91/at91sam926x_time.c
>> index 789e1ec..cc7ad2f 100644
>> --- a/arch/arm/mach-at91/at91sam926x_time.c
>> +++ b/arch/arm/mach-at91/at91sam926x_time.c
>> @@ -89,9 +89,9 @@ static int at91_pit_probe(struct device_d *dev)
>> 		return ret;
>> 	}
>> 
>> -	pit_base = dev_request_mem_region(dev, 0);
>> -	if (IS_ERR(pit_base))
>> -		return PTR_ERR(pit_base);
>> +	pit_base = dev_request_mem_region_err_null(dev, 0);
>> +	if (!pit_base)
>> +		return -ENOENT;
>> 
>> 	pit_rate = clk_get_rate(clk) / 16;
>> 
>> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
>> index e0125a1..2375004 100644
>> --- a/drivers/base/driver.c
>> +++ b/drivers/base/driver.c
>> @@ -242,8 +242,8 @@ int register_driver(struct driver_d *drv)
>> }
>> EXPORT_SYMBOL(register_driver);
>> 
>> -struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
>> -				  int num)
>> +static struct resource *__dev_get_resource(struct device_d *dev,
>> +		unsigned long type, int num)
>> {
>> 	int i, n = 0;
>> 
>> @@ -256,6 +256,17 @@ struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
>> 		}
>> 	}
>> 
>> +	return NULL;
>> +}
>> +
>> +struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
>> +				  int num)
>> +{
>> +	struct resource *res = __dev_get_resource(dev, type, num);
>> +
>> +	if (res)
>> +		return res;
>> +
>> 	return ERR_PTR(-ENOENT);
>> }
> 
> No need to change dev_get_resource(), it returns a struct resource *
> which will never conflict with an err ptr.

but as if no ressource is found it return ERR_PTR(-ENOENT)

so it does conflict

Best Regards,
J.
> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] driver: workarroud resource request that conflist with errno PTR

[Sascha Hauer]

On Wed, Jan 07, 2015 at 11:14:32AM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> > On Jan 6, 2015, at 8:44 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 
> > On Tue, Jan 06, 2015 at 12:37:06PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >> broken since
> >> 
> >> commit ed6e965824303255cacc1c1a195d3684caa26bce
> >> Author:     Sascha Hauer <s.hauer@pengutronix.de>
> >> resource: Let dev_request_mem_region return an error pointer
> >> 
> >> Introduce dev_request_mem_region_err_null
> >> 
> >> only used on platform like at91 where the Ressource address collision with
> >> 
> >> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> >> ---
> >> arch/arm/mach-at91/at91sam926x_time.c |  6 +++---
> >> drivers/base/driver.c                 | 31 +++++++++++++++++++++++++++++--
> >> drivers/pinctrl/pinctrl-at91.c        |  6 +++---
> >> drivers/serial/atmel.c                |  6 +++---
> >> include/driver.h                      |  8 ++++++++
> >> 5 files changed, 46 insertions(+), 11 deletions(-)
> >> 
> >> diff --git a/arch/arm/mach-at91/at91sam926x_time.c b/arch/arm/mach-at91/at91sam926x_time.c
> >> index 789e1ec..cc7ad2f 100644
> >> --- a/arch/arm/mach-at91/at91sam926x_time.c
> >> +++ b/arch/arm/mach-at91/at91sam926x_time.c
> >> @@ -89,9 +89,9 @@ static int at91_pit_probe(struct device_d *dev)
> >> 		return ret;
> >> 	}
> >> 
> >> -	pit_base = dev_request_mem_region(dev, 0);
> >> -	if (IS_ERR(pit_base))
> >> -		return PTR_ERR(pit_base);
> >> +	pit_base = dev_request_mem_region_err_null(dev, 0);
> >> +	if (!pit_base)
> >> +		return -ENOENT;
> >> 
> >> 	pit_rate = clk_get_rate(clk) / 16;
> >> 
> >> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> >> index e0125a1..2375004 100644
> >> --- a/drivers/base/driver.c
> >> +++ b/drivers/base/driver.c
> >> @@ -242,8 +242,8 @@ int register_driver(struct driver_d *drv)
> >> }
> >> EXPORT_SYMBOL(register_driver);
> >> 
> >> -struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
> >> -				  int num)
> >> +static struct resource *__dev_get_resource(struct device_d *dev,
> >> +		unsigned long type, int num)
> >> {
> >> 	int i, n = 0;
> >> 
> >> @@ -256,6 +256,17 @@ struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
> >> 		}
> >> 	}
> >> 
> >> +	return NULL;
> >> +}
> >> +
> >> +struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
> >> +				  int num)
> >> +{
> >> +	struct resource *res = __dev_get_resource(dev, type, num);
> >> +
> >> +	if (res)
> >> +		return res;
> >> +
> >> 	return ERR_PTR(-ENOENT);
> >> }
> > 
> > No need to change dev_get_resource(), it returns a struct resource *
> > which will never conflict with an err ptr.
> 
> but as if no ressource is found it return ERR_PTR(-ENOENT)

Yes, and this is always correct. Only your
dev_request_mem_region_err_null() function must return NULL in this case
and not ERR_CAST(res) like dev_request_mem_region() does.

void __iomem *dev_request_mem_region_err_null(struct device_d *dev, int num)
{
       struct resource *res;

       res = dev_get_resource(dev, IORESOURCE_MEM, num);
       if (IS_ERR(res))
               return NULL;

       res = request_iomem_region(dev_name(dev), res->start, res->end);
       if (IS_ERR(res))
               return ERR_CAST(res);

       return (void __force __iomem *)res->start;
}
EXPORT_SYMBOL(dev_request_mem_region_err_null);

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] driver: workarroud resource request that conflist with errno PTR

[Sascha Hauer]

On Tue, Jan 06, 2015 at 12:37:06PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> broken since
> 
> commit ed6e965824303255cacc1c1a195d3684caa26bce
> Author:     Sascha Hauer <s.hauer@pengutronix.de>
> resource: Let dev_request_mem_region return an error pointer
> 
> Introduce dev_request_mem_region_err_null
> 
> only used on platform like at91 where the Ressource address collision with
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  arch/arm/mach-at91/at91sam926x_time.c |  6 +++---
>  drivers/base/driver.c                 | 31 +++++++++++++++++++++++++++++--
>  drivers/pinctrl/pinctrl-at91.c        |  6 +++---
>  drivers/serial/atmel.c                |  6 +++---
>  include/driver.h                      |  8 ++++++++
>  5 files changed, 46 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/at91sam926x_time.c b/arch/arm/mach-at91/at91sam926x_time.c
> index 789e1ec..cc7ad2f 100644
> --- a/arch/arm/mach-at91/at91sam926x_time.c
> +++ b/arch/arm/mach-at91/at91sam926x_time.c
> @@ -89,9 +89,9 @@ static int at91_pit_probe(struct device_d *dev)
>  		return ret;
>  	}
>  
> -	pit_base = dev_request_mem_region(dev, 0);
> -	if (IS_ERR(pit_base))
> -		return PTR_ERR(pit_base);
> +	pit_base = dev_request_mem_region_err_null(dev, 0);
> +	if (!pit_base)
> +		return -ENOENT;
>  
>  	pit_rate = clk_get_rate(clk) / 16;
>  
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index e0125a1..2375004 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -242,8 +242,8 @@ int register_driver(struct driver_d *drv)
>  }
>  EXPORT_SYMBOL(register_driver);
>  
> -struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
> -				  int num)
> +static struct resource *__dev_get_resource(struct device_d *dev,
> +		unsigned long type, int num)
>  {
>  	int i, n = 0;
>  
> @@ -256,6 +256,17 @@ struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
>  		}
>  	}
>  
> +	return NULL;
> +}
> +
> +struct resource *dev_get_resource(struct device_d *dev, unsigned long type,
> +				  int num)
> +{
> +	struct resource *res = __dev_get_resource(dev, type, num);
> +
> +	if (res)
> +		return res;
> +
>  	return ERR_PTR(-ENOENT);
>  }
>  
> @@ -318,6 +329,22 @@ void __iomem *dev_request_mem_region_by_name(struct device_d *dev, const char *n
>  }
>  EXPORT_SYMBOL(dev_request_mem_region_by_name);
>  
> +void __iomem *dev_request_mem_region_err_null(struct device_d *dev, int num)
> +{
> +	struct resource *res;
> +
> +	res = __dev_get_resource(dev, IORESOURCE_MEM, num);
> +	if (!res)
> +		return res;
> +
> +	res = request_iomem_region(dev_name(dev), res->start, res->end);
> +	if (IS_ERR(res))
> +		return ERR_CAST(res);

The function promises to return NULL on error, so this is wrong here.
I just sent an updated patch so that the release doesn't get delayed any
more.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox