From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753611AbcGDMcD (ORCPT ); Mon, 4 Jul 2016 08:32:03 -0400 Received: from eusmtp01.atmel.com ([212.144.249.243]:4007 "EHLO eusmtp01.atmel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753076AbcGDMcA (ORCPT ); Mon, 4 Jul 2016 08:32:00 -0400 Subject: Re: [RFC PATCH v2 2/4] net: ethernet: xilinx: Add gmii2rgmii converter support To: Appana Durga Kedareswara Rao , "robh+dt@kernel.org" , "mark.rutland@arm.com" , Michal Simek , Soren Brinkmann , Punnaiah Choudary Kalluri , "f.fainelli@gmail.com" , "andrew@lunn.ch" , Anirudha Sarangi , Harini Katakam References: <1467623084-15471-1-git-send-email-appanad@xilinx.com> <1467623084-15471-3-git-send-email-appanad@xilinx.com> <577A323A.4080207@atmel.com> CC: "netdev@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" From: Nicolas Ferre Organization: atmel Message-ID: <577A572F.1050004@atmel.com> Date: Mon, 4 Jul 2016 14:31:43 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.161.30.18] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le 04/07/2016 13:47, Appana Durga Kedareswara Rao a écrit : > 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 >>> +#include >>> +#include >>> + >>> +#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 You may need to have: #if defined(CONFIG_NET_VENDOR_XILINX) && defined(CONFIG_XILINX_GMII2RGMII) > extern int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy); > #else > extern void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy); No need for the line above... > void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy) > { > } On one single line, like: static inline void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy) { } > #endif > For me the changes are looking odd... For me, it's okay... > > 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 Nope! > 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... First one with my changes is the best. But maybe wait for more feedback... Bye, -- Nicolas Ferre