[PATCH net-next 0/4] net: lan966x: hardcode port count

[PATCH net-next 0/4] net: lan966x: hardcode port count

[Michael Walle]

Don't rely on the device tree to count the number of physical port. Instead
introduce a new compatible string which the driver can use to select the
correct port count.

This also hardcodes the generic compatible string to 8. The rationale is
that this compatible string was just used for the LAN9668 for now and I'm
not even sure the current driver would support the LAN9662.

Michael Walle (4):
  net: lan966x: hardcode the number of external ports
  dt-bindings: net: lan966x: add specific compatible string
  net: lan966x: add new compatible microchip,lan9668-switch
  ARM: dts: lan966x: use new microchip,lan9668-switch compatible

 .../net/microchip,lan966x-switch.yaml         |  5 +++-
 arch/arm/boot/dts/lan966x.dtsi                |  2 +-
 .../ethernet/microchip/lan966x/lan966x_main.c | 24 +++++++++++++------
 3 files changed, 22 insertions(+), 9 deletions(-)

-- 
2.30.2

[PATCH net-next 1/4] net: lan966x: hardcode the number of external ports

[Michael Walle]

Instead of counting the child nodes in the device tree, hardcode the
number of ports in the driver itself. The counting won't work at all
if an ethernet port is marked as disabled, eg. because it is not
connected on the board at all.

This is hardcoding the number of ports to eight for the generic
compatible string "microchip,lan966x-switch", although it clearly only
applies to the LAN9668. This is because, first, there is only one user
for now, and that is the LAN9668 and second, the generic compatible
string will be deprecated in favor of a more specific one. Therefore,
if there will be support for the LAN9662, it can be added by another
specific compatible string.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 .../ethernet/microchip/lan966x/lan966x_main.c | 23 +++++++++++++------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
index 5784c4161e5e..d611b52d3a07 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
@@ -26,8 +26,16 @@
 
 #define IO_RANGES 2
 
