from:"Daniel Golle"

Re: [PATCH 3/4] clocksource/drivers/oxnas: Fix OX820 compatible

2019-04-16 Thread Daniel Golle

Works great here

On Tue, Apr 16, 2019 at 02:46:39PM +0200, Daniel Lezcano wrote:
> From: Neil Armstrong 
> 
> The OX820 compatible is wrong is the driver, fix it.
> 
> Fixes: 2ea3401e2a84 ("clocksource/drivers/oxnas: Add OX820 compatible")
> Reported-by: Daniel Golle 
> Signed-off-by: Neil Armstrong 
> Signed-off-by: Daniel Lezcano 
Tested-by: Daniel Golle 

> ---
>  drivers/clocksource/timer-oxnas-rps.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clocksource/timer-oxnas-rps.c 
> b/drivers/clocksource/timer-oxnas-rps.c
> index eed6feff8b5f..30c6f4ce672b 100644
> --- a/drivers/clocksource/timer-oxnas-rps.c
> +++ b/drivers/clocksource/timer-oxnas-rps.c
> @@ -296,4 +296,4 @@ static int __init oxnas_rps_timer_init(struct device_node 
> *np)
>  TIMER_OF_DECLARE(ox810se_rps,
>  "oxsemi,ox810se-rps-timer", oxnas_rps_timer_init);
>  TIMER_OF_DECLARE(ox820_rps,
> -"oxsemi,ox820se-rps-timer", oxnas_rps_timer_init);
> +"oxsemi,ox820-rps-timer", oxnas_rps_timer_init);
> -- 
> 2.17.1
>

Re: [PATCH 1/2] libata: add ledtrig support

2018-09-21 Thread Daniel Golle

Hi Pavel,

On Fri, Sep 21, 2018 at 12:04:49AM +0200, Pavel Machek wrote:
> Hi!
> 
> > > > +#ifdef CONFIG_ATA_LEDS
> > > > +   /* register LED triggers for all ports */
> > > > +   for (i = 0; i < host->n_ports; i++) {
> > > > +   if (unlikely(!host->ports[i]->ledtrig))
> > > > +   continue;
> > > > +
> > > > +   snprintf(host->ports[i]->ledtrig_name,
> > > > +   sizeof(host->ports[i]->ledtrig_name), "ata%u",
> > > > +   host->ports[i]->print_id);
> > > 
> > > > +   host->ports[i]->ledtrig->name = 
> > > > host->ports[i]->ledtrig_name;
> > > > +
> > > > +   if (led_trigger_register(host->ports[i]->ledtrig)) {
> > > > +   kfree(host->ports[i]->ledtrig);
> > > > +   host->ports[i]->ledtrig = NULL;
> > > > +   }
> > > > +   }
> > > > +#endif
> > > 
> > > No, we don't want you to register multiple triggers. We want one
> > > trigger, than has parameter "which port to watch". (Number of triggers
> > > is limited as by sysfs limitations).
> > 
> > Back then I implemented it that way to be able to define the
> > default trigger for each LED in device tree and "trigger-sources"
> > didn't exist yet (it was introduced for USB ports and isn't yet used
> > for anything other than that)
> 
> I see why you did it... BUt I believe we still want single trigger solution...
> 
> > However, the problem till today is also that ATA ports are often not
> > individual device-tree objects we can refer to, see for example
> > marvell,armada-370-sata which appears as one opaque controller. Ie.
> > all SATA drivers have to be converted to expose individual ports on
> > device-tree before the trigger-sources approach can be applied...
> 
> Yep, well... something to do in SATA then.
> 
> Perhaps this should also have an option for single LED for _any_ SATA 
> activity,
> and 90% devices will be happy with that?

The whole reason for not using one of the existing disk-activity
triggers was to address individual SATA ports to individual LEDs of NAS
devices (in my case Shuttle KD20)...

Re: [PATCH 1/2] libata: add ledtrig support

2018-09-21 Thread Daniel Golle

Hi Pavel,

