[PATCH 0/3] Fix backward compatibility of newer firmware with older kernel

[PATCH 0/3] Fix backward compatibility of newer firmware with older kernel

[Iyappan Subramanian]

Kernel 3.17 driver supports only RGMII ethernet interface.
Since the Tianocore DT contains same compatibility string for RGMII,
SGMII based 1G and XFI based 10G interfaces, crash happens when probe called
on SGMII based 1G and XFI based 10G interface.

This patch fixes the backward compatibility of the older driver with the
newer firmware by making the binding unique so that the older driver won't
recognize the non-supported interfaces.

---

Iyappan Subramanian (3):
  Documentation: dts: Update compatible field description for APM X-Gene
  dtb: change binding name to match with newer firmware DT
  drivers: net: xgene: fix new firmware backward compatibility with
    older driver

 Documentation/devicetree/bindings/net/apm-xgene-enet.txt | 3 ++-
 arch/arm64/boot/dts/apm/apm-storm.dtsi                   | 4 ++--
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c         | 4 ++++
 3 files changed, 8 insertions(+), 3 deletions(-)

-- 
1.9.1

[PATCH 1/3] Documentation: dts: Update compatible field description for APM X-Gene

[Iyappan Subramanian]

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
Tested-by: Mark Langsdorf <mlangsdo@redhat.com>
---
 Documentation/devicetree/bindings/net/apm-xgene-enet.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
index cfcc527..5355996 100644
--- a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
+++ b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
@@ -4,7 +4,8 @@ Ethernet nodes are defined to describe on-chip ethernet interfaces in
 APM X-Gene SoC.
 
 Required properties for all the ethernet interfaces:
-- compatible: Should be "apm,xgene-enet"
+- compatible: Should be "apm,xgene-enet", "apm,xgene1-sgenet" or
+  "apm,xgene1-xgenet"
 - reg: Address and length of the register set for the device. It contains the
   information of registers in the same order as described by reg-names
 - reg-names: Should contain the register set names
-- 
1.9.1

Re: [PATCH 1/3] Documentation: dts: Update compatible field description for APM X-Gene

[Mark Rutland]

On Fri, Feb 27, 2015 at 08:08:28PM +0000, Iyappan Subramanian wrote:
> Signed-off-by: Iyappan Subramanian <isubramanian-qTEPVZfXA3Y@public.gmane.org>
> Signed-off-by: Keyur Chudgar <kchudgar-qTEPVZfXA3Y@public.gmane.org>
> Tested-by: Mark Langsdorf <mlangsdo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/net/apm-xgene-enet.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
> index cfcc527..5355996 100644
> --- a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
> +++ b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
> @@ -4,7 +4,8 @@ Ethernet nodes are defined to describe on-chip ethernet interfaces in
>  APM X-Gene SoC.
>  
>  Required properties for all the ethernet interfaces:
> -- compatible: Should be "apm,xgene-enet"
> +- compatible: Should be "apm,xgene-enet", "apm,xgene1-sgenet" or
> +  "apm,xgene1-xgenet"

Please format this as a list. It'll be easier to read, and you can add
more later.

What are the diffences between these variants?

Mark.

>  - reg: Address and length of the register set for the device. It contains the
>    information of registers in the same order as described by reg-names
>  - reg-names: Should contain the register set names
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] Documentation: dts: Update compatible field description for APM X-Gene

[Iyappan Subramanian]

On Fri, Feb 27, 2015 at 12:15 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Feb 27, 2015 at 08:08:28PM +0000, Iyappan Subramanian wrote:
>> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
>> Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
>> Tested-by: Mark Langsdorf <mlangsdo@redhat.com>
>> ---
>>  Documentation/devicetree/bindings/net/apm-xgene-enet.txt | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>> index cfcc527..5355996 100644
>> --- a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>> +++ b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>> @@ -4,7 +4,8 @@ Ethernet nodes are defined to describe on-chip ethernet interfaces in
>>  APM X-Gene SoC.
>>
>>  Required properties for all the ethernet interfaces:
>> -- compatible: Should be "apm,xgene-enet"
>> +- compatible: Should be "apm,xgene-enet", "apm,xgene1-sgenet" or
>> +  "apm,xgene1-xgenet"
>
> Please format this as a list. It'll be easier to read, and you can add
> more later.

