* [PATCH net-next 0/3] sfc: Clean up Siena SR-IOV support in preparation for EF10 SR-IOV support
From: Shradha Shah @ 2014-11-05 12:15 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
This patch series provides a base and clean up for the upcoming
EF10 SRIOV patches.
Shradha Shah (3):
sfc: Move the current VF state from efx_nic into siena_nic_data
sfc: Rename implementations in siena_sriov.c to have a 'siena' prefix
sfc: Add NIC type operations to replace direct calls from efx.c into
siena_sriov.c
drivers/net/ethernet/sfc/ef10.c | 5 +
drivers/net/ethernet/sfc/efx.c | 22 +--
drivers/net/ethernet/sfc/falcon.c | 10 ++
drivers/net/ethernet/sfc/farch.c | 27 ++--
drivers/net/ethernet/sfc/mcdi.c | 2 +-
drivers/net/ethernet/sfc/net_driver.h | 19 +--
drivers/net/ethernet/sfc/nic.h | 112 +++++++++-----
drivers/net/ethernet/sfc/siena.c | 8 +-
drivers/net/ethernet/sfc/siena_sriov.c | 269 ++++++++++++++++++---------------
9 files changed, 281 insertions(+), 193 deletions(-)
^ permalink raw reply
* [PATCH net-next 3/3] sfc: Add NIC type operations to replace direct calls from efx.c into siena_sriov.c
From: Shradha Shah @ 2014-11-05 12:16 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <545A14CD.6040809@solarflare.com>
Also add dummy functions where required to avoid NULL pointer dereference.
Signed-off-by: Shradha Shah <sshah@solarflare.com>
---
drivers/net/ethernet/sfc/ef10.c | 5 +++++
drivers/net/ethernet/sfc/efx.c | 13 +++++++------
drivers/net/ethernet/sfc/falcon.c | 10 ++++++++++
drivers/net/ethernet/sfc/farch.c | 2 +-
drivers/net/ethernet/sfc/net_driver.h | 5 +++++
drivers/net/ethernet/sfc/nic.h | 24 +++++++++++++++++++++++-
drivers/net/ethernet/sfc/siena.c | 5 +++++
7 files changed, 56 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index 002d4cd..ff55a19 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -3688,6 +3688,11 @@ const struct efx_nic_type efx_hunt_a0_nic_type = {
.ptp_write_host_time = efx_ef10_ptp_write_host_time,
.ptp_set_ts_sync_events = efx_ef10_ptp_set_ts_sync_events,
.ptp_set_ts_config = efx_ef10_ptp_set_ts_config,
+ .sriov_init = efx_ef10_sriov_init,
+ .sriov_fini = efx_ef10_sriov_fini,
+ .sriov_mac_address_changed = efx_ef10_sriov_mac_address_changed,
+ .sriov_wanted = efx_ef10_sriov_wanted,
+ .sriov_reset = efx_ef10_sriov_reset,
.revision = EFX_REV_HUNT_A0,
.max_dma_mask = DMA_BIT_MASK(ESF_DZ_TX_KER_BUF_ADDR_WIDTH),
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 2236ffc..b49d048 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -1314,7 +1314,7 @@ static unsigned int efx_wanted_parallelism(struct efx_nic *efx)
/* If RSS is requested for the PF *and* VFs then we can't write RSS
* table entries that are inaccessible to VFs
*/
- if (efx_siena_sriov_wanted(efx) && efx_vf_size(efx) > 1 &&
+ if (efx->type->sriov_wanted(efx) && efx_vf_size(efx) > 1 &&
count > efx_vf_size(efx)) {
netif_warn(efx, probe, efx->net_dev,
"Reducing number of RSS channels from %u to %u for "
@@ -1426,8 +1426,9 @@ static int efx_probe_interrupts(struct efx_nic *efx)
}
/* RSS might be usable on VFs even if it is disabled on the PF */
+
efx->rss_spread = ((efx->n_rx_channels > 1 ||
- !efx_siena_sriov_wanted(efx)) ?
+ !efx->type->sriov_wanted(efx)) ?
efx->n_rx_channels : efx_vf_size(efx));
return 0;
@@ -2167,7 +2168,7 @@ static int efx_set_mac_address(struct net_device *net_dev, void *data)
}
ether_addr_copy(net_dev->dev_addr, new_addr);
- efx_siena_sriov_mac_address_changed(efx);
+ efx->type->sriov_mac_address_changed(efx);
/* Reconfigure the MAC */
mutex_lock(&efx->mac_lock);
@@ -2434,7 +2435,7 @@ int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok)
if (rc)
goto fail;
efx_restore_filters(efx);
- efx_siena_sriov_reset(efx);
+ efx->type->sriov_reset(efx);
mutex_unlock(&efx->mac_lock);
@@ -2827,7 +2828,7 @@ static void efx_pci_remove(struct pci_dev *pci_dev)
efx_disable_interrupts(efx);
rtnl_unlock();
- efx_siena_sriov_fini(efx);
+ efx->type->sriov_fini(efx);
efx_unregister_netdev(efx);
efx_mtd_remove(efx);
@@ -3024,7 +3025,7 @@ static int efx_pci_probe(struct pci_dev *pci_dev,
if (rc)
goto fail4;
- rc = efx_siena_sriov_init(efx);
+ rc = efx->type->sriov_init(efx);
if (rc)
netif_err(efx, probe, efx->net_dev,
"SR-IOV can't be enabled rc %d\n", rc);
diff --git a/drivers/net/ethernet/sfc/falcon.c b/drivers/net/ethernet/sfc/falcon.c
index 1570375..f166c8e 100644
--- a/drivers/net/ethernet/sfc/falcon.c
+++ b/drivers/net/ethernet/sfc/falcon.c
@@ -2766,6 +2766,11 @@ const struct efx_nic_type falcon_a1_nic_type = {
.mtd_write = falcon_mtd_write,
.mtd_sync = falcon_mtd_sync,
#endif
+ .sriov_init = efx_falcon_sriov_init,
+ .sriov_fini = efx_falcon_sriov_fini,
+ .sriov_mac_address_changed = efx_falcon_sriov_mac_address_changed,
+ .sriov_wanted = efx_falcon_sriov_wanted,
+ .sriov_reset = efx_falcon_sriov_reset,
.revision = EFX_REV_FALCON_A1,
.txd_ptr_tbl_base = FR_AA_TX_DESC_PTR_TBL_KER,
@@ -2862,6 +2867,11 @@ const struct efx_nic_type falcon_b0_nic_type = {
.mtd_write = falcon_mtd_write,
.mtd_sync = falcon_mtd_sync,
#endif
+ .sriov_init = efx_falcon_sriov_init,
+ .sriov_fini = efx_falcon_sriov_fini,
+ .sriov_mac_address_changed = efx_falcon_sriov_mac_address_changed,
+ .sriov_wanted = efx_falcon_sriov_wanted,
+ .sriov_reset = efx_falcon_sriov_reset,
.revision = EFX_REV_FALCON_B0,
.txd_ptr_tbl_base = FR_BZ_TX_DESC_PTR_TBL,
diff --git a/drivers/net/ethernet/sfc/farch.c b/drivers/net/ethernet/sfc/farch.c
index f5549a9..7597532 100644
--- a/drivers/net/ethernet/sfc/farch.c
+++ b/drivers/net/ethernet/sfc/farch.c
@@ -1685,7 +1685,7 @@ void efx_farch_dimension_resources(struct efx_nic *efx, unsigned sram_lim_qw)
vi_count = max(efx->n_channels, efx->n_tx_channels * EFX_TXQ_TYPES);
#ifdef CONFIG_SFC_SRIOV
- if (efx_siena_sriov_wanted(efx)) {
+ if (efx->type->sriov_wanted(efx)) {
unsigned vi_dc_entries, buftbl_free, entries_per_vf, vf_limit;
nic_data->vf_buftbl_base = buftbl_min;
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 779a1f5..325dd94 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -1330,6 +1330,11 @@ struct efx_nic_type {
int (*ptp_set_ts_sync_events)(struct efx_nic *efx, bool en, bool temp);
int (*ptp_set_ts_config)(struct efx_nic *efx,
struct hwtstamp_config *init);
+ int (*sriov_init)(struct efx_nic *efx);
+ void (*sriov_fini)(struct efx_nic *efx);
+ void (*sriov_mac_address_changed)(struct efx_nic *efx);
+ bool (*sriov_wanted)(struct efx_nic *efx);
+ void (*sriov_reset)(struct efx_nic *efx);
int revision;
unsigned int txd_ptr_tbl_base;
diff --git a/drivers/net/ethernet/sfc/nic.h b/drivers/net/ethernet/sfc/nic.h
index 1ab3eda..93d10cbb 100644
--- a/drivers/net/ethernet/sfc/nic.h
+++ b/drivers/net/ethernet/sfc/nic.h
@@ -540,6 +540,7 @@ struct efx_ef10_nic_data {
#ifdef CONFIG_SFC_SRIOV
+/* SIENA */
static inline bool efx_siena_sriov_wanted(struct efx_nic *efx)
{
return efx->vf_count != 0;
@@ -568,12 +569,19 @@ void efx_siena_sriov_reset(struct efx_nic *efx);
void efx_siena_sriov_fini(struct efx_nic *efx);
void efx_fini_sriov(void);
+/* EF10 */
+static inline bool efx_ef10_sriov_wanted(struct efx_nic *efx) { return false; }
+static inline int efx_ef10_sriov_init(struct efx_nic *efx) { return -EOPNOTSUPP; }
+static inline void efx_ef10_sriov_mac_address_changed(struct efx_nic *efx) {}
+static inline void efx_ef10_sriov_reset(struct efx_nic *efx) {}
+static inline void efx_ef10_sriov_fini(struct efx_nic *efx) {}
+
#else
+/* SIENA */
static inline bool efx_siena_sriov_wanted(struct efx_nic *efx) { return false; }
static inline bool efx_siena_sriov_enabled(struct efx_nic *efx) { return false; }
static inline unsigned int efx_vf_size(struct efx_nic *efx) { return 0; }
-
static inline int efx_init_sriov(void) { return 0; }
static inline void efx_siena_sriov_probe(struct efx_nic *efx) {}
static inline int efx_siena_sriov_init(struct efx_nic *efx) { return -EOPNOTSUPP; }
@@ -591,8 +599,22 @@ static inline void efx_siena_sriov_reset(struct efx_nic *efx) {}
static inline void efx_siena_sriov_fini(struct efx_nic *efx) {}
static inline void efx_fini_sriov(void) {}
+/* EF10 */
+static inline bool efx_ef10_sriov_wanted(struct efx_nic *efx) { return false; }
+static inline int efx_ef10_sriov_init(struct efx_nic *efx) { return -EOPNOTSUPP; }
+static inline void efx_ef10_sriov_mac_address_changed(struct efx_nic *efx) {}
+static inline void efx_ef10_sriov_reset(struct efx_nic *efx) {}
+static inline void efx_ef10_sriov_fini(struct efx_nic *efx) {}
+
#endif
+/* FALCON */
+static inline bool efx_falcon_sriov_wanted(struct efx_nic *efx) { return false; }
+static inline int efx_falcon_sriov_init(struct efx_nic *efx) { return -EOPNOTSUPP; }
+static inline void efx_falcon_sriov_mac_address_changed(struct efx_nic *efx) {}
+static inline void efx_falcon_sriov_reset(struct efx_nic *efx) {}
+static inline void efx_falcon_sriov_fini(struct efx_nic *efx) {}
+
int efx_siena_sriov_set_vf_mac(struct net_device *dev, int vf, u8 *mac);
int efx_siena_sriov_set_vf_vlan(struct net_device *dev, int vf,
u16 vlan, u8 qos);
diff --git a/drivers/net/ethernet/sfc/siena.c b/drivers/net/ethernet/sfc/siena.c
index cf40d60..3583f02 100644
--- a/drivers/net/ethernet/sfc/siena.c
+++ b/drivers/net/ethernet/sfc/siena.c
@@ -997,6 +997,11 @@ const struct efx_nic_type siena_a0_nic_type = {
#endif
.ptp_write_host_time = siena_ptp_write_host_time,
.ptp_set_ts_config = siena_ptp_set_ts_config,
+ .sriov_init = efx_siena_sriov_init,
+ .sriov_fini = efx_siena_sriov_fini,
+ .sriov_mac_address_changed = efx_siena_sriov_mac_address_changed,
+ .sriov_wanted = efx_siena_sriov_wanted,
+ .sriov_reset = efx_siena_sriov_reset,
.revision = EFX_REV_SIENA_A0,
.txd_ptr_tbl_base = FR_BZ_TX_DESC_PTR_TBL,
^ permalink raw reply related
* Re: [PATCH net] ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs
From: Daniel Borkmann @ 2014-11-05 12:09 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, lw1a2.jing, fw, hannes, netdev, Eric Dumazet,
David L Stevens
In-Reply-To: <1415149616.30247.1.camel@edumazet-glaptop2.roam.corp.google.com>
On 11/05/2014 02:06 AM, Eric Dumazet wrote:
> On Wed, 2014-11-05 at 01:58 +0100, Daniel Borkmann wrote:
>> It has been reported that generating an MLD listener report on
>> devices with large MTUs (e.g. 9000) and a high number of IPv6
>> addresses can trigger a skb_over_panic():
>>
[...]
>>
>> Reported-by: lw1a2.jing@gmail.com
>> Fixes: 72e09ad107e7 ("ipv6: avoid high order allocations")
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Cc: David L Stevens <david.stevens@oracle.com>
>> ---
>> In skb_nofrag_tailroom(), we could actually omit the !skb->dev check,
>> but I leave that rather as a possible cleanup item for net-next.
Thanks for your feedback!
> Hmm... we have a proliferation of such things.
>
> Could you take a look at sk_stream_alloc_skb(), skb->reserved_tailroom,
> and skb_availroom() ?
Ok, here would be a proposal based on skb_availroom():
http://patchwork.ozlabs.org/patch/406959/
Thanks,
Daniel
^ permalink raw reply
* [PATCH net v2] ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs
From: Daniel Borkmann @ 2014-11-05 12:04 UTC (permalink / raw)
To: davem; +Cc: lw1a2.jing, fw, hannes, netdev, Eric Dumazet, David L Stevens
It has been reported that generating an MLD listener report on
devices with large MTUs (e.g. 9000) and a high number of IPv6
addresses can trigger a skb_over_panic():
skbuff: skb_over_panic: text:ffffffff80612a5d len:3776 put:20
head:ffff88046d751000 data:ffff88046d751010 tail:0xed0 end:0xec0
dev:port1
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:100!
invalid opcode: 0000 [#1] SMP
Modules linked in: ixgbe(O)
CPU: 3 PID: 0 Comm: swapper/3 Tainted: G O 3.14.23+ #4
[...]
Call Trace:
<IRQ>
[<ffffffff80578226>] ? skb_put+0x3a/0x3b
[<ffffffff80612a5d>] ? add_grhead+0x45/0x8e
[<ffffffff80612e3a>] ? add_grec+0x394/0x3d4
[<ffffffff80613222>] ? mld_ifc_timer_expire+0x195/0x20d
[<ffffffff8061308d>] ? mld_dad_timer_expire+0x45/0x45
[<ffffffff80255b5d>] ? call_timer_fn.isra.29+0x12/0x68
[<ffffffff80255d16>] ? run_timer_softirq+0x163/0x182
[<ffffffff80250e6f>] ? __do_softirq+0xe0/0x21d
[<ffffffff8025112b>] ? irq_exit+0x4e/0xd3
[<ffffffff802214bb>] ? smp_apic_timer_interrupt+0x3b/0x46
[<ffffffff8063f10a>] ? apic_timer_interrupt+0x6a/0x70
mld_newpack() skb allocations are usually requested with dev->mtu
in size, since commit 72e09ad107e7 ("ipv6: avoid high order allocations")
we have changed the limit in order to be less likely to fail.
However, in MLD/IGMP code, we have some rather ugly AVAILABLE(skb)
macros, which determine if we may end up doing an skb_put() for
adding another record. To avoid possible fragmentation, we check
the skb's tailroom as skb->dev->mtu - skb->len, which is a wrong
assumption as the actual max allocation size can be much smaller.
The IGMP case doesn't have this issue as commit 57e1ab6eaddc
("igmp: refine skb allocations") stores the allocation size in
the cb[].
Set a reserved_tailroom to make it fit into the MTU and use
skb_availroom() helper instead. This also allows to get rid of
igmp_skb_size().
Reported-by: Wei Liu <lw1a2.jing@gmail.com>
Fixes: 72e09ad107e7 ("ipv6: avoid high order allocations")
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: David L Stevens <david.stevens@oracle.com>
---
v1->v2:
- Don't introduce skb_nofrag_tailroom(), but reuse skb_availroom()
as suggested by Eric
net/ipv4/igmp.c | 7 ++-----
net/ipv6/mcast.c | 5 ++---
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index fb70e3e..ad64b94 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -318,8 +318,6 @@ igmp_scount(struct ip_mc_list *pmc, int type, int gdeleted, int sdeleted)
return scount;
}
-#define igmp_skb_size(skb) (*(unsigned int *)((skb)->cb))
-
static struct sk_buff *igmpv3_newpack(struct net_device *dev, int size)
{
struct sk_buff *skb;
@@ -341,7 +339,6 @@ static struct sk_buff *igmpv3_newpack(struct net_device *dev, int size)
return NULL;
}
skb->priority = TC_PRIO_CONTROL;
- igmp_skb_size(skb) = size;
rt = ip_route_output_ports(net, &fl4, NULL, IGMPV3_ALL_MCR, 0,
0, 0,
@@ -354,6 +351,7 @@ static struct sk_buff *igmpv3_newpack(struct net_device *dev, int size)
skb_dst_set(skb, &rt->dst);
skb->dev = dev;
+ skb->reserved_tailroom = max_t(int, 0, skb_end_offset(skb) - dev->mtu);
skb_reserve(skb, hlen);
skb_reset_network_header(skb);
@@ -423,8 +421,7 @@ static struct sk_buff *add_grhead(struct sk_buff *skb, struct ip_mc_list *pmc,
return skb;
}
-#define AVAILABLE(skb) ((skb) ? ((skb)->dev ? igmp_skb_size(skb) - (skb)->len : \
- skb_tailroom(skb)) : 0)
+#define AVAILABLE(skb) ((skb) ? skb_availroom(skb) : 0)
static struct sk_buff *add_grec(struct sk_buff *skb, struct ip_mc_list *pmc,
int type, int gdeleted, int sdeleted)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 9648de2..51237d6 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1571,11 +1571,11 @@ static struct sk_buff *mld_newpack(struct inet6_dev *idev, int size)
/* limit our allocations to order-0 page */
size = min_t(int, size, SKB_MAX_ORDER(0, 0));
skb = sock_alloc_send_skb(sk, size, 1, &err);
-
if (!skb)
return NULL;
skb->priority = TC_PRIO_CONTROL;
+ skb->reserved_tailroom = max_t(int, 0, skb_end_offset(skb) - dev->mtu);
skb_reserve(skb, hlen);
if (__ipv6_get_lladdr(idev, &addr_buf, IFA_F_TENTATIVE)) {
@@ -1690,8 +1690,7 @@ static struct sk_buff *add_grhead(struct sk_buff *skb, struct ifmcaddr6 *pmc,
return skb;
}
-#define AVAILABLE(skb) ((skb) ? ((skb)->dev ? (skb)->dev->mtu - (skb)->len : \
- skb_tailroom(skb)) : 0)
+#define AVAILABLE(skb) ((skb) ? skb_availroom(skb) : 0)
static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
int type, int gdeleted, int sdeleted, int crsend)
--
1.7.11.7
^ permalink raw reply related
* RE: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net onarndale platform
From: Stam, Michel [FINT] @ 2014-11-05 12:04 UTC (permalink / raw)
To: Charles Keepax
Cc: Riku Voipio, davem, linux-usb, netdev, linux-kernel,
linux-samsung-soc
In-Reply-To: <20141104200914.GN23178@opensource.wolfsonmicro.com>
Hello Charles,
After looking around I found the reset value for the 8772 chip, which
seems to be 0x1E1 (ANAR register).
This equates to (according to include/uapi/linux/mii.h)
ADVERTISE_ALL | ADVERTISE_CSMA.
The register only seems to become 0 if the software reset fails.
Unfortunately, this is exactly what I get when the patch is applied;
asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5
asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2,
ASIX AX88772 USB 2.0 Ethernet
asix 1-2:1.0 eth1: Failed to send software reset: ffffffb5
asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-0000:00:1d.0-2,
ASIX AX88772 USB 2.0 Ethernet
A little while after this its 'Failed to enable software MII access'.
The ethernet device fails to see any link or accept any ethtool -s
command.
My device has vid:devid 0b95:772a (ASIX Elec. Corp.).
Can you tell me what device is on the Andale platform, Charles? Same
vendor/device id?
Another thing that bothers me is that dev->mii.advertising seems to
contain the same value, so maybe that can be used instead of a call to
asix_mdio_read(). Can anyone comment on its purpose? Should it be a
shadow copy of the real register or something?
Riku, can you test Charles' patch as well?
Kind regards,
Michel Stam
-----Original Message-----
From: Charles Keepax [mailto:ckeepax@opensource.wolfsonmicro.com]
Sent: Tuesday, November 04, 2014 21:09 PM
To: Stam, Michel [FINT]
Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
linux-samsung-soc@vger.kernel.org
Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
onarndale platform
On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> Hello Riku,
>
> >Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> this case on arndale). This is now a regression of functionality from
> 3.17.
> >
> >I think it would better to revert the change now and with less hurry
> introduce a ethtool fix that doesn't break arndale.
>
> I don't fully agree here;
> I would like to point out that this commit is a revert itself. Fixing
> the armdale will then cause breakage in other implementations, such as
> ours. Blankly reverting breaks other peoples' implementations.
>
> The PHY reset is the thing that breaks ethtool support, so any fix
> that appeases all would have to take existing PHY state into account.
>
> I'm not an expert on the ASIX driver, nor the MII, but I think this is
> the cause;
> drivers/net/usb/asix_devices.c:
> 361 asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> BMCR_RESET);
> 362 asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> 363 ADVERTISE_ALL | ADVERTISE_CSMA);
> 364 mii_nway_restart(&dev->mii);
>
> I would think that the ADVERTISE_ALL is the cause here, as it will
> reset the MII back to default, thus overriding ethtool settings.
> Would an:
> Int reg;
> reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
>
> prior to the offending lines, and then;
>
> 362 asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> 363 reg);
>
> solve the problem for you guys?
If I revert the patch in question and add this in:
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev) {
struct asix_data *data = (struct asix_data *)&dev->data;
int ret, embd_phy;
+ int reg;
u16 rx_ctl;
ret = asix_write_gpio(dev,
@@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
msleep(150);
asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
BMCR_RESET);
- asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
- ADVERTISE_ALL | ADVERTISE_CSMA);
+ reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
+ if (!reg)
+ reg = ADVERTISE_ALL | ADVERTISE_CSMA;
+ asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, reg);
mii_nway_restart(&dev->mii);
ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);
Then things work on Arndale for me. Does that work for you?
Whether that is a sensible fix I don't know however.
>
> Mind, maybe the read function should take into account the reset value
> of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't have
> any documention here at the moment.
Yeah I also have no documentation.
Thanks,
Charles
>
> Is anyone able to confirm my suspicions?
>
> Kind regards,
>
> Michel Stam
> -----Original Message-----
> From: Riku Voipio [mailto:riku.voipio@iki.fi]
> Sent: Tuesday, November 04, 2014 10:44 AM
> To: Stam, Michel [FINT]
> Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-samsung-soc@vger.kernel.org; ckeepax@opensource.wolfsonmicro.com
> Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks
> net on arndale platform
>
> On Tue, Nov 04, 2014 at 09:19:26AM +0100, Stam, Michel [FINT] wrote:
> > Interesting, as the commit itself is a revert from a kernel back to
> > 2.6 somewhere. The problem I had is related to the PHY being reset
> > on interface-up, can you confirm that you require this?
>
> I can't confirm what exactly is needed on arndale. I'm neither expert
> in USB or ethernet. However, I can confirm that without the PHY reset,
> networking doesn't work on arndale.
>
> I now see someone else has the same problem, adding Charles to CC.
>
> http://www.spinics.net/lists/linux-usb/msg116656.html
>
> > Reverting this
> > breaks ethtool support in turn.
>
> Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> this case on arndale). This is now a regression of functionality from
> 3.17.
>
> I think it would better to revert the change now and with less hurry
> introduce a ethtool fix that doesn't break arndale.
>
> > Kind regards,
> >
> > Michel Stam
> >
> > -----Original Message-----
> > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > Sent: Tuesday, November 04, 2014 8:23 AM
> > To: davem@davemloft.net; Stam, Michel [FINT]
> > Cc: linux-usb@vger.kernel.org; netdev@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> > Subject: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
> > on
>
> > arndale platform
> >
> > Hi,
> >
> > With 3.18-rc3, asix on arndale (samsung exynos 5250 based board),
> > fails to work. Interface is initialized but network traffic seem not
> > to pass through. With kernel IP config the result looks like:
> >
> > [ 3.323275] usb 3-3.2.4: new high-speed USB device number 4 using
> > exynos-ehci
> > [ 3.419151] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > idProduct=772a
> > [ 3.424735] usb 3-3.2.4: New USB device strings: Mfr=1,
Product=2,
> > SerialNumber=3
> > [ 3.432196] usb 3-3.2.4: Product: AX88772
> > [ 3.436279] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > [ 3.441486] usb 3-3.2.4: SerialNumber: 000001
> > [ 3.447530] asix 3-3.2.4:1.0 (unnamed net_device)
(uninitialized):
> > invalid hw address, using random
> > [ 3.764352] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> de:a2:66:bf:ca:4f
> > [ 4.488773] asix 3-3.2.4:1.0 eth0: link down
> > [ 5.690025] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> lpa
> > 0xC5E1
> > [ 5.712947] Sending DHCP requests ...... timed out!
> > [ 83.165303] IP-Config: Retrying forever (NFS root)...
> > [ 83.170397] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> lpa
> > 0xC5E1
> > [ 83.192944] Sending DHCP requests .....
> >
> > Similar results also with dhclient. Git bisect identified the
> > breaking
>
> > commit as:
> >
> > commit 3cc81d85ee01e5a0b7ea2f4190e2ed1165f53c31
> > Author: Michel Stam <m.stam@fugro.nl>
> > Date: Thu Oct 2 10:22:02 2014 +0200
> >
> > asix: Don't reset PHY on if_up for ASIX 88772
> >
> > Taking 3.18-rc3 and that commit reverted, network works again:
> >
> > [ 3.303500] usb 3-3.2.4: new high-speed USB device number 4 using
> > exynos-ehci
> > [ 3.399375] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > idProduct=772a
> > [ 3.404963] usb 3-3.2.4: New USB device strings: Mfr=1,
Product=2,
> > SerialNumber=3
> > [ 3.412424] usb 3-3.2.4: Product: AX88772
> > [ 3.416508] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > [ 3.421715] usb 3-3.2.4: SerialNumber: 000001
> > [ 3.427755] asix 3-3.2.4:1.0 (unnamed net_device)
(uninitialized):
> > invalid hw address, using random
> > [ 3.744837] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> 12:59:f1:a8:43:90
> > [ 7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> lpa
> > 0xC5E1
> > [ 7.118258] Sending DHCP requests ., OK
> > [ 9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my
address
> > is 192.168.1.111
> >
> > There might something wrong on the samsung platform code (I
> > understand
>
> > the USB on arndale is "funny"), but this is still an regression from
> > 3.17.
> >
> > Riku
^ permalink raw reply
* [PATCH net-next 1/2] ethtool: Support for configurable RSS hash function
From: Amir Vadai @ 2014-11-05 11:59 UTC (permalink / raw)
To: David S. Miller, Ben Hutchings
Cc: netdev, Or Gerlitz, Eyal Perry, Yevgeny Petrilin, Amir Vadai
In-Reply-To: <1415188769-19593-1-git-send-email-amirv@mellanox.com>
From: Eyal Perry <eyalpe@mellanox.com>
This patch adds an RSS hash functions string-set, and two
ethtool-options for set/get current RSS hash function. User-kernel API is done
through the new hfunc mask field in the ethtool_rxfh struct. A bit set
in the hfunc is corresponding to an index in the string-set.
Signed-off-by: Eyal Perry <eyalpe@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
include/linux/ethtool.h | 28 ++++++++++++++++++++++++
include/uapi/linux/ethtool.h | 6 ++++-
net/core/ethtool.c | 52 ++++++++++++++++++++++++++++++--------------
3 files changed, 69 insertions(+), 17 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index c1a2d60..61003b1 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -59,6 +59,29 @@ enum ethtool_phys_id_state {
ETHTOOL_ID_OFF
};
+enum {
+ RSS_HASH_TOP_BIT, /* Configurable RSS hash function - Toeplitz */
+ RSS_HASH_XOR_BIT, /* Configurable RSS hash function - Xor */
+
+ /*
+ * Add your fresh new hash function bits above and remember to update
+ * rss_hash_func_strings[] below
+ */
+ RSS_HASH_FUNCS_COUNT
+};
+
+#define __RSS_HASH_BIT(bit) ((u32)1 << (bit))
+#define __RSS_HASH(name) __RSS_HASH_BIT(RSS_HASH_##name##_BIT)
+
+#define RSS_HASH_TOP __RSS_HASH(TOP)
+#define RSS_HASH_XOR __RSS_HASH(XOR)
+
+static const char
+rss_hash_func_strings[RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = {
+ [RSS_HASH_TOP_BIT] = "toeplitz",
+ [RSS_HASH_XOR_BIT] = "xor",
+};
+
struct net_device;
/* Some generic methods drivers may use in their ethtool_ops */
@@ -158,6 +181,9 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
* Returns zero if not supported for this specific device.
* @get_rxfh_indir_size: Get the size of the RX flow hash indirection table.
* Returns zero if not supported for this specific device.
+ * @get_rxfh_func: Get the hardware RX flow hash function.
+ * @set_rxfh_func: Set the hardware RX flow hash function. Returns a negative
+ * error code or zero.
* @get_rxfh: Get the contents of the RX flow hash indirection table and hash
* key.
* Will only be called if one or both of @get_rxfh_indir_size and
@@ -241,6 +267,8 @@ struct ethtool_ops {
int (*reset)(struct net_device *, u32 *);
u32 (*get_rxfh_key_size)(struct net_device *);
u32 (*get_rxfh_indir_size)(struct net_device *);
+ u32 (*get_rxfh_func)(struct net_device *);
+ int (*set_rxfh_func)(struct net_device *, u32);
int (*get_rxfh)(struct net_device *, u32 *indir, u8 *key);
int (*set_rxfh)(struct net_device *, const u32 *indir,
const u8 *key);
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index eb2095b..eb91da4 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -534,6 +534,7 @@ struct ethtool_pauseparam {
* @ETH_SS_NTUPLE_FILTERS: Previously used with %ETHTOOL_GRXNTUPLE;
* now deprecated
* @ETH_SS_FEATURES: Device feature names
+ * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
*/
enum ethtool_stringset {
ETH_SS_TEST = 0,
@@ -541,6 +542,7 @@ enum ethtool_stringset {
ETH_SS_PRIV_FLAGS,
ETH_SS_NTUPLE_FILTERS,
ETH_SS_FEATURES,
+ ETH_SS_RSS_HASH_FUNCS,
};
/**
@@ -900,7 +902,9 @@ struct ethtool_rxfh {
__u32 rss_context;
__u32 indir_size;
__u32 key_size;
- __u32 rsvd[2];
+ __u8 hfunc;
+ __u8 rsvd8[3];
+ __u32 rsvd32;
__u32 rss_config[0];
};
#define ETH_RXFH_INDIR_NO_CHANGE 0xffffffff
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 06dfb29..4791c17 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -185,6 +185,9 @@ static int __ethtool_get_sset_count(struct net_device *dev, int sset)
if (sset == ETH_SS_FEATURES)
return ARRAY_SIZE(netdev_features_strings);
+ if (sset == ETH_SS_RSS_HASH_FUNCS)
+ return ARRAY_SIZE(rss_hash_func_strings);
+
if (ops->get_sset_count && ops->get_strings)
return ops->get_sset_count(dev, sset);
else
@@ -199,6 +202,9 @@ static void __ethtool_get_strings(struct net_device *dev,
if (stringset == ETH_SS_FEATURES)
memcpy(data, netdev_features_strings,
sizeof(netdev_features_strings));
+ else if (stringset == ETH_SS_RSS_HASH_FUNCS)
+ memcpy(data, rss_hash_func_strings,
+ sizeof(rss_hash_func_strings));
else
/* ops->get_strings is valid because checked earlier */
ops->get_strings(dev, stringset, data);
@@ -682,7 +688,7 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
int ret;
const struct ethtool_ops *ops = dev->ethtool_ops;
u32 user_indir_size, user_key_size;
- u32 dev_indir_size = 0, dev_key_size = 0;
+ u32 dev_indir_size = 0, dev_key_size = 0, dev_hfunc = 0;
struct ethtool_rxfh rxfh;
u32 total_size;
u32 indir_bytes;
@@ -690,17 +696,18 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
u8 *hkey = NULL;
u8 *rss_config;
- if (!(dev->ethtool_ops->get_rxfh_indir_size ||
- dev->ethtool_ops->get_rxfh_key_size) ||
- !dev->ethtool_ops->get_rxfh)
+ if (!(ops->get_rxfh_indir_size || ops->get_rxfh_key_size ||
+ ops->get_rxfh_func) || !ops->get_rxfh)
return -EOPNOTSUPP;
+ if (ops->get_rxfh_func)
+ dev_hfunc = ops->get_rxfh_func(dev);
if (ops->get_rxfh_indir_size)
dev_indir_size = ops->get_rxfh_indir_size(dev);
if (ops->get_rxfh_key_size)
dev_key_size = ops->get_rxfh_key_size(dev);
- if ((dev_key_size + dev_indir_size) == 0)
+ if ((dev_key_size + dev_indir_size + dev_hfunc) == 0)
return -EOPNOTSUPP;
if (copy_from_user(&rxfh, useraddr, sizeof(rxfh)))
@@ -709,17 +716,19 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
user_key_size = rxfh.key_size;
/* Check that reserved fields are 0 for now */
- if (rxfh.rss_context || rxfh.rsvd[0] || rxfh.rsvd[1])
+ if (rxfh.rss_context || rxfh.rsvd8[0] || rxfh.rsvd8[1] ||
+ rxfh.rsvd8[2] || rxfh.rsvd32)
return -EINVAL;
rxfh.indir_size = dev_indir_size;
rxfh.key_size = dev_key_size;
+ rxfh.hfunc = dev_hfunc;
if (copy_to_user(useraddr, &rxfh, sizeof(rxfh)))
return -EFAULT;
- /* If the user buffer size is 0, this is just a query for the
- * device table size and key size. Otherwise, if the User size is
- * not equal to device table size or key size it's an error.
+ /* If the user buffer size is 0, this is just a query for the device
+ * hash function, table size and key size. Otherwise, if the User size
+ * is not equal to device table size or key size it's an error.
*/
if (!user_indir_size && !user_key_size)
return 0;
@@ -760,32 +769,43 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
const struct ethtool_ops *ops = dev->ethtool_ops;
struct ethtool_rxnfc rx_rings;
struct ethtool_rxfh rxfh;
- u32 dev_indir_size = 0, dev_key_size = 0, i;
+ u32 dev_indir_size = 0, dev_key_size = 0, dev_hfunc = 0, i;
u32 *indir = NULL, indir_bytes = 0;
u8 *hkey = NULL;
u8 *rss_config;
u32 rss_cfg_offset = offsetof(struct ethtool_rxfh, rss_config[0]);
- if (!(ops->get_rxfh_indir_size || ops->get_rxfh_key_size) ||
- !ops->get_rxnfc || !ops->set_rxfh)
+ if (!(ops->get_rxfh_indir_size || ops->get_rxfh_key_size ||
+ ops->get_rxfh_func) || !ops->get_rxnfc || !ops->set_rxfh)
return -EOPNOTSUPP;
+ if (ops->get_rxfh_func)
+ dev_hfunc = ops->get_rxfh_func(dev);
if (ops->get_rxfh_indir_size)
dev_indir_size = ops->get_rxfh_indir_size(dev);
if (ops->get_rxfh_key_size)
dev_key_size = dev->ethtool_ops->get_rxfh_key_size(dev);
- if ((dev_key_size + dev_indir_size) == 0)
+ if ((dev_key_size + dev_indir_size + dev_hfunc) == 0)
return -EOPNOTSUPP;
if (copy_from_user(&rxfh, useraddr, sizeof(rxfh)))
return -EFAULT;
/* Check that reserved fields are 0 for now */
- if (rxfh.rss_context || rxfh.rsvd[0] || rxfh.rsvd[1])
+ if (rxfh.rss_context || rxfh.rsvd8[0] || rxfh.rsvd8[1] ||
+ rxfh.rsvd8[2] || rxfh.rsvd32)
return -EINVAL;
+ if (rxfh.hfunc != dev_hfunc) {
+ if (!ops->set_rxfh_func)
+ return -EOPNOTSUPP;
+ ret = ops->set_rxfh_func(dev, rxfh.hfunc);
+ if (ret)
+ return ret;
+ }
+
/* If either indir or hash key is valid, proceed further.
- * It is not valid to request that both be unchanged.
+ * Must request at least one change: indir size, hash key or function.
*/
if ((rxfh.indir_size &&
rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE &&
@@ -793,7 +813,7 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
(rxfh.key_size && (rxfh.key_size != dev_key_size)) ||
(rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE &&
rxfh.key_size == 0))
- return -EINVAL;
+ return rxfh.hfunc ? 0 : -EINVAL;
if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE)
indir_bytes = dev_indir_size * sizeof(indir[0]);
--
1.8.3.4
^ permalink raw reply related
* [PATCH net-next 2/2] net/mlx4_en: Support for configurable RSS hash function
From: Amir Vadai @ 2014-11-05 11:59 UTC (permalink / raw)
To: David S. Miller, Ben Hutchings
Cc: netdev, Or Gerlitz, Eyal Perry, Yevgeny Petrilin, Amir Vadai
In-Reply-To: <1415188769-19593-1-git-send-email-amirv@mellanox.com>
From: Eyal Perry <eyalpe@mellanox.com>
The ConnectX HW is capable of using one of the following hash functions:
Toeplitz and an XOR hash function.
This patch implements ethtool_ops {get,set}_rxfh_func callbacks for the
mlx4_en driver in order to query and configure the RSS hash function to
be used by the device.
Signed-off-by: Eyal Perry <eyalpe@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 40 +++++++++++++++++++++++++
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 9 ++++++
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 11 ++++---
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 3 +-
4 files changed, 58 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index 8ea4d5b..f33785a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -973,6 +973,44 @@ static u32 mlx4_en_get_rxfh_indir_size(struct net_device *dev)
return priv->rx_ring_num;
}
+static u32 mlx4_en_get_rxfh_func(struct net_device *dev)
+{
+ struct mlx4_en_priv *priv = netdev_priv(dev);
+
+ return priv->rss_hash_fn;
+}
+
+static int mlx4_en_set_rxfh_func(struct net_device *dev, u32 hfunc)
+{
+ struct mlx4_en_priv *priv = netdev_priv(dev);
+ struct mlx4_en_dev *mdev = priv->mdev;
+ u32 prev_rss_hash_fn = priv->rss_hash_fn;
+ int err = 0;
+
+ if (!(hfunc & priv->rss_hash_fn_caps))
+ return -EINVAL;
+
+ priv->rss_hash_fn = hfunc;
+ if (hfunc == RSS_HASH_TOP && !(dev->features & NETIF_F_RXHASH))
+ en_warn(priv,
+ "Toeplitz hash function should be used in conjunction with RX hashing for optimal performance\n");
+ if (hfunc == RSS_HASH_XOR && (dev->features & NETIF_F_RXHASH))
+ en_warn(priv,
+ "Enabling both XOR Hash function and RX Hashing can limit RPS functionality\n");
+
+ mutex_lock(&mdev->state_lock);
+ if (priv->port_up && priv->rss_hash_fn != prev_rss_hash_fn) {
+ mlx4_en_stop_port(dev, 1);
+ err = mlx4_en_start_port(dev);
+ }
+ mutex_unlock(&mdev->state_lock);
+
+ if (err)
+ en_err(priv, "Failed to restart port %d\n", priv->port);
+
+ return err;
+}
+
static int mlx4_en_get_rxfh(struct net_device *dev, u32 *ring_index, u8 *key)
{
struct mlx4_en_priv *priv = netdev_priv(dev);
@@ -1799,6 +1837,8 @@ const struct ethtool_ops mlx4_en_ethtool_ops = {
.get_rxnfc = mlx4_en_get_rxnfc,
.set_rxnfc = mlx4_en_set_rxnfc,
.get_rxfh_indir_size = mlx4_en_get_rxfh_indir_size,
+ .get_rxfh_func = mlx4_en_get_rxfh_func,
+ .set_rxfh_func = mlx4_en_set_rxfh_func,
.get_rxfh = mlx4_en_get_rxfh,
.set_rxfh = mlx4_en_set_rxfh,
.get_channels = mlx4_en_get_channels,
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 0efbae9..a9e96a6 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2585,6 +2585,15 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
dev->features |= NETIF_F_GSO_UDP_TUNNEL;
}
+ if (mdev->dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_RSS_XOR) {
+ priv->rss_hash_fn_caps |= RSS_HASH_XOR;
+ priv->rss_hash_fn = RSS_HASH_XOR;
+ }
+ if (mdev->dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_RSS_TOP) {
+ priv->rss_hash_fn_caps |= RSS_HASH_TOP;
+ priv->rss_hash_fn = RSS_HASH_TOP;
+ }
+
mdev->pndev[port] = dev;
netif_carrier_off(dev);
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index b173a0c..41c1ff9 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -1113,10 +1113,13 @@ int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv)
}
rss_context->flags = rss_mask;
- rss_context->hash_fn = MLX4_RSS_HASH_TOP;
- for (i = 0; i < 10; i++)
- rss_context->rss_key[i] = cpu_to_be32(rsskey[i]);
-
+ if (priv->rss_hash_fn & MLX4_RSS_HASH_XOR) {
+ rss_context->hash_fn = MLX4_RSS_HASH_XOR;
+ } else {
+ rss_context->hash_fn = MLX4_RSS_HASH_TOP;
+ for (i = 0; i < 10; i++)
+ rss_context->rss_key[i] = cpu_to_be32(rsskey[i]);
+ }
err = mlx4_qp_to_ready(mdev->dev, &priv->res.mtt, &context,
&rss_map->indir_qp, &rss_map->indir_state);
if (err)
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index ef83d12..e1c3364 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -375,7 +375,6 @@ struct mlx4_en_port_profile {
};
struct mlx4_en_profile {
- int rss_xor;
int udp_rss;
u8 rss_mask;
u32 active_ports;
@@ -615,6 +614,8 @@ struct mlx4_en_priv {
__be16 vxlan_port;
u32 pflags;
+ u8 rss_hash_fn;
+ u8 rss_hash_fn_caps;
};
enum mlx4_en_wol {
--
1.8.3.4
^ permalink raw reply related
* [PATCH net-next 0/2] ethtool, net/mlx4_en: RSS hash function selection
From: Amir Vadai @ 2014-11-05 11:59 UTC (permalink / raw)
To: David S. Miller, Ben Hutchings
Cc: netdev, Or Gerlitz, Eyal Perry, Yevgeny Petrilin, Amir Vadai
Hi,
This patchset by Eyal adds support in set/get of RSS hash function. Current
supported functions are Toeplitz and XOR. The API is design to enable adding
new hash functions without breaking backward compatibility.
If a driver want to use this API need to implement [gs]et_rxfh_func() callbacks
in ethtool_ops.
Userspace patch will be sent after API is available in kernel.
The patchset was applied and tested over commit 30349bd ("net: phy: spi_ks8995:
remove sysfs bin file by registered attribute")
Amir
Eyal Perry (2):
ethtool: Support for configurable RSS hash function
net/mlx4_en: Support for configurable RSS hash function
drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 40 +++++++++++++++++++
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 9 +++++
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 11 ++++--
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 3 +-
include/linux/ethtool.h | 28 +++++++++++++
include/uapi/linux/ethtool.h | 6 ++-
net/core/ethtool.c | 52 +++++++++++++++++--------
7 files changed, 127 insertions(+), 22 deletions(-)
--
1.8.3.4
^ permalink raw reply
* Re: [PATCH V2 1/4] can: m_can: update to support CAN FD features
From: Varka Bhadram @ 2014-11-05 11:35 UTC (permalink / raw)
To: Dong Aisheng, linux-can; +Cc: mkl, wg, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1415174326-6623-1-git-send-email-b29396@freescale.com>
Hi Dong Aisheng,
On 11/05/2014 01:28 PM, Dong Aisheng wrote:
> Bosch M_CAN is CAN FD capable device. This patch implements the CAN
> FD features include up to 64 bytes payload and bitrate switch function.
> 1) Change the Rx FIFO and Tx Buffer to 64 bytes for support CAN FD
> up to 64 bytes payload. It's backward compatible with old 8 bytes
> normal CAN frame.
> 2) Allocate can frame or canfd frame based on EDL bit
> 3) Bitrate Switch function is disabled by default and will be enabled
> according to CANFD_BRS bit in cf->flags.
>
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
In these four patches two of them are V1 and other two are V2.
Is there any reason..?
How will you be able to generate like this by git format-patch..?
--
Thanks and Regards,
Varka Bhadram.
^ permalink raw reply
* Re: [PATCH v3 4/8] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Roger Quadros @ 2014-11-05 11:37 UTC (permalink / raw)
To: wg, mkl, Tomi Valkeinen
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <1415096461-25576-5-git-send-email-rogerq@ti.com>
On 11/04/2014 12:20 PM, Roger Quadros wrote:
> Some TI SoCs like DRA7 have a RAMINIT register specification
> different from the other AMxx SoCs and as expected by the
> existing driver.
>
> To add more insanity, this register is shared with other
> IPs like DSS, PCIe and PWM.
>
> Provides a more generic mechanism to specify the RAMINIT
> register location and START/DONE bit position and use the
> syscon/regmap framework to access the register.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> .../devicetree/bindings/net/can/c_can.txt | 3 +
> drivers/net/can/c_can/c_can.h | 9 ++-
> drivers/net/can/c_can/c_can_platform.c | 93 +++++++++++++---------
> 3 files changed, 65 insertions(+), 40 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
> index 8f1ae81..917ac0e 100644
> --- a/Documentation/devicetree/bindings/net/can/c_can.txt
> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
> @@ -12,6 +12,9 @@ Required properties:
> Optional properties:
> - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the
> instance number
> +- syscon-raminit : Handle to system control region that contains the
> + RAMINIT register and register offset to the RAMINIT
> + register.
>
> Note: "ti,hwmods" field is used to fetch the base address and irq
> resources from TI, omap hwmod data base during device registration.
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index c3b2108..b5067bd 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -178,6 +178,12 @@ struct c_can_driver_data {
> bool raminit_pulse; /* If set, sets and clears START bit (pulse) */
> };
>
> +/* Out of band RAMINIT register access via syscon regmap */
> +struct c_can_raminit {
> + struct regmap *syscon; /* for raminit ctrl. reg. access */
> + unsigned int reg; /* register index within syscon */
> +};
> +
> /* c_can private data structure */
> struct c_can_priv {
> struct can_priv can; /* must be the first member */
> @@ -195,8 +201,7 @@ struct c_can_priv {
> const u16 *regs;
> void *priv; /* for board-specific data */
> enum c_can_dev_id type;
> - u32 __iomem *raminit_ctrlreg;
> - int instance;
> + struct c_can_raminit raminit_sys; /* RAMINIT via syscon regmap */
> void (*raminit) (const struct c_can_priv *priv, bool enable);
> u32 comm_rcv_high;
> u32 rxmasked;
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index 11946e8..d0ce439 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -32,14 +32,13 @@
> #include <linux/clk.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
>
> #include <linux/can/dev.h>
>
> #include "c_can.h"
>
> -#define CAN_RAMINIT_START_MASK(i) (0x001 << (i))
> -#define CAN_RAMINIT_DONE_MASK(i) (0x100 << (i))
> -#define CAN_RAMINIT_ALL_MASK(i) (0x101 << (i))
> #define DCAN_RAM_INIT_BIT (1 << 3)
> static DEFINE_SPINLOCK(raminit_lock);
> /*
> @@ -72,47 +71,61 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv,
> writew(val, priv->base + 2 * priv->regs[index]);
> }
>
> -static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask,
> - u32 val)
> +static void c_can_hw_raminit_wait_syscon(const struct c_can_priv *priv,
> + u32 mask, u32 val)
> {
> int timeout = 0;
> + const struct c_can_raminit *raminit = &priv->raminit_sys;
> + u32 ctrl;
> +
> /* We look only at the bits of our instance. */
> val &= mask;
> - while ((readl(priv->raminit_ctrlreg) & mask) != val) {
> + do {
> udelay(1);
> timeout++;
>
> + regmap_read(raminit->syscon, raminit->reg, &ctrl);
> if (timeout == 1000) {
> dev_err(&priv->dev->dev, "%s: time out\n", __func__);
> break;
> }
> - }
> + } while ((ctrl & mask) != val);
> }
>
> -static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable)
> +static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable)
> {
> - u32 mask = CAN_RAMINIT_ALL_MASK(priv->instance);
> + u32 mask;
> u32 ctrl;
> + const struct c_can_raminit *raminit = &priv->raminit_sys;
> + u8 start_bit, done_bit;
> +
> + start_bit = priv->drvdata->raminit_start_bit;
> + done_bit = priv->drvdata->raminit_done_bit;
>
> spin_lock(&raminit_lock);
>
> - ctrl = readl(priv->raminit_ctrlreg);
> + mask = 1 << start_bit | 1 << done_bit;
> + regmap_read(raminit->syscon, raminit->reg, &ctrl);
> +
> /* We clear the done and start bit first. The start bit is
> * looking at the 0 -> transition, but is not self clearing;
> * And we clear the init done bit as well.
> + * NOTE: DONE must be written with 1 to clear it.
> */
> - ctrl &= ~CAN_RAMINIT_START_MASK(priv->instance);
> - ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
> - writel(ctrl, priv->raminit_ctrlreg);
> - ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance);
> - c_can_hw_raminit_wait_ti(priv, mask, ctrl);
> + ctrl &= ~(1 << start_bit);
> + ctrl |= 1 << done_bit;
> + regmap_write(raminit->syscon, raminit->reg, ctrl);
Thanks to Tomi for pointing out.
I need to use regmap_update_bits() instead of regmap_write() for atomic modification,
as this register needs to be shared between multiple drivers in case of DRA7 SoC.
cheers,
-roger
> +
> + ctrl &= ~(1 << done_bit);
> + c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
>
> if (enable) {
> /* Set start bit and wait for the done bit. */
> - ctrl |= CAN_RAMINIT_START_MASK(priv->instance);
> - writel(ctrl, priv->raminit_ctrlreg);
> - ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
> - c_can_hw_raminit_wait_ti(priv, mask, ctrl);
> + ctrl |= 1 << start_bit;
> + regmap_write(raminit->syscon, raminit->reg, ctrl);
> +
> + ctrl |= 1 << done_bit;
> + c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
> }
> spin_unlock(&raminit_lock);
> }
> @@ -206,10 +219,11 @@ static int c_can_plat_probe(struct platform_device *pdev)
> struct net_device *dev;
> struct c_can_priv *priv;
> const struct of_device_id *match;
> - struct resource *mem, *res;
> + struct resource *mem;
> int irq;
> struct clk *clk;
> const struct c_can_driver_data *drvdata;
> + struct device_node *np = pdev->dev.of_node;
>
> match = of_match_device(c_can_of_table, &pdev->dev);
> if (match) {
> @@ -279,27 +293,30 @@ static int c_can_plat_probe(struct platform_device *pdev)
> priv->read_reg32 = d_can_plat_read_reg32;
> priv->write_reg32 = d_can_plat_write_reg32;
>
> - if (pdev->dev.of_node)
> - priv->instance = of_alias_get_id(pdev->dev.of_node, "d_can");
> - else
> - priv->instance = pdev->id;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> - /* Not all D_CAN modules have a separate register for the D_CAN
> - * RAM initialization. Use default RAM init bit in D_CAN module
> - * if not specified in DT.
> + /* Check if we need custom RAMINIT via syscon. Mostly for TI
> + * platforms. Only supported with DT boot.
> */
> - if (!res) {
> + if (np && of_property_read_bool(np, "syscon-raminit")) {
> + ret = -EINVAL;
> + priv->raminit_sys.syscon = syscon_regmap_lookup_by_phandle(np,
> + "syscon-raminit");
> + if (IS_ERR(priv->raminit_sys.syscon)) {
> + dev_err(&pdev->dev,
> + "couldn't get syscon regmap for RAMINIT reg.\n");
> + goto exit_free_device;
> + }
> +
> + if (of_property_read_u32_index(np, "syscon-raminit", 1,
> + &priv->raminit_sys.reg)) {
> + dev_err(&pdev->dev,
> + "couldn't get the RAMINIT reg. offset!\n");
> + goto exit_free_device;
> + }
> +
> + priv->raminit = c_can_hw_raminit_syscon;
> + } else {
> priv->raminit = c_can_hw_raminit;
> - break;
> }
> -
> - priv->raminit_ctrlreg = devm_ioremap(&pdev->dev, res->start,
> - resource_size(res));
> - if (!priv->raminit_ctrlreg || priv->instance < 0)
> - dev_info(&pdev->dev, "control memory is not used for raminit\n");
> - else
> - priv->raminit = c_can_hw_raminit_ti;
> break;
> default:
> ret = -EINVAL;
>
^ permalink raw reply
* Re: [PATCH V2 1/4] can: m_can: update to support CAN FD features
From: Dong Aisheng @ 2014-11-05 11:36 UTC (permalink / raw)
To: Varka Bhadram; +Cc: linux-can, mkl, wg, netdev, socketcan, linux-arm-kernel
In-Reply-To: <545A0B6F.8060706@gmail.com>
On Wed, Nov 05, 2014 at 05:05:11PM +0530, Varka Bhadram wrote:
> Hi Dong Aisheng,
>
> On 11/05/2014 01:28 PM, Dong Aisheng wrote:
>
> >Bosch M_CAN is CAN FD capable device. This patch implements the CAN
> >FD features include up to 64 bytes payload and bitrate switch function.
> >1) Change the Rx FIFO and Tx Buffer to 64 bytes for support CAN FD
> > up to 64 bytes payload. It's backward compatible with old 8 bytes
> > normal CAN frame.
> >2) Allocate can frame or canfd frame based on EDL bit
> >3) Bitrate Switch function is disabled by default and will be enabled
> > according to CANFD_BRS bit in cf->flags.
> >
> >Signed-off-by: Dong Aisheng <b29396@freescale.com>
>
> In these four patches two of them are V1 and other two are V2.
>
> Is there any reason..?
>
Sorry for confusion.
Because some patches are already picked by Macr on the first round.
See below:
http://www.spinics.net/lists/netdev/msg302133.html
> How will you be able to generate like this by git format-patch..?
>
Generated as v2 and i manually changed the later two to v1 since they
are new.
Not sure if it's suitable to do like that. :-)
Regards
Dong Aisheng
> --
> Thanks and Regards,
> Varka Bhadram.
>
^ permalink raw reply
* Re: [PATCH V2 2/4] can: m_can: workaround for transmit data less than 4 bytes
From: Marc Kleine-Budde @ 2014-11-05 11:32 UTC (permalink / raw)
To: Dong Aisheng
Cc: linux-can, wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <20141105103349.GA4007@shlinux1.ap.freescale.net>
[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]
On 11/05/2014 11:33 AM, Dong Aisheng wrote:
> On Wed, Nov 05, 2014 at 11:17:48AM +0100, Marc Kleine-Budde wrote:
>> On 11/05/2014 08:58 AM, Dong Aisheng wrote:
>>> At least on the i.MX6SX with M_CAN IP version 3.0.x, an issue with
>>
>> Can you add the imx mask revision, too and the exact m_can version (is
>> available).
>>
>
> What do you mean imx mask revision?
In my imx6sdl/imx6q data sheet it's called: Chip Silicon Version
(USB_ANALOG_DIGPROG), I expect that the imx6sx has a the same register.
> By reading the Core Release Register (CREL), it seems the exact m_can
> version is 3.0.1. (CREL = 30130506).
>
> So what about changing to as follows?
> "At least on the i.MX6SX with M_CAN IP version 3.0.1 (CREL = 30130506),
> an issue with"
Yes, please ass the imx silicon revision, too.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH V2 2/4] can: m_can: workaround for transmit data less than 4 bytes
From: Dong Aisheng @ 2014-11-05 11:32 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: linux-can, wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <545A0AC5.80006@pengutronix.de>
On Wed, Nov 05, 2014 at 12:32:21PM +0100, Marc Kleine-Budde wrote:
> On 11/05/2014 11:33 AM, Dong Aisheng wrote:
> > On Wed, Nov 05, 2014 at 11:17:48AM +0100, Marc Kleine-Budde wrote:
> >> On 11/05/2014 08:58 AM, Dong Aisheng wrote:
> >>> At least on the i.MX6SX with M_CAN IP version 3.0.x, an issue with
> >>
> >> Can you add the imx mask revision, too and the exact m_can version (is
> >> available).
> >>
> >
> > What do you mean imx mask revision?
>
> In my imx6sdl/imx6q data sheet it's called: Chip Silicon Version
> (USB_ANALOG_DIGPROG), I expect that the imx6sx has a the same register.
>
> > By reading the Core Release Register (CREL), it seems the exact m_can
> > version is 3.0.1. (CREL = 30130506).
> >
> > So what about changing to as follows?
> > "At least on the i.MX6SX with M_CAN IP version 3.0.1 (CREL = 30130506),
> > an issue with"
>
> Yes, please ass the imx silicon revision, too.
>
Okay, it's i.MX6SX TO1.2. Will add it.
Regards
Dong Aisheng
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
>
^ permalink raw reply
* Re: [PATCH V2 1/4] can: m_can: update to support CAN FD features
From: Dong Aisheng @ 2014-11-05 11:26 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: linux-can, mkl, wg, varkabhadram, netdev, linux-arm-kernel
In-Reply-To: <5459F808.3020903@hartkopp.net>
On Wed, Nov 05, 2014 at 11:12:24AM +0100, Oliver Hartkopp wrote:
> On 05.11.2014 08:58, Dong Aisheng wrote:
>
> >@@ -327,41 +357,65 @@ static inline void m_can_disable_all_interrupts(const struct m_can_priv *priv)
> > m_can_write(priv, M_CAN_ILE, 0x0);
> > }
> >
> >-static void m_can_read_fifo(const struct net_device *dev, struct can_frame *cf,
> >- u32 rxfs)
> >+static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
> > {
> >+ struct net_device_stats *stats = &dev->stats;
> > struct m_can_priv *priv = netdev_priv(dev);
> >- u32 id, fgi;
> >+ struct canfd_frame *cf;
> >+ struct sk_buff *skb;
> >+ u32 id, fgi, dlc;
> >+ int i;
> >
> > /* calculate the fifo get index for where to read data */
> > fgi = (rxfs & RXFS_FGI_MASK) >> RXFS_FGI_OFF;
> >+ dlc = m_can_fifo_read(priv, fgi, M_CAN_FIFO_DLC);
> >+ if (dlc & RX_BUF_EDL)
> >+ skb = alloc_canfd_skb(dev, &cf);
> >+ else
> >+ skb = alloc_can_skb(dev, (struct can_frame **)&cf);
>
> Yes. That's the way it should look like ;-)
>
> >+ if (!skb) {
> >+ stats->rx_dropped++;
> >+ return;
> >+ }
> >+
> > id = m_can_fifo_read(priv, fgi, M_CAN_FIFO_ID);
> > if (id & RX_BUF_XTD)
> > cf->can_id = (id & CAN_EFF_MASK) | CAN_EFF_FLAG;
> > else
> > cf->can_id = (id >> 18) & CAN_SFF_MASK;
> >
> >- if (id & RX_BUF_RTR) {
> >+ if (id & RX_BUF_ESI) {
> >+ cf->flags |= CANFD_ESI;
> >+ netdev_dbg(dev, "ESI Error\n");
> >+ }
> >+
> >+ if (!(dlc & RX_BUF_EDL) && (id & RX_BUF_RTR)) {
> > cf->can_id |= CAN_RTR_FLAG;
> > } else {
> > id = m_can_fifo_read(priv, fgi, M_CAN_FIFO_DLC);
> >- cf->can_dlc = get_can_dlc((id >> 16) & 0x0F);
> >- *(u32 *)(cf->data + 0) = m_can_fifo_read(priv, fgi,
> >- M_CAN_FIFO_DATA(0));
> >- *(u32 *)(cf->data + 4) = m_can_fifo_read(priv, fgi,
> >- M_CAN_FIFO_DATA(1));
> >+ cf->len = can_dlc2len(get_canfd_dlc((id >> 16) & 0x0F));
>
> if (dlc & RX_BUF_EDL)
> cf->len = can_dlc2len((id >> 16) & 0x0F);
> else
> cf->len = get_can_dlc((id >> 16) & 0x0F);
>
> (..)
>
Correct.
Will change it.
> >@@ -804,7 +870,8 @@ static void m_can_chip_config(struct net_device *dev)
> > RXFC_FWM_1 | priv->mcfg[MRAM_RXF1].off);
> >
> > cccr = m_can_read(priv, M_CAN_CCCR);
> >- cccr &= ~(CCCR_TEST | CCCR_MON);
> >+ cccr &= ~(CCCR_TEST | CCCR_MON | (CCCR_CMR_MASK << CCCR_CMR_SHIFT) |
> >+ (CCCR_CME_MASK << CCCR_CME_SHIFT));
> > test = m_can_read(priv, M_CAN_TEST);
> > test &= ~TEST_LBCK;
> >
> >@@ -816,6 +883,9 @@ static void m_can_chip_config(struct net_device *dev)
> > test |= TEST_LBCK;
> > }
> >
> >+ if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
> >+ cccr |= CCCR_CME_CANFD_BRS << CCCR_CME_SHIFT;
> >+
> > m_can_write(priv, M_CAN_CCCR, cccr);
> > m_can_write(priv, M_CAN_TEST, test);
> >
>
> (..)
>
> >
> >+ if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
> >+ cccr = m_can_read(priv, M_CAN_CCCR);
> >+ cccr &= ~(CCCR_CMR_MASK << CCCR_CMR_SHIFT);
> >+ if (cf->flags & CANFD_BRS)
> >+ cccr |= CCCR_CMR_CANFD_BRS << CCCR_CMR_SHIFT;
> >+ else
> >+ cccr |= CCCR_CMR_CANFD << CCCR_CMR_SHIFT;
> >+ m_can_write(priv, M_CAN_CCCR, cccr);
> >+ }
>
> Unfortunately No. Here it becomes complicated due to the fact that
> the revision 3.0.x does not support per-frame switching for FD/BRS
> ...
I'm not sure i got your point.
From m_can spec, it allows switch CAN mode by setting CMR bit.
Bits 11:10 CMR[1:0]: CAN Mode Request
A change of the CAN operation mode is requested by writing to this bit field. After change to the
requested operation mode the bit field is reset to “00” and the status flags FDBS and FDO are set
accordingly. In case the requested CAN operation mode is not enabled, the value written to CMR is
retained until it is overwritten by the next mode change request. In case CME = “01”/”10”/”11” a
change to CAN operation according to ISO 11898-1 is always possible. Default is CAN operation
according to ISO11898-1.
00 unchanged
01 Request CAN FD operation
10 Request CAN FD operation with bit rate switching
11 Request CAN operation according ISO11898-1
So what's the difference between this function and the per-frame switching
you mentioned?
>
> When (priv->can.ctrlmode & CAN_CTRLMODE_FD) is true this *only*
> tells us, that the controller is _capable_ to send either CAN or CAN
> FD frames.
>
> It does not configure the controller into one of these specific settings!
>
> Additionally: AFAIK when writing to the CCCR you have to make sure
> that there's currently no ongoing transfer.
>
I don't know about it before.
By searching m_can user manual v302 again, i still did not find this
limitation. Can you point me if you know where it is?
BTW, since we only use one Tx Buffer for transmission currently, so we
should not meet that case that CAN mode is switched during transfer.
So the issue you concern may not happen.
Additionally it looks to me like m_can does allow set CMR bit at runtime
since the mode change will take affect when controller becomes idle.
See below:
"A mode change requested by writing to CCCR.CMR will be executed next
time the CAN protocol controller FSM reaches idle phase between CAN frames.
Upon this event CCCR.CMR is reset to “00” and the status flags CCCR.FDBS
and CCCR.FDO are set accordingly. In case the requested
CAN operation mode is not enabled, the value written to CCCR.CMR is
retained until it is overwritten by the next mode change request.
Default is CAN operation according to ISO11898-1."
> I would suggest the following approach to make the revision 3.0.x
> act like a correct CAN FD capable controller:
>
> - create a helper function to switch FD and BRS while taking care of
> ongoing transmissions
>
> - create a variable that knows the current tx_mode for FD and BRS
>
> When we need to send a CAN frame which doesn't fit the settings in
> the tx_mode we need to switch the CCCR and update the tx_mode
> variable.
>
> This at least reduces the needed CCCR operations.
>
Yes, i can do like that.
But what i see by doing that is only reduces the needed CCCR operations?
Any other benefits i missed?
And the test for current code showed it seemed work well on the Mode
switch among std frame/fd frame/brs frame.
Regards
Dong Aisheng
> And it also addresses the intention of your patch
> [PATCH V1 4/4] can: m_can: allow to send std frame on CAN FD mode
>
> Regards,
> Oliver
>
^ permalink raw reply
* Re: [PATCH V1 4/4] can: m_can: allow to send std frame on CAN FD mode
From: Oliver Hartkopp @ 2014-11-05 11:08 UTC (permalink / raw)
To: Marc Kleine-Budde, Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, linux-arm-kernel
In-Reply-To: <5459FEDE.9050405@pengutronix.de>
On 05.11.2014 11:41, Marc Kleine-Budde wrote:
> On 11/05/2014 08:58 AM, Dong Aisheng wrote:
>> The current code sends all CAN frames on CAN FD format(with BRS or not)
>> if CAN_CTRLMODE_FD is enabled.
>> However, even CAN_CTRLMODE_FD is enabled, the can tool may still
>> send normal frames.
>> e.g.
>> ip link set can0 up type can bitrate 1000000 dbitrate 1000000 fd on
>> cansend can0 123#112233
>>
>> Therefore sending normal CAN frame on FD format seems not reasonable
>> and the CAN FD incapable device may not be able to receive it correctly.
>>
>> The patch switches the M_CAN operation mode to ISO11898-1 instead of
>> staying on CAN FD operation mode by writing "11" to CMR bit if find
>> we're sending a normal can skb.
>
> With this patch applied and Olivre's version of 3/4, how does the
> application send CAN-FD frames?
This patch becomes obsolete when we do it like in my answer of [3/4].
> 1. witch on CAN-FD via "ip fd on"
With
ip link set can0 type can fd on
the netdevice switches to the MTU of CAN FD (72) instead of 16.
This means that this netdevice can handle CAN frames (MTU 16) and CAN FD
frames (MTU 72).
When you send a standard CAN frame, e.g.
cansend can0 123#112233
you would get a CAN 2.0 frame (dlc = 3) on the bus.
When you send a CAN FD frame, e.g.
cansend can0 123##0112233
you would get a CAN FD frame (dlc = 3) on the bus.
With
cansend can0 123##1112233
you would get a CAN FD frame (dlc = 3) with BRS on the bus.
Whether it is CAN or CAN FD is given by checking skb->len for CAN_MTU of
CANFD_MTU in the driver.
Regards,
Oliver
> 2. write a struct canfd_frame
>
> Correct?
>
> What happens if:
> 3. write a struct can_frame
>
> A CAN frame is send?
>
> Oliver are you okay with this behaviour?
>
> Marc
>
^ permalink raw reply
* Re: [PATCHv2 net-next] xen-netback: remove unconditional __pskb_pull_tail() in guest Tx path
From: Ian Campbell @ 2014-11-05 10:57 UTC (permalink / raw)
To: David Vrabel; +Cc: netdev, xen-devel, Wei Liu, Malcolm Crossley
In-Reply-To: <1415184622-19421-1-git-send-email-david.vrabel@citrix.com>
On Wed, 2014-11-05 at 10:50 +0000, David Vrabel wrote:
> From: Malcolm Crossley <malcolm.crossley@citrix.com>
>
> Unconditionally pulling 128 bytes into the linear area is not required
> for:
>
> - security: Every protocol demux starts with pskb_may_pull() to pull
> frag data into the linear area, if necessary, before looking at
> headers.
>
> - performance: Netback has already grant copied up-to 128 bytes from
> the first slot of a packet into the linear area. The first slot
> normally contain all the IPv4/IPv6 and TCP/UDP headers.
Thanks for adding these.
> The unconditional pull would often copy frag data unnecessarily. This
> is a performance problem when running on a version of Xen where grant
> unmap avoids TLB flushes for pages which are not accessed. TLB
> flushes can now be avoided for > 99% of unmaps (it was 0% before).
>
> Grant unmap TLB flush avoidance will be available in a future version
> of Xen (probably 4.6).
>
> Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply
* Re: [Xen-devel] [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: Ian Campbell @ 2014-11-05 10:53 UTC (permalink / raw)
To: David Vrabel
Cc: Eric Dumazet, David Miller, zoltan.kiss, netdev, malcolm.crossley,
wei.liu2, xen-devel
In-Reply-To: <545A000D.8070808@citrix.com>
On Wed, 2014-11-05 at 10:46 +0000, David Vrabel wrote:
> On 04/11/14 21:43, Eric Dumazet wrote:
> > On Tue, 2014-11-04 at 16:17 -0500, David Miller wrote:
> >
> >
> >>
> >> Every protocol demux starts with pskb_may_pull() to pull frag data
> >> into the linear area, if necessary, before looking at headers.
> >
> > eth_get_headlen() might be relevant as well, to perform a single copy of
> > exactly all headers.
>
> In netback's case we need an estimate of the header length before
> reading any of the packet, since peeking at any frag would prevent any
> TLB flush avoidance.
>
> It might be useful to use eth_get_headlen() to adjust the estimate at
> runtime, but for now the fixed amount of 128 bytes is simple and seems
> good enough.
I think what Eric meant was that having done the 128 copy you could call
eth_get_headlen which in the common case should be a nop but would
ensure you always had the headers in the linear area for the uncommon
case.
It looks like the difference compared with skb_checksum_setup is that
eth_get_headlen deals with L4 too whereas skb_checksum_setup only goes
to L3 (and then only for some subset of protocols with checksums).
Ian.
^ permalink raw reply
* [PATCHv2 net-next] xen-netback: remove unconditional __pskb_pull_tail() in guest Tx path
From: David Vrabel @ 2014-11-05 10:50 UTC (permalink / raw)
To: netdev; +Cc: David Vrabel, xen-devel, Ian Campbell, Wei Liu, Malcolm Crossley
From: Malcolm Crossley <malcolm.crossley@citrix.com>
Unconditionally pulling 128 bytes into the linear area is not required
for:
- security: Every protocol demux starts with pskb_may_pull() to pull
frag data into the linear area, if necessary, before looking at
headers.
- performance: Netback has already grant copied up-to 128 bytes from
the first slot of a packet into the linear area. The first slot
normally contain all the IPv4/IPv6 and TCP/UDP headers.
The unconditional pull would often copy frag data unnecessarily. This
is a performance problem when running on a version of Xen where grant
unmap avoids TLB flushes for pages which are not accessed. TLB
flushes can now be avoided for > 99% of unmaps (it was 0% before).
Grant unmap TLB flush avoidance will be available in a future version
of Xen (probably 4.6).
Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
drivers/net/xen-netback/netback.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 730252c..14e18bb 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -82,6 +82,16 @@ MODULE_PARM_DESC(max_queues,
static unsigned int fatal_skb_slots = FATAL_SKB_SLOTS_DEFAULT;
module_param(fatal_skb_slots, uint, 0444);
+/* The amount to copy out of the first guest Tx slot into the skb's
+ * linear area. If the first slot has more data, it will be mapped
+ * and put into the first frag.
+ *
+ * This is sized to avoid pulling headers from the frags for most
+ * TCP/IP packets.
+ */
+#define XEN_NETBACK_TX_COPY_LEN 128
+
+
static void xenvif_idx_release(struct xenvif_queue *queue, u16 pending_idx,
u8 status);
@@ -125,13 +135,6 @@ static inline struct xenvif_queue *ubuf_to_queue(const struct ubuf_info *ubuf)
pending_tx_info[0]);
}
-/* This is a miniumum size for the linear area to avoid lots of
- * calls to __pskb_pull_tail() as we set up checksum offsets. The
- * value 128 was chosen as it covers all IPv4 and most likely
- * IPv6 headers.
- */
-#define PKT_PROT_LEN 128
-
static u16 frag_get_pending_idx(skb_frag_t *frag)
{
return (u16)frag->page_offset;
@@ -1446,9 +1449,9 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
index = pending_index(queue->pending_cons);
pending_idx = queue->pending_ring[index];
- data_len = (txreq.size > PKT_PROT_LEN &&
+ data_len = (txreq.size > XEN_NETBACK_TX_COPY_LEN &&
ret < XEN_NETBK_LEGACY_SLOTS_MAX) ?
- PKT_PROT_LEN : txreq.size;
+ XEN_NETBACK_TX_COPY_LEN : txreq.size;
skb = xenvif_alloc_skb(data_len);
if (unlikely(skb == NULL)) {
@@ -1653,11 +1656,6 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
}
}
- if (skb_is_nonlinear(skb) && skb_headlen(skb) < PKT_PROT_LEN) {
- int target = min_t(int, skb->len, PKT_PROT_LEN);
- __pskb_pull_tail(skb, target - skb_headlen(skb));
- }
-
skb->dev = queue->vif->dev;
skb->protocol = eth_type_trans(skb, skb->dev);
skb_reset_network_header(skb);
--
1.7.10.4
^ permalink raw reply related
* Re: [Xen-devel] [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: David Vrabel @ 2014-11-05 10:46 UTC (permalink / raw)
To: Eric Dumazet, David Miller
Cc: zoltan.kiss, Ian.Campbell, netdev, malcolm.crossley, wei.liu2,
xen-devel
In-Reply-To: <1415137397.25370.7.camel@edumazet-glaptop2.roam.corp.google.com>
On 04/11/14 21:43, Eric Dumazet wrote:
> On Tue, 2014-11-04 at 16:17 -0500, David Miller wrote:
>
>
>>
>> Every protocol demux starts with pskb_may_pull() to pull frag data
>> into the linear area, if necessary, before looking at headers.
>
> eth_get_headlen() might be relevant as well, to perform a single copy of
> exactly all headers.
In netback's case we need an estimate of the header length before
reading any of the packet, since peeking at any frag would prevent any
TLB flush avoidance.
It might be useful to use eth_get_headlen() to adjust the estimate at
runtime, but for now the fixed amount of 128 bytes is simple and seems
good enough.
David
^ permalink raw reply
* Possible bug in net/core/neighbor.c
From: Ulf Samuelsson @ 2014-11-05 10:46 UTC (permalink / raw)
To: netdev
I find the following in "net/core/neighbor.c"
/* Compare new lladdr with cached one */
if (!dev->addr_len) {
/* First case: device needs no address. */
lladdr = neigh->ha;
} else if (lladdr) {
/* The second case: if something is already cached
and a new address is proposed:
- compare new & old
- if they are different, check override flag
*/
/* POSSIBLE BUG */
if ((old & NUD_VALID) &&
!memcmp(lladdr, neigh->ha, dev->addr_len))
lladdr = neigh->ha;
/* END POSSIBLE BUG */
} else {
/* No address is supplied; if we know something,
use it, otherwise discard the request.
*/
err = -EINVAL;
if (!(old & NUD_VALID))
goto out;
lladdr = neigh->ha;
}
It looks to me like the code is saying
if the proposed address is equal to the original address,
set the proposed address to the original address.
which is pretty meaningless.
Should it not be:
if ((old & NUD_VALID) &&
memcmp(lladdr, neigh->ha, dev->addr_len)) /* True if
addresses are not equal */
neigh->ha = lladdr; /* Update to new address */
If not, I would appreciate an explanation what the code is doing.
--
Best Regards,
Ulf Samuelsson
^ permalink raw reply
* Re: [PATCH V1 4/4] can: m_can: allow to send std frame on CAN FD mode
From: Marc Kleine-Budde @ 2014-11-05 10:41 UTC (permalink / raw)
To: Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1415174326-6623-4-git-send-email-b29396@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 1267 bytes --]
On 11/05/2014 08:58 AM, Dong Aisheng wrote:
> The current code sends all CAN frames on CAN FD format(with BRS or not)
> if CAN_CTRLMODE_FD is enabled.
> However, even CAN_CTRLMODE_FD is enabled, the can tool may still
> send normal frames.
> e.g.
> ip link set can0 up type can bitrate 1000000 dbitrate 1000000 fd on
> cansend can0 123#112233
>
> Therefore sending normal CAN frame on FD format seems not reasonable
> and the CAN FD incapable device may not be able to receive it correctly.
>
> The patch switches the M_CAN operation mode to ISO11898-1 instead of
> staying on CAN FD operation mode by writing "11" to CMR bit if find
> we're sending a normal can skb.
With this patch applied and Olivre's version of 3/4, how does the
application send CAN-FD frames?
1. witch on CAN-FD via "ip fd on"
2. write a struct canfd_frame
Correct?
What happens if:
3. write a struct can_frame
A CAN frame is send?
Oliver are you okay with this behaviour?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH V2 2/4] can: m_can: workaround for transmit data less than 4 bytes
From: Dong Aisheng @ 2014-11-05 10:33 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: linux-can, wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <5459F94C.1030506@pengutronix.de>
On Wed, Nov 05, 2014 at 11:17:48AM +0100, Marc Kleine-Budde wrote:
> On 11/05/2014 08:58 AM, Dong Aisheng wrote:
> > At least on the i.MX6SX with M_CAN IP version 3.0.x, an issue with
>
> Can you add the imx mask revision, too and the exact m_can version (is
> available).
>
What do you mean imx mask revision?
By reading the Core Release Register (CREL), it seems the exact m_can
version is 3.0.1. (CREL = 30130506).
So what about changing to as follows?
"At least on the i.MX6SX with M_CAN IP version 3.0.1 (CREL = 30130506),
an issue with"
Regards
Dong Aisheng
> > the Message RAM was discovered. Sending CAN frames with dlc less
> > than 4 bytes will lead to bit errors, when the first 8 bytes of
> > the Message RAM have not been initialized (i.e. written to).
> > To work around this issue, the first 8 bytes are initialized in open()
> > function.
> >
> > Without the workaround, we can easily see the following errors:
> > root@imx6qdlsolo:~# ip link set can0 up type can bitrate 1000000
> > [ 66.882520] IPv6: ADDRCONF(NETDEV_CHANGE): can0: link becomes ready
> > root@imx6qdlsolo:~# cansend can0 123#112233
> > [ 66.935640] m_can 20e8000.can can0: Bit Error Uncorrected
> >
> > Signed-off-by: Dong Aisheng <b29396@freescale.com>
> > ---
> > ChangeLog since v1:
> > * initialize the first 8 bytes of Tx Buffer of Message RAM in open()
> > to workaround the issue
> > ---
> > drivers/net/can/m_can/m_can.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > index 664fe30..f47c200 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -902,6 +902,15 @@ static void m_can_chip_config(struct net_device *dev)
> > /* set bittiming params */
> > m_can_set_bittiming(dev);
> >
> > + /* At least on the i.MX6SX with M_CAN IP version 3.0.x, an issue with
> > + * the Message RAM was discovered. Sending CAN frames with dlc less
> > + * than 4 bytes will lead to bit errors, when the first 8 bytes of
> > + * the Message RAM have not been initialized (i.e. written to).
> > + * To work around this issue, the first 8 bytes are initialized here.
> > + */
> > + m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(0), 0x0);
> > + m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(1), 0x0);
> > +
> > m_can_config_endisable(priv, false);
> > }
> >
> >
>
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
>
^ permalink raw reply
* [PATCH v3 2/4] stmmac: pci: convert to use dev_pm_ops
From: Andy Shevchenko @ 2014-11-05 10:27 UTC (permalink / raw)
To: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
Cc: Andy Shevchenko
In-Reply-To: <1415183249-9231-1-git-send-email-andriy.shevchenko@linux.intel.com>
Convert system PM callbacks to use dev_pm_ops. In addition remove the PCI calls
related to a power state since the bus code cares about this already.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 27 ++++++++++--------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index e45794d..f19ac8e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -150,30 +150,26 @@ static void stmmac_pci_remove(struct pci_dev *pdev)
pci_disable_device(pdev);
}
-#ifdef CONFIG_PM
-static int stmmac_pci_suspend(struct pci_dev *pdev, pm_message_t state)
+#ifdef CONFIG_PM_SLEEP
+static int stmmac_pci_suspend(struct device *dev)
{
+ struct pci_dev *pdev = to_pci_dev(dev);
struct net_device *ndev = pci_get_drvdata(pdev);
- int ret;
- ret = stmmac_suspend(ndev);
- pci_save_state(pdev);
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
- return ret;
+ return stmmac_suspend(ndev);
}
-static int stmmac_pci_resume(struct pci_dev *pdev)
+static int stmmac_pci_resume(struct device *dev)
{
+ struct pci_dev *pdev = to_pci_dev(dev);
struct net_device *ndev = pci_get_drvdata(pdev);
- pci_set_power_state(pdev, PCI_D0);
- pci_restore_state(pdev);
-
return stmmac_resume(ndev);
}
#endif
+static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_pci_suspend, stmmac_pci_resume);
+
#define STMMAC_VENDOR_ID 0x700
#define STMMAC_DEVICE_ID 0x1108
@@ -190,10 +186,9 @@ struct pci_driver stmmac_pci_driver = {
.id_table = stmmac_id_table,
.probe = stmmac_pci_probe,
.remove = stmmac_pci_remove,
-#ifdef CONFIG_PM
- .suspend = stmmac_pci_suspend,
- .resume = stmmac_pci_resume,
-#endif
+ .driver = {
+ .pm = &stmmac_pm_ops,
+ },
};
MODULE_DESCRIPTION("STMMAC 10/100/1000 Ethernet PCI driver");
--
2.1.1
^ permalink raw reply related
* [PATCH v3 4/4] stmmac: pci: convert to use dev_* macros
From: Andy Shevchenko @ 2014-11-05 10:27 UTC (permalink / raw)
To: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
Cc: Andy Shevchenko
In-Reply-To: <1415183249-9231-1-git-send-email-andriy.shevchenko@linux.intel.com>
Instead of pr_* macros let's use dev_* macros which provide device name.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 5357a3f..5084699 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -78,8 +78,8 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
/* Enable pci device */
ret = pcim_enable_device(pdev);
if (ret) {
- pr_err("%s : ERROR: failed to enable %s device\n", __func__,
- pci_name(pdev));
+ dev_err(&pdev->dev, "%s: ERROR: failed to enable device\n",
+ __func__);
return ret;
}
@@ -100,7 +100,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
priv = stmmac_dvr_probe(&pdev->dev, &plat_dat,
pcim_iomap_table(pdev)[i]);
if (IS_ERR(priv)) {
- pr_err("%s: main driver probe failed", __func__);
+ dev_err(&pdev->dev, "%s: main driver probe failed\n", __func__);
return PTR_ERR(priv);
}
priv->dev->irq = pdev->irq;
@@ -108,7 +108,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
pci_set_drvdata(pdev, priv->dev);
- pr_debug("STMMAC platform driver registration completed");
+ dev_dbg(&pdev->dev, "STMMAC PCI driver registration completed\n");
return 0;
}
--
2.1.1
^ permalink raw reply related
* [PATCH v3 1/4] stmmac: pci: use defined constant instead of magic number
From: Andy Shevchenko @ 2014-11-05 10:27 UTC (permalink / raw)
To: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
Cc: Andy Shevchenko
In-Reply-To: <1415183249-9231-1-git-send-email-andriy.shevchenko@linux.intel.com>
The last standard PCI resource is defined as PCI_STD_RESOURCE_END. Thus, we
could use it instead of plain integer.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index e17a970..e45794d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -90,7 +90,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
}
/* Get the base address of device */
- for (i = 0; i <= 5; i++) {
+ for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
if (pci_resource_len(pdev, i) == 0)
continue;
addr = pci_iomap(pdev, i, 0);
--
2.1.1
^ permalink raw reply related
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