[PATCH v2 0/3] mtd: nand: Rework/cleanup the Atmel NAND driver

[PATCH v2 0/3] mtd: nand: Rework/cleanup the Atmel NAND driver

[Boris Brezillon]

This is a complete rewrite of the driver whose main purpose is to
support the new DT representation where the NAND controller node is now
really visible in the DT and appears under the EBI bus. With this new
representation, we can add other devices under the EBI bus without
risking pinmuxing conflicts (the NAND controller is under the EBI
bus logic and as such, share some of its pins with other devices
connected on this bus).

Even though the goal of this rework was not necessarily to add new
features, the new driver has been designed with this in mind. With a
clearer separation between the different blocks and different IP
revisions, adding new functionalities should be easier (we already
have plans to support SMC timing configuration so that we no longer
have to rely on the configuration done by the bootloader/bootstrap).

Also note that we no longer have a custom ->cmdfunc() implementation,
which means we can now benefit from new features added in the core
implementation for free (support for new NAND operations for example).

The last thing that we gain with this rework is support for multi-chips
and multi-dies chips, thanks to the clean NAND controller <-> NAND
devices representation.

This new driver has been tested on several platforms (at91sam9261,
at91sam9g45, at91sam9x5, sama5d3 and sama5d4) to make sure it did not
introduce regressions, and it's worth mentioning that old bindings are
still supported (which partly explain the positive diffstat).

Regards,

Boris

Changes since v1:
- change function/structure prefixes (asked by Nicolas)
- drop applied patches
- use new GPIO helpers
- set ->chip_delay to 40 as done in the old driver (reported by Nicolas)
- rework read_page to improve perfs
- add a better commit message to patch 2

Boris Brezillon (3):
  mtd: nand: Cleanup/rework the atmel_nand driver
  mtd: nand: atmel: Document the new DT bindings
  mtd: nand: Remove unused chip->write_page() hook

 .../devicetree/bindings/mtd/atmel-nand.txt         |  107 +-
 MAINTAINERS                                        |    2 +-
 drivers/mtd/nand/Makefile                          |    2 +-
 drivers/mtd/nand/atmel/Makefile                    |    4 +
 drivers/mtd/nand/atmel/nand-controller.c           | 2269 ++++++++++++++++++
 drivers/mtd/nand/atmel/pmecc.c                     | 1020 ++++++++
 drivers/mtd/nand/atmel/pmecc.h                     |   73 +
 drivers/mtd/nand/atmel_nand.c                      | 2479 --------------------
 drivers/mtd/nand/atmel_nand_ecc.h                  |  163 --
 drivers/mtd/nand/atmel_nand_nfc.h                  |  103 -
 drivers/mtd/nand/nand_base.c                       |   10 +-
 include/linux/mtd/nand.h                           |    4 -
 12 files changed, 3478 insertions(+), 2758 deletions(-)
 create mode 100644 drivers/mtd/nand/atmel/Makefile
 create mode 100644 drivers/mtd/nand/atmel/nand-controller.c
 create mode 100644 drivers/mtd/nand/atmel/pmecc.c
 create mode 100644 drivers/mtd/nand/atmel/pmecc.h
 delete mode 100644 drivers/mtd/nand/atmel_nand.c
 delete mode 100644 drivers/mtd/nand/atmel_nand_ecc.h
 delete mode 100644 drivers/mtd/nand/atmel_nand_nfc.h

-- 
2.7.4

[PATCH v2 2/3] mtd: nand: atmel: Document the new DT bindings

[Boris Brezillon]

The old NAND bindings were not exactly describing the hardware topology
and were preventing definitions of several NAND chips under the same
NAND controller.

New bindings address these limitations and should be preferred over the
old ones for new SoCs/boards.
Old bindings are still supported for backward compatibility but are
marked deprecated in the doc.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 .../devicetree/bindings/mtd/atmel-nand.txt         | 107 ++++++++++++++++++++-
 1 file changed, 106 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
index 3e7ee99d3949..f6bee57e453a 100644
--- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
@@ -1,4 +1,109 @@
-Atmel NAND flash
+Atmel NAND flash controller bindings
+
+The NAND flash controller node should be defined under the EBI bus (see
+Documentation/devicetree/bindings/memory-controllers/atmel,ebi.txt).
+One or several NAND devices can be defined under this NAND controller.
+The NAND controller might be connected to an ECC engine.
+
+* NAND controller bindings:
+
+Required properties:
+- compatible: should be one of the following
+	"atmel,at91rm9200-nand-controller"
+	"atmel,at91sam9260-nand-controller"
+	"atmel,at91sam9261-nand-controller"
+	"atmel,at91sam9g45-nand-controller"
+	"atmel,sama5d3-nand-controller"
+- ranges: empty ranges property to forward EBI ranges definitions.
+- #address-cells: should be set to 2.
+- #size-cells: should be set to 1.
+- atmel,nfc-io: phandle to the NFC IO block. Only required for sama5d3
+		controllers.
+- atmel,nfc-sram: phandle to the NFC SRAM block. Only required for sama5d3
+		  controllers.
+
+Optional properties:
+- ecc-engine: phandle to the PMECC block. Only meaningful if the SoC embeds
+	      a PMECC engine.
+
+* NAND device/chip bindings:
+
+Required properties:
+- reg: describes the CS lines assigned to the NAND device. If the NAND device
+       exposes multiple CS lines (multi-dies chips), your reg property will
+       contain X tuples of 3 entries.
+       1st entry: the CS line this NAND chip is connected to
+       2nd entry: the base offset of the memory region assigned to this
+		  device (always 0)
+       3rd entry: the memory region size (always 0x800000)
+
+Optional properties:
+- rb-gpios: the GPIO(s) used to check the Ready/Busy status of the NAND.
+- cs-gpios: the GPIO(s) used to control the CS line.
+- det-gpios: the GPIO used to detect if a Smartmedia Card is present.
+- atmel,rb: an integer identifying the native Ready/Busy pin. Only meaningful
+	    on sama5 SoCs.
+
+All generic properties described in
+Documentation/devicetree/bindings/mtd/{common,nand}.txt also apply to the NAND
+device node, and NAND partitions should be defined under the NAND node as
+described in Documentation/devicetree/bindings/mtd/partition.txt.
+
+* ECC engine (PMECC) bindings:
+
+Required properties:
+- compatible: should be one of the following
+	"atmel,at91sam9g45-pmecc"
+	"atmel,sama5d4-pmecc"
+	"atmel,sama5d2-pmecc"
+- reg: should contain 2 register ranges. The first one is pointing to the PMECC
+       block, and the second one to the PMECC_ERRLOC block.
+
+Example:
+
+	pmecc: ecc-engine at ffffc070 {
+		compatible = "atmel,at91sam9g45-pmecc";
+                reg = <0xffffc070 0x490>,
+                      <0xffffc500 0x100>;
+	};
+
+	ebi: ebi at 10000000 {
+		compatible = "atmel,sama5d3-ebi";
+		#address-cells = <2>;
+		#size-cells = <1>;
+		atmel,smc = <&hsmc>;
+		reg = <0x10000000 0x10000000
+		       0x40000000 0x30000000>;
+		ranges = <0x0 0x0 0x10000000 0x10000000
+			  0x1 0x0 0x40000000 0x10000000
+			  0x2 0x0 0x50000000 0x10000000
+			  0x3 0x0 0x60000000 0x10000000>;
+		clocks = <&mck>;
+
+                nand_controller: nand-controller {
+			compatible = "atmel,sama5d3-nand-controller";
+			atmel,nfc-sram = <&nfc_sram>;
+			atmel,nfc-io = <&nfc_io>;
+			ecc-engine = <&pmecc>;
+			#address-cells = <2>;
+			#size-cells = <1>;
+			ranges;
+
+			nand at 3 {
+				reg = <0x3 0x0 0x800000>;
+				atmel,rb = <0>;
+
+				/*
+				 * Put generic NAND/MTD properties and
+				 * subnodes here.
+				 */
+			};
+		};
+	};
+
+-----------------------------------------------------------------------
+
+Deprecated bindings (should not be used in new device trees):
 
 Required properties:
 - compatible: The possible values are:
-- 
2.7.4

[PATCH v2 3/3] mtd: nand: Remove unused chip->write_page() hook

[Boris Brezillon]

The last/only user of the chip->write_page() hook (the Atmel NAND
controller driver) has been reworked and is no longer specifying a custom
->write_page() implementation.
Drop this hook before someone else start abusing it.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 10 ++++------
 include/linux/mtd/nand.h     |  4 ----
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ec1c28aaaf23..c8894f31392e 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2839,9 +2839,10 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 			/* We still need to erase leftover OOB data */
 			memset(chip->oob_poi, 0xff, mtd->oobsize);
 		}
-		ret = chip->write_page(mtd, chip, column, bytes, wbuf,
-					oob_required, page, cached,
-					(ops->mode == MTD_OPS_RAW));
+
+		ret = nand_write_page(mtd, chip, column, bytes, wbuf,
+				      oob_required, page, cached,
+				      (ops->mode == MTD_OPS_RAW));
 		if (ret)
 			break;
 
@@ -4623,9 +4624,6 @@ int nand_scan_tail(struct mtd_info *mtd)
 		}
 	}
 
-	if (!chip->write_page)
-		chip->write_page = nand_write_page;
-
 	/*
 	 * Check ECC mode, default to software if 3byte/512byte hardware ECC is
 	 * selected and we have 256 byte pagesize fallback to software ECC
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index c5f3a012ae62..9d51dee53be4 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -818,7 +818,6 @@ nand_get_sdr_timings(const struct nand_data_interface *conf)
  * @errstat:		[OPTIONAL] hardware specific function to perform
  *			additional error status checks (determine if errors are
  *			correctable).
- * @write_page:		[REPLACEABLE] High-level page write function
  */
 
 struct nand_chip {
@@ -843,9 +842,6 @@ struct nand_chip {
 	int (*scan_bbt)(struct mtd_info *mtd);
 	int (*errstat)(struct mtd_info *mtd, struct nand_chip *this, int state,
 			int status, int page);
-	int (*write_page)(struct mtd_info *mtd, struct nand_chip *chip,
-			uint32_t offset, int data_len, const uint8_t *buf,
-			int oob_required, int page, int cached, int raw);
 	int (*onfi_set_features)(struct mtd_info *mtd, struct nand_chip *chip,
 			int feature_addr, uint8_t *subfeature_para);
 	int (*onfi_get_features)(struct mtd_info *mtd, struct nand_chip *chip,
-- 
2.7.4

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Andy Shevchenko]

On Mon, Feb 20, 2017 at 2:28 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:

>  drivers/mtd/nand/atmel/nand-controller.c | 2269 +++++++++++++++++++++++++++
>  drivers/mtd/nand/atmel_nand.c            | 2479 ------------------------------

Does -M -C help you?
At least it would help reviewers

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Boris Brezillon]

On Mon, 20 Feb 2017 22:27:17 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Mon, Feb 20, 2017 at 2:28 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> 
> >  drivers/mtd/nand/atmel/nand-controller.c | 2269 +++++++++++++++++++++++++++
> >  drivers/mtd/nand/atmel_nand.c            | 2479 ------------------------------  
> 
> Does -M -C help you?
> At least it would help reviewers
> 

No it doesn't, because files were not just moved around using git mv,
it's a complete rewrite of the driver. IIUC, you're about to review
this submission, or are you just trolling like last time?

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Boris Brezillon]

On Mon, 20 Feb 2017 21:38:03 +0100
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Mon, 20 Feb 2017 22:27:17 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> > On Mon, Feb 20, 2017 at 2:28 PM, Boris Brezillon
> > <boris.brezillon@free-electrons.com> wrote:
> >   
> > >  drivers/mtd/nand/atmel/nand-controller.c | 2269 +++++++++++++++++++++++++++
> > >  drivers/mtd/nand/atmel_nand.c            | 2479 ------------------------------    
> > 
> > Does -M -C help you?
> > At least it would help reviewers
> >   
> 
> No it doesn't, because files were not just moved around using git mv,
> it's a complete rewrite of the driver. IIUC, you're about to review
> this submission, or are you just trolling like last time?

My bad, I mistaken you with someone else. Sorry for being harsh, but my
explanation stands ;-).

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Andy Shevchenko]

On Mon, Feb 20, 2017 at 10:50 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Mon, 20 Feb 2017 21:38:03 +0100
> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
>
>> On Mon, 20 Feb 2017 22:27:17 +0200
>> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>
>> > On Mon, Feb 20, 2017 at 2:28 PM, Boris Brezillon
>> > <boris.brezillon@free-electrons.com> wrote:
>> >
>> > >  drivers/mtd/nand/atmel/nand-controller.c | 2269 +++++++++++++++++++++++++++
>> > >  drivers/mtd/nand/atmel_nand.c            | 2479 ------------------------------
>> >
>> > Does -M -C help you?
>> > At least it would help reviewers
>> >
>>
>> No it doesn't, because files were not just moved around using git mv,
>> it's a complete rewrite of the driver. IIUC, you're about to review
>> this submission, or are you just trolling like last time?
>
> My bad, I mistaken you with someone else. Sorry for being harsh, but my
> explanation stands ;-).

No problem. I was asking since it so big and on first glance looks
like a partial copy (I dunno if parameter to -C makes it somehow
useful), though I can't review this. It's too big to me. Sorry I'm
really not trolling, just didn't read commit message carefully.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Andy Shevchenko]

On Tue, Feb 21, 2017 at 1:40 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Feb 20, 2017 at 10:50 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
>> On Mon, 20 Feb 2017 21:38:03 +0100
>> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
>>
>>> On Mon, 20 Feb 2017 22:27:17 +0200
>>> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>>
>>> > On Mon, Feb 20, 2017 at 2:28 PM, Boris Brezillon
>>> > <boris.brezillon@free-electrons.com> wrote:
>>> >
>>> > >  drivers/mtd/nand/atmel/nand-controller.c | 2269 +++++++++++++++++++++++++++
>>> > >  drivers/mtd/nand/atmel_nand.c            | 2479 ------------------------------
>>> >
>>> > Does -M -C help you?
>>> > At least it would help reviewers
>>> >
>>>
>>> No it doesn't, because files were not just moved around using git mv,
>>> it's a complete rewrite of the driver. IIUC, you're about to review
>>> this submission, or are you just trolling like last time?
>>
>> My bad, I mistaken you with someone else. Sorry for being harsh, but my
>> explanation stands ;-).
>
> No problem. I was asking since it so big and on first glance looks
> like a partial copy (I dunno if parameter to -C makes it somehow
> useful), though I can't review this. It's too big to me. Sorry I'm
> really not trolling, just didn't read commit message carefully.

Okay, I very quickly looked into the code, what I noticed
- you like extra parens and empty lines in some cases (not big deal)
- some functions perhaps might have been refactored to have common
pieces in error handling, though I didn't read core carefully.

Most important part I have noticed is a GPIO request.
I didn't get why you almost repeat gpiod_get() in case of platform data?
Shouldn't we have GPIO look up table?
Can we use builtin device properties (for GPIO and/or overall)?


-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Boris Brezillon]

On Tue, 21 Feb 2017 01:54:37 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Feb 21, 2017 at 1:40 AM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Feb 20, 2017 at 10:50 PM, Boris Brezillon
> > <boris.brezillon@free-electrons.com> wrote:  
> >> On Mon, 20 Feb 2017 21:38:03 +0100
> >> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> >>  
> >>> On Mon, 20 Feb 2017 22:27:17 +0200
> >>> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> >>>  
> >>> > On Mon, Feb 20, 2017 at 2:28 PM, Boris Brezillon
> >>> > <boris.brezillon@free-electrons.com> wrote:
> >>> >  
> >>> > >  drivers/mtd/nand/atmel/nand-controller.c | 2269 +++++++++++++++++++++++++++
> >>> > >  drivers/mtd/nand/atmel_nand.c            | 2479 ------------------------------  
> >>> >
> >>> > Does -M -C help you?
> >>> > At least it would help reviewers
> >>> >  
> >>>
> >>> No it doesn't, because files were not just moved around using git mv,
> >>> it's a complete rewrite of the driver. IIUC, you're about to review
> >>> this submission, or are you just trolling like last time?  
> >>
> >> My bad, I mistaken you with someone else. Sorry for being harsh, but my
> >> explanation stands ;-).  
> >
> > No problem. I was asking since it so big and on first glance looks
> > like a partial copy (I dunno if parameter to -C makes it somehow
> > useful), though I can't review this. It's too big to me. Sorry I'm
> > really not trolling, just didn't read commit message carefully.  
> 
> Okay, I very quickly looked into the code, what I noticed
> - you like extra parens and empty lines in some cases (not big deal)

Can you point specific places where you think these are not needed?

> - some functions perhaps might have been refactored to have common
> pieces in error handling, though I didn't read core carefully.

Again, be more precise.

> 
> Most important part I have noticed is a GPIO request.
> I didn't get why you almost repeat gpiod_get() in case of platform data?
> Shouldn't we have GPIO look up table?
> Can we use builtin device properties (for GPIO and/or overall)?

Sorry but I don't get it. Can give an example of what you'd like me to
do?

Thanks,

Boris

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Andy Shevchenko]

On Tue, Feb 21, 2017 at 10:06 AM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Tue, 21 Feb 2017 01:54:37 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
>> On Tue, Feb 21, 2017 at 1:40 AM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>> > On Mon, Feb 20, 2017 at 10:50 PM, Boris Brezillon
>> > <boris.brezillon@free-electrons.com> wrote:
>> >> On Mon, 20 Feb 2017 21:38:03 +0100
>> >> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
>> >>
>> >>> On Mon, 20 Feb 2017 22:27:17 +0200
>> >>> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>> >>>
>> >>> > On Mon, Feb 20, 2017 at 2:28 PM, Boris Brezillon
>> >>> > <boris.brezillon@free-electrons.com> wrote:
>> >>> >
>> >>> > >  drivers/mtd/nand/atmel/nand-controller.c | 2269 +++++++++++++++++++++++++++
>> >>> > >  drivers/mtd/nand/atmel_nand.c            | 2479 ------------------------------
>> >>> >
>> >>> > Does -M -C help you?
>> >>> > At least it would help reviewers
>> >>> >
>> >>>
>> >>> No it doesn't, because files were not just moved around using git mv,
>> >>> it's a complete rewrite of the driver. IIUC, you're about to review
>> >>> this submission, or are you just trolling like last time?
>> >>
>> >> My bad, I mistaken you with someone else. Sorry for being harsh, but my
>> >> explanation stands ;-).
>> >
>> > No problem. I was asking since it so big and on first glance looks
>> > like a partial copy (I dunno if parameter to -C makes it somehow
>> > useful), though I can't review this. It's too big to me. Sorry I'm
>> > really not trolling, just didn't read commit message carefully.
>>
>> Okay, I very quickly looked into the code, what I noticed
>> - you like extra parens and empty lines in some cases (not big deal)
>
> Can you point specific places where you think these are not needed?

1. For example,

#define ATMEL_NFC_CMD(pos, cmd)                        ((cmd) <<
(((pos) * 8) + 2))

 *events ^= (status & *events);

 (((x) * 0x4) + 0x28)

  memset(&si[1], 0, sizeof(s16) * ((2 * strength) - 1));

Perhaps more in the code. I'm not a LISP programmer.

2. For empty lines it's solely matter of style, I don't care. My motto
"less LOC better, but keep common sense in mind".

>> - some functions perhaps might have been refactored to have common
>> pieces in error handling, though I didn't read core carefully.
>
> Again, be more precise.

3. I don't remember anymore, sorry. Something I would refactor.

>> Most important part I have noticed is a GPIO request.
>> I didn't get why you almost repeat gpiod_get() in case of platform data?
>> Shouldn't we have GPIO look up table?
>> Can we use builtin device properties (for GPIO and/or overall)?
>
> Sorry but I don't get it. Can give an example of what you'd like me to
> do?
>

4. First of all, why do you need this function in the first place?

+struct gpio_desc *
+atmel_nand_pdata_get_gpio(struct atmel_nand_controller *nc, int gpioid,
+                         const char *name, bool active_low,
+                         enum gpiod_flags flags)

5. BIT() macro:

   const unsigned int k = 1 << deg(poly);
   unsigned int nn = (1 << mm) - 1;

6. Why this casting (unsigned int) ?

 dev_dbg(pmecc->dev,
                       "Bit flip in %s area, byte %d: 0x%02x -> 0x%02x\n",
                       area, byte, *ptr, (unsigned int)(*ptr ^ BIT(bit)));

7. Question to all that distribution or whatever functions, don't you
have a common helper? Or each vendor requires different logic behind
it?

8. Have you checked what kernel library provides?

And I believe there are still issues like those. After, who is on
topic, might even find some logical and other issues...

P.S. TBH, so big change is unreviewable in meaningful time. To have a
comprehensive review I, for example, spend ~1h/250LOC, and
~2.5h/1000LOC, I would estimate ~4h/2000LOC. Imagine one to spend one
day for this. Any volunteer? Not me.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Boris Brezillon]

On Tue, 21 Feb 2017 12:03:45 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Feb 21, 2017 at 10:06 AM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Tue, 21 Feb 2017 01:54:37 +0200
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> >  
> >> On Tue, Feb 21, 2017 at 1:40 AM, Andy Shevchenko
> >> <andy.shevchenko@gmail.com> wrote:  
> >> > On Mon, Feb 20, 2017 at 10:50 PM, Boris Brezillon
> >> > <boris.brezillon@free-electrons.com> wrote:  
> >> >> On Mon, 20 Feb 2017 21:38:03 +0100
> >> >> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> >> >>  
> >> >>> On Mon, 20 Feb 2017 22:27:17 +0200
> >> >>> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> >> >>>  
> >> >>> > On Mon, Feb 20, 2017 at 2:28 PM, Boris Brezillon
> >> >>> > <boris.brezillon@free-electrons.com> wrote:
> >> >>> >  
> >> >>> > >  drivers/mtd/nand/atmel/nand-controller.c | 2269 +++++++++++++++++++++++++++
> >> >>> > >  drivers/mtd/nand/atmel_nand.c            | 2479 ------------------------------  
> >> >>> >
> >> >>> > Does -M -C help you?
> >> >>> > At least it would help reviewers
> >> >>> >  
> >> >>>
> >> >>> No it doesn't, because files were not just moved around using git mv,
> >> >>> it's a complete rewrite of the driver. IIUC, you're about to review
> >> >>> this submission, or are you just trolling like last time?  
> >> >>
> >> >> My bad, I mistaken you with someone else. Sorry for being harsh, but my
> >> >> explanation stands ;-).  
> >> >
> >> > No problem. I was asking since it so big and on first glance looks
> >> > like a partial copy (I dunno if parameter to -C makes it somehow
> >> > useful), though I can't review this. It's too big to me. Sorry I'm
> >> > really not trolling, just didn't read commit message carefully.  
> >>
> >> Okay, I very quickly looked into the code, what I noticed
> >> - you like extra parens and empty lines in some cases (not big deal)  
> >
> > Can you point specific places where you think these are not needed?  
> 
> 1. For example,
> 
> #define ATMEL_NFC_CMD(pos, cmd)                        ((cmd) <<
> (((pos) * 8) + 2))

Well, I like to explicitly put parenthesis even when operator
precedence guarantees the order of the calculation ('*' is preceding
'+').

For the parenthesis around (cmd) and (pos), they are required to
guarantee that things like ATMEL_NFC_CMD(x + y, cmd) are working
correctly.

> 
>  *events ^= (status & *events);

