netdev - Re: [PATCH 1/3] net: stmmac: change GMAC control register for SGMII

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] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 26 Nov 2012 11:31:24 +0100
From:	Giuseppe CAVALLARO <peppe.cavallaro@...com>
To:	Byungho An <bh74.an@...sung.com>
Cc:	davem@...emloft.net, jeffrey.t.kirsher@...el.com,
	netdev@...r.kernel.org, kgene.kim@...sung.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] net: stmmac: change GMAC control register for SGMII

On 11/23/2012 10:04 AM, Byungho An wrote:
>
> This patch changes GMAC control register (TC(Transmit
> Configuration) and PS(Port Selection) bit for SGMII.
> In case of SGMII, TC bit is '1' and PS bit is 0.

IMO this new support that should be released for net-next and further 
effort is actually needed.

The availability of the PCS registers is given by looking at the HW 
feature register. In fact, these are optional registers.
I don't want to break the compatibility with old chips.

I do not see why we have to use Kconfig macro to select ANE etc
(as you do in your patches).
The driver could directly manage the phy device by itself if possible 
and the stmmac_init_phy should be reworked.

There are several things that need to be implemented. For example:

The ISR (e.g. priv->hw->mac->host_irq_status) should be able to manage 
these new interrupts.
The code has to be able to maintain the user interface.
For example if you want to enable ANE or manage Advertisement caps.

> Signed-off-by: Byungho An <bh74.an@...sung.com>
> ---

[snip]

> +	if (priv->phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> +		value = readl(priv->ioaddr);
> +		/* GMAC_CONTROL_TC : transmit config in RGMII/SGMII */
> +		value |= 0x1000000;
> +		/* GMAC_CONTROL_PS : Port Selection for GMII */
> +		value &= ~(0x8000);
> +		writel(value, priv->ioaddr);
> +	}
> +

This parts of code have to be moved in
drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c

Pls, do not use value |= 0x1000000 but provide the appropriate defines.

>   	/* Request the IRQ lines */
>   	ret = request_irq(dev->irq, stmmac_interrupt,
>   			 IRQF_SHARED, dev->name, dev);
>
--
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