On Fri, Sep 21, 2018 at 12:04:49AM +0200, Pavel Machek wrote:
> Hi!
> 
> > > > +#ifdef CONFIG_ATA_LEDS
> > > > +   /* register LED triggers for all ports */
> > > > +   for (i = 0; i < host->n_ports; i++) {
> > > > +   if (unlikely(!host->ports[i]->ledtrig))
> > > > +   continue;
> > > > +
> > > > +   snprintf(host->ports[i]->ledtrig_name,
> > > > +   sizeof(host->ports[i]->ledtrig_name), "ata%u",
> > > > +   host->ports[i]->print_id);
> > > 
> > > > +   host->ports[i]->ledtrig->name = 
> > > > host->ports[i]->ledtrig_name;
> > > > +
> > > > +   if (led_trigger_register(host->ports[i]->ledtrig)) {
> > > > +   kfree(host->ports[i]->ledtrig);
> > > > +   host->ports[i]->ledtrig = NULL;
> > > > +   }
> > > > +   }
> > > > +#endif
> > > 
> > > No, we don't want you to register multiple triggers. We want one
> > > trigger, than has parameter "which port to watch". (Number of triggers
> > > is limited as by sysfs limitations).
> > 
> > Back then I implemented it that way to be able to define the
> > default trigger for each LED in device tree and "trigger-sources"
> > didn't exist yet (it was introduced for USB ports and isn't yet used
> > for anything other than that)
> 
> I see why you did it... BUt I believe we still want single trigger solution...
> 
> > However, the problem till today is also that ATA ports are often not
> > individual device-tree objects we can refer to, see for example
> > marvell,armada-370-sata which appears as one opaque controller. Ie.
> > all SATA drivers have to be converted to expose individual ports on
> > device-tree before the trigger-sources approach can be applied...
> 
> Yep, well... something to do in SATA then.
> 
> Perhaps this should also have an option for single LED for _any_ SATA 
> activity,
> and 90% devices will be happy with that?

The whole reason for not using one of the existing disk-activity
triggers was to address individual SATA ports to individual LEDs of NAS
devices (in my case Shuttle KD20)...

Re: [PATCH 1/2] libata: add ledtrig support

2018-09-20 Thread Daniel Golle

Hi!

On Thu, Sep 20, 2018 at 09:23:54AM +0200, Pavel Machek wrote:
> Hi!
> 
> > +#ifdef CONFIG_ATA_LEDS
> > +   /* register LED triggers for all ports */
> > +   for (i = 0; i < host->n_ports; i++) {
> > +   if (unlikely(!host->ports[i]->ledtrig))
> > +   continue;
> > +
> > +   snprintf(host->ports[i]->ledtrig_name,
> > +   sizeof(host->ports[i]->ledtrig_name), "ata%u",
> > +   host->ports[i]->print_id);
> 
> > +   host->ports[i]->ledtrig->name = host->ports[i]->ledtrig_name;
> > +
> > +   if (led_trigger_register(host->ports[i]->ledtrig)) {
> > +   kfree(host->ports[i]->ledtrig);
> > +   host->ports[i]->ledtrig = NULL;
> > +   }
> > +   }
> > +#endif
> 
> No, we don't want you to register multiple triggers. We want one
> trigger, than has parameter "which port to watch". (Number of triggers
> is limited as by sysfs limitations).

Back then I implemented it that way to be able to define the
default trigger for each LED in device tree and "trigger-sources"
didn't exist yet (it was introduced for USB ports and isn't yet used
for anything other than that)
However, the problem till today is also that ATA ports are often not
individual device-tree objects we can refer to, see for example
marvell,armada-370-sata which appears as one opaque controller. Ie.
all SATA drivers have to be converted to expose individual ports on
device-tree before the trigger-sources approach can be applied...

> 
> Otherwise yes, ata trigger makes sense.
>   Pavel
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/2] libata: add ledtrig support

2018-09-20 Thread Daniel Golle

Hi!

On Thu, Sep 20, 2018 at 09:23:54AM +0200, Pavel Machek wrote:
> Hi!
> 
> > +#ifdef CONFIG_ATA_LEDS
> > +   /* register LED triggers for all ports */
> > +   for (i = 0; i < host->n_ports; i++) {
> > +   if (unlikely(!host->ports[i]->ledtrig))
> > +   continue;
> > +
> > +   snprintf(host->ports[i]->ledtrig_name,
> > +   sizeof(host->ports[i]->ledtrig_name), "ata%u",
> > +   host->ports[i]->print_id);
> 
> > +   host->ports[i]->ledtrig->name = host->ports[i]->ledtrig_name;
> > +
> > +   if (led_trigger_register(host->ports[i]->ledtrig)) {
> > +   kfree(host->ports[i]->ledtrig);
> > +   host->ports[i]->ledtrig = NULL;
> > +   }
> > +   }
> > +#endif
> 
> No, we don't want you to register multiple triggers. We want one
> trigger, than has parameter "which port to watch". (Number of triggers
> is limited as by sysfs limitations).

Back then I implemented it that way to be able to define the
default trigger for each LED in device tree and "trigger-sources"
didn't exist yet (it was introduced for USB ports and isn't yet used
for anything other than that)
However, the problem till today is also that ATA ports are often not
individual device-tree objects we can refer to, see for example
marvell,armada-370-sata which appears as one opaque controller. Ie.
all SATA drivers have to be converted to expose individual ports on
device-tree before the trigger-sources approach can be applied...

> 
> Otherwise yes, ata trigger makes sense.
>   Pavel
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] rt2x00: improve calling conventions for register accessors

2017-05-15 Thread Daniel Golle

On Mon, May 15, 2017 at 10:40:52AM -0400, David Miller wrote:
> From: Arnd Bergmann <a...@arndb.de>
> Date: Mon, 15 May 2017 16:36:45 +0200
> 
> > On Mon, May 15, 2017 at 4:28 PM, Stanislaw Gruszka <sgrus...@redhat.com> 
> > wrote:
> >> On Mon, May 15, 2017 at 03:46:55PM +0200, Arnd Bergmann wrote:
> >>> With CONFIG_KASAN enabled and gcc-7, we get a warning about rather high
> >>> stack usage (with a private patch set I have to turn on this warning,
> >>> which I intend to get into the next kernel release):
> >>>
> >>> wireless/ralink/rt2x00/rt2800lib.c: In function 
> >>> 'rt2800_bw_filter_calibration':
> >>> wireless/ralink/rt2x00/rt2800lib.c:7990:1: error: the frame size of 2144 
> >>> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> >>>
> >>> The problem is that KASAN inserts a redzone around each local variable 
> >>> that
> >>> gets passed by reference, and the newly added function has a lot of them.
> >>> We can easily avoid that here by changing the calling convention to have
> >>> the output as the return value of the function. This should also results 
> >>> in
> >>> smaller object code, saving around 4KB in .text with KASAN, or 2KB without
> >>> KASAN.
> >>>
> >>> Fixes: 41977e86c984 ("rt2x00: add support for MT7620")
> >>> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> >>> ---
> >>>  drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 319 
> >>> +
> >>>  1 file changed, 164 insertions(+), 155 deletions(-)
> >>
> >> We have read(, ) calling convention since forever in rt2x00 and that
> >> was never a problem. I dislike to change that now to make some tools
> >> happy, I think problem should be fixed in the tools instead.
> > 
> > How about adding 'depends on !KASAN' in Kconfig instead?
> 
> Please let's not go down that route and make such facilities less
> useful due to decreased coverage.

Being the one to blame for submitting the patch adding most of the
problem's footprint: Arnd's change looks good to me and I believe it
should be merged.
This is the type of feedback I was hoping for when submitting all the
long-forgotten and rotting patches from OpenWrt's mac80211 driver
patches! Thanks to Arnd for your efforts!

Consider this as
Acked-by: Daniel Golle <dan...@makrotopia.org>
for Arnd's original patch (and for NOT adding 'depends on !KASAN')

Cheers


Daniel

> 
> Thanks.

Re: [PATCH] rt2x00: improve calling conventions for register accessors

2017-05-15 Thread Daniel Golle

On Mon, May 15, 2017 at 10:40:52AM -0400, David Miller wrote:
> From: Arnd Bergmann 
> Date: Mon, 15 May 2017 16:36:45 +0200
> 
> > On Mon, May 15, 2017 at 4:28 PM, Stanislaw Gruszka  
> > wrote:
> >> On Mon, May 15, 2017 at 03:46:55PM +0200, Arnd Bergmann wrote:
> >>> With CONFIG_KASAN enabled and gcc-7, we get a warning about rather high
> >>> stack usage (with a private patch set I have to turn on this warning,
> >>> which I intend to get into the next kernel release):
> >>>
> >>> wireless/ralink/rt2x00/rt2800lib.c: In function 
> >>> 'rt2800_bw_filter_calibration':
> >>> wireless/ralink/rt2x00/rt2800lib.c:7990:1: error: the frame size of 2144 
> >>> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> >>>
> >>> The problem is that KASAN inserts a redzone around each local variable 
> >>> that
> >>> gets passed by reference, and the newly added function has a lot of them.
> >>> We can easily avoid that here by changing the calling convention to have
> >>> the output as the return value of the function. This should also results 
> >>> in
> >>> smaller object code, saving around 4KB in .text with KASAN, or 2KB without
> >>> KASAN.
> >>>
> >>> Fixes: 41977e86c984 ("rt2x00: add support for MT7620")
> >>> Signed-off-by: Arnd Bergmann 
> >>> ---
> >>>  drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 319 
> >>> +
> >>>  1 file changed, 164 insertions(+), 155 deletions(-)
> >>
> >> We have read(, ) calling convention since forever in rt2x00 and that
> >> was never a problem. I dislike to change that now to make some tools
> >> happy, I think problem should be fixed in the tools instead.
> > 
> > How about adding 'depends on !KASAN' in Kconfig instead?
> 
> Please let's not go down that route and make such facilities less
> useful due to decreased coverage.

