[PATCH 4/4] arm: rmobile: Add HopeRun HiHope RZ/G2M board support

[Biju Das]

Hi Marek,

Thanks for the feedback.

> Subject: Re: [PATCH 4/4] arm: rmobile: Add HopeRun HiHope RZ/G2M board
> support
>
> On 9/15/20 4:36 PM, Biju Das wrote:
> [...]
> > diff --git a/arch/arm/mach-rmobile/Kconfig.64
> > b/arch/arm/mach-rmobile/Kconfig.64
> > index 07f607dd9d..2290be725f 100644
> > --- a/arch/arm/mach-rmobile/Kconfig.64
> > +++ b/arch/arm/mach-rmobile/Kconfig.64
> > @@ -4,6 +4,8 @@ menu "Select Target SoC"
> >
> >  config R8A774A1
> >          bool "Renesas SoC R8A774A1"
> > +imply CLK_R8A774A1
> > +imply PINCTRL_PFC_R8A774A1
>
> Can you please fix the indent with the "bool" too ? It seems there is some
> inconsistency.

OK, will fix this in V2.

>
> [...]
>
> > +++ b/board/hoperun/hihope-rzg2/hihope-rzg2.c
> [...]
> > +dm_gpio_set_dir_flags(&rev_bit1, GPIOD_IS_IN);
> > +dm_gpio_set_dir_flags(&rev_bit0, GPIOD_IS_IN);
> > +revision = 0x03 + ((dm_gpio_get_value(&rev_bit1) << 1) |
> > +    dm_gpio_get_value(&rev_bit0));
>
> I think dm_gpio_get_value can return error too, so applying bit ops might fail.

I am dropping this function since there is no user for this function, after setting WLAN/BT REGON using gpio hog

> > +return revision;
> > +}
>
> [...]
>
> > +void clear_wlan_bt_reg_on(void)
> > +{
> > +struct gpio_desc bt_power, wifi_power;
> > +char *bt_power_gpio = "gpio at e605300013"; /* GP3_13 */
> > +char *wifi_power_gpio = "gpio at e60540006"; /* GP4_06 */
>
> Can you use DT gpio-hog to set the GPIOs instead ?
Will send V2 for this.

> [...]
>
> > +/* Kconfig forces this on, so just return 0 */ int
> > +board_early_init_f(void) {
>
> Likely should be fixed on Kconfig side.

Ok. On V2 will add #ifdef around this function

> > +return 0;
> > +}
> > +
> > +int board_init(void)
> > +{
> > +/* address of boot parameters */
> > +gd->bd->bi_boot_params = CONFIG_SYS_TEXT_BASE + 0x50000;
> > +
> > +/* USB1 pull-up */
> > +setbits_le32(PFC_PUEN6, PUEN_USB1_OVC | PUEN_USB1_PWEN);
>
> Is this USB stuff really needed on this board ?

Will drop this on v2. Without this USB2.0 host works.

> [...]
>
> > +int dram_init(void)
> > +{
> > +return fdtdec_setup_mem_size_base(); }
>
> Isn't this already in rcar-common.c ?

rcar-common.c  is board/renesas directory and this is in board/hoperun. So I believe we should not cross reference files in
board specific directory. Please correct me if I am wrong.

> > +int dram_init_banksize(void)
> > +{
> > +return fdtdec_setup_memory_banksize(); }
> > +
> > +void reset_cpu(ulong addr)
> > +{
> > +writel(RST_CODE, RST_CA57RESCNT);
> > +}
>
> [...]
> I'll apply the PFC patches to sh/next , so feel free to resend only a subset of
> the patches based on sh/next .


Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647