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@xxxxxxxxx>
> ---
> 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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
