Re: [PATCH 4/6] ARM: OMAP: PRM: Remove hardcoding of IRQENABLE_MPU_2 and IRQSTATUS_MPU_2 register offsets

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Tuesday 07 July 2015 05:21 PM, Tero Kristo wrote:
On 06/22/2015 09:22 AM, Keerthy wrote:
The register offsets of IRQENABLE_MPU_2 and IRQSTATUS_MPU_2 are
hardcoded.
This makes it difficult to reuse the code for single core SoCs like
AM437x.

Single core vs. having two sets of IRQENABLE / IRQSTATUS registers do
not have any relation to each other. OMAP4+ has two IRQ registers,
because the number of IRQ events is so large it does not fit into single
register. Thus, the commit message is somewhat misleading, please fix.

Okay i will make it clearer.


Hence making it part of omap_prcm_irq_setup structure so that case of
single set of IRQ* registers can be handled generically.

Signed-off-by: Keerthy <j-keerthy@xxxxxx>
---
  arch/arm/mach-omap2/prcm-common.h |  8 ++++----
  arch/arm/mach-omap2/prm3xxx.c     | 20 +++++++++---------
  arch/arm/mach-omap2/prm44xx.c     | 43
+++++++++++++++++++++------------------
  arch/arm/mach-omap2/prm_common.c  |  5 ++---
  4 files changed, 39 insertions(+), 37 deletions(-)

