| 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
| ||
|
Date: Thu, 9 Mar 2017 21:00:30 +0100
From: Thierry Reding <thierry.reding@...il.com>
To: Mikko Perttunen <cyndis@...si.fi>
Cc: "David S . Miller" <davem@...emloft.net>,
Giuseppe Cavallaro <peppe.cavallaro@...com>,
Alexandre Torgue <alexandre.torgue@...com>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Joao Pinto <Joao.Pinto@...opsys.com>,
Alexandre Courbot <gnurou@...il.com>,
Jon Hunter <jonathanh@...dia.com>, netdev@...r.kernel.org,
linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 7/7] net: stmmac: dwc-qos: Add Tegra186 support
On Mon, Feb 27, 2017 at 01:46:01PM +0200, Mikko Perttunen wrote:
> On 23.02.2017 19:24, Thierry Reding wrote:
> > From: Thierry Reding <treding@...dia.com>
> >
> > The NVIDIA Tegra186 SoC contains an instance of the Synopsys DWC
> > ethernet QOS IP core. The binding that it uses is slightly different
> > from existing ones because of the integration (clocks, resets, ...).
> >
> > Signed-off-by: Thierry Reding <treding@...dia.com>
> > ---
> > .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c | 252 +++++++++++++++++++++
> > 1 file changed, 252 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> > index 5071d3c15adc..54dfbdc48f6d 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> > @@ -14,6 +14,7 @@
> > #include <linux/clk.h>
> > #include <linux/clk-provider.h>
> > #include <linux/device.h>
> > +#include <linux/gpio/consumer.h>
> > #include <linux/ethtool.h>
> > #include <linux/io.h>
> > #include <linux/ioport.h>
> > @@ -22,10 +23,24 @@
> > #include <linux/of_net.h>
> > #include <linux/mfd/syscon.h>
> > #include <linux/platform_device.h>
> > +#include <linux/reset.h>
> > #include <linux/stmmac.h>
> >
> > #include "stmmac_platform.h"
> >
> > +struct tegra_eqos {
> > + struct device *dev;
> > + void __iomem *regs;
> > +
> > + struct reset_control *rst;
> > + struct clk *clk_master;
> > + struct clk *clk_slave;
> > + struct clk *clk_tx;
> > + struct clk *clk_rx;
> > +
> > + struct gpio_desc *reset;
> > +};
> > +
> > static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
> > struct plat_stmmacenet_data *plat_dat)
> > {
> > @@ -148,6 +163,237 @@ static int dwc_qos_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > +#define SDMEMCOMPPADCTRL 0x8800
> > +#define SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD BIT(31)
> > +
> > +#define AUTO_CAL_CONFIG 0x8804
> > +#define AUTO_CAL_CONFIG_START BIT(31)
> > +#define AUTO_CAL_CONFIG_ENABLE BIT(29)
> > +
> > +#define AUTO_CAL_STATUS 0x880c
> > +#define AUTO_CAL_STATUS_ACTIVE BIT(31)
> > +
> > +static void tegra_eqos_fix_speed(void *priv, unsigned int speed)
> > +{
> > + struct tegra_eqos *eqos = priv;
> > + unsigned long rate = 125000000;
> > + bool needs_calibration = false;
> > + unsigned int i;
> > + u32 value;
> > +
> > + switch (speed) {
> > + case SPEED_1000:
> > + needs_calibration = true;
> > + rate = 125000000;
> > + break;
> > +
> > + case SPEED_100:
> > + needs_calibration = true;
> > + rate = 25000000;
> > + break;
> > +
> > + case SPEED_10:
> > + rate = 2500000;
> > + break;
> > +
> > + default:
> > + dev_err(eqos->dev, "invalid speed %u\n", speed);
> > + break;
> > + }
> > +
> > + if (needs_calibration) {
> > + /* calibrate */
> > + value = readl(eqos->regs + SDMEMCOMPPADCTRL);
> > + value |= SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD;
> > + writel(value, eqos->regs + SDMEMCOMPPADCTRL);
> > +
> > + udelay(1);
> > +
> > + value = readl(eqos->regs + AUTO_CAL_CONFIG);
> > + value |= AUTO_CAL_CONFIG_START | AUTO_CAL_CONFIG_ENABLE;
> > + writel(value, eqos->regs + AUTO_CAL_CONFIG);
> > +
> > + for (i = 0; i <= 10; i++) {
> > + value = readl(eqos->regs + AUTO_CAL_STATUS);
> > + if (value & AUTO_CAL_STATUS_ACTIVE)
> > + break;
> > +
> > + udelay(1);
> > + }
> > +
> > + if ((value & AUTO_CAL_STATUS_ACTIVE) == 0) {
> > + dev_err(eqos->dev, "calibration did not start\n");
> > + goto failed;
> > + }
> > +
> > + for (i = 0; i <= 10; i++) {
> > + value = readl(eqos->regs + AUTO_CAL_STATUS);
> > + if ((value & AUTO_CAL_STATUS_ACTIVE) == 0)
> > + break;
> > +
> > + udelay(20);
> > + }
> > +
> > + if (value & AUTO_CAL_STATUS_ACTIVE) {
> > + dev_err(eqos->dev, "calibration didn't finish\n");
> > + goto failed;
> > + }
>
> Could use readl_poll_timeout/readl_poll_timeout_atomic for these loops
> instead.
I had considered that, but the code ends up looking a lot uglier with
that. But since you and Joao both prefer it, I'll switch over to the I/O
poll helpers anyway.
>
> > +
> > + failed:
> > + value = readl(eqos->regs + SDMEMCOMPPADCTRL);
> > + value &= ~SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD;
> > + writel(value, eqos->regs + SDMEMCOMPPADCTRL);
> > + } else {
> > + value = readl(eqos->regs + AUTO_CAL_CONFIG);
> > + value &= ~AUTO_CAL_CONFIG_ENABLE;
> > + writel(value, eqos->regs + AUTO_CAL_CONFIG);
> > + }
> > +
> > + clk_set_rate(eqos->clk_tx, rate);
>
> Could check error code here, and for other clock ops too.
Done.
> > +}
> > +
> > +static int tegra_eqos_init(struct platform_device *pdev, void *priv)
> > +{
> > + struct tegra_eqos *eqos = priv;
> > + unsigned long rate;
> > + u32 value;
> > +
> > + rate = clk_get_rate(eqos->clk_slave);
> > +
> > + value = readl(eqos->regs + 0xdc);
>
> No point in reading the value when it is fully overwritten.
Good catch.
> > + value = (rate / 1000000) - 1;
> > + writel(value, eqos->regs + 0xdc);
>
> Please add a define for 0xdc.
Right, I must've overlooked that while cleaning up the patch. I've added
a GMAC_1US_TIC_COUNTER define for it.
> > @@ -245,6 +496,7 @@ static int dwc_eth_dwmac_remove(struct platform_device *pdev)
> >
> > static const struct of_device_id dwc_eth_dwmac_match[] = {
> > { .compatible = "snps,dwc-qos-ethernet-4.10", .data = &dwc_qos_data },
> > + { .compatible = "nvidia,tegra186-eqos", .data = &tegra_eqos_data},
>
> Missing space before '}'.
Good catch! Fixed.
Thanks,
Thierry
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists