[U-Boot] [PATCH v3 6/9] net: stm32: add designware mac glue code for stm32

[Vikas MANOCHA]

Hi Michael,

> -----Original Message-----
> From: Michael Kurz [mailto:michi.kurz at gmail.com]
> Sent: Monday, December 05, 2016 9:37 AM
> To: Vikas MANOCHA <vikas.manocha at st.com>
> Cc: Michael Kurz <michi.kurz at gmail.com>; u-boot at lists.denx.de; Simon Glass <sjg at chromium.org>; Albert Aribaud
> <albert.u.boot at aribaud.net>; Toshifumi NISHINAGA <tnishinaga.dev at gmail.com>; Joe Hershberger <joe.hershberger at ni.com>
> Subject: Re: [PATCH v3 6/9] net: stm32: add designware mac glue code for stm32
> 
> 
> Hi Vikas,
> 
> On Thu, 1 Dec 2016, vikas wrote:
> 
> > Hi,
> >
> > On 11/24/2016 11:10 AM, Michael Kurz wrote:
> >> This patch adds glue code required for enabling the designware mac on
> >> stm32f7 devices.
> >
> > ethernet is not working at my end, is it working at your end.
> >
> 
> Just tested it again with a clean clone of u-boot master with my patchset and stm32f746-disco_defconfig. It works for me: dhcp, ping
> and dns to local and internet addresses.

Sounds good, thanks for testing it again.

> 
> >>
> >> Signed-off-by: Michael Kurz <michi.kurz at gmail.com>
> >> Acked-by: Joe Hershberger <joe.hershberger at ni.com>
> >>
> >> ---
> >>
> >> Changes in v3:
> >> - Add Acked-by tag to 'add designware mac glue code for stm32'
> >>
> >> Changes in v2:
> >> - Replaced bit shifts and masks with BIT() and GENMASK() macro
> >> - Moved STM32_SYSCFG_BASE into stm32.h header
> >>
> >>  arch/arm/include/asm/arch-stm32f7/stm32_periph.h |  1 +
> >>  arch/arm/include/asm/arch-stm32f7/syscfg.h       | 38 ++++++++++++++++
> >>  arch/arm/mach-stm32/stm32f7/clock.c              |  5 +++
> >>  board/st/stm32f746-disco/stm32f746-disco.c       | 56 ++++++++++++++++++++++++
> >>  configs/stm32f746-disco_defconfig                | 15 ++++++-
> >>  drivers/net/designware.c                         |  1 +
> >>  include/configs/stm32f746-disco.h                |  9 +++-
> >>  7 files changed, 122 insertions(+), 3 deletions(-)  create mode
> >> 100644 arch/arm/include/asm/arch-stm32f7/syscfg.h
> >>
> >
> > [...]
> >
> >> +CONFIG_NETCONSOLE=y
> >> +CONFIG_DM_ETH=y
> >> +CONFIG_ETH_DESIGNWARE=y
> >> +# CONFIG_SPL_SERIAL_PRESENT is not set CONFIG_OF_LIBFDT_OVERLAY=y
> >>  # CONFIG_EFI_LOADER is not set
> >> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> >> index 9e6d726..883ca5a 100644
> >> --- a/drivers/net/designware.c
> >> +++ b/drivers/net/designware.c
> >> @@ -738,6 +738,7 @@ static const struct udevice_id designware_eth_ids[] = {
> >>         { .compatible = "allwinner,sun7i-a20-gmac" },
> >>         { .compatible = "altr,socfpga-stmmac" },
> >>         { .compatible = "amlogic,meson6-dwmac" },
> >> +       { .compatible = "st,stm32-dwmac" },
> >
> > adding compatible for the soc in the peripheral driver ?
> > why not to use the existing one in the stm32 dts file. I see others (allwinner, altr ) have also done the same which is unnecesary &
> not as per binding.
> > please replace compatible string with one mentioned in binding doc(something like "snps, dwmac") in the driver as well the dts of
> stm32, allwinner, altr etc.
> >
> >
> > Cheers,
> > Vikas
> >
> 
> That makes sense. I followed the others. I'll change it to the common one in both dts and in the driver.

There might be more comments about the common compatibility string like 'snps, dwmac' from allwinner, altr or amlogic submitters,  let us keep this change separate from this patchset to make life easier.

I am ok with the "st, stm32-dwmac" for this patchset.

Cheers,
Vikas

> 
> Regards,
> Michael
> 
> >>         { }
> >>  };
> >>
> >> diff --git a/include/configs/stm32f746-disco.h
> >> b/include/configs/stm32f746-disco.h
> >> index 4391bff..4088064 100644
> >> --- a/include/configs/stm32f746-disco.h
> >> +++ b/include/configs/stm32f746-disco.h
> >> @@ -42,6 +42,11 @@
> >>  #define CONFIG_STM32_FLASH
> >>  #define CONFIG_STM32X7_SERIAL
> >>
> >> +#define CONFIG_DESIGNWARE_ETH
> >> +#define CONFIG_DW_GMAC_DEFAULT_DMA_PBL (8) #define
> >> +CONFIG_DW_ALTDESCRIPTOR #define CONFIG_MII
> >> +
> >>  #define CONFIG_STM32_HSE_HZ            25000000
> >>  #define CONFIG_SYS_CLK_FREQ            200000000 /* 200 MHz */
> >>  #define CONFIG_SYS_HZ_CLOCK            1000000 /* Timer is clocked at 1MHz */
> >> @@ -56,8 +61,8 @@
> >>                                         + sizeof(CONFIG_SYS_PROMPT) +
> >> 16)
> >>
> >>  #define CONFIG_SYS_MAXARGS             16
> >> -#define CONFIG_SYS_MALLOC_LEN          (16 * 1024)
> >> -#define CONFIG_STACKSIZE               (64 << 10)
> >> +#define CONFIG_SYS_MALLOC_LEN          (1 * 1024 * 1024)
> >> +#define CONFIG_STACKSIZE               (256 * 1024)
> >>
> >>  #define CONFIG_BAUDRATE                        115200
> >>  #define CONFIG_BOOTARGS                                                        \
> >> --
> >> 2.1.4
> >>
> >> .
> >>
> >