I agree with this one, it's uneeded.

> 
>  (((x) * 0x4) + 0x28)

See my comment about ATMEL_NFC_CMD().

> 
>   memset(&si[1], 0, sizeof(s16) * ((2 * strength) - 1));

Ditto.

> 
> Perhaps more in the code. I'm not a LISP programmer.
> 
> 2. For empty lines it's solely matter of style, I don't care. My motto
> "less LOC better, but keep common sense in mind".
> 
> >> - some functions perhaps might have been refactored to have common
> >> pieces in error handling, though I didn't read core carefully.  
> >
> > Again, be more precise.  
> 
> 3. I don't remember anymore, sorry. Something I would refactor.
> 
> >> Most important part I have noticed is a GPIO request.
> >> I didn't get why you almost repeat gpiod_get() in case of platform data?
> >> Shouldn't we have GPIO look up table?
> >> Can we use builtin device properties (for GPIO and/or overall)?  
> >
> > Sorry but I don't get it. Can give an example of what you'd like me to
> > do?
> >  
> 
> 4. First of all, why do you need this function in the first place?
> 
> +struct gpio_desc *
> +atmel_nand_pdata_get_gpio(struct atmel_nand_controller *nc, int gpioid,
> +                         const char *name, bool active_low,
> +                         enum gpiod_flags flags)

Because I don't want to duplicate the code done in
atmel_nand_pdata_get_gpio() each time I have to convert a GPIO number
into a GPIO descriptor, and that is needed to support platforms that
haven't moved to DT yet (in this case, avr32).

> 
> 5. BIT() macro:
> 
>    const unsigned int k = 1 << deg(poly);
>    unsigned int nn = (1 << mm) - 1;

Yes, I must admit I didn't polish the code in PMECC, and most of it has
been copied from the old driver.
We could probably use BIT() in a few places.

> 
> 6. Why this casting (unsigned int) ?
> 
>  dev_dbg(pmecc->dev,
>                        "Bit flip in %s area, byte %d: 0x%02x -> 0x%02x\n",
>                        area, byte, *ptr, (unsigned int)(*ptr ^ BIT(bit)));

Again, this has been copied from the old driver. I'll have a closer
look.

> 
> 7. Question to all that distribution or whatever functions, don't you
> have a common helper? Or each vendor requires different logic behind
> it?

What are you talking about? nand_chip hooks?

> 
> 8. Have you checked what kernel library provides?

I think so, but again, this is really vague, what kind of
open-coded functions do you think could be replaced with core libraries
helpers?

> 
> And I believe there are still issues like those. After, who is on
> topic, might even find some logical and other issues...
> 
> P.S. TBH, so big change is unreviewable in meaningful time. To have a
> comprehensive review I, for example, spend ~1h/250LOC, and
> ~2.5h/1000LOC, I would estimate ~4h/2000LOC. Imagine one to spend one
> day for this. Any volunteer? Not me.

I'm not asking you to review the whole driver, but you started to
comment on the code without pointing clearly to the things you wanted
me to address.

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Nicolas Ferre]

Le 21/02/2017 ? 11:26, Boris Brezillon a ?crit :
> On Tue, 21 Feb 2017 12:03:45 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
>> On Tue, Feb 21, 2017 at 10:06 AM, Boris Brezillon
>> <boris.brezillon@free-electrons.com> wrote:
>>> On Tue, 21 Feb 2017 01:54:37 +0200
>>> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>>  
>>>> On Tue, Feb 21, 2017 at 1:40 AM, Andy Shevchenko
>>>> <andy.shevchenko@gmail.com> wrote:  
>>>>> On Mon, Feb 20, 2017 at 10:50 PM, Boris Brezillon
>>>>> <boris.brezillon@free-electrons.com> wrote:  
>>>>>> On Mon, 20 Feb 2017 21:38:03 +0100
>>>>>> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
>>>>>>  
>>>>>>> On Mon, 20 Feb 2017 22:27:17 +0200
>>>>>>> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>>>>>>  
>>>>>>>> On Mon, Feb 20, 2017 at 2:28 PM, Boris Brezillon
>>>>>>>> <boris.brezillon@free-electrons.com> wrote:
>>>>>>>>  
>>>>>>>>>  drivers/mtd/nand/atmel/nand-controller.c | 2269 +++++++++++++++++++++++++++
>>>>>>>>>  drivers/mtd/nand/atmel_nand.c            | 2479 ------------------------------  
>>>>>>>>
>>>>>>>> Does -M -C help you?
>>>>>>>> At least it would help reviewers
>>>>>>>>  
>>>>>>>
>>>>>>> No it doesn't, because files were not just moved around using git mv,
>>>>>>> it's a complete rewrite of the driver. IIUC, you're about to review
>>>>>>> this submission, or are you just trolling like last time?  
>>>>>>
>>>>>> My bad, I mistaken you with someone else. Sorry for being harsh, but my
>>>>>> explanation stands ;-).  
>>>>>
>>>>> No problem. I was asking since it so big and on first glance looks
>>>>> like a partial copy (I dunno if parameter to -C makes it somehow
>>>>> useful), though I can't review this. It's too big to me. Sorry I'm
>>>>> really not trolling, just didn't read commit message carefully.  
>>>>
>>>> Okay, I very quickly looked into the code, what I noticed
>>>> - you like extra parens and empty lines in some cases (not big deal)  
>>>
>>> Can you point specific places where you think these are not needed?  
>>
>> 1. For example,
>>
>> #define ATMEL_NFC_CMD(pos, cmd)                        ((cmd) <<
>> (((pos) * 8) + 2))
> 
> Well, I like to explicitly put parenthesis even when operator
> precedence guarantees the order of the calculation ('*' is preceding
> '+').

Yes

> For the parenthesis around (cmd) and (pos), they are required to
> guarantee that things like ATMEL_NFC_CMD(x + y, cmd) are working
> correctly.

Absolutely.


Even when it's not needed, please keep this habit of using more
parenthesis than required by precedence to make code clearer.


>>  *events ^= (status & *events);
> 
> I agree with this one, it's uneeded.
> 
>>
>>  (((x) * 0x4) + 0x28)
> 
> See my comment about ATMEL_NFC_CMD().
> 
>>
>>   memset(&si[1], 0, sizeof(s16) * ((2 * strength) - 1));
> 
> Ditto.
> 
>>
>> Perhaps more in the code. I'm not a LISP programmer.

[..]

>> 8. Have you checked what kernel library provides?
> 
> I think so, but again, this is really vague, what kind of
> open-coded functions do you think could be replaced with core libraries
> helpers?
> 
>> And I believe there are still issues like those. After, who is on
>> topic, might even find some logical and other issues...
>>
>> P.S. TBH, so big change is unreviewable in meaningful time. To have a
>> comprehensive review I, for example, spend ~1h/250LOC, and
>> ~2.5h/1000LOC, I would estimate ~4h/2000LOC. Imagine one to spend one
>> day for this. Any volunteer? Not me.
> 
> I'm not asking you to review the whole driver, but you started to
> comment on the code without pointing clearly to the things you wanted
> me to address.

Moreover, it's not like if the driver would come without previous code.
So, this re-factoring comes with the experience of previous driver and
its aim is to be comparable feature-wise with the old one. So the amount
of changes doesn't surprise me.

As Boris noted in his patch series, additional optimization and use of
common BCH code can be studied afterwards.

Best regards,
-- 
Nicolas Ferre

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Andy Shevchenko]

On Tue, Feb 21, 2017 at 12:26 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Tue, 21 Feb 2017 12:03:45 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

>> 1. For example,
>>
>> #define ATMEL_NFC_CMD(pos, cmd)                        ((cmd) <<
>> (((pos) * 8) + 2))
>
> Well, I like to explicitly put parenthesis even when operator
> precedence guarantees the order of the calculation ('*' is preceding
> '+').

That's my point. I'm not a LISP programmer.
Personally I think it makes readability worse.

> For the parenthesis around (cmd) and (pos), they are required to
> guarantee that things like ATMEL_NFC_CMD(x + y, cmd) are working
> correctly.

I know that.

>> >> Most important part I have noticed is a GPIO request.
>> >> I didn't get why you almost repeat gpiod_get() in case of platform data?
>> >> Shouldn't we have GPIO look up table?
>> >> Can we use builtin device properties (for GPIO and/or overall)?
>> >
>> > Sorry but I don't get it. Can give an example of what you'd like me to
>> > do?
>> >
>>
>> 4. First of all, why do you need this function in the first place?
>>
>> +struct gpio_desc *
>> +atmel_nand_pdata_get_gpio(struct atmel_nand_controller *nc, int gpioid,
>> +                         const char *name, bool active_low,
>> +                         enum gpiod_flags flags)
>
> Because I don't want to duplicate the code done in
> atmel_nand_pdata_get_gpio() each time I have to convert a GPIO number
> into a GPIO descriptor, and that is needed to support platforms that
> haven't moved to DT yet

They should use GPIO lookup tables.

We don't encourage people to use platform data anymore.

We have unified device properties for something like "timeout-us", we
have look up tables when you need specifics like pwm, gpio, pinctrl,
...

Abusing platform data with pointers is also not welcome.

> (in this case, avr32).

It's dead de facto.

When last time did you compile kernel for it? What was the version of kernel?
Did it get successfully?

When are we going to remove avr32 support from kernel completely?

>> 5. BIT() macro:

> We could probably use BIT() in a few places.

There are more places including data structures assignments.

> Again, this has been copied from the old driver. I'll have a closer
> look.

Exactly. You overlooked due to enormous LOC in the one change. See my
point below.

>> 7. Question to all that distribution or whatever functions, don't you
>> have a common helper? Or each vendor requires different logic behind
>> it?
>
> What are you talking about? nand_chip hooks?

That long arithmetic with some data.

>> 8. Have you checked what kernel library provides?
>
> I think so, but again, this is really vague, what kind of
> open-coded functions do you think could be replaced with core libraries
> helpers?

I dunno, I'm asking you. Usually if I see a pattern I got a clue to
check lib/ and similar places. From time to time I discover something
new and interesting there.

>> And I believe there are still issues like those. After, who is on
>> topic, might even find some logical and other issues...
>>
>> P.S. TBH, so big change is unreviewable in meaningful time. To have a
>> comprehensive review I, for example, spend ~1h/250LOC, and
>> ~2.5h/1000LOC, I would estimate ~4h/2000LOC. Imagine one to spend one
>> day for this. Any volunteer? Not me.
>
> I'm not asking you to review the whole driver, but you started to
> comment on the code without pointing clearly to the things you wanted
> me to address.

Yes, because my point is *split* this to be reviewable.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Boris Brezillon]

On Tue, 21 Feb 2017 13:02:21 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Feb 21, 2017 at 12:26 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Tue, 21 Feb 2017 12:03:45 +0200
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:  
> 
> >> 1. For example,
> >>
> >> #define ATMEL_NFC_CMD(pos, cmd)                        ((cmd) <<
> >> (((pos) * 8) + 2))  
> >
> > Well, I like to explicitly put parenthesis even when operator
> > precedence guarantees the order of the calculation ('*' is preceding
> > '+').  
> 
> That's my point. I'm not a LISP programmer.
> Personally I think it makes readability worse.

So, it's a matter of taste.
> >>
> >> 4. First of all, why do you need this function in the first place?
> >>
> >> +struct gpio_desc *
> >> +atmel_nand_pdata_get_gpio(struct atmel_nand_controller *nc, int gpioid,
> >> +                         const char *name, bool active_low,
> >> +                         enum gpiod_flags flags)  
> >
> > Because I don't want to duplicate the code done in
> > atmel_nand_pdata_get_gpio() each time I have to convert a GPIO number
> > into a GPIO descriptor, and that is needed to support platforms that
> > haven't moved to DT yet  
> 
> They should use GPIO lookup tables.
> 
> We don't encourage people to use platform data anymore.
> 
> We have unified device properties for something like "timeout-us", we
> have look up tables when you need specifics like pwm, gpio, pinctrl,
> ...
> 
> Abusing platform data with pointers is also not welcome.
> 
> > (in this case, avr32).  
> 
> It's dead de facto.
> 
> When last time did you compile kernel for it? What was the version of kernel?
> Did it get successfully?
> 
> When are we going to remove avr32 support from kernel completely?