diff --git a/arch/arm/mach-omap2/prcm-common.h
b/arch/arm/mach-omap2/prcm-common.h
index 2e60406..99447e7 100644
--- a/arch/arm/mach-omap2/prcm-common.h
+++ b/arch/arm/mach-omap2/prcm-common.h
@@ -470,8 +470,8 @@ struct omap_prcm_irq {

  /**
   * struct omap_prcm_irq_setup - PRCM interrupt controller details
- * @ack: PRM register offset for the first PRM_IRQSTATUS_MPU register
- * @mask: PRM register offset for the first PRM_IRQENABLE_MPU register
+ * @ack: PRM register offsets for the PRM_IRQSTATUS_MPU registers
+ * @mask: PRM register offsets for the PRM_IRQENABLE_MPU registers
   * @nr_regs: number of PRM_IRQ{STATUS,ENABLE}_MPU* registers
   * @nr_irqs: number of entries in the @irqs array
   * @irqs: ptr to an array of PRCM interrupt bits (see @nr_irqs)
@@ -492,8 +492,8 @@ struct omap_prcm_irq {
   * specified in static initializers.
   */
  struct omap_prcm_irq_setup {
-    u16 ack;
-    u16 mask;
+    u16 ack[2];
+    u16 mask[2];

You don't really need two pairs of offsets; in generic case, if we have
two IRQ registers, the offset between the two is 4, as used elsewhere in
the code. By keeping the previous struct layout, you can save a few
bytes of memory, and don't need to touch the omap3 code all over the
place either.

Okay


      u16 pm_ctrl;
      u8 nr_regs;
      u8 nr_irqs;
diff --git a/arch/arm/mach-omap2/prm3xxx.c
b/arch/arm/mach-omap2/prm3xxx.c
index 62680aa..56649b0 100644
--- a/arch/arm/mach-omap2/prm3xxx.c
+++ b/arch/arm/mach-omap2/prm3xxx.c

<snip>

No need to touch the OMAP3 code, as the offsets are always the default
ones.

diff --git a/arch/arm/mach-omap2/prm44xx.c
b/arch/arm/mach-omap2/prm44xx.c
index 8149e5a..20b547a 100644
--- a/arch/arm/mach-omap2/prm44xx.c
+++ b/arch/arm/mach-omap2/prm44xx.c
@@ -43,8 +43,10 @@ static const struct omap_prcm_irq omap4_prcm_irqs[]
= {
  };

  static struct omap_prcm_irq_setup omap4_prcm_irq_setup = {
-    .ack            = OMAP4_PRM_IRQSTATUS_MPU_OFFSET,
-    .mask            = OMAP4_PRM_IRQENABLE_MPU_OFFSET,
+    .ack[0]            = OMAP4_PRM_IRQSTATUS_MPU_OFFSET,
+    .mask[0]        = OMAP4_PRM_IRQENABLE_MPU_OFFSET,
+    .ack[1]            = OMAP4_PRM_IRQSTATUS_MPU_2_OFFSET,
+    .mask[1]        = OMAP4_PRM_IRQENABLE_MPU_2_OFFSET,

I think you should just keep the single IRQENABLE / IRQSTATUS
definitions in place, and use +4 addressing where applicable. Having an
array of the ack + mask definitions is kind of ugly.

Okay.


      .pm_ctrl        = OMAP4_PRM_IO_PMCTRL_OFFSET,
      .nr_regs        = 2,
      .irqs            = omap4_prcm_irqs,
@@ -217,11 +219,11 @@ static inline u32 _read_pending_irq_reg(u16
irqen_offs, u16 irqst_offs)
   */
  static void omap44xx_prm_read_pending_irqs(unsigned long *events)
  {
-    events[0] = _read_pending_irq_reg(OMAP4_PRM_IRQENABLE_MPU_OFFSET,
-                      OMAP4_PRM_IRQSTATUS_MPU_OFFSET);
+    int i;

-    events[1] = _read_pending_irq_reg(OMAP4_PRM_IRQENABLE_MPU_2_OFFSET,
-                      OMAP4_PRM_IRQSTATUS_MPU_2_OFFSET);
+    for (i = 0; i < omap4_prcm_irq_setup.nr_regs; i++)
+        events[i] = _read_pending_irq_reg(omap4_prcm_irq_setup.mask[i],
+                          omap4_prcm_irq_setup.ack[i]);

... change to mask + i * 4 / ack + i * 4.

got it.


  }

  /**
@@ -251,17 +253,16 @@ static void omap44xx_prm_ocp_barrier(void)
   */
  static void omap44xx_prm_save_and_clear_irqen(u32 *saved_mask)
  {
-    saved_mask[0] =
-        omap4_prm_read_inst_reg(OMAP4430_PRM_OCP_SOCKET_INST,
-                    OMAP4_PRM_IRQENABLE_MPU_OFFSET);
-    saved_mask[1] =
-        omap4_prm_read_inst_reg(OMAP4430_PRM_OCP_SOCKET_INST,
-                    OMAP4_PRM_IRQENABLE_MPU_2_OFFSET);
+    int i;

-    omap4_prm_write_inst_reg(0, OMAP4430_PRM_OCP_SOCKET_INST,
-                 OMAP4_PRM_IRQENABLE_MPU_OFFSET);
-    omap4_prm_write_inst_reg(0, OMAP4430_PRM_OCP_SOCKET_INST,
-                 OMAP4_PRM_IRQENABLE_MPU_2_OFFSET);
+    for (i = 0; i < omap4_prcm_irq_setup.nr_regs; i++) {
+        saved_mask[i] =
+            omap4_prm_read_inst_reg(OMAP4430_PRM_OCP_SOCKET_INST,
+                        omap4_prcm_irq_setup.mask[i]);
+
+        omap4_prm_write_inst_reg(0, OMAP4430_PRM_OCP_SOCKET_INST,
+                     omap4_prcm_irq_setup.mask[i]);

ditto.

+    }

      /* OCP barrier */
      omap4_prm_read_inst_reg(OMAP4430_PRM_OCP_SOCKET_INST,
@@ -280,10 +281,12 @@ static void
omap44xx_prm_save_and_clear_irqen(u32 *saved_mask)
   */
  static void omap44xx_prm_restore_irqen(u32 *saved_mask)
  {
-    omap4_prm_write_inst_reg(saved_mask[0],
OMAP4430_PRM_OCP_SOCKET_INST,
-                 OMAP4_PRM_IRQENABLE_MPU_OFFSET);
-    omap4_prm_write_inst_reg(saved_mask[1],
OMAP4430_PRM_OCP_SOCKET_INST,
-                 OMAP4_PRM_IRQENABLE_MPU_2_OFFSET);
+    int i;
+
+    for (i = 0; i < omap4_prcm_irq_setup.nr_regs; i++)
+        omap4_prm_write_inst_reg(saved_mask[i],
+                     OMAP4430_PRM_OCP_SOCKET_INST,
+                     omap4_prcm_irq_setup.mask[i]);

...and here.

  }

  /**
diff --git a/arch/arm/mach-omap2/prm_common.c
b/arch/arm/mach-omap2/prm_common.c
index 7add799..10ef0da 100644
--- a/arch/arm/mach-omap2/prm_common.c
+++ b/arch/arm/mach-omap2/prm_common.c
@@ -281,7 +281,6 @@ int omap_prcm_register_chain_handler(struct
omap_prcm_irq_setup *irq_setup)
          pr_err("PRCM: already initialized; won't reinitialize\n");
          return -EINVAL;
      }
-

This change is against the coding style.


I will fix this. Thanks for catching this.

      if (nr_regs > OMAP_PRCM_MAX_NR_PENDING_REG) {
          pr_err("PRCM: nr_regs too large\n");
          return -EINVAL;
@@ -339,8 +338,8 @@ int omap_prcm_register_chain_handler(struct
omap_prcm_irq_setup *irq_setup)
          ct->chip.irq_mask = irq_gc_mask_clr_bit;
          ct->chip.irq_unmask = irq_gc_mask_set_bit;

-        ct->regs.ack = irq_setup->ack + i * 4;
-        ct->regs.mask = irq_setup->mask + i * 4;
+        ct->regs.ack = irq_setup->ack[i];
+        ct->regs.mask = irq_setup->mask[i];

... and you can drop this change.


          irq_setup_generic_chip(gc, mask[i], 0, IRQ_NOREQUEST, 0);
          prcm_irq_chips[i] = gc;


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux