[PATCH v2 1/6] net: mvneta: Add support for AlleyCat5

[Chris Packham]

On Tue, Sep 20, 2022 at 10:48 PM Pali Rohár <pali at kernel.org> wrote:
>
> On Tuesday 20 September 2022 20:31:48 Chris Packham wrote:
> > diff --git a/drivers/net/mvneta.c b/drivers/net/mvneta.c
> > index d2c42c4396..07919d6d35 100644
> > --- a/drivers/net/mvneta.c
> > +++ b/drivers/net/mvneta.c
> > @@ -91,6 +91,8 @@ DECLARE_GLOBAL_DATA_PTR;
> >  #define MVNETA_WIN_SIZE_MASK                 (0xffff0000)
> >  #define MVNETA_BASE_ADDR_ENABLE                 0x2290
> >  #define      MVNETA_BASE_ADDR_ENABLE_BIT     0x1
> > +#define      MVNETA_AC5_CNM_DDR_TARGET               0x2
> > +#define      MVNETA_AC5_CNM_DDR_ATTR         0xb
> >  #define MVNETA_PORT_ACCESS_PROTECT              0x2294
> >  #define      MVNETA_PORT_ACCESS_PROTECT_WIN0_RW      0x3
> >  #define MVNETA_PORT_CONFIG                      0x2400
> > @@ -282,6 +284,8 @@ struct mvneta_port {
> >       struct gpio_desc phy_reset_gpio;
> >       struct gpio_desc sfp_tx_disable_gpio;
> >  #endif
> > +
> > +     uintptr_t dma_base;     /* base address for DMA address decoding */
> >  };
> >
> >  /* The mvneta_tx_desc and mvneta_rx_desc structures describe the
> > @@ -513,10 +517,19 @@ static void mvneta_rxq_desc_num_update(struct mvneta_port *pp,
> >  static struct mvneta_rx_desc *
> >  mvneta_rxq_next_desc_get(struct mvneta_rx_queue *rxq)
> >  {
> > +     struct mvneta_rx_desc *curr;
> >       int rx_desc = rxq->next_desc_to_proc;
> >
> > +     /* validate RX descriptor */
> > +     curr = rxq->descs + rx_desc;
> > +     if (curr->data_size == 0) {
> > +             /* do it to read real descriptor next time */
> > +             DSB;
> > +             return NULL;
> > +     }
> > +
> >       rxq->next_desc_to_proc = MVNETA_QUEUE_NEXT_DESC(rxq, rx_desc);
> > -     return rxq->descs + rx_desc;
> > +     return curr;
> >  }
>
> Hello! This change looks like some fix, and not related to AC5 support.
> So it should be in separate patch as it affects all mvneta platforms,
> not only AC5.
>

Yep. I'll see if I can get away without this change for the AC5X. So
I'll either move it to a new patch or remove it completely in the next
round.

> > @@ -1508,11 +1544,15 @@ static int mvneta_recv(struct udevice *dev, int flags, uchar **packetp)
> >                */
> >               rx_desc = mvneta_rxq_next_desc_get(rxq);
> >
> > +             if (!rx_desc)
> > +                     return 0;
> > +
> >               rx_status = rx_desc->status;
> >               if (!mvneta_rxq_desc_is_first_last(rx_status) ||
> >                   (rx_status & MVNETA_RXD_ERR_SUMMARY)) {
> >                       mvneta_rx_error(pp, rx_desc);
> > -                     /* leave the descriptor untouched */
> > +                     /* invalidate the descriptor */
> > +                     rx_desc->data_size = 0;
> >                       return -EIO;
> >               }
>
> ditto.
>
> > @@ -1525,13 +1565,15 @@ static int mvneta_recv(struct udevice *dev, int flags, uchar **packetp)
> >                * No cache invalidation needed here, since the rx_buffer's are
> >                * located in a uncached memory region
> >                */
> > -             *packetp = data;
> > +             *packetp = data + pp->dma_base;
> >
> >               /*
> >                * Only mark one descriptor as free
> >                * since only one was processed
> >                */
> >               mvneta_rxq_desc_num_update(pp, rxq, 1, 1);
> > +             /* invalidate the descriptor */
> > +             rx_desc->data_size = 0;
>
> ditto.
>
> >       }
> >
> >       return rx_bytes;
> > @@ -1544,6 +1586,11 @@ static int mvneta_probe(struct udevice *dev)
> >       struct ofnode_phandle_args sfp_args;
> >  #endif
> >       void *bd_space;
> > +     void *blob = (void *)gd->fdt_blob;
> > +     int node = dev_of_offset(dev);
> > +     const int *cell;
> > +     int len;
> > +
> >
> >       /*
> >        * Allocate buffer area for descs and rx_buffers. This is only
> > @@ -1577,9 +1624,21 @@ static int mvneta_probe(struct udevice *dev)
> >       /* Configure MBUS address windows */
> >       if (device_is_compatible(dev, "marvell,armada-3700-neta"))
> >               mvneta_bypass_mbus_windows(pp);
> > +     else if (device_is_compatible(dev, "marvell,armada-ac5-neta"))
> > +             mvneta_conf_ac5_cnm_xbar_windows(pp);
> >       else
> >               mvneta_conf_mbus_windows(pp);
> >
> > +     /* fetch dma ranges property */
> > +     cell = fdt_getprop(blob, node, "dma-ranges", &len);
> > +     if (cell && len >= 4 * sizeof(int)) {
> > +             /* RAZA - TODO do that in loop by address-cells size */
> > +             pp->dma_base = fdt32_to_cpu(cell[1]);
> > +             pp->dma_base = (pp->dma_base << 32) | fdt32_to_cpu(cell[2]);
>
> Probably it should be better to use some function for parsing dma-ranges
> instead of open-coded implementation. It is not really clear why
> cell[0] and cell[3] are ignored there, if it is missing intentionally or
> it is a bug.
>
> And... This should be used only for AC5, right?
>

I think this is about handling 32-bit DMA on a 64-bit system with RAM
that starts above the 4GB boundary. So not strictly AC5 specific but
the AC5 is the only thing that fits those criteria currently.
Certainly the AC5 is the only thing that sets the dma-ranges property
on the Ethernet controller. I did find some APIs for parsing
dma-ranges but that led me down a rabbit hole of DM_DMA which looks
like it (or the SoC) might not be ready for prime-time. I'll see what
I can do to avoid the open coding.

> > +     } else {
> > +             pp->dma_base = 0;
> > +     }
> > +
> >  #if CONFIG_IS_ENABLED(DM_GPIO)
> >       if (!dev_read_phandle_with_args(dev, "sfp", NULL, 0, 0, &sfp_args) &&
> >           ofnode_is_enabled(sfp_args.node))
> > @@ -1620,6 +1679,7 @@ static const struct eth_ops mvneta_ops = {
> >
> >  static const struct udevice_id mvneta_ids[] = {
> >       { .compatible = "marvell,armada-370-neta" },
> > +     { .compatible = "marvell,armada-ac5-neta" },
> >       { .compatible = "marvell,armada-xp-neta" },
> >       { .compatible = "marvell,armada-3700-neta" },
> >       { }
> > --
> > 2.37.3
> >