* [PATCH 2/2] arm64: dts: ls1028a: disable the felix switch by default
2020-03-12 16:43 [PATCH 1/2] net: dsa: felix: allow the device to be disabled Michael Walle
@ 2020-03-12 16:43 ` Michael Walle
2020-03-12 16:50 ` Michael Walle
2020-03-16 2:33 ` Shawn Guo
2020-03-12 21:35 ` [PATCH 1/2] net: dsa: felix: allow the device to be disabled Vladimir Oltean
2020-03-15 3:53 ` David Miller
2 siblings, 2 replies; 10+ messages in thread
From: Michael Walle @ 2020-03-12 16:43 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-kernel, netdev
Cc: David S . Miller, Florian Fainelli, Vivien Didelot, Andrew Lunn,
Claudiu Manoil, Vladimir Oltean, Rob Herring, Li Yang, Shawn Guo,
Michael Walle
Disable the felix switch by default and enable it per board which are
actually using it.
Signed-off-by: Michael Walle <michael@walle.cc>
---
.../boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts | 4 ++++
.../boot/dts/freescale/fsl-ls1028a-kontron-sl28-var2.dts | 4 ++++
arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 4 ++++
arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 3 ++-
4 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
index a83a176cf18a..d4ca12b140b4 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
@@ -63,6 +63,10 @@
};
};
+&mscc_felix {
+ status = "okay";
+};
+
&mscc_felix_port0 {
label = "swp0";
managed = "in-band-status";
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var2.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var2.dts
index 0a34ff682027..901b5b161def 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var2.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var2.dts
@@ -48,6 +48,10 @@
status = "okay";
};
+&mscc_felix {
+ status = "okay";
+};
+
&mscc_felix_port0 {
label = "gbe0";
phy-handle = <&phy0>;
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
index 0d27b5667b8c..8294d364112e 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
@@ -228,6 +228,10 @@
status = "okay";
};
+&mscc_felix {
+ status = "okay";
+};
+
&mscc_felix_port0 {
label = "swp0";
managed = "in-band-status";
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index c09279379723..70a10268bb83 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -933,10 +933,11 @@
fsl,extts-fifo;
};
- ethernet-switch@0,5 {
+ mscc_felix: ethernet-switch@0,5 {
reg = <0x000500 0 0 0 0>;
/* IEP INT_B */
interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
ports {
#address-cells = <1>;
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/2] arm64: dts: ls1028a: disable the felix switch by default
2020-03-12 16:43 ` [PATCH 2/2] arm64: dts: ls1028a: disable the felix switch by default Michael Walle
@ 2020-03-12 16:50 ` Michael Walle
2020-03-16 2:33 ` Shawn Guo
1 sibling, 0 replies; 10+ messages in thread
From: Michael Walle @ 2020-03-12 16:50 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-kernel, netdev
Cc: David S . Miller, Florian Fainelli, Vivien Didelot, Andrew Lunn,
Claudiu Manoil, Vladimir Oltean, Rob Herring, Li Yang, Shawn Guo
Am 2020-03-12 17:43, schrieb Michael Walle:
> Disable the felix switch by default and enable it per board which are
> actually using it.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
I forgot to mention that this patch depends on the following series:
https://lore.kernel.org/linux-devicetree/20200311074929.19569-1-michael@walle.cc/
Sorry,
-michael
> ---
> .../boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts | 4 ++++
> .../boot/dts/freescale/fsl-ls1028a-kontron-sl28-var2.dts | 4 ++++
> arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 4 ++++
> arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 3 ++-
> 4 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git
> a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
> b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
> index a83a176cf18a..d4ca12b140b4 100644
> ---
> a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
> +++
> b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
> @@ -63,6 +63,10 @@
> };
> };
>
> +&mscc_felix {
> + status = "okay";
> +};
> +
> &mscc_felix_port0 {
> label = "swp0";
> managed = "in-band-status";
> diff --git
> a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var2.dts
> b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var2.dts
> index 0a34ff682027..901b5b161def 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var2.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var2.dts
> @@ -48,6 +48,10 @@
> status = "okay";
> };
>
> +&mscc_felix {
> + status = "okay";
> +};
> +
> &mscc_felix_port0 {
> label = "gbe0";
> phy-handle = <&phy0>;
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> index 0d27b5667b8c..8294d364112e 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> @@ -228,6 +228,10 @@
> status = "okay";
> };
>
> +&mscc_felix {
> + status = "okay";
> +};
> +
> &mscc_felix_port0 {
> label = "swp0";
> managed = "in-band-status";
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> index c09279379723..70a10268bb83 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> @@ -933,10 +933,11 @@
> fsl,extts-fifo;
> };
>
> - ethernet-switch@0,5 {
> + mscc_felix: ethernet-switch@0,5 {
> reg = <0x000500 0 0 0 0>;
> /* IEP INT_B */
> interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
>
> ports {
> #address-cells = <1>;
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/2] arm64: dts: ls1028a: disable the felix switch by default
2020-03-12 16:43 ` [PATCH 2/2] arm64: dts: ls1028a: disable the felix switch by default Michael Walle
2020-03-12 16:50 ` Michael Walle
@ 2020-03-16 2:33 ` Shawn Guo
1 sibling, 0 replies; 10+ messages in thread
From: Shawn Guo @ 2020-03-16 2:33 UTC (permalink / raw)
To: Michael Walle
Cc: linux-arm-kernel, devicetree, linux-kernel, netdev,
David S . Miller, Florian Fainelli, Vivien Didelot, Andrew Lunn,
Claudiu Manoil, Vladimir Oltean, Rob Herring, Li Yang
On Thu, Mar 12, 2020 at 05:43:20PM +0100, Michael Walle wrote:
> Disable the felix switch by default and enable it per board which are
> actually using it.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
Applied, thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] net: dsa: felix: allow the device to be disabled
2020-03-12 16:43 [PATCH 1/2] net: dsa: felix: allow the device to be disabled Michael Walle
2020-03-12 16:43 ` [PATCH 2/2] arm64: dts: ls1028a: disable the felix switch by default Michael Walle
@ 2020-03-12 21:35 ` Vladimir Oltean
2020-03-12 21:45 ` Florian Fainelli
2020-03-13 10:01 ` Andrew Lunn
2020-03-15 3:53 ` David Miller
2 siblings, 2 replies; 10+ messages in thread
From: Vladimir Oltean @ 2020-03-12 21:35 UTC (permalink / raw)
To: Michael Walle
Cc: moderated list:ARM/Mediatek SoC support, devicetree, lkml,
netdev, David S . Miller, Florian Fainelli, Vivien Didelot,
Andrew Lunn, Claudiu Manoil, Vladimir Oltean, Rob Herring,
Li Yang, Shawn Guo
On Thu, 12 Mar 2020 at 18:44, Michael Walle <michael@walle.cc> wrote:
>
> If there is no specific configuration of the felix switch in the device
> tree, but only the default configuration (ie. given by the SoCs dtsi
> file), the probe fails because no CPU port has been set. On the other
> hand you cannot set a default CPU port because that depends on the
> actual board using the switch.
>
> [ 2.701300] DSA: tree 0 has no CPU port
> [ 2.705167] mscc_felix 0000:00:00.5: Failed to register DSA switch: -22
> [ 2.711844] mscc_felix: probe of 0000:00:00.5 failed with error -22
>
> Thus let the device tree disable this device entirely, like it is also
> done with the enetc driver of the same SoC.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/net/dsa/ocelot/felix.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> index 69546383a382..531c7710063f 100644
> --- a/drivers/net/dsa/ocelot/felix.c
> +++ b/drivers/net/dsa/ocelot/felix.c
> @@ -699,6 +699,11 @@ static int felix_pci_probe(struct pci_dev *pdev,
> struct felix *felix;
> int err;
>
> + if (pdev->dev.of_node && !of_device_is_available(pdev->dev.of_node)) {
> + dev_info(&pdev->dev, "device is disabled, skipping\n");
> + return -ENODEV;
> + }
> +
IMHO since DSA is already dependent on device tree for PHY bindings,
it would make more sense to move this there:
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index e7c30b472034..f7ca01d93928 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -878,7 +878,7 @@ static int dsa_switch_probe(struct dsa_switch *ds)
if (!ds->num_ports)
return -EINVAL;
- if (np) {
+ if (np && of_device_is_available(np)) {
err = dsa_switch_parse_of(ds, np);
if (err)
dsa_switch_release_ports(ds);
so that we could enforce more uniform behavior across device drivers.
Then you might want to make felix shut up:
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 35124ef7e75b..fbd17fa94bff 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -712,10 +712,8 @@ static int felix_pci_probe(struct pci_dev *pdev,
felix->ds = ds;
err = dsa_register_switch(ds);
- if (err) {
- dev_err(&pdev->dev, "Failed to register DSA switch: %d\n", err);
+ if (err)
goto err_register_ds;
- }
return 0;
This has the disadvantage of not printing the "nice" "device is
disabled, skipping" message (useless in my opinion), but the advantage
of also shutting up on -EPROBE_DEFER.
> err = pci_enable_device(pdev);
> if (err) {
> dev_err(&pdev->dev, "device enable failed\n");
> --
> 2.20.1
>
Thanks,
-Vladimir
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/2] net: dsa: felix: allow the device to be disabled
2020-03-12 21:35 ` [PATCH 1/2] net: dsa: felix: allow the device to be disabled Vladimir Oltean
@ 2020-03-12 21:45 ` Florian Fainelli
2020-03-12 22:07 ` Vladimir Oltean
2020-03-13 10:01 ` Andrew Lunn
1 sibling, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2020-03-12 21:45 UTC (permalink / raw)
To: Vladimir Oltean, Michael Walle
Cc: moderated list:ARM/Mediatek SoC support, devicetree, lkml,
netdev, David S . Miller, Vivien Didelot, Andrew Lunn,
Claudiu Manoil, Vladimir Oltean, Rob Herring, Li Yang, Shawn Guo
On 3/12/20 2:35 PM, Vladimir Oltean wrote:
> On Thu, 12 Mar 2020 at 18:44, Michael Walle <michael@walle.cc> wrote:
>>
>> If there is no specific configuration of the felix switch in the device
>> tree, but only the default configuration (ie. given by the SoCs dtsi
>> file), the probe fails because no CPU port has been set. On the other
>> hand you cannot set a default CPU port because that depends on the
>> actual board using the switch.
>>
>> [ 2.701300] DSA: tree 0 has no CPU port
>> [ 2.705167] mscc_felix 0000:00:00.5: Failed to register DSA switch: -22
>> [ 2.711844] mscc_felix: probe of 0000:00:00.5 failed with error -22
>>
>> Thus let the device tree disable this device entirely, like it is also
>> done with the enetc driver of the same SoC.
>>
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>> drivers/net/dsa/ocelot/felix.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
>> index 69546383a382..531c7710063f 100644
>> --- a/drivers/net/dsa/ocelot/felix.c
>> +++ b/drivers/net/dsa/ocelot/felix.c
>> @@ -699,6 +699,11 @@ static int felix_pci_probe(struct pci_dev *pdev,
>> struct felix *felix;
>> int err;
>>
>> + if (pdev->dev.of_node && !of_device_is_available(pdev->dev.of_node)) {
>> + dev_info(&pdev->dev, "device is disabled, skipping\n");
>> + return -ENODEV;
>> + }
>> +
>
> IMHO since DSA is already dependent on device tree for PHY bindings,
> it would make more sense to move this there:
Michael's solution makes more sense, as this is a driver specific
problem whereby you have a pci_dev instance that is created and does not
honor the status property provided in Device Tree. If you were to look
for a proper solution it would likely be within the PCI core actually.
No other DSA switch has that problem because they use the
I2C/SPI/platform_device/GPIO/whatever entry points into the driver model.
--
Florian
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/2] net: dsa: felix: allow the device to be disabled
2020-03-12 21:45 ` Florian Fainelli
@ 2020-03-12 22:07 ` Vladimir Oltean
0 siblings, 0 replies; 10+ messages in thread
From: Vladimir Oltean @ 2020-03-12 22:07 UTC (permalink / raw)
To: Florian Fainelli
Cc: Michael Walle, moderated list:ARM/Mediatek SoC support,
devicetree, lkml, netdev, David S . Miller, Vivien Didelot,
Andrew Lunn, Claudiu Manoil, Vladimir Oltean, Rob Herring,
Li Yang, Shawn Guo
On Thu, 12 Mar 2020 at 23:45, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 3/12/20 2:35 PM, Vladimir Oltean wrote:
> > On Thu, 12 Mar 2020 at 18:44, Michael Walle <michael@walle.cc> wrote:
> >>
> >> If there is no specific configuration of the felix switch in the device
> >> tree, but only the default configuration (ie. given by the SoCs dtsi
> >> file), the probe fails because no CPU port has been set. On the other
> >> hand you cannot set a default CPU port because that depends on the
> >> actual board using the switch.
> >>
> >> [ 2.701300] DSA: tree 0 has no CPU port
> >> [ 2.705167] mscc_felix 0000:00:00.5: Failed to register DSA switch: -22
> >> [ 2.711844] mscc_felix: probe of 0000:00:00.5 failed with error -22
> >>
> >> Thus let the device tree disable this device entirely, like it is also
> >> done with the enetc driver of the same SoC.
> >>
> >> Signed-off-by: Michael Walle <michael@walle.cc>
> >> ---
> >> drivers/net/dsa/ocelot/felix.c | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> >> index 69546383a382..531c7710063f 100644
> >> --- a/drivers/net/dsa/ocelot/felix.c
> >> +++ b/drivers/net/dsa/ocelot/felix.c
> >> @@ -699,6 +699,11 @@ static int felix_pci_probe(struct pci_dev *pdev,
> >> struct felix *felix;
> >> int err;
> >>
> >> + if (pdev->dev.of_node && !of_device_is_available(pdev->dev.of_node)) {
> >> + dev_info(&pdev->dev, "device is disabled, skipping\n");
> >> + return -ENODEV;
> >> + }
> >> +
> >
> > IMHO since DSA is already dependent on device tree for PHY bindings,
> > it would make more sense to move this there:
>
> Michael's solution makes more sense, as this is a driver specific
> problem whereby you have a pci_dev instance that is created and does not
> honor the status property provided in Device Tree. If you were to look
> for a proper solution it would likely be within the PCI core actually.
>
> No other DSA switch has that problem because they use the
> I2C/SPI/platform_device/GPIO/whatever entry points into the driver model.
True, my problem with doing it in the PCI core is that "the book" [0]
doesn't actually say anything about the "status" property, so this
patch might get some pushback from the PCI maintainers (but I don't
actually know):
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 512cb4312ddd..50c2b3da134a 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2281,6 +2281,12 @@ static struct pci_dev *pci_scan_device(struct
pci_bus *bus, int devfn)
pci_set_of_node(dev);
+ if (dev->dev.of_node && !of_device_is_available(dev->dev.of_node)) {
+ pci_bus_put(dev->bus);
+ kfree(dev);
+ return NULL;
+ }
+
if (pci_setup_device(dev)) {
pci_bus_put(dev->bus);
kfree(dev);
OF bindings are completely optional with PCI, as you probably already
know. But there's nothing "driver" specific in disabling (i.e. not
probing) a device.
> --
>
[0] https://www.openfirmware.info/data/docs/bus.pci.pdf
Thanks,
-Vladimir
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] net: dsa: felix: allow the device to be disabled
2020-03-12 21:35 ` [PATCH 1/2] net: dsa: felix: allow the device to be disabled Vladimir Oltean
2020-03-12 21:45 ` Florian Fainelli
@ 2020-03-13 10:01 ` Andrew Lunn
1 sibling, 0 replies; 10+ messages in thread
From: Andrew Lunn @ 2020-03-13 10:01 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Michael Walle, moderated list:ARM/Mediatek SoC support,
devicetree, lkml, netdev, David S . Miller, Florian Fainelli,
Vivien Didelot, Claudiu Manoil, Vladimir Oltean, Rob Herring,
Li Yang, Shawn Guo
e> IMHO since DSA is already dependent on device tree for PHY bindings,
> it would make more sense to move this there:
That is not really true. You can instantiate a marvell switch using a
platform device. So long any you only have C22 PHYs in a sane
configuration, it will just work. There are boards out there do this,
on x86 platforms without device tree.
Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] net: dsa: felix: allow the device to be disabled
2020-03-12 16:43 [PATCH 1/2] net: dsa: felix: allow the device to be disabled Michael Walle
2020-03-12 16:43 ` [PATCH 2/2] arm64: dts: ls1028a: disable the felix switch by default Michael Walle
2020-03-12 21:35 ` [PATCH 1/2] net: dsa: felix: allow the device to be disabled Vladimir Oltean
@ 2020-03-15 3:53 ` David Miller
2020-03-20 11:03 ` Michael Walle
2 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2020-03-15 3:53 UTC (permalink / raw)
To: michael
Cc: linux-arm-kernel, devicetree, linux-kernel, netdev, f.fainelli,
vivien.didelot, andrew, claudiu.manoil, vladimir.oltean, robh+dt,
leoyang.li, shawnguo
This series depends upon some devicetree tree changes, so why don't you
submit these changes there and add my:
Acked-by: David S. Miller <davem@davemloft.net>
Thank you.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] net: dsa: felix: allow the device to be disabled
2020-03-15 3:53 ` David Miller
@ 2020-03-20 11:03 ` Michael Walle
0 siblings, 0 replies; 10+ messages in thread
From: Michael Walle @ 2020-03-20 11:03 UTC (permalink / raw)
To: David Miller
Cc: linux-arm-kernel, devicetree, linux-kernel, netdev, f.fainelli,
vivien.didelot, andrew, claudiu.manoil, vladimir.oltean, robh+dt,
leoyang.li, shawnguo
Hi David, Hi Shawnguo,
Am 2020-03-15 04:53, schrieb David Miller:
> This series depends upon some devicetree tree changes, so why don't you
> submit these changes there and add my:
>
> Acked-by: David S. Miller <davem@davemloft.net>
>
> Thank you.
Patch 2/2 is already in linux-next, picked by Shawnguo. Who will
pick 1/2? I guess it doesn't matter through which tree it will go.
-michael
^ permalink raw reply [flat|nested] 10+ messages in thread