Being the one to blame for submitting the patch adding most of the
problem's footprint: Arnd's change looks good to me and I believe it
should be merged.
This is the type of feedback I was hoping for when submitting all the
long-forgotten and rotting patches from OpenWrt's mac80211 driver
patches! Thanks to Arnd for your efforts!

Consider this as
Acked-by: Daniel Golle 
for Arnd's original patch (and for NOT adding 'depends on !KASAN')

Cheers


Daniel

> 
> Thanks.

Re: [RFC PATCH] mtd: nand: Add OX820 NAND Support

2016-10-18 Thread Daniel Golle

Hi Neil,

On Tue, Oct 18, 2016 at 01:24:22PM +0200, Neil Armstrong wrote:
> On 10/18/2016 01:08 PM, Daniel Golle wrote:
> > Hi Neil,
> > 
> > great to see progress towards supporting OX820!
> > The NAND driver I hacked up for Kernel 4.1 and 4.4 in OpenWrt/LEDE
> > looks very similar, see
> > 
> > https://git.lede-project.org/?p=lede/dangole/staging.git;a=blob;f=target/linux/oxnas/files/drivers/mtd/nand/oxnas_nand.c;h=f5a142950e32227fee304de731e619278350a91b;hb=refs/heads/oxnas-nand
> > 
> > To me therefore this looks quite good, just one small question below.
> 
> Hi Daniel,
> 
> Yes, I work step by step, I hope to have something booting for 4.10 !
> 
> Indeed, they look identical except the part_probes[] stuff, are they 
> necessary ?

Nah, this was dropped around kernel 4.7 from most drivers in favour of
using the default partition probes (ie. cmdline and dt) afaik.

> 
> My primary source of code was Ma Haiju's tree and OPenWRT's tree, but would 
> some people like to push some of the openwrt driver upstream somehow ?

I was planning this somehow, but lack the resources to seriously move
forward. I've mostly been focussing on cleaning up Ma Haiju's code and
improving support for the SATA driver (ie. adding support for both
ports) and re-factoring the stmmac-based Ethernet driver to match the
current kernel driver's style.
However, I'm still using platform-specific includes (hardware.h) and
thus look forward to rebase SATA and Ethernet support on top of your
tree, as that would unluck ox820 support in multi-v6 instead of having
a platform-specific kernel.

Currently there are problems with the NAND driver failing to detect the
chip on non-PCIe platform like the PogoPlug V3 (non-Pro), MitraStar
STG-212 and probably other non-PCIe boards which I didn't check yet.
This magically happened when switching from kernel 4.1 to 4.4, I'm
bisecting this since, but the problem seems to be hidden somewhere
between the lines (ie. probe-order-related or such)...
Did you test your NAND driver on any non-PCIe boards and did it work?


For a more or less complete history of the changes I made since
branching of Ma Haijun's tree, see

https://git.lede-project.org/?p=lede/dangole/staging.git;a=history;f=target/linux/oxnas;hb=refs/heads/oxnas-nand

and for the very early history, prior to the merge into the official
OpenWrt tree, see

http://gitorious.org/openwrt-oxnas/openwrt-oxnas.git


Cheers


Daniel

