2019-08-27 09:32:32

by Razvan Stefanescu

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

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


2019-08-27 09:32:45

by Razvan Stefanescu

[permalink] [raw]
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 <[email protected]>
---
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

2019-08-27 09:33:13

by Razvan Stefanescu

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

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 <[email protected]>
---
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

2019-08-27 09:33:19

by Razvan Stefanescu

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

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

Signed-off-by: Razvan Stefanescu <[email protected]>
---
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

2019-08-27 09:33:49

by Razvan Stefanescu

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

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

Signed-off-by: Razvan Stefanescu <[email protected]>
---
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

2019-08-27 12:53:07

by Andrew Lunn

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

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

2019-08-27 12:56:27

by Andrew Lunn

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

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 <[email protected]>
> ---
> 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

2019-08-27 13:25:28

by Razvan Stefanescu

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



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

2019-08-27 20:58:47

by Tristram.Ha

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

> 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.

2019-08-27 21:05:13

by Tristram.Ha

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

> 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 <[email protected]>
> ---
> 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.