From: Ian Molton <hidden> Date: 2012-07-30 15:15:53
This patch series adds support for the CSB1724 SoM by Cogent.
It also provides mv643xx DT support, and the CSB1724 was my test platform
for this support.
The mv643xx driver needs a LOT more TLC, but this should be a pretty
non-invasive way to get started on it.
The CSB1724 SoM platform support is ready-to-go. Supported so far is
SATA, Ethernet, and Serial.
SATA is built upon Thomas' work.
Ian Molton (6):
Initial csb1724 board support (FDT)
mv643xx.c: Remove magic numbers.
mv643xx.c: Add basic device tree support.
kirkwood: setup clock only in eth helpers.
csb1724: Enable device tree based mv643xx ethernet support.
DT: Convert all kirkwood boards with mv643xx that use DT
arch/arm/boot/dts/kirkwood-csb1724.dts | 55 ++++++++++++++
arch/arm/boot/dts/kirkwood-dnskw.dtsi | 12 +++
arch/arm/boot/dts/kirkwood-dreamplug.dts | 24 ++++++
arch/arm/boot/dts/kirkwood-goflexnet.dts | 11 +++
arch/arm/boot/dts/kirkwood-ib62x0.dts | 13 ++++
arch/arm/boot/dts/kirkwood-iconnect.dts | 13 ++++
arch/arm/boot/dts/kirkwood-lsxl.dtsi | 23 ++++++
arch/arm/boot/dts/kirkwood-ts219-6281.dts | 11 ++-
arch/arm/boot/dts/kirkwood-ts219-6282.dts | 11 ++-
arch/arm/boot/dts/kirkwood-ts219.dtsi | 3 +
arch/arm/boot/dts/kirkwood.dtsi | 16 ++++
arch/arm/configs/csb1724_defconfig | 92 ++++++++++++++++++++++
arch/arm/mach-kirkwood/Kconfig | 7 ++
arch/arm/mach-kirkwood/Makefile | 1 +
arch/arm/mach-kirkwood/Makefile.boot | 1 +
arch/arm/mach-kirkwood/board-csb1724.c | 62 +++++++++++++++
arch/arm/mach-kirkwood/board-dnskw.c | 7 +-
arch/arm/mach-kirkwood/board-dreamplug.c | 13 +---
arch/arm/mach-kirkwood/board-dt.c | 4 +
arch/arm/mach-kirkwood/board-goflexnet.c | 7 +-
arch/arm/mach-kirkwood/board-ib62x0.c | 7 +-
arch/arm/mach-kirkwood/board-iconnect.c | 7 +-
arch/arm/mach-kirkwood/board-lsxl.c | 13 +---
arch/arm/mach-kirkwood/board-ts219.c | 10 +--
arch/arm/mach-kirkwood/common.c | 6 +-
arch/arm/mach-kirkwood/common.h | 6 ++
drivers/net/ethernet/marvell/mv643xx_eth.c | 114 ++++++++++++++++++++++++----
27 files changed, 475 insertions(+), 74 deletions(-)
create mode 100644 arch/arm/boot/dts/kirkwood-csb1724.dts
create mode 100644 arch/arm/configs/csb1724_defconfig
create mode 100644 arch/arm/mach-kirkwood/board-csb1724.c
--
1.7.9.5
From: Ian Molton <hidden> Date: 2012-07-30 15:15:56
This patch adds basic device tree support to the mv643xx ethernet driver.
It should be enough for most current users of the device, and should allow
a fairly painless migration once proper support for clk devices is available
to those platforms.
Signed-off-by: Ian Molton <redacted>
---
drivers/net/ethernet/marvell/mv643xx_eth.c | 111 ++++++++++++++++++++++++----
1 file changed, 97 insertions(+), 14 deletions(-)
@@ -2625,6 +2628,23 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)if(msp->base==NULL)gotoout_free;+if(pdev->dev.of_node){+structdevice_node*np=NULL;++/* when all users of this driver use FDT, we can remove this */+pd=kzalloc(sizeof(*pd),GFP_ATOMIC);+if(!pd){+dev_dbg(&pdev->dev,"Could not allocate platform data\n");+gotoout_free;+}++np=of_parse_phandle(pdev->dev.of_node,"shared_smi",0);+if(np)+pd->shared_smi=of_find_device_by_node(np);++}else{+pd=pdev->dev.platform_data;+}/**SetupandregisterSMIbus.*/
@@ -2654,15 +2674,22 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)/**Checkwhethertheerrorinterruptishookedup.*/-res=platform_get_resource(pdev,IORESOURCE_IRQ,0);-if(res!=NULL){+if(pdev->dev.of_node){+irq=irq_of_parse_and_map(pdev->dev.of_node,0);+}else{+res=platform_get_resource(pdev,IORESOURCE_IRQ,0);+if(res)+irq=res->start;+}++if(irq!=-1){interr;-err=request_irq(res->start,mv643xx_eth_err_irq,+err=request_irq(irq,mv643xx_eth_err_irq,IRQF_SHARED,"mv643xx_eth",msp);if(!err){writel(ERR_INT_SMI_DONE,msp->base+ERR_INT_MASK);-msp->err_interrupt=res->start;+msp->err_interrupt=irq;}}
@@ -2675,6 +2702,10 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)msp->tx_csum_limit=(pd!=NULL&&pd->tx_csum_limit)?pd->tx_csum_limit:9*1024;++if(pdev->dev.of_node)+kfree(pd);/* If we created a fake pd, free it now */+infer_hw_params(msp);platform_set_drvdata(pdev,msp);
@@ -2708,12 +2739,21 @@ static int mv643xx_eth_shared_remove(struct platform_device *pdev)return0;}+#ifdef CONFIG_OF+staticstructof_device_idmv_mdio_dt_ids[]__devinitdata={+{.compatible="marvell,mdio-mv643xx",},+{},+};+MODULE_DEVICE_TABLE(of,mv_mdio_dt_ids);+#endif+staticstructplatform_drivermv643xx_eth_shared_driver={.probe=mv643xx_eth_shared_probe,.remove=mv643xx_eth_shared_remove,.driver={.name=MV643XX_ETH_SHARED_NAME,.owner=THIS_MODULE,+.of_match_table=of_match_ptr(mv_mdio_dt_ids),},};
@@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device *pdev)structresource*res;interr;-pd=pdev->dev.platform_data;+if(pdev->dev.of_node){+structdevice_node*np=NULL;++/* when all users of this driver use FDT, we can remove this */+pd=kzalloc(sizeof(*pd),GFP_ATOMIC);+if(!pd){+dev_dbg(&pdev->dev,"Could not allocate platform data\n");+return-ENOMEM;+}++of_property_read_u32(pdev->dev.of_node,+"port_number",&pd->port_number);+of_property_read_u32(pdev->dev.of_node,+"phy_addr",&pd->phy_addr);+np=of_parse_phandle(pdev->dev.of_node,"mdio",0);+if(np){+pd->shared=of_find_device_by_node(np);+}else{+kfree(pd);+return-ENODEV;+}+}else{+pd=pdev->dev.platform_data;+}+if(pd==NULL){dev_err(&pdev->dev,"no mv643xx_eth_platform_data\n");return-ENODEV;
@@ -2881,12 +2945,15 @@ static int mv643xx_eth_probe(struct platform_device *pdev)if(pd->shared==NULL){dev_err(&pdev->dev,"no mv643xx_eth_platform_data->shared\n");-return-ENODEV;+err=-ENODEV;+gotoout_free_pd;}dev=alloc_etherdev_mq(sizeof(structmv643xx_eth_private),8);-if(!dev)-return-ENOMEM;+if(!dev){+err=-ENOMEM;+gotoout_free_pd;+}mp=netdev_priv(dev);platform_set_drvdata(pdev,mp);
@@ -2923,6 +2990,8 @@ static int mv643xx_eth_probe(struct platform_device *pdev)init_pscr(mp,pd->speed,pd->duplex);+if(pdev->dev.of_node)+kfree(pd);/* If we created a fake pd, free it now */mib_counters_clear(mp);
@@ -2942,10 +3011,13 @@ static int mv643xx_eth_probe(struct platform_device *pdev)mp->rx_oom.data=(unsignedlong)mp;mp->rx_oom.function=oom_timer_wrapper;--res=platform_get_resource(pdev,IORESOURCE_IRQ,0);-BUG_ON(!res);-dev->irq=res->start;+if(pdev->dev.of_node){+dev->irq=irq_of_parse_and_map(pdev->dev.of_node,0);+}else{+res=platform_get_resource(pdev,IORESOURCE_IRQ,0);+BUG_ON(!res);+dev->irq=res->start;+}dev->netdev_ops=&mv643xx_eth_netdev_ops;
From: Ian Molton <hidden> Date: 2012-07-30 15:15:57
This patch modifies the ethernet setup helper functions so that they can be
used /purely/ to set up the clocks.
This is part of ongoing work to enable device tree support in the mv643xx.c
ethernet driver, where as yet the kirkwood platform does not have proper clk
support.
This should allow a gradual migration to device tree and later to proper clk
support, wherupon the helper functions can be removed entirely.
Signed-off-by: Ian Molton <redacted>
---
arch/arm/mach-kirkwood/common.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
@@ -299,7 +299,8 @@ void __init kirkwood_ehci_init(void)****************************************************************************/void__initkirkwood_ge00_init(structmv643xx_eth_platform_data*eth_data){-orion_ge00_init(eth_data,+if(eth_data)+orion_ge00_init(eth_data,GE00_PHYS_BASE,IRQ_KIRKWOOD_GE00_SUM,IRQ_KIRKWOOD_GE00_ERR);/* The interface forgets the MAC address assigned by u-boot if
From: Ian Molton <hidden> Date: 2012-07-30 15:15:58
This patch enables mv643xx based ethernet built into the SoM on the
csb1724, via flattened device tree.
Signed-off-by: Ian Molton [off-list ref]
---
arch/arm/boot/dts/kirkwood-csb1724.dts | 25 ++++++++++++++++++
arch/arm/boot/dts/kirkwood.dtsi | 16 ++++++++++++
arch/arm/configs/csb1724_defconfig | 45 ++++++++++++++++++++++++++++++++
arch/arm/mach-kirkwood/board-csb1724.c | 3 +++
4 files changed, 89 insertions(+)
@@ -34,14 +33,6 @@#include"common.h"#include"mpp.h"-staticstructmv643xx_eth_platform_datadreamplug_ge00_data={-.phy_addr=MV643XX_ETH_PHY_ADDR(0),-};--staticstructmv643xx_eth_platform_datadreamplug_ge01_data={-.phy_addr=MV643XX_ETH_PHY_ADDR(1),-};-staticstructmvsdio_platform_datadreamplug_mvsdio_data={/* unfortunately the CD signal has not been connected */};
@@ -36,10 +35,6 @@#include"common.h"#include"mpp.h"-staticstructmv643xx_eth_platform_datagoflexnet_ge00_data={-.phy_addr=MV643XX_ETH_PHY_ADDR(0),-};-staticunsignedintgoflexnet_mpp_config[]__initdata={MPP29_GPIO,/* USB Power Enable */MPP47_GPIO,/* LED Orange */
On Mon, Jul 30, 2012 at 04:15:56PM +0100, Ian Molton wrote:
This patch adds basic device tree support to the mv643xx ethernet driver.
It should be enough for most current users of the device, and should allow
a fairly painless migration once proper support for clk devices is available
to those platforms.
Signed-off-by: Ian Molton <redacted>
---
drivers/net/ethernet/marvell/mv643xx_eth.c | 111 ++++++++++++++++++++++++----
1 file changed, 97 insertions(+), 14 deletions(-)
Hi Ian
Please document the DT binding in Documentation/devicetree/binding/....
Also, this should also be CC: to the netdev mailing. I got into
trouble for not doing this and breaking the PPC build :-(
Andrew
On Mon, Jul 30, 2012 at 04:15:58PM +0100, Ian Molton wrote:
quoted hunk
This patch enables mv643xx based ethernet built into the SoM on the
csb1724, via flattened device tree.
Signed-off-by: Ian Molton [off-list ref]
---
arch/arm/boot/dts/kirkwood-csb1724.dts | 25 ++++++++++++++++++
arch/arm/boot/dts/kirkwood.dtsi | 16 ++++++++++++
arch/arm/configs/csb1724_defconfig | 45 ++++++++++++++++++++++++++++++++
arch/arm/mach-kirkwood/board-csb1724.c | 3 +++
4 files changed, 89 insertions(+)
Hi Ian
I'm trying to keep this in order. Currently i2c is in the wrong place,
but the rest are incremental. It looks like these should be between
wdt & sata.
What about default entries for egiga0 and egiga1 with status
"disabled"? It looks like the only thing which changes between boards
is phy_addr.
Andrew
On Mon, Jul 30, 2012 at 04:15:57PM +0100, Ian Molton wrote:
This patch modifies the ethernet setup helper functions so that they can be
used /purely/ to set up the clocks.
Hi Ian
Set up is a bit ambiguous. I would actually say, start the clock
ticking, if it is not already.
I'm also not sure this is the best way to do it. I'd like to throw in
a counter proposal, and then we can discuss....
Problems start then the ethernet driver is a module, not built in. In
this situation, the clock gets turned off, in a lateinitcall, and then
later turned back on again when the module loads. Unfortunately, by
then its forgotten its own MAC address, as programmed by u-boot. So
this clk_prepare_enable() is here to ensure that the clock does not
get turned off, when we know the module is likely to be loaded
sometime later.
I've not looked at the clk DT bindings yet. Does it provide a
mechanism to prepare & enable a named clock? Maybe it does, but this
seems a bit of an edge case.
What i would instead do is add code to board-dt.c which looks into the
DT and see if it finds nodes egige0/egige1 and if so, calls
clk_prepare_enable(). We then don't need any per board code.
Andrew
quoted hunk
This is part of ongoing work to enable device tree support in the mv643xx.c
ethernet driver, where as yet the kirkwood platform does not have proper clk
support.
This should allow a gradual migration to device tree and later to proper clk
support, wherupon the helper functions can be removed entirely.
Signed-off-by: Ian Molton <redacted>
---
arch/arm/mach-kirkwood/common.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
@@ -299,7 +299,8 @@ void __init kirkwood_ehci_init(void)****************************************************************************/void__initkirkwood_ge00_init(structmv643xx_eth_platform_data*eth_data){-orion_ge00_init(eth_data,+if(eth_data)+orion_ge00_init(eth_data,GE00_PHYS_BASE,IRQ_KIRKWOOD_GE00_SUM,IRQ_KIRKWOOD_GE00_ERR);/* The interface forgets the MAC address assigned by u-boot if
On Mon, Jul 30, 2012 at 04:15:58PM +0100, Ian Molton wrote:
quoted hunk
This patch enables mv643xx based ethernet built into the SoM on the
csb1724, via flattened device tree.
Signed-off-by: Ian Molton [off-list ref]
---
arch/arm/boot/dts/kirkwood-csb1724.dts | 25 ++++++++++++++++++
arch/arm/boot/dts/kirkwood.dtsi | 16 ++++++++++++
arch/arm/configs/csb1724_defconfig | 45 ++++++++++++++++++++++++++++++++
arch/arm/mach-kirkwood/board-csb1724.c | 3 +++
4 files changed, 89 insertions(+)
Hi Ian
Shouldn't there be two interrupt here?
292 orion_ge00_init(eth_data,
293 GE00_PHYS_BASE, IRQ_KIRKWOOD_GE00_SUM,
294 IRQ_KIRKWOOD_GE00_ERR);
Documentation of the binding would help....
Andrew
Hi Ian,
On Mon, Jul 30, 2012 at 8:45 PM, Ian Molton [off-list ref]wrote:
quoted hunk
This patch adds basic device tree support to the mv643xx ethernet driver.
It should be enough for most current users of the device, and should allow
a fairly painless migration once proper support for clk devices is
available
to those platforms.
Signed-off-by: Ian Molton <redacted>
---
drivers/net/ethernet/marvell/mv643xx_eth.c | 111
++++++++++++++++++++++++----
1 file changed, 97 insertions(+), 14 deletions(-)
@@ -2873,7 +2913,31 @@ static int mv643xx_eth_probe(struct platform_device
*pdev)
struct resource *res;
int err;
- pd = pdev->dev.platform_data;
+ if (pdev->dev.of_node) {
+ struct device_node *np = NULL;
+
+ /* when all users of this driver use FDT, we can remove
this */
+ pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
+ if (!pd) {
+ dev_dbg(&pdev->dev, "Could not allocate platform
data\n");
+ return -ENOMEM;
+ }
+
+ of_property_read_u32(pdev->dev.of_node,
+ "port_number", &pd->port_number);
+ of_property_read_u32(pdev->dev.of_node,
+ "phy_addr", &pd->phy_addr);
+ np = of_parse_phandle(pdev->dev.of_node, "mdio", 0);
+ if (np) {
+ pd->shared = of_find_device_by_node(np);
+ } else {
+ kfree(pd);
+ return -ENODEV;
+ }
+ } else {
+ pd = pdev->dev.platform_data;
+ }
+
if (pd == NULL) {
dev_err(&pdev->dev, "no mv643xx_eth_platform_data\n");
return -ENODEV;
Can this check for pd be moved in the else part above as well, as for the
pd allocation with kzalloc,
we have already made a check for memory allocation failure?
quoted hunk
@@ -2881,12 +2945,15 @@ static int mv643xx_eth_probe(struct
platform_device *pdev)
if (pd->shared == NULL) {
dev_err(&pdev->dev, "no
mv643xx_eth_platform_data->shared\n");
- return -ENODEV;
+ err = -ENODEV;
+ goto out_free_pd;
}
dev = alloc_etherdev_mq(sizeof(struct mv643xx_eth_private), 8);
- if (!dev)
- return -ENOMEM;
+ if (!dev) {
+ err = -ENOMEM;
+ goto out_free_pd;
+ }
mp = netdev_priv(dev);
platform_set_drvdata(pdev, mp);
@@ -2923,6 +2990,8 @@ static int mv643xx_eth_probe(struct platform_device
*pdev)
init_pscr(mp, pd->speed, pd->duplex);
+ if (pdev->dev.of_node)
+ kfree(pd); /* If we created a fake pd, free it now */
mib_counters_clear(mp);
@@ -2942,10 +3011,13 @@ static int mv643xx_eth_probe(struct
From: Ian Molton <hidden> Date: 2012-07-30 16:28:09
On 30/07/12 16:28, Andrew Lunn wrote:
Hi Ian
quoted
diff --git a/arch/arm/configs/csb1724_defconfig
b/arch/arm/configs/csb1724_defconfig new file mode 100644 index
0000000..927b269 --- /dev/null +++
b/arch/arm/configs/csb1724_defconfig @@ -0,0 +1,47 @@
+CONFIG_ARCH_KIRKWOOD=y +CONFIG_ARCH_KIRKWOOD_DT=y
+CONFIG_MACH_CSB1724_DT=y
Do you really need this? Its the only kirkwood board with its on
_defconfig file. What makes this board special?
Nothing. defconfigs are just there to make configuring your kernel easy.
The kirkwood defconfig doesnt fit the currently supported hardware
set on the csb1724 very well. Why build stuff you dont need?
-Ian
From: Ian Molton <hidden> Date: 2012-07-30 16:32:39
On 30/07/12 17:19, Amar Nath wrote:
Hi Ian,
On Mon, Jul 30, 2012 at 8:45 PM, Ian Molton [off-list ref]wrote:
quoted
- pd = pdev->dev.platform_data;
+ if (pdev->dev.of_node) {
+ struct device_node *np = NULL;
+
+ /* when all users of this driver use FDT, we can remove
this */
+ pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
+ if (!pd) {
+ dev_dbg(&pdev->dev, "Could not allocate platform
data\n");
+ return -ENOMEM;
+ }
+
+ of_property_read_u32(pdev->dev.of_node,
+ "port_number", &pd->port_number);
+ of_property_read_u32(pdev->dev.of_node,
+ "phy_addr", &pd->phy_addr);
+ np = of_parse_phandle(pdev->dev.of_node, "mdio", 0);
+ if (np) {
+ pd->shared = of_find_device_by_node(np);
+ } else {
+ kfree(pd);
+ return -ENODEV;
+ }
+ } else {
+ pd = pdev->dev.platform_data;
+ }
+
if (pd == NULL) {
dev_err(&pdev->dev, "no mv643xx_eth_platform_data\n");
return -ENODEV;
Can this check for pd be moved in the else part above as well, as for the
pd allocation with kzalloc,
we have already made a check for memory allocation failure?
If I'm reading this correctly, if kirkwood_ge00_init is called without
any data, all that happens is the clock is enabled. If the right data
is passed in, the ethernet interface is initialized, and then the
clock is enabled.
Patch 6 in your series sets all the existing DT board files to call
kirkwood_ge00_init with no data.
Would it make more sense for readability to separate the clock prep
and other interface initialization so DT board files don't appear to
be calling for a full turn up despite now handling that in the DTS
tree? IE instead of converting all the board-XXX.c to call
kirkwood_ge00_init(NULL) have them call a new function,
kirkwood_ge00_clk_prep(NULL) instead, assuming the clock can't be
handled directly via DTS.
Josh C
On Mon, Jul 30, 2012 at 11:15 AM, Ian Molton [off-list ref] wrote:
quoted hunk
This patch modifies the ethernet setup helper functions so that they can be
used /purely/ to set up the clocks.
This is part of ongoing work to enable device tree support in the mv643xx.c
ethernet driver, where as yet the kirkwood platform does not have proper clk
support.
This should allow a gradual migration to device tree and later to proper clk
support, wherupon the helper functions can be removed entirely.
Signed-off-by: Ian Molton <redacted>
---
arch/arm/mach-kirkwood/common.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
@@ -299,7 +299,8 @@ void __init kirkwood_ehci_init(void)****************************************************************************/void__initkirkwood_ge00_init(structmv643xx_eth_platform_data*eth_data){-orion_ge00_init(eth_data,+if(eth_data)+orion_ge00_init(eth_data,GE00_PHYS_BASE,IRQ_KIRKWOOD_GE00_SUM,IRQ_KIRKWOOD_GE00_ERR);/* The interface forgets the MAC address assigned by u-boot if
From: Ian Molton <hidden> Date: 2012-07-30 16:49:02
On 30/07/12 16:39, Andrew Lunn wrote:
On Mon, Jul 30, 2012 at 04:15:56PM +0100, Ian Molton wrote:
quoted
This patch adds basic device tree support to the mv643xx ethernet
driver.
It should be enough for most current users of the device, and
should allow a fairly painless migration once proper support for
clk devices is available to those platforms.
Signed-off-by: Ian Molton <redacted> ---
drivers/net/ethernet/marvell/mv643xx_eth.c | 111
++++++++++++++++++++++++---- 1 file changed, 97 insertions(+), 14
deletions(-)
Hi Ian
Please document the DT binding in
Documentation/devicetree/binding/....
Done.
Also, this should also be CC: to the netdev mailing. I got into
trouble for not doing this and breaking the PPC build :-(
From: Ian Molton <hidden> Date: 2012-07-30 16:52:57
On 30/07/12 17:12, Andrew Lunn wrote:
On Mon, Jul 30, 2012 at 04:15:57PM +0100, Ian Molton wrote:
quoted
This patch modifies the ethernet setup helper functions so that
they can be used /purely/ to set up the clocks.
Hi Ian
Set up is a bit ambiguous. I would actually say, start the clock
ticking, if it is not already.
Fair.
I'm also not sure this is the best way to do it. I'd like to throw
in a counter proposal, and then we can discuss....
Problems start then the ethernet driver is a module, not built in.
I find that even if built in, it fails, not because it forgets its MAC, but
later, it hangs because as yet it does not know how to handle the clk
when it is set up from DT bindings. (Kirkwood doesnt really have this
support yet AFAICT).
I've not looked at the clk DT bindings yet. Does it provide a
mechanism to prepare & enable a named clock? Maybe it does, but this
seems a bit of an edge case.
If it does, I cant see it.
What i would instead do is add code to board-dt.c which looks into
the DT and see if it finds nodes egige0/egige1 and if so, calls
clk_prepare_enable(). We then don't need any per board code.
I agree, this seems like a good compromise for now.
I will implement this for v2.
-Ian
-Ian
From: Ian Molton <hidden> Date: 2012-07-30 16:57:37
On 30/07/12 17:17, Andrew Lunn wrote:
Hi Ian
Shouldn't there be two interrupt here?
292 orion_ge00_init(eth_data, 293
GE00_PHYS_BASE, IRQ_KIRKWOOD_GE00_SUM, 294
IRQ_KIRKWOOD_GE00_ERR);
No. This is thanks to the horrific mess in the mv643xx driver - its got
two drivers in one source file, and they do horrid things like pass
round base addresses.
Im making an effort to try to split them out into seperate mdio and
MAC/PHY drivers.
This is a start. :)
-Ian
From: Ian Molton <hidden> Date: 2012-07-30 16:58:36
On 30/07/12 16:55, Andrew Lunn wrote:
On Mon, Jul 30, 2012 at 04:15:58PM +0100, Ian Molton wrote:
Hi Ian
I'm trying to keep this in order. Currently i2c is in the wrong
place, but the rest are incremental. It looks like these should be
between wdt & sata.
Will fix.
What about default entries for egiga0 and egiga1 with status
"disabled"? It looks like the only thing which changes between
boards is phy_addr.
I find that even if built in, it fails, not because it forgets its MAC, but
later, it hangs because as yet it does not know how to handle the clk
when it is set up from DT bindings. (Kirkwood doesnt really have this
support yet AFAICT).
Ah, of course. You are missing auxdata in board-dt.c!
Andrew
From: Ian Molton <hidden> Date: 2012-07-31 08:23:41
On 30/07/12 17:46, Josh Coombs wrote:
If I'm reading this correctly, if kirkwood_ge00_init is called without
any data, all that happens is the clock is enabled. If the right data
is passed in, the ethernet interface is initialized, and then the
clock is enabled.
Patch 6 in your series sets all the existing DT board files to call
kirkwood_ge00_init with no data.
Would it make more sense for readability to separate the clock prep
and other interface initialization so DT board files don't appear to
be calling for a full turn up despite now handling that in the DTS
tree? IE instead of converting all the board-XXX.c to call
kirkwood_ge00_init(NULL) have them call a new function,
kirkwood_ge00_clk_prep(NULL) instead, assuming the clock can't be
handled directly via DTS.
This is certainly possible, but I was trying to avoid yet another
function being added to the sources with a (necessarily) short
lifespan - it will only be needed until such time as the clk stuff
is sorted out, which is on my TODO list.
My preference would be to add a comment explaining the
choice in the code.
If its preferred, however, I have no real objection to splitting
out the functionality.
-Ian
From: Ian Molton <hidden> Date: 2012-07-31 11:04:16
On 30/07/12 17:46, Josh Coombs wrote:
If I'm reading this correctly, if kirkwood_ge00_init is called without
any data, all that happens is the clock is enabled. If the right data
is passed in, the ethernet interface is initialized, and then the
clock is enabled.
Patch 6 in your series sets all the existing DT board files to call
kirkwood_ge00_init with no data.
Actually Im thinking of dropping this change altogether - it would
seem that since adding aliases so that the driver can managed
the clocks itself (thanks Andrew), the DT code apparently doesnt
disable the clocks, so the MAC is preserved.
I see no reason to keep the calls now that the driver manages to
hang onto its clocks.
-Ian
b/arch/arm/configs/csb1724_defconfig new file mode 100644 index
0000000..927b269 --- /dev/null +++
b/arch/arm/configs/csb1724_defconfig @@ -0,0 +1,47 @@
+CONFIG_ARCH_KIRKWOOD=y +CONFIG_ARCH_KIRKWOOD_DT=y
+CONFIG_MACH_CSB1724_DT=y
Do you really need this? Its the only kirkwood board with its on
_defconfig file. What makes this board special?
Nothing. defconfigs are just there to make configuring your kernel easy.
The kirkwood defconfig doesnt fit the currently supported hardware
set on the csb1724 very well. Why build stuff you dont need?
The defconfig files serve a number of purposes. One of them is for
build regression testing, and for that reason we try to enable as
many boards as possible with a small number of defconfig files.
It's very rare that we have a board specific defconfig file, and
I would also ask you to just enable the hardware you need in the
kirkwood defconfig. It should not be hard for you to create
your own configuration by starting with that and disabling everything
you don't need.
Arnd
From: Ian Molton <hidden> Date: 2012-07-31 11:39:10
On 31/07/12 12:08, Andrew Lunn wrote:
quoted
I see no reason to keep the calls now that the driver manages to
hang onto its clocks.
Hi Ian
Did you test this with it built as a module? That is the real test....
Rats. Yes, however it managed to survive. The minute I read this was the
minute it died. sods law :(
Guess it'll still be needed for now.
So, the question is about how to implement it - I like the 'probe the OF
device name' idea - I can implement that quickly enough.
Once thats done, I can push out a v2 patch, all the other issues are now
addressed, AFAICS.
-Ian
Hello Ian,
On Monday 30 July 2012 17:32:39 Ian Molton wrote:
On 30/07/12 17:19, Amar Nath wrote:
quoted
Hi Ian,
On Mon, Jul 30, 2012 at 8:45 PM, Ian Molton
[off-list ref]wrote:
quoted
quoted
- pd = pdev->dev.platform_data;
+ if (pdev->dev.of_node) {
+ struct device_node *np = NULL;
+
+ /* when all users of this driver use FDT, we can remove
this */
+ pd = kzalloc(sizeof(*pd), GFP_ATOMIC);
Do you really need an allocation in atomic context here? Is not GFP_KERNEL
sufficient?
There was a second place like this where you used an atomic allocation that
needs fixing.
Can this check for pd be moved in the else part above as well, as for the
pd allocation with kzalloc,
we have already made a check for memory allocation failure?