> 
> Thanks,
> Neil
> 
> > 
> > Cheers
> > 
> > Daniel
> > 
> > 
> > On Tue, Oct 18, 2016 at 11:09:27AM +0200, Neil Armstrong wrote:
> >> Add NAND driver to support the Oxford Semiconductor OX820 NAND Controller.
> >> This is a simple memory mapped NAND controller with single chip select and
> >> software ECC.
> >>
> >> Signed-off-by: Neil Armstrong <narmstr...@baylibre.com>
> >> ---
> >>  .../devicetree/bindings/mtd/oxnas-nand.txt |  24 
> >>  drivers/mtd/nand/Kconfig   |   5 +
> >>  drivers/mtd/nand/Makefile  |   1 +
> >>  drivers/mtd/nand/oxnas_nand.c  | 144 
> >> +
> >>  4 files changed, 174 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mtd/oxnas-nand.txt
> >>  create mode 100644 drivers/mtd/nand/oxnas_nand.c
> >>
> 
> [...]
> 
> >> diff --git a/drivers/mtd/nand/oxnas_nand.c b/drivers/mtd/nand/oxnas_nand.c
> >> new file mode 100644
> >> index 000..ee402ab
> >> --- /dev/null
> >> +++ b/drivers/mtd/nand/oxnas_nand.c
> >> @@ -0,0 +1,144 @@
> >> +/*
> >> + * Oxford Semiconductor OXNAS NAND driver
> >> + *
> >> + * Heavily based on plat_nand.c :
> >> + * Author: Vitaly Wool <vitalyw...@gmail.com>
> 
> Hmm, I forgot the OpenWRT and Ma Haijun copyrights here...
> 
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + *
> >> + */
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +/* nand commands */
> >> +#define NAND_CMD_ALE  BIT(18)
> >> +#define NAND_CMD_CLE  BIT(19)
> >> +#define NAND_CMD_CS   0
> >> +#define NAND_CMD_RESET0xff
> >>

Re: [RFC PATCH] mtd: nand: Add OX820 NAND Support

2016-10-18 Thread Daniel Golle

Hi Neil,

On Tue, Oct 18, 2016 at 01:24:22PM +0200, Neil Armstrong wrote:
> On 10/18/2016 01:08 PM, Daniel Golle wrote:
> > Hi Neil,
> > 
> > great to see progress towards supporting OX820!
> > The NAND driver I hacked up for Kernel 4.1 and 4.4 in OpenWrt/LEDE
> > looks very similar, see
> > 
> > https://git.lede-project.org/?p=lede/dangole/staging.git;a=blob;f=target/linux/oxnas/files/drivers/mtd/nand/oxnas_nand.c;h=f5a142950e32227fee304de731e619278350a91b;hb=refs/heads/oxnas-nand
> > 
> > To me therefore this looks quite good, just one small question below.
> 
> Hi Daniel,
> 
> Yes, I work step by step, I hope to have something booting for 4.10 !
> 
> Indeed, they look identical except the part_probes[] stuff, are they 
> necessary ?

Nah, this was dropped around kernel 4.7 from most drivers in favour of
using the default partition probes (ie. cmdline and dt) afaik.

> 
> My primary source of code was Ma Haiju's tree and OPenWRT's tree, but would 
> some people like to push some of the openwrt driver upstream somehow ?

I was planning this somehow, but lack the resources to seriously move
forward. I've mostly been focussing on cleaning up Ma Haiju's code and
improving support for the SATA driver (ie. adding support for both
ports) and re-factoring the stmmac-based Ethernet driver to match the
current kernel driver's style.
However, I'm still using platform-specific includes (hardware.h) and
thus look forward to rebase SATA and Ethernet support on top of your
tree, as that would unluck ox820 support in multi-v6 instead of having
a platform-specific kernel.

Currently there are problems with the NAND driver failing to detect the
chip on non-PCIe platform like the PogoPlug V3 (non-Pro), MitraStar
STG-212 and probably other non-PCIe boards which I didn't check yet.
This magically happened when switching from kernel 4.1 to 4.4, I'm
bisecting this since, but the problem seems to be hidden somewhere
between the lines (ie. probe-order-related or such)...
Did you test your NAND driver on any non-PCIe boards and did it work?


For a more or less complete history of the changes I made since
branching of Ma Haijun's tree, see

https://git.lede-project.org/?p=lede/dangole/staging.git;a=history;f=target/linux/oxnas;hb=refs/heads/oxnas-nand

and for the very early history, prior to the merge into the official
OpenWrt tree, see

http://gitorious.org/openwrt-oxnas/openwrt-oxnas.git


Cheers


Daniel