Thanks.  I will do that.

>
> What are the diffences between these variants?

apm,xgene-enet - binding for RGMII based 1G interface
apm,xgene1-sgenet - binding for SGMII based 1G interface
apm,xgene1-xgenet - binding for XFI based 10G interface


>
> Mark.
>
>>  - reg: Address and length of the register set for the device. It contains the
>>    information of registers in the same order as described by reg-names
>>  - reg-names: Should contain the register set names
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

Re: [PATCH 1/3] Documentation: dts: Update compatible field description for APM X-Gene

[Mark Rutland]

On Fri, Feb 27, 2015 at 11:58:28PM +0000, Iyappan Subramanian wrote:
> On Fri, Feb 27, 2015 at 12:15 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Fri, Feb 27, 2015 at 08:08:28PM +0000, Iyappan Subramanian wrote:
> >> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
> >> Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
> >> Tested-by: Mark Langsdorf <mlangsdo@redhat.com>
> >> ---
> >>  Documentation/devicetree/bindings/net/apm-xgene-enet.txt | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
> >> index cfcc527..5355996 100644
> >> --- a/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
> >> +++ b/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
> >> @@ -4,7 +4,8 @@ Ethernet nodes are defined to describe on-chip ethernet interfaces in
> >>  APM X-Gene SoC.
> >>
> >>  Required properties for all the ethernet interfaces:
> >> -- compatible: Should be "apm,xgene-enet"
> >> +- compatible: Should be "apm,xgene-enet", "apm,xgene1-sgenet" or
> >> +  "apm,xgene1-xgenet"
> >
> > Please format this as a list. It'll be easier to read, and you can add
> > more later.
> 
> Thanks.  I will do that.
> 
> >
> > What are the diffences between these variants?
> 
> apm,xgene-enet - binding for RGMII based 1G interface
> apm,xgene1-sgenet - binding for SGMII based 1G interface
> apm,xgene1-xgenet - binding for XFI based 10G interface

I see. If you could keep notes to this effect in the list, that would
help to keep this obvious.

Thanks,
Mark.

[PATCH 2/3] dtb: change binding name to match with newer firmware DT

[Iyappan Subramanian]

This patch fixes the backward compatibility of the older driver with the
newer firmware by making the binding unique so that the older driver won't
recognize the non-supported interfaces.

The new bindings are in sync with the newer firmware.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
Tested-by: Mark Langsdorf <mlangsdo@redhat.com>
---
 arch/arm64/boot/dts/apm/apm-storm.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi b/arch/arm64/boot/dts/apm/apm-storm.dtsi
