From: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas•com>
To: Florian Fainelli <florian@openwrt•org>
Cc: netdev@vger•kernel.org
Subject: Re: [PATCH v2] net/sh-eth: Add support selecting MII function for SH7734 and R8A7740
Date: Tue, 26 Jun 2012 12:35:41 +0900 [thread overview]
Message-ID: <4FE92E0D.8000800@renesas.com> (raw)
In-Reply-To: <2360907.koU5zCUYMQ@flexo>
Hi,
Thank you for your review.
Florian Fainelli さんは書きました:
> On Tuesday 12 June 2012 16:29:02 Nobuhiro Iwamatsu wrote:
>> Ethernet IP of SH7734 and R8A7740 has selecting MII register.
>> The user needs to change a value according to MII to be used.
>> This adds the function to change the value of this register.
>>
>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas•com>
>> ---
>> V2: Fix the check by select_mii.
>> drivers/net/ethernet/renesas/sh_eth.c | 106
> ++++++++++++++++++++-------------
>> drivers/net/ethernet/renesas/sh_eth.h | 1 +
>> 2 files changed, 65 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c
> b/drivers/net/ethernet/renesas/sh_eth.c
>> index be3c221..5358804 100644
>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>> @@ -49,6 +49,33 @@
>> NETIF_MSG_RX_ERR| \
>> NETIF_MSG_TX_ERR)
>>
>> +#if defined(CONFIG_CPU_SUBTYPE_SH7734) || defined(CONFIG_CPU_SUBTYPE_SH7763)
> || \
>> + defined(CONFIG_ARCH_R8A7740)
>> +static void sh_eth_select_mii(struct net_device *ndev)
>> +{
>> + u32 value = 0x0;
>> + struct sh_eth_private *mdp = netdev_priv(ndev);
>> +
>> + switch (mdp->phy_interface) {
>> + case PHY_INTERFACE_MODE_GMII:
>> + value = 0x2;
>> + break;
>> + case PHY_INTERFACE_MODE_MII:
>> + value = 0x1;
>> + break;
>> + case PHY_INTERFACE_MODE_RMII:
>> + value = 0x0;
>> + break;
>> + default:
>> + pr_warn("PHY interface mode was not setup. Set to MII.\n");
>> + value = 0x1;
>> + break;
>> + }
>> +
>> + sh_eth_write(ndev, value, RMII_MII);
>> +}
>> +#endif
>> +
>> /* There is CPU dependent code */
>> #if defined(CONFIG_CPU_SUBTYPE_SH7724)
>> #define SH_ETH_RESET_DEFAULT 1
>> @@ -283,6 +310,7 @@ static struct sh_eth_cpu_data
> *sh_eth_get_cpu_data(struct sh_eth_private *mdp)
>> #elif defined(CONFIG_CPU_SUBTYPE_SH7734) ||
> defined(CONFIG_CPU_SUBTYPE_SH7763)
>> #define SH_ETH_HAS_TSU 1
>> static void sh_eth_reset_hw_crc(struct net_device *ndev);
>> +
>> static void sh_eth_chip_reset(struct net_device *ndev)
>> {
>> struct sh_eth_private *mdp = netdev_priv(ndev);
>> @@ -292,35 +320,6 @@ static void sh_eth_chip_reset(struct net_device *ndev)
>> mdelay(1);
>> }
>>
>> -static void sh_eth_reset(struct net_device *ndev)
>> -{
>> - int cnt = 100;
>> -
>> - sh_eth_write(ndev, EDSR_ENALL, EDSR);
>> - sh_eth_write(ndev, sh_eth_read(ndev, EDMR) | EDMR_SRST_GETHER, EDMR);
>> - while (cnt > 0) {
>> - if (!(sh_eth_read(ndev, EDMR) & 0x3))
>> - break;
>> - mdelay(1);
>> - cnt--;
>> - }
>> - if (cnt == 0)
>> - printk(KERN_ERR "Device reset fail\n");
>> -
>> - /* Table Init */
>> - sh_eth_write(ndev, 0x0, TDLAR);
>> - sh_eth_write(ndev, 0x0, TDFAR);
>> - sh_eth_write(ndev, 0x0, TDFXR);
>> - sh_eth_write(ndev, 0x0, TDFFR);
>> - sh_eth_write(ndev, 0x0, RDLAR);
>> - sh_eth_write(ndev, 0x0, RDFAR);
>> - sh_eth_write(ndev, 0x0, RDFXR);
>> - sh_eth_write(ndev, 0x0, RDFFR);
>> -
>> - /* Reset HW CRC register */
>> - sh_eth_reset_hw_crc(ndev);
>> -}
>> -
>> static void sh_eth_set_duplex(struct net_device *ndev)
>> {
>> struct sh_eth_private *mdp = netdev_priv(ndev);
>> @@ -377,9 +376,43 @@ static struct sh_eth_cpu_data sh_eth_my_cpu_data = {
>> .tsu = 1,
>> #if defined(CONFIG_CPU_SUBTYPE_SH7734)
>> .hw_crc = 1,
>> + .select_mii = 1,
>> #endif
>> };
>>
>> +static void sh_eth_reset(struct net_device *ndev)
>> +{
>> + int cnt = 100;
>> +
>> + sh_eth_write(ndev, EDSR_ENALL, EDSR);
>> + sh_eth_write(ndev, sh_eth_read(ndev, EDMR) | EDMR_SRST_GETHER, EDMR);
>> + while (cnt > 0) {
>> + if (!(sh_eth_read(ndev, EDMR) & 0x3))
>> + break;
>> + mdelay(1);
>> + cnt--;
>> + }
>> + if (cnt == 0)
>> + printk(KERN_ERR "Device reset fail\n");
>
> It looks like this would need a subsequent fix. Failing to reset the adapter
> and just erroring out and not returning an error looks obviously wrong. Since
> sh_eth_reset() is called in sh_eth_dev_init() which does return an int,
> propagate the error back to the caller.
Yes, you are right. I will fix your point and send a patch.
Best regards,
Nobuhiro
prev parent reply other threads:[~2012-06-26 3:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-12 7:29 [PATCH v2] net/sh-eth: Add support selecting MII function for SH7734 and R8A7740 Nobuhiro Iwamatsu
2012-06-12 7:29 ` David Miller
2012-06-26 3:32 ` Nobuhiro Iwamatsu
2012-06-12 8:40 ` Florian Fainelli
2012-06-26 3:35 ` Nobuhiro Iwamatsu [this message]
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=4FE92E0D.8000800@renesas.com \
--to=nobuhiro.iwamatsu.yj@renesas$(echo .)com \
--cc=florian@openwrt$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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