ARM: mmp: support OF on pxa168
Subsystems (8)
v2 cover ARM: mmp: support OF on pxa168
Add OF support on gpio/serial/rtc/i2c components in pxa168 aspenite.
Changelog v2:
1. Append interrupt-names property in rtc-sa1100, gpio-pxa driver.
2. Append alias in i2c-pxa driver.
3. Avoid embed CONFIG_OF in i2c-pxa driver.
PATCH v2 1/7 serial: pxa: add OF support
Parse uart device id from alias in DTS file.
--- drivers/tty/serial/pxa.c | 65 +++++++++++++++++++++++++++++++++++++-------- 1 files changed, 53 insertions(+), 12 deletions(-)
drivers/tty/serial/pxa.c +53 -12
@@ -36,6 +36,7 @@#include <linux/circ_buf.h> #include <linux/delay.h> #include <linux/interrupt.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/tty.h> #include <linux/tty_flip.h>
@@ -44,6 +45,8 @@#include <linux/io.h> #include <linux/slab.h> +#define PXA_NAME_LEN 8 +struct uart_pxa_port {struct uart_port port;unsigned char ier;
@@ -781,6 +784,39 @@ static const struct dev_pm_ops serial_pxa_pm_ops = { };#endif +static struct of_device_id serial_pxa_dt_ids[] = { + { .compatible ="mrvl,pxa-uart" , }, + { .compatible ="mrvl,mmp-uart" , }, + {} +}; +MODULE_DEVICE_TABLE (of, serial_pxa_dt_ids); + +#ifdef CONFIG_OF +static int serial_pxa_probe_dt (struct platform_device *pdev, +struct uart_pxa_port *sport) +{ +struct device_node *np = pdev->dev.of_node; +int ret; + +if (!np) +return 1 ; + + ret =of_alias_get_id (np,"serial" ); +if (ret <0 ) { +dev_err (&pdev->dev,"failed to get alias id, errno %d\n" , ret); +return ret; + } + sport->port.line = ret; +return 0 ; +} +#else +static int serial_pxa_probe_dt (struct platform_device *pdev, +struct uart_pxa_port *sport) +{ +return 1 ; +} +#endif +static int serial_pxa_probe (struct platform_device *dev) {struct uart_pxa_port *sport;
@@ -808,34 +844,37 @@ static int serial_pxa_probe(struct platform_device *dev) sport->port.irq = irqres->start; sport->port.fifosize =64 ; sport->port.ops = &serial_pxa_pops; - sport->port.line = dev->id; sport->port.dev = &dev->dev; sport->port.flags =UPF_IOREMAP |UPF_BOOT_AUTOCONF ; sport->port.uartclk =clk_get_rate (sport->clk); - -switch (dev->id) { -case 0 : sport->name ="FFUART" ;break ; -case 1 : sport->name ="BTUART" ;break ; -case 2 : sport->name ="STUART" ;break ; -case 3 : sport->name ="HWUART" ;break ; -default : - sport->name ="???" ; -break ; + sport->name =kzalloc (PXA_NAME_LEN ,GFP_KERNEL ); +if (!sport->name) { + ret = -ENOMEM ; + goto err_clk; } + ret =serial_pxa_probe_dt (dev, sport); +if (ret >0 ) + sport->port.line = dev->id; +else if (ret <0 ) + goto err_clk; +snprintf (sport->name,PXA_NAME_LEN -1 ,"UART%d" , sport->port.line +1 ); + sport->port.membase =ioremap (mmres->start,resource_size (mmres));if (!sport->port.membase) { ret = -ENOMEM ; - goto err_clk; + goto err_name; } - serial_pxa_ports[dev->id] = sport; + serial_pxa_ports[sport->port.line] = sport;uart_add_one_port (&serial_pxa_reg, &sport->port);platform_set_drvdata (dev, sport);return 0 ; + err_name: +kfree (sport->name); err_clk:clk_put (sport->clk); err_free:
@@ -850,6 +889,7 @@ static int serial_pxa_remove(struct platform_device *dev)platform_set_drvdata (dev,NULL );uart_remove_one_port (&serial_pxa_reg, &sport->port); +kfree (sport->name);clk_put (sport->clk);kfree (sport);
@@ -866,6 +906,7 @@ static struct platform_driver serial_pxa_driver = {#ifdef CONFIG_PM .pm = &serial_pxa_pm_ops, #endif + .of_match_table = serial_pxa_dt_ids, }, };
Signature · 1 line
1.7.0.4
PATCH v2 1/7 serial: pxa: add OF support
drivers/tty/serial/pxa.c line 52
Quoted 6 lines · Haojian Zhuang
drivers/tty/serial/pxa.c +2
-0+#define PXA_NAME_LEN 8 +struct uart_pxa_port {struct uart_port port;unsigned char ier;
Why didn't you just add a field here with that length?
drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 792
Quoted 10 lines
drivers/tty/serial/pxa.c +6
-0@@ -781,6 +784,39 @@ static const struct dev_pm_ops serial_pxa_pm_ops = { };#endif +static struct of_device_id serial_pxa_dt_ids[] = { + { .compatible ="mrvl,pxa-uart" , }, + { .compatible ="mrvl,mmp-uart" , }, + {} +}; +MODULE_DEVICE_TABLE (of, serial_pxa_dt_ids);
This one should have an #ifdef CONFIG_OF
drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 797
Quoted 4 lines
drivers/tty/serial/pxa.c +4
-0+#ifdef CONFIG_OF +static int serial_pxa_probe_dt (struct platform_device *pdev, +struct uart_pxa_port *sport) +{
While this one does not need it: it will already compile to nothing if you check the error value correctly.
drivers/tty/serial/pxa.c static int serial_pxa_probe(struct platform_device *dev) · line 854
Quoted 5 lines
drivers/tty/serial/pxa.c +4
-0+ sport->name =kzalloc (PXA_NAME_LEN ,GFP_KERNEL ); +if (!sport->name) { + ret = -ENOMEM ; + goto err_clk; }
No need for this allocation if you put the name into uart_pxa_port as a member instead of a pointer.
drivers/tty/serial/pxa.c static struct platform_driver serial_pxa_driver = { · line 911
Quoted 3 lines
drivers/tty/serial/pxa.c +1
-0+ .of_match_table = serial_pxa_dt_ids, }, };
.of_match_table = of_match_ptr(serial_pxa_dt_ids),
Arnd
PATCH v2 1/7 serial: pxa: add OF support
Quoted 41 lines · Arnd Bergmann
Quoted 6 lines · Haojian Zhuang
(quoted diff) +2
-0+#define PXA_NAME_LEN ? ? ? ? 8 +?struct
uart_pxa_port{
? ? ? structuart_port? ? ? ?port;
? ? ? unsigned char ? ? ? ? ? ier;Why didn't you just add a field here with that length?
drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 792
Quoted 10 lines
(quoted diff) +0
-0@@ -781,6 +784,39 @@ static const struct dev_pm_ops serial_pxa_pm_ops = { ?};?#endif
drivers/tty/serial/pxa.c +6
-0+static struct of_device_id serial_pxa_dt_ids[] = { + ? ? { .compatible = "mrvl,pxa-uart", }, + ? ? { .compatible = "mrvl,mmp-uart", }, + ? ? {} +}; +MODULE_DEVICE_TABLE (of, serial_pxa_dt_ids);This one should have an #ifdef
CONFIG_OFdrivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 797
Quoted 4 lines
drivers/tty/serial/pxa.c +4
-0+#ifdef CONFIG_OF +static int serial_pxa_probe_dt (struct platform_device *pdev, + ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct uart_pxa_port *sport) +{While this one does not need it: it will already compile to nothing if you check the error value correctly.
Quoted 5 lines
+ ? ?
sport->name=kzalloc(PXA_NAME_LEN, GFP_KERNEL);
+ ? ? if (!sport->name) {
+ ? ? ? ? ? ? ret =-ENOMEM;
+ ? ? ? ? ? ? gotoerr_clk;
? ? ? }No need for this allocation if you put the name into
uart_pxa_portas a member instead of a pointer.
Quoted 8 lines
Quoted 3 lines
+ ? ? ? ? ? ? .of_match_table =
serial_pxa_dt_ids, ? ? ? },
?};? ? ? ? ? ? ? ?.of_match_table =
of_match_ptr(serial_pxa_dt_ids),? ? ? ?Arnd
Thanks. I'll update it.
Best Regards
Haojian
PATCH v3 1/7 serial: pxa: add OF support
In reply to Arnd BergmannParse uart device id from alias in DTS file.
--- drivers/tty/serial/pxa.c | 52 +++++++++++++++++++++++++++++++++++---------- 1 files changed, 40 insertions(+), 12 deletions(-)
drivers/tty/serial/pxa.c +40 -12
@@ -36,6 +36,7 @@#include <linux/circ_buf.h> #include <linux/delay.h> #include <linux/interrupt.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/tty.h> #include <linux/tty_flip.h>
@@ -44,6 +45,8 @@#include <linux/io.h> #include <linux/slab.h> +#define PXA_NAME_LEN 8 +struct uart_pxa_port {struct uart_port port;unsigned char ier;
@@ -51,7 +54,7 @@ struct uart_pxa_port {unsigned char mcr;unsigned int lsr_break_flag;struct clk *clk; -char *name; +char name[PXA_NAME_LEN ]; };static inline unsigned int serial_in (struct uart_pxa_port *up,int offset)
@@ -781,6 +784,33 @@ static const struct dev_pm_ops serial_pxa_pm_ops = { };#endif +#ifdef CONFIG_OF +static struct of_device_id serial_pxa_dt_ids[] = { + { .compatible ="mrvl,pxa-uart" , }, + { .compatible ="mrvl,mmp-uart" , }, + {} +}; +MODULE_DEVICE_TABLE (of, serial_pxa_dt_ids); +#endif + +static int serial_pxa_probe_dt (struct platform_device *pdev, +struct uart_pxa_port *sport) +{ +struct device_node *np = pdev->dev.of_node; +int ret; + +if (!np) +return 1 ; + + ret =of_alias_get_id (np,"serial" ); +if (ret <0 ) { +dev_err (&pdev->dev,"failed to get alias id, errno %d\n" , ret); +return ret; + } + sport->port.line = ret; +return 0 ; +} +static int serial_pxa_probe (struct platform_device *dev) {struct uart_pxa_port *sport;
@@ -808,20 +838,16 @@ static int serial_pxa_probe(struct platform_device *dev) sport->port.irq = irqres->start; sport->port.fifosize =64 ; sport->port.ops = &serial_pxa_pops; - sport->port.line = dev->id; sport->port.dev = &dev->dev; sport->port.flags =UPF_IOREMAP |UPF_BOOT_AUTOCONF ; sport->port.uartclk =clk_get_rate (sport->clk); -switch (dev->id) { -case 0 : sport->name ="FFUART" ;break ; -case 1 : sport->name ="BTUART" ;break ; -case 2 : sport->name ="STUART" ;break ; -case 3 : sport->name ="HWUART" ;break ; -default : - sport->name ="???" ; -break ; - } + ret =serial_pxa_probe_dt (dev, sport); +if (ret >0 ) + sport->port.line = dev->id; +else if (ret <0 ) + goto err_clk; +snprintf (sport->name,PXA_NAME_LEN -1 ,"UART%d" , sport->port.line +1 ); sport->port.membase =ioremap (mmres->start,resource_size (mmres));if (!sport->port.membase) {
@@ -829,7 +855,7 @@ static int serial_pxa_probe(struct platform_device *dev) goto err_clk; } - serial_pxa_ports[dev->id] = sport; + serial_pxa_ports[sport->port.line] = sport;uart_add_one_port (&serial_pxa_reg, &sport->port);platform_set_drvdata (dev, sport);
@@ -850,6 +876,7 @@ static int serial_pxa_remove(struct platform_device *dev)platform_set_drvdata (dev,NULL );uart_remove_one_port (&serial_pxa_reg, &sport->port); +kfree (sport->name);clk_put (sport->clk);kfree (sport);
@@ -866,6 +893,7 @@ static struct platform_driver serial_pxa_driver = {#ifdef CONFIG_PM .pm = &serial_pxa_pm_ops, #endif + .of_match_table = serial_pxa_dt_ids, }, };
Signature · 1 line
1.7.0.4
PATCH v3 1/7 serial: pxa: add OF support
drivers/tty/serial/pxa.c struct uart_pxa_port { · line 58
Quoted 7 lines · Haojian Zhuang
drivers/tty/serial/pxa.c +1
-1@@ -51,7 +54,7 @@ struct uart_pxa_port {unsigned char mcr;unsigned int lsr_break_flag;struct clk *clk; -char *name; +char name[PXA_NAME_LEN ]; };
This looks better now
drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 794
Quoted 13 lines
drivers/tty/serial/pxa.c +8
-0static inline unsigned int serial_in (struct uart_pxa_port *up,int offset)@@ -781,6 +784,33 @@ static const struct dev_pm_ops serial_pxa_pm_ops = { };#endif +#ifdef CONFIG_OF +static struct of_device_id serial_pxa_dt_ids[] = { + { .compatible ="mrvl,pxa-uart" , }, + { .compatible ="mrvl,mmp-uart" , }, + {} +}; +MODULE_DEVICE_TABLE (of, serial_pxa_dt_ids); +#endif
If you do this, you have to use of_match_ptr for referencing serial_pxa_dt_ids or you will get a link error when CONFIG_OF is disabled.
drivers/tty/serial/pxa.c static int serial_pxa_remove(struct platform_device *dev) · line 882
Quoted 8 lines
drivers/tty/serial/pxa.c +1
-0@@ -850,6 +876,7 @@ static int serial_pxa_remove(struct platform_device *dev)platform_set_drvdata (dev,NULL );uart_remove_one_port (&serial_pxa_reg, &sport->port); +kfree (sport->name);clk_put (sport->clk);kfree (sport);
And this must be removed now that sport->name is allocated as part of sport.
Arnd
PATCH v3 1/7 serial: pxa: add OF support
Quoted 41 lines · Arnd Bergmann
Review struct uart_pxa_port { · line 58
Quoted 7 lines · Haojian Zhuang
(quoted diff) +1
-1@@ -51,7 +54,7 @@ struct uart_pxa_port { ? ? ? unsigned char ? ? ? ? ? mcr; ? ? ? unsigned int ? ? ? ? ? ?lsr_break_flag; ? ? ? struct clk ? ? ? ? ? ? ?*clk; - ? ? char ? ? ? ? ? ? ? ? ? ?*name; + ? ? char ? ? ? ? ? ? ? ? ? ?name[PXA_NAME_LEN]; ?};This looks better now
drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 794
Quoted 13 lines
(quoted diff) +0
-0?static inline unsigned int serial_in(struct uart_pxa_port *up, int offset)@@ -781,6 +784,33 @@ static const struct dev_pm_ops serial_pxa_pm_ops = { ?};?#endif
drivers/tty/serial/pxa.c +8
-0+#ifdef CONFIG_OF +static struct of_device_id serial_pxa_dt_ids[] = { + ? ? { .compatible = "mrvl,pxa-uart", }, + ? ? { .compatible = "mrvl,mmp-uart", }, + ? ? {} +}; +MODULE_DEVICE_TABLE (of, serial_pxa_dt_ids); +#endif If you do this, you have to use
of_match_ptrfor referencingserial_pxa_dt_idsor you will get a link error whenCONFIG_OFis disabled.Review static int serial_pxa_remove(struct platform_device *dev) · line 882
Quoted 8 lines
(quoted diff) +1
-0@@ -850,6 +876,7 @@ static int serial_pxa_remove(struct platform_device *dev) ? ? ? platform_set_drvdata(dev, NULL); ? ? ? uart_remove_one_port(&serial_pxa_reg, &sport->port); + ? ? kfree(sport->name); ? ? ? clk_put(sport->clk); ? ? ? kfree(sport);And this must be removed now that
sport->nameis allocated as part of sport.
Quoted 2 lines
? ? ? ?Arnd
Ah. It's a bug. I'll fix it tomorrow.
Thanks
Haojian
PATCH v3 1/6 serial: pxa: add OF support
In reply to Arnd BergmannFrom: Haojian Zhuang <[email protected]>
Parse uart device id from alias in DTS file.
--- drivers/tty/serial/pxa.c | 49 ++++++++++++++++++++++++++++++++++----------- 1 files changed, 37 insertions(+), 12 deletions(-)
drivers/tty/serial/pxa.c +37 -12
@@ -36,6 +36,7 @@#include <linux/circ_buf.h> #include <linux/delay.h> #include <linux/interrupt.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/tty.h> #include <linux/tty_flip.h>
@@ -44,6 +45,8 @@#include <linux/io.h> #include <linux/slab.h> +#define PXA_NAME_LEN 8 +struct uart_pxa_port {struct uart_port port;unsigned char ier;
@@ -51,7 +54,7 @@ struct uart_pxa_port {unsigned char mcr;unsigned int lsr_break_flag;struct clk *clk; -char *name; +char name[PXA_NAME_LEN ]; };static inline unsigned int serial_in (struct uart_pxa_port *up,int offset)
@@ -781,6 +784,31 @@ static const struct dev_pm_ops serial_pxa_pm_ops = { };#endif +static struct of_device_id serial_pxa_dt_ids[] = { + { .compatible ="mrvl,pxa-uart" , }, + { .compatible ="mrvl,mmp-uart" , }, + {} +}; +MODULE_DEVICE_TABLE (of, serial_pxa_dt_ids); + +static int serial_pxa_probe_dt (struct platform_device *pdev, +struct uart_pxa_port *sport) +{ +struct device_node *np = pdev->dev.of_node; +int ret; + +if (!np) +return 1 ; + + ret =of_alias_get_id (np,"serial" ); +if (ret <0 ) { +dev_err (&pdev->dev,"failed to get alias id, errno %d\n" , ret); +return ret; + } + sport->port.line = ret; +return 0 ; +} +static int serial_pxa_probe (struct platform_device *dev) {struct uart_pxa_port *sport;
@@ -808,20 +836,16 @@ static int serial_pxa_probe(struct platform_device *dev) sport->port.irq = irqres->start; sport->port.fifosize =64 ; sport->port.ops = &serial_pxa_pops; - sport->port.line = dev->id; sport->port.dev = &dev->dev; sport->port.flags =UPF_IOREMAP |UPF_BOOT_AUTOCONF ; sport->port.uartclk =clk_get_rate (sport->clk); -switch (dev->id) { -case 0 : sport->name ="FFUART" ;break ; -case 1 : sport->name ="BTUART" ;break ; -case 2 : sport->name ="STUART" ;break ; -case 3 : sport->name ="HWUART" ;break ; -default : - sport->name ="???" ; -break ; - } + ret =serial_pxa_probe_dt (dev, sport); +if (ret >0 ) + sport->port.line = dev->id; +else if (ret <0 ) + goto err_clk; +snprintf (sport->name,PXA_NAME_LEN -1 ,"UART%d" , sport->port.line +1 ); sport->port.membase =ioremap (mmres->start,resource_size (mmres));if (!sport->port.membase) {
@@ -829,7 +853,7 @@ static int serial_pxa_probe(struct platform_device *dev) goto err_clk; } - serial_pxa_ports[dev->id] = sport; + serial_pxa_ports[sport->port.line] = sport;uart_add_one_port (&serial_pxa_reg, &sport->port);platform_set_drvdata (dev, sport);
@@ -866,6 +890,7 @@ static struct platform_driver serial_pxa_driver = {#ifdef CONFIG_PM .pm = &serial_pxa_pm_ops, #endif + .of_match_table = serial_pxa_dt_ids, }, };
Signature · 1 line
1.7.5.4
PATCH v3 1/6 serial: pxa: add OF support
Quoted 5 lines · Haojian Zhuang
From: Haojian Zhuang <[email protected]>
Parse uart device id from alias in DTS file.
Signed-off-by Haojian Zhuang <[email protected]>
Looks good now,
PATCH v2 1/7 serial: pxa: add OF support
In reply to Haojian ZhuangQuoted 54 lines · Haojian Zhuang
Parse uart device id from alias in DTS file.
Signed-off-by Haojian Zhuang <[email protected]>--- drivers/tty/serial/pxa.c | 65 +++++++++++++++++++++++++++++++++++++-------- 1 files changed, 53 insertions(+), 12 deletions(-)drivers/tty/serial/pxa.c +25
-0@@ -36,6 +36,7 @@#include <linux/circ_buf.h> #include <linux/delay.h> #include <linux/interrupt.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/tty.h> #include <linux/tty_flip.h> @@ -44,6 +45,8 @@#include <linux/io.h> #include <linux/slab.h> +#define PXA_NAME_LEN 8 +struct uart_pxa_port {struct uart_port port;unsigned char ier;@@ -781,6 +784,39 @@ static const struct dev_pm_ops serial_pxa_pm_ops = { };#endif +static struct of_device_id serial_pxa_dt_ids[] = { + { .compatible ="mrvl,pxa-uart" , }, + { .compatible ="mrvl,mmp-uart" , }, + {} +}; +MODULE_DEVICE_TABLE (of, serial_pxa_dt_ids); + +#ifdef CONFIG_OF +static int serial_pxa_probe_dt (struct platform_device *pdev, +struct uart_pxa_port *sport) +{ +struct device_node *np = pdev->dev.of_node; +int ret; + +if (!np) +return 1 ; + + ret =of_alias_get_id (np,"serial" ); +if (ret <0 ) { +dev_err (&pdev->dev,"failed to get alias id, errno %d\n" , ret); +return ret; + }
drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 818
Quoted 10 lines
drivers/tty/serial/pxa.c +10
-0+ sport->port.line = ret; + return 0; +} +#else +static int serial_pxa_probe_dt (struct platform_device *pdev, +struct uart_pxa_port *sport) +{ +return 1 ; +} +#endif
Returning 1 on success is rather odd... see comment below.
Quoted 12 lines
drivers/tty/serial/pxa.c +1
-1+static int serial_pxa_probe (struct platform_device *dev) {struct uart_pxa_port *sport;@@ -808,34 +844,37 @@ static int serial_pxa_probe(struct platform_device *dev) sport->port.irq = irqres->start; sport->port.fifosize =64 ; sport->port.ops = &serial_pxa_pops; - sport->port.line = dev->id; sport->port.dev = &dev->dev; sport->port.flags =UPF_IOREMAP |UPF_BOOT_AUTOCONF ; sport->port.uartclk =clk_get_rate (sport->clk);
drivers/tty/serial/pxa.c static int serial_pxa_probe(struct platform_device *dev) · line 850
Quoted 10 lines
drivers/tty/serial/pxa.c +1
-9- -switch (dev->id) { -case 0 : sport->name ="FFUART" ;break ; -case 1 : sport->name ="BTUART" ;break ; -case 2 : sport->name ="STUART" ;break ; -case 3 : sport->name ="HWUART" ;break ; -default : - sport->name ="???" ; -break ; + sport->name =kzalloc (PXA_NAME_LEN ,GFP_KERNEL );
devm_kzalloc(). Then you don't need to unwind the allocation on failure.
drivers/tty/serial/pxa.c static int serial_pxa_probe(struct platform_device *dev) · line 861
Quoted 11 lines
drivers/tty/serial/pxa.c +9
-0+if (!sport->name) { + ret = -ENOMEM ; + goto err_clk; } + ret =serial_pxa_probe_dt (dev, sport); +if (ret >0 ) + sport->port.line = dev->id; +else if (ret <0 ) + goto err_clk; +snprintf (sport->name,PXA_NAME_LEN -1 ,"UART%d" , sport->port.line +1 );
returning a -/0/+ value from serial_pxa_probe_dt() is rather odd. It's easier to follow code which sticks to the convention of <0 for error, 0 for success. I would do:
sport->port.line = dev->id; ret = serial_pxa_probe_dt(dev, sport); if (ret < 0) goto err_clk;
Then serial_pxa_probe_dt() can return an error code on failure, or 0 on success or no-devicetree.
Quoted 30 lines
drivers/tty/serial/pxa.c +6
-2+ sport->port.membase =ioremap (mmres->start,resource_size (mmres));if (!sport->port.membase) { ret = -ENOMEM ; - goto err_clk; + goto err_name; } - serial_pxa_ports[dev->id] = sport; + serial_pxa_ports[sport->port.line] = sport;uart_add_one_port (&serial_pxa_reg, &sport->port);platform_set_drvdata (dev, sport);return 0 ; + err_name: +kfree (sport->name); err_clk:clk_put (sport->clk); err_free:@@ -850,6 +889,7 @@ static int serial_pxa_remove(struct platform_device *dev)platform_set_drvdata (dev,NULL );uart_remove_one_port (&serial_pxa_reg, &sport->port); +kfree (sport->name);clk_put (sport->clk);kfree (sport);
drivers/tty/serial/pxa.c static struct platform_driver serial_pxa_driver = { · line 909
Quoted 4 lines
drivers/tty/serial/pxa.c +1
-0@@ -866,6 +906,7 @@ static struct platform_driver serial_pxa_driver = {#ifdef CONFIG_PM .pm = &serial_pxa_pm_ops, #endif + .of_match_table = serial_pxa_dt_ids,
.of_match_table = of_match_ptr(serial_pxa_dt_ids),
Otherwise looks great to me.
g.
PATCH v2 1/7 serial: pxa: add OF support
In reply to Haojian ZhuangQuoted 6 lines · Haojian Zhuang
Parse uart device id from alias in DTS file.
Signed-off-by Haojian Zhuang <[email protected]>--- drivers/tty/serial/pxa.c | 65 +++++++++++++++++++++++++++++++++++++-------- 1 files changed, 53 insertions(+), 12 deletions(-)
Binding documentation!
Quoted 70 lines
drivers/tty/serial/pxa.c +36
-1@@ -36,6 +36,7 @@#include <linux/circ_buf.h> #include <linux/delay.h> #include <linux/interrupt.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/tty.h> #include <linux/tty_flip.h> @@ -44,6 +45,8 @@#include <linux/io.h> #include <linux/slab.h> +#define PXA_NAME_LEN 8 +struct uart_pxa_port {struct uart_port port;unsigned char ier;@@ -781,6 +784,39 @@ static const struct dev_pm_ops serial_pxa_pm_ops = { };#endif +static struct of_device_id serial_pxa_dt_ids[] = { + { .compatible ="mrvl,pxa-uart" , }, + { .compatible ="mrvl,mmp-uart" , }, + {} +}; +MODULE_DEVICE_TABLE (of, serial_pxa_dt_ids); + +#ifdef CONFIG_OF +static int serial_pxa_probe_dt (struct platform_device *pdev, +struct uart_pxa_port *sport) +{ +struct device_node *np = pdev->dev.of_node; +int ret; + +if (!np) +return 1 ; + + ret =of_alias_get_id (np,"serial" ); +if (ret <0 ) { +dev_err (&pdev->dev,"failed to get alias id, errno %d\n" , ret); +return ret; + } + sport->port.line = ret; +return 0 ; +} +#else +static int serial_pxa_probe_dt (struct platform_device *pdev, +struct uart_pxa_port *sport) +{ +return 1 ; +} +#endif +static int serial_pxa_probe (struct platform_device *dev) {struct uart_pxa_port *sport;@@ -808,34 +844,37 @@ static int serial_pxa_probe(struct platform_device *dev) sport->port.irq = irqres->start; sport->port.fifosize =64 ; sport->port.ops = &serial_pxa_pops; - sport->port.line = dev->id; sport->port.dev = &dev->dev; sport->port.flags =UPF_IOREMAP |UPF_BOOT_AUTOCONF ; sport->port.uartclk =clk_get_rate (sport->clk);
drivers/tty/serial/pxa.c static int serial_pxa_probe(struct platform_device *dev) · line 850
Quoted 10 lines
drivers/tty/serial/pxa.c +1
-9- -switch (dev->id) { -case 0 : sport->name ="FFUART" ;break ; -case 1 : sport->name ="BTUART" ;break ; -case 2 : sport->name ="STUART" ;break ; -case 3 : sport->name ="HWUART" ;break ; -default : - sport->name ="???" ; -break ; + sport->name =kzalloc (PXA_NAME_LEN ,GFP_KERNEL );
devm_kzalloc()
drivers/tty/serial/pxa.c static int serial_pxa_probe(struct platform_device *dev) · line 861
Quoted 11 lines
drivers/tty/serial/pxa.c +9
-0+if (!sport->name) { + ret = -ENOMEM ; + goto err_clk; } + ret =serial_pxa_probe_dt (dev, sport); +if (ret >0 ) + sport->port.line = dev->id; +else if (ret <0 ) + goto err_clk; +snprintf (sport->name,PXA_NAME_LEN -1 ,"UART%d" , sport->port.line +1 );
Why +1 here? Won't that just cause confusion when compared with the DT aliases?
Quoted 40 lines
drivers/tty/serial/pxa.c +7
-2+ sport->port.membase =ioremap (mmres->start,resource_size (mmres));if (!sport->port.membase) { ret = -ENOMEM ; - goto err_clk; + goto err_name; } - serial_pxa_ports[dev->id] = sport; + serial_pxa_ports[sport->port.line] = sport;uart_add_one_port (&serial_pxa_reg, &sport->port);platform_set_drvdata (dev, sport);return 0 ; + err_name: +kfree (sport->name); err_clk:clk_put (sport->clk); err_free:@@ -850,6 +889,7 @@ static int serial_pxa_remove(struct platform_device *dev)platform_set_drvdata (dev,NULL );uart_remove_one_port (&serial_pxa_reg, &sport->port); +kfree (sport->name);clk_put (sport->clk);kfree (sport);@@ -866,6 +906,7 @@ static struct platform_driver serial_pxa_driver = {#ifdef CONFIG_PM .pm = &serial_pxa_pm_ops, #endif + .of_match_table = serial_pxa_dt_ids, }, };Signature · 1 line
1.7.0.4
Signature · 2 lines
Grant Likely, B.Sc, P.Eng. Secret Lab Technologies,Ltd.
PATCH v2 2/7 rtc: sa1100: add OF support
In reply to Haojian ZhuangAdd OF support on rtc-sa1100.
--- drivers/rtc/rtc-sa1100.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
drivers/rtc/rtc-sa1100.c +9 -0
@@ -30,6 +30,7 @@#include <linux/interrupt.h> #include <linux/slab.h> #include <linux/string.h> +#include <linux/of.h> #include <linux/pm.h> #include <linux/bitops.h>
@@ -349,6 +350,13 @@ static const struct dev_pm_ops sa1100_rtc_pm_ops = { };#endif +static struct of_device_id sa1100_rtc_dt_ids[] = { + { .compatible ="mrvl,sa1100-rtc" , }, + { .compatible ="mrvl,mmp-rtc" , }, + {} +}; +MODULE_DEVICE_TABLE (of, sa1100_rtc_dt_ids); +static struct platform_driver sa1100_rtc_driver = { .probe = sa1100_rtc_probe, .remove = sa1100_rtc_remove,
@@ -357,6 +365,7 @@ static struct platform_driver sa1100_rtc_driver = {#ifdef CONFIG_PM .pm = &sa1100_rtc_pm_ops, #endif + .of_match_table = sa1100_rtc_dt_ids, }, };
Signature · 1 line
1.7.0.4
PATCH v2 2/7 rtc: sa1100: add OF support
Quoted 3 lines · Haojian Zhuang
Add OF support on rtc-sa1100.
Signed-off-by Haojian Zhuang <[email protected]>
PATCH v2 3/7 i2c: pxa: add OF support
In reply to Haojian ZhuangAppend these properties in below.
mrvl,i2c-polling
mrvl,i2c-fast-mode
Still keep slave, slave_addr and class in platform data.
--- drivers/i2c/busses/i2c-pxa.c | 95 ++++++++++++++++++++++++++++++++--------- 1 files changed, 74 insertions(+), 21 deletions(-)
drivers/i2c/busses/i2c-pxa.c +74 -21
@@ -29,6 +29,8 @@#include <linux/errno.h> #include <linux/interrupt.h> #include <linux/i2c-pxa.h> +#include <linux/of.h> +#include <linux/of_device.h> #include <linux/of_i2c.h> #include <linux/platform_device.h> #include <linux/err.h>
@@ -1044,23 +1046,60 @@ static const struct i2c_algorithm i2c_pxa_pio_algorithm = { .functionality = i2c_pxa_functionality, }; -static int i2c_pxa_probe (struct platform_device *dev) +static struct of_device_id i2c_pxa_dt_ids[] = { + { .compatible ="mrvl,pxa-i2c" , .data = (void *)REGS_PXA2XX }, + { .compatible ="mrvl,pwri2c" , .data = (void *)REGS_PXA3XX }, + { .compatible ="mrvl,mmp-twsi" , .data = (void *)REGS_PXA2XX }, + {} +}; +MODULE_DEVICE_TABLE (of, i2c_pxa_dt_ids); + +static int i2c_pxa_probe_dt (struct platform_device *pdev,struct pxa_i2c *i2c, +enum pxa_i2c_types *i2c_types) { -struct pxa_i2c *i2c; -struct resource *res; -struct i2c_pxa_platform_data *plat = dev->dev.platform_data; -const struct platform_device_id *id =platform_get_device_id (dev); -enum pxa_i2c_types i2c_type = id->driver_data; +struct device_node *np = pdev->dev.of_node; +const struct of_device_id *of_id = +of_match_device (i2c_pxa_dt_ids, &pdev->dev);int ret; -int irq; - res =platform_get_resource (dev,IORESOURCE_MEM ,0 ); - irq =platform_get_irq (dev,0 ); -if (res ==NULL || irq <0 ) -return -ENODEV ; +if (!of_id) +return 1 ; + ret =of_alias_get_id (np,"i2c" ); +if (ret <0 ) { +dev_err (&pdev->dev,"failed to get alias id, errno %d\n" , ret); +return ret; + } + pdev->id = ret; +if (of_get_property (np,"mrvl,i2c-polling" ,NULL )) + i2c->use_pio =1 ; +if (of_get_property (np,"mrvl,i2c-fast-mode" ,NULL )) + i2c->fast_mode =1 ; + *i2c_types = (u32)(of_id->data); +return 0 ; +} -if (!request_mem_region (res->start,resource_size (res), res->name)) -return -ENOMEM ; +static int i2c_pxa_probe_pdata (struct platform_device *pdev, +struct pxa_i2c *i2c, +enum pxa_i2c_types *i2c_types) +{ +struct i2c_pxa_platform_data *plat = pdev->dev.platform_data; +const struct platform_device_id *id =platform_get_device_id (pdev); + + *i2c_types = id->driver_data; +if (plat) { + i2c->use_pio = plat->use_pio; + i2c->fast_mode = plat->fast_mode; + } +return 0 ; +} + +static int i2c_pxa_probe (struct platform_device *dev) +{ +struct i2c_pxa_platform_data *plat = dev->dev.platform_data; +enum pxa_i2c_types i2c_type; +struct pxa_i2c *i2c; +struct resource *res =NULL ; +int ret, irq; i2c =kzalloc (sizeof (struct pxa_i2c ),GFP_KERNEL );if (!i2c) {
@@ -1068,6 +1107,24 @@ static int i2c_pxa_probe(struct platform_device *dev) goto emalloc; } + ret =i2c_pxa_probe_dt (dev, i2c, &i2c_type); +if (ret >0 ) + ret =i2c_pxa_probe_pdata (dev, i2c, &i2c_type); +if (ret <0 ) + goto eclk; + + res =platform_get_resource (dev,IORESOURCE_MEM ,0 ); + irq =platform_get_irq (dev,0 ); +if (res ==NULL || irq <0 ) { + ret = -ENODEV ; + goto eclk; + } + +if (!request_mem_region (res->start,resource_size (res), res->name)) { + ret = -ENOMEM ; + goto eclk; + } + i2c->adap.owner =THIS_MODULE ; i2c->adap.retries =5 ;
@@ -1109,21 +1166,16 @@ static int i2c_pxa_probe(struct platform_device *dev) i2c->slave_addr =I2C_PXA_SLAVE_ADDR ; -#ifdef CONFIG_I2C_PXA_SLAVE if (plat) { +#ifdef CONFIG_I2C_PXA_SLAVE i2c->slave_addr = plat->slave_addr; i2c->slave = plat->slave; - }#endif - -clk_enable (i2c->clk); - -if (plat) { i2c->adap.class = plat->class; - i2c->use_pio = plat->use_pio; - i2c->fast_mode = plat->fast_mode; } +clk_enable (i2c->clk); +if (i2c->use_pio) { i2c->adap.algo = &i2c_pxa_pio_algorithm; }else {
@@ -1234,6 +1286,7 @@ static struct platform_driver i2c_pxa_driver = { .name = "pxa2xx-i2c", .owner = THIS_MODULE, .pm = I2C_PXA_DEV_PM_OPS, + .of_match_table = i2c_pxa_dt_ids, }, .id_table = i2c_pxa_id_table, };
Signature · 1 line
1.7.0.4
PATCH v2 3/7 i2c: pxa: add OF support
Quoted 7 lines · Haojian Zhuang
Append these properties in below.
mrvl,i2c-polling
mrvl,i2c-fast-modeStill keep slave,
slave_addrand class in platform data.Signed-off-by Haojian Zhuang <[email protected]>
Just one tiny detail:
Quoted 1 line
+
*i2c_types= (u32)(of_id->data);
By convention, please use (unsigned long) as the cast from pointer to an integer type. While this doesn't make any difference on 32 bit systems, it's better to always write code that is 64-bit clean.
Arnd
PATCH v2 4/7 ARM: mmp: enable rtc clk in pxa168
In reply to Haojian ZhuangEnable clk of rtc-sa1100 device in pxa168.
--- arch/arm/mach-mmp/pxa168.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
arch/arm/mach-mmp/pxa168.c +2 -0
@@ -65,6 +65,7 @@ static APBC_CLK(ssp4, PXA168_SSP4, 4, 0); static APBC_CLK(ssp5, PXA168_SSP5, 4, 0); static APBC_CLK(gpio, PXA168_GPIO, 0, 13000000); static APBC_CLK(keypad, PXA168_KPC, 0, 32000); +static APBC_CLK(rtc, PXA168_RTC, 8, 32768); static APMU_CLK(nand, NAND, 0x19b, 156000000); static APMU_CLK(lcd, LCD, 0x7f, 312000000);
@@ -93,6 +94,7 @@ static struct clk_lookup pxa168_clkregs[] = {INIT_CLKREG (&clk_keypad,"pxa27x-keypad" ,NULL ),INIT_CLKREG (&clk_eth,"pxa168-eth" ,"MFUCLK" ),INIT_CLKREG (&clk_usb,"pxa168-ehci" ,"PXA168-USBCLK" ), + INIT_CLKREG(&clk_rtc, "sa1100-rtc", NULL), };static int __init pxa168_init (void )
Signature · 1 line
1.7.0.4
PATCH v2 4/7 ARM: mmp: enable rtc clk in pxa168
Quoted 3 lines · Haojian Zhuang
Enable clk of rtc-sa1100 device in pxa168.
Signed-off-by Haojian Zhuang <[email protected]>
PATCH v2 5/7 ARM: mmp: append OF support on pxa168
In reply to Haojian ZhuangEnable PXA168 and aspenite support.
--- arch/arm/mach-mmp/Kconfig | 10 ++++++ arch/arm/mach-mmp/Makefile | 1 + arch/arm/mach-mmp/mmp-dt.c | 75 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-mmp/mmp-dt.c
arch/arm/mach-mmp/Kconfig +10 -0
@@ -2,6 +2,16 @@ if ARCH_MMP menu "Marvell PXA168/910/MMP2 Implmentations" +config MACH_MMP_DT + bool "Support MMP2 platforms from device tree" + select CPU_PXA168 + select CPU_PXA910 + select USE_OF + help + Include support for Marvell MMP2 based platforms using + the device tree. Needn't select any other machine while + MACH_MMP_DT is enabled. + config MACH_ASPENITE bool "Marvell's PXA168 Aspenite Development Board" select CPU_PXA168
arch/arm/mach-mmp/Makefile +1 -0
@@ -18,5 +18,6 @@ obj-$(CONFIG_MACH_TTC_DKB) += ttc_dkb.o obj-$(CONFIG_MACH_BROWNSTONE) += brownstone.o obj-$(CONFIG_MACH_FLINT) += flint.o obj-$(CONFIG_MACH_MARVELL_JASPER) += jasper.o +obj-$(CONFIG_MACH_MMP_DT) += mmp-dt.o obj-$(CONFIG_MACH_TETON_BGA) += teton_bga.o obj-$(CONFIG_MACH_GPLUGD) += gplugd.o
arch/arm/mach-mmp/mmp-dt.c new file +75 -0
75 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH v2 5/7 ARM: mmp: append OF support on pxa168
Quoted 3 lines · Haojian Zhuang
Enable PXA168 and aspenite support.
Signed-off-by Haojian Zhuang <[email protected]>
PATCH v2 5/7 ARM: mmp: append OF support on pxa168
In reply to Haojian ZhuangHi,
Quoted 18 lines · Haojian Zhuang
Enable PXA168 and aspenite support.
Signed-off-by Haojian Zhuang <[email protected]>--- arch/arm/mach-mmp/Kconfig | 10 ++++++ arch/arm/mach-mmp/Makefile | 1 + arch/arm/mach-mmp/mmp-dt.c | 75 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-mmp/mmp-dt.carch/arm/mach-mmp/Kconfig +0
-0@@ -2,6 +2,16 @@ if ARCH_MMP menu "Marvell PXA168/910/MMP2 Implmentations"
arch/arm/mach-mmp/Kconfig if ARCH_MMP · line 14
Quoted 10 lines
arch/arm/mach-mmp/Kconfig +10
-0+config MACH_MMP_DT + bool "Support MMP2 platforms from device tree" + select CPU_PXA168 + select CPU_PXA910 + select USE_OF + help + Include support for Marvell MMP2 based platforms using + the device tree. Needn't select any other machine while + MACH_MMP_DT is enabled. +
This patch breaks the build with MACH_MMP_DT=y in 3.4-rc2 for me:
MACH_MMP_DT selects CPU_PXA168 which selects CPU_32v5, which results in:
armv7-unknown-linux-gnueabi-gcc -Wp,-MD,arch/arm/mm/.dma-mapping.o.d -nostdinc -isystem /opt/crosstool/gcc-4.6.0/lib/gcc/armv7-unknown-linux-gnueabi/4.6.0/include -I/home/cjb/git/olpc-kernel/arch/arm/include -Iarch/arm/include/generated -Iinclude -I/home/cjb/git/olpc-kernel/include -include /home/cjb/git/olpc-kernel/include/linux/kconfig.h -I/home/cjb/git/olpc-kernel/arch/arm/mm -Iarch/arm/mm -D__KERNEL__ -mlittle-endian -I/home/cjb/git/olpc-kernel/arch/arm/mach-mmp/include -I/home/cjb/git/olpc-kernel/arch/arm/plat-pxa/include -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Werror-implicit-function-declaration -Wno-format-security -fno-delete-null-pointer-checks -Os -marm -fno-dwarf2-cfi-asm -mabi=aapcs-linux -mno-thumb-interwork -funwind-tables -D__LINUX_ARM_ARCH__=5 -march=armv5te -msoft-float -Uarm -Wframe-larger-than=1024 -fno-stack-protector -Wno-unused-but-set-variable -fomit-frame-pointer -g -Wdeclaration-after-statement -Wno-pointer-sign -fno-strict-overflow -fconserve-stack -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(dma_mapping)" -D"KBUILD_MODNAME=KBUILD_STR(dma_mapping)" -c -o arch/arm/mm/dma-mapping.o /home/cjb/git/olpc-kernel/arch/arm/mm/dma-mapping.c/home/cjb/git/olpc-kernel/arch/arm/kernel/entry-armv.S: Assembler messages:/home/cjb/git/olpc-kernel/arch/arm/kernel/entry-armv.S:207: Error: selected processor does not support ARM mode `clrex'
CPU_32v5=y adds:
arch/arm/Makefile:arch-$(CONFIG_CPU_32v5) :=-D__LINUX_ARM_ARCH__=5 $(call cc-option,-march=armv5te,-march=armv4t)
- Chris.
Signature · 2 lines
Chris Ball <[email protected]> <http://printf.net/> One Laptop Per Child
PATCH v2 5/7 ARM: mmp: append OF support on pxa168
Quoted 36 lines · Chris Ball
Hi,
Quoted 18 lines · Haojian Zhuang
Enable PXA168 and aspenite support.
Signed-off-by Haojian Zhuang <[email protected]>---
?arch/arm/mach-mmp/Kconfig ?| ? 10 ++++++ ?arch/arm/mach-mmp/Makefile | ? ?1 + ?arch/arm/mach-mmp/mmp-dt.c | ? 75 ++++++++++++++++++++++++++++++++++++++++++++?3 files changed, 86 insertions(+), 0 deletions(-) ?create mode 100644
arch/arm/mach-mmp/mmp-dt.carch/arm/mach-mmp/Kconfig +0
-0@@ -2,6 +2,16 @@ if ARCH_MMP?menu "Marvell PXA168/910/MMP2 Implmentations"
arch/arm/mach-mmp/Kconfig
Quoted 10 lines
arch/arm/mach-mmp/Kconfig +10
-0+config MACH_MMP_DT + ? ? bool "Support MMP2 platforms from device tree" + ? ? select CPU_PXA168 + ? ? select CPU_PXA910 + ? ? select USE_OF + ? ? help + ? ? ? Include support for Marvell MMP2 based platforms using + ? ? ? the device tree. Needn't select any other machine while + ? ? ? MACH_MMP_DT is enabled. +This patch breaks the build with MACH_MMP_DT=y in 3.4-rc2 for me:
MACH_MMP_DTselectsCPU_PXA168which selectsCPU_32v5, which results in:
Quoted 9 lines
?armv7-unknown-linux-gnueabi-gcc -Wp,-MD,arch/arm/mm/.dma-mapping.o.d ?-nostdinc -isystem
/opt/crosstool/gcc-4.6.0/lib/gcc/armv7-unknown-linux-gnueabi/4.6.0/include-I/home/cjb/git/olpc-kernel/arch/arm/include -Iarch/arm/include/generated -Iinclude ?-I/home/cjb/git/olpc-kernel/include -include/home/cjb/git/olpc-kernel/include/linux/kconfig.h?-I/home/cjb/git/olpc-kernel/arch/arm/mm -Iarch/arm/mm -D__KERNEL__ -mlittle-endian ? -I/home/cjb/git/olpc-kernel/arch/arm/mach-mmp/include ? -I/home/cjb/git/olpc-kernel/arch/arm/plat-pxa/include -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Werror-implicit-function-declaration -Wno-format-security -fno-delete-null-pointer-checks -Os -marm -fno-dwarf2-cfi-asm -mabi=aapcs-linux -mno-thumb-interwork -funwind-tables -D__LINUX_ARM_ARCH__=5 -march=armv5te -msoft-float -Uarm -Wframe-larger-than=1024 -fno-stack-protector -Wno-unused-but-set-variable -fomit-frame-pointer -g -Wdeclaration-after-statement -Wno-pointer ?-sign -fno-strict-overflow -fconserve-stack ? ?-D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(dma_mapping)" ?-D"KBUILD_MODNAME=KBUILD_STR(dma_mapping)" -c -oarch/arm/mm/dma-mapping.o/home/cjb/git/olpc-kernel/arch/arm/mm/dma-mapping.c/home/cjb/git/olpc-kernel/arch/arm/kernel/entry-armv.S: Assembler messages:/home/cjb/git/olpc-kernel/arch/arm/kernel/entry-armv.S:207: Error: selected processor does not support ARM mode `clrex'CPU_32v5=y adds:
arch/arm/Makefile:arch-$(CONFIG_CPU_32v5) :=-D__LINUX_ARM_ARCH__=5 $(call cc-option,-march=armv5te,-march=armv4t)
There's a typo error in comments. MMP_DT is only for PXA168 & PXA910. We need to append MMP2_DT for ARMv7 in arch-mmp directory. Since we are not intended to build ARMv5 and ARMv7 together.
Thanks
Haojian
PATCH v2 5/7 ARM: mmp: append OF support on pxa168
Hi,
Quoted 3 lines · Haojian Zhuang
There's a typo error in comments.
MMP_DTis only for PXA168 & PXA910. We need to appendMMP2_DTfor ARMv7 in arch-mmp directory. Since we are not intended to build ARMv5 and ARMv7 together.
Okay. In that case, you should make sure that MMP_DT cannot be selected when CPU_MMP2 is selected, by encoding that dependency in the Kconfig.
Thanks,
- Chris.
Signature · 2 lines
Chris Ball <[email protected]> <http://printf.net/> One Laptop Per Child
PATCH v2 5/7 ARM: mmp: append OF support on pxa168
Quoted 10 lines · Chris Ball
Hi,
Quoted 3 lines · Haojian Zhuang
There's a typo error in comments.
MMP_DTis only for PXA168 & PXA910. We need to appendMMP2_DTfor ARMv7 in arch-mmp directory. Since we are not intended to build ARMv5 and ARMv7 together.Okay. ?In that case, you should make sure that
MMP_DTcannot be selected whenCPU_MMP2is selected, by encoding that dependency in theKconfig.
Sure. I'll protect it in Kconfig.
Thanks
Haojian
PATCH v2 5/7 ARM: mmp: append OF support on pxa168
In reply to Haojian ZhuangHi,
arch/arm/mach-mmp/mmp-dt.c line 47
Quoted 13 lines · Haojian Zhuang
arch/arm/mach-mmp/mmp-dt.c new file +13
-0+static int __init mmp_intc_add_irq_domain (struct device_node *np, +struct device_node *parent) +{ +irq_domain_add_simple (np,0 ); +return 0 ; +} + +static int __init mmp_gpio_add_irq_domain (struct device_node *np, +struct device_node *parent) +{ +irq_domain_add_simple (np,IRQ_GPIO_START ); +return 0 ; +}
This patch was merged for 3.4-rc1, but it fails to compile because Grant removed irq_domain_add_simple() in 3.4-rc1.
- Chris.
Signature · 2 lines
Chris Ball <[email protected]> <http://printf.net/> One Laptop Per Child
PATCH v2 5/7 ARM: mmp: append OF support on pxa168
Quoted 20 lines · Chris Ball
Hi,
arch/arm/mach-mmp/mmp-dt.c line 47
Quoted 13 lines · Haojian Zhuang
arch/arm/mach-mmp/mmp-dt.c new file +13
-0+static int __init mmp_intc_add_irq_domain (struct device_node *np, + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct device_node *parent) +{ + ? ? irq_domain_add_simple(np, 0); + ? ? return 0; +} + +static int __init mmp_gpio_add_irq_domain (struct device_node *np, + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct device_node *parent) +{ + ? ? irq_domain_add_simple(np, IRQ_GPIO_START); + ? ? return 0; +}This patch was merged for 3.4-rc1, but it fails to compile because Grant removed
irq_domain_add_simple()in 3.4-rc1.
Noticed. I'll fix it.
Best Regards
Haojian
PATCH v2 6/7 ARM: dts: append DTS file of pxa168
In reply to Haojian ZhuangDTS files of both PXA168 and aspenite are appended.
--- arch/arm/boot/dts/pxa168-aspenite.dts | 38 +++++++++++++ arch/arm/boot/dts/pxa168.dtsi | 98 +++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 0 deletions(-) create mode 100644 arch/arm/boot/dts/pxa168-aspenite.dts create mode 100644 arch/arm/boot/dts/pxa168.dtsi
arch/arm/boot/dts/pxa168-aspenite.dts new file +38 -0
@@ -0,0 +1,38 @@ +/* + * Copyright (C) 2012 Marvell Technology Group Ltd. + * Author: Haojian Zhuang <[email protected]> + * + * 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 + * publishhed by the Free Software Foundation. + */ + +/dts-v1/; +/include/ "pxa168.dtsi" + +/ { + model = "Marvell PXA168 Aspenite Development Board"; + compatible = "mrvl,pxa168-aspenite", "mrvl,pxa168"; + + chosen { + bootargs = "console=ttyS0,115200 root=/dev/nfs nfsroot=192.168.1.100:/nfsroot/ ip=192.168.1.101:192.168.1.100::255.255.255.0::eth0:on"; + }; + + memory { + reg = <0x00000000 0x04000000>; + }; + + soc { + apb at d4000000 { + uart1: uart at d4017000 { + status = "okay"; + }; + twsi1: i2c at d4011000 { + status = "okay"; + }; + rtc: rtc at d4010000 { + status = "okay"; + }; + }; + }; +};
arch/arm/boot/dts/pxa168.dtsi new file +98 -0
98 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH v2 6/7 ARM: dts: append DTS file of pxa168
Quoted 4 lines · Haojian Zhuang
DTS files of both PXA168 and aspenite are appended.
Signed-off-by Haojian Zhuang <[email protected]>
PATCH v2 7/7 Document: devicetree: add OF documents for arch-mmp
In reply to Haojian ZhuangAdd OF support in Document/devicetree directory.
--- Documentation/devicetree/bindings/arm/mrvl.txt | 6 +++ .../devicetree/bindings/gpio/mrvl-gpio.txt | 23 ++++++++++++ Documentation/devicetree/bindings/i2c/mrvl-i2c.txt | 37 ++++++++++++++++++++ .../devicetree/bindings/rtc/sa1100-rtc.txt | 17 +++++++++ .../devicetree/bindings/serial/mrvl-serial.txt | 4 ++ 5 files changed, 87 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/mrvl.txt create mode 100644 Documentation/devicetree/bindings/gpio/mrvl-gpio.txt create mode 100644 Documentation/devicetree/bindings/i2c/mrvl-i2c.txt create mode 100644 Documentation/devicetree/bindings/rtc/sa1100-rtc.txt create mode 100644 Documentation/devicetree/bindings/serial/mrvl-serial.txt
Documentation/devicetree/bindings/arm/mrvl.txt new file +6 -0
@@ -0,0 +1,6 @@ +Marvell Platforms Device Tree Bindings +---------------------------------------------------- + +PXA168 Aspenite Board +Required root node properties: + - compatible = "mrvl,pxa168-aspenite", "mrvl,pxa168";
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +23 -0
@@ -0,0 +1,23 @@ +* Marvell PXA GPIO controller + +Required properties: +- compatible : Should be "mrvl,pxa-gpio" or "mrvl,mmp-gpio" +- reg : Address and length of the register set for the device +- interrupts : Should be the port interrupt shared by all gpio pins, if +- interrupt-name : Should be the name of irq resource. + one number. +- gpio-controller : Marks the device node as a gpio controller. +- #gpio-cells : Should be one. It is the pin number. + +Example: + + gpio: gpio at d4019000 { + compatible = "mrvl,mmp-gpio", "mrvl,pxa-gpio"; + reg = <0xd4019000 0x1000>; + interrupts = <49>, <17>, <18>; + interrupt-name = "gpio_mux", "gpio0", "gpio1"; + gpio-controller; + #gpio-cells = <1>; + interrupt-controller; + #interrupt-cells = <1>; + };
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +37 -0
@@ -0,0 +1,37 @@ +* I2C + +Required properties : + + - reg : Offset and length of the register set for the device + - compatible : should be "mrvl,mmp-twsi" where CHIP is the name of a + compatible processor, e.g. pxa168, pxa910, mmp2, mmp3. + For the pxa2xx/pxa3xx, an additional node "mrvl,pxa-i2c" is required + as shown in the example below. + +Recommended properties : + + - interrupts : <a b> where a is the interrupt number and b is a + field that represents an encoding of the sense and level + information for the interrupt. This should be encoded based on + the information in section 2) depending on the type of interrupt + controller you have. + - interrupt-parent : the phandle for the interrupt controller that + services interrupts for this device. + - mrvl,i2c-polling : Disable interrupt of i2c controller. Polling + status register of i2c controller instead. + - mrvl,i2c-fast-mode : Enable fast mode of i2c controller. + +Examples: + twsi1: i2c at d4011000 { + compatible = "mrvl,mmp-twsi", "mrvl,pxa-i2c"; + reg = <0xd4011000 0x1000>; + interrupts = <7>; + mrvl,i2c-fast-mode; + }; + + twsi2: i2c at d4025000 { + compatible = "mrvl,mmp-twsi", "mrvl,pxa-i2c"; + reg = <0xd4025000 0x1000>; + interrupts = <58>; + }; +
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +17 -0
@@ -0,0 +1,17 @@ +* Marvell Real Time Clock controller + +Required properties: +- compatible: should be "mrvl,sa1100-rtc" +- reg: physical base address of the controller and length of memory mapped + region. +- interrupts: Should be two. The first interrupt number is the rtc alarm + interrupt and the second interrupt number is the rtc hz interrupt. +- interrupt-names: Assign name of irq resource. + +Example: + rtc: rtc at d4010000 { + compatible = "mrvl,mmp-rtc"; + reg = <0xd4010000 0x1000>; + interrupts = <5>, <6>; + interrupt-name = "rtc 1Hz", "rtc alarm"; + };
Documentation/devicetree/bindings/serial/mrvl-serial.txt new file +4 -0
@@ -0,0 +1,4 @@ +PXA UART controller + +Required properties: +- compatible : should be "mrvl,mmp-uart" or "mrvl,pxa-uart".
Signature · 1 line
1.7.0.4
PATCH v2 7/7 Document: devicetree: add OF documents for arch-mmp
Quoted 3 lines · Haojian Zhuang
Add OF support in Document/devicetree directory.
Signed-off-by Haojian Zhuang <[email protected]>
One question for Benoit, Grant or Rob:
Quoted 5 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +5
-0+Required properties: +- compatible: should be "mrvl,sa1100-rtc" +- reg: physical base address of the controller and length of memory mapped + region. +- interrupts: Should be two. The first interrupt number is the rtc alarm
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt line 17
Quoted 10 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +10
-0+ interrupt and the second interrupt number is the rtc hz interrupt. +- interrupt-names: Assign name of irq resource. + +Example: + rtc: rtc at d4010000 { + compatible = "mrvl,mmp-rtc"; + reg = <0xd4010000 0x1000>; + interrupts = <5>, <6>; + interrupt-name = "rtc 1Hz", "rtc alarm"; + };
When the binding for a device lists the interrupt-names property, should it also list the specific values for it? This one says that which ones they are, but I think we should actually mandate the exact string in the binding if we want to allow drivers to get the resource by its name.
Arnd
PATCH v2 7/7 Document: devicetree: add OF documents for arch-mmp
Quoted 21 lines · Arnd Bergmann
Quoted 3 lines · Haojian Zhuang
Add OF support in Document/devicetree directory.
Signed-off-by Haojian Zhuang <[email protected]>Acked-by Arnd Bergmann <[email protected]>One question for Benoit, Grant or Rob:
Quoted 12 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +12
-0+Required properties: +- compatible: should be "mrvl,sa1100-rtc" +- reg: physical base address of the controller and length of memory mapped + region. +- interrupts: Should be two. The first interrupt number is the rtc alarm + interrupt and the second interrupt number is the rtc hz interrupt. +- interrupt-names: Assign name of irq resource. + +Example: + rtc: rtc at d4010000 { + compatible = "mrvl,mmp-rtc"; + reg = <0xd4010000 0x1000>;
Quoted 8 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt line 17
Quoted 3 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +3
-0+ interrupts = <5>, <6>; + interrupt-name = "rtc 1Hz", "rtc alarm"; + };When the binding for a device lists the interrupt-names property, should it also list the specific values for it? This one says that which ones they are, but I think we should actually mandate the exact string in the binding if we want to allow drivers to get the resource by its name.
Sounds good to me. Requiring documentation is a good way to discourage its use. :)
Ideally we would discourage spaces in the names, but since the whole point is using the existing names from Linux we can't really enforce that.
Rob
PATCH v2 7/7 Document: devicetree: add OF documents for arch-mmp
In reply to Arnd BergmannQuoted 21 lines · Arnd Bergmann
Quoted 3 lines · Haojian Zhuang
Add OF support in Document/devicetree directory.
Signed-off-by Haojian Zhuang<[email protected]>Acked-by Arnd Bergmann<[email protected]>One question for Benoit, Grant or Rob:
Quoted 12 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +12
-0+Required properties: +- compatible: should be "mrvl,sa1100-rtc" +- reg: physical base address of the controller and length of memory mapped + region. +- interrupts: Should be two. The first interrupt number is the rtc alarm + interrupt and the second interrupt number is the rtc hz interrupt. +- interrupt-names: Assign name of irq resource. + +Example: + rtc: rtc at d4010000 { + compatible = "mrvl,mmp-rtc"; + reg =<0xd4010000 0x1000>;
Quoted 8 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt line 17
Quoted 3 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +3
-0+ interrupts =<5>,<6>; + interrupt-name = "rtc 1Hz", "rtc alarm"; + };When the binding for a device lists the interrupt-names property, should it also list the specific values for it? This one says that which ones they are, but I think we should actually mandate the exact string in the binding if we want to allow drivers to get the resource by its name.
Yes, indeed. The names are not some random strings, the driver will use get_resource_by_name and expect the name to be aligned with HW documentation and thus should be well documented in the binding.
Regards,
Benoit
PATCH v2 7/7 Document: devicetree: add OF documents for arch-mmp
In reply to Haojian ZhuangThere are some inconsistencies in this patch as indicated in-line below:
Quoted 34 lines · Haojian Zhuang
Add OF support in Document/devicetree directory.
Signed-off-by Haojian Zhuang<[email protected]>--- Documentation/devicetree/bindings/arm/mrvl.txt | 6 +++ .../devicetree/bindings/gpio/mrvl-gpio.txt | 23 ++++++++++++ Documentation/devicetree/bindings/i2c/mrvl-i2c.txt | 37 ++++++++++++++++++++ .../devicetree/bindings/rtc/sa1100-rtc.txt | 17 +++++++++ .../devicetree/bindings/serial/mrvl-serial.txt | 4 ++ 5 files changed, 87 insertions(+), 0 deletions(-)create mode 100644
Documentation/devicetree/bindings/arm/mrvl.txtcreate mode 100644Documentation/devicetree/bindings/gpio/mrvl-gpio.txtcreate mode 100644Documentation/devicetree/bindings/i2c/mrvl-i2c.txtcreate mode 100644Documentation/devicetree/bindings/rtc/sa1100-rtc.txtcreate mode 100644Documentation/devicetree/bindings/serial/mrvl-serial.txtDocumentation/devicetree/bindings/arm/mrvl.txt new file +6
-0@@ -0,0 +1,6 @@ +Marvell Platforms Device Tree Bindings +---------------------------------------------------- + +PXA168 Aspenite Board +Required root node properties: + - compatible = "mrvl,pxa168-aspenite", "mrvl,pxa168";Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +0
-0
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt line 6
Quoted 6 lines
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +6
-0@@ -0,0 +1,23 @@ +* Marvell PXA GPIO controller + +Required properties: +- compatible : Should be "mrvl,pxa-gpio" or "mrvl,mmp-gpio" +- reg : Address and length of the register set for the device +- interrupts : Should be the port interrupt shared by all gpio pins, if
This sentence is incomplete. Maybe the "one number." two lines below is part of this sentence? If so, I'm not sure I understand the meaning. Did you mean to say "If a single interrupt is shared by all GPIO pins, the value is a single integer." ? What is the meaning if there are multiple integers in the value?
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt line 7
Quoted 2 lines
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +1
-0+- interrupt-name : Should be the name of irq resource.
"The name" implies that there is a single value, but the example below shows multiple values.
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt line 8
Quoted 2 lines
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +1
-0+ one number.
See above.
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt line 15
Quoted 8 lines
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +7
-0+- gpio-controller : Marks the device node as a gpio controller. +- #gpio-cells : Should be one. It is the pin number. + +Example: + + gpio: gpio at d4019000 { + compatible = "mrvl,mmp-gpio", "mrvl,pxa-gpio";
The text above says "mrvl,pxa-gpio" *or* "mrvl,mmp-gpio", but the example has both. If both are necessary, the word "and" would be more appropriate.
What is the point of having both names in the device tree? If the Linux driver is known to support the MMP version of the GPIO hardware, there
is no need for a "fallback" to the PXA version. Fallback names are
rarely needed for Linux, because new customized Linux kernels are easy to develop for new hardware. Fallback names were intended for commercial operating systems where the hardware vendor must work with an OS version that is already in the field.
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt line 22
Quoted 8 lines
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +7
-0+ reg =<0xd4019000 0x1000>; + interrupts =<49>,<17>,<18>; + interrupt-name = "gpio_mux", "gpio0", "gpio1"; + gpio-controller; + #gpio-cells =<1>; + interrupt-controller; + #interrupt-cells =<1>;
The text above does not mention "interrupt-controller" and "#interrupt-cells" properties for this node.
Quoted 8 lines
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +1
-0+ };Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +0
-0
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt line 6
Quoted 6 lines
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +6
-0@@ -0,0 +1,37 @@ +* I2C + +Required properties : + + - reg : Offset and length of the register set for the device + - compatible : should be "mrvl,mmp-twsi" where CHIP is the name of a
"CHIP" makes no sense in this sentence. I presume that you meant to refer to a template name that contains "CHIP", but "mrvl,mmp-twsi" does not contain "CHIP".
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt line 17
Quoted 12 lines
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +11
-0+ compatible processor, e.g. pxa168, pxa910, mmp2, mmp3. + For the pxa2xx/pxa3xx, an additional node "mrvl,pxa-i2c" is required + as shown in the example below. + +Recommended properties : + + - interrupts :<a b> where a is the interrupt number and b is a + field that represents an encoding of the sense and level + information for the interrupt. This should be encoded based on + the information in section 2) depending on the type of interrupt + controller you have.
Was this text copied from somewhere else and not edited? It doesn't make sense here, and the example below has only a single number, so <a b> cannot be correct.
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt line 19
Quoted 3 lines
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +2
-0+ - interrupt-parent : the phandle for the interrupt controller that + services interrupts for this device.
There is no interrupt-parent property in the example below. I presume you mean to use the default where the device tree parent node is the interrupt parent. If so, you should say that.
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt line 26
Quoted 8 lines
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +7
-0+ - mrvl,i2c-polling : Disable interrupt of i2c controller. Polling + status register of i2c controller instead. + - mrvl,i2c-fast-mode : Enable fast mode of i2c controller. + +Examples: + twsi1: i2c at d4011000 { + compatible = "mrvl,mmp-twsi", "mrvl,pxa-i2c";
No need for two different values, as discussed above.
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt line 33
Quoted 8 lines
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +7
-0+ reg =<0xd4011000 0x1000>; + interrupts =<7>; + mrvl,i2c-fast-mode; + }; + + twsi2: i2c at d4025000 { + compatible = "mrvl,mmp-twsi", "mrvl,pxa-i2c";
No need for two different values, as discussed above.
Quoted 38 lines
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +4
-0+ reg =<0xd4025000 0x1000>; + interrupts =<58>; + }; +Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +17
-0@@ -0,0 +1,17 @@ +* Marvell Real Time Clock controller + +Required properties: +- compatible: should be "mrvl,sa1100-rtc" +- reg: physical base address of the controller and length of memory mapped + region. +- interrupts: Should be two. The first interrupt number is the rtc alarm + interrupt and the second interrupt number is the rtc hz interrupt. +- interrupt-names: Assign name of irq resource. + +Example: + rtc: rtc at d4010000 { + compatible = "mrvl,mmp-rtc"; + reg =<0xd4010000 0x1000>; + interrupts =<5>,<6>; + interrupt-name = "rtc 1Hz", "rtc alarm"; + };Documentation/devicetree/bindings/serial/mrvl-serial.txt new file +4
-0@@ -0,0 +1,4 @@ +PXA UART controller + +Required properties: +- compatible : should be "mrvl,mmp-uart" or "mrvl,pxa-uart".