[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTikxYtG620D4z09k15v7=r5CZ7xCcDWenbJmM_sP@mail.gmail.com>
Date: Thu, 17 Feb 2011 09:56:13 +0900
From: Nobuhiro Iwamatsu <iwamatsu@...auri.org>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
Cc: netdev@...r.kernel.org, SH-Linux <linux-sh@...r.kernel.org>
Subject: Re: [RFC, PATCH 1/4] net: sh_eth: modify the definitions of register
2011/2/17 Nobuhiro Iwamatsu <iwamatsu@...auri.org>:
> 2011/2/15 Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>:
>> The previous code cannot handle the ETHER and GETHER both as same time
>> because the definitions of register was hardcoded.
>>
>> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
>> ---
>> arch/sh/include/asm/sh_eth.h | 7 +
>> drivers/net/sh_eth.c | 322 +++++++++++-----------
>> drivers/net/sh_eth.h | 623 ++++++++++++++++++++++++------------------
>> 3 files changed, 537 insertions(+), 415 deletions(-)
>>
>
> <snip>
>
>>
>> +static const u16 *sh_eth_get_register_offset(int register_type)
>> +{
>> + const u16 *reg_offset = NULL;
>> +
>> + switch (register_type) {
>> + case SH_ETH_REG_GIGABIT:
>> + reg_offset = sh_eth_offset_gigabit;
>> + break;
>> + case SH_ETH_REG_FAST_SH4:
>> + reg_offset = sh_eth_offset_fast_sh4;
>> + break;
>> + case SH_ETH_REG_FAST_SH3_SH2:
>> + reg_offset = sh_eth_offset_fast_sh3_sh2;
>> + break;
>> + case SH_ETH_REG_DEFAULT:
>> +#if defined(CONFIG_CPU_SUBTYPE_SH7763)
>> + reg_offset = sh_eth_offset_gigabit;
>> +#elif defined(CONFIG_CPU_SH4)
>> + reg_offset = sh_eth_offset_fast_sh4;
>> +#else
>> + reg_offset = sh_eth_offset_fast_sh3_sh2;
>> +#endif
>> + break;
>> + default:
>> + printk(KERN_ERR "Unknown register type (%d)\n", register_type);
>> + break;
>> + }
>> +
>> + return reg_offset;
>> +}
>> +
>
> Is the handling of SH_ETH_REG_DEFAULT necessary?
>
>>
>> +static inline void sh_eth_write(struct net_device *ndev, unsigned long data,
>> + int enum_index)
>> +{
>> + struct sh_eth_private *mdp = netdev_priv(ndev);
>> +
>> + writel(data, ndev->base_addr + mdp->reg_offset[enum_index]);
>> +}
>> +
>> +static inline unsigned long sh_eth_read(struct net_device *ndev,
>> + int enum_index)
>> +{
>> + struct sh_eth_private *mdp = netdev_priv(ndev);
>> +
>> + return readl(ndev->base_addr + mdp->reg_offset[enum_index]);
>> +}
>> +
>> +static inline void sh_eth_tsu_write(struct sh_eth_private *mdp,
>> + unsigned long data, int enum_index)
>> +{
>> + writel(data, mdp->tsu_addr + mdp->reg_offset[enum_index]);
>> +}
>> +
>> +static inline unsigned long sh_eth_tsu_read(struct sh_eth_private *mdp,
>> + int enum_index)
>> +{
>> + return readl(mdp->tsu_addr + mdp->reg_offset[enum_index]);
>> +}
>> +
>> #endif /* #ifndef __SH_ETH_H__ */
>
> I do not think that a new function is necessary.
> I think it use MACRO.
>
> For example,
> #define sh_eth_write(data,reg) \
> {\
> writel(data, ndev->base_addr + mdp->reg_offset[reg]); \
> }
>
Oh sorry, this is bad coding style.
Please ignore this comment.
Nobuhiro
--
Nobuhiro Iwamatsu
iwamatsu at {nigauri.org / debian.org}
GPG ID: 40AD1FA6
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists