* Re: [net-next v2 2/8] i40e: transmit, receive, and napi
From: David Miller @ 2013-08-23 18:04 UTC (permalink / raw)
To: sassmann
Cc: jeffrey.t.kirsher, jesse.brandeburg, netdev, gospo,
shannon.nelson, peter.p.waskiewicz.jr, e1000-devel
In-Reply-To: <5217589F.6000602@kpanic.de>
From: Stefan Assmann <sassmann@kpanic.de>
Date: Fri, 23 Aug 2013 14:42:07 +0200
> On 23.08.2013 04:15, Jeff Kirsher wrote:
>> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
>>
>> This patch contains the transmit, receive, and napi routines, as well
>> as ancillary routines.
>>
>> This file is code that is (will be) shared between the VF and PF
>> drivers.
>
> Just some small nitpicks.
>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>> new file mode 100644
>> index 0000000..ceafef0
>> --- /dev/null
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>
> [...]
>
>> +static void i40e_receive_skb(struct i40e_ring *rx_ring,
>> + struct sk_buff *skb, u16 vlan_tag)
>> +{
>> + struct i40e_vsi *vsi = rx_ring->vsi;
>> + struct i40e_q_vector *q_vector = rx_ring->q_vector;
>> + u64 flags = vsi->back->flags;
>> +
>> + if (vlan_tag & VLAN_VID_MASK)
>> + __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
>
> Suggesting __constant_htons instead of htons here.
We don't suggest that anymore, because it's completely unnecessary
with the way the macros are implemented.
^ permalink raw reply
* Re: [PATCH RESEND net-next 06/16] sfc: Introduce and use MCDI_DECLARE_BUF macro
From: David Miller @ 2013-08-23 18:00 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1377253570.2586.15.camel@deadeye.wl.decadent.org.uk>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Fri, 23 Aug 2013 11:26:10 +0100
> My intent was to separate refactoring changes with no immediate effect
> (#6-9) from functional changes (#10-11), making them easier to review.
> The alignment gets fixed by #11.
Ok I didn't see that, I'll keep reviewing this series then.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/2] qlge: feature update
From: Jitendra Kalsaria @ 2013-08-23 17:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Ron Mercer, Dept-NX Linux NIC Driver
In-Reply-To: <20130822.221246.527881072702590889.davem@davemloft.net>
On 8/22/13 10:12 PM, "David Miller" <davem@davemloft.net> wrote:
>From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
>Date: Thu, 22 Aug 2013 15:54:10 -0400
>
>> This patch series enhance the handling of nested vlan tags in Rx path.
>
>Sorry no module parameters.
>You must make a generic, kernel wide, interface for controlling behavior
>such as this.
>
>Otherwise users will need to learn unique configuration mechanisms
>to control this kind of thing for every single driver and that's
>beyond terrible.
>
>I'm not applying these patches.
I will remove module parameter and will submit it again. Meantime will
look into making it generic interface to toggle the behavior.
Thanks,
Jiten
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Sekhar Nori @ 2013-08-23 17:39 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Daniel Mack, netdev, bcousson, sergei.shtylyov, davem, ujhelyi.m,
mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
linux-omap, devicetree
In-Reply-To: <52179BDA.50603@ti.com>
On 8/23/2013 10:58 PM, Santosh Shilimkar wrote:
> On Friday 23 August 2013 01:24 PM, Daniel Mack wrote:
>> On 23.08.2013 19:19, Santosh Shilimkar wrote:
>>> On Friday 23 August 2013 01:09 PM, Sekhar Nori wrote:
>>>> On 8/23/2013 10:26 PM, Santosh Shilimkar wrote:
>>
>>>>> So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
>>>>
>>>> If this could be handled using IP version then the right way would be to
>>>> just read the IP version from hardware and use it. No need of DT property.
>>>>
>>> Thats fine as well but I thought the patch needed additional properties like
>>> CM reg-address come from DT and hence the separate compatible. If you can
>>> manage without that, thats even better.
>>
>> We can't, that's the whole point :)
>>
> I saw that from the patch :)
>
>> Well, theoretically, we could for now, but that's not a clean solution.
>> Again: the problem here is that the control port is separated from the
>> cpsw core, and so we have to implement something specific for the AM3352
>> SoC. I know that's a violation of clean and generic driver ideas, but
>> there's no way we can assume that every cpsw v2 ip block has a control
>> port that is compatible to the one found on am335x chips.
>>
> But there is a possibility that other SOC will just use the same
> control module approach. So using a revision IP is just fine. BTW,
But this is misleading because it makes appear like the same compatible
can be used on on another SoC like DRA7 which probably has the same
version of IP but a different control module implementation, when in
practice it cannot.
The fact is we are doing something SoC specific in the driver and we
cannot hide that behind IP versions. If really in practice there comes
another SoC with the same control module definition then it can always
use ti,am3352-cpsw compatible as well. The compatible name does not
preclude its usage.
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH v4 2/5] net: ethernet: cpsw: add optional third memory region for CONTROL module
From: Sergei Shtylyov @ 2013-08-23 17:37 UTC (permalink / raw)
To: Daniel Mack
Cc: netdev, bcousson, nsekhar, davem, ujhelyi.m, mugunthanvnm,
vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
devicetree
In-Reply-To: <52178BED.1070308@gmail.com>
On 08/23/2013 08:21 PM, Daniel Mack wrote:
>>> + priv->gmii_sel_reg = devm_ioremap_resource(&pdev->dev, res);
>>> + if (IS_ERR(priv->gmii_sel_reg)) {
>>> + dev_err(priv->dev, "unable to map control i/o region\n");
>> You didn't actually seem to heed my words about error message.
> Well yes I did, but only in the check for platform_get_resource(). As
> the comment says - we pass on if that memory region is not given, but if
> it is given, it also has to be valid.
Yes, but what I told you was devm_ioremap_resource() prints the error
messages itself, so that you don't have to. And you even consented with that. :-)
>> And don't you want to do:
>> res = PTR_ERR(priv->gmii_sel_reg);
Well, I've messed with the variable name: 'res' is struct resource *',
what I meant was *int* variable.
> Erm, of course. Sorry for that.
> Daniel
WBR, Sergei
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Santosh Shilimkar @ 2013-08-23 17:28 UTC (permalink / raw)
To: Daniel Mack
Cc: Sekhar Nori, netdev, bcousson, sergei.shtylyov, davem, ujhelyi.m,
mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
linux-omap, devicetree
In-Reply-To: <52179AD6.8090406@gmail.com>
On Friday 23 August 2013 01:24 PM, Daniel Mack wrote:
> On 23.08.2013 19:19, Santosh Shilimkar wrote:
>> On Friday 23 August 2013 01:09 PM, Sekhar Nori wrote:
>>> On 8/23/2013 10:26 PM, Santosh Shilimkar wrote:
>
>>>> So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
>>>
>>> If this could be handled using IP version then the right way would be to
>>> just read the IP version from hardware and use it. No need of DT property.
>>>
>> Thats fine as well but I thought the patch needed additional properties like
>> CM reg-address come from DT and hence the separate compatible. If you can
>> manage without that, thats even better.
>
> We can't, that's the whole point :)
>
I saw that from the patch :)
> Well, theoretically, we could for now, but that's not a clean solution.
> Again: the problem here is that the control port is separated from the
> cpsw core, and so we have to implement something specific for the AM3352
> SoC. I know that's a violation of clean and generic driver ideas, but
> there's no way we can assume that every cpsw v2 ip block has a control
> port that is compatible to the one found on am335x chips.
>
But there is a possibility that other SOC will just use the same
control module approach. So using a revision IP is just fine. BTW,
CPSW is not the only driver where control module is used. There are
many examples like USB, MMC etc
There is nothing wrong in using the version info and associating
the additional functionality with it.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Daniel Mack @ 2013-08-23 17:24 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Sekhar Nori, netdev, bcousson, sergei.shtylyov, davem, ujhelyi.m,
mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
linux-omap, devicetree
In-Reply-To: <521799A3.7080706@ti.com>
On 23.08.2013 19:19, Santosh Shilimkar wrote:
> On Friday 23 August 2013 01:09 PM, Sekhar Nori wrote:
>> On 8/23/2013 10:26 PM, Santosh Shilimkar wrote:
>>> So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
>>
>> If this could be handled using IP version then the right way would be to
>> just read the IP version from hardware and use it. No need of DT property.
>>
> Thats fine as well but I thought the patch needed additional properties like
> CM reg-address come from DT and hence the separate compatible. If you can
> manage without that, thats even better.
We can't, that's the whole point :)
Well, theoretically, we could for now, but that's not a clean solution.
Again: the problem here is that the control port is separated from the
cpsw core, and so we have to implement something specific for the AM3352
SoC. I know that's a violation of clean and generic driver ideas, but
there's no way we can assume that every cpsw v2 ip block has a control
port that is compatible to the one found on am335x chips.
Daniel
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Mugunthan V N @ 2013-08-23 17:23 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Daniel Mack, netdev, bcousson, nsekhar, sergei.shtylyov, davem,
ujhelyi.m, vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
devicetree, Gupta, Pekon
In-Reply-To: <52179448.1020906@ti.com>
On Friday 23 August 2013 10:26 PM, Santosh Shilimkar wrote:
> On Friday 23 August 2013 12:30 PM, Daniel Mack wrote:
>> On 23.08.2013 16:23, Santosh Shilimkar wrote:
>>> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>>>> +static const struct of_device_id cpsw_of_mtable[] = {
>>>> + {
>>>> + .compatible = "ti,am3352-cpsw",
>>> I didn't notice this earlier, but can't you use the IP version
>>> as a compatible instead of using a SOC name. Whats really SOC specific
>>> on this IP ? Sorry i have missed any earlier discussion on this but
>>> this approach doesn't seem good. Its like adding SOC checks in the
>>> driver subsystem.
>> As I already mentioned in the cover letter and in the commit message, I
>> just don't know which criteria makes most sense here.
>>
>> On a general note, I would say that chances that this exactly IP core
>> with the same version number will appear on some other silicon which
>> doesn't support the control mode register in an AM33xx fashion, is not
>> necessarily negligible.
>>
>> So what that new compatible string denotes is the cpsw in a version as
>> found on am3352 SoCs, which is actually exactly what it does.
>>
>> I don't have a strong opinion here, but see your point. I just don't
>> have a better idea on how to treat that.
>>
> So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
> That way if in future if someone uses those features, they can use
> this compatible if they don't they use the one which suites that
> SOC.
>
We cannot map control module register with CPSW IP version as both comes
from different design team and CPSW ip version can be same across SoC
and gmii sel register definition can be different. Control module
defines may vary in different SoC as per SoC requirements.
Adding Pekon Gupta who had worked in Silicon team before.
Regards
Mugunthan V N
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Santosh Shilimkar @ 2013-08-23 17:19 UTC (permalink / raw)
To: Sekhar Nori
Cc: mugunthanvnm, sergei.shtylyov, d-gerlach, netdev, vaibhav.bedia,
Daniel Mack, devicetree, bcousson, ujhelyi.m, linux-omap, davem,
linux-arm-kernel
In-Reply-To: <5217974B.5090503@ti.com>
On Friday 23 August 2013 01:09 PM, Sekhar Nori wrote:
>
>
> On 8/23/2013 10:26 PM, Santosh Shilimkar wrote:
>> On Friday 23 August 2013 12:30 PM, Daniel Mack wrote:
>>> On 23.08.2013 16:23, Santosh Shilimkar wrote:
>>>> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>>>
>>>>> +static const struct of_device_id cpsw_of_mtable[] = {
>>>>> + {
>>>>> + .compatible = "ti,am3352-cpsw",
>>>>
>>>> I didn't notice this earlier, but can't you use the IP version
>>>> as a compatible instead of using a SOC name. Whats really SOC specific
>>>> on this IP ? Sorry i have missed any earlier discussion on this but
>>>> this approach doesn't seem good. Its like adding SOC checks in the
>>>> driver subsystem.
>>>
>>> As I already mentioned in the cover letter and in the commit message, I
>>> just don't know which criteria makes most sense here.
>>>
>>> On a general note, I would say that chances that this exactly IP core
>>> with the same version number will appear on some other silicon which
>>> doesn't support the control mode register in an AM33xx fashion, is not
>>> necessarily negligible.
>>>
>>> So what that new compatible string denotes is the cpsw in a version as
>>> found on am3352 SoCs, which is actually exactly what it does.
>>>
>>> I don't have a strong opinion here, but see your point. I just don't
>>> have a better idea on how to treat that.
>>>
>> So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
>
> If this could be handled using IP version then the right way would be to
> just read the IP version from hardware and use it. No need of DT property.
>
Thats fine as well but I thought the patch needed additional properties like
CM reg-address come from DT and hence the separate compatible. If you can
manage without that, thats even better.
Regards,
Santosh
^ permalink raw reply
* Deal of $25,275,000.00
From: Q @ 2013-08-23 13:58 UTC (permalink / raw)
To: Recipients
Deal of $25,275,000.00
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Daniel Mack @ 2013-08-23 17:17 UTC (permalink / raw)
To: Sekhar Nori
Cc: Santosh Shilimkar, netdev, bcousson, sergei.shtylyov, davem,
ujhelyi.m, mugunthanvnm, vaibhav.bedia, d-gerlach,
linux-arm-kernel, linux-omap, devicetree
In-Reply-To: <5217974B.5090503@ti.com>
On 23.08.2013 19:09, Sekhar Nori wrote:
> On 8/23/2013 10:26 PM, Santosh Shilimkar wrote:
>> So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
>
> If this could be handled using IP version then the right way would be to
> just read the IP version from hardware and use it. No need of DT property.
Yes, exactly. The information we need to support the phy interface
setting is completely disconnected from the cpsw core itself.
Daniel
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Sekhar Nori @ 2013-08-23 17:09 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Daniel Mack, netdev, bcousson, sergei.shtylyov, davem, ujhelyi.m,
mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
linux-omap, devicetree
In-Reply-To: <52179448.1020906@ti.com>
On 8/23/2013 10:26 PM, Santosh Shilimkar wrote:
> On Friday 23 August 2013 12:30 PM, Daniel Mack wrote:
>> On 23.08.2013 16:23, Santosh Shilimkar wrote:
>>> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>>
>>>> +static const struct of_device_id cpsw_of_mtable[] = {
>>>> + {
>>>> + .compatible = "ti,am3352-cpsw",
>>>
>>> I didn't notice this earlier, but can't you use the IP version
>>> as a compatible instead of using a SOC name. Whats really SOC specific
>>> on this IP ? Sorry i have missed any earlier discussion on this but
>>> this approach doesn't seem good. Its like adding SOC checks in the
>>> driver subsystem.
>>
>> As I already mentioned in the cover letter and in the commit message, I
>> just don't know which criteria makes most sense here.
>>
>> On a general note, I would say that chances that this exactly IP core
>> with the same version number will appear on some other silicon which
>> doesn't support the control mode register in an AM33xx fashion, is not
>> necessarily negligible.
>>
>> So what that new compatible string denotes is the cpsw in a version as
>> found on am3352 SoCs, which is actually exactly what it does.
>>
>> I don't have a strong opinion here, but see your point. I just don't
>> have a better idea on how to treat that.
>>
> So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
If this could be handled using IP version then the right way would be to
just read the IP version from hardware and use it. No need of DT property.
Thanks,
Sekhar
^ permalink raw reply
* Re: [net-next v2 3/8] i40e: driver ethtool core
From: Stefan Assmann @ 2013-08-23 17:08 UTC (permalink / raw)
To: Jeff Kirsher
Cc: davem, Jesse Brandeburg, netdev, gospo, Shannon Nelson,
PJ Waskiewicz, e1000-devel
In-Reply-To: <1377224142-25160-4-git-send-email-jeffrey.t.kirsher@intel.com>
On 23.08.2013 04:15, Jeff Kirsher wrote:
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> This patch contains the ethtool interface and implementation.
>
> The goal in this patch series is minimal functionality while not
> including much in the way of "set support."
[...]
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
[...]
> +#define I40E_QUEUE_STATS_LEN(n) \
> + ((((struct i40e_netdev_priv *)netdev_priv((n)))->vsi->num_queue_pairs + \
> + ((struct i40e_netdev_priv *)netdev_priv((n)))->vsi->num_queue_pairs) * 2)
> +#define I40E_GLOBAL_STATS_LEN ARRAY_SIZE(i40e_gstrings_stats)
> +#define I40E_NETDEV_STATS_LEN ARRAY_SIZE(i40e_gstrings_net_stats)
> +#define I40E_VSI_STATS_LEN(n) (I40E_NETDEV_STATS_LEN + \
Please use tabs for spacing here.
> + I40E_QUEUE_STATS_LEN((n)))
> +#define I40E_PFC_STATS_LEN ( \
> + (FIELD_SIZEOF(struct i40e_pf, stats.priority_xoff_rx) + \
> + FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_rx) + \
> + FIELD_SIZEOF(struct i40e_pf, stats.priority_xoff_tx) + \
> + FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_tx) + \
> + FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_2_xoff)) \
> + / sizeof(u64))
> +#define I40E_PF_STATS_LEN(n) (I40E_GLOBAL_STATS_LEN + \
Here as well.
[...]
> +static void i40e_get_regs(struct net_device *netdev, struct ethtool_regs *regs,
> + void *p)
> +{
> + struct i40e_netdev_priv *np = netdev_priv(netdev);
> + struct i40e_pf *pf = np->vsi->back;
> + struct i40e_hw *hw = &pf->hw;
> + u32 *reg_buf = p;
> + int i, j, ri;
> + u32 reg;
> +
> + /* Tell ethtool which driver-version-specific regs output we have.
> + *
> + * At some point, if we have ethtool doing special formatting of
> + * this data, it will rely on this version number to know how to
> + * interpret things. Hence, this needs to be updated if/when the
> + * diags register table is changed.
> + */
> + regs->version = 1;
> +
> + /* loop through the diags reg table for what to print */
> + ri = 0;
> + for (i = 0; i40e_reg_list[i].offset != 0; i++) {
> + for (j = 0; j < i40e_reg_list[i].elements; j++) {
> + reg = i40e_reg_list[i].offset
> + + (j * i40e_reg_list[i].stride);
> + reg_buf[ri++] = rd32(hw, reg);
> + }
> + }
> +
> + return;
void function, no return necessary.
> +}
[...]
> +static void i40e_get_ethtool_stats(struct net_device *netdev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + struct i40e_netdev_priv *np = netdev_priv(netdev);
> + struct i40e_vsi *vsi = np->vsi;
> + struct i40e_pf *pf = vsi->back;
> + struct rtnl_link_stats64 *net_stats = i40e_get_vsi_stats_struct(vsi);
> + char *p;
> + int i, j;
> +
> + i40e_update_stats(vsi);
> +
> + i = 0;
This could be avoided by int i = 0 few lines above.
> + for (j = 0; j < I40E_NETDEV_STATS_LEN; j++) {
> + p = (char *)net_stats + i40e_gstrings_net_stats[j].stat_offset;
> + data[i++] = (i40e_gstrings_net_stats[j].sizeof_stat ==
> + sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
> + }
> + for (j = 0; j < vsi->num_queue_pairs; j++) {
> + data[i++] = vsi->tx_rings[j].tx_stats.packets;
> + data[i++] = vsi->tx_rings[j].tx_stats.bytes;
> + }
> + for (j = 0; j < vsi->num_queue_pairs; j++) {
> + data[i++] = vsi->rx_rings[j].rx_stats.packets;
> + data[i++] = vsi->rx_rings[j].rx_stats.bytes;
> + }
> + if (vsi == pf->vsi[pf->lan_vsi]) {
> + for (j = 0; j < I40E_GLOBAL_STATS_LEN; j++) {
> + p = (char *)pf + i40e_gstrings_stats[j].stat_offset;
> + data[i++] = (i40e_gstrings_stats[j].sizeof_stat ==
> + sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
> + }
> + for (j = 0; j < I40E_MAX_USER_PRIORITY; j++) {
> + data[i++] = pf->stats.priority_xon_tx[j];
> + data[i++] = pf->stats.priority_xoff_tx[j];
> + }
> + for (j = 0; j < I40E_MAX_USER_PRIORITY; j++) {
> + data[i++] = pf->stats.priority_xon_rx[j];
> + data[i++] = pf->stats.priority_xoff_rx[j];
> + }
> + for (j = 0; j < I40E_MAX_USER_PRIORITY; j++)
> + data[i++] = pf->stats.priority_xon_2_xoff[j];
> + }
> +
> + return;
Another void function.
[...]
> +static struct ethtool_ops i40e_ethtool_ops = {
> + .get_settings = i40e_get_settings,
> + .get_drvinfo = i40e_get_drvinfo,
> + .get_regs_len = i40e_get_regs_len,
> + .get_regs = i40e_get_regs,
> + .nway_reset = i40e_nway_reset,
> + .get_link = ethtool_op_get_link,
> + .get_wol = i40e_get_wol,
> + .get_ringparam = i40e_get_ringparam,
> + .set_ringparam = i40e_set_ringparam,
> + .get_pauseparam = i40e_get_pauseparam,
> + .get_msglevel = i40e_get_msglevel,
> + .set_msglevel = i40e_set_msglevel,
> + .get_rxnfc = i40e_get_rxnfc,
> + .set_rxnfc = i40e_set_rxnfc,
> + .self_test = i40e_diag_test,
> + .get_strings = i40e_get_strings,
> + .set_phys_id = i40e_set_phys_id,
> + .get_sset_count = i40e_get_sset_count,
> + .get_ethtool_stats = i40e_get_ethtool_stats,
> + .get_coalesce = i40e_get_coalesce,
> + .set_coalesce = i40e_set_coalesce,
> + .get_ts_info = i40e_get_ts_info,
> +};
It would be nice if you could use tabs for spacing here.
Stefan
^ permalink raw reply
* Re: [PATCH 2/2] genl: Hold reference on correct module while netlink-dump.
From: Pravin Shelar @ 2013-08-23 17:05 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev@vger.kernel.org, Jesse Gross
In-Reply-To: <1377242625.14021.5.camel@jlt4.sipsolutions.net>
On Fri, Aug 23, 2013 at 12:23 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
>> +extern int __genl_register_family(struct genl_family *family, struct module *module);
>
> Why add an extra argument instead of just assigning family->module in
> the inline wrapper(s):
>
> static inline int genl_register_family(struct genl_family *family)
> {
> family->module = THIS_MODULE;
> return __genl_register_family(family);
> }
>
This is better, I will send updated patch.
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Santosh Shilimkar @ 2013-08-23 17:05 UTC (permalink / raw)
To: Mugunthan V N
Cc: Daniel Mack, netdev, bcousson, nsekhar, sergei.shtylyov, davem,
ujhelyi.m, vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
devicetree
In-Reply-To: <521791C1.2060803@ti.com>
On Friday 23 August 2013 12:45 PM, Mugunthan V N wrote:
> On Friday 23 August 2013 07:53 PM, Santosh Shilimkar wrote:
>> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>>> In order to support features that are specific to the AM335x IP, we have
>>> to add hardware types and another compatible string.
>>>
>>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>>> ---
>>> Documentation/devicetree/bindings/net/cpsw.txt | 3 ++-
>>> drivers/net/ethernet/ti/cpsw.c | 32 ++++++++++++++++++++------
>>> drivers/net/ethernet/ti/cpsw.h | 1 +
>>> 3 files changed, 28 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
>>> index 4e5ca54..b717458 100644
>>> --- a/Documentation/devicetree/bindings/net/cpsw.txt
>>> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
>>> @@ -2,7 +2,8 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
>>> ------------------------------------------------------
>>>
>>> Required properties:
>>> -- compatible : Should be "ti,cpsw"
>>> +- compatible : Should be "ti,cpsw" for generic cpsw support, or
>>> + "ti,am3352-cpsw" for AM3352 SoCs
>>> - reg : physical base address and size of the cpsw
>>> registers map.
>>> An optional third memory region can be supplied if
>>> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
>>> index 7a25ff4..73c44cb6 100644
>>> --- a/drivers/net/ethernet/ti/cpsw.c
>>> +++ b/drivers/net/ethernet/ti/cpsw.c
>>> @@ -155,6 +155,11 @@ do { \
>>> ((priv->data.dual_emac) ? priv->emac_port : \
>>> priv->data.active_slave)
>>>
>>> +enum {
>>> + CPSW_TYPE_GENERIC,
>>> + CPSW_TYPE_AM33XX
>>> +};
>>> +
>>> static int debug_level;
>>> module_param(debug_level, int, 0);
>>> MODULE_PARM_DESC(debug_level, "cpsw debug level (NETIF_MSG bits)");
>>> @@ -1692,17 +1697,36 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
>>> slave->port_vlan = data->dual_emac_res_vlan;
>>> }
>>>
>>> +static const struct of_device_id cpsw_of_mtable[] = {
>>> + {
>>> + .compatible = "ti,am3352-cpsw",
>> I didn't notice this earlier, but can't you use the IP version
>> as a compatible instead of using a SOC name. Whats really SOC specific
>> on this IP ? Sorry i have missed any earlier discussion on this but
>> this approach doesn't seem good. Its like adding SOC checks in the
>> driver subsystem.
>>
>>
> But the same IP can be used in different SoC as well where the control
> register may be different as per the Silicon Integration team's decision?
>
> Ideally there should be a separate control module driver so that it can
> take care of different SoC related needs.
>
Instead of having one control module driver to address all the requirements
in various IP's, you could specifically address that in subsystem driver layer.
SOC guys invariably use control modules for certain functionality since it
is easy and quick fix. And their is uniformity in the way they are
implemented for different IP needs.
In this specific case, if the CPSW IP version has some part of the
functionality implemented using control module that is just harfaware
choice. Its not mandatory to partition that in two sub-system. Rather
consider it as a special IP version and treat it accordingly in the
driver.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH 1/2] genl: Fix genl dumpit() locking.
From: Pravin Shelar @ 2013-08-23 17:05 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev@vger.kernel.org, Jesse Gross
In-Reply-To: <1377242432.14021.2.camel@jlt4.sipsolutions.net>
On Fri, Aug 23, 2013 at 12:20 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
>> @@ -572,15 +594,29 @@ static int genl_family_rcv_msg(struct
>> genl_family *family,
>> return -EPERM;
>>
>> if ((nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP) {
>> - struct netlink_dump_control c = {
>> - .dump = ops->dumpit,
>> - .done = ops->done,
>> - };
>> + struct netlink_dump_control c;
>> + int rc;
>>
>> if (ops->dumpit == NULL)
>> return -EOPNOTSUPP;
>>
>> - return netlink_dump_start(net->genl_sock, skb, nlh,
>> &c);
>> + memset(&c, 0, sizeof(c));
>> + if (!family->parallel_ops) {
>> + genl_unlock();
>> + c.data = ops;
>> + c.dump = genl_lock_dumpit;
>> + if (ops->done)
>> + c.done = genl_lock_done;
>> + } else {
>> + c.dump = ops->dumpit;
>> + c.done = ops->done;
>> + }
>> +
>> + rc = netlink_dump_start(net->genl_sock, skb, nlh, &c);
>> + if (!family->parallel_ops)
>> + genl_lock();
>> + return rc;
>> +
>
> I think this piece would be easier to read if you call
> netlink_dump_start() separately in the two branches. If you also move
> the "c" variable into the branches then you can keep initializing it
> with an explicit initializer which would also be more readable IMHO.
>
ok, I will update patch.
> Either way, this seems fine. I still think that not assigning cb_mutex
> wasn't the smartest idea, but I'll reply over in the other thread.
^ permalink raw reply
* RE: [net-next v2 1/8] i40e: main driver core
From: Nelson, Shannon @ 2013-08-23 17:00 UTC (permalink / raw)
To: David Miller, Kirsher, Jeffrey T
Cc: Brandeburg, Jesse, netdev@vger.kernel.org, gospo@redhat.com,
sassmann@redhat.com, Waskiewicz Jr, Peter P,
e1000-devel@lists.sourceforge.net
In-Reply-To: <20130823.002805.1975660490102085087.davem@davemloft.net>
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Friday, August 23, 2013 12:28 AM
> To: Kirsher, Jeffrey T
> Cc: Brandeburg, Jesse; netdev@vger.kernel.org; gospo@redhat.com;
> sassmann@redhat.com; Nelson, Shannon; Waskiewicz Jr, Peter P; e1000-
> devel@lists.sourceforge.net
> Subject: Re: [net-next v2 1/8] i40e: main driver core
Thanks, Dave, for your time and the notes.
>
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Thu, 22 Aug 2013 19:15:35 -0700
>
> > +enum i40e_status_code i40e_allocate_dma_mem_d(struct i40e_hw *hw,
> > + struct i40e_dma_mem *mem,
> > + u64 size, u32 alignment)
> > +{
> ...
> > + mem->va = dma_alloc_coherent(&pf->pdev->dev, mem->size,
> > + &mem->pa, GFP_ATOMIC | __GFP_ZERO);
>
> First, I see no reason to specify GFP_ATOMIC here, code paths that
> call this thing even have comments above them like:
>
> --------------------
> + * Do *NOT* hold the lock when calling this as the memory allocation
> routines
> + * called are not going to be atomic context safe
> --------------------
>
> Secondly, use dma_zalloc_coherent() if you want __GFP_ZERO.
Sure, we'll adjust.
>
> > +static int i40e_get_lump(struct i40e_lump_tracking *pile, u16 needed,
> u16 id)
> > +{
> > + int i = 0, j = 0;
> > + int ret = I40E_ERR_NO_MEMORY;
> > +
> > + if (pile == NULL || needed == 0 || id >= I40E_PILE_VALID_BIT) {
> > + pr_info("%s: param err: pile=%p needed=%d id=0x%04x\n",
> > + __func__, pile, needed, id);
> > + return I40E_ERR_PARAM;
>
> Since there is absolutely no context passed into these helper routines,
> the log messages are less useful than they could be. If you did this
> right you could use netdev_info() or dev_info() here.
Perhaps we went a little too far in trying for loosely coupled code? We'll add enough context for dev_info().
>
> > +void i40e_pf_reset_stats(struct i40e_pf *pf)
> > +{
> > + memset(&pf->stats, 0, sizeof(struct i40e_hw_port_stats));
> > + memset(&pf->stats_offsets, 0, sizeof(struct i40e_hw_port_stats));
> > + pf->stat_offsets_loaded = false;
> > +
> > +}
>
> Spurious empty line at end of that function.
Obviously we need another pass at catching these little whitespace issues.
>
> > + flush(hw);
>
> I think this brief and common name is asking for namespace collision
> problems. Maybe name it i40e_flush or i40e_hw_flush or something like
> that.
Good call - thanks.
>
> > +{
> > + int i;
> > + struct i40e_pf *pf = vsi->back;
>
> Please order local variable declarations from longest line to shortest.
Will do, on these and the rest.
[...]
>
> > +static u8 i40e_dcb_get_num_tc(struct i40e_dcbx_config *dcbcfg)
> > +{
> > + int num_tc = 0, i;
> > + /* Scan the ETS Config Priority Table to find
> > + * traffic class enabled for a given priority
> > + * and use the traffic class index to get the
> > + * number of traffic classes enabled
> > + */
>
> Please put an empty line between the local variables and
> the rest of the function.
Yep.
[...]
>
> > +static inline int i40e_prev_power_of_2(int n)
> > +{
> > + int p = n;
> > + --p;
> > + p |= p >> 1;
> > + p |= p >> 2;
> > + p |= p >> 4;
> > + p |= p >> 8;
> > + p |= p >> 16;
> > + if (p == (n - 1))
> > + return n; /* it was already a power of 2 */
> > + p >>= 1;
> > + return ++p;
> > +}
>
> I think something using rounddown_pow_of_two() would accomplish this.
>
> Perhaps:
>
> if (!is_power_of_2(x))
> x = rounddown_pow_of_two(x);
Oh, didn't know about that one, thanks!
sln
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Santosh Shilimkar @ 2013-08-23 16:56 UTC (permalink / raw)
To: Daniel Mack
Cc: mugunthanvnm, sergei.shtylyov, d-gerlach, netdev, nsekhar,
vaibhav.bedia, devicetree, bcousson, ujhelyi.m, linux-omap, davem,
linux-arm-kernel
In-Reply-To: <52178E20.5040404@gmail.com>
On Friday 23 August 2013 12:30 PM, Daniel Mack wrote:
> On 23.08.2013 16:23, Santosh Shilimkar wrote:
>> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>
>>> +static const struct of_device_id cpsw_of_mtable[] = {
>>> + {
>>> + .compatible = "ti,am3352-cpsw",
>>
>> I didn't notice this earlier, but can't you use the IP version
>> as a compatible instead of using a SOC name. Whats really SOC specific
>> on this IP ? Sorry i have missed any earlier discussion on this but
>> this approach doesn't seem good. Its like adding SOC checks in the
>> driver subsystem.
>
> As I already mentioned in the cover letter and in the commit message, I
> just don't know which criteria makes most sense here.
>
> On a general note, I would say that chances that this exactly IP core
> with the same version number will appear on some other silicon which
> doesn't support the control mode register in an AM33xx fashion, is not
> necessarily negligible.
>
> So what that new compatible string denotes is the cpsw in a version as
> found on am3352 SoCs, which is actually exactly what it does.
>
> I don't have a strong opinion here, but see your point. I just don't
> have a better idea on how to treat that.
>
So just stick the IP version or call it cpsw-v1... cpsw-v2 etc.
That way if in future if someone uses those features, they can use
this compatible if they don't they use the one which suites that
SOC.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH v4 4/5] net: ethernet: cpsw: add support for hardware interface mode config
From: Mugunthan V N @ 2013-08-23 16:50 UTC (permalink / raw)
To: Daniel Mack
Cc: netdev, bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
devicetree
In-Reply-To: <1377267365-24057-5-git-send-email-zonque@gmail.com>
On Friday 23 August 2013 07:46 PM, Daniel Mack wrote:
> The cpsw currently lacks code to properly set up the hardware interface
> mode on AM33xx. Other platforms might be equally affected.
>
> Usually, the bootloader will configure the control module register, so
> probably that's why such support wasn't needed in the past. In suspend
> mode though, this register is modified, and so it needs reprogramming
> after resume.
>
> This patch adds code that makes use of the previously added and optional
> support for passing the control mode register, and configures the
> correct register bits when the slave is opened.
>
> The AM33xx also has a bit for each slave to configure the RMII reference
> clock direction. Setting it is now supported by a per-slave DT property.
>
> This code path introducted by this patch is currently exclusive for
> am33xx.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
> Documentation/devicetree/bindings/net/cpsw.txt | 2 +
> drivers/net/ethernet/ti/cpsw.c | 61 ++++++++++++++++++++++++++
> drivers/net/ethernet/ti/cpsw.h | 1 +
> 3 files changed, 64 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> index b717458..0895a51 100644
> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -34,6 +34,8 @@ Required properties:
> - phy_id : Specifies slave phy id
> - phy-mode : The interface between the SoC and the PHY (a string
> that of_get_phy_mode() can understand)
> +- ti,rmii-clock-ext : If present, the driver will configure the RMII
> + interface to external clock usage
> - mac-address : Specifies slave MAC address
>
> Optional properties:
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 73c44cb6..86b8f7a 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -138,6 +138,13 @@ do { \
> #define CPSW_CMINTMAX_INTVL (1000 / CPSW_CMINTMIN_CNT)
> #define CPSW_CMINTMIN_INTVL ((1000 / CPSW_CMINTMAX_CNT) + 1)
>
> +#define AM33XX_GMII_SEL_MODE_MII (0)
> +#define AM33XX_GMII_SEL_MODE_RMII (1)
> +#define AM33XX_GMII_SEL_MODE_RGMII (2)
> +
> +#define AM33XX_GMII_SEL_RMII2_IO_CLK_EN BIT(7)
> +#define AM33XX_GMII_SEL_RMII1_IO_CLK_EN BIT(6)
These defined can go into cpsw.h so that we can maintain platform
defines in one file.
> +
> #define cpsw_enable_irq(priv) \
> do { \
> u32 i; \
> @@ -980,6 +987,56 @@ static inline void cpsw_add_dual_emac_def_ale_entries(
> priv->host_port, ALE_VLAN, slave->port_vlan);
> }
>
> +static void cpsw_set_phy_interface_mode(struct cpsw_slave *slave,
> + struct cpsw_priv *priv)
> +{
> + u32 reg, mask, mode = 0;
Please define each variable in separate line to make it clean.
Please check for IS_ERR(gmii_sel_reg) and return if gmii register is not
specified in DT then the below code will crash when trying to access
gmii_sel_reg.
> +
> + switch (priv->data.hw_type) {
> + case CPSW_TYPE_AM33XX:
> + if (!priv->gmii_sel_reg)
> + break;
> +
> + reg = readl(priv->gmii_sel_reg);
> +
> + if (slave->phy) {
> + switch (slave->phy->interface) {
> + case PHY_INTERFACE_MODE_MII:
> + default:
> + mode = AM33XX_GMII_SEL_MODE_MII;
> + break;
> + case PHY_INTERFACE_MODE_RMII:
> + mode = AM33XX_GMII_SEL_MODE_RMII;
> + break;
> + case PHY_INTERFACE_MODE_RGMII:
You need to take care of other RGMII modes as well, for your info
AM335xevm phy mode is "rgmii-txid"
Regards
Mugunthan V N
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Mugunthan V N @ 2013-08-23 16:45 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Daniel Mack, netdev, bcousson, nsekhar, sergei.shtylyov, davem,
ujhelyi.m, vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
devicetree
In-Reply-To: <52177052.1030308@ti.com>
On Friday 23 August 2013 07:53 PM, Santosh Shilimkar wrote:
> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>> In order to support features that are specific to the AM335x IP, we have
>> to add hardware types and another compatible string.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> ---
>> Documentation/devicetree/bindings/net/cpsw.txt | 3 ++-
>> drivers/net/ethernet/ti/cpsw.c | 32 ++++++++++++++++++++------
>> drivers/net/ethernet/ti/cpsw.h | 1 +
>> 3 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
>> index 4e5ca54..b717458 100644
>> --- a/Documentation/devicetree/bindings/net/cpsw.txt
>> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
>> @@ -2,7 +2,8 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
>> ------------------------------------------------------
>>
>> Required properties:
>> -- compatible : Should be "ti,cpsw"
>> +- compatible : Should be "ti,cpsw" for generic cpsw support, or
>> + "ti,am3352-cpsw" for AM3352 SoCs
>> - reg : physical base address and size of the cpsw
>> registers map.
>> An optional third memory region can be supplied if
>> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
>> index 7a25ff4..73c44cb6 100644
>> --- a/drivers/net/ethernet/ti/cpsw.c
>> +++ b/drivers/net/ethernet/ti/cpsw.c
>> @@ -155,6 +155,11 @@ do { \
>> ((priv->data.dual_emac) ? priv->emac_port : \
>> priv->data.active_slave)
>>
>> +enum {
>> + CPSW_TYPE_GENERIC,
>> + CPSW_TYPE_AM33XX
>> +};
>> +
>> static int debug_level;
>> module_param(debug_level, int, 0);
>> MODULE_PARM_DESC(debug_level, "cpsw debug level (NETIF_MSG bits)");
>> @@ -1692,17 +1697,36 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
>> slave->port_vlan = data->dual_emac_res_vlan;
>> }
>>
>> +static const struct of_device_id cpsw_of_mtable[] = {
>> + {
>> + .compatible = "ti,am3352-cpsw",
> I didn't notice this earlier, but can't you use the IP version
> as a compatible instead of using a SOC name. Whats really SOC specific
> on this IP ? Sorry i have missed any earlier discussion on this but
> this approach doesn't seem good. Its like adding SOC checks in the
> driver subsystem.
>
>
But the same IP can be used in different SoC as well where the control
register may be different as per the Silicon Integration team's decision?
Ideally there should be a separate control module driver so that it can
take care of different SoC related needs.
Regards
Mugunthan V N
^ permalink raw reply
* Re: [PATCH v2 4/5] net: ethernet: cpsw: add support for hardware interface mode config
From: Mugunthan V N @ 2013-08-23 16:38 UTC (permalink / raw)
To: Daniel Mack
Cc: netdev, nsekhar, sergei.shtylyov, davem, ujhelyi.m, vaibhav.bedia,
d-gerlach, linux-arm-kernel, linux-omap, devicetree
In-Reply-To: <1377247417-27386-5-git-send-email-zonque@gmail.com>
On Friday 23 August 2013 02:13 PM, Daniel Mack wrote:
> The cpsw currently lacks code to properly set up the hardware interface
> mode on AM33xx. Other platforms might be equally affected.
>
> Usually, the bootloader will configure the control module register, so
> probably that's why such support wasn't needed in the past. In suspend
> mode though, this register is modified, and so it needs reprogramming
> after resume.
>
> This patch adds code that makes use of the previously added and optional
> support for passing the control mode register, and configures the
> correct register bits when the slave is opened.
>
> The AM33xx also has a bit for each slave to configure the RMII reference
> clock direction. Setting it is now supported by a per-slave DT property.
>
> This code path introducted by this patch is currently exclusive for
> am33xx.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
> Documentation/devicetree/bindings/net/cpsw.txt | 2 +
> drivers/net/ethernet/ti/cpsw.c | 61 ++++++++++++++++++++++++++
> drivers/net/ethernet/ti/cpsw.h | 1 +
> 3 files changed, 64 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> index b717458..0895a51 100644
> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -34,6 +34,8 @@ Required properties:
> - phy_id : Specifies slave phy id
> - phy-mode : The interface between the SoC and the PHY (a string
> that of_get_phy_mode() can understand)
> +- ti,rmii-clock-ext : If present, the driver will configure the RMII
> + interface to external clock usage
> - mac-address : Specifies slave MAC address
>
> Optional properties:
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 2e19de0..6e36f49 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -138,6 +138,13 @@ do { \
> #define CPSW_CMINTMAX_INTVL (1000 / CPSW_CMINTMIN_CNT)
> #define CPSW_CMINTMIN_INTVL ((1000 / CPSW_CMINTMAX_CNT) + 1)
>
> +#define AM33XX_GMII_SEL_MODE_MII (0)
> +#define AM33XX_GMII_SEL_MODE_RMII (1)
> +#define AM33XX_GMII_SEL_MODE_RGMII (2)
> +
> +#define AM33XX_GMII_SEL_RMII2_IO_CLK_EN BIT(7)
> +#define AM33XX_GMII_SEL_RMII1_IO_CLK_EN BIT(6)
These defined can go into cpsw.h so that we can maintain platform
defines in one file.
> +
> #define cpsw_enable_irq(priv) \
> do { \
> u32 i; \
> @@ -980,6 +987,56 @@ static inline void cpsw_add_dual_emac_def_ale_entries(
> priv->host_port, ALE_VLAN, slave->port_vlan);
> }
>
> +static void cpsw_set_phy_interface_mode(struct cpsw_slave *slave,
> + struct cpsw_priv *priv)
> +{
> + u32 reg, mask, mode = 0;
> +
Please define each variable in separate line to make it clean.
Please check for IS_ERR(gmii_sel_reg) and return if gmii register is not
specified in DT then the below code will crash when trying to access
gmii_sel_reg.
> + switch (priv->data.hw_type) {
> + case CPSW_TYPE_AM33XX:
> + if (!priv->gmii_sel_reg)
> + break;
> +
> + reg = readl(priv->gmii_sel_reg);
> +
> + if (slave->phy) {
> + switch (slave->phy->interface) {
> + case PHY_INTERFACE_MODE_MII:
> + default:
> + mode = AM33XX_GMII_SEL_MODE_MII;
> + break;
> + case PHY_INTERFACE_MODE_RMII:
> + mode = AM33XX_GMII_SEL_MODE_RMII;
> + break;
> + case PHY_INTERFACE_MODE_RGMII:
You need to take care of other RGMII modes as well, for your info
AM335xevm phy mode is "rgmii-txid"
Regards
Mugunthan V N
^ permalink raw reply
* [PATCH] VMXNET3: Add support for virtual IOMMU
From: Andy King @ 2013-08-23 16:33 UTC (permalink / raw)
To: netdev, linux-kernel, virtualization; +Cc: gregkh, davem, pv-drivers, Andy King
This patch adds support for virtual IOMMU to the vmxnet3 module. We
switch to DMA consistent mappings for anything we pass to the device.
There were a few places where we already did this, but using pci_blah();
these have been fixed to use dma_blah(), along with all new occurrences
where we've replaced kmalloc() and friends.
Also fix two small bugs:
1) use after free of rq->buf_info in vmxnet3_rq_destroy()
2) a cpu_to_le32() that should have been a cpu_to_le64()
Acked-by: George Zhang <georgezhang@vmware.com>
Acked-by: Aditya Sarwade <asarwade@vmware.com>
Signed-off-by: Andy King <acking@vmware.com>
---
drivers/net/vmxnet3/vmxnet3_drv.c | 211 +++++++++++++++++++++++--------------
drivers/net/vmxnet3/vmxnet3_int.h | 10 ++-
2 files changed, 138 insertions(+), 83 deletions(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 55a62ca..7e2788c 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -313,10 +313,10 @@ vmxnet3_unmap_tx_buf(struct vmxnet3_tx_buf_info *tbi,
struct pci_dev *pdev)
{
if (tbi->map_type == VMXNET3_MAP_SINGLE)
- pci_unmap_single(pdev, tbi->dma_addr, tbi->len,
+ dma_unmap_single(&pdev->dev, tbi->dma_addr, tbi->len,
PCI_DMA_TODEVICE);
else if (tbi->map_type == VMXNET3_MAP_PAGE)
- pci_unmap_page(pdev, tbi->dma_addr, tbi->len,
+ dma_unmap_page(&pdev->dev, tbi->dma_addr, tbi->len,
PCI_DMA_TODEVICE);
else
BUG_ON(tbi->map_type != VMXNET3_MAP_NONE);
@@ -429,25 +429,29 @@ vmxnet3_tq_destroy(struct vmxnet3_tx_queue *tq,
struct vmxnet3_adapter *adapter)
{
if (tq->tx_ring.base) {
- pci_free_consistent(adapter->pdev, tq->tx_ring.size *
- sizeof(struct Vmxnet3_TxDesc),
- tq->tx_ring.base, tq->tx_ring.basePA);
+ dma_free_coherent(&adapter->pdev->dev, tq->tx_ring.size *
+ sizeof(struct Vmxnet3_TxDesc),
+ tq->tx_ring.base, tq->tx_ring.basePA);
tq->tx_ring.base = NULL;
}
if (tq->data_ring.base) {
- pci_free_consistent(adapter->pdev, tq->data_ring.size *
- sizeof(struct Vmxnet3_TxDataDesc),
- tq->data_ring.base, tq->data_ring.basePA);
+ dma_free_coherent(&adapter->pdev->dev, tq->data_ring.size *
+ sizeof(struct Vmxnet3_TxDataDesc),
+ tq->data_ring.base, tq->data_ring.basePA);
tq->data_ring.base = NULL;
}
if (tq->comp_ring.base) {
- pci_free_consistent(adapter->pdev, tq->comp_ring.size *
- sizeof(struct Vmxnet3_TxCompDesc),
- tq->comp_ring.base, tq->comp_ring.basePA);
+ dma_free_coherent(&adapter->pdev->dev, tq->comp_ring.size *
+ sizeof(struct Vmxnet3_TxCompDesc),
+ tq->comp_ring.base, tq->comp_ring.basePA);
tq->comp_ring.base = NULL;
}
- kfree(tq->buf_info);
- tq->buf_info = NULL;
+ if (tq->buf_info) {
+ dma_free_coherent(&adapter->pdev->dev,
+ tq->tx_ring.size * sizeof(tq->buf_info[0]),
+ tq->buf_info, tq->buf_info_pa);
+ tq->buf_info = NULL;
+ }
}
@@ -496,37 +500,38 @@ static int
vmxnet3_tq_create(struct vmxnet3_tx_queue *tq,
struct vmxnet3_adapter *adapter)
{
+ size_t sz;
+
BUG_ON(tq->tx_ring.base || tq->data_ring.base ||
tq->comp_ring.base || tq->buf_info);
- tq->tx_ring.base = pci_alloc_consistent(adapter->pdev, tq->tx_ring.size
- * sizeof(struct Vmxnet3_TxDesc),
- &tq->tx_ring.basePA);
+ tq->tx_ring.base = dma_alloc_coherent(&adapter->pdev->dev,
+ tq->tx_ring.size * sizeof(struct Vmxnet3_TxDesc),
+ &tq->tx_ring.basePA, GFP_KERNEL);
if (!tq->tx_ring.base) {
netdev_err(adapter->netdev, "failed to allocate tx ring\n");
goto err;
}
- tq->data_ring.base = pci_alloc_consistent(adapter->pdev,
- tq->data_ring.size *
- sizeof(struct Vmxnet3_TxDataDesc),
- &tq->data_ring.basePA);
+ tq->data_ring.base = dma_alloc_coherent(&adapter->pdev->dev,
+ tq->data_ring.size * sizeof(struct Vmxnet3_TxDataDesc),
+ &tq->data_ring.basePA, GFP_KERNEL);
if (!tq->data_ring.base) {
netdev_err(adapter->netdev, "failed to allocate data ring\n");
goto err;
}
- tq->comp_ring.base = pci_alloc_consistent(adapter->pdev,
- tq->comp_ring.size *
- sizeof(struct Vmxnet3_TxCompDesc),
- &tq->comp_ring.basePA);
+ tq->comp_ring.base = dma_alloc_coherent(&adapter->pdev->dev,
+ tq->comp_ring.size * sizeof(struct Vmxnet3_TxCompDesc),
+ &tq->comp_ring.basePA, GFP_KERNEL);
if (!tq->comp_ring.base) {
netdev_err(adapter->netdev, "failed to allocate tx comp ring\n");
goto err;
}
- tq->buf_info = kcalloc(tq->tx_ring.size, sizeof(tq->buf_info[0]),
- GFP_KERNEL);
+ sz = tq->tx_ring.size * sizeof(tq->buf_info[0]);
+ tq->buf_info = dma_zalloc_coherent(&adapter->pdev->dev, sz,
+ &tq->buf_info_pa, GFP_KERNEL);
if (!tq->buf_info)
goto err;
@@ -578,7 +583,8 @@ vmxnet3_rq_alloc_rx_buf(struct vmxnet3_rx_queue *rq, u32 ring_idx,
break;
}
- rbi->dma_addr = pci_map_single(adapter->pdev,
+ rbi->dma_addr = dma_map_single(
+ &adapter->pdev->dev,
rbi->skb->data, rbi->len,
PCI_DMA_FROMDEVICE);
} else {
@@ -595,7 +601,8 @@ vmxnet3_rq_alloc_rx_buf(struct vmxnet3_rx_queue *rq, u32 ring_idx,
rq->stats.rx_buf_alloc_failure++;
break;
}
- rbi->dma_addr = pci_map_page(adapter->pdev,
+ rbi->dma_addr = dma_map_page(
+ &adapter->pdev->dev,
rbi->page, 0, PAGE_SIZE,
PCI_DMA_FROMDEVICE);
} else {
@@ -705,7 +712,7 @@ vmxnet3_map_pkt(struct sk_buff *skb, struct vmxnet3_tx_ctx *ctx,
tbi = tq->buf_info + tq->tx_ring.next2fill;
tbi->map_type = VMXNET3_MAP_SINGLE;
- tbi->dma_addr = pci_map_single(adapter->pdev,
+ tbi->dma_addr = dma_map_single(&adapter->pdev->dev,
skb->data + buf_offset, buf_size,
PCI_DMA_TODEVICE);
@@ -1221,7 +1228,8 @@ vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
goto rcd_done;
}
- pci_unmap_single(adapter->pdev, rbi->dma_addr, rbi->len,
+ dma_unmap_single(&adapter->pdev->dev, rbi->dma_addr,
+ rbi->len,
PCI_DMA_FROMDEVICE);
#ifdef VMXNET3_RSS
@@ -1233,7 +1241,7 @@ vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
/* Immediate refill */
rbi->skb = new_skb;
- rbi->dma_addr = pci_map_single(adapter->pdev,
+ rbi->dma_addr = dma_map_single(&adapter->pdev->dev,
rbi->skb->data, rbi->len,
PCI_DMA_FROMDEVICE);
rxd->addr = cpu_to_le64(rbi->dma_addr);
@@ -1267,7 +1275,7 @@ vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
}
if (rcd->len) {
- pci_unmap_page(adapter->pdev,
+ dma_unmap_page(&adapter->pdev->dev,
rbi->dma_addr, rbi->len,
PCI_DMA_FROMDEVICE);
@@ -1276,7 +1284,8 @@ vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
/* Immediate refill */
rbi->page = new_page;
- rbi->dma_addr = pci_map_page(adapter->pdev, rbi->page,
+ rbi->dma_addr = dma_map_page(&adapter->pdev->dev,
+ rbi->page,
0, PAGE_SIZE,
PCI_DMA_FROMDEVICE);
rxd->addr = cpu_to_le64(rbi->dma_addr);
@@ -1352,13 +1361,13 @@ vmxnet3_rq_cleanup(struct vmxnet3_rx_queue *rq,
if (rxd->btype == VMXNET3_RXD_BTYPE_HEAD &&
rq->buf_info[ring_idx][i].skb) {
- pci_unmap_single(adapter->pdev, rxd->addr,
+ dma_unmap_single(&adapter->pdev->dev, rxd->addr,
rxd->len, PCI_DMA_FROMDEVICE);
dev_kfree_skb(rq->buf_info[ring_idx][i].skb);
rq->buf_info[ring_idx][i].skb = NULL;
} else if (rxd->btype == VMXNET3_RXD_BTYPE_BODY &&
rq->buf_info[ring_idx][i].page) {
- pci_unmap_page(adapter->pdev, rxd->addr,
+ dma_unmap_page(&adapter->pdev->dev, rxd->addr,
rxd->len, PCI_DMA_FROMDEVICE);
put_page(rq->buf_info[ring_idx][i].page);
rq->buf_info[ring_idx][i].page = NULL;
@@ -1400,25 +1409,31 @@ static void vmxnet3_rq_destroy(struct vmxnet3_rx_queue *rq,
}
- kfree(rq->buf_info[0]);
-
for (i = 0; i < 2; i++) {
if (rq->rx_ring[i].base) {
- pci_free_consistent(adapter->pdev, rq->rx_ring[i].size
- * sizeof(struct Vmxnet3_RxDesc),
- rq->rx_ring[i].base,
- rq->rx_ring[i].basePA);
+ dma_free_coherent(&adapter->pdev->dev,
+ rq->rx_ring[i].size
+ * sizeof(struct Vmxnet3_RxDesc),
+ rq->rx_ring[i].base,
+ rq->rx_ring[i].basePA);
rq->rx_ring[i].base = NULL;
}
rq->buf_info[i] = NULL;
}
if (rq->comp_ring.base) {
- pci_free_consistent(adapter->pdev, rq->comp_ring.size *
- sizeof(struct Vmxnet3_RxCompDesc),
- rq->comp_ring.base, rq->comp_ring.basePA);
+ dma_free_coherent(&adapter->pdev->dev, rq->comp_ring.size
+ * sizeof(struct Vmxnet3_RxCompDesc),
+ rq->comp_ring.base, rq->comp_ring.basePA);
rq->comp_ring.base = NULL;
}
+
+ if (rq->buf_info[0]) {
+ size_t sz = sizeof(struct vmxnet3_rx_buf_info) *
+ (rq->rx_ring[0].size + rq->rx_ring[1].size);
+ dma_free_coherent(&adapter->pdev->dev, sz, rq->buf_info[0],
+ rq->buf_info_pa);
+ }
}
@@ -1503,8 +1518,10 @@ vmxnet3_rq_create(struct vmxnet3_rx_queue *rq, struct vmxnet3_adapter *adapter)
for (i = 0; i < 2; i++) {
sz = rq->rx_ring[i].size * sizeof(struct Vmxnet3_RxDesc);
- rq->rx_ring[i].base = pci_alloc_consistent(adapter->pdev, sz,
- &rq->rx_ring[i].basePA);
+ rq->rx_ring[i].base = dma_alloc_coherent(
+ &adapter->pdev->dev, sz,
+ &rq->rx_ring[i].basePA,
+ GFP_KERNEL);
if (!rq->rx_ring[i].base) {
netdev_err(adapter->netdev,
"failed to allocate rx ring %d\n", i);
@@ -1513,8 +1530,9 @@ vmxnet3_rq_create(struct vmxnet3_rx_queue *rq, struct vmxnet3_adapter *adapter)
}
sz = rq->comp_ring.size * sizeof(struct Vmxnet3_RxCompDesc);
- rq->comp_ring.base = pci_alloc_consistent(adapter->pdev, sz,
- &rq->comp_ring.basePA);
+ rq->comp_ring.base = dma_alloc_coherent(&adapter->pdev->dev, sz,
+ &rq->comp_ring.basePA,
+ GFP_KERNEL);
if (!rq->comp_ring.base) {
netdev_err(adapter->netdev, "failed to allocate rx comp ring\n");
goto err;
@@ -1522,7 +1540,8 @@ vmxnet3_rq_create(struct vmxnet3_rx_queue *rq, struct vmxnet3_adapter *adapter)
sz = sizeof(struct vmxnet3_rx_buf_info) * (rq->rx_ring[0].size +
rq->rx_ring[1].size);
- bi = kzalloc(sz, GFP_KERNEL);
+ bi = dma_zalloc_coherent(&adapter->pdev->dev, sz, &rq->buf_info_pa,
+ GFP_KERNEL);
if (!bi)
goto err;
@@ -2005,6 +2024,7 @@ vmxnet3_set_mc(struct net_device *netdev)
struct Vmxnet3_RxFilterConf *rxConf =
&adapter->shared->devRead.rxFilterConf;
u8 *new_table = NULL;
+ dma_addr_t new_table_pa = 0;
u32 new_mode = VMXNET3_RXM_UCAST;
if (netdev->flags & IFF_PROMISC) {
@@ -2028,8 +2048,12 @@ vmxnet3_set_mc(struct net_device *netdev)
new_mode |= VMXNET3_RXM_MCAST;
rxConf->mfTableLen = cpu_to_le16(
netdev_mc_count(netdev) * ETH_ALEN);
- rxConf->mfTablePA = cpu_to_le64(virt_to_phys(
- new_table));
+ new_table_pa = dma_map_single(
+ &adapter->pdev->dev,
+ new_table,
+ rxConf->mfTableLen,
+ PCI_DMA_TODEVICE);
+ rxConf->mfTablePA = cpu_to_le64(new_table_pa);
} else {
netdev_info(netdev, "failed to copy mcast list"
", setting ALL_MULTI\n");
@@ -2056,7 +2080,11 @@ vmxnet3_set_mc(struct net_device *netdev)
VMXNET3_CMD_UPDATE_MAC_FILTERS);
spin_unlock_irqrestore(&adapter->cmd_lock, flags);
- kfree(new_table);
+ if (new_table) {
+ dma_unmap_single(&adapter->pdev->dev, new_table_pa,
+ rxConf->mfTableLen, PCI_DMA_TODEVICE);
+ kfree(new_table);
+ }
}
void
@@ -2096,7 +2124,7 @@ vmxnet3_setup_driver_shared(struct vmxnet3_adapter *adapter)
devRead->misc.driverInfo.vmxnet3RevSpt = cpu_to_le32(1);
devRead->misc.driverInfo.uptVerSpt = cpu_to_le32(1);
- devRead->misc.ddPA = cpu_to_le64(virt_to_phys(adapter));
+ devRead->misc.ddPA = cpu_to_le64(adapter->adapter_pa);
devRead->misc.ddLen = cpu_to_le32(sizeof(struct vmxnet3_adapter));
/* set up feature flags */
@@ -2125,7 +2153,7 @@ vmxnet3_setup_driver_shared(struct vmxnet3_adapter *adapter)
tqc->txRingBasePA = cpu_to_le64(tq->tx_ring.basePA);
tqc->dataRingBasePA = cpu_to_le64(tq->data_ring.basePA);
tqc->compRingBasePA = cpu_to_le64(tq->comp_ring.basePA);
- tqc->ddPA = cpu_to_le64(virt_to_phys(tq->buf_info));
+ tqc->ddPA = cpu_to_le64(tq->buf_info_pa);
tqc->txRingSize = cpu_to_le32(tq->tx_ring.size);
tqc->dataRingSize = cpu_to_le32(tq->data_ring.size);
tqc->compRingSize = cpu_to_le32(tq->comp_ring.size);
@@ -2143,8 +2171,7 @@ vmxnet3_setup_driver_shared(struct vmxnet3_adapter *adapter)
rqc->rxRingBasePA[0] = cpu_to_le64(rq->rx_ring[0].basePA);
rqc->rxRingBasePA[1] = cpu_to_le64(rq->rx_ring[1].basePA);
rqc->compRingBasePA = cpu_to_le64(rq->comp_ring.basePA);
- rqc->ddPA = cpu_to_le64(virt_to_phys(
- rq->buf_info));
+ rqc->ddPA = cpu_to_le64(rq->buf_info_pa);
rqc->rxRingSize[0] = cpu_to_le32(rq->rx_ring[0].size);
rqc->rxRingSize[1] = cpu_to_le32(rq->rx_ring[1].size);
rqc->compRingSize = cpu_to_le32(rq->comp_ring.size);
@@ -2184,8 +2211,9 @@ vmxnet3_setup_driver_shared(struct vmxnet3_adapter *adapter)
i, adapter->num_rx_queues);
devRead->rssConfDesc.confVer = 1;
- devRead->rssConfDesc.confLen = sizeof(*rssConf);
- devRead->rssConfDesc.confPA = virt_to_phys(rssConf);
+ devRead->rssConfDesc.confLen = cpu_to_le32(sizeof(*rssConf));
+ devRead->rssConfDesc.confPA =
+ cpu_to_le64(adapter->rss_conf_pa);
}
#endif /* VMXNET3_RSS */
@@ -2948,9 +2976,13 @@ vmxnet3_probe_device(struct pci_dev *pdev,
adapter->pdev = pdev;
spin_lock_init(&adapter->cmd_lock);
- adapter->shared = pci_alloc_consistent(adapter->pdev,
- sizeof(struct Vmxnet3_DriverShared),
- &adapter->shared_pa);
+ adapter->adapter_pa = dma_map_single(&adapter->pdev->dev, adapter,
+ sizeof(struct vmxnet3_adapter),
+ PCI_DMA_TODEVICE);
+ adapter->shared = dma_alloc_coherent(
+ &adapter->pdev->dev,
+ sizeof(struct Vmxnet3_DriverShared),
+ &adapter->shared_pa, GFP_KERNEL);
if (!adapter->shared) {
dev_err(&pdev->dev, "Failed to allocate memory\n");
err = -ENOMEM;
@@ -2963,8 +2995,9 @@ vmxnet3_probe_device(struct pci_dev *pdev,
size = sizeof(struct Vmxnet3_TxQueueDesc) * adapter->num_tx_queues;
size += sizeof(struct Vmxnet3_RxQueueDesc) * adapter->num_rx_queues;
- adapter->tqd_start = pci_alloc_consistent(adapter->pdev, size,
- &adapter->queue_desc_pa);
+ adapter->tqd_start = dma_alloc_coherent(&adapter->pdev->dev, size,
+ &adapter->queue_desc_pa,
+ GFP_KERNEL);
if (!adapter->tqd_start) {
dev_err(&pdev->dev, "Failed to allocate memory\n");
@@ -2974,7 +3007,10 @@ vmxnet3_probe_device(struct pci_dev *pdev,
adapter->rqd_start = (struct Vmxnet3_RxQueueDesc *)(adapter->tqd_start +
adapter->num_tx_queues);
- adapter->pm_conf = kmalloc(sizeof(struct Vmxnet3_PMConf), GFP_KERNEL);
+ adapter->pm_conf = dma_alloc_coherent(&adapter->pdev->dev,
+ sizeof(struct Vmxnet3_PMConf),
+ &adapter->pm_conf_pa,
+ GFP_KERNEL);
if (adapter->pm_conf == NULL) {
err = -ENOMEM;
goto err_alloc_pm;
@@ -2982,7 +3018,10 @@ vmxnet3_probe_device(struct pci_dev *pdev,
#ifdef VMXNET3_RSS
- adapter->rss_conf = kmalloc(sizeof(struct UPT1_RSSConf), GFP_KERNEL);
+ adapter->rss_conf = dma_alloc_coherent(&adapter->pdev->dev,
+ sizeof(struct UPT1_RSSConf),
+ &adapter->rss_conf_pa,
+ GFP_KERNEL);
if (adapter->rss_conf == NULL) {
err = -ENOMEM;
goto err_alloc_rss;
@@ -3077,17 +3116,22 @@ err_ver:
vmxnet3_free_pci_resources(adapter);
err_alloc_pci:
#ifdef VMXNET3_RSS
- kfree(adapter->rss_conf);
+ dma_free_coherent(&adapter->pdev->dev, sizeof(struct UPT1_RSSConf),
+ adapter->rss_conf, adapter->rss_conf_pa);
err_alloc_rss:
#endif
- kfree(adapter->pm_conf);
+ dma_free_coherent(&adapter->pdev->dev, sizeof(struct Vmxnet3_PMConf),
+ adapter->pm_conf, adapter->pm_conf_pa);
err_alloc_pm:
- pci_free_consistent(adapter->pdev, size, adapter->tqd_start,
- adapter->queue_desc_pa);
+ dma_free_coherent(&adapter->pdev->dev, size, adapter->tqd_start,
+ adapter->queue_desc_pa);
err_alloc_queue_desc:
- pci_free_consistent(adapter->pdev, sizeof(struct Vmxnet3_DriverShared),
- adapter->shared, adapter->shared_pa);
+ dma_free_coherent(&adapter->pdev->dev,
+ sizeof(struct Vmxnet3_DriverShared),
+ adapter->shared, adapter->shared_pa);
err_alloc_shared:
+ dma_unmap_single(&adapter->pdev->dev, adapter->adapter_pa,
+ sizeof(struct vmxnet3_adapter), PCI_DMA_TODEVICE);
pci_set_drvdata(pdev, NULL);
free_netdev(netdev);
return err;
@@ -3118,16 +3162,21 @@ vmxnet3_remove_device(struct pci_dev *pdev)
vmxnet3_free_intr_resources(adapter);
vmxnet3_free_pci_resources(adapter);
#ifdef VMXNET3_RSS
- kfree(adapter->rss_conf);
+ dma_free_coherent(&adapter->pdev->dev, sizeof(struct UPT1_RSSConf),
+ adapter->rss_conf, adapter->rss_conf_pa);
#endif
- kfree(adapter->pm_conf);
+ dma_free_coherent(&adapter->pdev->dev, sizeof(struct Vmxnet3_PMConf),
+ adapter->pm_conf, adapter->pm_conf_pa);
size = sizeof(struct Vmxnet3_TxQueueDesc) * adapter->num_tx_queues;
size += sizeof(struct Vmxnet3_RxQueueDesc) * num_rx_queues;
- pci_free_consistent(adapter->pdev, size, adapter->tqd_start,
- adapter->queue_desc_pa);
- pci_free_consistent(adapter->pdev, sizeof(struct Vmxnet3_DriverShared),
- adapter->shared, adapter->shared_pa);
+ dma_free_coherent(&adapter->pdev->dev, size, adapter->tqd_start,
+ adapter->queue_desc_pa);
+ dma_free_coherent(&adapter->pdev->dev,
+ sizeof(struct Vmxnet3_DriverShared),
+ adapter->shared, adapter->shared_pa);
+ dma_unmap_single(&adapter->pdev->dev, adapter->adapter_pa,
+ sizeof(struct vmxnet3_adapter), PCI_DMA_TODEVICE);
free_netdev(netdev);
}
@@ -3227,8 +3276,8 @@ skip_arp:
adapter->shared->devRead.pmConfDesc.confVer = cpu_to_le32(1);
adapter->shared->devRead.pmConfDesc.confLen = cpu_to_le32(sizeof(
*pmConf));
- adapter->shared->devRead.pmConfDesc.confPA = cpu_to_le64(virt_to_phys(
- pmConf));
+ adapter->shared->devRead.pmConfDesc.confPA =
+ cpu_to_le64(adapter->pm_conf_pa);
spin_lock_irqsave(&adapter->cmd_lock, flags);
VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
@@ -3265,8 +3314,8 @@ vmxnet3_resume(struct device *device)
adapter->shared->devRead.pmConfDesc.confVer = cpu_to_le32(1);
adapter->shared->devRead.pmConfDesc.confLen = cpu_to_le32(sizeof(
*pmConf));
- adapter->shared->devRead.pmConfDesc.confPA = cpu_to_le64(virt_to_phys(
- pmConf));
+ adapter->shared->devRead.pmConfDesc.confPA =
+ cpu_to_le64(adapter->pm_conf_pa);
netif_device_attach(netdev);
pci_set_power_state(pdev, PCI_D0);
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index 3541814..a03f358 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -70,10 +70,10 @@
/*
* Version numbers
*/
-#define VMXNET3_DRIVER_VERSION_STRING "1.1.30.0-k"
+#define VMXNET3_DRIVER_VERSION_STRING "1.2.0.0-k"
/* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
-#define VMXNET3_DRIVER_VERSION_NUM 0x01011E00
+#define VMXNET3_DRIVER_VERSION_NUM 0x01020000
#if defined(CONFIG_PCI_MSI)
/* RSS only makes sense if MSI-X is supported. */
@@ -229,6 +229,7 @@ struct vmxnet3_tx_queue {
spinlock_t tx_lock;
struct vmxnet3_cmd_ring tx_ring;
struct vmxnet3_tx_buf_info *buf_info;
+ dma_addr_t buf_info_pa;
struct vmxnet3_tx_data_ring data_ring;
struct vmxnet3_comp_ring comp_ring;
struct Vmxnet3_TxQueueCtrl *shared;
@@ -277,6 +278,7 @@ struct vmxnet3_rx_queue {
u32 qid; /* rqID in RCD for buffer from 1st ring */
u32 qid2; /* rqID in RCD for buffer from 2nd ring */
struct vmxnet3_rx_buf_info *buf_info[2];
+ dma_addr_t buf_info_pa;
struct Vmxnet3_RxQueueCtrl *shared;
struct vmxnet3_rq_driver_stats stats;
} __attribute__((__aligned__(SMP_CACHE_BYTES)));
@@ -353,6 +355,10 @@ struct vmxnet3_adapter {
unsigned long state; /* VMXNET3_STATE_BIT_xxx */
int share_intr;
+
+ dma_addr_t adapter_pa;
+ dma_addr_t pm_conf_pa;
+ dma_addr_t rss_conf_pa;
};
#define VMXNET3_WRITE_BAR0_REG(adapter, reg, val) \
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Sekhar Nori @ 2013-08-23 16:31 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Daniel Mack, netdev, bcousson, sergei.shtylyov, davem, ujhelyi.m,
mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
linux-omap, devicetree
In-Reply-To: <52177052.1030308@ti.com>
On 8/23/2013 7:53 PM, Santosh Shilimkar wrote:
> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>> In order to support features that are specific to the AM335x IP, we have
>> to add hardware types and another compatible string.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> ---
>> Documentation/devicetree/bindings/net/cpsw.txt | 3 ++-
>> drivers/net/ethernet/ti/cpsw.c | 32 ++++++++++++++++++++------
>> drivers/net/ethernet/ti/cpsw.h | 1 +
>> 3 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
>> index 4e5ca54..b717458 100644
>> --- a/Documentation/devicetree/bindings/net/cpsw.txt
>> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
>> @@ -2,7 +2,8 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
>> ------------------------------------------------------
>>
>> Required properties:
>> -- compatible : Should be "ti,cpsw"
>> +- compatible : Should be "ti,cpsw" for generic cpsw support, or
>> + "ti,am3352-cpsw" for AM3352 SoCs
>> - reg : physical base address and size of the cpsw
>> registers map.
>> An optional third memory region can be supplied if
>> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
>> index 7a25ff4..73c44cb6 100644
>> --- a/drivers/net/ethernet/ti/cpsw.c
>> +++ b/drivers/net/ethernet/ti/cpsw.c
>> @@ -155,6 +155,11 @@ do { \
>> ((priv->data.dual_emac) ? priv->emac_port : \
>> priv->data.active_slave)
>>
>> +enum {
>> + CPSW_TYPE_GENERIC,
>> + CPSW_TYPE_AM33XX
>> +};
>> +
>> static int debug_level;
>> module_param(debug_level, int, 0);
>> MODULE_PARM_DESC(debug_level, "cpsw debug level (NETIF_MSG bits)");
>> @@ -1692,17 +1697,36 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
>> slave->port_vlan = data->dual_emac_res_vlan;
>> }
>>
>> +static const struct of_device_id cpsw_of_mtable[] = {
>> + {
>> + .compatible = "ti,am3352-cpsw",
> I didn't notice this earlier, but can't you use the IP version
> as a compatible instead of using a SOC name. Whats really SOC specific
> on this IP ? Sorry i have missed any earlier discussion on this but
> this approach doesn't seem good. Its like adding SOC checks in the
> driver subsystem.
The objection then should not just be to the name of compatible string,
but to handling AM335x control module register in CPSW driver. What
would be the alternate method of doing what Daniel is doing? I vaguely
remember there were attempts to develop an independent control module
driver that were abandoned.
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Daniel Mack @ 2013-08-23 16:30 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: netdev, bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
linux-omap, devicetree
In-Reply-To: <52177052.1030308@ti.com>
On 23.08.2013 16:23, Santosh Shilimkar wrote:
> On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
>> +static const struct of_device_id cpsw_of_mtable[] = {
>> + {
>> + .compatible = "ti,am3352-cpsw",
>
> I didn't notice this earlier, but can't you use the IP version
> as a compatible instead of using a SOC name. Whats really SOC specific
> on this IP ? Sorry i have missed any earlier discussion on this but
> this approach doesn't seem good. Its like adding SOC checks in the
> driver subsystem.
As I already mentioned in the cover letter and in the commit message, I
just don't know which criteria makes most sense here.
On a general note, I would say that chances that this exactly IP core
with the same version number will appear on some other silicon which
doesn't support the control mode register in an AM33xx fashion, is not
necessarily negligible.
So what that new compatible string denotes is the cpsw in a version as
found on am3352 SoCs, which is actually exactly what it does.
I don't have a strong opinion here, but see your point. I just don't
have a better idea on how to treat that.
Daniel
^ permalink raw reply
* Re: [PATCH v4 2/5] net: ethernet: cpsw: add optional third memory region for CONTROL module
From: Daniel Mack @ 2013-08-23 16:21 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: netdev, bcousson, nsekhar, davem, ujhelyi.m, mugunthanvnm,
vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
devicetree
In-Reply-To: <521778DB.7040402@cogentembedded.com>
On 23.08.2013 16:59, Sergei Shtylyov wrote:
> On 23-08-2013 18:16, Daniel Mack wrote:
>> + priv->gmii_sel_reg = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(priv->gmii_sel_reg)) {
>> + dev_err(priv->dev, "unable to map control i/o region\n");
>
> You didn't actually seem to heed my words about error message.
Well yes I did, but only in the check for platform_get_resource(). As
the comment says - we pass on if that memory region is not given, but if
it is given, it also has to be valid.
> And don't you want to do:
>
> res = PTR_ERR(priv->gmii_sel_reg);
Erm, of course. Sorry for that.
Daniel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox