lkml.org 
[lkml]   [2016]   [Jul]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [RFC PATCH v2 2/4] net: ethernet: xilinx: Add gmii2rgmii converter support
Date
Hi Nicolas,

Thanks for the review...

> > diff --git a/include/linux/xilinx_gmii2rgmii.h
> > b/include/linux/xilinx_gmii2rgmii.h
> > new file mode 100644
> > index 0000000..b328ee7
> > --- /dev/null
> > +++ b/include/linux/xilinx_gmii2rgmii.h
> > @@ -0,0 +1,24 @@
>
>
> Here, header of the file seems needed.

Sure will fix in the next version...

>
> > +#ifndef _GMII2RGMII_H
> > +#define _GMII2RGMII_H
> > +
> > +#include <linux/of.h>
> > +#include <linux/phy.h>
> > +#include <linux/mii.h>
> > +
> > +#define XILINX_GMII2RGMII_FULLDPLX BMCR_FULLDPLX
> > +#define XILINX_GMII2RGMII_SPEED1000 BMCR_SPEED1000
> > +#define XILINX_GMII2RGMII_SPEED100 BMCR_SPEED100
> > +#define XILINX_GMII2RGMII_REG_NUM 0x10
> > +
> > +struct gmii2rgmii {
> > + struct net_device *dev;
> > + struct mii_bus *mii_bus;
> > + struct phy_device *gmii2rgmii_phy_dev;
> > + void *platform_data;
> > + int (*mdio_write)(struct mii_bus *bus, int mii_id, int reg,
> > + u16 val);
> > + void (*fix_mac_speed)(struct gmii2rgmii *xphy, unsigned int speed);
> > +};
> > +
> > +extern int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy); #endif
>
> I see a compilation issue here:
>
> You should provide a way to have this function even if the NET_VENDOR_XILINX
> config option is not selected (test to compile with the sama5_defconfig and
> you'll see).

Ok will fix in the next version...

>
> What about making this function void in case of !XILINX?

This is one way to get rid of compilation error. Changes will be look like below

#ifdef CONFIG_NET_VENDOR_XILINX
extern int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy);
#else
extern void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy);
void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy)
{
}
#endif
For me the changes are looking odd...

Other possible ways
1) Put a config check around phyprobe api in the macb driver.
#ifdef CONFIG_XILINX_GMII2RGMII
gmii2rgmii_phyprobe(&bp->converter_phy);
#endif
2) Select NET_VENDOR_XILINX in the macb Kconfig
@ -22,6 +22,7 @@ config MACB
tristate "Cadence MACB/GEM support"
depends on HAS_DMA
select PHYLIB
+ select NET_VENDOR_XILINX

Please let me know which one you prefer will fix that and will post v3...

Regards,
Kedar.

\
 
 \ /
  Last update: 2016-07-04 14:21    [W:0.052 / U:12.628 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site