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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <C246CAC1457055469EF09E3A7AC4E11A4A5BB227@XAP-PVEXMBX01.xlnx.xilinx.com>
Date:	Mon, 4 Jul 2016 11:47:50 +0000
From:	Appana Durga Kedareswara Rao <appana.durga.rao@...inx.com>
To:	Nicolas Ferre <nicolas.ferre@...el.com>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	"mark.rutland@....com" <mark.rutland@....com>,
	"Michal Simek" <michals@...inx.com>,
	Soren Brinkmann <sorenb@...inx.com>,
	"Punnaiah Choudary Kalluri" <punnaia@...inx.com>,
	"f.fainelli@...il.com" <f.fainelli@...il.com>,
	"andrew@...n.ch" <andrew@...n.ch>,
	Anirudha Sarangi <anirudh@...inx.com>,
	Harini Katakam <harinik@...inx.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH v2 2/4] net: ethernet: xilinx: Add gmii2rgmii
 converter support

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.