Hey
On Wednesday, 24 July 2019 14:26:44 CEST Kever Yang wrote:
> Hi Rohan,
> 
> On 2019/7/24 下午7:09, Rohan Garg wrote:
> > We should use the shared helpers to setup the necessary parts
> > 
> > Signed-off-by: Rohan Garg <rohan.g...@collabora.com>
> > ---
> > 
> >   .../puma_rk3399/puma-rk3399.c                 | 108 +++---------------
> >   1 file changed, 18 insertions(+), 90 deletions(-)
> > 
> > diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> > b/board/theobroma-systems/puma_rk3399/puma-rk3399.c index
> > 251cd2d566..fb6c7c1f0a 100644
> > --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> > +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> > @@ -17,6 +17,7 @@
> > 
> >   #include <asm/arch-rockchip/clock.h>
> >   #include <asm/arch-rockchip/hardware.h>
> >   #include <asm/arch-rockchip/grf_rk3399.h>
> > 
> > +#include <asm/arch-rockchip/misc.h>
> > 
> >   #include <asm/arch-rockchip/periph.h>
> >   #include <power/regulator.h>
> >   #include <u-boot/sha256.h>
> > 
> > @@ -36,94 +37,6 @@ int board_init(void)
> > 
> >     return 0;
> >   
> >   }
> > 
> > -static void setup_macaddr(void)
> > -{
> > -#if CONFIG_IS_ENABLED(CMD_NET)
> > -   int ret;
> > -   const char *cpuid = env_get("cpuid#");
> > -   u8 hash[SHA256_SUM_LEN];
> > -   int size = sizeof(hash);
> > -   u8 mac_addr[6];
> > -
> > -   /* Only generate a MAC address, if none is set in the environment 
*/
> > -   if (env_get("ethaddr"))
> > -           return;
> > -
> > -   if (!cpuid) {
> > -           debug("%s: could not retrieve 'cpuid#'\n", __func__);
> > -           return;
> > -   }
> > -
> > -   ret = hash_block("sha256", (void *)cpuid, strlen(cpuid), hash, 
&size);
> > -   if (ret) {
> > -           debug("%s: failed to calculate SHA256\n", __func__);
> > -           return;
> > -   }
> > -
> > -   /* Copy 6 bytes of the hash to base the MAC address on */
> > -   memcpy(mac_addr, hash, 6);
> > -
> > -   /* Make this a valid MAC address and set it */
> > -   mac_addr[0] &= 0xfe;  /* clear multicast bit */
> > -   mac_addr[0] |= 0x02;  /* set local assignment bit (IEEE802) */
> > -   eth_env_set_enetaddr("ethaddr", mac_addr);
> > -#endif
> > -}
> > -
> > -static void setup_serial(void)
> > -{
> > -#if CONFIG_IS_ENABLED(ROCKCHIP_EFUSE)
> > -   const u32 cpuid_offset = 0x7;
> > -   const u32 cpuid_length = 0x10;
> > -
> > -   struct udevice *dev;
> > -   int ret, i;
> > -   u8 cpuid[cpuid_length];
> > -   u8 low[cpuid_length/2], high[cpuid_length/2];
> > -   char cpuid_str[cpuid_length * 2 + 1];
> > -   u64 serialno;
> > -   char serialno_str[17];
> > -
> > -   /* retrieve the device */
> > -   ret = uclass_get_device_by_driver(UCLASS_MISC,
> > -                                     
DM_GET_DRIVER(rockchip_efuse), &dev);
> > -   if (ret) {
> > -           debug("%s: could not find efuse device\n", __func__);
> > -           return;
> > -   }
> > -
> > -   /* read the cpu_id range from the efuses */
> > -   ret = misc_read(dev, cpuid_offset, &cpuid, sizeof(cpuid));
> > -   if (ret) {
> > -           debug("%s: reading cpuid from the efuses failed\n",
> > -                 __func__);
> > -           return;
> > -   }
> > -
> > -   memset(cpuid_str, 0, sizeof(cpuid_str));
> > -   for (i = 0; i < 16; i++)
> > -           sprintf(&cpuid_str[i * 2], "%02x", cpuid[i]);
> > -
> > -   debug("cpuid: %s\n", cpuid_str);
> > -
> > -   /*
> > -    * Mix the cpuid bytes using the same rules as in
> > -    *   ${linux}/drivers/soc/rockchip/rockchip-cpuinfo.c
> > -    */
> > -   for (i = 0; i < 8; i++) {
> > -           low[i] = cpuid[1 + (i << 1)];
> > -           high[i] = cpuid[i << 1];
> > -   }
> > -
> > -   serialno = crc32_no_comp(0, low, 8);
> > -   serialno |= (u64)crc32_no_comp(serialno, high, 8) << 32;
> > -   snprintf(serialno_str, sizeof(serialno_str), "%016llx", serialno);
> > -
> > -   env_set("cpuid#", cpuid_str);
> > -   env_set("serial#", serialno_str);
> > -#endif
> > -}
> > -
> > 
> >   static void setup_iodomain(void)
> >   {
> >   
> >     const u32 GRF_IO_VSEL_GPIO4CD_SHIFT = 3;
> > 
> > @@ -213,8 +126,23 @@ static int setup_boottargets(void)
> > 
> >   int misc_init_r(void)
> 
> After misc_init_r has add into rk3399_board, then this misc_init_r()
> here will be multi-defined?
> 
> In this case, maybe we need to remove the misc_init_r() from
> rk3399_board.c first? :(
> 

I'm a bit confused. I thought the idea was to hoist this code up into rk3399-
board.c?

On a separate note, perhaps setup_iodomain should move into a more generic 
helper for the rk3399 boards.

I'm also not sure why setup_boottargets fiddles around with boot order, but 
perhaps we shouldn't be doing that? None of the other boards seem to do that ( 
or at least none that I could find anyway ).

Cheers
Rohan Garg

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to