Re: [PATCH 1/8] ARM: rockchip: Add basic RK3288 support

[Sascha Hauer <s.hauer@pengutronix.de>]

On Wed, Jul 20, 2016 at 10:52:52AM -0700, Andrey Smirnov wrote:
> On Wed, Jul 20, 2016 at 7:17 AM, Wadim Egorov <w.egorov@phytec.de> wrote:

> > +
> > +struct rk3288_cru {
> > +       struct rk3288_pll {
> > +               u32 con0;
> > +               u32 con1;
> > +               u32 con2;
> > +               u32 con3;
> > +       } pll[5];
> > +       u32 cru_mode_con;
> > +       u32 reserved0[3];
> > +       u32 cru_clksel_con[43];
> > +       u32 reserved1[21];
> > +       u32 cru_clkgate_con[19];
> > +       u32 reserved2;
> > +       u32 cru_glb_srst_fst_value;
> > +       u32 cru_glb_srst_snd_value;
> > +       u32 cru_softrst_con[12];
> > +       u32 cru_misc_con;
> > +       u32 cru_glb_cnt_th;
> > +       u32 cru_glb_rst_con;
> > +       u32 reserved3;
> > +       u32 cru_glb_rst_st;
> > +       u32 reserved4;
> > +       u32 cru_sdmmc_con[2];
> > +       u32 cru_sdio0_con[2];
> > +       u32 cru_sdio1_con[2];
> > +       u32 cru_emmc_con[2];
> > +};
> 
> I, personally, am not a big fan of using structs to describe register
> file layout since it makes it hard to figure out individual register's
> offset just by reading the code, is compiler implementation/arch
> specific (that probably wouldn't be a problem though) and goes against
> the style in which majority of Linux kernel driver code(and by
> extension Barebox) is written in.

I do not like structs over registers also, for said reasons. Also they
become really messy when different register sizes and strides get
involved, these are typically solved with ifdeffery which limits the
resulting binary to a single board.

Anyway, for some reason structs over registers are policy in U-Boot
and when sharing code like done in this clock driver it just makes it
easier to a) share it in the first place and b) keep it in sync with
U-Boot. So, I am fine with structs over registers in code copied from
U-Boot, but I won't accept original code using it.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox