[PATCH 0/6] Ethernet over hdlc

[PATCH 0/6] Ethernet over hdlc

[David Gounaris]

Hello!

I have a 83xx platform and I am using non tsa mode in the fsl hdlc driver to enable ethernet over hdlc. These patches are the minimal changes that I needed to do in order to get it to work with our legacy products that used old FSL SDK driver. Please review.

Best Regards,
David Gounaris

David Gounaris (6):
  net/wan/fsl_ucc_hdlc: allow ucc index up to 4
  net/wan/fsl_ucc_hdlc: allow PARITY_CRC16_PR0_CCITT parity
  net/wan/fsl_ucc_hdlc: Adding ARPHRD_ETHER
  net/wan/fsl_ucc_hdlc: default hmask value
  net/wan/fsl_ucc_hdlc: GUMR for non tsa mode
  net/wan/fsl_ucc_hdlc: tx timeout handler

 drivers/net/wan/fsl_ucc_hdlc.c | 23 +++++++++++++++++++++--
 drivers/net/wan/fsl_ucc_hdlc.h |  2 +-
 2 files changed, 22 insertions(+), 3 deletions(-)

-- 
2.13.6

[PATCH 1/6] net/wan/fsl_ucc_hdlc: allow ucc index up to 4

[David Gounaris]

There is a need to allow higher indexes to be
able to support MPC83xx platforms. (UCC1-UCC5)

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 5f0366a125e2..3c0e0a1d19ba 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1015,7 +1015,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
 	}
 
 	ucc_num = val - 1;
