Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1434903 > unrolled thread

[PATCH 0/7] net: ethernet: bgmac: Add platform device support

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] net: ethernet: bgmac: Add platform device support Jon Mason <jon.mason@broadcom.com> - 2016-07-01 01:10 +0200
    [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac Jon Mason <jon.mason@broadcom.com> - 2016-07-01 01:10 +0200
      Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation  for bgmac Rob Herring <robh@kernel.org> - 2016-07-01 05:00 +0200
        Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation  for bgmac Jon Mason <jon.mason@broadcom.com> - 2016-07-01 16:30 +0200
      Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac Arnd Bergmann <arnd@arndb.de> - 2016-07-01 11:50 +0200
        Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation  for bgmac Jon Mason <jon.mason@broadcom.com> - 2016-07-01 17:20 +0200
          Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac Arnd Bergmann <arnd@arndb.de> - 2016-07-01 17:50 +0200
            Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation  for bgmac Ray Jui <ray.jui@broadcom.com> - 2016-07-04 18:40 +0200
              Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac Arnd Bergmann <arnd@arndb.de> - 2016-07-05 15:40 +0200
                Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation  for bgmac Jon Mason <jon.mason@broadcom.com> - 2016-07-06 01:20 +0200
                  Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac Arnd Bergmann <arnd@arndb.de> - 2016-07-06 09:40 +0200
                    Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation  for bgmac Jon Mason <jon.mason@broadcom.com> - 2016-07-08 00:50 +0200
    Re: [PATCH 5/7] net: ethernet: bgmac: Add platform device support Florian Fainelli <f.fainelli@gmail.com> - 2016-07-01 01:30 +0200
    Re: [PATCH 0/7] net: ethernet: bgmac: Add platform device support Arnd Bergmann <arnd@arndb.de> - 2016-07-01 12:10 +0200

#1434903 — [PATCH 0/7] net: ethernet: bgmac: Add platform device support

Well, no compilained too loudly at the RFC version of this patch series
(see https://lkml.org/lkml/2016/6/28/863).  So, I'm officially sending
this out for inclusion.  All comments from the RFC were addressed in
this version.

This patch series adds support for other, non-bcma iProc SoC's to the
bgmac driver.  This series only adds NSP support, but we are interested
in adding support for the Cygnus and NS2 families (with more possible
down the road).

To support non-bcma enabled SoCs, we need to add the standard device
tree "platform device" support.  Unfortunately, this driver is very
tighly coupled with the bcma bus and much unwinding is needed.  I tried
to break this up into a number of patches to make it more obvious what
was being done to add platform device support.  I was able to verify
that the bcma code still works using a 53012K board (NS SoC), and that
the platform code works using a 58625K board (NSP SoC).

Thanks,
Jon

Jon Mason (7):
  net: ethernet: bgmac: change bgmac_* prints to dev_* prints
  net: ethernet: bgmac: add dma_dev pointer
  net: ethernet: bgmac: move BCMA MDIO Phy code into a separate file
  net: ethernet: bgmac: convert to feature flags
  net: ethernet: bgmac: Add platform device support
  dt-bindings: net: bgmac: add bindings documentation for bgmac
  ARM: dts: NSP: Add bgmac entries

 .../devicetree/bindings/net/brcm,bgmac-nsp.txt     |  24 +
 arch/arm/boot/dts/bcm-nsp.dtsi                     |  18 +
 arch/arm/boot/dts/bcm958625k.dts                   |   8 +
 drivers/net/ethernet/broadcom/Kconfig              |  23 +-
 drivers/net/ethernet/broadcom/Makefile             |   2 +
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c    | 266 +++++++++
 drivers/net/ethernet/broadcom/bgmac-bcma.c         | 315 ++++++++++
 drivers/net/ethernet/broadcom/bgmac-platform.c     | 210 +++++++
 drivers/net/ethernet/broadcom/bgmac.c              | 658 +++++----------------
 drivers/net/ethernet/broadcom/bgmac.h              | 112 +++-
 10 files changed, 1120 insertions(+), 516 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-bcma.c
 create mode 100644 drivers/net/ethernet/broadcom/bgmac-platform.c

-- 
1.9.1

[toc] | [next] | [standalone]

#1434904 — [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 .../devicetree/bindings/net/brcm,bgmac-nsp.txt     | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt

diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt b/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
new file mode 100644
index 0000000..022946c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
@@ -0,0 +1,24 @@
+Broadcom GMAC Ethernet Controller Device Tree Bindings
+-------------------------------------------------------------
+
+Required properties:
+ - compatible:	"brcm,bgmac-nsp"
+ - reg:		Address and length of the GMAC registers,
+		Address and length of the GMAC IDM registers
+ - reg-names:	Names of the registers.  Must have both "gmac_base" and
+		"idm_base"
+ - interrupts:	Interrupt number
+
+Optional properties:
+- mac-address:	See ethernet.txt file in the same directory
+
+Examples:
+
+gmac0: ethernet@18022000 {
+	compatible = "brcm,bgmac-nsp";
+	reg = <0x18022000 0x1000>,
+	      <0x18110000 0x1000>;
+	reg-names = "gmac_base", "idm_base";
+	interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
+	status = "disabled";
+};
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1434990 — Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

On Thu, Jun 30, 2016 at 06:59:13PM -0400, Jon Mason wrote:
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
>  .../devicetree/bindings/net/brcm,bgmac-nsp.txt     | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt b/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
> new file mode 100644
> index 0000000..022946c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
> @@ -0,0 +1,24 @@
> +Broadcom GMAC Ethernet Controller Device Tree Bindings
> +-------------------------------------------------------------
> +
> +Required properties:
> + - compatible:	"brcm,bgmac-nsp"

Usually we do <soc>-<block> order.

> + - reg:		Address and length of the GMAC registers,
> +		Address and length of the GMAC IDM registers
> + - reg-names:	Names of the registers.  Must have both "gmac_base" and
> +		"idm_base"
> + - interrupts:	Interrupt number
> +
> +Optional properties:
> +- mac-address:	See ethernet.txt file in the same directory
> +
> +Examples:
> +
> +gmac0: ethernet@18022000 {
> +	compatible = "brcm,bgmac-nsp";
> +	reg = <0x18022000 0x1000>,
> +	      <0x18110000 0x1000>;
> +	reg-names = "gmac_base", "idm_base";
> +	interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
> +	status = "disabled";
> +};
> -- 
> 1.9.1
> 

[toc] | [prev] | [next] | [standalone]

#1435375 — Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

On Thu, Jun 30, 2016 at 10:56 PM, Rob Herring <robh@kernel.org> wrote:
> On Thu, Jun 30, 2016 at 06:59:13PM -0400, Jon Mason wrote:
>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>> ---
>>  .../devicetree/bindings/net/brcm,bgmac-nsp.txt     | 24 ++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt b/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
>> new file mode 100644
>> index 0000000..022946c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/brcm,bgmac-nsp.txt
>> @@ -0,0 +1,24 @@
>> +Broadcom GMAC Ethernet Controller Device Tree Bindings
>> +-------------------------------------------------------------
>> +
>> +Required properties:
>> + - compatible:       "brcm,bgmac-nsp"
>
> Usually we do <soc>-<block> order.

Thanks, I'll make the necessary changes and push out a v2.

>
>> + - reg:              Address and length of the GMAC registers,
>> +             Address and length of the GMAC IDM registers
>> + - reg-names:        Names of the registers.  Must have both "gmac_base" and
>> +             "idm_base"
>> + - interrupts:       Interrupt number
>> +
>> +Optional properties:
>> +- mac-address:       See ethernet.txt file in the same directory
>> +
>> +Examples:
>> +
>> +gmac0: ethernet@18022000 {
>> +     compatible = "brcm,bgmac-nsp";
>> +     reg = <0x18022000 0x1000>,
>> +           <0x18110000 0x1000>;
>> +     reg-names = "gmac_base", "idm_base";
>> +     interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
>> +     status = "disabled";
>> +};
>> --
>> 1.9.1
>>

[toc] | [prev] | [next] | [standalone]

#1435211 — Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

On Thursday, June 30, 2016 6:59:13 PM CEST Jon Mason wrote:
> +
> +Required properties:
> + - compatible: "brcm,bgmac-nsp"
> + - reg:                Address and length of the GMAC registers,
> +               Address and length of the GMAC IDM registers
> + - reg-names:  Names of the registers.  Must have both "gmac_base" and
> +               "idm_base"
> + - interrupts: Interrupt number
> +


"brcm,bgmac-nsp" sounds a bit too general. As I understand, this is a family
of SoCs that might not all have the exact same implementation of this
ethernet device, as we can see from the long lookup table in bgmac_probe().

Please document the specific product numbers here that are publically
known already. Having the driver match just on "brcm,bgmac-nsp" as a fallback
is fine, so you can document that one as required for all users.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1435414 — Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

On Fri, Jul 1, 2016 at 5:46 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday, June 30, 2016 6:59:13 PM CEST Jon Mason wrote:
>> +
>> +Required properties:
>> + - compatible: "brcm,bgmac-nsp"
>> + - reg:                Address and length of the GMAC registers,
>> +               Address and length of the GMAC IDM registers
>> + - reg-names:  Names of the registers.  Must have both "gmac_base" and
>> +               "idm_base"
>> + - interrupts: Interrupt number
>> +
>
>
> "brcm,bgmac-nsp" sounds a bit too general. As I understand, this is a family
> of SoCs that might not all have the exact same implementation of this
> ethernet device, as we can see from the long lookup table in bgmac_probe().

The Broadcom iProc family of SoCs contains:
Northstar
Northstar Plus
Cygnus
Northstar 2
a few SoCs that are under development
and a number of ethernet switches (which might never be officially supported)

Each one of these SoCs could have a different revision of the gmac IP
block, but they should be uniform within each SoC (though there might
be a A0/B0 change necessary).  The Northstar Plus product family has a
number of different implementations, but the SoC is unchanged.  So, I
think this might be too specific, when we really need a general compat
string.

Broadcom has a history of sharing IP blocks amongst the different
divisions.  So, this driver might be used on other SoC families (as it
apparently has been done in the past, based on the code you
reference).  I do not know of any way to know what legacy, non-iProc
chips have used this IP block.  I can make this "brcm,iproc-bgmac",
and add "brcm,iproc-nsp-bgmac" as an alternative compatible string in
this file (which I believe you are suggesting), but there might be
non-iProc SoCs that use this driver.  Is this acceptable?

Thanks,
Jon

> Please document the specific product numbers here that are publically
> known already. Having the driver match just on "brcm,bgmac-nsp" as a fallback
> is fine, so you can document that one as required for all users.
>
>         Arnd

[toc] | [prev] | [next] | [standalone]

#1435466 — Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

On Friday, July 1, 2016 11:17:25 AM CEST Jon Mason wrote:
> On Fri, Jul 1, 2016 at 5:46 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday, June 30, 2016 6:59:13 PM CEST Jon Mason wrote:
> >> +
> >> +Required properties:
> >> + - compatible: "brcm,bgmac-nsp"
> >> + - reg:                Address and length of the GMAC registers,
> >> +               Address and length of the GMAC IDM registers
> >> + - reg-names:  Names of the registers.  Must have both "gmac_base" and
> >> +               "idm_base"
> >> + - interrupts: Interrupt number
> >> +
> >
> >
> > "brcm,bgmac-nsp" sounds a bit too general. As I understand, this is a family
> > of SoCs that might not all have the exact same implementation of this
> > ethernet device, as we can see from the long lookup table in bgmac_probe().
> 
> The Broadcom iProc family of SoCs contains:
> Northstar
> Northstar Plus
> Cygnus
> Northstar 2
> a few SoCs that are under development
> and a number of ethernet switches (which might never be officially supported)
> 
> Each one of these SoCs could have a different revision of the gmac IP
> block, but they should be uniform within each SoC (though there might
> be a A0/B0 change necessary).  The Northstar Plus product family has a
> number of different implementations, but the SoC is unchanged.  So, I
> think this might be too specific, when we really need a general compat
> string.

Ok, thanks for the clarification, that sounds good enough.

> Broadcom has a history of sharing IP blocks amongst the different
> divisions.  So, this driver might be used on other SoC families (as it
> apparently has been done in the past, based on the code you
> reference).  I do not know of any way to know what legacy, non-iProc
> chips have used this IP block.  I can make this "brcm,iproc-bgmac",
> and add "brcm,iproc-nsp-bgmac" as an alternative compatible string in
> this file (which I believe you are suggesting), but there might be
> non-iProc SoCs that use this driver.  Is this acceptable?

If it is also used outside of iProc, then I see no need for the
extra compatible string, although it would not do any harm either.

Ideally we should name it whatever the name for this IP block is
inside of the company, with "nsp" as the designation for the variant
in Northstar Plus. A lot of Broadcom IP blocks themselves seem to have
some four-digit or five-digit number, maybe this one does too?

	Arnd

[toc] | [prev] | [next] | [standalone]

#1436392 — Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

On 7/1/2016 8:42 AM, Arnd Bergmann wrote:
> On Friday, July 1, 2016 11:17:25 AM CEST Jon Mason wrote:
>> On Fri, Jul 1, 2016 at 5:46 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Thursday, June 30, 2016 6:59:13 PM CEST Jon Mason wrote:
>>>> +
>>>> +Required properties:
>>>> + - compatible: "brcm,bgmac-nsp"
>>>> + - reg:                Address and length of the GMAC registers,
>>>> +               Address and length of the GMAC IDM registers
>>>> + - reg-names:  Names of the registers.  Must have both "gmac_base" and
>>>> +               "idm_base"
>>>> + - interrupts: Interrupt number
>>>> +
>>>
>>>
>>> "brcm,bgmac-nsp" sounds a bit too general. As I understand, this is a family
>>> of SoCs that might not all have the exact same implementation of this
>>> ethernet device, as we can see from the long lookup table in bgmac_probe().
>>
>> The Broadcom iProc family of SoCs contains:
>> Northstar
>> Northstar Plus
>> Cygnus
>> Northstar 2
>> a few SoCs that are under development
>> and a number of ethernet switches (which might never be officially supported)
>>
>> Each one of these SoCs could have a different revision of the gmac IP
>> block, but they should be uniform within each SoC (though there might
>> be a A0/B0 change necessary).  The Northstar Plus product family has a
>> number of different implementations, but the SoC is unchanged.  So, I
>> think this might be too specific, when we really need a general compat
>> string.
>
> Ok, thanks for the clarification, that sounds good enough.
>
>> Broadcom has a history of sharing IP blocks amongst the different
>> divisions.  So, this driver might be used on other SoC families (as it
>> apparently has been done in the past, based on the code you
>> reference).  I do not know of any way to know what legacy, non-iProc
>> chips have used this IP block.  I can make this "brcm,iproc-bgmac",
>> and add "brcm,iproc-nsp-bgmac" as an alternative compatible string in
>> this file (which I believe you are suggesting), but there might be
>> non-iProc SoCs that use this driver.  Is this acceptable?
>
> If it is also used outside of iProc, then I see no need for the
> extra compatible string, although it would not do any harm either.
>
> Ideally we should name it whatever the name for this IP block is
> inside of the company, with "nsp" as the designation for the variant
> in Northstar Plus. A lot of Broadcom IP blocks themselves seem to have
> some four-digit or five-digit number, maybe this one does too?
>
> 	Arnd
>

Note this IP block has an official IP controller name of "amac" from the 
ASIC team.

Thanks,

Ray

[toc] | [prev] | [next] | [standalone]

#1437029 — Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

On Monday, July 4, 2016 9:34:35 AM CEST Ray Jui wrote:
> On 7/1/2016 8:42 AM, Arnd Bergmann wrote:
> > On Friday, July 1, 2016 11:17:25 AM CEST Jon Mason wrote:
> >> On Fri, Jul 1, 2016 at 5:46 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >>> On Thursday, June 30, 2016 6:59:13 PM CEST Jon Mason wrote:
> >>>> +
> >>>> +Required properties:
> >>>> + - compatible: "brcm,bgmac-nsp"
> >>>> + - reg:                Address and length of the GMAC registers,
> >>>> +               Address and length of the GMAC IDM registers
> >>>> + - reg-names:  Names of the registers.  Must have both "gmac_base" and
> >>>> +               "idm_base"
> >>>> + - interrupts: Interrupt number
> >>>> +
> >>>
> >>>
> >>> "brcm,bgmac-nsp" sounds a bit too general. As I understand, this is a family
> >>> of SoCs that might not all have the exact same implementation of this
> >>> ethernet device, as we can see from the long lookup table in bgmac_probe().
> >>
> >> The Broadcom iProc family of SoCs contains:
> >> Northstar
> >> Northstar Plus
> >> Cygnus
> >> Northstar 2
> >> a few SoCs that are under development
> >> and a number of ethernet switches (which might never be officially supported)
> >>
> >> Each one of these SoCs could have a different revision of the gmac IP
> >> block, but they should be uniform within each SoC (though there might
> >> be a A0/B0 change necessary).  The Northstar Plus product family has a
> >> number of different implementations, but the SoC is unchanged.  So, I
> >> think this might be too specific, when we really need a general compat
> >> string.
> >
> > Ok, thanks for the clarification, that sounds good enough.
> >
> >> Broadcom has a history of sharing IP blocks amongst the different
> >> divisions.  So, this driver might be used on other SoC families (as it
> >> apparently has been done in the past, based on the code you
> >> reference).  I do not know of any way to know what legacy, non-iProc
> >> chips have used this IP block.  I can make this "brcm,iproc-bgmac",
> >> and add "brcm,iproc-nsp-bgmac" as an alternative compatible string in
> >> this file (which I believe you are suggesting), but there might be
> >> non-iProc SoCs that use this driver.  Is this acceptable?
> >
> > If it is also used outside of iProc, then I see no need for the
> > extra compatible string, although it would not do any harm either.
> >
> > Ideally we should name it whatever the name for this IP block is
> > inside of the company, with "nsp" as the designation for the variant
> > in Northstar Plus. A lot of Broadcom IP blocks themselves seem to have
> > some four-digit or five-digit number, maybe this one does too?
> >
> >       Arnd
> >
> 
> Note this IP block has an official IP controller name of "amac" from the 
> ASIC team.

Ok, then I'd suggest making the compatible string here

	compatible = "brcm,nsp-amac", "brcm,amac";

or even better if you have a version number associated with it, make that

	compatible = "brcm,nsp-amac", "brcm,amac-1.234", "brcm,amac";

replacing 1.234 with the actual version of course.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1437329 — Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

On Tue, Jul 5, 2016 at 9:37 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday, July 4, 2016 9:34:35 AM CEST Ray Jui wrote:
>> On 7/1/2016 8:42 AM, Arnd Bergmann wrote:
>> > On Friday, July 1, 2016 11:17:25 AM CEST Jon Mason wrote:
>> >> On Fri, Jul 1, 2016 at 5:46 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> >>> On Thursday, June 30, 2016 6:59:13 PM CEST Jon Mason wrote:
>> >>>> +
>> >>>> +Required properties:
>> >>>> + - compatible: "brcm,bgmac-nsp"
>> >>>> + - reg:                Address and length of the GMAC registers,
>> >>>> +               Address and length of the GMAC IDM registers
>> >>>> + - reg-names:  Names of the registers.  Must have both "gmac_base" and
>> >>>> +               "idm_base"
>> >>>> + - interrupts: Interrupt number
>> >>>> +
>> >>>
>> >>>
>> >>> "brcm,bgmac-nsp" sounds a bit too general. As I understand, this is a family
>> >>> of SoCs that might not all have the exact same implementation of this
>> >>> ethernet device, as we can see from the long lookup table in bgmac_probe().
>> >>
>> >> The Broadcom iProc family of SoCs contains:
>> >> Northstar
>> >> Northstar Plus
>> >> Cygnus
>> >> Northstar 2
>> >> a few SoCs that are under development
>> >> and a number of ethernet switches (which might never be officially supported)
>> >>
>> >> Each one of these SoCs could have a different revision of the gmac IP
>> >> block, but they should be uniform within each SoC (though there might
>> >> be a A0/B0 change necessary).  The Northstar Plus product family has a
>> >> number of different implementations, but the SoC is unchanged.  So, I
>> >> think this might be too specific, when we really need a general compat
>> >> string.
>> >
>> > Ok, thanks for the clarification, that sounds good enough.
>> >
>> >> Broadcom has a history of sharing IP blocks amongst the different
>> >> divisions.  So, this driver might be used on other SoC families (as it
>> >> apparently has been done in the past, based on the code you
>> >> reference).  I do not know of any way to know what legacy, non-iProc
>> >> chips have used this IP block.  I can make this "brcm,iproc-bgmac",
>> >> and add "brcm,iproc-nsp-bgmac" as an alternative compatible string in
>> >> this file (which I believe you are suggesting), but there might be
>> >> non-iProc SoCs that use this driver.  Is this acceptable?
>> >
>> > If it is also used outside of iProc, then I see no need for the
>> > extra compatible string, although it would not do any harm either.
>> >
>> > Ideally we should name it whatever the name for this IP block is
>> > inside of the company, with "nsp" as the designation for the variant
>> > in Northstar Plus. A lot of Broadcom IP blocks themselves seem to have
>> > some four-digit or five-digit number, maybe this one does too?
>> >
>> >       Arnd
>> >
>>
>> Note this IP block has an official IP controller name of "amac" from the
>> ASIC team.
>
> Ok, then I'd suggest making the compatible string here
>
>         compatible = "brcm,nsp-amac", "brcm,amac";

It is called GMAC in the NS and NSP documentation, but AMAC is fine
with me (as it is called this in the NS2 documentation).  I'll make
the necessary change and repush.

Thanks for all of the input.

> or even better if you have a version number associated with it, make that
>
>         compatible = "brcm,nsp-amac", "brcm,amac-1.234", "brcm,amac";
>
> replacing 1.234 with the actual version of course.
>
>         Arnd
>

[toc] | [prev] | [next] | [standalone]

#1437510 — Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

On Tuesday, July 5, 2016 7:18:45 PM CEST Jon Mason wrote:
> >
> > Ok, then I'd suggest making the compatible string here
> >
> >         compatible = "brcm,nsp-amac", "brcm,amac";
> 
> It is called GMAC in the NS and NSP documentation, but AMAC is fine
> with me (as it is called this in the NS2 documentation).  I'll make
> the necessary change and repush.

Ok, then we can use

	compatible = "brcm,nsp-gmac", "brcm,amac";

to be consistent with that documentation and have the generic name as the
fallback.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1438995 — Re: [PATCH 6/7] dt-bindings: net: bgmac: add bindings documentation for bgmac

On Wed, Jul 6, 2016 at 3:34 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday, July 5, 2016 7:18:45 PM CEST Jon Mason wrote:
>> >
>> > Ok, then I'd suggest making the compatible string here
>> >
>> >         compatible = "brcm,nsp-amac", "brcm,amac";
>>
>> It is called GMAC in the NS and NSP documentation, but AMAC is fine
>> with me (as it is called this in the NS2 documentation).  I'll make
>> the necessary change and repush.
>
> Ok, then we can use
>
>         compatible = "brcm,nsp-gmac", "brcm,amac";
>
> to be consistent with that documentation and have the generic name as the
> fallback.

After looking at the docs again, I see the blocks being referred to as
"AXI MAC" (while the registers are being called GMAC).  To keep it
consistent, I'll just change everything to be "amac".

Thanks,
Jon

>
>         Arnd

[toc] | [prev] | [next] | [standalone]

#1434905 — Re: [PATCH 5/7] net: ethernet: bgmac: Add platform device support

[snip]

+
> +	return 0;
> +
> +err2:
> +	devm_iounmap(&pdev->dev, bgmac->plat.idm_base);
> +err1:
> +	devm_iounmap(&pdev->dev, bgmac->plat.base);
> +err:
> +	devm_kfree(&pdev->dev, bgmac);


This is not needed actually, now that you use the device managed helper
functions.

> +
> +	return rc;
> +}
> +
> +static int bgmac_remove(struct platform_device *pdev)
> +{
> +	struct bgmac *bgmac = platform_get_drvdata(pdev);
> +
> +	bgmac_enet_remove(bgmac);
> +	devm_iounmap(&pdev->dev, bgmac->plat.idm_base);
> +	devm_iounmap(&pdev->dev, bgmac->plat.base);
> +	devm_kfree(&pdev->dev, bgmac);

Same here.
-- 
Florian

[toc] | [prev] | [next] | [standalone]

#1435230

On Thursday, June 30, 2016 6:59:07 PM CEST Jon Mason wrote:
> Well, no compilained too loudly at the RFC version of this patch series
> (see https://lkml.org/lkml/2016/6/28/863).  So, I'm officially sending
> this out for inclusion.  All comments from the RFC were addressed in
> this version.
> 
> This patch series adds support for other, non-bcma iProc SoC's to the
> bgmac driver.  This series only adds NSP support, but we are interested
> in adding support for the Cygnus and NS2 families (with more possible
> down the road).
> 
> To support non-bcma enabled SoCs, we need to add the standard device
> tree "platform device" support.  Unfortunately, this driver is very
> tighly coupled with the bcma bus and much unwinding is needed.  I tried
> to break this up into a number of patches to make it more obvious what
> was being done to add platform device support.  I was able to verify
> that the bcma code still works using a 53012K board (NS SoC), and that
> the platform code works using a 58625K board (NSP SoC).

Looks reasonable overall. I have one comment for the binding, but
we can handle them separately. I think the first five patches
can just get merged through net-next if there are no other concerns,
and feel free to add my

Acked-by: Arnd Bergmann <arnd@arndb.de>


For the last two patches, once we have worked out the right
compatible strings, please send them to Florian for merging through
the arm-soc next/dt branch.

	Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web