> 
> Thanks,
> Neil
> 
> > 
> > Cheers
> > 
> > Daniel
> > 
> > 
> > On Tue, Oct 18, 2016 at 11:09:27AM +0200, Neil Armstrong wrote:
> >> Add NAND driver to support the Oxford Semiconductor OX820 NAND Controller.
> >> This is a simple memory mapped NAND controller with single chip select and
> >> software ECC.
> >>
> >> Signed-off-by: Neil Armstrong 
> >> ---
> >>  .../devicetree/bindings/mtd/oxnas-nand.txt |  24 
> >>  drivers/mtd/nand/Kconfig   |   5 +
> >>  drivers/mtd/nand/Makefile  |   1 +
> >>  drivers/mtd/nand/oxnas_nand.c  | 144 
> >> +
> >>  4 files changed, 174 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mtd/oxnas-nand.txt
> >>  create mode 100644 drivers/mtd/nand/oxnas_nand.c
> >>
> 
> [...]
> 
> >> diff --git a/drivers/mtd/nand/oxnas_nand.c b/drivers/mtd/nand/oxnas_nand.c
> >> new file mode 100644
> >> index 000..ee402ab
> >> --- /dev/null
> >> +++ b/drivers/mtd/nand/oxnas_nand.c
> >> @@ -0,0 +1,144 @@
> >> +/*
> >> + * Oxford Semiconductor OXNAS NAND driver
> >> + *
> >> + * Heavily based on plat_nand.c :
> >> + * Author: Vitaly Wool 
> 
> Hmm, I forgot the OpenWRT and Ma Haijun copyrights here...
> 
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + *
> >> + */
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +/* nand commands */
> >> +#define NAND_CMD_ALE  BIT(18)
> >> +#define NAND_CMD_CLE  BIT(19)
> >> +#define NAND_CMD_CS   0
> >> +#define NAND_CMD_RESET0xff
> >> +#define NAND_CMD  (NAND_CMD_CS | NAND_CMD_CLE)
> &g

Re: [RFC PATCH] mtd: nand: Add OX820 NAND Support

2016-10-18 Thread Daniel Golle

Hi Neil,

great to see progress towards supporting OX820!
The NAND driver I hacked up for Kernel 4.1 and 4.4 in OpenWrt/LEDE
looks very similar, see

https://git.lede-project.org/?p=lede/dangole/staging.git;a=blob;f=target/linux/oxnas/files/drivers/mtd/nand/oxnas_nand.c;h=f5a142950e32227fee304de731e619278350a91b;hb=refs/heads/oxnas-nand

To me therefore this looks quite good, just one small question below.

Cheers

Daniel


