On Sat, Jun 29, 2024 at 05:01:38PM +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 feedback from my side.
> +static void airoha_qdma_set_irqmask(struct airoha_eth *eth, int index,
> + u32 clear, u32 set)
> +{
> + unsigned long flags;
> +
> + if (WARN_ON_ONCE(index >= ARRAY_SIZE(eth->irqmask)))
> + return;
> +
> + spin_lock_irqsave(ð->irq_lock, flags);
> +
> + eth->irqmask[index] &= ~clear;
> + eth->irqmask[index] |= set;
> + airoha_qdma_wr(eth, REG_INT_ENABLE(index), eth->irqmask[index]);
> + /* Read irq_enable register in order to guarantee the update above
> + * completes in the spinlock critical section.
> + */
> + airoha_rr(eth, REG_INT_ENABLE(index));
airoha_rr() expects an __iomem pointer as it's first argument,
but the type of eth is struct airoha_eth *eth.
Should this be using airoha_qdma_rr() instead?
Flagged by Sparse.
> +
> + spin_unlock_irqrestore(ð->irq_lock, flags);
> +}
...
> +static void airoha_ethtool_get_strings(struct net_device *dev, u32 sset,
> + u8 *data)
> +{
> + int i;
> +
> + if (sset != ETH_SS_STATS)
> + return;
> +
> + for (i = 0; i < ARRAY_SIZE(airoha_ethtool_stats_name); i++) {
> + memcpy(data + i * ETH_GSTRING_LEN,
> + airoha_ethtool_stats_name[i], ETH_GSTRING_LEN);
> + }
> +
> + data += ETH_GSTRING_LEN * ARRAY_SIZE(airoha_ethtool_stats_name);
> + page_pool_ethtool_stats_get_strings(data);
> +}
W=1 allmodconfig builds on x86_64 with gcc-13 complain about the use
of memcpy above because the source is (often?) less than ETH_GSTRING_LEN
bytes long.
I think the preferred solution is to use ethtool_puts(),
something like this (compile tested only!):
@@ -2291,12 +2291,9 @@ static void airoha_ethtool_get_strings(struct net_device *dev, u32 sset,
if (sset != ETH_SS_STATS)
return;
- for (i = 0; i < ARRAY_SIZE(airoha_ethtool_stats_name); i++) {
- memcpy(data + i * ETH_GSTRING_LEN,
- airoha_ethtool_stats_name[i], ETH_GSTRING_LEN);
- }
+ for (i = 0; i < ARRAY_SIZE(airoha_ethtool_stats_name); i++)
+ ethtool_puts(&data, airoha_ethtool_stats_name[i]);
- data += ETH_GSTRING_LEN * ARRAY_SIZE(airoha_ethtool_stats_name);
page_pool_ethtool_stats_get_strings(data);
}
...
> +static int airoha_alloc_gdm_port(struct airoha_eth *eth, struct device_node *np)
> +{
> + const __be32 *id_ptr = of_get_property(np, "reg", NULL);
> + struct net_device *dev;
> + struct airoha_gdm_port *port;
nit: reverse xmas tree
> + int err, index;
> + u32 id;
...
--
pw-bot: changes-requested