I'll let Nicolas answer that one.

> 
> >> 5. BIT() macro:  
> 
> > We could probably use BIT() in a few places.  
> 
> There are more places including data structures assignments.

Yes. These are minor changes. I'll try to fix them.
Note that I sometime prefer to keep (1 << X).

Example:

#define PMECC_CFG_READ_OP			(0 << 12)
#define PMECC_CFG_WRITE_OP			(1 << 12)

> 
> > Again, this has been copied from the old driver. I'll have a closer
> > look.  
> 
> Exactly. You overlooked due to enormous LOC in the one change. See my
> point below.
> 
> >> 7. Question to all that distribution or whatever functions, don't you
> >> have a common helper? Or each vendor requires different logic behind
> >> it?  
> >
> > What are you talking about? nand_chip hooks?  
> 
> That long arithmetic with some data.

Okay, so the code in pmecc.c. See, it's hard to follow a review when
you don't comment inline.

> 
> >> 8. Have you checked what kernel library provides?  
> >
> > I think so, but again, this is really vague, what kind of
> > open-coded functions do you think could be replaced with core libraries
> > helpers?  
> 
> I dunno, I'm asking you. Usually if I see a pattern I got a clue to
> check lib/ and similar places. From time to time I discover something
> new and interesting there.

If you're talking about the code in pmecc.c, yes, I already mentioned
in the header that it should be reworked to use some helpers from
lib/bch.c, but that's not the point of this series, and is left as
future improvements.

> 
> >> And I believe there are still issues like those. After, who is on
> >> topic, might even find some logical and other issues...
> >>
> >> P.S. TBH, so big change is unreviewable in meaningful time. To have a
> >> comprehensive review I, for example, spend ~1h/250LOC, and
> >> ~2.5h/1000LOC, I would estimate ~4h/2000LOC. Imagine one to spend one
> >> day for this. Any volunteer? Not me.  
> >
> > I'm not asking you to review the whole driver, but you started to
> > comment on the code without pointing clearly to the things you wanted
> > me to address.  
> 
> Yes, because my point is *split* this to be reviewable.
> 

And how do you do with new drivers? Do you ask people to split their
submissions in micro changes? I'm regularly reviewing drivers that are
several thousands LOC, and I don't ask people to split things just
because it's too long. When I ask them to split in different commits,
it's because they are doing several unrelated changes at once.

Note that I considered refactoring the existing driver in smaller
steps, but it's almost impossible, because the code is too messy and I
would end up with a huge series of patches that is not easier to review.

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Alexandre Belloni]

(adding Hans-Christian)

On 21/02/2017 at 13:02:21 +0200, Andy Shevchenko wrote:
> Abusing platform data with pointers is also not welcome.
> 
> > (in this case, avr32).
> 
> It's dead de facto.
> 
> When last time did you compile kernel for it? What was the version of kernel?
> Did it get successfully?
> 

v4.10-rc3 was building successfully but had some issues in the network
code.

> When are we going to remove avr32 support from kernel completely?
> 

Ask that to the avr32 maintainers. It still builds and is still booted
by some people. And that actually seems to be you as you reported a bug
we introduced in 4.3. I don't think we had any other report after that.

It can be frustrating at times to handle that platform but if it is
working for someone, I don't see why we would remove it.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Nicolas Ferre]

Le 20/02/2017 ? 13:28, Boris Brezillon a ?crit :
> This is a complete rewrite of the driver whose main purpose is to
> support the new DT representation where the NAND controller node is now
> really visible in the DT and appears under the EBI bus. With this new
> representation, we can add other devices under the EBI bus without
> risking pinmuxing conflicts (the NAND controller is under the EBI
> bus logic and as such, share some of its pins with other devices
> connected on this bus).
> 
> Even though the goal of this rework was not necessarily to add new
> features, the new driver has been designed with this in mind. With a
> clearer separation between the different blocks and different IP
> revisions, adding new functionalities should be easier (we already
> have plans to support SMC timing configuration so that we no longer
> have to rely on the configuration done by the bootloader/bootstrap).
> 
> Also note that we no longer have a custom ->cmdfunc() implementation,
> which means we can now benefit from new features added in the core
> implementation for free (support for new NAND operations for example).
> 
> The last thing that we gain with this rework is support for multi-chips
> and multi-dies chips, thanks to the clean NAND controller <-> NAND
> devices representation.
> 
> This new driver has been tested on several platforms (at91sam9261,
> at91sam9g45, at91sam9x5, sama5d3 and sama5d4) to make sure it did not
> introduce regressions, and it's worth mentioning that old bindings are
> still supported (which partly explain the positive diffstat).
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  MAINTAINERS                              |    2 +-
>  drivers/mtd/nand/Makefile                |    2 +-
>  drivers/mtd/nand/atmel/Makefile          |    4 +
>  drivers/mtd/nand/atmel/nand-controller.c | 2269 +++++++++++++++++++++++++++
>  drivers/mtd/nand/atmel/pmecc.c           | 1020 ++++++++++++
>  drivers/mtd/nand/atmel/pmecc.h           |   73 +
>  drivers/mtd/nand/atmel_nand.c            | 2479 ------------------------------
>  drivers/mtd/nand/atmel_nand_ecc.h        |  163 --
>  drivers/mtd/nand/atmel_nand_nfc.h        |  103 --
>  9 files changed, 3368 insertions(+), 2747 deletions(-)
>  create mode 100644 drivers/mtd/nand/atmel/Makefile
>  create mode 100644 drivers/mtd/nand/atmel/nand-controller.c
>  create mode 100644 drivers/mtd/nand/atmel/pmecc.c
>  create mode 100644 drivers/mtd/nand/atmel/pmecc.h
>  delete mode 100644 drivers/mtd/nand/atmel_nand.c
>  delete mode 100644 drivers/mtd/nand/atmel_nand_ecc.h
>  delete mode 100644 drivers/mtd/nand/atmel_nand_nfc.h

[..]

> + * A few words about the naming convention in this file. This convention
> + * applies to structure and function names.
> + *
> + * Prefixes:
> + *
> + * - atmel_nand_: all generic structures/functions
> + * - atmel_smc_nand_: all structures/functions specific to the SMC interface
> + *		      (at91sam9 and avr32 SoCs)
> + * - atmel_hsmc_nand_: all structures/functions specific to the HSMC interface
> + *		       (sama5 SoCs and later)
> + * - atmel_nfc_: all structures/functions used to manipulate the NFC sub-block
> + *		 that is available in the HSMC block
> + * - <soc>_nand_: all SoC specific structures/functions

Ok, good.

> + */

[..]

> +static irqreturn_t atmel_nfc_interrupt(int irq, void *data)
> +{
> +	struct atmel_hsmc_nand_controller *nc = data;
> +	u32 imr, sr;
> +
> +	regmap_read(nc->base.smc, ATMEL_HSMC_NFC_IMR, &imr);
> +	regmap_read(nc->base.smc, ATMEL_HSMC_NFC_SR, &sr);
> +
> +	sr &= imr;
> +
> +	if (sr)
> +		regmap_write(nc->base.smc, ATMEL_HSMC_NFC_IDR, sr);
> +
> +	if (sr == imr)
> +		complete(&nc->complete);
> +
> +	return sr ? IRQ_HANDLED : IRQ_NONE;
> +}

It can be good as well to print out the error conditions. Not that it
changes the behavior of the driver but it can warn us about issues like
in the old function nfc_read_status().

Othewise, I'm okay with the patch, so you can add my:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Best regards,
-- 
Nicolas Ferre

[PATCH v2 2/3] mtd: nand: atmel: Document the new DT bindings

[Nicolas Ferre]

Le 20/02/2017 ? 13:28, Boris Brezillon a ?crit :
> The old NAND bindings were not exactly describing the hardware topology
> and were preventing definitions of several NAND chips under the same
> NAND controller.
> 
> New bindings address these limitations and should be preferred over the
> old ones for new SoCs/boards.
> Old bindings are still supported for backward compatibility but are
> marked deprecated in the doc.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>


I think I already added it but here is my:
Reviewed-by: Nicolas Ferre <nicolas.ferre@microchip.com>


> ---
>  .../devicetree/bindings/mtd/atmel-nand.txt         | 107 ++++++++++++++++++++-
>  1 file changed, 106 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> index 3e7ee99d3949..f6bee57e453a 100644
> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> @@ -1,4 +1,109 @@
> -Atmel NAND flash
> +Atmel NAND flash controller bindings
> +
> +The NAND flash controller node should be defined under the EBI bus (see
> +Documentation/devicetree/bindings/memory-controllers/atmel,ebi.txt).
> +One or several NAND devices can be defined under this NAND controller.
> +The NAND controller might be connected to an ECC engine.
> +
> +* NAND controller bindings:
> +
> +Required properties:
> +- compatible: should be one of the following
> +	"atmel,at91rm9200-nand-controller"
> +	"atmel,at91sam9260-nand-controller"
> +	"atmel,at91sam9261-nand-controller"
> +	"atmel,at91sam9g45-nand-controller"
> +	"atmel,sama5d3-nand-controller"
> +- ranges: empty ranges property to forward EBI ranges definitions.
> +- #address-cells: should be set to 2.
> +- #size-cells: should be set to 1.
> +- atmel,nfc-io: phandle to the NFC IO block. Only required for sama5d3
> +		controllers.
> +- atmel,nfc-sram: phandle to the NFC SRAM block. Only required for sama5d3
> +		  controllers.
> +
> +Optional properties:
> +- ecc-engine: phandle to the PMECC block. Only meaningful if the SoC embeds
> +	      a PMECC engine.
> +
> +* NAND device/chip bindings:
> +
> +Required properties:
> +- reg: describes the CS lines assigned to the NAND device. If the NAND device
> +       exposes multiple CS lines (multi-dies chips), your reg property will
> +       contain X tuples of 3 entries.
> +       1st entry: the CS line this NAND chip is connected to
> +       2nd entry: the base offset of the memory region assigned to this
> +		  device (always 0)
> +       3rd entry: the memory region size (always 0x800000)
> +
> +Optional properties:
> +- rb-gpios: the GPIO(s) used to check the Ready/Busy status of the NAND.
> +- cs-gpios: the GPIO(s) used to control the CS line.
> +- det-gpios: the GPIO used to detect if a Smartmedia Card is present.
> +- atmel,rb: an integer identifying the native Ready/Busy pin. Only meaningful
> +	    on sama5 SoCs.
> +
> +All generic properties described in
> +Documentation/devicetree/bindings/mtd/{common,nand}.txt also apply to the NAND
> +device node, and NAND partitions should be defined under the NAND node as
> +described in Documentation/devicetree/bindings/mtd/partition.txt.
> +
> +* ECC engine (PMECC) bindings:
> +
> +Required properties:
> +- compatible: should be one of the following
> +	"atmel,at91sam9g45-pmecc"
> +	"atmel,sama5d4-pmecc"
> +	"atmel,sama5d2-pmecc"
> +- reg: should contain 2 register ranges. The first one is pointing to the PMECC
> +       block, and the second one to the PMECC_ERRLOC block.
> +
> +Example:
> +
> +	pmecc: ecc-engine at ffffc070 {
> +		compatible = "atmel,at91sam9g45-pmecc";
> +                reg = <0xffffc070 0x490>,
> +                      <0xffffc500 0x100>;
> +	};
> +
> +	ebi: ebi at 10000000 {
> +		compatible = "atmel,sama5d3-ebi";
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		atmel,smc = <&hsmc>;
> +		reg = <0x10000000 0x10000000
> +		       0x40000000 0x30000000>;
> +		ranges = <0x0 0x0 0x10000000 0x10000000
> +			  0x1 0x0 0x40000000 0x10000000
> +			  0x2 0x0 0x50000000 0x10000000
> +			  0x3 0x0 0x60000000 0x10000000>;
> +		clocks = <&mck>;
> +
> +                nand_controller: nand-controller {
> +			compatible = "atmel,sama5d3-nand-controller";
> +			atmel,nfc-sram = <&nfc_sram>;
> +			atmel,nfc-io = <&nfc_io>;
> +			ecc-engine = <&pmecc>;
> +			#address-cells = <2>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			nand at 3 {
> +				reg = <0x3 0x0 0x800000>;
> +				atmel,rb = <0>;
> +
> +				/*
> +				 * Put generic NAND/MTD properties and
> +				 * subnodes here.
> +				 */
> +			};
> +		};
> +	};
> +
> +-----------------------------------------------------------------------
> +
> +Deprecated bindings (should not be used in new device trees):
>  
>  Required properties:
>  - compatible: The possible values are:
> 


