[U-Boot] [PATCH v7 6/9] net: macb: Extend MACB driver for SiFive Unleashed board

Ramon Fried rfried.dev at gmail.com
Tue Jun 25 05:21:17 UTC 2019


On Mon, Jun 24, 2019 at 7:03 AM Anup Patel <Anup.Patel at wdc.com> wrote:
> The SiFive MACB ethernet has a custom TX_CLK_SEL register to select
> different TX clock for 1000mbps vs 10/100mbps.
>
> This patch adds SiFive MACB compatible string and extends the MACB
> ethernet driver to change TX clock using TX_CLK_SEL register for
> SiFive MACB.
>
> Signed-off-by: Anup Patel <anup.patel at wdc.com>
> ---
>   drivers/net/macb.c | 86 +++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 69 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index c072f99d8f..727cb0bc49 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -83,7 +83,9 @@ struct macb_dma_desc {
>
>   struct macb_device {
>          void                    *regs;
> -       unsigned int            dma_burst_length;
> +       void                    *regs1;
This is just littering the macb_device with RISCV soc specifics,
basically this is the SOC's
mapping of clock tree. there's no clock tree support here, but you
should map and use it in the
the clock init function.

> +
> +       const struct macb_config*config;
>
>          unsigned int            rx_tail;
>          unsigned int            tx_head;
> @@ -123,6 +125,9 @@ struct macb_device {
>
>   struct macb_config {
>          unsigned int            dma_burst_length;
> +
> +       int                     (*probe)(struct udevice *dev);
> +       int                     (*set_tx_clk)(struct udevice *dev, ulong rate);
Let's be more generic, let's call this function clk_init and
initialize all the clocks there.
A good approach will be that by default we'll call the current clock
configuration.
In your case you should create a new callback function that calls the
current clock function and also
configures the specific tx clock.
>   };
>
>   #ifndef CONFIG_DM_ETH
> @@ -483,21 +488,32 @@ static int macb_phy_find(struct macb_device *macb, const char *name)
>    * when operation failed.
>    */
>   #ifdef CONFIG_DM_ETH
> +static int macb_sifive_set_tx_clk(struct udevice *dev, ulong rate)
> +{
> +       struct macb_device *macb = dev_get_priv(dev);
> +
> +       if (!macb->regs1)
> +               return -ENODEV;
> +
> +       /*
> +        * SiFive GEMGXL TX clock operation mode:
> +        *
> +        * 0 = GMII mode. Use 125 MHz gemgxlclk from PRCI in TX logic
> +        *     and output clock on GMII output signal GTX_CLK
> +        * 1 = MII mode. Use MII input signal TX_CLK in TX logic
> +        */
> +       writel(rate != 125000000, macb->regs1);
> +       return 0;
> +}
> +
>   int __weak macb_linkspd_cb(struct udevice *dev, unsigned int speed)
>   {
>   #ifdef CONFIG_CLK
> +       struct macb_device *macb = dev_get_priv(dev);
>          struct clk tx_clk;
>          ulong rate;
>          int ret;
>
> -       /*
> -        * "tx_clk" is an optional clock source for MACB.
> -        * Ignore if it does not exist in DT.
> -        */
> -       ret = clk_get_by_name(dev, "tx_clk", &tx_clk);
> -       if (ret)
> -               return 0;
> -
>          switch (speed) {
>          case _10BASET:
>                  rate = 2500000;         /* 2.5 MHz */
> @@ -513,6 +529,17 @@ int __weak macb_linkspd_cb(struct udevice *dev, unsigned int speed)
>                  return 0;
>          }
>
> +       if (macb->config->set_tx_clk)
> +               return macb->config->set_tx_clk(dev, rate);
> +
> +       /*
> +        * "tx_clk" is an optional clock source for MACB.
> +        * Ignore if it does not exist in DT.
> +        */
> +       ret = clk_get_by_name(dev, "tx_clk", &tx_clk);
> +       if (ret)
> +               return 0;
> +
>          if (tx_clk.dev) {
>                  ret = clk_set_rate(&tx_clk, rate);
>                  if (ret)
> @@ -701,12 +728,14 @@ static void gmac_configure_dma(struct macb_device *macb)
>          u32 buffer_size;
>          u32 dmacfg;
>
> +       if (!macb->config->dma_burst_length)
> +               return;
> +
Don't skip it, we configure other things as well, including buffer size,
which I just sent a patch that on GEM it sets it to 2K, greatly improving
performance.

>          buffer_size = 128 / RX_BUFFER_MULTIPLE;
>          dmacfg = gem_readl(macb, DMACFG) & ~GEM_BF(RXBS, -1L);
>          dmacfg |= GEM_BF(RXBS, buffer_size);
>
> -       if (macb->dma_burst_length)
> -               dmacfg = GEM_BFINS(FBLDO, macb->dma_burst_length, dmacfg);
> +       dmacfg = GEM_BFINS(FBLDO, macb->config->dma_burst_length, dmacfg);
>
>          dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
>          dmacfg &= ~GEM_BIT(ENDIA_PKT);
> @@ -1171,13 +1200,25 @@ static const struct macb_config default_gem_config = {
>          .dma_burst_length = 16,
>   };
>
> +static int macb_sifive_probe(struct udevice *dev)
> +{
> +       struct macb_device *macb = dev_get_priv(dev);
> +       fdt_addr_t addr;
> +
> +       addr = dev_read_addr_index(dev, 1);
> +       if (addr == FDT_ADDR_T_NONE)
> +               return -ENODEV;
> +       macb->regs1 = (void __iomem *)addr;
> +
> +       return 0;
> +}
Can we get rid of this and place it in the clock init function instead?
> +
>   static int macb_eth_probe(struct udevice *dev)
>   {
> -       const struct macb_config *macb_config;
>          struct eth_pdata *pdata = dev_get_platdata(dev);
>          struct macb_device *macb = dev_get_priv(dev);
>          const char *phy_mode;
> -       __maybe_unused int ret;
> +       int ret;
>
>          phy_mode = fdt_getprop(gd->fdt_blob, dev_of_offset(dev), "phy-mode",
>                                 NULL);
> @@ -1190,11 +1231,16 @@ static int macb_eth_probe(struct udevice *dev)
>
>          macb->regs = (void *)pdata->iobase;
>
> -       macb_config = (struct macb_config *)dev_get_driver_data(dev);
> -       if (!macb_config)
> -               macb_config = &default_gem_config;
> +       macb->config = (struct macb_config *)dev_get_driver_data(dev);
> +       if (!macb->config)
> +               macb->config = &default_gem_config;
> +
> +       if (macb->config->probe) {
> +               ret = macb->config->probe(dev);
> +               if (ret)
> +                       return ret;
> +       }
>
> -       macb->dma_burst_length = macb_config->dma_burst_length;
>   #ifdef CONFIG_CLK
>          ret = macb_enable_clk(dev);
>          if (ret)
> @@ -1259,6 +1305,11 @@ static const struct macb_config sama5d4_config = {
>          .dma_burst_length = 4,
>   };
>
> +static const struct macb_config sifive_config = {
> +       .probe = macb_sifive_probe,
as stated before, probe is redundant.
> +       .set_tx_clk = macb_sifive_set_tx_clk,
> +};
> +
>   static const struct udevice_id macb_eth_ids[] = {
>          { .compatible = "cdns,macb" },
>          { .compatible = "cdns,at91sam9260-macb" },
> @@ -1266,6 +1317,7 @@ static const struct udevice_id macb_eth_ids[] = {
>          { .compatible = "atmel,sama5d3-gem" },
>          { .compatible = "atmel,sama5d4-gem", .data = (ulong)&sama5d4_config },
>          { .compatible = "cdns,zynq-gem" },
> +       { .compatible = "sifive,fu540-macb", .data = (ulong)&sifive_config },
>          { }
>   };
>
> --
> 2.17.1
>


More information about the U-Boot mailing list