Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1584574 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/3] mtd: nand: Rework/cleanup the Atmel NAND driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-02-20 13:30 +0100
    [PATCH v2 3/3] mtd: nand: Remove unused chip->write_page() hook Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-02-20 13:30 +0100
    Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-20 21:30 +0100
      Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-02-20 21:40 +0100
        Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-02-20 22:00 +0100
          Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-21 00:50 +0100
            Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-21 01:00 +0100
              Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-02-21 09:10 +0100
                Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-21 11:10 +0100
                  Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-02-21 11:30 +0100
                    Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Nicolas Ferre <nicolas.ferre@atmel.com> - 2017-02-21 11:50 +0100
                    Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-21 12:10 +0100
                      Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-02-21 12:30 +0100
                        Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-21 17:10 +0100
                          Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-02-21 17:30 +0100
                            Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-21 17:40 +0100
                              Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-21 17:50 +0100
                                Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-02-21 18:20 +0100
                                  Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Håvard Skinnemoen <hskinnemoen@gmail.com> - 2017-02-24 06:20 +0100
                                    Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-02-24 09:30 +0100
                                      Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-02-24 10:00 +0100
                                        Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-02-24 10:40 +0100
                                        Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-02-24 11:00 +0100
                                          Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-24 12:50 +0100
                                        Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> - 2017-02-24 11:10 +0100
                                      Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> - 2017-02-24 10:40 +0100
                                    Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-02-24 10:30 +0100
                                    Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Hans-Christian Noren Egtvedt <egtvedt@samfundet.no> - 2017-02-24 10:40 +0100
                              Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-02-21 18:10 +0100
                      Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-02-21 12:30 +0100
                        Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Nicolas Ferre <nicolas.ferre@microchip.com> - 2017-02-21 14:50 +0100
                        Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-21 17:00 +0100
                          Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-02-21 17:20 +0100
                      Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-21 15:00 +0100
    Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver Nicolas Ferre <nicolas.ferre@microchip.com> - 2017-02-21 14:10 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1587420 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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.

[toc] | [prev] | [next] | [standalone]

#1587460 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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.

[toc] | [prev] | [next] | [standalone]

#1587481 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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

[toc] | [prev] | [next] | [standalone]

#1587534 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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

[toc] | [prev] | [next] | [standalone]

#1587489 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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

[toc] | [prev] | [next] | [standalone]

#1587462 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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

[toc] | [prev] | [next] | [standalone]

#1587456 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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

[toc] | [prev] | [next] | [standalone]

#1587463 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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

[toc] | [prev] | [next] | [standalone]

#1585532 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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

[toc] | [prev] | [next] | [standalone]

#1585235 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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.

[toc] | [prev] | [next] | [standalone]

#1585343 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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

[toc] | [prev] | [next] | [standalone]

#1585463 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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

[toc] | [prev] | [next] | [standalone]

#1585489 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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

[toc] | [prev] | [next] | [standalone]

#1585348 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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.

[toc] | [prev] | [next] | [standalone]

#1585309 — Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

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

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web