Re: [PATCH] cpsw: Add support to read cpu MAC address

2013-01-14 Thread Grant Likely 
On Fri, 11 Jan 2013 16:15:02 +0100, Michal Bachraty 
 wrote:
> Signed-off-by: Michal Bachraty 
> ---
>  Documentation/devicetree/bindings/net/cpsw.txt |   10 +-
>  arch/arm/boot/dts/am33xx.dtsi  |5 +-
>  drivers/net/ethernet/ti/cpsw.c |  121 
> +---
>  include/linux/platform_data/cpsw.h |8 ++
>  4 files changed, 128 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt 
> b/Documentation/devicetree/bindings/net/cpsw.txt
> index dcaabe9..432122c 100644
> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -4,7 +4,7 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
>  Required properties:
>  - compatible : Should be "ti,cpsw"
>  - reg: physical base address and size of the cpsw
> -   registers map
> +   registers map and mac-address cpu config registers
>  - interrupts : property with a value describing the interrupt
> number
>  - interrupt-parent   : The parent interrupt controller
> @@ -25,17 +25,23 @@ Required properties:
>  - slave_reg_ofs  : Specifies slave register offset
>  - sliver_reg_ofs : Specifies slave sliver register offset
>  - phy_id : Specifies slave phy id
> -- mac-address: Specifies slave MAC address
>  
>  Optional properties:
>  - ti,hwmods  : Must be "cpgmac0"
>  - no_bd_ram  : Must be 0 or 1
> +- mac-address-source : Specifies source of MAC address 
> ("user-defined-mac",
> +   "cpu-id0-mac", "cpu-id01-mac", "random-mac"). If not
> +   specified, "cpu-id0-mac" is selected

Drop the '-mac' suffix on the values. The property is already named
"mac-address-source", so I think it is already unambiguious from the
context.  :-)

Otherwise the patch looks good to me, but I haven't gone over the code
in fine detail.

Acked-by: Grant Likely 

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH] cpsw: Add support to read cpu MAC address

2013-01-11 Thread Russell King - ARM Linux 
On Fri, Jan 11, 2013 at 04:15:02PM +0100, Michal Bachraty wrote:
> + if (!request_mem_region(priv->conf_res->start,
> + resource_size(priv->conf_res), ndev->name)) {
> + dev_err(priv->dev, "failed request i/o region\n");
> + ret = -ENXIO;
> + goto clean_clk_ret;
> + }
> +
> + regs = ioremap(priv->conf_res->start,
> + resource_size(priv->conf_res));
> + if (!regs) {
> + dev_err(priv->dev, "unable to map i/o region\n");
> + goto clean_configuration_iores_ret;
> + }

In this day and age where error paths don't get any testing, and are
frequently buggy, where we have alternative APIs which make those paths
more reliable, I think we should do everything to make use of that.

And, to prove the point, your error paths are buggy.  Yes, you release
the mem region correctly (well done for picking the right interface for
that!) but the ioremap() is never cleaned up.

So, any chance of converting the above to devm_request_and_ioremap() ?

Thanks.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH] cpsw: Add support to read cpu MAC address

2013-01-11 Thread Michal Bachraty 
Signed-off-by: Michal Bachraty 
---
 Documentation/devicetree/bindings/net/cpsw.txt |   10 +-
 arch/arm/boot/dts/am33xx.dtsi  |5 +-
 drivers/net/ethernet/ti/cpsw.c |  121 +---
 include/linux/platform_data/cpsw.h |8 ++
 4 files changed, 128 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt 
b/Documentation/devicetree/bindings/net/cpsw.txt
index dcaabe9..432122c 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -4,7 +4,7 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
 Required properties:
 - compatible   : Should be "ti,cpsw"
 - reg  : physical base address and size of the cpsw
- registers map
+ registers map and mac-address cpu config registers
 - interrupts   : property with a value describing the interrupt
  number
 - interrupt-parent : The parent interrupt controller
@@ -25,17 +25,23 @@ Required properties:
 - slave_reg_ofs: Specifies slave register offset
 - sliver_reg_ofs   : Specifies slave sliver register offset
 - phy_id   : Specifies slave phy id
-- mac-address  : Specifies slave MAC address
 
 Optional properties:
 - ti,hwmods: Must be "cpgmac0"
 - no_bd_ram: Must be 0 or 1
+- mac-address-source   : Specifies source of MAC address ("user-defined-mac",
+ "cpu-id0-mac", "cpu-id01-mac", "random-mac"). If not
+ specified, "cpu-id0-mac" is selected
+- mac-address  : Specifies slave MAC address for "user-defined-mac"
+ property value. When MAC address is not correct,
+ "cpu-id01-mac" is selected
 
 Note: "ti,hwmods" field is used to fetch the base address and irq
 resources from TI, omap hwmod data base during device registration.
 Future plan is to migrate hwmod data base contents into device tree
 blob so that, all the required data will be used from device tree dts
 file.
+As default, MAC address is set from CPU (MAC_ID0 register)
 
 Examples:
 
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 47fb059..f4845a3 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -307,7 +307,8 @@
slaves = <1>;
reg = <0x4a10 0x800
0x4a101200 0x100
-   0x4a101000 0x100>;
+   0x4a101000 0x100
+   0x44e10630 0x0C>;
#address-cells = <1>;
#size-cells = <1>;
interrupt-parent = <&intc>;
@@ -317,7 +318,7 @@
cpsw_emac0: slave@0 {
slave_reg_ofs = <0x208>;
sliver_reg_ofs = <0xd80>;
-   mac-address = [ 00 00 00 00 00 00 ];
+   mac-address-source = "cpu-id0-mac";
};
 
davinci_mdio: mdio@4a101000 {
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index fb1a692..b777116 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -184,6 +184,13 @@ struct cpsw_sliver_regs {
u32 rx_pri_map;
 };
 
+struct config_regs {
+   u32 mac_id0_lo;
+   u32 mac_id0_hi;
+   u32 mac_id1_lo;
+   u32 mac_id1_hi;
+};
+
 struct cpsw_slave {
struct cpsw_slave_regs __iomem  *regs;
struct cpsw_sliver_regs __iomem *sliver;
@@ -199,12 +206,14 @@ struct cpsw_priv {
struct net_device   *ndev;
struct resource *cpsw_res;
struct resource *cpsw_ss_res;
+   struct resource *conf_res;
struct napi_struct  napi;
struct device   *dev;
struct cpsw_platform_data   data;
struct cpsw_regs __iomem*regs;
struct cpsw_ss_regs __iomem *ss_regs;
struct cpsw_host_regs __iomem   *host_port_regs;
+   struct config_regs __iomem  *conf_regs;
u32 msg_enable;
struct net_device_stats stats;
int rx_packet_max;
@@ -363,6 +372,20 @@ static void cpsw_set_slave_mac(struct cpsw_slave *slave,
__raw_writel(mac_lo(priv->mac_addr), &slave->regs->sa_lo);
 }
 
+static void cpsw_get_cpu_id0_mac(struct cpsw_priv *priv)
+{
+   *((u32 *)priv->mac_addr) = __raw_readl(&priv->conf_regs->mac_id0_hi);
+   *((u16 *)(priv->mac_addr + 4)) = __raw_readw(
+   &priv->conf_regs->mac_id0_lo);
+}
+
+static void cpsw_get_cpu_id1_mac(stru