index f1ad9c2..a857794 100644
--- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
@@ -622,7 +622,7 @@
 		};
 
 		sgenet0: ethernet@1f210000 {
-			compatible = "apm,xgene-enet";
+			compatible = "apm,xgene1-sgenet";
 			status = "disabled";
 			reg = <0x0 0x1f210000 0x0 0xd100>,
 			      <0x0 0x1f200000 0x0 0Xc300>,
@@ -636,7 +636,7 @@
 		};
 
 		xgenet: ethernet@1f610000 {
-			compatible = "apm,xgene-enet";
+			compatible = "apm,xgene1-xgenet";
 			status = "disabled";
 			reg = <0x0 0x1f610000 0x0 0xd100>,
 			      <0x0 0x1f600000 0x0 0Xc300>,
-- 
1.9.1

Re: [PATCH 2/3] dtb: change binding name to match with newer firmware DT

[Arnd Bergmann]

On Friday 27 February 2015 12:08:29 Iyappan Subramanian wrote:
> diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi b/arch/arm64/boot/dts/apm/apm-storm.dtsi
> index f1ad9c2..a857794 100644
> --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
> +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
> @@ -622,7 +622,7 @@
>                 };
>  
>                 sgenet0: ethernet@1f210000 {
> -                       compatible = "apm,xgene-enet";
> +                       compatible = "apm,xgene1-sgenet";
>                         status = "disabled";
>                         reg = <0x0 0x1f210000 0x0 0xd100>,
>                               <0x0 0x1f200000 0x0 0Xc300>,
> 

This will break booting the new dtb on older kernels, right?

You should normally list the most generic model in addition to the
specific one.

	Arnd

Re: [PATCH 2/3] dtb: change binding name to match with newer firmware DT

[Iyappan Subramanian]

On Fri, Feb 27, 2015 at 12:33 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 27 February 2015 12:08:29 Iyappan Subramanian wrote:
>> diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi b/arch/arm64/boot/dts/apm/apm-storm.dtsi
>> index f1ad9c2..a857794 100644
>> --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
>> +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
>> @@ -622,7 +622,7 @@
>>                 };
>>
>>                 sgenet0: ethernet@1f210000 {
>> -                       compatible = "apm,xgene-enet";
>> +                       compatible = "apm,xgene1-sgenet";
>>                         status = "disabled";
>>                         reg = <0x0 0x1f210000 0x0 0xd100>,
>>                               <0x0 0x1f200000 0x0 0Xc300>,
>>
>
> This will break booting the new dtb on older kernels, right?

This will not break booting older kernels.  In fact, this will fix it,
by having the specific binding,
older kernel that does not have support for these interfaces, will not
call probe.

For example, 3.17 kernel that does not have support for SGMII based 1G
and XFI 10G interfaces
will not probe these interfaces when booted with newer dtb.

>
> You should normally list the most generic model in addition to the
> specific one.

If we list generic model ("xgene-enet") as well, probe will be called
for the interfaces that the kernel does not have support for,
which is what we are trying to fix.

>
>         Arnd

Re: [PATCH 2/3] dtb: change binding name to match with newer firmware DT

[Mark Rutland]

On Fri, Feb 27, 2015 at 11:52:54PM +0000, Iyappan Subramanian wrote:
> On Fri, Feb 27, 2015 at 12:33 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Friday 27 February 2015 12:08:29 Iyappan Subramanian wrote:
> >> diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi b/arch/arm64/boot/dts/apm/apm-storm.dtsi
> >> index f1ad9c2..a857794 100644
> >> --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
> >> +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
> >> @@ -622,7 +622,7 @@
> >>                 };
> >>
> >>                 sgenet0: ethernet@1f210000 {
> >> -                       compatible = "apm,xgene-enet";
> >> +                       compatible = "apm,xgene1-sgenet";
> >>                         status = "disabled";
> >>                         reg = <0x0 0x1f210000 0x0 0xd100>,
> >>                               <0x0 0x1f200000 0x0 0Xc300>,
> >>
> >
> > This will break booting the new dtb on older kernels, right?
> 
> This will not break booting older kernels.  In fact, this will fix it,
> by having the specific binding,
> older kernel that does not have support for these interfaces, will not
> call probe.
> 
> For example, 3.17 kernel that does not have support for SGMII based 1G
> and XFI 10G interfaces
> will not probe these interfaces when booted with newer dtb.

I'm guessing that in the existing DTBs, these nodes are always disabled?

If that's the case then we don't have a problem; we'll never probe these
nodes for an existing DTB.

Mark.

Re: [PATCH 2/3] dtb: change binding name to match with newer firmware DT

[Iyappan Subramanian]

On Mon, Mar 2, 2015 at 6:48 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Feb 27, 2015 at 11:52:54PM +0000, Iyappan Subramanian wrote:
>> On Fri, Feb 27, 2015 at 12:33 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Friday 27 February 2015 12:08:29 Iyappan Subramanian wrote:
>> >> diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi b/arch/arm64/boot/dts/apm/apm-storm.dtsi
>> >> index f1ad9c2..a857794 100644
>> >> --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
>> >> +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
>> >> @@ -622,7 +622,7 @@
>> >>                 };
>> >>
>> >>                 sgenet0: ethernet@1f210000 {
>> >> -                       compatible = "apm,xgene-enet";
>> >> +                       compatible = "apm,xgene1-sgenet";
>> >>                         status = "disabled";
>> >>                         reg = <0x0 0x1f210000 0x0 0xd100>,
>> >>                               <0x0 0x1f200000 0x0 0Xc300>,
>> >>
>> >
>> > This will break booting the new dtb on older kernels, right?
>>
>> This will not break booting older kernels.  In fact, this will fix it,
>> by having the specific binding,
>> older kernel that does not have support for these interfaces, will not
>> call probe.
>>
>> For example, 3.17 kernel that does not have support for SGMII based 1G
>> and XFI 10G interfaces
>> will not probe these interfaces when booted with newer dtb.
>
> I'm guessing that in the existing DTBs, these nodes are always disabled?

These nodes does not exist on 3.17 kernel dtb.
The problem was reported when booted using Tianocore with Device tree,
which has these nodes, but the 3.17 kernel does not have support for
these nodes.

>
> If that's the case then we don't have a problem; we'll never probe these
> nodes for an existing DTB.

With this patch, the following combinations were tested,
u-boot + 3.17 kernel + 3.17 dtb -> this patch will no effect on this
combination, only RGMII based 1G will be probed
u-boot + 3.19 kernel + 3.19 dtb + this patch -> all the 3 ethernet
interfaces was probed
Tianocore (w/ Device Tree) + 3.19 kernel + this patch -> all the 3
ethernet interfaces were probed
Tianocore (w/ ACPI) + 3.19 kernel + this patch -> all the 3 ethernet
interfaces were probed


>
> Mark.

[PATCH 3/3] drivers: net: xgene: fix new firmware backward compatibility with older driver

[Iyappan Subramanian]

This patch fixes the backward compatibile of the older driver with the
newer firmware by making the binding unique so that the older driver won't
recognize the non-supported interfaces.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
Tested-by: Mark Langsdorf <mlangsdo@redhat.com>
---
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
index 4de62b2..635a83b 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
@@ -1025,6 +1025,8 @@ static int xgene_enet_remove(struct platform_device *pdev)
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id xgene_enet_acpi_match[] = {
 	{ "APMC0D05", },
+	{ "APMC0D30", },
+	{ "APMC0D31", },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, xgene_enet_acpi_match);
@@ -1033,6 +1035,8 @@ MODULE_DEVICE_TABLE(acpi, xgene_enet_acpi_match);
 #ifdef CONFIG_OF
 static struct of_device_id xgene_enet_of_match[] = {
 	{.compatible = "apm,xgene-enet",},
+	{.compatible = "apm,xgene1-sgenet",},
+	{.compatible = "apm,xgene1-xgenet",},
 	{},
 };
 
-- 
1.9.1

Re: [PATCH 0/3] Fix backward compatibility of newer firmware with older kernel

[David Miller]

This subject line is lacking, it should mention the driver (using a
subsystem prefix following by a colon character and a space) for which
this series's changes are being made. when someone reads:

	Fix backward compatibility of newer firmware with older kernel

in the git shortlog, they will have no idea where you are actually
making these changes.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] Fix backward compatibility of newer firmware with older kernel

[Iyappan Subramanian]

On Fri, Feb 27, 2015 at 12:49 PM, David Miller <davem@davemloft.net> wrote:
>
> This subject line is lacking, it should mention the driver (using a
> subsystem prefix following by a colon character and a space) for which
> this series's changes are being made. when someone reads:
>
>         Fix backward compatibility of newer firmware with older kernel
>
> in the git shortlog, they will have no idea where you are actually
> making these changes.

Thanks.  I will fix it.