* Re: [PATCH v3 7/8] net: can: c_can: Add support for TI DRA7 DCAN
From: Roger Quadros @ 2014-11-05 13:36 UTC (permalink / raw)
To: Marc Kleine-Budde, wg
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <545A2679.2000905@pengutronix.de>
On 11/05/2014 03:30 PM, Marc Kleine-Budde wrote:
> On 11/04/2014 11:21 AM, Roger Quadros wrote:
>> DRA7 SoC has 2 CAN IPs. Provide compatible IDs and RAMINIT
>> register data for both.
>
> My understanding of the discussion with Wolfram was:
> - We should put the number of the Interface into to DT as a regmap
> parameter.
> - We put the method how to find the correct bits into the DT, via the
> compatible.
>
> So for both CAN instances on the DRA7 we have a single compatible
> "ti,dra7-d_can" and in the driver a mechanism that translates the number
> of the instance into the needed bit offsets, e.g. via two arrays.
>
OK. I'll revise this series.
The new syscon-raminit property will be like
syscon-raminit = <syscon_phandle raminit-reg-offset dcan-interface-number>;
cheers,
-roger
> Same comments for patch 8/8.
>
> Marc
>
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> Documentation/devicetree/bindings/net/can/c_can.txt | 1 +
>> drivers/net/can/c_can/c_can_platform.c | 16 ++++++++++++++++
>> 2 files changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
>> index 917ac0e..746cc07 100644
>> --- a/Documentation/devicetree/bindings/net/can/c_can.txt
>> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
>> @@ -4,6 +4,7 @@ Bosch C_CAN/D_CAN controller Device Tree Bindings
>> Required properties:
>> - compatible : Should be "bosch,c_can" for C_CAN controllers and
>> "bosch,d_can" for D_CAN controllers.
>> + Can be "ti,dra7-d_can1" or "ti,dra7-d_can2".
>> - reg : physical base address and size of the C_CAN/D_CAN
>> registers map
>> - interrupts : property with a value describing the interrupt
>> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
>> index d058820..dc618ce 100644
>> --- a/drivers/net/can/c_can/c_can_platform.c
>> +++ b/drivers/net/can/c_can/c_can_platform.c
>> @@ -195,6 +195,20 @@ static struct c_can_driver_data d_can_drvdata = {
>> .id = BOSCH_D_CAN,
>> };
>>
>> +static struct c_can_driver_data dra7_dcan1_drvdata = {
>> + .id = BOSCH_D_CAN,
>> + .raminit_start_bit = 3,
>> + .raminit_done_bit = 1,
>> + .raminit_pulse = true,
>> +};
>> +
>> +static struct c_can_driver_data dra7_dcan2_drvdata = {
>> + .id = BOSCH_D_CAN,
>> + .raminit_start_bit = 5,
>> + .raminit_done_bit = 2,
>> + .raminit_pulse = true,
>> +};
>> +
>> static struct platform_device_id c_can_id_table[] = {
>> {
>> .name = KBUILD_MODNAME,
>> @@ -215,6 +229,8 @@ MODULE_DEVICE_TABLE(platform, c_can_id_table);
>> static const struct of_device_id c_can_of_table[] = {
>> { .compatible = "bosch,c_can", .data = &c_can_drvdata },
>> { .compatible = "bosch,d_can", .data = &d_can_drvdata },
>> + { .compatible = "ti,dra7-d_can1", .data = &dra7_dcan1_drvdata },
>> + { .compatible = "ti,dra7-d_can2", .data = &dra7_dcan2_drvdata },
>> { /* sentinel */ },
>> };
>> MODULE_DEVICE_TABLE(of, c_can_of_table);
>>
>
>
^ permalink raw reply
* Re: [PATCH v3 7/8] net: can: c_can: Add support for TI DRA7 DCAN
From: Marc Kleine-Budde @ 2014-11-05 13:43 UTC (permalink / raw)
To: Roger Quadros, wg
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <545A27E8.9090600@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1175 bytes --]
On 11/05/2014 02:36 PM, Roger Quadros wrote:
> On 11/05/2014 03:30 PM, Marc Kleine-Budde wrote:
>> On 11/04/2014 11:21 AM, Roger Quadros wrote:
>>> DRA7 SoC has 2 CAN IPs. Provide compatible IDs and RAMINIT
>>> register data for both.
>>
>> My understanding of the discussion with Wolfram was:
>> - We should put the number of the Interface into to DT as a regmap
>> parameter.
>> - We put the method how to find the correct bits into the DT, via the
>> compatible.
>>
>> So for both CAN instances on the DRA7 we have a single compatible
>> "ti,dra7-d_can" and in the driver a mechanism that translates the number
>> of the instance into the needed bit offsets, e.g. via two arrays.
>>
>
> OK. I'll revise this series.
>
> The new syscon-raminit property will be like
> syscon-raminit = <syscon_phandle raminit-reg-offset dcan-interface-number>;
Yes. Wolfram?
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 1/4] can: m_can: update to support CAN FD features
From: Dong Aisheng @ 2014-11-05 13:46 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Oliver Hartkopp, linux-can, wg, varkabhadram, netdev,
linux-arm-kernel
In-Reply-To: <545A23DA.7030303@pengutronix.de>
On Wed, Nov 05, 2014 at 02:19:22PM +0100, Marc Kleine-Budde wrote:
> On 11/05/2014 02:10 PM, Oliver Hartkopp wrote:
> > On 05.11.2014 12:26, Dong Aisheng wrote:
> >> On Wed, Nov 05, 2014 at 11:12:24AM +0100, Oliver Hartkopp wrote:
> >>> On 05.11.2014 08:58, Dong Aisheng wrote:
> >
> >
> >>> 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.
> >
> > Yes. You are right. Having a FIFO with a size of 1 will help here :-)
>
> Errrr....sorry...no.
>
> Taking an easy route here but making it x times harder to extend the
> driver to make use of the FIFO is not an option.
>
Hmm, this way is just following the original approach the current driver
used. It's initial work and won't make things complicated.
Extend the driver to use FIFO for TX is another story and based on
my understanding it may be a bit complicated to do CAN FD mode switch on this
case due to hw limitation that the revision 3.0.x does not support per-frame
switching for FD/BRS as Oliver pointed out.
(e.g. how to switch FD MODE for each frame on Tx FIFO?)
Probably that's why the 3.1.x version will add the FD/BRS bit controller
in Tx Buffer to fix this issue.
Anyway, that's future work and we can discuss it when adding FIFO support
for Tx function.
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 net-next 2/2] net/mlx4_en: Support for configurable RSS hash function
From: Eyal perry @ 2014-11-05 13:48 UTC (permalink / raw)
To: Or Gerlitz, Eyal Perry
Cc: Amir Vadai, David S. Miller, Ben Hutchings, netdev,
Yevgeny Petrilin
In-Reply-To: <545A2664.7040001@mellanox.com>
On 11/5/2014 3:30 PM, Or Gerlitz wrote:
> On 11/5/2014 3:05 PM, Eyal perry wrote:
>> On 11/5/2014 2:27 PM, Or Gerlitz wrote:
>>>> +
>>>> +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;
>>> Seems that you are blindly setting here priv->rss_hash_fn to the
>>> function (xor or top) requested by the user w.o prior checking if the
>>> firmware actually supports that
>>> (e.g test it against priv->rss_hash_fn_caps, right?
>>>
>> That exactly what I do 3 lines above, priv->rss_hash_fn_caps is
>> derived from firmware capabilities.
>
> got it, thanks.
>
>>
>> [...]
>>
>>>> --- 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]);
>>>> + }
>>> So this patch effectively changes the default from top to xor, right? is
>>> that intentional? if yes, spare few words on the change log.
>>>
>> No, in general the default shouldn't be affected by the patch (unless
>> there's a bug). The default value is set in mlx4_en_init_netdev() and it
>> will remain Toeplitz unless firmware is supports only XOR hash function.
>>
>
> Oh, I see why I was confused and what I think need to be fixed. You are
> using both MLX4_RSS_HASH_TOP/XOR and RSS_HASH_TOP/XOR, any reason for that?
Yes, RSS_HASH_TOP/XOR are bits on a generic bitmask while the
MLX4_RSS_HASH_TOP/XOR are HW specific values.
>
> Also, priv->rss_hash_fn should equal one value, right? so this code "if
> (priv->rss_hash_fn & something)" is confusing, should be "if
> (priv->rss_hash_fn == something)"
That's right, I'll fix it for V1.
Thanks.
>
> Or.
^ permalink raw reply
* Re: [PATCH net-next 2/2] net/mlx4_en: Support for configurable RSS hash function
From: Or Gerlitz @ 2014-11-05 13:53 UTC (permalink / raw)
To: Eyal Perry
Cc: Amir Vadai, David S. Miller, Ben Hutchings, netdev,
Yevgeny Petrilin
In-Reply-To: <1415188769-19593-3-git-send-email-amirv@mellanox.com>
On 11/5/2014 1:59 PM, Amir Vadai wrote:
> --- 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;
> + }
> +
can we somehow change this code to be more clear and assign
priv->rss_hash_fn once?
> mdev->pndev[port] = dev;
>
> netif_carrier_off(dev);
^ permalink raw reply
* Re: [PATCH net-next v4 0/4] netns: allow to identify peer netns
From: Nicolas Dichtel @ 2014-11-05 14:23 UTC (permalink / raw)
To: Eric W. Biederman
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, luto-kltTT9wpgjJwATOyAt5JVQ,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
cwang-xCSkyg8dI+0RB7SZvlqPiA, linux-api-u79uwXL29TY76Z2rM5mHXA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <87wq7g831b.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
Le 31/10/2014 20:14, Eric W. Biederman a écrit :
> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>
>> Le 30/10/2014 19:41, Eric W. Biederman a écrit :
>>> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>>>
>>>> The goal of this serie is to be able to multicast netlink messages with an
>>>> attribute that identify a peer netns.
>>>> This is needed by the userland to interpret some informations contained in
>>>> netlink messages (like IFLA_LINK value, but also some other attributes in case
>>>> of x-netns netdevice (see also
>>>> http://thread.gmane.org/gmane.linux.network/315933/focus=316064 and
>>>> http://thread.gmane.org/gmane.linux.kernel.containers/28301/focus=4239)).
>>>>
>>>> Ids of peer netns are set by userland via a new genl messages. These ids are
>>>> stored per netns and are local (ie only valid in the netns where they are set).
>>>> To avoid allocating an int for each peer netns, I use idr_for_each() to retrieve
>>>> the id of a peer netns. Note that it will be possible to add a table (struct net
>>>> -> id) later to optimize this lookup if needed.
>>>>
>>>> Patch 1/4 introduces the netlink API mechanism to set and get these ids.
>>>> Patch 2/4 and 3/4 implements an example of how to use these ids in rtnetlink
>>>> messages. And patch 4/4 shows that the netlink messages can be symetric between
>>>> a GET and a SET.
>>>>
>>>> iproute2 patches are available, I can send them on demand.
>>>
>>> A quick reply. I think this patchset is in the right general direction.
>>> There are some oddball details that seem odd/awkward to me such as using
>>> genetlink instead of rtnetlink to get and set the ids, and not having
>>> ids if they are not set (that feels like a maintenance/usability challenge).
>> No problem to use rtnetlink, in fact, I hesitated.
>>
>> For the second point, I'm not sure to follow you: how to have an id, which will
>> not break migration, without asking the user to set it?
>
> We have that situtation with ifindex already. Basically the thought is
> to allow an id to be set, but also allow an id to be auto-generated if
> we use an namespace without an id being set.
If my understanding is correct, the difference is that we want to hide some
netns.
Do you think we can generate an id for each netns that does not have one and
relying on the fact that this id has no meaning unless you have a netns file
descriptor that allow you to get the id of this netns?
Regards,
Nicolas
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers
^ permalink raw reply
* [PATCH V3 2/3] can: m_can: update to support CAN FD features
From: Dong Aisheng @ 2014-11-05 13:16 UTC (permalink / raw)
To: linux-can
Cc: mkl, wg, varkabhadram, netdev, socketcan, b29396,
linux-arm-kernel
In-Reply-To: <1415193393-30023-1-git-send-email-b29396@freescale.com>
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>
---
ChangeLog:
v2->v3:
* integrate patch 4 into patch 1 in last series(allow to send std frame
on can fd mode)
* use suitable API to get cf->len according to RX_BUF_EDL bit
v1->v2:
* Allocate can frame or canfd frame based on EDL bit
* Only check and set RTR bit for normal frame (no EDL bit set)
---
drivers/net/can/m_can/m_can.c | 179 ++++++++++++++++++++++++++++++++----------
1 file changed, 136 insertions(+), 43 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 6160b9c..eee1533 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -105,14 +105,36 @@ enum m_can_mram_cfg {
MRAM_CFG_NUM,
};
+/* Fast Bit Timing & Prescaler Register (FBTP) */
+#define FBTR_FBRP_MASK 0x1f
+#define FBTR_FBRP_SHIFT 16
+#define FBTR_FTSEG1_SHIFT 8
+#define FBTR_FTSEG1_MASK (0xf << FBTR_FTSEG1_SHIFT)
+#define FBTR_FTSEG2_SHIFT 4
+#define FBTR_FTSEG2_MASK (0x7 << FBTR_FTSEG2_SHIFT)
+#define FBTR_FSJW_SHIFT 0
+#define FBTR_FSJW_MASK 0x3
+
/* Test Register (TEST) */
#define TEST_LBCK BIT(4)
/* CC Control Register(CCCR) */
-#define CCCR_TEST BIT(7)
-#define CCCR_MON BIT(5)
-#define CCCR_CCE BIT(1)
-#define CCCR_INIT BIT(0)
+#define CCCR_TEST BIT(7)
+#define CCCR_CMR_MASK 0x3
+#define CCCR_CMR_SHIFT 10
+#define CCCR_CMR_CANFD 0x1
+#define CCCR_CMR_CANFD_BRS 0x2
+#define CCCR_CMR_CAN 0x3
+#define CCCR_CME_MASK 0x3
+#define CCCR_CME_SHIFT 8
+#define CCCR_CME_CAN 0
+#define CCCR_CME_CANFD 0x1
+#define CCCR_CME_CANFD_BRS 0x2
+#define CCCR_TEST BIT(7)
+#define CCCR_MON BIT(5)
+#define CCCR_CCE BIT(1)
+#define CCCR_INIT BIT(0)
+#define CCCR_CANFD 0x10
/* Bit Timing & Prescaler Register (BTP) */
#define BTR_BRP_MASK 0x3ff
@@ -204,6 +226,7 @@ enum m_can_mram_cfg {
/* Rx Buffer / FIFO Element Size Configuration (RXESC) */
#define M_CAN_RXESC_8BYTES 0x0
+#define M_CAN_RXESC_64BYTES 0x777
/* Tx Buffer Configuration(TXBC) */
#define TXBC_NDTB_OFF 16
@@ -211,6 +234,7 @@ enum m_can_mram_cfg {
/* Tx Buffer Element Size Configuration(TXESC) */
#define TXESC_TBDS_8BYTES 0x0
+#define TXESC_TBDS_64BYTES 0x7
/* Tx Event FIFO Con.guration (TXEFC) */
#define TXEFC_EFS_OFF 16
@@ -219,11 +243,11 @@ enum m_can_mram_cfg {
/* Message RAM Configuration (in bytes) */
#define SIDF_ELEMENT_SIZE 4
#define XIDF_ELEMENT_SIZE 8
-#define RXF0_ELEMENT_SIZE 16
-#define RXF1_ELEMENT_SIZE 16
+#define RXF0_ELEMENT_SIZE 72
+#define RXF1_ELEMENT_SIZE 72
#define RXB_ELEMENT_SIZE 16
#define TXE_ELEMENT_SIZE 8
-#define TXB_ELEMENT_SIZE 16
+#define TXB_ELEMENT_SIZE 72
/* Message RAM Elements */
#define M_CAN_FIFO_ID 0x0
@@ -231,11 +255,17 @@ enum m_can_mram_cfg {
#define M_CAN_FIFO_DATA(n) (0x8 + ((n) << 2))
/* Rx Buffer Element */
+/* R0 */
#define RX_BUF_ESI BIT(31)
#define RX_BUF_XTD BIT(30)
#define RX_BUF_RTR BIT(29)
+/* R1 */
+#define RX_BUF_ANMF BIT(31)
+#define RX_BUF_EDL BIT(21)
+#define RX_BUF_BRS BIT(20)
/* Tx Buffer Element */
+/* R0 */
#define TX_BUF_XTD BIT(30)
#define TX_BUF_RTR BIT(29)
@@ -327,41 +357,68 @@ 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);
+ 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));
+ if (dlc & RX_BUF_EDL)
+ cf->len = can_dlc2len((id >> 16) & 0x0F);
+ else
+ cf->len = get_can_dlc((id >> 16) & 0x0F);
+
+ if (id & RX_BUF_BRS)
+ cf->flags |= CANFD_BRS;
+
+ for (i = 0; i < cf->len; i += 4)
+ *(u32 *)(cf->data + i) =
+ m_can_fifo_read(priv, fgi,
+ M_CAN_FIFO_DATA(i / 4));
}
/* acknowledge rx fifo 0 */
m_can_write(priv, M_CAN_RXF0A, fgi);
+
+ stats->rx_packets++;
+ stats->rx_bytes += cf->len;
+
+ netif_receive_skb(skb);
}
static int m_can_do_rx_poll(struct net_device *dev, int quota)
{
struct m_can_priv *priv = netdev_priv(dev);
- struct net_device_stats *stats = &dev->stats;
- struct sk_buff *skb;
- struct can_frame *frame;
u32 pkts = 0;
u32 rxfs;
@@ -375,18 +432,7 @@ static int m_can_do_rx_poll(struct net_device *dev, int quota)
if (rxfs & RXFS_RFL)
netdev_warn(dev, "Rx FIFO 0 Message Lost\n");
- skb = alloc_can_skb(dev, &frame);
- if (!skb) {
- stats->rx_dropped++;
- return pkts;
- }
-
- m_can_read_fifo(dev, frame, rxfs);
-
- stats->rx_packets++;
- stats->rx_bytes += frame->can_dlc;
-
- netif_receive_skb(skb);
+ m_can_read_fifo(dev, rxfs);
quota--;
pkts++;
@@ -744,10 +790,23 @@ static const struct can_bittiming_const m_can_bittiming_const = {
.brp_inc = 1,
};
+static const struct can_bittiming_const m_can_data_bittiming_const = {
+ .name = KBUILD_MODNAME,
+ .tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */
+ .tseg1_max = 16,
+ .tseg2_min = 1, /* Time segment 2 = phase_seg2 */
+ .tseg2_max = 8,
+ .sjw_max = 4,
+ .brp_min = 1,
+ .brp_max = 32,
+ .brp_inc = 1,
+};
+
static int m_can_set_bittiming(struct net_device *dev)
{
struct m_can_priv *priv = netdev_priv(dev);
const struct can_bittiming *bt = &priv->can.bittiming;
+ const struct can_bittiming *dbt = &priv->can.data_bittiming;
u16 brp, sjw, tseg1, tseg2;
u32 reg_btp;
@@ -758,7 +817,17 @@ static int m_can_set_bittiming(struct net_device *dev)
reg_btp = (brp << BTR_BRP_SHIFT) | (sjw << BTR_SJW_SHIFT) |
(tseg1 << BTR_TSEG1_SHIFT) | (tseg2 << BTR_TSEG2_SHIFT);
m_can_write(priv, M_CAN_BTP, reg_btp);
- netdev_dbg(dev, "setting BTP 0x%x\n", reg_btp);
+
+ if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
+ brp = dbt->brp - 1;
+ sjw = dbt->sjw - 1;
+ tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
+ tseg2 = dbt->phase_seg2 - 1;
+ reg_btp = (brp << FBTR_FBRP_SHIFT) | (sjw << FBTR_FSJW_SHIFT) |
+ (tseg1 << FBTR_FTSEG1_SHIFT) |
+ (tseg2 << FBTR_FTSEG2_SHIFT);
+ m_can_write(priv, M_CAN_FBTP, reg_btp);
+ }
return 0;
}
@@ -778,8 +847,8 @@ static void m_can_chip_config(struct net_device *dev)
m_can_config_endisable(priv, true);
- /* RX Buffer/FIFO Element Size 8 bytes data field */
- m_can_write(priv, M_CAN_RXESC, M_CAN_RXESC_8BYTES);
+ /* RX Buffer/FIFO Element Size 64 bytes data field */
+ m_can_write(priv, M_CAN_RXESC, M_CAN_RXESC_64BYTES);
/* Accept Non-matching Frames Into FIFO 0 */
m_can_write(priv, M_CAN_GFC, 0x0);
@@ -788,8 +857,8 @@ static void m_can_chip_config(struct net_device *dev)
m_can_write(priv, M_CAN_TXBC, (1 << TXBC_NDTB_OFF) |
priv->mcfg[MRAM_TXB].off);
- /* only support 8 bytes firstly */
- m_can_write(priv, M_CAN_TXESC, TXESC_TBDS_8BYTES);
+ /* support 64 bytes payload */
+ m_can_write(priv, M_CAN_TXESC, TXESC_TBDS_64BYTES);
m_can_write(priv, M_CAN_TXEFC, (1 << TXEFC_EFS_OFF) |
priv->mcfg[MRAM_TXE].off);
@@ -804,7 +873,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 +886,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);
@@ -880,11 +953,13 @@ static struct net_device *alloc_m_can_dev(void)
priv->dev = dev;
priv->can.bittiming_const = &m_can_bittiming_const;
+ priv->can.data_bittiming_const = &m_can_data_bittiming_const;
priv->can.do_set_mode = m_can_set_mode;
priv->can.do_get_berr_counter = m_can_get_berr_counter;
priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
CAN_CTRLMODE_LISTENONLY |
- CAN_CTRLMODE_BERR_REPORTING;
+ CAN_CTRLMODE_BERR_REPORTING |
+ CAN_CTRLMODE_FD;
return dev;
}
@@ -967,8 +1042,9 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
struct net_device *dev)
{
struct m_can_priv *priv = netdev_priv(dev);
- struct can_frame *cf = (struct can_frame *)skb->data;
- u32 id;
+ struct canfd_frame *cf = (struct canfd_frame *)skb->data;
+ u32 id, cccr;
+ int i;
if (can_dropped_invalid_skb(dev, skb))
return NETDEV_TX_OK;
@@ -987,11 +1063,28 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
/* message ram configuration */
m_can_fifo_write(priv, 0, M_CAN_FIFO_ID, id);
- m_can_fifo_write(priv, 0, M_CAN_FIFO_DLC, cf->can_dlc << 16);
- m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(0), *(u32 *)(cf->data + 0));
- m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(1), *(u32 *)(cf->data + 4));
+ m_can_fifo_write(priv, 0, M_CAN_FIFO_DLC, can_len2dlc(cf->len) << 16);
+
+ for (i = 0; i < cf->len; i += 4)
+ m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(i / 4),
+ *(u32 *)(cf->data + i));
+
can_put_echo_skb(skb, dev, 0);
+ if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
+ cccr = m_can_read(priv, M_CAN_CCCR);
+ cccr &= ~(CCCR_CMR_MASK << CCCR_CMR_SHIFT);
+ if (can_is_canfd_skb(skb)) {
+ if (cf->flags & CANFD_BRS)
+ cccr |= CCCR_CMR_CANFD_BRS << CCCR_CMR_SHIFT;
+ else
+ cccr |= CCCR_CMR_CANFD << CCCR_CMR_SHIFT;
+ } else {
+ cccr |= CCCR_CMR_CAN << CCCR_CMR_SHIFT;
+ }
+ m_can_write(priv, M_CAN_CCCR, cccr);
+ }
+
/* enable first TX buffer to start transfer */
m_can_write(priv, M_CAN_TXBTIE, 0x1);
m_can_write(priv, M_CAN_TXBAR, 0x1);
--
1.9.1
^ permalink raw reply related
* [PATCH V3 3/3] can: m_can: workaround for transmit data less than 4 bytes
From: Dong Aisheng @ 2014-11-05 13:16 UTC (permalink / raw)
To: linux-can
Cc: mkl, wg, varkabhadram, netdev, socketcan, b29396,
linux-arm-kernel
In-Reply-To: <1415193393-30023-1-git-send-email-b29396@freescale.com>
At least on the i.MX6SX TO1.2 with M_CAN IP version 3.0.1, 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 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
v2->v3:
* add i.MX chip version in issue in commit message
v1->v2:
* 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 | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index eee1533..567cd27 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -905,6 +905,16 @@ static void m_can_chip_config(struct net_device *dev)
/* set bittiming params */
m_can_set_bittiming(dev);
+ /* At least on the i.MX6SX TO1.2 with M_CAN IP version 3.0.1,
+ * (CREL = 30130506) 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);
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH V3 3/3] can: m_can: workaround for transmit data less than 4 bytes
From: Marc Kleine-Budde @ 2014-11-05 14:29 UTC (permalink / raw)
To: Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1415193393-30023-3-git-send-email-b29396@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]
On 11/05/2014 02:16 PM, Dong Aisheng wrote:
> At least on the i.MX6SX TO1.2 with M_CAN IP version 3.0.1, 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 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>
Applied to can/master
Thanks,
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 V3 2/3] can: m_can: update to support CAN FD features
From: Marc Kleine-Budde @ 2014-11-05 14:31 UTC (permalink / raw)
To: Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1415193393-30023-2-git-send-email-b29396@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 952 bytes --]
On 11/05/2014 02:16 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>
I'm waiting on Oliver's Ack on this patch, then I'll apply 1+2.
regards,
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
* [PATCH] net: mv643xx_eth: reclaim TX skbs only when released by the HW
From: Karl Beldan @ 2014-11-05 14:32 UTC (permalink / raw)
To: David Miller
Cc: Karl Beldan, netdev, Ian Campbell, Eric Dumazet, Ezequiel Garcia,
Sebastian Hesselbarth
From: Karl Beldan <karl.beldan@rivierawaves.com>
ATM, txq_reclaim will dequeue and free an skb for each tx desc released
by the hw that has TX_LAST_DESC set. However, in case of TSO, each
hw desc embedding the last part of a segment has TX_LAST_DESC set,
losing the one-to-one 'last skb frag'/'TX_LAST_DESC set' correspondance,
which causes data corruption.
Fix this by checking TX_ENABLE_INTERRUPT instead of TX_LAST_DESC, and
warn when trying to dequeue from an empty txq (which can be symptomatic
of releasing skbs prematurely).
Fixes: 3ae8f4e0b98 ('net: mv643xx_eth: Implement software TSO')
Reported-by: Slawomir Gajzner <slawomir.gajzner@gmail.com>
Reported-by: Julien D'Ascenzio <jdascenzio@yahoo.fr>
Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
Cc: Ian Campbell <ijc@hellion.org.uk>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Ian, I refrained myself from embedding your Tested-by since this change is a
little bit different from my first.
David, please consider queueing this one up for -stable, and possibly adjust
Ian's Tested-by (I am not sure about the process).
drivers/net/ethernet/marvell/mv643xx_eth.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index b151a94..d44560d 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -1047,7 +1047,6 @@ static int txq_reclaim(struct tx_queue *txq, int budget, int force)
int tx_index;
struct tx_desc *desc;
u32 cmd_sts;
- struct sk_buff *skb;
tx_index = txq->tx_used_desc;
desc = &txq->tx_desc_area[tx_index];
@@ -1066,19 +1065,22 @@ static int txq_reclaim(struct tx_queue *txq, int budget, int force)
reclaimed++;
txq->tx_desc_count--;
- skb = NULL;
- if (cmd_sts & TX_LAST_DESC)
- skb = __skb_dequeue(&txq->tx_skb);
+ if (!IS_TSO_HEADER(txq, desc->buf_ptr))
+ dma_unmap_single(mp->dev->dev.parent, desc->buf_ptr,
+ desc->byte_cnt, DMA_TO_DEVICE);
+
+ if (cmd_sts & TX_ENABLE_INTERRUPT) {
+ struct sk_buff *skb = __skb_dequeue(&txq->tx_skb);
+
+ if (!WARN_ON(!skb))
+ dev_kfree_skb(skb);
+ }
if (cmd_sts & ERROR_SUMMARY) {
netdev_info(mp->dev, "tx error\n");
mp->dev->stats.tx_errors++;
}
- if (!IS_TSO_HEADER(txq, desc->buf_ptr))
- dma_unmap_single(mp->dev->dev.parent, desc->buf_ptr,
- desc->byte_cnt, DMA_TO_DEVICE);
- dev_kfree_skb(skb);
}
__netif_tx_unlock_bh(nq);
--
2.0.1
^ permalink raw reply related
* Re: [PATCH V2 1/4] can: m_can: update to support CAN FD features
From: Oliver Hartkopp @ 2014-11-05 14:35 UTC (permalink / raw)
To: Dong Aisheng, Marc Kleine-Budde
Cc: linux-can, wg, varkabhadram, netdev, linux-arm-kernel
In-Reply-To: <20141105134625.GG4007@shlinux1.ap.freescale.net>
On 05.11.2014 14:46, Dong Aisheng wrote:
> On Wed, Nov 05, 2014 at 02:19:22PM +0100, Marc Kleine-Budde wrote:
>> On 11/05/2014 02:10 PM, Oliver Hartkopp wrote:
>>> On 05.11.2014 12:26, Dong Aisheng wrote:
>>>> On Wed, Nov 05, 2014 at 11:12:24AM +0100, Oliver Hartkopp wrote:
>>>>> On 05.11.2014 08:58, Dong Aisheng wrote:
>>>
>>>
>>>>> 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 th
> 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 |
>>
>
>
at case that CAN mode is switched during transfer.
>>>> So the issue you concern may not happen.
>>>
>>> Yes. You are right. Having a FIFO with a size of 1 will help here :-)
>>
>> Errrr....sorry...no.
>>
>> Taking an easy route here but making it x times harder to extend the
>> driver to make use of the FIFO is not an option.
>>
>
> Hmm, this way is just following the original approach the current driver
> used. It's initial work and won't make things complicated.
>
> Extend the driver to use FIFO for TX is another story and based on
> my understanding it may be a bit complicated to do CAN FD mode switch on this
> case due to hw limitation that the revision 3.0.x does not support per-frame
> switching for FD/BRS as Oliver pointed out.
> (e.g. how to switch FD MODE for each frame on Tx FIFO?)
> Probably that's why the 3.1.x version will add the FD/BRS bit controller
> in Tx Buffer to fix this issue.
>
> Anyway, that's future work and we can discuss it when adding FIFO support
> for Tx function.
>
Yes. I have to second this opinion.
I also would like to have a TX FIFO. But due to the limitations of the 3.0.x
M_CAN I would suggest to prefer a 'correct" CAN FD driver implementation in
favor of having a TX FIFO which is unusable for mixed CAN frame types.
Let's try the FIFO stuff with the next M_CAN revision.
It's a bit of a SJA1000 for CAN FD :-)
Regards,
Oliver
^ permalink raw reply
* Re: [PATCH V3 2/3] can: m_can: update to support CAN FD features
From: Oliver Hartkopp @ 2014-11-05 14:42 UTC (permalink / raw)
To: Marc Kleine-Budde, Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, linux-arm-kernel
In-Reply-To: <545A34B9.2060001@pengutronix.de>
Looks good to me now.
Thanks for your patience.
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
On 05.11.2014 15:31, Marc Kleine-Budde wrote:
> On 11/05/2014 02:16 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>
>
> I'm waiting on Oliver's Ack on this patch, then I'll apply 1+2.
>
> regards,
> Marc
>
^ permalink raw reply
* [PATCH net v3] ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs
From: Daniel Borkmann @ 2014-11-05 14:42 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>
---
v2->v3:
- Still had a discussion w/ Hannes and improved the code a bit to
make it more clear to read
v1->v2:
- Don't introduce skb_nofrag_tailroom(), but reuse skb_availroom()
as suggested by Eric
net/ipv4/igmp.c | 17 +++++++----------
net/ipv6/mcast.c | 19 ++++++++++---------
2 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index fb70e3e..d90bdbf 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -318,9 +318,7 @@ 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)
+static struct sk_buff *igmpv3_newpack(struct net_device *dev, unsigned int mtu)
{
struct sk_buff *skb;
struct rtable *rt;
@@ -330,6 +328,7 @@ static struct sk_buff *igmpv3_newpack(struct net_device *dev, int size)
struct flowi4 fl4;
int hlen = LL_RESERVED_SPACE(dev);
int tlen = dev->needed_tailroom;
+ unsigned int size = mtu;
while (1) {
skb = alloc_skb(size + hlen + tlen,
@@ -340,20 +339,19 @@ static struct sk_buff *igmpv3_newpack(struct net_device *dev, int size)
if (size < 256)
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,
- IPPROTO_IGMP, 0, dev->ifindex);
+ 0, 0, IPPROTO_IGMP, 0, dev->ifindex);
if (IS_ERR(rt)) {
kfree_skb(skb);
return NULL;
}
+ skb->priority = TC_PRIO_CONTROL;
skb_dst_set(skb, &rt->dst);
skb->dev = dev;
-
+ skb->reserved_tailroom = skb_end_offset(skb) -
+ min(mtu, skb_end_offset(skb));
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..d817737 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1550,7 +1550,7 @@ static void ip6_mc_hdr(struct sock *sk, struct sk_buff *skb,
hdr->daddr = *daddr;
}
-static struct sk_buff *mld_newpack(struct inet6_dev *idev, int size)
+static struct sk_buff *mld_newpack(struct inet6_dev *idev, unsigned int mtu)
{
struct net_device *dev = idev->dev;
struct net *net = dev_net(dev);
@@ -1560,22 +1560,23 @@ static struct sk_buff *mld_newpack(struct inet6_dev *idev, int size)
struct in6_addr addr_buf;
const struct in6_addr *saddr;
int hlen = LL_RESERVED_SPACE(dev);
- int tlen = dev->needed_tailroom;
- int err;
+ int err, tlen = dev->needed_tailroom;
+ unsigned int size = mtu + hlen + tlen;
u8 ra[8] = { IPPROTO_ICMPV6, 0,
IPV6_TLV_ROUTERALERT, 2, 0, 0,
IPV6_TLV_PADN, 0 };
- /* we assume size > sizeof(ra) here */
- size += hlen + tlen;
- /* limit our allocations to order-0 page */
+ /* We assume size > sizeof(ra) here. 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 = skb_end_offset(skb) -
+ min(mtu, skb_end_offset(skb));
skb_reserve(skb, hlen);
if (__ipv6_get_lladdr(idev, &addr_buf, IFA_F_TENTATIVE)) {
@@ -1599,6 +1600,7 @@ static struct sk_buff *mld_newpack(struct inet6_dev *idev, int size)
pmr->mld2r_cksum = 0;
pmr->mld2r_resv2 = 0;
pmr->mld2r_ngrec = 0;
+
return skb;
}
@@ -1690,8 +1692,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
* [PATCH] net/9p: remove a comment about pref member which doesn't exist
From: Ryo Munakata @ 2014-11-05 14:45 UTC (permalink / raw)
To: ericvh; +Cc: rminnich, lucho, davem, netdev, linux-kernel, Ryo Munakata
Signed-off-by: Ryo Munakata <ryomnktml@gmail.com>
---
include/net/9p/transport.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/net/9p/transport.h b/include/net/9p/transport.h
index d9fa68f..2a25dec 100644
--- a/include/net/9p/transport.h
+++ b/include/net/9p/transport.h
@@ -34,7 +34,6 @@
* @list: used to maintain a list of currently available transports
* @name: the human-readable name of the transport
* @maxsize: transport provided maximum packet size
- * @pref: Preferences of this transport
* @def: set if this transport should be considered the default
* @create: member function to create a new connection on this transport
* @close: member function to discard a connection on this transport
--
2.1.3
^ permalink raw reply related
* Re: [PATCH] net: mv643xx_eth: reclaim TX skbs only when released by the HW
From: Ezequiel Garcia @ 2014-11-05 14:46 UTC (permalink / raw)
To: Karl Beldan, David Miller, Ian Campbell
Cc: Karl Beldan, netdev, Eric Dumazet, Sebastian Hesselbarth
In-Reply-To: <1415197979-1702-1-git-send-email-karl.beldan@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]
Hi Karl,
On 11/05/2014 11:32 AM, Karl Beldan wrote:> From: Karl Beldan <karl.beldan@rivierawaves.com>
>
> ATM, txq_reclaim will dequeue and free an skb for each tx desc released
> by the hw that has TX_LAST_DESC set. However, in case of TSO, each
> hw desc embedding the last part of a segment has TX_LAST_DESC set,
> losing the one-to-one 'last skb frag'/'TX_LAST_DESC set' correspondance,
> which causes data corruption.
>
> Fix this by checking TX_ENABLE_INTERRUPT instead of TX_LAST_DESC, and
> warn when trying to dequeue from an empty txq (which can be symptomatic
> of releasing skbs prematurely).
>
> Fixes: 3ae8f4e0b98 ('net: mv643xx_eth: Implement software TSO')
Although your change makes sense, this isn't fixing the issue for me,
neither did the previous one.
Ian: Can you double check that you have corruption *without* the patch,
and that the patch fixes the issue?
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 1/7] bpf: add 'flags' attribute to BPF_MAP_UPDATE_ELEM command
From: Daniel Borkmann @ 2014-11-05 14:57 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Andy Lutomirski,
Hannes Frederic Sowa, Eric Dumazet, Linux API,
Network Development, LKML
In-Reply-To: <CAMEtUuy5gJbeLqiSr1=SiNQ7WyqocUVV-siwhEnpBVqmzYzzCQ@mail.gmail.com>
On 11/05/2014 12:04 AM, Alexei Starovoitov wrote:
> On Tue, Nov 4, 2014 at 1:25 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
>> On 11/04/2014 03:54 AM, Alexei Starovoitov wrote:
>>>
>>> the current meaning of BPF_MAP_UPDATE_ELEM syscall command is:
>>> either update existing map element or create a new one.
>>> Initially the plan was to add a new command to handle the case of
>>> 'create new element if it didn't exist', but 'flags' style looks
>>> cleaner and overall diff is much smaller (more code reused), so add 'flags'
>>> attribute to BPF_MAP_UPDATE_ELEM command with the following meaning:
>>> enum {
>>> BPF_MAP_UPDATE_OR_CREATE = 0, /* add new element or update existing */
>>> BPF_MAP_CREATE_ONLY, /* add new element if it didn't exist */
>>> BPF_MAP_UPDATE_ONLY /* update existing element */
>>> };
>>
>> From you commit message/code I currently don't see an explanation why
>> it cannot be done in typical ``flags style'' as various syscalls do,
>> i.e. BPF_MAP_UPDATE_OR_CREATE rather represented as ...
>>
>> BPF_MAP_CREATE | BPF_MAP_UPDATE
>>
>> Do you expect more than 64 different flags to be passed from user space
>> for BPF_MAP?
>
> several reasons:
> - preserve flags==0 as default behavior
> - avoid holes and extra checks for invalid combinations, so
> if (flags > BPF_MAP_UPDATE_ONLY) goto err, is enough.
> - it looks much neater when user space uses
> BPF_MAP_UPDATE_OR_CREATE instead of ORing bits.
>
> Note this choice doesn't prevent adding bit-like flags
> in the future. Today I cannot think of any new flags
> for the update() command, but if somebody comes up with
> a new selector that can apply to all three combinations,
> we can add it as 3rd bit that can be ORed.
Hm, mixing enums together with bitfield-like flags seems
kind of hacky ... :/ Or, do you mean to say you're adding
a 2nd flag field, i.e. splitting the 64bits into a 32bit
``cmd enum'' and 32bit ``flag section''?
I see the point with flags == 0 as default behavior though,
but at this point in time we won't get burnt by it since
the API is not yet in a usable state and defaults to be
compiled-out.
> Default will stay zero and 'if >' check in older
> kernels will seamlessly work with new userspace.
> I don't like holes in flags and combinatorial
> explosion of bits and checks for them unless
> absolutely necessary.
Hm, my concern is that we start to add many *_OR_* enum
elements once we find that a flag might be a useful in
combination with many other flags ... even though if we
only can think of 3 flags /right now/.
^ permalink raw reply
* Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net onarndale platform
From: Charles Keepax @ 2014-11-05 15:02 UTC (permalink / raw)
To: Stam, Michel [FINT]
Cc: Riku Voipio, davem, linux-usb, netdev, linux-kernel,
linux-samsung-soc
In-Reply-To: <C89EFD3CD56F64468D3D206D683A8D22039FFC5F@ldam-msx2.fugro-nl.local>
On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> 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.
Odd it definitely reads back as zero on Arndale. I am guessing
that the root of the problem here is that for some reason Arndale
POR of the ethernet is pants and it needs a full software reset
before it will work and the patch removes the full reset
callback.
>
> 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
Ok so I am guessing you have a value in the register which is
neither the reset value or 0 and this causing problems later in
the reset/on the next reset. I do find the naming confusing in
the error message there as it says link reset failed but the
link_reset callback can't fail in the driver and I modified the
reset callback. But I guess that is just oddities of the network
stack I am not familiar with.
The other thing that feels odd is (and again apologies as I know
next to nothing about the networking stack) how come it is
unexpected that the reset callback destroys the state of the
device. Naively I would have expected that a reset callback would
reset the device back to its default state. Here we seem to be
trying to avoid that happening.
>
> 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?
Yeah mine also idVendor=0b95, idProduct=772a
Thanks,
Charles
>
> 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
* mlx4+vxlan offload breaks gre tunnels
From: Florian Westphal @ 2014-11-05 15:04 UTC (permalink / raw)
To: netdev; +Cc: amirv, ogerlitz
tl,dr: all tcp packets sent via gre tunnel have broken tcp csum if vxlan offload
is enabled with mlx4 driver.
Given following config on tx-side:
dev=enp3s0
ip addr add dev $dev 192.168.23.1/24
ip link set $dev up
ip link add mygre type gretap remote 192.168.23.2 local 192.168.23.1
ip addr add dev mygre 192.168.42.1/24
ip link set gre0 up
ip link set mygre up
and
options mlx4_core log_num_mgm_entry_size=-1 debug_level=1
port_type_array=2,2
in
/etc/modprobe.d/mlx4.conf
all tcp packets sent to destinations over the gre tunnel have bogus tcp
checksums (and are tossed on rx side when stack validates tcp checksum).
net-next head is commit 30349bdbc4da5ecf0efa25556e3caff9c9b8c5f7 .
What makes things work for me:
either
options mlx4_core 1 debug_level=1 port_type_array=2,2
(ie. no MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
or not setting NETIF_F_IP_CSUM in enc_features:
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2579,10 +2579,12 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
dev->priv_flags |= IFF_UNICAST_FLT;
if (mdev->dev->caps.tunnel_offload_mode == MLX4_TUNNEL_OFFLOAD_MODE_VXLAN) {
- dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
+ dev->hw_enc_features |= NETIF_F_RXCSUM |
NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL;
I am not sure if its right fix, but to my eyes this basically looks like
mlx4 is telling stack that it can handle tcp checksum offload within
tunnels, and that doesn't seem to be the case for all types (e.g. gre).
Could someone who understand the enc_features specifics better confirm that
above patch is correct (or provide a better/proper fix)?
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH] net: mv643xx_eth: reclaim TX skbs only when released by the HW
From: Karl Beldan @ 2014-11-05 15:05 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: David Miller, Ian Campbell, Karl Beldan, netdev, Eric Dumazet,
Sebastian Hesselbarth
In-Reply-To: <545A3838.3090606@free-electrons.com>
On Wed, Nov 05, 2014 at 11:46:16AM -0300, Ezequiel Garcia wrote:
> Hi Karl,
>
> On 11/05/2014 11:32 AM, Karl Beldan wrote:> From: Karl Beldan <karl.beldan@rivierawaves.com>
> >
> > ATM, txq_reclaim will dequeue and free an skb for each tx desc released
> > by the hw that has TX_LAST_DESC set. However, in case of TSO, each
> > hw desc embedding the last part of a segment has TX_LAST_DESC set,
> > losing the one-to-one 'last skb frag'/'TX_LAST_DESC set' correspondance,
> > which causes data corruption.
> >
> > Fix this by checking TX_ENABLE_INTERRUPT instead of TX_LAST_DESC, and
> > warn when trying to dequeue from an empty txq (which can be symptomatic
> > of releasing skbs prematurely).
> >
> > Fixes: 3ae8f4e0b98 ('net: mv643xx_eth: Implement software TSO')
>
> Although your change makes sense, this isn't fixing the issue for me,
> neither did the previous one.
>
This change fixes a serious issue.
On my side I can now trigger misc NFS and md5sums errors very easily,
which I haven't detected so far with it applied.
Are you running little endian ? Do you have the tso alignment fix
a63ba13e (I don't expect it to be required but I don't know what SoC you
are using) ? I suppose you are running with all 3 fixes applied.
Karl
^ permalink raw reply
* Re: [PATCH 02/20] selftests/net: move test out of Makefile into a shell script
From: Shuah Khan @ 2014-11-05 15:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20141104.145315.548353349719796428.davem@davemloft.net>
On 11/04/2014 12:53 PM, David Miller wrote:
> From: Shuah Khan <shuahkh@osg.samsung.com>
> Date: Tue, 4 Nov 2014 10:10:58 -0700
>
>> Currently bpf test run from the Makefile. Move it out of the
>> Makefile to be run from a shell script to allow the test to
>> be run as stand-alone test, in addition to allowing the test
>> run from a make target.
>>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>
> Acked-by: David S. Miller <davem@davemloft.net>
>
Thanks. Queuing this patch up for 3.19-rc1.
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ permalink raw reply
* Re: [PATCH v2 net] tcp: zero retrans_stamp if all retrans were acked
From: Marcelo Ricardo Leitner @ 2014-11-05 15:32 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Neal Cardwell, Netdev, Yuchung Cheng, Eric Dumazet
In-Reply-To: <1415150445.1458.1.camel@edumazet-glaptop2.roam.corp.google.com>
On 04-11-2014 23:20, Eric Dumazet wrote:
> On Tue, 2014-11-04 at 18:51 -0200, Marcelo Ricardo Leitner wrote:
>
>> And thank you guys for all the assistance on it. Btw, would you send me that
>> packetdrill script? I'm curious to see how such corner case could be written
>> on it.
>
> One of the script I saw was :
>
> You might have to adapt preconditions (tcp_rmem[]/tcp_wmem[])
>
> 0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
> // Set a 10s timeout
> +.000 setsockopt(3, SOL_TCP, TCP_USER_TIMEOUT, [10000], 4) = 0
> +.000 bind(3, ..., ...) = 0
> +.000 listen(3, 1) = 0
> +.000 < S 0:0(0) win 32792 <mss 1460,nop,wscale 7>
> +.000 > S. 0:0(0) ack 1 <mss 1460,nop,wscale 6>
> +.010 < . 1:1(0) ack 1 win 257
> +.000 accept(3, ..., ...) = 4
> +.000 write(4, ..., 1000) = 1000
> +.000 > P. 1:1001(1000) ack 1
> +.625 > P. 1:1001(1000) ack 1
> +.020 < . 1:1(0) ack 1001 win 257
> // Purposely write more after the specified timeout for testing
> +11.0 write(4, ..., 1000) = 1000
> +.000 > P. 1001:2001(1000) ack 1
> +1.25 > P. 1001:2001(1000) ack 1
> // socket is killed when the 2nd RTO fires at +2.50 w/o this patch
> // so the next write returns ETIMEOUT
> +2.60 write(4, ..., 1000) = 1000
Cool, thanks!
Marcelo
^ permalink raw reply
* Re: [PATCH] net: mv643xx_eth: reclaim TX skbs only when released by the HW
From: Eric Dumazet @ 2014-11-05 15:41 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Karl Beldan, David Miller, Ian Campbell, Karl Beldan, netdev,
Sebastian Hesselbarth
In-Reply-To: <545A3838.3090606@free-electrons.com>
On Wed, 2014-11-05 at 11:46 -0300, Ezequiel Garcia wrote:
> Hi Karl,
>
> On 11/05/2014 11:32 AM, Karl Beldan wrote:> From: Karl Beldan <karl.beldan@rivierawaves.com>
> >
> > ATM, txq_reclaim will dequeue and free an skb for each tx desc released
> > by the hw that has TX_LAST_DESC set. However, in case of TSO, each
> > hw desc embedding the last part of a segment has TX_LAST_DESC set,
> > losing the one-to-one 'last skb frag'/'TX_LAST_DESC set' correspondance,
> > which causes data corruption.
> >
> > Fix this by checking TX_ENABLE_INTERRUPT instead of TX_LAST_DESC, and
> > warn when trying to dequeue from an empty txq (which can be symptomatic
> > of releasing skbs prematurely).
> >
> > Fixes: 3ae8f4e0b98 ('net: mv643xx_eth: Implement software TSO')
>
> Although your change makes sense, this isn't fixing the issue for me,
> neither did the previous one.
>
> Ian: Can you double check that you have corruption *without* the patch,
> and that the patch fixes the issue?
>
Have you also applied my patch ?
^ permalink raw reply
* Re: Possible bug in net/core/neighbor.c
From: Ulf Samuelsson @ 2014-11-05 15:46 UTC (permalink / raw)
To: netdev
In-Reply-To: <5459FFEA.3000101@ericsson.com>
On 11/05/2014 11:46 AM, Ulf Samuelsson wrote:
> 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.
>
OK, I think I figured it out.
If laddr and neigh->ha are identical, we want lladdr (which is a pointer)
to have the same value as neigh->ha so after this, you know that
laddr is identical to neigh->ha by justr comparing the pointers.
When I google, I only find other people which does not understand
the purpose of the code.
The comments are also obsolete, since "check override flag" refers
to code which has been removed.
Best Regards,
Ulf Samuelsson
KI/EAB/ILM/GF
ulf.samuelsson@ericsson.com
+46 722 427 437
^ permalink raw reply
* RE: "asix: Don't reset PHY on if_up for ASIX 88772" breaks netonarndale platform
From: Stam, Michel [FINT] @ 2014-11-05 16:17 UTC (permalink / raw)
To: Charles Keepax
Cc: Riku Voipio, davem, linux-usb, netdev, linux-kernel,
linux-samsung-soc
In-Reply-To: <20141105150258.GR23178@opensource.wolfsonmicro.com>
Hello Charles,
First of all, my apologies. I manually applied your patch and made a
mistake; I swapped ax88772_link_reset with ax88772_reset for struct
driver_info_ax88772_info structure, which caused the software reset
failures. Blame it on a lack of coffee... Please disregard my previous
mail.
After (correctly) applying the patch I still don't get the desired
results; see below.
Test situation:
ifconfig eth1 down
ethtool -s advertise 1
ifconfig eth1 up
Check dmesg, It will report a link down, followed by the new speed,
which when using advertise 1, should be 10 Mbps/half duplex.
To make it more interesting, ethtool eth1 reports 10 Mbps/half duplex
even though the kernel reports 100 Mbps/full duplex.
What does work (but did before this whole situation as well):
ifconfig eth1 up
ethtool -s eth1 advertise 1
The interface will now be in 10 Mbps/half duplex, that is, until the
reset function is called (interface down or otherwise).
What I do notice, is that dev->mii.advertising follows whatever I set
with ethtool. I tried writing the ADVERTISE register with the value of
the dev->mii.advertising variable, but that did not give me the desired
result either.
Kind regards,
Michel Stam
-----Original Message-----
From: Charles Keepax [mailto:ckeepax@opensource.wolfsonmicro.com]
Sent: Wednesday, November 05, 2014 16:03 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
netonarndale platform
On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> 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.
Odd it definitely reads back as zero on Arndale. I am guessing that the
root of the problem here is that for some reason Arndale POR of the
ethernet is pants and it needs a full software reset before it will work
and the patch removes the full reset callback.
>
> 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
Ok so I am guessing you have a value in the register which is neither
the reset value or 0 and this causing problems later in the reset/on the
next reset. I do find the naming confusing in the error message there as
it says link reset failed but the link_reset callback can't fail in the
driver and I modified the reset callback. But I guess that is just
oddities of the network stack I am not familiar with.
The other thing that feels odd is (and again apologies as I know next to
nothing about the networking stack) how come it is unexpected that the
reset callback destroys the state of the device. Naively I would have
expected that a reset callback would reset the device back to its
default state. Here we seem to be trying to avoid that happening.
>
> 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?
Yeah mine also idVendor=0b95, idProduct=772a
Thanks,
Charles
>
> 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
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