RE: [RFC PATCH v2 2/4] net: ethernet: xilinx: Add gmii2rgmii converter support

[Appana Durga Kedareswara Rao]

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.