> On Sun, Jun 23, 2024 at 06:19:57PM +0200, Lorenzo Bianconi wrote: > > Add airoha_eth driver in order to introduce ethernet support for > > Airoha EN7581 SoC available on EN7581 development board (en7581-evb). > > en7581-evb networking architecture is composed by airoha_eth as mac > > controller (cpu port) and a mt7530 dsa based switch. > > EN7581 mac controller is mainly composed by Frame Engine (FE) and > > QoS-DMA (QDMA) modules. FE is used for traffic offloading (just basic > > functionalities are supported now) while QDMA is used for DMA operation > > and QOS functionalities between mac layer and the dsa switch (hw QoS is > > not available yet and it will be added in the future). > > Currently only hw lan features are available, hw wan will be added with > > subsequent patches. > > > > Tested-by: Benjamin Larsson <benjamin.larsson@xxxxxxxxxx> > > Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx> > > Hi Lorenzo, > > Some minor nits from my side. Hi Simon, thx for the review. > > ... > > > diff --git a/drivers/net/ethernet/mediatek/airoha_eth.c b/drivers/net/ethernet/mediatek/airoha_eth.c > > ... > > > +#define airoha_fe_rr(eth, offset) airoha_rr((eth)->fe_regs, (offset)) > > +#define airoha_fe_wr(eth, offset, val) airoha_wr((eth)->fe_regs, (offset), (val)) > > +#define airoha_fe_rmw(eth, offset, mask, val) airoha_rmw((eth)->fe_regs, (offset), (mask), (val)) > > +#define airoha_fe_set(eth, offset, val) airoha_rmw((eth)->fe_regs, (offset), 0, (val)) > > +#define airoha_fe_clear(eth, offset, val) airoha_rmw((eth)->fe_regs, (offset), (val), 0) > > + > > +#define airoha_qdma_rr(eth, offset) airoha_rr((eth)->qdma_regs, (offset)) > > +#define airoha_qdma_wr(eth, offset, val) airoha_wr((eth)->qdma_regs, (offset), (val)) > > +#define airoha_qdma_rmw(eth, offset, mask, val) airoha_rmw((eth)->qdma_regs, (offset), (mask), (val)) > > +#define airoha_qdma_set(eth, offset, val) airoha_rmw((eth)->qdma_regs, (offset), 0, (val)) > > +#define airoha_qdma_clear(eth, offset, val) airoha_rmw((eth)->qdma_regs, (offset), (val), 0) > > nit: Please consider line-wrapping the above so lines are 80 columns wide > or less, which is still preferred in Networking code. > > Flagged by checkpatch.pl --max-line-length=80 ack, I will fix it in v4. > > ... > > > +static netdev_tx_t airoha_dev_xmit(struct sk_buff *skb, > > + struct net_device *dev) > > +{ > > + struct skb_shared_info *sinfo = skb_shinfo(skb); > > + struct airoha_eth *eth = netdev_priv(dev); > > + int i, qid = skb_get_queue_mapping(skb); > > + u32 nr_frags, msg0 = 0, msg1; > > + u32 len = skb_headlen(skb); > > + struct netdev_queue *txq; > > + struct airoha_queue *q; > > + void *data = skb->data; > > + u16 index; > > + > > + if (skb->ip_summed == CHECKSUM_PARTIAL) > > + msg0 |= FIELD_PREP(QDMA_ETH_TXMSG_TCO_MASK, 1) | > > + FIELD_PREP(QDMA_ETH_TXMSG_UCO_MASK, 1) | > > + FIELD_PREP(QDMA_ETH_TXMSG_ICO_MASK, 1); > > + > > + /* TSO: fill MSS info in tcp checksum field */ > > + if (skb_is_gso(skb)) { > > + if (skb_cow_head(skb, 0)) > > + goto error; > > + > > + if (sinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)) { > > + tcp_hdr(skb)->check = cpu_to_be16(sinfo->gso_size); > > Probably we could do better with an appropriate helper - perhaps > there is one I couldn't find one - but I think you need a cast here > to keep Sparse happy. > > Something like this (completely untested!): > > tcp_hdr(skb)->check = > (__force __sum16)cpu_to_be16(sinfo->gso_size); > > ... ack, I will fix it in v4. > > > +static int airoha_probe(struct platform_device *pdev) > > +{ > > + struct device_node *np = pdev->dev.of_node; > > + struct net_device *dev; > > + struct airoha_eth *eth; > > + int err; > > + > > + dev = devm_alloc_etherdev_mqs(&pdev->dev, sizeof(*eth), > > + AIROHA_NUM_TX_RING, AIROHA_NUM_RX_RING); > > + if (!dev) { > > + dev_err(&pdev->dev, "alloc_etherdev failed\n"); > > + return -ENOMEM; > > + } > > + > > + eth = netdev_priv(dev); > > + eth->net_dev = dev; > > + > > + err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > + if (err) { > > + dev_err(&pdev->dev, "failed configuring DMA mask\n"); > > + return err; > > + } > > + > > + eth->fe_regs = devm_platform_ioremap_resource_byname(pdev, "fe"); > > + if (IS_ERR(eth->fe_regs)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(eth->fe_regs), > > + "failed to iomap fe regs\n"); > > + > > + eth->qdma_regs = devm_platform_ioremap_resource_byname(pdev, "qdma0"); > > + if (IS_ERR(eth->qdma_regs)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(eth->qdma_regs), > > + "failed to iomap qdma regs\n"); > > + > > + eth->rsts[0].id = "fe"; > > + eth->rsts[1].id = "pdma"; > > + eth->rsts[2].id = "qdma"; > > + err = devm_reset_control_bulk_get_exclusive(&pdev->dev, > > + ARRAY_SIZE(eth->rsts), > > + eth->rsts); > > + if (err) { > > + dev_err(&pdev->dev, "failed to get bulk reset lines\n"); > > + return err; > > + } > > + > > + eth->xsi_rsts[0].id = "xsi-mac"; > > + eth->xsi_rsts[1].id = "hsi0-mac"; > > + eth->xsi_rsts[2].id = "hsi1-mac"; > > + eth->xsi_rsts[3].id = "hsi-mac"; > > + eth->xsi_rsts[4].id = "xfp-mac"; > > + err = devm_reset_control_bulk_get_exclusive(&pdev->dev, > > + ARRAY_SIZE(eth->xsi_rsts), > > + eth->xsi_rsts); > > + if (err) { > > + dev_err(&pdev->dev, "failed to get bulk xsi reset lines\n"); > > + return err; > > + } > > + > > + spin_lock_init(ð->irq_lock); > > + eth->irq = platform_get_irq(pdev, 0); > > + if (eth->irq < 0) { > > + dev_err(&pdev->dev, "failed reading irq line\n"); > > Coccinelle says: > > .../airoha_eth.c:1698:2-9: line 1698 is redundant because platform_get_irq() already prints an error > > ... ack, I will fix it in v4. > > > +const struct of_device_id of_airoha_match[] = { > > + { .compatible = "airoha,en7581-eth" }, > > + { /* sentinel */ } > > +}; > > of_airoha_match appears to only be used in this file. > If so, it should be static. > > Flagged by Sparse. ack, I will fix it in v4. > > > + > > +static struct platform_driver airoha_driver = { > > + .probe = airoha_probe, > > + .remove_new = airoha_remove, > > + .driver = { > > + .name = KBUILD_MODNAME, > > + .of_match_table = of_airoha_match, > > + }, > > +}; > > +module_platform_driver(airoha_driver); > > + > > +MODULE_LICENSE("GPL"); > > +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo@xxxxxxxxxx>"); > > +MODULE_DESCRIPTION("Ethernet driver for Airoha SoC"); > > > diff --git a/drivers/net/ethernet/mediatek/airoha_eth.h b/drivers/net/ethernet/mediatek/airoha_eth.h > > new file mode 100644 > > index 000000000000..f7b984be4d60 > > --- /dev/null > > +++ b/drivers/net/ethernet/mediatek/airoha_eth.h > > @@ -0,0 +1,793 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > The correct SPDX header comment style for .h (but not .c) files is /* ... */ > > https://docs.kernel.org/6.9/process/license-rules.html#license-identifier-syntax > > Flagged by checkpatch ack, I will fix it in v4. Regards, Lorenzo > > ...
Attachment:
signature.asc
Description: PGP signature