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@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
> > .../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
Attachment:
signature.asc
Description: PGP signature