-	if ((ucc_num > 3) || (ucc_num < 0)) {
+	if ((ucc_num > 4) || (ucc_num < 0)) {
 		dev_err(&pdev->dev, ": Invalid UCC num\n");
 		return -EINVAL;
 	}
-- 
2.13.6

[PATCH 2/6] net/wan/fsl_ucc_hdlc: allow PARITY_CRC16_PR0_CCITT parity

[David Gounaris]

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 3c0e0a1d19ba..963b3b5ae15e 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -780,7 +780,8 @@ static int ucc_hdlc_attach(struct net_device *dev, unsigned short encoding,
 
 	if (parity != PARITY_NONE &&
 	    parity != PARITY_CRC32_PR1_CCITT &&
-	    parity != PARITY_CRC16_PR1_CCITT)
+	    parity != PARITY_CRC16_PR1_CCITT && 
+	    parity != PARITY_CRC16_PR0_CCITT)
 		return -EINVAL;
 
 	priv->encoding = encoding;
-- 
2.13.6

[PATCH 3/6] net/wan/fsl_ucc_hdlc: Adding ARPHRD_ETHER

[David Gounaris]

This was done to avoid discarding ethernet
packets when using HDLC_ETH protocol.

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 963b3b5ae15e..54e2b2143e36 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -375,6 +375,10 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
 		dev->stats.tx_bytes += skb->len;
 		break;
 
+	case ARPHRD_ETHER:
+		dev->stats.tx_bytes += skb->len;
+		break;
+
 	default:
 		dev->stats.tx_dropped++;
 		dev_kfree_skb(skb);
@@ -512,6 +516,8 @@ static int hdlc_rx_done(struct ucc_hdlc_private *priv, int rx_work_limit)
 			break;
 
 		case ARPHRD_PPP:
+		case ARPHRD_ETHER:
+			
 			length -= HDLC_CRC_SIZE;
 
 			skb = dev_alloc_skb(length);
-- 
2.13.6

[PATCH 4/6] net/wan/fsl_ucc_hdlc: default hmask value

[David Gounaris]

Set default HMASK to 0x0000 to use
promiscuous mode in the hdlc controller.

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h
index c21134c1f180..5bc3d1a6ca6e 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.h
+++ b/drivers/net/wan/fsl_ucc_hdlc.h
@@ -134,7 +134,7 @@ struct ucc_hdlc_private {
 
 #define HDLC_HEAD_MASK		0x0000
 #define DEFAULT_HDLC_HEAD	0xff44
-#define DEFAULT_ADDR_MASK	0x00ff
+#define DEFAULT_ADDR_MASK	0x0000
 #define DEFAULT_HDLC_ADDR	0x00ff
 
 #define BMR_GBL			0x20000000
-- 
2.13.6

[PATCH 5/6] net/wan/fsl_ucc_hdlc: GUMR for non tsa mode

[David Gounaris]

The following bits in the GUMR is changed for non
tsa mode: CDS, CTSP and CTSS are set to zero.

When set, there is no tx interrupts from the controller.

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 54e2b2143e36..e6154a6547e6 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -97,6 +97,13 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
 	if (priv->tsa) {
 		uf_info->tsa = 1;
 		uf_info->ctsp = 1;
+		uf_info->cds = 1;
+		uf_info->ctss = 1;
+	}
+	else {
+		uf_info->cds = 0;
+		uf_info->ctsp = 0;
+		uf_info->ctss = 0;
 	}
 
 	/* This sets HPM register in CMXUCR register which configures a
-- 
2.13.6

[PATCH 6/6] net/wan/fsl_ucc_hdlc: tx timeout handler

[David Gounaris]

Added tx timerout handler. This helps
when troubleshooting.

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index e6154a6547e6..30f416bace2c 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1001,11 +1001,15 @@ static const struct dev_pm_ops uhdlc_pm_ops = {
 #define HDLC_PM_OPS NULL
 
 #endif
+static void uhdlc_tx_timeout(struct net_device *ndev) {
+	netdev_err(ndev, "%s\n", __FUNCTION__);
+}
 static const struct net_device_ops uhdlc_ops = {
 	.ndo_open       = uhdlc_open,
 	.ndo_stop       = uhdlc_close,
 	.ndo_start_xmit = hdlc_start_xmit,
 	.ndo_do_ioctl   = uhdlc_ioctl,
+	.ndo_tx_timeout	= uhdlc_tx_timeout,
 };
 
 static int ucc_hdlc_probe(struct platform_device *pdev)
@@ -1121,6 +1125,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
 	hdlc = dev_to_hdlc(dev);
 	dev->tx_queue_len = 16;
 	dev->netdev_ops = &uhdlc_ops;
+	dev->watchdog_timeo = 2*HZ;
 	hdlc->attach = ucc_hdlc_attach;
 	hdlc->xmit = ucc_hdlc_tx;
 	netif_napi_add(dev, &uhdlc_priv->napi, ucc_hdlc_poll, 32);
-- 
2.13.6

Re: [PATCH 1/6] net/wan/fsl_ucc_hdlc: allow ucc index up to 4

[Christophe LEROY]

Le 28/08/2018 à 13:09, David Gounaris a écrit :
> There is a need to allow higher indexes to be
> able to support MPC83xx platforms. (UCC1-UCC5)

As far as I can see, MPC8358 has 8 UCCs (ref 
https://www.nxp.com/products/processors-and-microcontrollers/power-architecture-processors/powerquicc-processors/powerquicc-ii-pro/powerquicc-ii-pro-processor-with-ddr2-tdm-pci-security-usb-quicc-engine-with-1-gb-ethernet-utopia:MPC8358E)

Christophe

> 
> Signed-off-by: David Gounaris <david.gounaris@infinera.com>
> ---
>   drivers/net/wan/fsl_ucc_hdlc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
> index 5f0366a125e2..3c0e0a1d19ba 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.c
> +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> @@ -1015,7 +1015,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
>   	}
>   
>   	ucc_num = val - 1;
> -	if ((ucc_num > 3) || (ucc_num < 0)) {
> +	if ((ucc_num > 4) || (ucc_num < 0)) {
>   		dev_err(&pdev->dev, ": Invalid UCC num\n");
>   		return -EINVAL;
>   	}
> 

Re: [PATCH 3/6] net/wan/fsl_ucc_hdlc: Adding ARPHRD_ETHER

[Christophe LEROY]

Le 28/08/2018 à 13:09, David Gounaris a écrit :
> This was done to avoid discarding ethernet
> packets when using HDLC_ETH protocol.

Maybe use 'present' form instead of 'past form', ie 'This patch is to 
avoid discarding ethernet ...'

Christophe

> 
> Signed-off-by: David Gounaris <david.gounaris@infinera.com>
> ---
>   drivers/net/wan/fsl_ucc_hdlc.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
> index 963b3b5ae15e..54e2b2143e36 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.c
> +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> @@ -375,6 +375,10 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
>   		dev->stats.tx_bytes += skb->len;
>   		break;
>   
> +	case ARPHRD_ETHER:
> +		dev->stats.tx_bytes += skb->len;
> +		break;
> +
>   	default:
>   		dev->stats.tx_dropped++;
>   		dev_kfree_skb(skb);
> @@ -512,6 +516,8 @@ static int hdlc_rx_done(struct ucc_hdlc_private *priv, int rx_work_limit)
>   			break;
>   
>   		case ARPHRD_PPP:
> +		case ARPHRD_ETHER:
> +			
>   			length -= HDLC_CRC_SIZE;
>   
>   			skb = dev_alloc_skb(length);
> 

RE: [PATCH 4/6] net/wan/fsl_ucc_hdlc: default hmask value

[Qiang Zhao]

From: David Gounaris <david.gounaris@infinera.com>
Date: 2018/8/28 19:09
> Subject: [PATCH 4/6] net/wan/fsl_ucc_hdlc: default hmask value
>=20
> Set default HMASK to 0x0000 to use
> promiscuous mode in the hdlc controller.
>=20
> Signed-off-by: David Gounaris <david.gounaris@infinera.com>
> ---
>  drivers/net/wan/fsl_ucc_hdlc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>=20
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdl=
c.h
> index c21134c1f180..5bc3d1a6ca6e 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.h
> +++ b/drivers/net/wan/fsl_ucc_hdlc.h
> @@ -134,7 +134,7 @@ struct ucc_hdlc_private {
>=20
>  #define HDLC_HEAD_MASK		0x0000
>  #define DEFAULT_HDLC_HEAD	0xff44
> -#define DEFAULT_ADDR_MASK	0x00ff
> +#define DEFAULT_ADDR_MASK	0x0000
>  #define DEFAULT_HDLC_ADDR	0x00ff
>=20
>  #define BMR_GBL			0x20000000
> --

It is not proper to set default HMASK to 0x0000, how about to add a new pro=
perty standing for hmask into device tree,
If get this property from dtb, then set it with the value from dtb, otherwi=
se, set it with default HMASK ox00ff?

Best Regards
Qiang Zhao

Re: [PATCH 1/6] net/wan/fsl_ucc_hdlc: allow ucc index up to 4

[Christophe LEROY]

Le 28/08/2018 à 15:34, Christophe LEROY a écrit :
> 
> 
> Le 28/08/2018 à 13:09, David Gounaris a écrit :
>> There is a need to allow higher indexes to be
>> able to support MPC83xx platforms. (UCC1-UCC5)
> 
> As far as I can see, MPC8358 has 8 UCCs (ref 
> https://www.nxp.com/products/processors-and-microcontrollers/power-architecture-processors/powerquicc-processors/powerquicc-ii-pro/powerquicc-ii-pro-processor-with-ddr2-tdm-pci-security-usb-quicc-engine-with-1-gb-ethernet-utopia:MPC8358E) 

Indeed, the code should use UCC_MAX_NUM which is defined in 
include/soc/fsl/qe/ucc.h

Christophe

> 
> 
> Christophe
> 
>>
>> Signed-off-by: David Gounaris <david.gounaris@infinera.com>
>> ---
>>   drivers/net/wan/fsl_ucc_hdlc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wan/fsl_ucc_hdlc.c 
>> b/drivers/net/wan/fsl_ucc_hdlc.c
>> index 5f0366a125e2..3c0e0a1d19ba 100644
>> --- a/drivers/net/wan/fsl_ucc_hdlc.c
>> +++ b/drivers/net/wan/fsl_ucc_hdlc.c
>> @@ -1015,7 +1015,7 @@ static int ucc_hdlc_probe(struct platform_device 
>> *pdev)
>>       }
>>       ucc_num = val - 1;
>> -    if ((ucc_num > 3) || (ucc_num < 0)) {
>> +    if ((ucc_num > 4) || (ucc_num < 0)) {
>>           dev_err(&pdev->dev, ": Invalid UCC num\n");
>>           return -EINVAL;
>>       }
>>

Re: [PATCH 4/6] net/wan/fsl_ucc_hdlc: default hmask value

[Christophe LEROY]

Le 29/08/2018 à 04:54, Qiang Zhao a écrit :
> From: David Gounaris <david.gounaris@infinera.com>
> Date: 2018/8/28 19:09
>> Subject: [PATCH 4/6] net/wan/fsl_ucc_hdlc: default hmask value
>>
>> Set default HMASK to 0x0000 to use
>> promiscuous mode in the hdlc controller.

Why do you need to do that ?

An HDLC frame encapsulating Ethernet should look like:

HDLC Frame includes:
– Opening Flag		7E hex
– Address field		FF hex
– Control field		03 hex
– Information field	Original Ethernet Packet, 1522 octets max.
– FCS CRC16		(2 bytes)
– Closing Flag		7E hex

What do you mean by 'promiscuous mode' ?

In any case, should a filter mask be changed for promiscuous mode, I 
believe the change should be done at the time you enter promiscuous 
mode, not at all time.

Christophe

>>
>> Signed-off-by: David Gounaris <david.gounaris@infinera.com>
>> ---
>>   drivers/net/wan/fsl_ucc_hdlc.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h
>> index c21134c1f180..5bc3d1a6ca6e 100644
>> --- a/drivers/net/wan/fsl_ucc_hdlc.h
>> +++ b/drivers/net/wan/fsl_ucc_hdlc.h
>> @@ -134,7 +134,7 @@ struct ucc_hdlc_private {
>>
>>   #define HDLC_HEAD_MASK		0x0000
>>   #define DEFAULT_HDLC_HEAD	0xff44
>> -#define DEFAULT_ADDR_MASK	0x00ff
>> +#define DEFAULT_ADDR_MASK	0x0000
>>   #define DEFAULT_HDLC_ADDR	0x00ff
>>
>>   #define BMR_GBL			0x20000000
>> --
> 
> It is not proper to set default HMASK to 0x0000, how about to add a new property standing for hmask into device tree,
> If get this property from dtb, then set it with the value from dtb, otherwise, set it with default HMASK ox00ff?
> 
> Best Regards
> Qiang Zhao
> 

Re: [PATCH 4/6] net/wan/fsl_ucc_hdlc: default hmask value

[Joakim Tjernlund]

T24gV2VkLCAyMDE4LTA4LTI5IGF0IDA5OjE4ICswMjAwLCBDaHJpc3RvcGhlIExFUk9ZIHdyb3Rl
Og0KPiBDQVVUSU9OOiBUaGlzIGVtYWlsIG9yaWdpbmF0ZWQgZnJvbSBvdXRzaWRlIG9mIHRoZSBv
cmdhbml6YXRpb24uIERvIG5vdCBjbGljayBsaW5rcyBvciBvcGVuIGF0dGFjaG1lbnRzIHVubGVz
cyB5b3UgcmVjb2duaXplIHRoZSBzZW5kZXIgYW5kIGtub3cgdGhlIGNvbnRlbnQgaXMgc2FmZS4N
Cj4gDQo+IA0KPiBMZSAyOS8wOC8yMDE4IMOgIDA0OjU0LCBRaWFuZyBaaGFvIGEgw6ljcml0IDoN
Cj4gPiBGcm9tOiBEYXZpZCBHb3VuYXJpcyA8ZGF2aWQuZ291bmFyaXNAaW5maW5lcmEuY29tPg0K
PiA+IERhdGU6IDIwMTgvOC8yOCAxOTowOQ0KPiA+ID4gU3ViamVjdDogW1BBVENIIDQvNl0gbmV0
L3dhbi9mc2xfdWNjX2hkbGM6IGRlZmF1bHQgaG1hc2sgdmFsdWUNCj4gPiA+IA0KPiA+ID4gU2V0
IGRlZmF1bHQgSE1BU0sgdG8gMHgwMDAwIHRvIHVzZQ0KPiA+ID4gcHJvbWlzY3VvdXMgbW9kZSBp
biB0aGUgaGRsYyBjb250cm9sbGVyLg0KPiANCj4gV2h5IGRvIHlvdSBuZWVkIHRvIGRvIHRoYXQg
Pw0KPiANCj4gQW4gSERMQyBmcmFtZSBlbmNhcHN1bGF0aW5nIEV0aGVybmV0IHNob3VsZCBsb29r
IGxpa2U6DQo+IA0KPiBIRExDIEZyYW1lIGluY2x1ZGVzOg0KPiDigJMgT3BlbmluZyBGbGFnICAg
ICAgICAgIDdFIGhleA0KPiDigJMgQWRkcmVzcyBmaWVsZCAgICAgICAgIEZGIGhleA0KPiDigJMg
Q29udHJvbCBmaWVsZCAgICAgICAgIDAzIGhleA0KPiDigJMgSW5mb3JtYXRpb24gZmllbGQgICAg
IE9yaWdpbmFsIEV0aGVybmV0IFBhY2tldCwgMTUyMiBvY3RldHMgbWF4Lg0KPiDigJMgRkNTIENS
QzE2ICAgICAgICAgICAgICgyIGJ5dGVzKQ0KPiDigJMgQ2xvc2luZyBGbGFnICAgICAgICAgIDdF
IGhleA0KDQpJbiBvdXIgY2FzZSB0aGUgRXRoIGZyYW1lIHN0YXJ0cyBkaXJlY3RseSBhZnRlciB0
aGUgT3BlbmluZyBGbGFnIHNvDQp0aGVyZSBpcyBubyBGRiBhZGRyZXNzIGZpZWxkLg0KDQo+IA0K
PiBXaGF0IGRvIHlvdSBtZWFuIGJ5ICdwcm9taXNjdW91cyBtb2RlJyA/DQoNCkp1c3QgdGhhdCB3
ZSBkb24ndCB3YW50IGFueSBhZGRyZXNzIGZpbHRlcmluZyBvZiBwYWNrZXRzLiBJIGRvbid0IHRo
aW5rIHRoaXMgd291bGQNCmJlIGEgcHJvYmxlbSBmb3IgcHVyZSBIRExDIGZyYW1lcyBlaXRoZXIs
IGRvIHlvdT8NCg0KPiANCj4gSW4gYW55IGNhc2UsIHNob3VsZCBhIGZpbHRlciBtYXNrIGJlIGNo
YW5nZWQgZm9yIHByb21pc2N1b3VzIG1vZGUsIEkNCj4gYmVsaWV2ZSB0aGUgY2hhbmdlIHNob3Vs
ZCBiZSBkb25lIGF0IHRoZSB0aW1lIHlvdSBlbnRlciBwcm9taXNjdW91cw0KPiBtb2RlLCBub3Qg
YXQgYWxsIHRpbWUuDQo+IA0KPiBDaHJpc3RvcGhlDQo+IA0KPiA+ID4gDQo+ID4gPiBTaWduZWQt
b2ZmLWJ5OiBEYXZpZCBHb3VuYXJpcyA8ZGF2aWQuZ291bmFyaXNAaW5maW5lcmEuY29tPg0KPiA+
ID4gLS0tDQo+ID4gPiAgIGRyaXZlcnMvbmV0L3dhbi9mc2xfdWNjX2hkbGMuaCB8IDIgKy0NCj4g
PiA+ICAgMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0aW9uKCspLCAxIGRlbGV0aW9uKC0pDQo+ID4g
PiANCj4gPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL25ldC93YW4vZnNsX3VjY19oZGxjLmggYi9k
cml2ZXJzL25ldC93YW4vZnNsX3VjY19oZGxjLmgNCj4gPiA+IGluZGV4IGMyMTEzNGMxZjE4MC4u
NWJjM2QxYTZjYTZlIDEwMDY0NA0KPiA+ID4gLS0tIGEvZHJpdmVycy9uZXQvd2FuL2ZzbF91Y2Nf
aGRsYy5oDQo+ID4gPiArKysgYi9kcml2ZXJzL25ldC93YW4vZnNsX3VjY19oZGxjLmgNCj4gPiA+
IEBAIC0xMzQsNyArMTM0LDcgQEAgc3RydWN0IHVjY19oZGxjX3ByaXZhdGUgew0KPiA+ID4gDQo+
ID4gPiAgICNkZWZpbmUgSERMQ19IRUFEX01BU0sgICAgICAgICAgICAgMHgwMDAwDQo+ID4gPiAg
ICNkZWZpbmUgREVGQVVMVF9IRExDX0hFQUQgIDB4ZmY0NA0KPiA+ID4gLSNkZWZpbmUgREVGQVVM
VF9BRERSX01BU0sgICAweDAwZmYNCj4gPiA+ICsjZGVmaW5lIERFRkFVTFRfQUREUl9NQVNLICAg
MHgwMDAwDQo+ID4gPiAgICNkZWZpbmUgREVGQVVMVF9IRExDX0FERFIgIDB4MDBmZg0KPiA+ID4g
DQo+ID4gPiAgICNkZWZpbmUgQk1SX0dCTCAgICAgICAgICAgICAgICAgICAgMHgyMDAwMDAwMA0K
PiA+ID4gLS0NCj4gPiANCj4gPiBJdCBpcyBub3QgcHJvcGVyIHRvIHNldCBkZWZhdWx0IEhNQVNL
IHRvIDB4MDAwMCwgaG93IGFib3V0IHRvIGFkZCBhIG5ldyBwcm9wZXJ0eSBzdGFuZGluZyBmb3Ig
aG1hc2sgaW50byBkZXZpY2UgdHJlZSwNCj4gPiBJZiBnZXQgdGhpcyBwcm9wZXJ0eSBmcm9tIGR0
YiwgdGhlbiBzZXQgaXQgd2l0aCB0aGUgdmFsdWUgZnJvbSBkdGIsIG90aGVyd2lzZSwgc2V0IGl0
IHdpdGggZGVmYXVsdCBITUFTSyBveDAwZmY/DQo+ID4gDQo+ID4gQmVzdCBSZWdhcmRzDQo+ID4g
UWlhbmcgWmhhbw0KPiA+IA0KDQo=

[PATCH v2 0/6] Ethernet over hdlc

[David Gounaris]

Here is what has been changed in v2 after the review comments.

v2-0001: Using UCC_MAX_NUM
v2-0002: Unchanged
v2-0003: Changed commit message
v2-0004: Adding fsl,hmask into the dt instead of changing the default value.
v2-0005: Unchanged
v2-0006: Unchanged

Adding robh+dt@kernel.org for comments regarding dt.

Best Regards
David Gounaris

David Gounaris (6):
  net/wan/fsl_ucc_hdlc: allow ucc index up to 7
  net/wan/fsl_ucc_hdlc: allow PARITY_CRC16_PR0_CCITT parity
  net/wan/fsl_ucc_hdlc: Adding ARPHRD_ETHER
  net/wan/fsl_ucc_hdlc: hmask
  net/wan/fsl_ucc_hdlc: GUMR for non tsa mode
  net/wan/fsl_ucc_hdlc: tx timeout handler

 .../devicetree/bindings/soc/fsl/cpm_qe/network.txt |  6 +++++
 drivers/net/wan/fsl_ucc_hdlc.c                     | 28 +++++++++++++++++++---
 drivers/net/wan/fsl_ucc_hdlc.h                     |  1 +
 3 files changed, 32 insertions(+), 3 deletions(-)

-- 
2.13.6

[PATCH v2 1/6] net/wan/fsl_ucc_hdlc: allow ucc index up to 7

[David Gounaris]

There is a need to allow higher indexes to be
able to support MPC83xx platforms. (UCC1-UCC8)

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 33df76405b86..5cf6dcba039c 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1016,7 +1016,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
 	}
 
 	ucc_num = val - 1;
-	if ((ucc_num > 3) || (ucc_num < 0)) {
+	if ((ucc_num > UCC_MAX_NUM - 1) || (ucc_num < 0)) {
 		dev_err(&pdev->dev, ": Invalid UCC num\n");
 		return -EINVAL;
 	}
-- 
2.13.6

[PATCH v2 2/6] net/wan/fsl_ucc_hdlc: allow PARITY_CRC16_PR0_CCITT parity

[David Gounaris]

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 5cf6dcba039c..c8e526bf1130 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -781,7 +781,8 @@ static int ucc_hdlc_attach(struct net_device *dev, unsigned short encoding,
 
 	if (parity != PARITY_NONE &&
 	    parity != PARITY_CRC32_PR1_CCITT &&
-	    parity != PARITY_CRC16_PR1_CCITT)
+	    parity != PARITY_CRC16_PR1_CCITT && 
+	    parity != PARITY_CRC16_PR0_CCITT)
 		return -EINVAL;
 
 	priv->encoding = encoding;
-- 
2.13.6

[PATCH v2 3/6] net/wan/fsl_ucc_hdlc: Adding ARPHRD_ETHER

[David Gounaris]

This patch is to avoid discarding ethernet
packets when using HDLC_ETH protocol.

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index c8e526bf1130..0f703d7be5e0 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -376,6 +376,10 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
 		dev->stats.tx_bytes += skb->len;
 		break;
 
+	case ARPHRD_ETHER:
+		dev->stats.tx_bytes += skb->len;
+		break;
+
 	default:
 		dev->stats.tx_dropped++;
 		dev_kfree_skb(skb);
@@ -513,6 +517,8 @@ static int hdlc_rx_done(struct ucc_hdlc_private *priv, int rx_work_limit)
 			break;
 
 		case ARPHRD_PPP:
+		case ARPHRD_ETHER:
+			
 			length -= HDLC_CRC_SIZE;
 
 			skb = dev_alloc_skb(length);
-- 
2.13.6

[PATCH v2 4/6] net/wan/fsl_ucc_hdlc: hmask

[David Gounaris]

Ability to set hmask in the device-tree,
which can be used to change address
filtering of packets.

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt | 6 ++++++
 drivers/net/wan/fsl_ucc_hdlc.c                               | 5 ++++-
 drivers/net/wan/fsl_ucc_hdlc.h                               | 1 +
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt
index 03c741602c6d..6d2dd8a31482 100644
--- a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt
+++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt
@@ -98,6 +98,12 @@ The property below is dependent on fsl,tdm-interface:
 	usage: optional for tdm interface
 	value type: <empty>
 	Definition : Internal loopback connecting on TDM layer.
+- fsl,hmask
+	usage: optional
+	Value type: <u16>
+	Definition: HDLC address recognition. Set to zero to disable
+		    address filtering of packets:
+		    fsl,hmask = /bits/ 16 <0x0000>;
 
 Example for tdm interface:
 
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 0f703d7be5e0..be5b0096af3b 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -263,7 +263,7 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
 	iowrite16be(MAX_FRAME_LENGTH, &priv->ucc_pram->mflr);
 	iowrite16be(DEFAULT_RFTHR, &priv->ucc_pram->rfthr);
 	iowrite16be(DEFAULT_RFTHR, &priv->ucc_pram->rfcnt);
-	iowrite16be(DEFAULT_ADDR_MASK, &priv->ucc_pram->hmask);
+	iowrite16be(priv->hmask, &priv->ucc_pram->hmask);
 	iowrite16be(DEFAULT_HDLC_ADDR, &priv->ucc_pram->haddr1);
 	iowrite16be(DEFAULT_HDLC_ADDR, &priv->ucc_pram->haddr2);
 	iowrite16be(DEFAULT_HDLC_ADDR, &priv->ucc_pram->haddr3);
@@ -1097,6 +1097,9 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
 		if (ret)
 			goto free_utdm;
 	}
+	
+	if (of_property_read_u16(np, "fsl,hmask", &uhdlc_priv->hmask))
+		uhdlc_priv->hmask = DEFAULT_ADDR_MASK;
 
 	ret = uhdlc_init(uhdlc_priv);
 	if (ret) {
diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h
index c21134c1f180..b99fa2f1cd99 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.h
+++ b/drivers/net/wan/fsl_ucc_hdlc.h
@@ -106,6 +106,7 @@ struct ucc_hdlc_private {
 
 	unsigned short encoding;
 	unsigned short parity;
+	unsigned short hmask;
 	u32 clocking;
 	spinlock_t lock;	/* lock for Tx BD and Tx buffer */
 #ifdef CONFIG_PM
-- 
2.13.6

[PATCH v2 5/6] net/wan/fsl_ucc_hdlc: GUMR for non tsa mode

[David Gounaris]

The following bits in the GUMR is changed for non
tsa mode: CDS, CTSP and CTSS are set to zero.

When set, there is no tx interrupts from the controller.

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index be5b0096af3b..248f1f5bcd04 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -97,6 +97,13 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
 	if (priv->tsa) {
 		uf_info->tsa = 1;
 		uf_info->ctsp = 1;
+		uf_info->cds = 1;
+		uf_info->ctss = 1;
+	}
+	else {
+		uf_info->cds = 0;
+		uf_info->ctsp = 0;
+		uf_info->ctss = 0;
 	}
 
 	/* This sets HPM register in CMXUCR register which configures a
-- 
2.13.6

[PATCH v2 6/6] net/wan/fsl_ucc_hdlc: tx timeout handler

[David Gounaris]

Added tx timerout handler. This helps
when troubleshooting.

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 248f1f5bcd04..629ef5049d27 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1002,11 +1002,15 @@ static const struct dev_pm_ops uhdlc_pm_ops = {
 #define HDLC_PM_OPS NULL
 
 #endif
+static void uhdlc_tx_timeout(struct net_device *ndev) {
+	netdev_err(ndev, "%s\n", __FUNCTION__);
+}
 static const struct net_device_ops uhdlc_ops = {
 	.ndo_open       = uhdlc_open,
 	.ndo_stop       = uhdlc_close,
 	.ndo_start_xmit = hdlc_start_xmit,
 	.ndo_do_ioctl   = uhdlc_ioctl,
+	.ndo_tx_timeout	= uhdlc_tx_timeout,
 };
 
 static int ucc_hdlc_probe(struct platform_device *pdev)
@@ -1125,6 +1129,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
 	hdlc = dev_to_hdlc(dev);
 	dev->tx_queue_len = 16;
 	dev->netdev_ops = &uhdlc_ops;
+	dev->watchdog_timeo = 2*HZ;
 	hdlc->attach = ucc_hdlc_attach;
 	hdlc->xmit = ucc_hdlc_tx;
 	netif_napi_add(dev, &uhdlc_priv->napi, ucc_hdlc_poll, 32);
-- 
2.13.6

Re: [PATCH v2 3/6] net/wan/fsl_ucc_hdlc: Adding ARPHRD_ETHER

[David Miller]

From: David Gounaris <david.gounaris@infinera.com>
Date: Wed, 29 Aug 2018 15:13:25 +0200

> @@ -513,6 +517,8 @@ static int hdlc_rx_done(struct ucc_hdlc_private *priv, int rx_work_limit)
>  			break;
>  
>  		case ARPHRD_PPP:
> +		case ARPHRD_ETHER:
> +			

Please don't add such an extraneous empty line.

Thanks.

Re: [PATCH v2 1/6] net/wan/fsl_ucc_hdlc: allow ucc index up to 7

[christophe leroy]

Le 29/08/2018 à 15:13, David Gounaris a écrit :
> There is a need to allow higher indexes to be
> able to support MPC83xx platforms. (UCC1-UCC8)
> 
> Signed-off-by: David Gounaris <david.gounaris@infinera.com>
> ---
>   drivers/net/wan/fsl_ucc_hdlc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
> index 33df76405b86..5cf6dcba039c 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.c
> +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> @@ -1016,7 +1016,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
>   	}
>   
>   	ucc_num = val - 1;
> -	if ((ucc_num > 3) || (ucc_num < 0)) {
> +	if ((ucc_num > UCC_MAX_NUM - 1) || (ucc_num < 0)) {

CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'ucc_num < 0'
#23: FILE: drivers/net/wan/fsl_ucc_hdlc.c:1018:
+	if ((ucc_num > UCC_MAX_NUM - 1) || (ucc_num < 0)) {


>   		dev_err(&pdev->dev, ": Invalid UCC num\n");
>   		return -EINVAL;
>   	}
> 

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH v2 2/6] net/wan/fsl_ucc_hdlc: allow PARITY_CRC16_PR0_CCITT parity

[christophe leroy]

Le 29/08/2018 à 15:13, David Gounaris a écrit :
> Signed-off-by: David Gounaris <david.gounaris@infinera.com>
> ---
>   drivers/net/wan/fsl_ucc_hdlc.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
> index 5cf6dcba039c..c8e526bf1130 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.c
> +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> @@ -781,7 +781,8 @@ static int ucc_hdlc_attach(struct net_device *dev, unsigned short encoding,
>   
>   	if (parity != PARITY_NONE &&
>   	    parity != PARITY_CRC32_PR1_CCITT &&
> -	    parity != PARITY_CRC16_PR1_CCITT)
> +	    parity != PARITY_CRC16_PR1_CCITT &&
> +	    parity != PARITY_CRC16_PR0_CCITT)

ERROR:TRAILING_WHITESPACE: trailing whitespace
#20: FILE: drivers/net/wan/fsl_ucc_hdlc.c:783:
+^I    parity != PARITY_CRC16_PR1_CCITT && $

Also, you could have left the line 'parity != PARITY_CRC16_PR1_CCITT)' 
as is and just have added 'parity != PARITY_CRC16_PR0_CCITT &&' on the 
previous line, would make more sense to have PR0 before PR1, and instead 
of having 2+1- you would only have 1+

>   		return -EINVAL;
>   
>   	priv->encoding = encoding;
> 

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH v2 3/6] net/wan/fsl_ucc_hdlc: Adding ARPHRD_ETHER

[christophe leroy]

Le 29/08/2018 à 15:13, David Gounaris a écrit :
> This patch is to avoid discarding ethernet
> packets when using HDLC_ETH protocol.
> 
> Signed-off-by: David Gounaris <david.gounaris@infinera.com>
> ---
>   drivers/net/wan/fsl_ucc_hdlc.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
> index c8e526bf1130..0f703d7be5e0 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.c
> +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> @@ -376,6 +376,10 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
>   		dev->stats.tx_bytes += skb->len;
>   		break;
>   
> +	case ARPHRD_ETHER:
> +		dev->stats.tx_bytes += skb->len;
> +		break;
> +
>   	default:
>   		dev->stats.tx_dropped++;
>   		dev_kfree_skb(skb);
> @@ -513,6 +517,8 @@ static int hdlc_rx_done(struct ucc_hdlc_private *priv, int rx_work_limit)
>   			break;
>   
>   		case ARPHRD_PPP:
> +		case ARPHRD_ETHER:
> +			

ERROR:TRAILING_WHITESPACE: trailing whitespace
#34: FILE: drivers/net/wan/fsl_ucc_hdlc.c:520:
+^I^I^I$

Note that 'git show' would show it to you in red background so you 
shouldn't miss it.

And this additional blank line is unnecessary.

>   			length -= HDLC_CRC_SIZE;
>   
>   			skb = dev_alloc_skb(length);
> 

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH v2 4/6] net/wan/fsl_ucc_hdlc: hmask

[christophe leroy]

Le 29/08/2018 à 15:13, David Gounaris a écrit :
> Ability to set hmask in the device-tree,
> which can be used to change address
> filtering of packets.
> 
> Signed-off-by: David Gounaris <david.gounaris@infinera.com>
> ---
>   Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt | 6 ++++++
>   drivers/net/wan/fsl_ucc_hdlc.c                               | 5 ++++-
>   drivers/net/wan/fsl_ucc_hdlc.h                               | 1 +
>   3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt
> index 03c741602c6d..6d2dd8a31482 100644
> --- a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt
> +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt
> @@ -98,6 +98,12 @@ The property below is dependent on fsl,tdm-interface:
>   	usage: optional for tdm interface
>   	value type: <empty>
>   	Definition : Internal loopback connecting on TDM layer.
> +- fsl,hmask
> +	usage: optional
> +	Value type: <u16>
> +	Definition: HDLC address recognition. Set to zero to disable
> +		    address filtering of packets:
> +		    fsl,hmask = /bits/ 16 <0x0000>;
>   
>   Example for tdm interface:
>   
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
> index 0f703d7be5e0..be5b0096af3b 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.c
> +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> @@ -263,7 +263,7 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
>   	iowrite16be(MAX_FRAME_LENGTH, &priv->ucc_pram->mflr);
>   	iowrite16be(DEFAULT_RFTHR, &priv->ucc_pram->rfthr);
>   	iowrite16be(DEFAULT_RFTHR, &priv->ucc_pram->rfcnt);
> -	iowrite16be(DEFAULT_ADDR_MASK, &priv->ucc_pram->hmask);
> +	iowrite16be(priv->hmask, &priv->ucc_pram->hmask);
>   	iowrite16be(DEFAULT_HDLC_ADDR, &priv->ucc_pram->haddr1);
>   	iowrite16be(DEFAULT_HDLC_ADDR, &priv->ucc_pram->haddr2);
>   	iowrite16be(DEFAULT_HDLC_ADDR, &priv->ucc_pram->haddr3);
> @@ -1097,6 +1097,9 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
>   		if (ret)
>   			goto free_utdm;
>   	}
> +	

ERROR:TRAILING_WHITESPACE: trailing whitespace
#51: FILE: drivers/net/wan/fsl_ucc_hdlc.c:1099:
+^I$

'git show' (or git log -p) would show it to you in red background.

> +	if (of_property_read_u16(np, "fsl,hmask", &uhdlc_priv->hmask))
> +		uhdlc_priv->hmask = DEFAULT_ADDR_MASK;
>   
>   	ret = uhdlc_init(uhdlc_priv);
>   	if (ret) {
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h
> index c21134c1f180..b99fa2f1cd99 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.h
> +++ b/drivers/net/wan/fsl_ucc_hdlc.h
> @@ -106,6 +106,7 @@ struct ucc_hdlc_private {
>   
>   	unsigned short encoding;
>   	unsigned short parity;
> +	unsigned short hmask;
>   	u32 clocking;
>   	spinlock_t lock;	/* lock for Tx BD and Tx buffer */
>   #ifdef CONFIG_PM
> 

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH v2 5/6] net/wan/fsl_ucc_hdlc: GUMR for non tsa mode

[christophe leroy]

Le 29/08/2018 à 15:13, David Gounaris a écrit :
> The following bits in the GUMR is changed for non
> tsa mode: CDS, CTSP and CTSS are set to zero.
> 
> When set, there is no tx interrupts from the controller.
> 
> Signed-off-by: David Gounaris <david.gounaris@infinera.com>
> ---
>   drivers/net/wan/fsl_ucc_hdlc.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
> index be5b0096af3b..248f1f5bcd04 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.c
> +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> @@ -97,6 +97,13 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
>   	if (priv->tsa) {
>   		uf_info->tsa = 1;
>   		uf_info->ctsp = 1;
> +		uf_info->cds = 1;
> +		uf_info->ctss = 1;
> +	}
> +	else {

ERROR:ELSE_AFTER_BRACE: else should follow close brace '}'
#27: FILE: drivers/net/wan/fsl_ucc_hdlc.c:103:
+	}
+	else {

CHECK:BRACES: Unbalanced braces around else statement
#27: FILE: drivers/net/wan/fsl_ucc_hdlc.c:103:
+	else {

Should be:

} else {

> +		uf_info->cds = 0;
> +		uf_info->ctsp = 0;
> +		uf_info->ctss = 0;
>   	}
>   
>   	/* This sets HPM register in CMXUCR register which configures a
> 

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH v2 6/6] net/wan/fsl_ucc_hdlc: tx timeout handler

[christophe leroy]

Le 29/08/2018 à 15:13, David Gounaris a écrit :
> Added tx timerout handler. This helps
> when troubleshooting.
> 
> Signed-off-by: David Gounaris <david.gounaris@infinera.com>
> ---
>   drivers/net/wan/fsl_ucc_hdlc.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
> index 248f1f5bcd04..629ef5049d27 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.c
> +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> @@ -1002,11 +1002,15 @@ static const struct dev_pm_ops uhdlc_pm_ops = {
>   #define HDLC_PM_OPS NULL
>   
>   #endif
> +static void uhdlc_tx_timeout(struct net_device *ndev) {

ERROR:OPEN_BRACE: open brace '{' following function definitions go on 
the next line
#22: FILE: drivers/net/wan/fsl_ucc_hdlc.c:1004:
+static void uhdlc_tx_timeout(struct net_device *ndev) {

> +	netdev_err(ndev, "%s\n", __FUNCTION__);

WARNING:USE_FUNC: __func__ should be used instead of gcc specific 
__FUNCTION__
#23: FILE: drivers/net/wan/fsl_ucc_hdlc.c:1005:
+	netdev_err(ndev, "%s\n", __FUNCTION__);

> +}
>   static const struct net_device_ops uhdlc_ops = {
>   	.ndo_open       = uhdlc_open,
>   	.ndo_stop       = uhdlc_close,
>   	.ndo_start_xmit = hdlc_start_xmit,
>   	.ndo_do_ioctl   = uhdlc_ioctl,
> +	.ndo_tx_timeout	= uhdlc_tx_timeout,
>   };
>   
>   static int ucc_hdlc_probe(struct platform_device *pdev)
> @@ -1125,6 +1129,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
>   	hdlc = dev_to_hdlc(dev);
>   	dev->tx_queue_len = 16;
>   	dev->netdev_ops = &uhdlc_ops;
> +	dev->watchdog_timeo = 2*HZ;

CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#38: FILE: drivers/net/wan/fsl_ucc_hdlc.c:1131:
+	dev->watchdog_timeo = 2*HZ;

>   	hdlc->attach = ucc_hdlc_attach;
>   	hdlc->xmit = ucc_hdlc_tx;
>   	netif_napi_add(dev, &uhdlc_priv->napi, ucc_hdlc_poll, 32);
> 

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH v2 0/6] Ethernet over hdlc

[christophe leroy]

Le 29/08/2018 à 15:13, David Gounaris a écrit :
> Here is what has been changed in v2 after the review comments.
> 
> v2-0001: Using UCC_MAX_NUM

This one has a warning/check from checkpatch

> v2-0002: Unchanged
> v2-0003: Changed commit message
> v2-0004: Adding fsl,hmask into the dt instead of changing the default value.
> v2-0005: Unchanged
> v2-0006: Unchanged

All the above have an error from checkpatch

You should run checkpatch on your patches and fix the errors and warnings.

You can also see them at 
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=63075

Christophe

> 
> Adding robh+dt@kernel.org for comments regarding dt.
> 
> Best Regards
> David Gounaris
> 
> David Gounaris (6):
>    net/wan/fsl_ucc_hdlc: allow ucc index up to 7
>    net/wan/fsl_ucc_hdlc: allow PARITY_CRC16_PR0_CCITT parity
>    net/wan/fsl_ucc_hdlc: Adding ARPHRD_ETHER
>    net/wan/fsl_ucc_hdlc: hmask
>    net/wan/fsl_ucc_hdlc: GUMR for non tsa mode
>    net/wan/fsl_ucc_hdlc: tx timeout handler
> 
>   .../devicetree/bindings/soc/fsl/cpm_qe/network.txt |  6 +++++
>   drivers/net/wan/fsl_ucc_hdlc.c                     | 28 +++++++++++++++++++---
>   drivers/net/wan/fsl_ucc_hdlc.h                     |  1 +
>   3 files changed, 32 insertions(+), 3 deletions(-)
> 

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

[PATCH v3 0/6] Ethernet over hdlc

[David Gounaris]

Here is what has been changed in v3 after the review comments from v2.

v3-0001: corrected style problems
v3-0002: corrected style problems
v3-0003: corrected style problems
v3-0004: corrected style problems
v3-0005: corrected style problems
v3-0006: corrected style problems

Sorry for that, I did not know about scripts/checkpatch.pl.

David Gounaris (6):
  net/wan/fsl_ucc_hdlc: allow ucc index up to 7
  net/wan/fsl_ucc_hdlc: allow PARITY_CRC16_PR0_CCITT parity
  net/wan/fsl_ucc_hdlc: Adding ARPHRD_ETHER
  net/wan/fsl_ucc_hdlc: hmask
  net/wan/fsl_ucc_hdlc: GUMR for non tsa mode
  net/wan/fsl_ucc_hdlc: tx timeout handler

 .../devicetree/bindings/soc/fsl/cpm_qe/network.txt |  6 +++++
 drivers/net/wan/fsl_ucc_hdlc.c                     | 26 ++++++++++++++++++++--
 drivers/net/wan/fsl_ucc_hdlc.h                     |  1 +
 3 files changed, 31 insertions(+), 2 deletions(-)

-- 
2.13.6

[PATCH v3 1/6] net/wan/fsl_ucc_hdlc: allow ucc index up to 7

[David Gounaris]

There is a need to allow higher indexes to be
able to support MPC83xx platforms. (UCC1-UCC8)

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 33df76405b86..485764c537d9 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1016,7 +1016,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
 	}
 
 	ucc_num = val - 1;
-	if ((ucc_num > 3) || (ucc_num < 0)) {
+	if (ucc_num > (UCC_MAX_NUM - 1) || ucc_num < 0) {
 		dev_err(&pdev->dev, ": Invalid UCC num\n");
 		return -EINVAL;
 	}
-- 
2.13.6

[PATCH v3 2/6] net/wan/fsl_ucc_hdlc: allow PARITY_CRC16_PR0_CCITT parity

[David Gounaris]

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 485764c537d9..5619e2c2e02d 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -781,6 +781,7 @@ static int ucc_hdlc_attach(struct net_device *dev, unsigned short encoding,
 
 	if (parity != PARITY_NONE &&
 	    parity != PARITY_CRC32_PR1_CCITT &&
+	    parity != PARITY_CRC16_PR0_CCITT &&
 	    parity != PARITY_CRC16_PR1_CCITT)
 		return -EINVAL;
 
-- 
2.13.6

[PATCH v3 3/6] net/wan/fsl_ucc_hdlc: Adding ARPHRD_ETHER

[David Gounaris]

This patch is to avoid discarding ethernet
packets when using HDLC_ETH protocol.

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 5619e2c2e02d..2aaa5e1959a9 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -376,6 +376,10 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
 		dev->stats.tx_bytes += skb->len;
 		break;
 
+	case ARPHRD_ETHER:
+		dev->stats.tx_bytes += skb->len;
+		break;
+
 	default:
 		dev->stats.tx_dropped++;
 		dev_kfree_skb(skb);
@@ -513,6 +517,7 @@ static int hdlc_rx_done(struct ucc_hdlc_private *priv, int rx_work_limit)
 			break;
 
 		case ARPHRD_PPP:
+		case ARPHRD_ETHER:
 			length -= HDLC_CRC_SIZE;
 
 			skb = dev_alloc_skb(length);
-- 
2.13.6

[PATCH v3 4/6] net/wan/fsl_ucc_hdlc: hmask

[David Gounaris]

Ability to set hmask in the device-tree,
which can be used to change address
filtering of packets.

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt | 6 ++++++
 drivers/net/wan/fsl_ucc_hdlc.c                               | 5 ++++-
 drivers/net/wan/fsl_ucc_hdlc.h                               | 1 +
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt
index 03c741602c6d..6d2dd8a31482 100644
--- a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt
+++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/network.txt
@@ -98,6 +98,12 @@ The property below is dependent on fsl,tdm-interface:
 	usage: optional for tdm interface
 	value type: <empty>
 	Definition : Internal loopback connecting on TDM layer.
+- fsl,hmask
+	usage: optional
+	Value type: <u16>
+	Definition: HDLC address recognition. Set to zero to disable
+		    address filtering of packets:
+		    fsl,hmask = /bits/ 16 <0x0000>;
 
 Example for tdm interface:
 
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 2aaa5e1959a9..3dacafb219c6 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -263,7 +263,7 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
 	iowrite16be(MAX_FRAME_LENGTH, &priv->ucc_pram->mflr);
 	iowrite16be(DEFAULT_RFTHR, &priv->ucc_pram->rfthr);
 	iowrite16be(DEFAULT_RFTHR, &priv->ucc_pram->rfcnt);
-	iowrite16be(DEFAULT_ADDR_MASK, &priv->ucc_pram->hmask);
+	iowrite16be(priv->hmask, &priv->ucc_pram->hmask);
 	iowrite16be(DEFAULT_HDLC_ADDR, &priv->ucc_pram->haddr1);
 	iowrite16be(DEFAULT_HDLC_ADDR, &priv->ucc_pram->haddr2);
 	iowrite16be(DEFAULT_HDLC_ADDR, &priv->ucc_pram->haddr3);
@@ -1097,6 +1097,9 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
 			goto free_utdm;
 	}
 
+	if (of_property_read_u16(np, "fsl,hmask", &uhdlc_priv->hmask))
+		uhdlc_priv->hmask = DEFAULT_ADDR_MASK;
+
 	ret = uhdlc_init(uhdlc_priv);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to init uhdlc\n");
diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h
index c21134c1f180..b99fa2f1cd99 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.h
+++ b/drivers/net/wan/fsl_ucc_hdlc.h
@@ -106,6 +106,7 @@ struct ucc_hdlc_private {
 
 	unsigned short encoding;
 	unsigned short parity;
+	unsigned short hmask;
 	u32 clocking;
 	spinlock_t lock;	/* lock for Tx BD and Tx buffer */
 #ifdef CONFIG_PM
-- 
2.13.6

[PATCH v3 5/6] net/wan/fsl_ucc_hdlc: GUMR for non tsa mode

[David Gounaris]

The following bits in the GUMR is changed for non
tsa mode: CDS, CTSP and CTSS are set to zero.

When set, there is no tx interrupts from the controller.

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 3dacafb219c6..999d93fa54f7 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -97,6 +97,12 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
 	if (priv->tsa) {
 		uf_info->tsa = 1;
 		uf_info->ctsp = 1;
+		uf_info->cds = 1;
+		uf_info->ctss = 1;
+	} else {
+		uf_info->cds = 0;
+		uf_info->ctsp = 0;
+		uf_info->ctss = 0;
 	}
 
 	/* This sets HPM register in CMXUCR register which configures a
-- 
2.13.6

[PATCH v3 6/6] net/wan/fsl_ucc_hdlc: tx timeout handler

[David Gounaris]

Added tx timeout handler. This helps
when troubleshooting.

Signed-off-by: David Gounaris <david.gounaris@infinera.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 999d93fa54f7..124fea454ac4 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1000,11 +1000,17 @@ static const struct dev_pm_ops uhdlc_pm_ops = {
 #define HDLC_PM_OPS NULL
 
 #endif
+static void uhdlc_tx_timeout(struct net_device *ndev)
+{
+	netdev_err(ndev, "%s\n", __func__);
+}
+
 static const struct net_device_ops uhdlc_ops = {
 	.ndo_open       = uhdlc_open,
 	.ndo_stop       = uhdlc_close,
 	.ndo_start_xmit = hdlc_start_xmit,
 	.ndo_do_ioctl   = uhdlc_ioctl,
+	.ndo_tx_timeout	= uhdlc_tx_timeout,
 };
 
 static int ucc_hdlc_probe(struct platform_device *pdev)
@@ -1123,6 +1129,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
 	hdlc = dev_to_hdlc(dev);
 	dev->tx_queue_len = 16;
 	dev->netdev_ops = &uhdlc_ops;
+	dev->watchdog_timeo = 2 * HZ;
 	hdlc->attach = ucc_hdlc_attach;
 	hdlc->xmit = ucc_hdlc_tx;
 	netif_napi_add(dev, &uhdlc_priv->napi, ucc_hdlc_poll, 32);
-- 
2.13.6

Re: [PATCH v3 0/6] Ethernet over hdlc

[David Miller]

From: David Gounaris <david.gounaris@infinera.com>
Date: Mon,  3 Sep 2018 14:47:24 +0200

> Here is what has been changed in v3 after the review comments from v2.
> 
> v3-0001: corrected style problems
> v3-0002: corrected style problems
> v3-0003: corrected style problems
> v3-0004: corrected style problems
> v3-0005: corrected style problems
> v3-0006: corrected style problems
> 
> Sorry for that, I did not know about scripts/checkpatch.pl.

Series applied to net-next.