-- 
Nicolas Ferre

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Nicolas Ferre]

Le 21/02/2017 ? 12:20, Boris Brezillon a ?crit :
>>> (in this case, avr32).  
>> It's dead de facto.
>>
>> When last time did you compile kernel for it? What was the version of kernel?
>> Did it get successfully?

Alexandre answered to this one.

>> When are we going to remove avr32 support from kernel completely?
> I'll let Nicolas answer that one.

It's not up to me to decide this, the community only can decide to
remove the support from the kernel.
What I can tell, and it's been the case for a handful of years now, is
that Atmel/Microchip will not work on this platform anymore and won't
stop a removal of this platform from the Linux kernel.

I know that the dual approach DT/non-DT for some drivers is somehow
painful but I don't see AVR32 moving to DT in the near future...

Regards,
-- 
Nicolas Ferre

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Russell King - ARM Linux]

On Tue, Feb 21, 2017 at 01:02:21PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 21, 2017 at 12:26 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Tue, 21 Feb 2017 12:03:45 +0200
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> >> 1. For example,
> >>
> >> #define ATMEL_NFC_CMD(pos, cmd)                        ((cmd) <<
> >> (((pos) * 8) + 2))
> >
> > Well, I like to explicitly put parenthesis even when operator
> > precedence guarantees the order of the calculation ('*' is preceding
> > '+').
> 
> That's my point. I'm not a LISP programmer.
> Personally I think it makes readability worse.

+1.  I find unnecessary parenthesis is an effective obfuscation technique.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Andy Shevchenko]

On Tue, Feb 21, 2017 at 1:20 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Tue, 21 Feb 2017 13:02:21 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>> On Tue, Feb 21, 2017 at 12:26 PM, Boris Brezillon
>> <boris.brezillon@free-electrons.com> wrote:
>> > On Tue, 21 Feb 2017 12:03:45 +0200
>> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> So, it's a matter of taste.

Yes, and I'm not objecting this.

>> >> 4. First of all, why do you need this function in the first place?
>> >>
>> >> +struct gpio_desc *
>> >> +atmel_nand_pdata_get_gpio(struct atmel_nand_controller *nc, int gpioid,
>> >> +                         const char *name, bool active_low,
>> >> +                         enum gpiod_flags flags)
>> >
>> > Because I don't want to duplicate the code done in
>> > atmel_nand_pdata_get_gpio() each time I have to convert a GPIO number
>> > into a GPIO descriptor, and that is needed to support platforms that
>> > haven't moved to DT yet
>>
>> They should use GPIO lookup tables.
>>
>> We don't encourage people to use platform data anymore.
>>
>> We have unified device properties for something like "timeout-us", we
>> have look up tables when you need specifics like pwm, gpio, pinctrl,
>> ...
>>
>> Abusing platform data with pointers is also not welcome.
>>
>> > (in this case, avr32).
>>
>> It's dead de facto.
>>
>> When last time did you compile kernel for it? What was the version of kernel?
>> Did it get successfully?
>>
>> When are we going to remove avr32 support from kernel completely?
>
> I'll let Nicolas answer that one.

In any case it's discouraging to use platform data for GPIOs and plain
GPIO pin numbering.

> Note that I sometime prefer to keep (1 << X).
>
> Example:
>
> #define PMECC_CFG_READ_OP                       (0 << 12)
> #define PMECC_CFG_WRITE_OP                      (1 << 12)

I understand that.

> Okay, so the code in pmecc.c. See, it's hard to follow a review when
> you don't comment inline.

It's hard to review (n+1) thousands of LOC.

>> >> 8. Have you checked what kernel library provides?
>> >
>> > I think so, but again, this is really vague, what kind of
>> > open-coded functions do you think could be replaced with core libraries
>> > helpers?
>>
>> I dunno, I'm asking you. Usually if I see a pattern I got a clue to
>> check lib/ and similar places. From time to time I discover something
>> new and interesting there.
>
> If you're talking about the code in pmecc.c, yes, I already mentioned
> in the header that it should be reworked to use some helpers from
> lib/bch.c, but that's not the point of this series, and is left as
> future improvements.

OK.

>> Yes, because my point is *split* this to be reviewable.

> And how do you do with new drivers?

To be more pedantic the new drivers do not have "minus" thousands LOC.

> Do you ask people to split their
> submissions in micro changes?

To logical ones.

> I'm regularly reviewing drivers that are
> several thousands LOC, and I don't ask people to split things just
> because it's too long. When I ask them to split in different commits,
> it's because they are doing several unrelated changes at once.

What did prevent you to:
1. Introduce new driver
2. Switch to new driver
3. Remove old one.

...if you are not splitting it in the first place?

> Note that I considered refactoring the existing driver in smaller
> steps, but it's almost impossible, because the code is too messy and I
> would end up with a huge series of patches that is not easier to review.

I can object this, but it will be no point except waste of time to
this discussion.

It's good that you considered several options. I suppose someone who
is on topic can do comprehensive review.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Andy Shevchenko]

On Tue, Feb 21, 2017 at 1:27 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> (adding Hans-Christian)
>
> On 21/02/2017 at 13:02:21 +0200, Andy Shevchenko wrote:
>> Abusing platform data with pointers is also not welcome.
>>
>> > (in this case, avr32).
>>
>> It's dead de facto.
>>
>> When last time did you compile kernel for it? What was the version of kernel?
>> Did it get successfully?
>>
>
> v4.10-rc3 was building successfully but had some issues in the network
> code.

Newer kernel doesn't link...

>> When are we going to remove avr32 support from kernel completely?

> Ask that to the avr32 maintainers. It still builds and is still booted
> by some people. And that actually seems to be you as you reported a bug
> we introduced in 4.3. I don't think we had any other report after that.

https://patchwork.kernel.org/patch/9505727/

After that I gave up on it. Next time I will escalate directly to
Linus. It's a complete necrophilia. I spent already enough time to
look at that code. It brings now more burden than supports someone
somewhere.

> It can be frustrating at times to handle that platform but if it is
> working for someone, I don't see why we would remove it.

How it's working if it's not linked?

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Alexandre Belloni]

On 21/02/2017 at 17:55:15 +0200, Andy Shevchenko wrote:
> > And how do you do with new drivers?
> 
> To be more pedantic the new drivers do not have "minus" thousands LOC.

Because all the "minus" are located in the same file (the one that
disappear), they can be safely ignored. So it is basically the same as
having a new driver.

> > I'm regularly reviewing drivers that are
> > several thousands LOC, and I don't ask people to split things just
> > because it's too long. When I ask them to split in different commits,
> > it's because they are doing several unrelated changes at once.
> 
> What did prevent you to:
> 1. Introduce new driver
> 2. Switch to new driver
> 3. Remove old one.
> 
> ...if you are not splitting it in the first place?
> 

Having a new Kconfig symbol and switching to it, then switching to the
previous one to avoid breaking existing configurations. That's a lot of
churn for exactly 0 benefit because as said, you can safely ignore the
removed file when reviewing.

> > Note that I considered refactoring the existing driver in smaller
> > steps, but it's almost impossible, because the code is too messy and I
> > would end up with a huge series of patches that is not easier to review.
> 
> I can object this, but it will be no point except waste of time to
> this discussion.
> 
> It's good that you considered several options. I suppose someone who
> is on topic can do comprehensive review.
> 

Maybe the NAND subsystem maintainer can review the change... oh,
wait...nevermind.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Alexandre Belloni]

On 21/02/2017 at 18:09:09 +0200, Andy Shevchenko wrote:
> On Tue, Feb 21, 2017 at 1:27 PM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
> > (adding Hans-Christian)
> >
> > On 21/02/2017 at 13:02:21 +0200, Andy Shevchenko wrote:
> >> Abusing platform data with pointers is also not welcome.
> >>
> >> > (in this case, avr32).
> >>
> >> It's dead de facto.
> >>
> >> When last time did you compile kernel for it? What was the version of kernel?
> >> Did it get successfully?
> >>
> >
> > v4.10-rc3 was building successfully but had some issues in the network
> > code.
> 
> Newer kernel doesn't link...
> 
> >> When are we going to remove avr32 support from kernel completely?
> 
> > Ask that to the avr32 maintainers. It still builds and is still booted
> > by some people. And that actually seems to be you as you reported a bug
> > we introduced in 4.3. I don't think we had any other report after that.
> 
> https://patchwork.kernel.org/patch/9505727/
> 
> After that I gave up on it. Next time I will escalate directly to
> Linus. It's a complete necrophilia. I spent already enough time to
> look at that code. It brings now more burden than supports someone
> somewhere.
> 

As said, it builds fine without networking. Maybe the first step is to
ask the avr32 maintainers. If you already did so, please feel free to
send a patch to remove the whole architecture.
The benefits for atmel will be: proper big endian support, removal of
platform data from all the drivers, better clocksource handling.

> > It can be frustrating at times to handle that platform but if it is
> > working for someone, I don't see why we would remove it.
> 
> How it's working if it's not linked?
> 

Come on, v4.10 has just been release and v4.9 was building just fine. Do
you really expect everybody to closely follow linux-next or update
overnight?

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Andy Shevchenko]

On Tue, Feb 21, 2017 at 6:21 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> On 21/02/2017 at 18:09:09 +0200, Andy Shevchenko wrote:
>> On Tue, Feb 21, 2017 at 1:27 PM, Alexandre Belloni
>> <alexandre.belloni@free-electrons.com> wrote:
>> > On 21/02/2017 at 13:02:21 +0200, Andy Shevchenko wrote:
>> >> Abusing platform data with pointers is also not welcome.

>> >> > (in this case, avr32).
>> >>
>> >> It's dead de facto.
>> >>
>> >> When last time did you compile kernel for it? What was the version of kernel?
>> >> Did it get successfully?
>> >>
>> >
>> > v4.10-rc3 was building successfully but had some issues in the network
>> > code.
>>
>> Newer kernel doesn't link...
>>
>> >> When are we going to remove avr32 support from kernel completely?
>>
>> > Ask that to the avr32 maintainers. It still builds and is still booted
>> > by some people. And that actually seems to be you as you reported a bug
>> > we introduced in 4.3. I don't think we had any other report after that.
>>
>> https://patchwork.kernel.org/patch/9505727/
>>
>> After that I gave up on it. Next time I will escalate directly to
>> Linus. It's a complete necrophilia. I spent already enough time to
>> look at that code. It brings now more burden than supports someone
>> somewhere.
>>
>
> As said, it builds fine without networking.

It sounds a bit sarcastic. Irony is that I *have* hardware here which
was dedicated as Network Gateway (ATNGW100). I'm accessing to it
remotely.
How useful it would be?

> Maybe the first step is to
> ask the avr32 maintainers. If you already did so,

I did it ~year or so before where another relocation bug was discovered (fixed).

> please feel free to
> send a patch to remove the whole architecture.
> The benefits for atmel will be: proper big endian support, removal of
> platform data from all the drivers, better clocksource handling.

That is good point, but if maintainers don't care, why anyone else should?
Neither do I.

>> > It can be frustrating at times to handle that platform but if it is
>> > working for someone, I don't see why we would remove it.
>>
>> How it's working if it's not linked?
>>
>
> Come on, v4.10 has just been release and v4.9 was building just fine. Do
> you really expect everybody to closely follow linux-next or update
> overnight?

What version do you use as compiler?

Today's linux-next:
$ make O=~/prj/TMP/out/avr32 C=1 CF=-D__CHECK_ENDIAN__ -j64 CONFIG_DEBUG_INFO=
y CONFIG_DEBUG_SECTION_MISMATCH=y

  CC      lib/sbitmap.o
{standard input}: Assembler messages:
{standard input}:378: Warning: Unary operator + ignored because bad
operand follows
{standard input}:378: Warning: missing operand; zero assumed
{standard input}:378: Internal error!
Assertion failure in finish_insn at .././gas/config/tc-avr32.c line 3498.
Please report this bug.
scripts/Makefile.build:294: recipe for target 'lib/sbitmap.o' failed

$ avr32-linux-gcc --version
avr32-linux-gcc (GCC) 4.2.2-atmel.1.0.8


-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Andy Shevchenko]

On Tue, Feb 21, 2017 at 6:32 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Feb 21, 2017 at 6:21 PM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
>> On 21/02/2017 at 18:09:09 +0200, Andy Shevchenko wrote:
>>> On Tue, Feb 21, 2017 at 1:27 PM, Alexandre Belloni
>>> <alexandre.belloni@free-electrons.com> wrote:
>>> > On 21/02/2017 at 13:02:21 +0200, Andy Shevchenko wrote:
>>> >> Abusing platform data with pointers is also not welcome.
>
>>> >> > (in this case, avr32).
>>> >>
>>> >> It's dead de facto.
>>> >>
>>> >> When last time did you compile kernel for it? What was the version of kernel?
>>> >> Did it get successfully?
>>> >>
>>> >
>>> > v4.10-rc3 was building successfully but had some issues in the network
>>> > code.
>>>
>>> Newer kernel doesn't link...
>>>
>>> >> When are we going to remove avr32 support from kernel completely?
>>>
>>> > Ask that to the avr32 maintainers. It still builds and is still booted
>>> > by some people. And that actually seems to be you as you reported a bug
>>> > we introduced in 4.3. I don't think we had any other report after that.
>>>
>>> https://patchwork.kernel.org/patch/9505727/
>>>
>>> After that I gave up on it. Next time I will escalate directly to
>>> Linus. It's a complete necrophilia. I spent already enough time to
>>> look at that code. It brings now more burden than supports someone
>>> somewhere.
>>>
>>
>> As said, it builds fine without networking.
>
> It sounds a bit sarcastic. Irony is that I *have* hardware here which
> was dedicated as Network Gateway (ATNGW100). I'm accessing to it
> remotely.
> How useful it would be?
>
>> Maybe the first step is to
>> ask the avr32 maintainers. If you already did so,
>
> I did it ~year or so before where another relocation bug was discovered (fixed).
>
>> please feel free to
>> send a patch to remove the whole architecture.
>> The benefits for atmel will be: proper big endian support, removal of
>> platform data from all the drivers, better clocksource handling.
>
> That is good point, but if maintainers don't care, why anyone else should?
> Neither do I.
>
>>> > It can be frustrating at times to handle that platform but if it is
>>> > working for someone, I don't see why we would remove it.
>>>
>>> How it's working if it's not linked?
>>>
>>
>> Come on, v4.10 has just been release and

It doesn't build anymore. And current case even worse
Face it. It's dead.

  MODPOST vmlinux.o
WARNING: vmlinux.o(.text+0x1f2bd4): Section mismatch in reference from
the variable __param_ops_mtd to the functio
n .init.text:ubi_mtd_param_parse()
The function __param_ops_mtd() references
the function __init ubi_mtd_param_parse().
This is often because __param_ops_mtd lacks a __init
annotation or the annotation of ubi_mtd_param_parse is wrong.

crypto/built-in.o: warning: input is not relaxable
virt/built-in.o: warning: input is not relaxable
net/built-in.o: In function `rtnl_fill_ifinfo':
net/socket.c:451: relocation truncated to fit: R_AVR32_11H_PCREL
against `.text'+22768
Makefile:969: recipe for target 'vmlinux' failed


> v4.9 was building just fine. Do
>> you really expect everybody to closely follow linux-next or update
>> overnight?
>
> What version do you use as compiler?
>
> Today's linux-next:
> $ make O=~/prj/TMP/out/avr32 C=1 CF=-D__CHECK_ENDIAN__ -j64 CONFIG_DEBUG_INFO=
> y CONFIG_DEBUG_SECTION_MISMATCH=y
>
>   CC      lib/sbitmap.o
> {standard input}: Assembler messages:
> {standard input}:378: Warning: Unary operator + ignored because bad
> operand follows
> {standard input}:378: Warning: missing operand; zero assumed
> {standard input}:378: Internal error!
> Assertion failure in finish_insn at .././gas/config/tc-avr32.c line 3498.
> Please report this bug.
> scripts/Makefile.build:294: recipe for target 'lib/sbitmap.o' failed
>
> $ avr32-linux-gcc --version
> avr32-linux-gcc (GCC) 4.2.2-atmel.1.0.8

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Alexandre Belloni]

On 21/02/2017 at 18:32:26 +0200, Andy Shevchenko wrote:
> I did it ~year or so before where another relocation bug was discovered (fixed).
> 
> > please feel free to
> > send a patch to remove the whole architecture.
> > The benefits for atmel will be: proper big endian support, removal of
> > platform data from all the drivers, better clocksource handling.
> 
> That is good point, but if maintainers don't care, why anyone else should?
> Neither do I.
> 
> >> > It can be frustrating at times to handle that platform but if it is
> >> > working for someone, I don't see why we would remove it.
> >>
> >> How it's working if it's not linked?
> >>
> >
> > Come on, v4.10 has just been release and v4.9 was building just fine. Do
> > you really expect everybody to closely follow linux-next or update
> > overnight?
> 
> What version do you use as compiler?
> 
> Today's linux-next:
> $ make O=~/prj/TMP/out/avr32 C=1 CF=-D__CHECK_ENDIAN__ -j64 CONFIG_DEBUG_INFO=
> y CONFIG_DEBUG_SECTION_MISMATCH=y
> 
>   CC      lib/sbitmap.o
> {standard input}: Assembler messages:
> {standard input}:378: Warning: Unary operator + ignored because bad
> operand follows
> {standard input}:378: Warning: missing operand; zero assumed
> {standard input}:378: Internal error!
> Assertion failure in finish_insn at .././gas/config/tc-avr32.c line 3498.
> Please report this bug.
> scripts/Makefile.build:294: recipe for target 'lib/sbitmap.o' failed
> 
> $ avr32-linux-gcc --version
> avr32-linux-gcc (GCC) 4.2.2-atmel.1.0.8
> 

avr32-linux-gcc (GCC) 4.2.4-atmel.1.1.3.avr32linux.1

