| Message ID | 1408202315-20006-6-git-send-email-mpa@pengutronix.de |
|---|---|
| State | Superseded, archived |
| Headers | show |
> diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile > index 9cfaab8152be..5a31c2b322ee 100644 > --- a/drivers/net/ethernet/ti/Makefile > +++ b/drivers/net/ethernet/ti/Makefile > @@ -8,5 +8,6 @@ obj-$(CONFIG_TI_DAVINCI_EMAC) += davinci_emac.o > obj-$(CONFIG_TI_DAVINCI_MDIO) += davinci_mdio.o > obj-$(CONFIG_TI_DAVINCI_CPDMA) += davinci_cpdma.o > obj-$(CONFIG_TI_CPSW_PHY_SEL) += cpsw-phy-sel.o > +obj-$(CONFIG_TI_CPSW_CTRL_MACID) += cpsw-ctrl-macid.o > obj-$(CONFIG_TI_CPSW) += ti_cpsw.o > ti_cpsw-y := cpsw_ale.o cpsw.o cpts.o Leftover from your last series?
> + mac_addr[5] = (macid_lo >> 8) & 0xff; > + mac_addr[4] = macid_lo & 0xff; > + mac_addr[3] = (macid_hi >> 24) & 0xff; > + mac_addr[2] = (macid_hi >> 16) & 0xff; > + mac_addr[1] = (macid_hi >> 8) & 0xff; > + mac_addr[0] = macid_hi & 0xff; That looks twisted, but I assume that you tested it is correct.
Hi, On Sat, Aug 16, 2014 at 11:46:48AM -0500, Wolfram Sang wrote: > > > diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile > > index 9cfaab8152be..5a31c2b322ee 100644 > > --- a/drivers/net/ethernet/ti/Makefile > > +++ b/drivers/net/ethernet/ti/Makefile > > @@ -8,5 +8,6 @@ obj-$(CONFIG_TI_DAVINCI_EMAC) += davinci_emac.o > > obj-$(CONFIG_TI_DAVINCI_MDIO) += davinci_mdio.o > > obj-$(CONFIG_TI_DAVINCI_CPDMA) += davinci_cpdma.o > > obj-$(CONFIG_TI_CPSW_PHY_SEL) += cpsw-phy-sel.o > > +obj-$(CONFIG_TI_CPSW_CTRL_MACID) += cpsw-ctrl-macid.o > > obj-$(CONFIG_TI_CPSW) += ti_cpsw.o > > ti_cpsw-y := cpsw_ale.o cpsw.o cpts.o > > Leftover from your last series? Yes thanks, will remove it for the next version. Best regards, Markus
On Sat, Aug 16, 2014 at 11:53:18AM -0500, Wolfram Sang wrote: > > > + mac_addr[5] = (macid_lo >> 8) & 0xff; > > + mac_addr[4] = macid_lo & 0xff; > > + mac_addr[3] = (macid_hi >> 24) & 0xff; > > + mac_addr[2] = (macid_hi >> 16) & 0xff; > > + mac_addr[1] = (macid_hi >> 8) & 0xff; > > + mac_addr[0] = macid_hi & 0xff; > > That looks twisted, but I assume that you tested it is correct. The registers are twisted that way. Best regards, Markus
On Saturday 16 August 2014 08:48 PM, Markus Pargmann wrote: > + mac_addr[5] = (macid_lo >> 8) & 0xff; > + mac_addr[4] = macid_lo & 0xff; > + mac_addr[3] = (macid_hi >> 24) & 0xff; > + mac_addr[2] = (macid_hi >> 16) & 0xff; > + mac_addr[1] = (macid_hi >> 8) & 0xff; > + mac_addr[0] = macid_hi & 0xff; > + This will fail incase of DRA74x and DRA72x platforms, please check for u-boot src for parsing logic as TRM is not out yet. Below is the actual code for DRA7 platforms for MAC address parsing mac_addr[0] = (mac_hi & 0xFF0000) >> 16; mac_addr[1] = (mac_hi & 0xFF00) >> 8; mac_addr[2] = mac_hi & 0xFF; mac_addr[3] = (mac_lo & 0xFF0000) >> 16; mac_addr[4] = (mac_lo & 0xFF00) >> 8; mac_addr[5] = mac_lo & 0xFF; Regards Mugunthan V N -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 18 Aug 2014 23:54:26 +0530 Mugunthan V N <mugunthanvnm@ti.com> wrote: > On Saturday 16 August 2014 08:48 PM, Markus Pargmann wrote: > > + mac_addr[5] = (macid_lo >> 8) & 0xff; > > + mac_addr[4] = macid_lo & 0xff; > > + mac_addr[3] = (macid_hi >> 24) & 0xff; > > + mac_addr[2] = (macid_hi >> 16) & 0xff; > > + mac_addr[1] = (macid_hi >> 8) & 0xff; > > + mac_addr[0] = macid_hi & 0xff; > > + > This will fail incase of DRA74x and DRA72x platforms, please check for > u-boot src for parsing logic as TRM is not out yet. Below is the actual > code for DRA7 platforms for MAC address parsing > > mac_addr[0] = (mac_hi & 0xFF0000) >> 16; > mac_addr[1] = (mac_hi & 0xFF00) >> 8; > mac_addr[2] = mac_hi & 0xFF; > mac_addr[3] = (mac_lo & 0xFF0000) >> 16; > mac_addr[4] = (mac_lo & 0xFF00) >> 8; > mac_addr[5] = mac_lo & 0xFF; > But this fails with my beaglebone white. I tested Markus's patches and it came up with the same ethaddr that U-Boot had. >From U-Boot: ethaddr=d4:94:a1:8b:ec:78 With Markus's changes: eth0 Link encap:Ethernet HWaddr D4:94:A1:8B:EC:78 But when I changed the code to match what you wrote, I got this: eth0 Link encap:Ethernet HWaddr CE:5A:8B:0E:44:45 but it also gave me: cpsw 4a100000.ethernet: Random MACID = ce:5a:8b:0e:44:45 which means it failed the valid mac test. Here's how I implemented your change: #if 1 mac_addr[0] = (macid_hi & 0xFF0000) >> 16; mac_addr[1] = (macid_hi & 0xFF00) >> 8; mac_addr[2] = macid_hi & 0xFF; mac_addr[3] = (macid_lo & 0xFF0000) >> 16; mac_addr[4] = (macid_lo & 0xFF00) >> 8; mac_addr[5] = macid_lo & 0xFF; #else mac_addr[5] = (macid_lo >> 8) & 0xff; mac_addr[4] = macid_lo & 0xff; mac_addr[3] = (macid_hi >> 24) & 0xff; mac_addr[2] = (macid_hi >> 16) & 0xff; mac_addr[1] = (macid_hi >> 8) & 0xff; mac_addr[0] = macid_hi & 0xff; #endif Just to be consistent, I updated the code as this too: mac_addr[0] = (macid_hi >> 16) & 0xFF; mac_addr[1] = (macid_hi >> 8) & 0xFF; mac_addr[2] = macid_hi & 0xFF; mac_addr[3] = (macid_lo >> 16) & 0xFF; mac_addr[4] = (macid_lo >> 8) & 0xFF; mac_addr[5] = macid_lo & 0xFF; With the same affect. Thus, for this patchset, as is: Tested-by: Steven Rostedt <rostedt@goodmis.org> -- Steve -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 19 August 2014 01:11 AM, Steven Rostedt wrote: > On Mon, 18 Aug 2014 23:54:26 +0530 > Mugunthan V N <mugunthanvnm@ti.com> wrote: > >> On Saturday 16 August 2014 08:48 PM, Markus Pargmann wrote: >>> + mac_addr[5] = (macid_lo >> 8) & 0xff; >>> + mac_addr[4] = macid_lo & 0xff; >>> + mac_addr[3] = (macid_hi >> 24) & 0xff; >>> + mac_addr[2] = (macid_hi >> 16) & 0xff; >>> + mac_addr[1] = (macid_hi >> 8) & 0xff; >>> + mac_addr[0] = macid_hi & 0xff; >>> + >> This will fail incase of DRA74x and DRA72x platforms, please check for >> u-boot src for parsing logic as TRM is not out yet. Below is the actual >> code for DRA7 platforms for MAC address parsing >> >> mac_addr[0] = (mac_hi & 0xFF0000) >> 16; >> mac_addr[1] = (mac_hi & 0xFF00) >> 8; >> mac_addr[2] = mac_hi & 0xFF; >> mac_addr[3] = (mac_lo & 0xFF0000) >> 16; >> mac_addr[4] = (mac_lo & 0xFF00) >> 8; >> mac_addr[5] = mac_lo & 0xFF; >> > But this fails with my beaglebone white. > > I tested Markus's patches and it came up with the same ethaddr that > U-Boot had. > > From U-Boot: > > ethaddr=d4:94:a1:8b:ec:78 > > With Markus's changes: > > eth0 Link encap:Ethernet HWaddr D4:94:A1:8B:EC:78 > > But when I changed the code to match what you wrote, I got this: > > eth0 Link encap:Ethernet HWaddr CE:5A:8B:0E:44:45 > > but it also gave me: > > cpsw 4a100000.ethernet: Random MACID = ce:5a:8b:0e:44:45 > > which means it failed the valid mac test. > > Here's how I implemented your change: > > #if 1 > mac_addr[0] = (macid_hi & 0xFF0000) >> 16; > mac_addr[1] = (macid_hi & 0xFF00) >> 8; > mac_addr[2] = macid_hi & 0xFF; > mac_addr[3] = (macid_lo & 0xFF0000) >> 16; > mac_addr[4] = (macid_lo & 0xFF00) >> 8; > mac_addr[5] = macid_lo & 0xFF; > > #else > mac_addr[5] = (macid_lo >> 8) & 0xff; > mac_addr[4] = macid_lo & 0xff; > mac_addr[3] = (macid_hi >> 24) & 0xff; > mac_addr[2] = (macid_hi >> 16) & 0xff; > mac_addr[1] = (macid_hi >> 8) & 0xff; > mac_addr[0] = macid_hi & 0xff; > #endif > > Just to be consistent, I updated the code as this too: > > mac_addr[0] = (macid_hi >> 16) & 0xFF; > mac_addr[1] = (macid_hi >> 8) & 0xFF; > mac_addr[2] = macid_hi & 0xFF; > mac_addr[3] = (macid_lo >> 16) & 0xFF; > mac_addr[4] = (macid_lo >> 8) & 0xFF; > mac_addr[5] = macid_lo & 0xFF; > > With the same affect. > > Thus, for this patchset, as is: > > Tested-by: Steven Rostedt <rostedt@goodmis.org> This will fail for DRA7xx not in AM33xx Regards Mugunthan V N -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 19 Aug 2014 01:28:09 +0530
Mugunthan V N <mugunthanvnm@ti.com> wrote:
> This will fail for DRA7xx not in AM33xx
OK, is there a way to test the difference?
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Mugunthan, On Mon, Aug 18, 2014 at 9:58 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote: >> >> Thus, for this patchset, as is: >> >> Tested-by: Steven Rostedt <rostedt@goodmis.org> > > This will fail for DRA7xx not in AM33xx > cpsw_am33xx_cm_get_macid() checks for of_machine_is_compatible("ti,am33xx") and returns 0 if the machine is not an am33xx. cpsw_probe_dt() only propagates the return value if is not 0 so this patch does not change the semantics for other SoCs besides am33xx. If the driver already fails for DRA7xx that certainly is not this patch's fault. Of course it would be nice to add support for DRA7xx as well but I think that could be a follow-up patch and shouldn't be a blocker to merge this change if is useful for users. > Regards > Mugunthan V N > -- Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Tue, Aug 19, 2014 at 12:50:59AM +0200, Javier Martinez Canillas wrote: > Hello Mugunthan, > > On Mon, Aug 18, 2014 at 9:58 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote: > >> > >> Thus, for this patchset, as is: > >> > >> Tested-by: Steven Rostedt <rostedt@goodmis.org> > > > > This will fail for DRA7xx not in AM33xx > > > > cpsw_am33xx_cm_get_macid() checks for > of_machine_is_compatible("ti,am33xx") and returns 0 if the machine is > not an am33xx. cpsw_probe_dt() only propagates the return value if is > not 0 so this patch does not change the semantics for other SoCs > besides am33xx. Yes, this patch is only about the am33xx. I don't have the DRA7xx hardware so I am not able to test on that hardware. Mugunthan, perhaps you can supply some followup patches for DRA7xx. Best regards, Markus
On Tuesday 19 August 2014 02:20 PM, Markus Pargmann wrote: > Hi, > > On Tue, Aug 19, 2014 at 12:50:59AM +0200, Javier Martinez Canillas wrote: >> Hello Mugunthan, >> >> On Mon, Aug 18, 2014 at 9:58 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote: >>>> Thus, for this patchset, as is: >>>> >>>> Tested-by: Steven Rostedt <rostedt@goodmis.org> >>> This will fail for DRA7xx not in AM33xx >>> >> cpsw_am33xx_cm_get_macid() checks for >> of_machine_is_compatible("ti,am33xx") and returns 0 if the machine is >> not an am33xx. cpsw_probe_dt() only propagates the return value if is >> not 0 so this patch does not change the semantics for other SoCs >> besides am33xx. > Yes, this patch is only about the am33xx. I don't have the DRA7xx > hardware so I am not able to test on that hardware. Mugunthan, perhaps > you can supply some followup patches for DRA7xx. > > I will check on this thursday and update. Regards Mugunthan V N -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt index 107caf174a0e..33fe8462edf4 100644 --- a/Documentation/devicetree/bindings/net/cpsw.txt +++ b/Documentation/devicetree/bindings/net/cpsw.txt @@ -24,6 +24,8 @@ Optional properties: - ti,hwmods : Must be "cpgmac0" - no_bd_ram : Must be 0 or 1 - dual_emac : Specifies Switch to act as Dual EMAC +- syscon : Phandle to the system control device node, which is + the control module device of the am33x Slave Properties: Required properties: @@ -57,6 +59,7 @@ Examples: active_slave = <0>; cpts_clock_mult = <0x80000000>; cpts_clock_shift = <29>; + syscon = <&cm>; cpsw_emac0: slave@0 { phy_id = <&davinci_mdio>, <0>; phy-mode = "rgmii-txid"; @@ -85,6 +88,7 @@ Examples: active_slave = <0>; cpts_clock_mult = <0x80000000>; cpts_clock_shift = <29>; + syscon = <&cm>; cpsw_emac0: slave@0 { phy_id = <&davinci_mdio>, <0>; phy-mode = "rgmii-txid"; diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig index 53150c25a96b..afaf0196ffd2 100644 --- a/drivers/net/ethernet/ti/Kconfig +++ b/drivers/net/ethernet/ti/Kconfig @@ -62,6 +62,8 @@ config TI_CPSW select TI_DAVINCI_CPDMA select TI_DAVINCI_MDIO select TI_CPSW_PHY_SEL + select MFD_SYSCON + select REGMAP ---help--- This driver supports TI's CPSW Ethernet Switch. diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile index 9cfaab8152be..5a31c2b322ee 100644 --- a/drivers/net/ethernet/ti/Makefile +++ b/drivers/net/ethernet/ti/Makefile @@ -8,5 +8,6 @@ obj-$(CONFIG_TI_DAVINCI_EMAC) += davinci_emac.o obj-$(CONFIG_TI_DAVINCI_MDIO) += davinci_mdio.o obj-$(CONFIG_TI_DAVINCI_CPDMA) += davinci_cpdma.o obj-$(CONFIG_TI_CPSW_PHY_SEL) += cpsw-phy-sel.o +obj-$(CONFIG_TI_CPSW_CTRL_MACID) += cpsw-ctrl-macid.o obj-$(CONFIG_TI_CPSW) += ti_cpsw.o ti_cpsw-y := cpsw_ale.o cpsw.o cpts.o diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index b52df53441b0..aa13f68a178c 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -33,6 +33,8 @@ #include <linux/of_net.h> #include <linux/of_device.h> #include <linux/if_vlan.h> +#include <linux/mfd/syscon.h> +#include <linux/regmap.h> #include <linux/pinctrl/consumer.h> @@ -1796,6 +1798,39 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv, slave->port_vlan = data->dual_emac_res_vlan; } +#define AM33XX_CTRL_MAC_LO_REG(id) (0x630 + 0x8 * id) +#define AM33XX_CTRL_MAC_HI_REG(id) (0x630 + 0x8 * id + 0x4) + +static int cpsw_am33xx_cm_get_macid(struct device *dev, int slave, + u8 *mac_addr) +{ + u32 macid_lo; + u32 macid_hi; + struct regmap *syscon; + + if (!of_machine_is_compatible("ti,am33xx")) + return 0; + + syscon = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon"); + if (IS_ERR(syscon)) { + if (PTR_ERR(syscon) == -ENODEV) + return 0; + return PTR_ERR(syscon); + } + + regmap_read(syscon, AM33XX_CTRL_MAC_LO_REG(slave), &macid_lo); + regmap_read(syscon, AM33XX_CTRL_MAC_HI_REG(slave), &macid_hi); + + mac_addr[5] = (macid_lo >> 8) & 0xff; + mac_addr[4] = macid_lo & 0xff; + mac_addr[3] = (macid_hi >> 24) & 0xff; + mac_addr[2] = (macid_hi >> 16) & 0xff; + mac_addr[1] = (macid_hi >> 8) & 0xff; + mac_addr[0] = macid_hi & 0xff; + + return 0; +} + static int cpsw_probe_dt(struct cpsw_platform_data *data, struct platform_device *pdev) { @@ -1908,8 +1943,15 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data, PHY_ID_FMT, mdio->name, phyid); mac_addr = of_get_mac_address(slave_node); - if (mac_addr) - memcpy(slave_data->mac_addr, mac_addr, ETH_ALEN); + if (mac_addr) { + memcpy(slave_data->mac_addr, mac_addr, + ETH_ALEN); + } else { + ret = cpsw_am33xx_cm_get_macid(&pdev->dev, i, + slave_data->mac_addr); + if (ret) + return ret; + } slave_data->phy_if = of_get_phy_mode(slave_node); if (slave_data->phy_if < 0) {
This patch adds a function to get the MACIDs from the am33xx SoC control module registers which hold unique vendor MACIDs. This is only used if of_get_mac_address() fails to get a valid mac address. Signed-off-by: Markus Pargmann <mpa@pengutronix.de> --- Documentation/devicetree/bindings/net/cpsw.txt | 4 +++ drivers/net/ethernet/ti/Kconfig | 2 ++ drivers/net/ethernet/ti/Makefile | 1 + drivers/net/ethernet/ti/cpsw.c | 46 ++++++++++++++++++++++++-- 4 files changed, 51 insertions(+), 2 deletions(-)