[PATCH 0/4] net: dsa: microchip: add KSZ8563 support

[PATCH 0/4] net: dsa: microchip: add KSZ8563 support

[Razvan Stefanescu]

This patchset adds compatiblity strings for the KSZ8563 switch and
also adds two small fixes to the ksz9477 driver.

Razvan Stefanescu (4):
  dt-bindings: net: dsa: document additional Microchip KSZ8563 switch
  net: dsa: microchip: add KSZ8563 compatibility string
  net: dsa: microchip: fix interrupt mask
  net: dsa: microchip: avoid hard-codded port count

 Documentation/devicetree/bindings/net/dsa/ksz.txt | 1 +
 drivers/net/dsa/microchip/ksz9477.c               | 2 +-
 drivers/net/dsa/microchip/ksz9477_reg.h           | 2 +-
 drivers/net/dsa/microchip/ksz9477_spi.c           | 1 +
 4 files changed, 4 insertions(+), 2 deletions(-)

-- 
2.20.1

[PATCH 1/4] dt-bindings: net: dsa: document additional Microchip KSZ8563 switch

[Razvan Stefanescu]

It is a 3-Port 10/100 Ethernet Switch with 1588v2 PTP.

Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com>
---
 Documentation/devicetree/bindings/net/dsa/ksz.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt b/Documentation/devicetree/bindings/net/dsa/ksz.txt
index 5e8429b6f9ca..95e91e84151c 100644
--- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
+++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
@@ -15,6 +15,7 @@ Required properties:
   - "microchip,ksz8565"
   - "microchip,ksz9893"
   - "microchip,ksz9563"
+  - "microchip,ksz8563"
 
 Optional properties:
 
-- 
2.20.1

[PATCH 2/4] net: dsa: microchip: add KSZ8563 compatibility string

[Razvan Stefanescu]

It is a 3-Port 10/100 Ethernet Switch with 1588v2 PTP.

Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477_spi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index a226b389e12d..2e402e4d866f 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -80,6 +80,7 @@ static const struct of_device_id ksz9477_dt_ids[] = {
 	{ .compatible = "microchip,ksz9897" },
 	{ .compatible = "microchip,ksz9893" },
 	{ .compatible = "microchip,ksz9563" },
+	{ .compatible = "microchip,ksz8563" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, ksz9477_dt_ids);
-- 
2.20.1

[PATCH 3/4] net: dsa: microchip: fix interrupt mask

[Razvan Stefanescu]

Global Interrupt Mask Register comprises of Lookup Engine (LUE) Interrupt
Mask (bit 31) and GPIO Pin Output Trigger and Timestamp Unit Interrupt
Mask (bit 29).

This corrects LUE bit.

Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477_reg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz9477_reg.h b/drivers/net/dsa/microchip/ksz9477_reg.h
index 2938e892b631..f3949d7b9bbd 100644
--- a/drivers/net/dsa/microchip/ksz9477_reg.h
+++ b/drivers/net/dsa/microchip/ksz9477_reg.h
@@ -76,7 +76,7 @@
 #define TRIG_TS_INT			BIT(30)
 #define APB_TIMEOUT_INT			BIT(29)
 
-#define SWITCH_INT_MASK			(TRIG_TS_INT | APB_TIMEOUT_INT)
+#define SWITCH_INT_MASK			(LUE_INT | TRIG_TS_INT)
 
 #define REG_SW_PORT_INT_STATUS__4	0x0018
 #define REG_SW_PORT_INT_MASK__4		0x001C
-- 
2.20.1

[PATCH 4/4] net: dsa: microchip: avoid hard-codded port count

[Razvan Stefanescu]

Use port_cnt value to disable interrupts on switch reset.

Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 187be42de5f1..54fc05595d48 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -213,7 +213,7 @@ static int ksz9477_reset_switch(struct ksz_device *dev)
 
 	/* disable interrupts */
 	ksz_write32(dev, REG_SW_INT_MASK__4, SWITCH_INT_MASK);
-	ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0x7F);
+	ksz_write32(dev, REG_SW_PORT_INT_MASK__4, dev->port_cnt);
 	ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
 
 	/* set broadcast storm protection 10% rate */
-- 
2.20.1

Re: [PATCH 3/4] net: dsa: microchip: fix interrupt mask

[Andrew Lunn]

On Tue, Aug 27, 2019 at 12:31:09PM +0300, Razvan Stefanescu wrote:
> Global Interrupt Mask Register comprises of Lookup Engine (LUE) Interrupt
> Mask (bit 31) and GPIO Pin Output Trigger and Timestamp Unit Interrupt
> Mask (bit 29).
> 
> This corrects LUE bit.

Hi Razvan

Is this a fix? Something that should be back ported to old kernels?

Thanks
	Andrew

