[U-Boot] [PATCH 01/11 v2] drivers/net/vsc9953: Cleanup patch

Codrin Constantin Ciubotariu codrin.ciubotariu at freescale.com
Thu Jun 25 18:35:40 CEST 2015


Hi Joe,

> >
> >         /* alloc eth device */
> >         dev = (struct eth_device *)calloc(1, sizeof(struct eth_device));
> >         if (!dev)
> > -               return 1;
> > +               return -1;
> 
> Is it reasonable to use values from asm/errno.h here and elsewhere in
> the driver? This seems like it should return -ENODEV instead of
> -EPERM.

Yes, I should use values from errno.h . -ENOMEM seems more appropriate to me. What do you think?

> > +       enabled = !!(vsc9953_l2sw.port[port_no].enabled &
> > +                    val & CONFIG_VSC9953_PORT_ENA);
> 
> This is incorrect... Should be:
> 
> +       enabled = vsc9953_l2sw.port[port_no].enabled &&
> +                    (val & CONFIG_VSC9953_PORT_ENA);

Ok.

> > diff --git a/include/vsc9953.h b/include/vsc9953.h
> > index 3d11b87..920402f 100644
> > --- a/include/vsc9953.h
> > +++ b/include/vsc9953.h
> > @@ -33,29 +33,60 @@
> >  #define T1040_SWITCH_GMII_DEV_OFFSET   0x010000
> >  #define VSC9953_PHY_REGS_OFFST         0x0000AC
> >
> > +/* Macros for vsc9953_chip_regs.soft_rst register */
> >  #define CONFIG_VSC9953_SOFT_SWC_RST_ENA        0x00000001
> 
> All of there that are register constants should not have "CONFIG_"
> prepended to them. That should only be for constants that configure
> something, eventually only from Kconfig.  Please add another patch
> before this one that removes that.

Ok, I will add another patch before this one.

> > +/* Macros for vsc9953_sys_pause_cfg.pause_cfg register */
> >  #define CONFIG_VSC9953_PAUSE_CFG       0x001ffffe
> > +
> > +/* Macros for vsc9953_sys_pause_cfg.pause_cfg register */
> > +#define CONFIG_VSC9953_PAUSE_CFG       0x001ffffe

> This adds a duplicate of the define above it.

I will remove one of them.

> >
> > +/* Macros for vsc9953_vcap_core_cfg.vcap_mv_cfg register */
> >  #define        CONFIG_VSC9953_VCAP_MV_CFG      0x0000ffff
> >  #define        CONFIG_VSC9953_VCAP_UPDATE_CTRL 0x01000004
> 
> May as well get rid of the tabs here after the defines.

Ok.

Thank you for your review. I will make v3.

Best regards,
Codrin


More information about the U-Boot mailing list