On Tue, Oct 18, 2016 at 11:09:27AM +0200, Neil Armstrong wrote:
> Add NAND driver to support the Oxford Semiconductor OX820 NAND Controller.
> This is a simple memory mapped NAND controller with single chip select and
> software ECC.
> 
> Signed-off-by: Neil Armstrong 
> ---
>  .../devicetree/bindings/mtd/oxnas-nand.txt |  24 
>  drivers/mtd/nand/Kconfig   |   5 +
>  drivers/mtd/nand/Makefile  |   1 +
>  drivers/mtd/nand/oxnas_nand.c  | 144 
> +
>  4 files changed, 174 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/oxnas-nand.txt
>  create mode 100644 drivers/mtd/nand/oxnas_nand.c
> 
> diff --git a/Documentation/devicetree/bindings/mtd/oxnas-nand.txt 
> b/Documentation/devicetree/bindings/mtd/oxnas-nand.txt
> new file mode 100644
> index 000..83b684d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/oxnas-nand.txt
> @@ -0,0 +1,24 @@
> +* Oxford Semiconductor OXNAS NAND Controller
> +
> +Please refer to nand.txt for generic information regarding MTD NAND bindings.
> +
> +Required properties:
> + - compatible: "oxsemi,ox820-nand"
> + - reg: Base address and length for NAND mapped memory.
> +
> +Optional Properties:
> + - clocks: phandle to the NAND gate clock if needed.
> + - resets: phandle to the NAND reset control if needed.
> +
> +Example:
> +
> +nand: nand@4100 {
> + compatible = "oxsemi,ox820-nand";
> + reg = <0x4100 0x10>;
> + nand-ecc-mode = "soft";
> + clocks = < CLK_820_NAND>;
> + resets = < RESET_NAND>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + status = "disabled";
> +};
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 7b7a887..c023125 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -426,6 +426,11 @@ config MTD_NAND_ORION
> No board specific support is done by this driver, each board
> must advertise a platform_device for the driver to attach.
>  
> +config MTD_NAND_OXNAS
> + tristate "NAND Flash support for Oxford Semiconductor SoC"
> + help
> +   This enables the NAND flash controller on Oxford Semiconductor SoCs.
> +
>  config MTD_NAND_FSL_ELBC
>   tristate "NAND support for Freescale eLBC controllers"
>   depends on FSL_SOC
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index cafde6f..05fc054 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -35,6 +35,7 @@ obj-$(CONFIG_MTD_NAND_TMIO) += tmio_nand.o
>  obj-$(CONFIG_MTD_NAND_PLATFORM)  += plat_nand.o
>  obj-$(CONFIG_MTD_NAND_PASEMI)+= pasemi_nand.o
>  obj-$(CONFIG_MTD_NAND_ORION) += orion_nand.o
> +obj-$(CONFIG_MTD_NAND_OXNAS) += oxnas_nand.o
>  obj-$(CONFIG_MTD_NAND_FSL_ELBC)  += fsl_elbc_nand.o
>  obj-$(CONFIG_MTD_NAND_FSL_IFC)   += fsl_ifc_nand.o
>  obj-$(CONFIG_MTD_NAND_FSL_UPM)   += fsl_upm.o
> diff --git a/drivers/mtd/nand/oxnas_nand.c b/drivers/mtd/nand/oxnas_nand.c
> new file mode 100644
> index 000..ee402ab
> --- /dev/null
> +++ b/drivers/mtd/nand/oxnas_nand.c
> @@ -0,0 +1,144 @@
> +/*
> + * Oxford Semiconductor OXNAS NAND driver
> + *
> + * Heavily based on plat_nand.c :
> + * Author: Vitaly Wool 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* nand commands */
> +#define NAND_CMD_ALE BIT(18)
> +#define NAND_CMD_CLE BIT(19)
> +#define NAND_CMD_CS  0
> +#define NAND_CMD_RESET   0xff
> +#define NAND_CMD (NAND_CMD_CS | NAND_CMD_CLE)
> +#define NAND_ADDR(NAND_CMD_CS | NAND_CMD_ALE)
> +#define NAND_DATA(NAND_CMD_CS)
> +
> +struct oxnas_nand_data {
> + struct nand_chipchip;
> + void __iomem*io_base;

Why do you redundantly keep io_base in platform data rather than
just using chip.IO_ADDR_R and >chip.IO_ADDR_W?


> + struct clk  *clk;
> +};
> +
> +static void oxnas_nand_cmd_ctrl(struct mtd_info *mtd, int cmd,
> + unsigned int ctrl)
> +{
> + struct nand_chip *this = mtd->priv;
> + unsigned long

Re: [RFC PATCH] mtd: nand: Add OX820 NAND Support

2016-10-18 Thread Daniel Golle

Hi Neil,

great to see progress towards supporting OX820!
The NAND driver I hacked up for Kernel 4.1 and 4.4 in OpenWrt/LEDE
looks very similar, see

https://git.lede-project.org/?p=lede/dangole/staging.git;a=blob;f=target/linux/oxnas/files/drivers/mtd/nand/oxnas_nand.c;h=f5a142950e32227fee304de731e619278350a91b;hb=refs/heads/oxnas-nand

To me therefore this looks quite good, just one small question below.

Cheers

Daniel


On Tue, Oct 18, 2016 at 11:09:27AM +0200, Neil Armstrong wrote:
> Add NAND driver to support the Oxford Semiconductor OX820 NAND Controller.
> This is a simple memory mapped NAND controller with single chip select and
> software ECC.
> 
> Signed-off-by: Neil Armstrong 
> ---
>  .../devicetree/bindings/mtd/oxnas-nand.txt |  24 
>  drivers/mtd/nand/Kconfig   |   5 +
>  drivers/mtd/nand/Makefile  |   1 +
>  drivers/mtd/nand/oxnas_nand.c  | 144 
> +
>  4 files changed, 174 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/oxnas-nand.txt
>  create mode 100644 drivers/mtd/nand/oxnas_nand.c
> 
> diff --git a/Documentation/devicetree/bindings/mtd/oxnas-nand.txt 
> b/Documentation/devicetree/bindings/mtd/oxnas-nand.txt
> new file mode 100644
> index 000..83b684d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/oxnas-nand.txt
> @@ -0,0 +1,24 @@
> +* Oxford Semiconductor OXNAS NAND Controller
> +
> +Please refer to nand.txt for generic information regarding MTD NAND bindings.
> +
> +Required properties:
> + - compatible: "oxsemi,ox820-nand"
> + - reg: Base address and length for NAND mapped memory.
> +
> +Optional Properties:
> + - clocks: phandle to the NAND gate clock if needed.
> + - resets: phandle to the NAND reset control if needed.
> +
> +Example:
> +
> +nand: nand@4100 {
> + compatible = "oxsemi,ox820-nand";
> + reg = <0x4100 0x10>;
> + nand-ecc-mode = "soft";
> + clocks = < CLK_820_NAND>;
> + resets = < RESET_NAND>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + status = "disabled";
> +};
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 7b7a887..c023125 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -426,6 +426,11 @@ config MTD_NAND_ORION
> No board specific support is done by this driver, each board
> must advertise a platform_device for the driver to attach.
>  
> +config MTD_NAND_OXNAS
> + tristate "NAND Flash support for Oxford Semiconductor SoC"
> + help
> +   This enables the NAND flash controller on Oxford Semiconductor SoCs.
> +
>  config MTD_NAND_FSL_ELBC
>   tristate "NAND support for Freescale eLBC controllers"
>   depends on FSL_SOC
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index cafde6f..05fc054 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -35,6 +35,7 @@ obj-$(CONFIG_MTD_NAND_TMIO) += tmio_nand.o
>  obj-$(CONFIG_MTD_NAND_PLATFORM)  += plat_nand.o
>  obj-$(CONFIG_MTD_NAND_PASEMI)+= pasemi_nand.o
>  obj-$(CONFIG_MTD_NAND_ORION) += orion_nand.o
> +obj-$(CONFIG_MTD_NAND_OXNAS) += oxnas_nand.o
>  obj-$(CONFIG_MTD_NAND_FSL_ELBC)  += fsl_elbc_nand.o
>  obj-$(CONFIG_MTD_NAND_FSL_IFC)   += fsl_ifc_nand.o
>  obj-$(CONFIG_MTD_NAND_FSL_UPM)   += fsl_upm.o
> diff --git a/drivers/mtd/nand/oxnas_nand.c b/drivers/mtd/nand/oxnas_nand.c
> new file mode 100644
> index 000..ee402ab
> --- /dev/null
> +++ b/drivers/mtd/nand/oxnas_nand.c
> @@ -0,0 +1,144 @@
> +/*
> + * Oxford Semiconductor OXNAS NAND driver
> + *
> + * Heavily based on plat_nand.c :
> + * Author: Vitaly Wool 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* nand commands */
> +#define NAND_CMD_ALE BIT(18)
> +#define NAND_CMD_CLE BIT(19)
> +#define NAND_CMD_CS  0
> +#define NAND_CMD_RESET   0xff
> +#define NAND_CMD (NAND_CMD_CS | NAND_CMD_CLE)
> +#define NAND_ADDR(NAND_CMD_CS | NAND_CMD_ALE)
> +#define NAND_DATA(NAND_CMD_CS)
> +
> +struct oxnas_nand_data {
> + struct nand_chipchip;
> + void __iomem*io_base;

Why do you redundantly keep io_base in platform data rather than
just using chip.IO_ADDR_R and >chip.IO_ADDR_W?


> + struct clk  *clk;
> +};
> +
> +static void oxnas_nand_cmd_ctrl(struct mtd_info *mtd, int cmd,
> + unsigned int ctrl)
> +{
> + struct nand_chip *this = mtd->priv;
> + unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
> +

Re: [PATCH 3/4] clocksource/drivers/oxnas: Fix OX820 compatible

Re: [PATCH 1/2] libata: add ledtrig support

Re: [PATCH 1/2] libata: add ledtrig support

Re: [PATCH 1/2] libata: add ledtrig support

Re: [PATCH 1/2] libata: add ledtrig support

Re: [PATCH] rt2x00: improve calling conventions for register accessors

Re: [PATCH] rt2x00: improve calling conventions for register accessors

Re: [RFC PATCH] mtd: nand: Add OX820 NAND Support

Re: [RFC PATCH] mtd: nand: Add OX820 NAND Support

Re: [RFC PATCH] mtd: nand: Add OX820 NAND Support

Re: [RFC PATCH] mtd: nand: Add OX820 NAND Support

11 matches

Site Navigation

Mail list logo

Footer information