From: Nicolas Ferre <nicolas.ferre@atmel.com>
To: Appana Durga Kedareswara Rao <appana.durga.rao@xilinx.com>,
"robh+dt@kernel.org" <robh+dt@kernel.org>,
"mark.rutland@arm.com" <mark.rutland@arm.com>,
Michal Simek <michals@xilinx.com>,
Soren Brinkmann <sorenb@xilinx.com>,
Punnaiah Choudary Kalluri <punnaia@xilinx.com>,
"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
"andrew@lunn.ch" <andrew@lunn.ch>,
Anirudha Sarangi <anirudh@xilinx.com>,
Harini Katakam <harinik@xilinx.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH v2 2/4] net: ethernet: xilinx: Add gmii2rgmii converter support
Date: Mon, 4 Jul 2016 14:31:43 +0200 [thread overview]
Message-ID: <577A572F.1050004@atmel.com> (raw)
In-Reply-To: <C246CAC1457055469EF09E3A7AC4E11A4A5BB227@XAP-PVEXMBX01.xlnx.xilinx.com>
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 <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
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
next prev parent reply other threads:[~2016-07-04 12:32 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-04 9:04 [RFC PATCH v2 0/4] net: ethernet: Add support for gmii2rgmii converter Kedareswara rao Appana
2016-07-04 9:04 ` [RFC PATCH v2 1/4] Documentation: DT: net: Add Xilinx gmiitorgmii converter device tree binding documentation Kedareswara rao Appana
2016-07-04 14:04 ` Andrew Lunn
2016-07-06 14:12 ` Punnaiah Choudary Kalluri
2016-07-06 14:21 ` Andrew Lunn
2016-07-06 14:51 ` Punnaiah Choudary Kalluri
2016-07-26 15:09 ` Appana Durga Kedareswara Rao
2016-07-27 8:05 ` Andrew Lunn
2016-08-04 3:42 ` Florian Fainelli
2016-08-04 10:34 ` Appana Durga Kedareswara Rao
2016-07-04 9:04 ` [RFC PATCH v2 2/4] net: ethernet: xilinx: Add gmii2rgmii converter support Kedareswara rao Appana
2016-07-04 9:54 ` Nicolas Ferre
2016-07-04 11:47 ` Appana Durga Kedareswara Rao
2016-07-04 12:31 ` Nicolas Ferre [this message]
2016-07-04 12:36 ` Appana Durga Kedareswara Rao
2016-07-04 9:04 ` [RFC PATCH v2 3/4] Documentation: DT: net: Update binding doc for gmiitorgmii conveter Kedareswara rao Appana
2016-07-04 9:04 ` [RFC PATCH v2 4/4] net: macb: Add gmii2rgmii phy converter support Kedareswara rao Appana
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=577A572F.1050004@atmel.com \
--to=nicolas.ferre@atmel.com \
--cc=andrew@lunn.ch \
--cc=anirudh@xilinx.com \
--cc=appana.durga.rao@xilinx.com \
--cc=devicetree@vger.kernel.org \
--cc=f.fainelli@gmail.com \
--cc=harinik@xilinx.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=michals@xilinx.com \
--cc=netdev@vger.kernel.org \
--cc=punnaia@xilinx.com \
--cc=robh+dt@kernel.org \
--cc=sorenb@xilinx.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®