Re: [PATCH 4/4] net: dsa: microchip: avoid hard-codded port count

[Andrew Lunn]

On Tue, Aug 27, 2019 at 12:31:10PM +0300, Razvan Stefanescu wrote:
> Use port_cnt value to disable interrupts on switch reset.
> 
> Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com>
> ---
>  drivers/net/dsa/microchip/ksz9477.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
> index 187be42de5f1..54fc05595d48 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -213,7 +213,7 @@ static int ksz9477_reset_switch(struct ksz_device *dev)
>  
>  	/* disable interrupts */
>  	ksz_write32(dev, REG_SW_INT_MASK__4, SWITCH_INT_MASK);
> -	ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0x7F);
> +	ksz_write32(dev, REG_SW_PORT_INT_MASK__4, dev->port_cnt);
>  	ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);

Humm, is this correct? 0x7f suggests this is a bitmap for 7 ports.
The name port_cnt suggests it is a count, e.g. 7 for 7 ports.

0x7f != 7.

Thanks
	Andrew

Re: [PATCH 3/4] net: dsa: microchip: fix interrupt mask

[Razvan.Stefanescu]

On 27/08/2019 15:51, Andrew Lunn wrote:
> 
> On Tue, Aug 27, 2019 at 12:31:09PM +0300, Razvan Stefanescu wrote:
>> Global Interrupt Mask Register comprises of Lookup Engine (LUE) Interrupt
>> Mask (bit 31) and GPIO Pin Output Trigger and Timestamp Unit Interrupt
>> Mask (bit 29).
>>
>> This corrects LUE bit.
> 
> Hi Razvan
> 
> Is this a fix? Something that should be back ported to old kernels?

Hello,

During testing I did not observed any issues with the old value. So I am 
not sure how the switch is affected by the incorrect setting.

Maybe maintainers will be able to make a better assessment if this needs 
back-porting. And I will be happy to do it if it is necessary.

Best regards,
Razvan

RE: [PATCH 3/4] net: dsa: microchip: fix interrupt mask

[Tristram.Ha]

> On 27/08/2019 15:51, Andrew Lunn wrote:
> >
> > On Tue, Aug 27, 2019 at 12:31:09PM +0300, Razvan Stefanescu wrote:
> >> Global Interrupt Mask Register comprises of Lookup Engine (LUE)
> Interrupt
> >> Mask (bit 31) and GPIO Pin Output Trigger and Timestamp Unit Interrupt
> >> Mask (bit 29).
> >>
> >> This corrects LUE bit.
> >
> > Hi Razvan
> >
> > Is this a fix? Something that should be back ported to old kernels?
> 
> Hello,
> 
> During testing I did not observed any issues with the old value. So I am
> not sure how the switch is affected by the incorrect setting.
> 
> Maybe maintainers will be able to make a better assessment if this needs
> back-porting. And I will be happy to do it if it is necessary.
> 

I do not think the change has any effect as the interrupt handling is not implemented in the driver, unless I am mistaken and do not know about the new code.

Currently those 3 interrupts do not do anything that are required in normal operation.

The first one LUE_INT notifies the driver when there are learn/write fails in the MAC table.  This condition rarely happens unless the switch is going through stress test.  When this interrupt happens software cannot do anything to resolve the issue.  It may become a denial of service if the MAC table keeps triggering learn fail.

The second one is used by PTP code, which is not implemented.

The third one is triggered when register access space does not exist.  It is useful during development so driver knows it is accessing the wrong register.  It can also become a denial of service if someone keeps accessing wrong registers.  But then that person can do anything with the chip.

RE: [PATCH 4/4] net: dsa: microchip: avoid hard-codded port count

[Tristram.Ha]

> Subject: [PATCH 4/4] net: dsa: microchip: avoid hard-codded port count
> 
> Use port_cnt value to disable interrupts on switch reset.
> 
> Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com>
> ---
>  drivers/net/dsa/microchip/ksz9477.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz9477.c
> b/drivers/net/dsa/microchip/ksz9477.c
> index 187be42de5f1..54fc05595d48 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -213,7 +213,7 @@ static int ksz9477_reset_switch(struct ksz_device
> *dev)
> 
>  	/* disable interrupts */
>  	ksz_write32(dev, REG_SW_INT_MASK__4, SWITCH_INT_MASK);
> -	ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0x7F);
> +	ksz_write32(dev, REG_SW_PORT_INT_MASK__4, dev->port_cnt);
>  	ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
> 
>  	/* set broadcast storm protection 10% rate */

The register value is a portmap, so using port_cnt may be wrong.

The chip is a 7-port switch.  There is a 6-port variant, but it is okay to write 0x7F.

There is also a 3-port variant which uses a different design.  It is a bit of stretch to use 0x7F on it.

It is more a code readability or correctness than incorrect hardware operation.