[PATCHv2 1/2] ARM: socfpga: Enable soft reset

[dinguyen@altera.com (Dinh Nguyen)]

Hi Pavel,

On Wed, 2013-04-03 at 14:12 -0700, Olof Johansson wrote:
> On Wed, Apr 3, 2013 at 1:32 PM, Pavel Machek <pavel@denx.de> wrote:
> > Hi!
> >
> >> > +struct socfpga_rstmgr_hw {
> >> > +   u32 unk;
> >> > +   u32 ctrl;               /* 0x04 */
> >> > +   u32 unk2, unk3;
> >> > +/* MPU Module Reset Register */
> >> > +#define RSTMGR_MPUMODRST_CPU0      0x1     /* CPU0 Reset */
> >> > +#define RSTMGR_MPUMODRST_CPU1      0x2     /* CPU1 Reset */
> >> > +#define RSTMGR_MPUMODRST_WDS       0x4     /* Watchdog Reset */
> >> > +#define RSTMGR_MPUMODRST_SCUPER    0x8     /* SCU and periphs reset */
> >> > +#define RSTMGR_MPUMODRST_L2        0x10    /* L2 Cache reset */
> >> > +   u32 mpumodrst;          /* 0x10 */
> >> > +   u32 modperrst;          /* 0x14 */
> >> > +   u32 unk5;
> >> > +   u32 bgrmodrst;          /* 0x1c */
> >> > +};
> >>
> >> As Russell already replied, struct used to represent register layout is
> >> normally frowned upon in drivers.
> >
> > Well... as Russell also said, this is arch-specific code, so it is
> > actually ok.
> 
> Which is why I said "normally frowned upon" instead of "always frowned upon".
> 
> >> If you want to tighten up the code in this case, make a local helper
> >> function that just takes the register offset instead, i.e.:
> >>
> >> > -   temp = __raw_readl(rst_manager_base_addr + SOCFPGA_RSTMGR_CTRL);
> >> > +   temp = readl(&rst_manager_base_addr->ctrl);
> >>
> >>       temp = rst_readl(SOCFPGA_RSTMGR_CTRL);
> >>
> >> (and then have rst_readl/rst_writel do the math based on base
> >> address).
> >
> > That does not prevent mistake such as
> > rst_readl(SOCFPGA_SYSMGR_CTRL)... and with number of different
> > subsystems on socfpga (each with separate base address), I fear that
> > might be an issue.
> >
> > Unfortunately, C does not check type of enums, so they can't be used
> > to solve that.
> 
> That's no different from accidentally doing
> readl(&sys_manager_base_addr->ctrl) instead of
> rst_manager_base_addr->ctrl.
> 

I also don't really like to use structs for register offsets. So unless,
there's some standard to use them in machine code, I'm not going to use
them in mach-socfpga.

I apologize for not getting to a v3 for this patch series with Mike's
recommended change for using composite clocks as quickly as I would have
like. I hope to have a v3 by next week.

Dinh
> 
> -Olof
>