Re: [U-Boot] [PATCH 2/3] ARM1176: TI: TNETV107X soc initial support

Dear Cyril Chemparathy,

In message 1269893792-15248-3-git-send-email-cyril@ti.com you wrote:

TNETV107X is a Texas Instruments SoC based on an ARM1176 core, and with a bunch on on-chip integrated peripherals. This is an initial commit with basic functionality, more commits with drivers, etc. to follow.

Signed-off-by: Cyril Chemparathy cyril@ti.com

...

+void configure_async_emif(int cs, struct async_emif_config *cfg) +{

...

• if (cfg->select_strobe != -1) {

• tmp &= ~CONFIG_SELECT_STROBE(MASK);
  

• tmp |= CONFIG_SELECT_STROBE(cfg->select_strobe);
  

• }
• if (cfg->extend_wait != -1) {

• tmp &= ~CONFIG_EXTEND_WAIT(MASK);
  

• tmp |= CONFIG_EXTEND_WAIT(cfg->extend_wait);
  

• }
• if (cfg->wr_setup != -1) {

• tmp &= ~CONFIG_WR_SETUP(MASK);
  

• tmp |= CONFIG_WR_SETUP(cfg->wr_setup);
  

• }
• if (cfg->wr_strobe != -1) {

• tmp &= ~CONFIG_WR_STROBE(MASK);
  

• tmp |= CONFIG_WR_STROBE(cfg->wr_strobe);
  

• }
• if (cfg->wr_hold != -1) {

• tmp &= ~CONFIG_WR_HOLD(MASK);
  

• tmp |= CONFIG_WR_HOLD(cfg->wr_hold);
  

• }
• if (cfg->rd_setup != -1) {

• tmp &= ~CONFIG_RD_SETUP(MASK);
  

• tmp |= CONFIG_RD_SETUP(cfg->rd_setup);
  

• }
• if (cfg->rd_strobe != -1) {

• tmp &= ~CONFIG_RD_STROBE(MASK);
  

• tmp |= CONFIG_RD_STROBE(cfg->rd_strobe);
  

• }
• if (cfg->rd_hold != -1) {

• tmp &= ~CONFIG_RD_HOLD(MASK);
  

• tmp |= CONFIG_RD_HOLD(cfg->rd_hold);
  

• }
• if (cfg->turn_around != -1) {

• tmp &= ~CONFIG_TURN_AROUND(MASK);
  

• tmp |= CONFIG_TURN_AROUND(cfg->turn_around);
  

• }
• if (cfg->width != -1) {

• tmp &= ~CONFIG_WIDTH(MASK);
  

• tmp |= CONFIG_WIDTH(cfg->width);
  

• }

You might want to define a macro to reduce the amount of repeated code here.

+void lpsc_control(unsigned int id, int state) +{

• __lpsc_control(1, -1, id, state);

+}

+int lpsc_status(unsigned int id) +{

• return psc_reg_read(PSC_MDSTAT(id)) & 0x1f;

+}

+void clk_enable(unsigned int id) +{

• lpsc_control(id, PSC_MDCTL_NEXT_ENABLE);

+}

+void clk_disable(unsigned int id) +{

• lpsc_control(id, PSC_MDCTL_NEXT_DISABLE);

+}

These should probably be inlined ?

Best regards,

Wolfgang Denk