+struct lan966x_info {
+	u8 num_phys_ports;
+};
+
+static const struct lan966x_info lan9668_info = {
+	.num_phys_ports = 8,
+};
+
 static const struct of_device_id lan966x_match[] = {
-	{ .compatible = "microchip,lan966x-switch" },
+	{ .compatible = "microchip,lan966x-switch", .data = &lan9668_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, lan966x_match);
@@ -992,9 +1000,10 @@ static int lan966x_reset_switch(struct lan966x *lan966x)
 static int lan966x_probe(struct platform_device *pdev)
 {
 	struct fwnode_handle *ports, *portnp;
+	const struct lan966x_info *info;
 	struct lan966x *lan966x;
 	u8 mac_addr[ETH_ALEN];
-	int err, i;
+	int err;
 
 	lan966x = devm_kzalloc(&pdev->dev, sizeof(*lan966x), GFP_KERNEL);
 	if (!lan966x)
@@ -1003,6 +1012,10 @@ static int lan966x_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, lan966x);
 	lan966x->dev = &pdev->dev;
 
+	info = device_get_match_data(&pdev->dev);
+	if (!info)
+		return -ENODEV;
+
 	if (!device_get_mac_address(&pdev->dev, mac_addr)) {
 		ether_addr_copy(lan966x->base_mac, mac_addr);
 	} else {
@@ -1025,11 +1038,7 @@ static int lan966x_probe(struct platform_device *pdev)
 	if (err)
 		return dev_err_probe(&pdev->dev, err, "Reset failed");
 
-	i = 0;
-	fwnode_for_each_available_child_node(ports, portnp)
-		++i;
-
-	lan966x->num_phys_ports = i;
+	lan966x->num_phys_ports = info->num_phys_ports;
 	lan966x->ports = devm_kcalloc(&pdev->dev, lan966x->num_phys_ports,
 				      sizeof(struct lan966x_port *),
 				      GFP_KERNEL);
-- 
2.30.2

[PATCH net-next 2/4] dt-bindings: net: lan966x: add specific compatible string

[Michael Walle]

Add a specific compatible string for the LAN9668 and deprecate the old
one.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 .../devicetree/bindings/net/microchip,lan966x-switch.yaml    | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml b/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
index dc116f14750e..efd6bbd51d68 100644
--- a/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
+++ b/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
@@ -20,7 +20,10 @@ properties:
     pattern: "^switch@[0-9a-f]+$"
 
   compatible:
-    const: microchip,lan966x-switch
+    oneOf:
+      - const: microchip,lan9668-switch
+      - const: microchip,lan966x-switch
+        deprecated: true
 
   reg:
     items:
-- 
2.30.2

[PATCH net-next 3/4] net: lan966x: add new compatible microchip,lan9668-switch

[Michael Walle]

The old generic compatible string is deprecated. Add the new specific
one.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/ethernet/microchip/lan966x/lan966x_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
index d611b52d3a07..adc5474041a6 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
@@ -35,6 +35,7 @@ static const struct lan966x_info lan9668_info = {
 };
 
 static const struct of_device_id lan966x_match[] = {
+	{ .compatible = "microchip,lan9668-switch", .data = &lan9668_info },
 	{ .compatible = "microchip,lan966x-switch", .data = &lan9668_info },
 	{ }
 };
-- 
2.30.2

[PATCH net-next 4/4] ARM: dts: lan966x: use new microchip,lan9668-switch compatible

[Michael Walle]

The old generic microchip,lan966x-switch compatible string was
deprecated. Use the new one.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 arch/arm/boot/dts/lan966x.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
index 48971d80c82c..da0657c57cdf 100644
--- a/arch/arm/boot/dts/lan966x.dtsi
+++ b/arch/arm/boot/dts/lan966x.dtsi
@@ -85,7 +85,7 @@ soc {
 		ranges;
 
 		switch: switch@e0000000 {
-			compatible = "microchip,lan966x-switch";
+			compatible = "microchip,lan9668-switch";
 			reg = <0xe0000000 0x0100000>,
 			      <0xe2000000 0x0800000>;
 			reg-names = "cpu", "gcb";
-- 
2.30.2

Re: [PATCH net-next 4/4] ARM: dts: lan966x: use new microchip,lan9668-switch compatible

[Krzysztof Kozlowski]

On 30/06/2022 16:02, Michael Walle wrote:
> The old generic microchip,lan966x-switch compatible string was
> deprecated. Use the new one.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  arch/arm/boot/dts/lan966x.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/lan966x.dtsi b/arch/arm/boot/dts/lan966x.dtsi
> index 48971d80c82c..da0657c57cdf 100644
> --- a/arch/arm/boot/dts/lan966x.dtsi
> +++ b/arch/arm/boot/dts/lan966x.dtsi
> @@ -85,7 +85,7 @@ soc {
>  		ranges;
>  
>  		switch: switch@e0000000 {
> -			compatible = "microchip,lan966x-switch";
> +			compatible = "microchip,lan9668-switch";

While technically correct, the change cannot go via independent trees
and may break out-of-tree users of this DTS.

Usually the nicer way is to use two compatibles (the old as fallback).
Anyway, this one is fine with me - up to subarch maintainer.

Best regards,
Krzysztof

Re: [PATCH net-next 0/4] net: lan966x: hardcode port count

[Horatiu Vultur]

The 06/30/2022 16:02, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Don't rely on the device tree to count the number of physical port. Instead
> introduce a new compatible string which the driver can use to select the
> correct port count.
> 
> This also hardcodes the generic compatible string to 8. The rationale is
> that this compatible string was just used for the LAN9668 for now and I'm
> not even sure the current driver would support the LAN9662.

It works also on LAN9662, but I didn't have time to send patches for
DTs. Then when I send patches for LAN9662, do I need to go in all dts
files to change the compatible string for the 'switch' node?

> 
> Michael Walle (4):
>   net: lan966x: hardcode the number of external ports
>   dt-bindings: net: lan966x: add specific compatible string
>   net: lan966x: add new compatible microchip,lan9668-switch
>   ARM: dts: lan966x: use new microchip,lan9668-switch compatible
> 
>  .../net/microchip,lan966x-switch.yaml         |  5 +++-
>  arch/arm/boot/dts/lan966x.dtsi                |  2 +-
>  .../ethernet/microchip/lan966x/lan966x_main.c | 24 +++++++++++++------
>  3 files changed, 22 insertions(+), 9 deletions(-)
> 
> --
> 2.30.2
> 

-- 
/Horatiu

Re: [PATCH net-next 0/4] net: lan966x: hardcode port count

[Michael Walle]

Am 2022-06-30 22:44, schrieb Horatiu Vultur:
> The 06/30/2022 16:02, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the content is safe
>> 
>> Don't rely on the device tree to count the number of physical port. 
>> Instead
>> introduce a new compatible string which the driver can use to select 
>> the
>> correct port count.
>> 
>> This also hardcodes the generic compatible string to 8. The rationale 
>> is
>> that this compatible string was just used for the LAN9668 for now and 
>> I'm
>> not even sure the current driver would support the LAN9662.
> 
> It works also on LAN9662, but I didn't have time to send patches for
> DTs. Then when I send patches for LAN9662, do I need to go in all dts
> files to change the compatible string for the 'switch' node?

I'd assume there is one lan9662.dtsi and yes, there should then be
   compatible = "microchip,lan9662-switch";
or
   compatible = "microchip,lan9662-switch", "microchip,lan966x-switch";
depending on the outcome of the question Krzysztof raised.

And of course adding the compatible string to the driver with a port
count of 4 (?). I can't find anything about the lan9662, and you've
mentioned it has 4 ports. Are there four external ports? I was
under the impression the last digit of the SoC name stands for the
number of ports.

-michael

Re: [PATCH net-next 0/4] net: lan966x: hardcode port count

[Andrew Lunn]

On Thu, Jun 30, 2022 at 04:02:33PM +0200, Michael Walle wrote:
> Don't rely on the device tree to count the number of physical port. Instead
> introduce a new compatible string which the driver can use to select the
> correct port count.

Does the silicon have an ID register which identifies what the device
is?

	Andrew

Re: [PATCH net-next 0/4] net: lan966x: hardcode port count

[Horatiu Vultur]

The 06/30/2022 22:56, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Am 2022-06-30 22:44, schrieb Horatiu Vultur:
> > The 06/30/2022 16:02, Michael Walle wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > > the content is safe
> > > 
> > > Don't rely on the device tree to count the number of physical port.
> > > Instead
> > > introduce a new compatible string which the driver can use to select
> > > the
> > > correct port count.
> > > 
> > > This also hardcodes the generic compatible string to 8. The rationale
> > > is
> > > that this compatible string was just used for the LAN9668 for now and
> > > I'm
> > > not even sure the current driver would support the LAN9662.
> > 
> > It works also on LAN9662, but I didn't have time to send patches for
> > DTs. Then when I send patches for LAN9662, do I need to go in all dts
> > files to change the compatible string for the 'switch' node?
> 
> I'd assume there is one lan9662.dtsi and yes, there should then be
>   compatible = "microchip,lan9662-switch";
> or
>   compatible = "microchip,lan9662-switch", "microchip,lan966x-switch";
> depending on the outcome of the question Krzysztof raised.
> 
> And of course adding the compatible string to the driver with a port
> count of 4 (?). I can't find anything about the lan9662,

I am not sure why they have not upload yet the datasheet for lan9662.

>and you've > mentioned it has 4 ports.  Are there four external ports? 

You can have up to 4 ports.
You can have 4 external ports or you can use the internal ones plus two
external.

> I was  under the impression the last digit of the SoC name stands for the
> number of ports.

That would make much more sense but I don't understand why they have
name it like this.

> 
> -michael

-- 
/Horatiu