Re: [U-Boot] [PATCH 1/1] sunxi: R40: add support for gigabit ethernet

[Maxime Ripard]

Hi Lothar,

On Thu, Apr 12, 2018 at 08:19:03PM +0200, Lothar Felten wrote:
> This patch adds support for the gigabit interface found on the
> Allwinner R40/V40 CPU.
> It does not support the 100Mbit interface (EMAC).
> 
> Fixes:
> - add GMAC gating clock and reset control
> - GPIO mux settings
> - device tree entries
> 
> Tested on BananaPi M2 Berry (R40), custom board (V40).
> 
> Signed-off-by: Lothar Felten 
> ---
>  arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts  | 14 ++
>  arch/arm/dts/sun8i-r40.dtsi   | 37 
> +++
>  arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 10 +++-
>  board/sunxi/gmac.c|  5 +++-
>  configs/Bananapi_M2_Ultra_defconfig   |  4 +++
>  drivers/net/sun8i_emac.c  |  2 ++
>  6 files changed, 70 insertions(+), 2 deletions(-)

This patch looks mostly correct, but it should be split into multiple,
smaller patches, one per logical change. This would mean something like:
  - One to add the clock / reset changes
  - One to add the pinctrl change
  - One to add the DTSI bits
  - And one to enable it on the BPI M2-U (with the config and DTS changes).

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 1/1] sunxi: R40: add support for gigabit ethernet

[Joe Hershberger]

On Thu, Apr 12, 2018 at 1:19 PM, Lothar Felten  wrote:
> This patch adds support for the gigabit interface found on the
> Allwinner R40/V40 CPU.
> It does not support the 100Mbit interface (EMAC).
>
> Fixes:
> - add GMAC gating clock and reset control
> - GPIO mux settings
> - device tree entries
>
> Tested on BananaPi M2 Berry (R40), custom board (V40).
>
> Signed-off-by: Lothar Felten 
> ---
>  arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts  | 14 ++
>  arch/arm/dts/sun8i-r40.dtsi   | 37 
> +++
>  arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 10 +++-
>  board/sunxi/gmac.c|  5 +++-
>  configs/Bananapi_M2_Ultra_defconfig   |  4 +++
>  drivers/net/sun8i_emac.c  |  2 ++
>  6 files changed, 70 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts 
> b/arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts
> index ab471ab0bf..25f2112fbf 100644
> --- a/arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts
> +++ b/arch/arm/dts/sun8i-r40-bananapi-m2-ultra.dts
> @@ -67,3 +67,17 @@
> pinctrl-0 = <_pb_pins>;
> status = "okay";
>  };
> +
> + {
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_pins_rgmii>;
> +   status = "okay";
> +   phy-handle = <_phy>;
> +};
> +
> + {
> +   rgmii_phy: ethernet-phy@1 {
> +   compatible = "ethernet-phy-ieee802.3-c22";
> +   reg = <1>;
> +   };
> +};
> diff --git a/arch/arm/dts/sun8i-r40.dtsi b/arch/arm/dts/sun8i-r40.dtsi
> index 48ec2e855a..93bf811014 100644
> --- a/arch/arm/dts/sun8i-r40.dtsi
> +++ b/arch/arm/dts/sun8i-r40.dtsi
> @@ -114,6 +114,12 @@
> #size-cells = <1>;
> ranges;
>
> +   syscon: syscon@1c0 {
> +   compatible = "allwinner,sun8i-r40-system-controller",
> +   "syscon";
> +   reg = <0x01c0 0x1000>;
> +   };
> +
> pio: pinctrl@1c20800 {
> compatible = "allwinner,sun8i-r40-pinctrl";
> reg = <0x01c20800 0x400>;
> @@ -126,6 +132,15 @@
> #interrupt-cells = <3>;
> #gpio-cells = <3>;
>
> +   gmac_pins_rgmii: gmac_rgmii {
> +   pins =  "PA0", "PA1", "PA2",
> +   "PA3", "PA4", "PA5", "PA6",
> +   "PA7", "PA8", "PA10",
> +   "PA11", "PA12", "PA13",
> +   "PA15", "PA16";
> +   function = "gmac";
> +   };
> +
> i2c0_pins: i2c0_pins {
> pins = "PB0", "PB1";
> function = "i2c0";
> @@ -159,6 +174,28 @@
> #size-cells = <0>;
> };
>
> +   gmac: ethernet@01c5 {
> +   compatible = "allwinner,sun8i-h3-emac";
> +   syscon = <>;
> +   reg = <0x01c5 0x2000>;
> +   interrupts = ;
> +   interrupt-names = "macirq";
> +   clocks = <>, <>;
> +   clock-names = "stmmaceth", "allwinner_gmac_tx";
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_pins_rgmii>;
> +   phy-mode = "rgmii";
> +   status = "disabled";
> +
> +   mdio: mdio {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   compatible = "snps,dwmac-mdio";
> +   };
> +   };
> +
> gic: interrupt-controller@1c81000 {
> compatible = "arm,cortex-a7-gic", 
> "arm,cortex-a15-gic";
> reg = <0x01c81000 0x1000>,
> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h 
> b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> index d35aa479f7..5af8c64fa2 100644
> --- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> @@ -61,7 +61,11 @@ struct sunxi_ccm_reg {
> u32 reserved11;
> u32 sata_clk_cfg;   /* 0xc8 SATA clock control (R40 only) */
> u32 usb_clk_cfg;/* 0xcc USB clock control */
> -   u32 gmac_clk_cfg;   /* 0xd0 GMAC clock control */
> +#ifndef CONFIG_MACH_SUN8I_R40

You should use positive logic for this type of thing (where you have
both cases).

> +   u32 gmac_clk_cfg;   /* 0xd0 GMAC clock control (not for R40) */
> +#else
> +   u32 cir0_clk_cfg;   /* 0xd0 CIR0