netdev - Re: [PATCH v2 4/5] net: ethernet: cpsw: add support for hardware interface mode config

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 23 Aug 2013 22:08:10 +0530
From:	Mugunthan V N <mugunthanvnm@...com>
To:	Daniel Mack <zonque@...il.com>
CC:	<netdev@...r.kernel.org>, <nsekhar@...com>,
	<sergei.shtylyov@...entembedded.com>, <davem@...emloft.net>,
	<ujhelyi.m@...il.com>, <vaibhav.bedia@...com>, <d-gerlach@...com>,
	<linux-arm-kernel@...ts.infradead.org>,
	<linux-omap@...r.kernel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH v2 4/5] net: ethernet: cpsw: add support for hardware
 interface mode config

On Friday 23 August 2013 02:13 PM, Daniel Mack wrote:
> The cpsw currently lacks code to properly set up the hardware interface
> mode on AM33xx. Other platforms might be equally affected.
>
> Usually, the bootloader will configure the control module register, so
> probably that's why such support wasn't needed in the past. In suspend
> mode though, this register is modified, and so it needs reprogramming
> after resume.
>
> This patch adds code that makes use of the previously added and optional
> support for passing the control mode register, and configures the
> correct register bits when the slave is opened.
>
> The AM33xx also has a bit for each slave to configure the RMII reference
> clock direction. Setting it is now supported by a per-slave DT property.
>
> This code path introducted by this patch is currently exclusive for
> am33xx.
>
> Signed-off-by: Daniel Mack <zonque@...il.com>
> ---
>  Documentation/devicetree/bindings/net/cpsw.txt |  2 +
>  drivers/net/ethernet/ti/cpsw.c                 | 61 ++++++++++++++++++++++++++
>  drivers/net/ethernet/ti/cpsw.h                 |  1 +
>  3 files changed, 64 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> index b717458..0895a51 100644
> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -34,6 +34,8 @@ Required properties:
>  - phy_id		: Specifies slave phy id
>  - phy-mode		: The interface between the SoC and the PHY (a string
>  			  that of_get_phy_mode() can understand)
> +- ti,rmii-clock-ext	: If present, the driver will configure the RMII
> +			  interface to external clock usage
>  - mac-address		: Specifies slave MAC address
>  
>  Optional properties:
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 2e19de0..6e36f49 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -138,6 +138,13 @@ do {								\
>  #define CPSW_CMINTMAX_INTVL	(1000 / CPSW_CMINTMIN_CNT)
>  #define CPSW_CMINTMIN_INTVL	((1000 / CPSW_CMINTMAX_CNT) + 1)
>  
> +#define AM33XX_GMII_SEL_MODE_MII	(0)
> +#define AM33XX_GMII_SEL_MODE_RMII	(1)
> +#define AM33XX_GMII_SEL_MODE_RGMII	(2)
> +
> +#define AM33XX_GMII_SEL_RMII2_IO_CLK_EN	BIT(7)
> +#define AM33XX_GMII_SEL_RMII1_IO_CLK_EN	BIT(6)

These defined can go into cpsw.h so that we can maintain platform
defines in one file.

> +
>  #define cpsw_enable_irq(priv)	\
>  	do {			\
>  		u32 i;		\
> @@ -980,6 +987,56 @@ static inline void cpsw_add_dual_emac_def_ale_entries(
>  		priv->host_port, ALE_VLAN, slave->port_vlan);
>  }
>  
> +static void cpsw_set_phy_interface_mode(struct cpsw_slave *slave,
> +					struct cpsw_priv *priv)
> +{
> +	u32 reg, mask, mode = 0;
> +

Please define each variable in separate line to make it clean.
Please check for IS_ERR(gmii_sel_reg) and return if gmii register is not
specified in DT then the below code will crash when trying to access
gmii_sel_reg.

> +	switch (priv->data.hw_type) {
> +	case CPSW_TYPE_AM33XX:
> +		if (!priv->gmii_sel_reg)
> +			break;
> +
> +		reg = readl(priv->gmii_sel_reg);
> +
> +		if (slave->phy) {
> +			switch (slave->phy->interface) {
> +			case PHY_INTERFACE_MODE_MII:
> +			default:
> +				mode = AM33XX_GMII_SEL_MODE_MII;
> +				break;
> +			case PHY_INTERFACE_MODE_RMII:
> +				mode = AM33XX_GMII_SEL_MODE_RMII;
> +				break;
> +			case PHY_INTERFACE_MODE_RGMII:

You need to take care of other RGMII modes as well, for your info
AM335xevm phy mode is "rgmii-txid"

Regards
Mugunthan V N
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html