[PATCH v3 1/3] net: cortina_ni: Add eth support for Cortina Access CAxxxx SoCs

Tom Rini trini at konsulko.com
Tue Sep 29 19:55:42 CEST 2020


On Wed, Sep 23, 2020 at 05:59:12PM -0700, Alex Nemirovsky wrote:

> From: Abbie Chang <abbie.chang at cortina-access.com>
> 
> Add Cortina Access Ethernet device driver for CAxxxx SoCs.
> This driver supports both legacy and DM_ETH network models.
> 
> Signed-off-by: Aaron Tseng <aaron.tseng at cortina-access.com>
> Signed-off-by: Alex Nemirovsky <alex.nemirovsky at cortina-access.com>
> Signed-off-by: Abbie Chang <abbie.chang at cortina-access.com>
> 
> CC: Joe Hershberger <joe.hershberger at ni.com>
> CC: Abbie Chang <abbie.chang at Cortina-Access.com>
> CC: Tom Rini <trini at konsulko.com>
[snip]

Again, please give the whole driver a read and compare it with other
network drivers that've been most recently added.  I question things
like:
> +static u32 *rdwrptr_adv_one(u32 *x, unsigned long base, unsigned long max)
> +{
> +	if (x + 1 >= (u32 *)max)
> +		return (u32 *)base;
> +	else
> +		return (x + 1);
> +}
[snip]
> +static u32 REG_TO_U32(void *reg)
> +{
> +	return *(u32 *)reg;
> +}
[snip]
> +int ca_miiphy_read(const char *devname,
> +		   unsigned char addr,
> +		   unsigned char reg,
> +		   unsigned short *value)
> +{
> +	return ca_mdio_read(addr, reg, value);
> +}
> +
> +int ca_miiphy_write(const char *devname,
> +		    unsigned char addr,
> +		    unsigned char reg,
> +		    unsigned short value)
> +{
> +	return ca_mdio_write(addr, reg, value);
> +}

And lots of other simple looking wrappers.

[snip]
> +static void ca_ni_setup_mac_addr(void)
> +{
> +	unsigned char mac[6];
> +
> +	struct NI_HV_GLB_MAC_ADDR_CFG0_t	mac_addr_cfg0;
> +	struct NI_HV_GLB_MAC_ADDR_CFG1_t	mac_addr_cfg1;
> +	struct NI_HV_PT_PORT_STATIC_CFG_t	port_static_cfg;
> +	struct NI_HV_XRAM_CPUXRAM_CFG_t		cpuxram_cfg;
> +	struct cortina_ni_priv *priv = dev_get_priv(curr_dev);

While checkpatch didn't complain, can you please fix the driver to use
consistent spacing?

I feel like I asked before about Linux upstreaming of these drivers and
the answer was that it was planned for later, is that right?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200929/46927711/attachment.sig>


More information about the U-Boot mailing list