Today's linux-next built without network support, the issue still being:
virt/built-in.o: warning: input is not relaxable net/built-in.o: In function `rtnl_fill_vfinfo':
rtnetlink.c:(.text+0x21974): relocation truncated to fit: R_AVR32_11H_PCREL against `.text'+2156c
rtnetlink.c:(.text+0x2198a): relocation truncated to fit: R_AVR32_11H_PCREL against `.text'+2156c
Makefile:983: recipe for target 'vmlinux' failed


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Alexandre Belloni]

On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:
> >> Come on, v4.10 has just been release and
> 
> It doesn't build anymore. And current case even worse
> Face it. It's dead.
> 

I agree it hasn't seen any significant development in a while but I'm
not the on able to take that decision.

A few weeks ago, I was telling Boris to let it not build for a while and
then remove it. You already went out of your way to make it work. Again,
feel free to send a patch removing avr32. I can only see a lot of
benefits for the Atmel ARM SoCs and the many cleanups that will follow.

If nobody complains about the 4.10 breakage, You'll have plenty of time
to remove it for 4.12

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Håvard Skinnemoen]

On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:
>> >> Come on, v4.10 has just been release and
>>
>> It doesn't build anymore. And current case even worse
>> Face it. It's dead.
>>
>
> I agree it hasn't seen any significant development in a while but I'm
> not the on able to take that decision.

I've been wanting to add devicetree support for some time, but I no
longer remember how to build the toolchain, and I don't have fond
memories of that whole process. And the fact that
4.2.4-atmel.1.1.3.avr32linux.1 is still the most current version of
gcc doesn't make me very optimistic.

So while Hans-Christian and others have been doing a great job of
keeping AVR32 on life support, I tend to think that if there's not
enough enthusiasm for the architecture to build a modern toolchain or
add support for device tree, avr32-linux probably isn't going anywhere
exciting.

> A few weeks ago, I was telling Boris to let it not build for a while and
> then remove it. You already went out of your way to make it work. Again,
> feel free to send a patch removing avr32. I can only see a lot of
> benefits for the Atmel ARM SoCs and the many cleanups that will follow.

Agree, I can't help but feel that the AVR32 support is doing more harm
than good at this point.

> If nobody complains about the 4.10 breakage, You'll have plenty of time
> to remove it for 4.12

I'm fine with that, but I haven't put much effort into keeping it
alive lately. If Hans-Christian agrees, I'm willing to post a patch to
remove it, or ack someone else's patch.

H?vard

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Hans-Christian Noren Egtvedt]

Around Thu 23 Feb 2017 21:18:13 -0800 or thereabout, H?vard Skinnemoen wrote:
> On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
>> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:

<snipp>

>> A few weeks ago, I was telling Boris to let it not build for a while and
>> then remove it. You already went out of your way to make it work. Again,
>> feel free to send a patch removing avr32. I can only see a lot of
>> benefits for the Atmel ARM SoCs and the many cleanups that will follow.
> 
> Agree, I can't help but feel that the AVR32 support is doing more harm
> than good at this point.

I also agree on this, I can relate to Nicolas (and Atmel friends) having to
always think about the less-maintained AVR32 parts when improving drivers.

>> If nobody complains about the 4.10 breakage, You'll have plenty of time
>> to remove it for 4.12
> 
> I'm fine with that, but I haven't put much effort into keeping it
> alive lately. If Hans-Christian agrees, I'm willing to post a patch to
> remove it, or ack someone else's patch.

Then lets plan this for 4.12, either you H?vard whip up a patch or I can
eventually do it.

I can push it through the linux-avr32 git tree on kernel.org.

-- 
mvh
Hans-Christian Noren Egtvedt

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Boris Brezillon]

Hi Hans-Cristrian,

On Fri, 24 Feb 2017 09:14:30 +0100
Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:

> Around Thu 23 Feb 2017 21:18:13 -0800 or thereabout, H?vard Skinnemoen wrote:
> > On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
> > <alexandre.belloni@free-electrons.com> wrote:  
> >> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:  
> 
> <snipp>
> 
> >> A few weeks ago, I was telling Boris to let it not build for a while and
> >> then remove it. You already went out of your way to make it work. Again,
> >> feel free to send a patch removing avr32. I can only see a lot of
> >> benefits for the Atmel ARM SoCs and the many cleanups that will follow.  
> > 
> > Agree, I can't help but feel that the AVR32 support is doing more harm
> > than good at this point.  
> 
> I also agree on this, I can relate to Nicolas (and Atmel friends) having to
> always think about the less-maintained AVR32 parts when improving drivers.

Indeed, that should make atmel drivers maintainance a bit easier.

> 
> >> If nobody complains about the 4.10 breakage, You'll have plenty of time
> >> to remove it for 4.12  
> > 
> > I'm fine with that, but I haven't put much effort into keeping it
> > alive lately. If Hans-Christian agrees, I'm willing to post a patch to
> > remove it, or ack someone else's patch.  
> 
> Then lets plan this for 4.12, either you H?vard whip up a patch or I can
> eventually do it.
> 
> I can push it through the linux-avr32 git tree on kernel.org.
> 

Can you do that just after 4.11-rc1 is released and provide a topic
branch I can pull in my nand/next branch, so that I can rework this
patch and drop all the pdata-compat code (as suggested by Andy).

Thanks,

Boris

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Hans-Christian Noren Egtvedt]

Around Fri 24 Feb 2017 09:27:42 +0100 or thereabout, Boris Brezillon wrote:
> On Fri, 24 Feb 2017 09:14:30 +0100 Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:
>> Around Thu 23 Feb 2017 21:18:13 -0800 or thereabout, H?vard Skinnemoen wrote:
>> > On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
>> > <alexandre.belloni@free-electrons.com> wrote:  
>> >> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:  

<snipp>

>> >> If nobody complains about the 4.10 breakage, You'll have plenty of time
>> >> to remove it for 4.12  
>> > 
>> > I'm fine with that, but I haven't put much effort into keeping it
>> > alive lately. If Hans-Christian agrees, I'm willing to post a patch to
>> > remove it, or ack someone else's patch.  
>> 
>> Then lets plan this for 4.12, either you H?vard whip up a patch or I can
>> eventually do it.
>> 
>> I can push it through the linux-avr32 git tree on kernel.org.
>> 
> 
> Can you do that just after 4.11-rc1 is released and provide a topic
> branch I can pull in my nand/next branch, so that I can rework this
> patch and drop all the pdata-compat code (as suggested by Andy).

OK, I will try to prepare it during the weekend.

Any reason to wait for 4.11-rc1? AFAIK Linus prefers the larger changes
before he starts tagging rc's.

-- 
mvh
Hans-Christian Noren Egtvedt

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Boris Brezillon]

On Fri, 24 Feb 2017 09:52:09 +0100
Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:

> Around Fri 24 Feb 2017 09:27:42 +0100 or thereabout, Boris Brezillon wrote:
> > On Fri, 24 Feb 2017 09:14:30 +0100 Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:  
> >> Around Thu 23 Feb 2017 21:18:13 -0800 or thereabout, H?vard Skinnemoen wrote:  
> >> > On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
> >> > <alexandre.belloni@free-electrons.com> wrote:    
> >> >> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:    
> 
> <snipp>
> 
> >> >> If nobody complains about the 4.10 breakage, You'll have plenty of time
> >> >> to remove it for 4.12    
> >> > 
> >> > I'm fine with that, but I haven't put much effort into keeping it
> >> > alive lately. If Hans-Christian agrees, I'm willing to post a patch to
> >> > remove it, or ack someone else's patch.    
> >> 
> >> Then lets plan this for 4.12, either you H?vard whip up a patch or I can
> >> eventually do it.
> >> 
> >> I can push it through the linux-avr32 git tree on kernel.org.
> >>   
> > 
> > Can you do that just after 4.11-rc1 is released and provide a topic
> > branch I can pull in my nand/next branch, so that I can rework this
> > patch and drop all the pdata-compat code (as suggested by Andy).  
> 
> OK, I will try to prepare it during the weekend.
> 
> Any reason to wait for 4.11-rc1? AFAIK Linus prefers the larger changes
> before he starts tagging rc's.
> 

Oh, so you want to queue it for 4.11, that's even better.

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Hans-Christian Noren Egtvedt]

Around Fri 24 Feb 2017 09:55:09 +0100 or thereabout, Boris Brezillon wrote:
> On Fri, 24 Feb 2017 09:52:09 +0100
> Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:
>> Around Fri 24 Feb 2017 09:27:42 +0100 or thereabout, Boris Brezillon wrote:
>> > On Fri, 24 Feb 2017 09:14:30 +0100 Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:  
>> >> Around Thu 23 Feb 2017 21:18:13 -0800 or thereabout, H?vard Skinnemoen wrote:  
>> >> > On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
>> >> > <alexandre.belloni@free-electrons.com> wrote:    
>> >> >> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:    
>> 
>> <snipp>
>> 
>> >> >> If nobody complains about the 4.10 breakage, You'll have plenty of time
>> >> >> to remove it for 4.12    
>> >> > 
>> >> > I'm fine with that, but I haven't put much effort into keeping it
>> >> > alive lately. If Hans-Christian agrees, I'm willing to post a patch to
>> >> > remove it, or ack someone else's patch.    
>> >> 
>> >> Then lets plan this for 4.12, either you H?vard whip up a patch or I can
>> >> eventually do it.
>> >> 
>> >> I can push it through the linux-avr32 git tree on kernel.org.
>> >>   
>> > 
>> > Can you do that just after 4.11-rc1 is released and provide a topic
>> > branch I can pull in my nand/next branch, so that I can rework this
>> > patch and drop all the pdata-compat code (as suggested by Andy).  
>> 
>> OK, I will try to prepare it during the weekend.
>> 
>> Any reason to wait for 4.11-rc1? AFAIK Linus prefers the larger changes
>> before he starts tagging rc's.
>> 
> 
> Oh, so you want to queue it for 4.11, that's even better.

Perhaps I misunderstood you, by after 4.11-rc1 you mean queue it for 4.12?

I will see what I get around to do in the weekend, it should be pretty
straightforward, just want to make sure we remove all the bits.

-- 
mvh
Hans-Christian Noren Egtvedt

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Boris Brezillon]

On Fri, 24 Feb 2017 10:04:35 +0100
Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:

> Around Fri 24 Feb 2017 09:55:09 +0100 or thereabout, Boris Brezillon wrote:
> > On Fri, 24 Feb 2017 09:52:09 +0100
> > Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:  
> >> Around Fri 24 Feb 2017 09:27:42 +0100 or thereabout, Boris Brezillon wrote:  
> >> > On Fri, 24 Feb 2017 09:14:30 +0100 Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:    
> >> >> Around Thu 23 Feb 2017 21:18:13 -0800 or thereabout, H?vard Skinnemoen wrote:    
> >> >> > On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
> >> >> > <alexandre.belloni@free-electrons.com> wrote:      
> >> >> >> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:      
> >> 
> >> <snipp>
> >>   
> >> >> >> If nobody complains about the 4.10 breakage, You'll have plenty of time
> >> >> >> to remove it for 4.12      
> >> >> > 
> >> >> > I'm fine with that, but I haven't put much effort into keeping it
> >> >> > alive lately. If Hans-Christian agrees, I'm willing to post a patch to
> >> >> > remove it, or ack someone else's patch.      
> >> >> 
> >> >> Then lets plan this for 4.12, either you H?vard whip up a patch or I can
> >> >> eventually do it.
> >> >> 
> >> >> I can push it through the linux-avr32 git tree on kernel.org.
> >> >>     
> >> > 
> >> > Can you do that just after 4.11-rc1 is released and provide a topic
> >> > branch I can pull in my nand/next branch, so that I can rework this
> >> > patch and drop all the pdata-compat code (as suggested by Andy).    
> >> 
> >> OK, I will try to prepare it during the weekend.
> >> 
> >> Any reason to wait for 4.11-rc1? AFAIK Linus prefers the larger changes
> >> before he starts tagging rc's.
> >>   
> > 
> > Oh, so you want to queue it for 4.11, that's even better.  
> 
> Perhaps I misunderstood you, by after 4.11-rc1 you mean queue it for 4.12?

Yep, that's what I understood from your previous answer where you said
'Then lets plan this for 4.12, either you H?vard whip up a patch or I
can eventually do it.'. If you queue it for 4.12, you'll probably want
to base your patch on 4.11-rc1 to make sure it does not conflict with
changes pulled by Linus during the merge window.

OTOH, if you want to remove avr32 support in 4.11 (which would make
things easier for me ;-)), you still have one week before the end of
the merge window.

> 
> I will see what I get around to do in the weekend, it should be pretty
> straightforward, just want to make sure we remove all the bits.
> 

Okay.

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Alexandre Belloni]

On 24/02/2017 at 09:14:30 +0100, Hans-Christian Noren Egtvedt wrote:
> Around Thu 23 Feb 2017 21:18:13 -0800 or thereabout, H?vard Skinnemoen wrote:
> > On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
> > <alexandre.belloni@free-electrons.com> wrote:
> >> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:
> 
> <snipp>
> 
> >> A few weeks ago, I was telling Boris to let it not build for a while and
> >> then remove it. You already went out of your way to make it work. Again,
> >> feel free to send a patch removing avr32. I can only see a lot of
> >> benefits for the Atmel ARM SoCs and the many cleanups that will follow.
> > 
> > Agree, I can't help but feel that the AVR32 support is doing more harm
> > than good at this point.
> 
> I also agree on this, I can relate to Nicolas (and Atmel friends) having to
> always think about the less-maintained AVR32 parts when improving drivers.
> 
> >> If nobody complains about the 4.10 breakage, You'll have plenty of time
> >> to remove it for 4.12
> > 
> > I'm fine with that, but I haven't put much effort into keeping it
> > alive lately. If Hans-Christian agrees, I'm willing to post a patch to
> > remove it, or ack someone else's patch.
> 
> Then lets plan this for 4.12, either you H?vard whip up a patch or I can
> eventually do it.
> 
> I can push it through the linux-avr32 git tree on kernel.org.
> 

I think think it is fair to have one of you two prepare the patch. It is
definitively a sad decision :( but it will help us immensely! Thank you!

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Alexandre Belloni]

On 24/02/2017 at 10:04:35 +0100, Hans-Christian Noren Egtvedt wrote:
> Around Fri 24 Feb 2017 09:55:09 +0100 or thereabout, Boris Brezillon wrote:
> > On Fri, 24 Feb 2017 09:52:09 +0100
> > Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:
> >> OK, I will try to prepare it during the weekend.
> >> 
> >> Any reason to wait for 4.11-rc1? AFAIK Linus prefers the larger changes
> >> before he starts tagging rc's.
> >> 
> > 
> > Oh, so you want to queue it for 4.11, that's even better.
> 
> Perhaps I misunderstood you, by after 4.11-rc1 you mean queue it for 4.12?
> 
> I will see what I get around to do in the weekend, it should be pretty
> straightforward, just want to make sure we remove all the bits.
> 

I think the main task is removing arch/avr32 and update MAINTAINERS
(don't forget to add yourself to CREDITS) and Documentation/ anything
else will have to go through the driver maintainers tree.

If you feel like it, you can also prepare patches for the avr32 only
drivers. I'll be happy to help with the individual drivers if you can't
find the time to do it. We will definitively take care of the shared
avr32/at91 drivers.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Andy Shevchenko]

On Fri, Feb 24, 2017 at 11:51 AM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> On 24/02/2017 at 10:04:35 +0100, Hans-Christian Noren Egtvedt wrote:
>> Around Fri 24 Feb 2017 09:55:09 +0100 or thereabout, Boris Brezillon wrote:
>> > On Fri, 24 Feb 2017 09:52:09 +0100
>> > Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:

> I think the main task is removing arch/avr32 and update MAINTAINERS
> (don't forget to add yourself to CREDITS) and Documentation/ anything
> else will have to go through the driver maintainers tree.
>
> If you feel like it, you can also prepare patches for the avr32 only
> drivers. I'll be happy to help with the individual drivers if you can't
> find the time to do it. We will definitively take care of the shared
> avr32/at91 drivers.

I can do for the drivers where DW DMA is involved (sound/soc/,
drivers/dma/dw/) since it's my main concern wrt avr32.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 2/3] mtd: nand: atmel: Document the new DT bindings

[Rob Herring]

On Mon, Feb 20, 2017 at 01:28:37PM +0100, Boris Brezillon wrote:
> The old NAND bindings were not exactly describing the hardware topology
> and were preventing definitions of several NAND chips under the same
> NAND controller.
> 
> New bindings address these limitations and should be preferred over the
> old ones for new SoCs/boards.
> Old bindings are still supported for backward compatibility but are
> marked deprecated in the doc.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  .../devicetree/bindings/mtd/atmel-nand.txt         | 107 ++++++++++++++++++++-
>  1 file changed, 106 insertions(+), 1 deletion(-)

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Boris Brezillon]

Hi Hans-Christian,

On Fri, 24 Feb 2017 10:04:35 +0100
Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:

> Around Fri 24 Feb 2017 09:55:09 +0100 or thereabout, Boris Brezillon wrote:
> > On Fri, 24 Feb 2017 09:52:09 +0100
> > Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:  
> >> Around Fri 24 Feb 2017 09:27:42 +0100 or thereabout, Boris Brezillon wrote:  
> >> > On Fri, 24 Feb 2017 09:14:30 +0100 Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:    
> >> >> Around Thu 23 Feb 2017 21:18:13 -0800 or thereabout, H?vard Skinnemoen wrote:    
> >> >> > On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
> >> >> > <alexandre.belloni@free-electrons.com> wrote:      
> >> >> >> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:      
> >> 
> >> <snipp>
> >>   
> >> >> >> If nobody complains about the 4.10 breakage, You'll have plenty of time
> >> >> >> to remove it for 4.12      
> >> >> > 
> >> >> > I'm fine with that, but I haven't put much effort into keeping it
> >> >> > alive lately. If Hans-Christian agrees, I'm willing to post a patch to
> >> >> > remove it, or ack someone else's patch.      
> >> >> 
> >> >> Then lets plan this for 4.12, either you H?vard whip up a patch or I can
> >> >> eventually do it.
> >> >> 
> >> >> I can push it through the linux-avr32 git tree on kernel.org.
> >> >>     
> >> > 
> >> > Can you do that just after 4.11-rc1 is released and provide a topic
> >> > branch I can pull in my nand/next branch, so that I can rework this
> >> > patch and drop all the pdata-compat code (as suggested by Andy).    
> >> 
> >> OK, I will try to prepare it during the weekend.
> >> 
> >> Any reason to wait for 4.11-rc1? AFAIK Linus prefers the larger changes
> >> before he starts tagging rc's.
> >>   
> > 
> > Oh, so you want to queue it for 4.11, that's even better.  
> 
> Perhaps I misunderstood you, by after 4.11-rc1 you mean queue it for 4.12?
> 
> I will see what I get around to do in the weekend, it should be pretty
> straightforward, just want to make sure we remove all the bits.
> 

Any progress on this? I plan to send a new version of this series soon
and I'd like to know if I should drop pdata/avr32 support or not. Note
that I'm targeting 4.12, so, as long as you drop avr32 support in 4.12
we should be good.

Thanks,

Boris

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Hans-Christian Noren Egtvedt]

Around Wed 01 Mar 2017 09:22:24 +0100 or thereabout, Boris Brezillon wrote:
> Hi Hans-Christian,
> 
> On Fri, 24 Feb 2017 10:04:35 +0100
> Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:
> 
>> Around Fri 24 Feb 2017 09:55:09 +0100 or thereabout, Boris Brezillon wrote:
>> > On Fri, 24 Feb 2017 09:52:09 +0100
>> > Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:  
>> >> Around Fri 24 Feb 2017 09:27:42 +0100 or thereabout, Boris Brezillon wrote:  
>> >> > On Fri, 24 Feb 2017 09:14:30 +0100 Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:    
>> >> >> Around Thu 23 Feb 2017 21:18:13 -0800 or thereabout, H?vard Skinnemoen wrote:    
>> >> >> > On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
>> >> >> > <alexandre.belloni@free-electrons.com> wrote:      
>> >> >> >> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:      
>> >> 
>> >> <snipp>
>> >>   
>> >> >> >> If nobody complains about the 4.10 breakage, You'll have plenty of time
>> >> >> >> to remove it for 4.12      
>> >> >> > 
>> >> >> > I'm fine with that, but I haven't put much effort into keeping it
>> >> >> > alive lately. If Hans-Christian agrees, I'm willing to post a patch to
>> >> >> > remove it, or ack someone else's patch.      
>> >> >> 
>> >> >> Then lets plan this for 4.12, either you H?vard whip up a patch or I can
>> >> >> eventually do it.
>> >> >> 
>> >> >> I can push it through the linux-avr32 git tree on kernel.org.
>> >> >>     
>> >> > 
>> >> > Can you do that just after 4.11-rc1 is released and provide a topic
>> >> > branch I can pull in my nand/next branch, so that I can rework this
>> >> > patch and drop all the pdata-compat code (as suggested by Andy).    
>> >> 
>> >> OK, I will try to prepare it during the weekend.
>> >> 
>> >> Any reason to wait for 4.11-rc1? AFAIK Linus prefers the larger changes
>> >> before he starts tagging rc's.
>> >>   
>> > 
>> > Oh, so you want to queue it for 4.11, that's even better.  
>> 
>> Perhaps I misunderstood you, by after 4.11-rc1 you mean queue it for 4.12?
>> 
>> I will see what I get around to do in the weekend, it should be pretty
>> straightforward, just want to make sure we remove all the bits.
>> 
> 
> Any progress on this? I plan to send a new version of this series soon
> and I'd like to know if I should drop pdata/avr32 support or not. Note
> that I'm targeting 4.12, so, as long as you drop avr32 support in 4.12
> we should be good.

I got around to make the patch series during the weekend, but I thought it
would be a good idea sending them to the kernel mailing list as a FYI.

Also, I was unsure if I should send the driver removals through the
sub-maintainers trees, or if I can push them through linux-avr32 tree.

I have a patch removing the pata driver for AVR32.

-- 
mvh
Hans-Christian Noren Egtvedt

[PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Boris Brezillon]

On Wed, 1 Mar 2017 09:38:07 +0100
Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:

> Around Wed 01 Mar 2017 09:22:24 +0100 or thereabout, Boris Brezillon wrote:
> > Hi Hans-Christian,
> > 
> > On Fri, 24 Feb 2017 10:04:35 +0100
> > Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:
> >   
> >> Around Fri 24 Feb 2017 09:55:09 +0100 or thereabout, Boris Brezillon wrote:  
> >> > On Fri, 24 Feb 2017 09:52:09 +0100
> >> > Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:    
> >> >> Around Fri 24 Feb 2017 09:27:42 +0100 or thereabout, Boris Brezillon wrote:    
> >> >> > On Fri, 24 Feb 2017 09:14:30 +0100 Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> wrote:      
> >> >> >> Around Thu 23 Feb 2017 21:18:13 -0800 or thereabout, H?vard Skinnemoen wrote:      
> >> >> >> > On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
> >> >> >> > <alexandre.belloni@free-electrons.com> wrote:        
> >> >> >> >> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:        
> >> >> 
> >> >> <snipp>
> >> >>     
> >> >> >> >> If nobody complains about the 4.10 breakage, You'll have plenty of time
> >> >> >> >> to remove it for 4.12        
> >> >> >> > 
> >> >> >> > I'm fine with that, but I haven't put much effort into keeping it
> >> >> >> > alive lately. If Hans-Christian agrees, I'm willing to post a patch to
> >> >> >> > remove it, or ack someone else's patch.        
> >> >> >> 
> >> >> >> Then lets plan this for 4.12, either you H?vard whip up a patch or I can
> >> >> >> eventually do it.
> >> >> >> 
> >> >> >> I can push it through the linux-avr32 git tree on kernel.org.
> >> >> >>       
> >> >> > 
> >> >> > Can you do that just after 4.11-rc1 is released and provide a topic
> >> >> > branch I can pull in my nand/next branch, so that I can rework this
> >> >> > patch and drop all the pdata-compat code (as suggested by Andy).      
> >> >> 
> >> >> OK, I will try to prepare it during the weekend.
> >> >> 
> >> >> Any reason to wait for 4.11-rc1? AFAIK Linus prefers the larger changes
> >> >> before he starts tagging rc's.
> >> >>     
> >> > 
> >> > Oh, so you want to queue it for 4.11, that's even better.    
> >> 
> >> Perhaps I misunderstood you, by after 4.11-rc1 you mean queue it for 4.12?
> >> 
> >> I will see what I get around to do in the weekend, it should be pretty
> >> straightforward, just want to make sure we remove all the bits.
> >>   
> > 
> > Any progress on this? I plan to send a new version of this series soon
> > and I'd like to know if I should drop pdata/avr32 support or not. Note
> > that I'm targeting 4.12, so, as long as you drop avr32 support in 4.12
> > we should be good.  
> 
> I got around to make the patch series during the weekend, but I thought it
> would be a good idea sending them to the kernel mailing list as a FYI.

Definitely.

> 
> Also, I was unsure if I should send the driver removals through the
> sub-maintainers trees, or if I can push them through linux-avr32 tree.

I think it should go through the sub-maintainers trees, but I guess you
can remove the arch code without breaking the build, so it shouldn't be
a problem if drivers removal don't go through the avr32 tree.

> 
> I have a patch removing the pata driver for AVR32.
> 

Great! Just send everything to the MLs (and relevant maintainers).

Thanks,

Boris

[PATCH v2 3/3] mtd: nand: Remove unused chip->write_page() hook

[Masahiro Yamada]

Hi Boris,

2017-02-20 21:28 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> The last/only user of the chip->write_page() hook (the Atmel NAND
> controller driver) has been reworked and is no longer specifying a custom
> ->write_page() implementation.
> Drop this hook before someone else start abusing it.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>


Just a minor comment.


/**
 * nand_write_page - [REPLACEABLE] write one page
 * @mtd: MTD device structure


Can you update the comment block for nand_write_page(), too?

  [REPLACEABLE]  ->  [INTERNAL]




-- 
Best Regards
Masahiro Yamada

[PATCH v2 3/3] mtd: nand: Remove unused chip->write_page() hook

[Boris Brezillon]

On Tue, 7 Mar 2017 21:04:50 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Hi Boris,
> 
> 2017-02-20 21:28 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> > The last/only user of the chip->write_page() hook (the Atmel NAND
> > controller driver) has been reworked and is no longer specifying a custom  
> > ->write_page() implementation.  
> > Drop this hook before someone else start abusing it.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>  
> 
> 
> Just a minor comment.
> 
> 
> /**
>  * nand_write_page - [REPLACEABLE] write one page
>  * @mtd: MTD device structure
> 
> 
> Can you update the comment block for nand_write_page(), too?
> 
>   [REPLACEABLE]  ->  [INTERNAL]

I think I'll just drop it, it does not mean anything to flag a static
function as replaceable (the ->write_page() method could be overloaded,
but not the nand_write_page() implementation itself).

[PATCH v2 3/3] mtd: nand: Remove unused chip->write_page() hook

[Masahiro Yamada]

2017-03-08 3:34 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> On Tue, 7 Mar 2017 21:04:50 +0900
> Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
>> Hi Boris,
>>
>> 2017-02-20 21:28 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
>> > The last/only user of the chip->write_page() hook (the Atmel NAND
>> > controller driver) has been reworked and is no longer specifying a custom
>> > ->write_page() implementation.
>> > Drop this hook before someone else start abusing it.
>> >
>> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
>>
>>
>> Just a minor comment.
>>
>>
>> /**
>>  * nand_write_page - [REPLACEABLE] write one page
>>  * @mtd: MTD device structure
>>
>>
>> Can you update the comment block for nand_write_page(), too?
>>
>>   [REPLACEABLE]  ->  [INTERNAL]
>
> I think I'll just drop it, it does not mean anything to flag a static
> function as replaceable (the ->write_page() method could be overloaded,
> but not the nand_write_page() implementation itself).
>

Sounds good to me.   Thank you!



-- 
Best Regards
Masahiro Yamada