OMAP2+: hwmod: Add API to enable IO ring wakeup.

Add API to determine IO-PAD wakeup event status for a given hwmod dynamic_mux pad.

Wake up event set will be cleared on pad mux_read.

Signed-off-by Govindraj.R<[email protected]>

---
  arch/arm/mach-omap2/mux.c                    |   30 ++++++++++++++++++++++++++
  arch/arm/mach-omap2/mux.h                    |   13 +++++++++++
  arch/arm/mach-omap2/omap_hwmod.c             |    7 ++++++
  arch/arm/plat-omap/include/plat/omap_hwmod.h |    1 +
  4 files changed, 51 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/mux.c +30 -0

index 655e948..fb75aae 100644
@@ -351,6 +351,36 @@ err1:
  	return NULL;
  }
 
+/**
+ * omap_hwmod_mux_get_wake_status - omap hwmod check pad wakeup
+ * @hmux:		Pads for a hwmod
+ *
+ * Gets the wakeup status of given pad from omap-hwmod.
+ * Returns true if wakeup capability is set and wakeup event occurred.
+ * Returns false if wakeup event has not occurred or pads are not available.
+ */
+bool omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux)
+{
+	int i;
+	unsigned int val;
+	u8 ret = false;
+
+	for (i = 0; i<  hmux->nr_pads; i++) {
+		struct omap_device_pad *pad =&hmux->pads[i];
+
+		if (pad->flags&  OMAP_DEVICE_PAD_WAKEUP) {
+			val = omap_mux_read(pad->partition,
+					pad->mux->reg_offset);
+			if (val&  OMAP_WAKEUP_EVENT) {
+				ret = true;
+				break;
+			}
+		}
+	}
+
+	return ret;
+}
+
  /* Assumes the calling function takes care of locking */
  void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
  {

arch/arm/mach-omap2/mux.h +13 -0

index 2132308..8b2150a 100644
@@ -225,8 +225,21 @@ omap_hwmod_mux_init(struct omap_device_pad *bpads, int nr_pads);
   */
  void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state);
 
+/**
+ * omap_hwmod_mux_get_wake_status - omap hwmod check pad wakeup
+ * @hmux:		Pads for a hwmod
+ *
+ * Called only from omap_hwmod.c, do not use.
+ */
+bool omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux);
  #else
 
+static inline bool
+omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux)
+{
+	return 0;
+}
+
  static inline int omap_mux_init_gpio(int gpio, int val)
  {
  	return 0;

arch/arm/mach-omap2/omap_hwmod.c +7 -0

index e751dd9..a8b24d7 100644
@@ -2724,3 +2724,10 @@ int omap_hwmod_no_setup_reset(struct omap_hwmod *oh)
 
  	return 0;
  }
+
+int omap_hwmod_pad_get_wakeup_status(struct omap_hwmod *oh)
+{
+	if (oh&&  oh->mux)
+		return omap_hwmod_mux_get_wake_status(oh->mux);
+	return -EINVAL;
+}

arch/arm/plat-omap/include/plat/omap_hwmod.h +1 -0

index 0e329ca..9a6195c 100644
@@ -607,6 +607,7 @@ u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
 
  int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
 
+int omap_hwmod_pad_get_wakeup_status(struct omap_hwmod *oh);
  /*
   * Chip variant-specific hwmod init routines - XXX should be converted
   * to use initcalls once the initial boot ordering is straightened out

Quoted 5 lines · Govindraj.R

Add API to determine IO-PAD wakeup event status for a given hwmod dynamic_mux pad.

Wake up event set will be cleared on pad mux_read.

Are these api's even getting used in this series?

Quoted 108 lines

Signed-off-by Govindraj.R<[email protected]>

---
?arch/arm/mach-omap2/mux.c ? ? ? ? ? ? ? ? ? ?| ? 30
++++++++++++++++++++++++++

?arch/arm/mach-omap2/mux.h ? ? ? ? ? ? ? ? ? ?| ? 13 +++++++++++
?arch/arm/mach-omap2/omap_hwmod.c ? ? ? ? ? ? | ? ?7 ++++++
?arch/arm/plat-omap/include/plat/omap_hwmod.h | ? ?1 +

?4 files changed, 51 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/mux.c +7 -0

index 655e948..fb75aae 100644
@@ -351,6 +351,36 @@ err1:
 ? ? ? ?return NULL;
 ?}
 
+/**
+ * omap_hwmod_mux_get_wake_status - omap hwmod check pad wakeup
+ * @hmux: ? ? ? ? ? ? ?Pads for a hwmod
+ *
+ * Gets the wakeup status of given pad from omap-hwmod.
+ * Returns true if wakeup capability is set and wakeup event occurred.
+ * Returns false if wakeup event has not occurred or pads are not

available.
+ */

arch/arm/mach-omap2/mux.c +22 -0

+bool omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux)
+{
+ ? ? ? int i;
+ ? ? ? unsigned int val;
+ ? ? ? u8 ret = false;
+
+ ? ? ? for (i = 0; i< ?hmux->nr_pads; i++) {
+ ? ? ? ? ? ? ? struct omap_device_pad *pad =&hmux->pads[i];
+
+ ? ? ? ? ? ? ? if (pad->flags& ?OMAP_DEVICE_PAD_WAKEUP) {
+ ? ? ? ? ? ? ? ? ? ? ? val = omap_mux_read(pad->partition,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pad->mux->reg_offset);
+ ? ? ? ? ? ? ? ? ? ? ? if (val& ?OMAP_WAKEUP_EVENT) {
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ret = true;
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? }
+ ? ? ? }
+
+ ? ? ? return ret;
+}
+

?/* Assumes the calling function takes care of locking */ ?void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state) ?{

arch/arm/mach-omap2/mux.h +0 -0

index 2132308..8b2150a 100644
@@ -225,8 +225,21 @@ omap_hwmod_mux_init(struct omap_device_pad *bpads,
 int nr_pads);

? */
?void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state);

arch/arm/mach-omap2/mux.h +7 -0

+/**
+ * omap_hwmod_mux_get_wake_status - omap hwmod check pad wakeup
+ * @hmux: ? ? ? ? ? ? ?Pads for a hwmod
+ *
+ * Called only from omap_hwmod.c, do not use.
+ */
+bool omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux);

?#else

arch/arm/mach-omap2/mux.h +6 -0

+static inline bool
+omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux)
+{
+ ? ? ? return 0;
+}
+

?static inline int omap_mux_init_gpio(int gpio, int val) ?{
? ? ? ?return 0;

arch/arm/mach-omap2/omap_hwmod.c +0 -0

index e751dd9..a8b24d7 100644
@@ -2724,3 +2724,10 @@ int omap_hwmod_no_setup_reset(struct omap_hwmod
 *oh)
 
 ? ? ? ?return 0;

?}

arch/arm/mach-omap2/omap_hwmod.c +7 -0

+
+int omap_hwmod_pad_get_wakeup_status(struct omap_hwmod *oh)
+{
+ ? ? ? if (oh&& ?oh->mux)
+ ? ? ? ? ? ? ? return omap_hwmod_mux_get_wake_status(oh->mux);
+ ? ? ? return -EINVAL;
+}

arch/arm/plat-omap/include/plat/omap_hwmod.h +1 -0

index 0e329ca..9a6195c 100644
@@ -607,6 +607,7 @@ u32 omap_hwmod_get_context_loss_count(struct
 omap_hwmod *oh);
 
 ?int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
 
+int omap_hwmod_pad_get_wakeup_status(struct omap_hwmod *oh);

Quoted 3 lines

?/*
? * Chip variant-specific hwmod init routines - XXX should be converted ? * to use initcalls once the initial boot ordering is straightened out

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

Thanks for the review,

Quoted 2 lines · Rajendra Nayak

Quoted 1 line · Govindraj.R

Quoted 6 lines

Quoted 4 lines

Add API to determine IO-PAD wakeup event status for a given hwmod dynamic_mux pad.

Wake up event set will be cleared on pad mux_read.

Are these api's even getting used in this series?

Used in Tero's irq_chaining patches.

http://lkml.org/lkml/2011/9/23/121

--
Thanks,
Govindraj.R

Quoted 117 lines

Quoted 110 lines

Signed-off-by Govindraj.R<[email protected]>

---
  arch/arm/mach-omap2/mux.c                    |   30
++++++++++++++++++++++++++
  arch/arm/mach-omap2/mux.h                    |   13 +++++++++++
  arch/arm/mach-omap2/omap_hwmod.c             |    7 ++++++
  arch/arm/plat-omap/include/plat/omap_hwmod.h |    1 +
  4 files changed, 51 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/mux.c +7 -0

index 655e948..fb75aae 100644
@@ -351,6 +351,36 @@ err1:
        return NULL;
  }
 
+/**
+ * omap_hwmod_mux_get_wake_status - omap hwmod check pad wakeup
+ * @hmux:              Pads for a hwmod
+ *
+ * Gets the wakeup status of given pad from omap-hwmod.
+ * Returns true if wakeup capability is set and wakeup event occurred.
+ * Returns false if wakeup event has not occurred or pads are not

available.
+ */

arch/arm/mach-omap2/mux.c +22 -0

+bool omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux)
+{
+       int i;
+       unsigned int val;
+       u8 ret = false;
+
+       for (i = 0; i<    hmux->nr_pads; i++) {
+               struct omap_device_pad *pad =&hmux->pads[i];
+
+               if (pad->flags&    OMAP_DEVICE_PAD_WAKEUP) {
+                       val = omap_mux_read(pad->partition,
+                                       pad->mux->reg_offset);
+                       if (val&    OMAP_WAKEUP_EVENT) {
+                               ret = true;
+                               break;
+                       }
+               }
+       }
+
+       return ret;
+}
+
  /* Assumes the calling function takes care of locking */
  void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
  {

arch/arm/mach-omap2/mux.h +13 -0

index 2132308..8b2150a 100644
@@ -225,8 +225,21 @@ omap_hwmod_mux_init(struct omap_device_pad *bpads,
 int nr_pads);
   */
  void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state);
 
+/**
+ * omap_hwmod_mux_get_wake_status - omap hwmod check pad wakeup
+ * @hmux:              Pads for a hwmod
+ *
+ * Called only from omap_hwmod.c, do not use.
+ */
+bool omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux);
  #else
 
+static inline bool
+omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux)
+{
+       return 0;
+}
+
  static inline int omap_mux_init_gpio(int gpio, int val)
  {

        return 0;

arch/arm/mach-omap2/omap_hwmod.c +7 -0

index e751dd9..a8b24d7 100644
@@ -2724,3 +2724,10 @@ int omap_hwmod_no_setup_reset(struct omap_hwmod
 *oh)
 
        return 0;
  }
+
+int omap_hwmod_pad_get_wakeup_status(struct omap_hwmod *oh)
+{
+       if (oh&&    oh->mux)
+               return omap_hwmod_mux_get_wake_status(oh->mux);
+       return -EINVAL;
+}

arch/arm/plat-omap/include/plat/omap_hwmod.h +1 -0

index 0e329ca..9a6195c 100644
@@ -607,6 +607,7 @@ u32 omap_hwmod_get_context_loss_count(struct
 omap_hwmod *oh);
 
  int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
 
+int omap_hwmod_pad_get_wakeup_status(struct omap_hwmod *oh);
  /*
   * Chip variant-specific hwmod init routines - XXX should be converted

Quoted 1 line

* to use initcalls once the initial boot ordering is straightened out

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

Quoted 11 lines · Govindraj

Thanks for the review,

Quoted 6 lines · On Sat, Oct 1, 2011 at 8:03 PM, Rajendra Nayak<[email protected]> ?wrote:

Quoted 4 lines · Govindraj.R

Add API to determine IO-PAD wakeup event status for a given hwmod dynamic_mux pad.

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Wake up event set will be cleared on pad mux_read.

Are these api's even getting used in this series?

Used in Tero's irq_chaining patches.

So shouldn't this patch be part of his series instead?

Quoted 127 lines

http://lkml.org/lkml/2011/9/23/121

--
Thanks,
Govindraj.R

Quoted 118 lines

Quoted 108 lines

Signed-off-by Govindraj.R<[email protected]>

---
?arch/arm/mach-omap2/mux.c ? ? ? ? ? ? ? ? ? ?| ? 30
++++++++++++++++++++++++++

?arch/arm/mach-omap2/mux.h ? ? ? ? ? ? ? ? ? ?| ? 13 +++++++++++
?arch/arm/mach-omap2/omap_hwmod.c ? ? ? ? ? ? | ? ?7 ++++++
?arch/arm/plat-omap/include/plat/omap_hwmod.h | ? ?1 +

?4 files changed, 51 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/mux.c +7 -0

index 655e948..fb75aae 100644
@@ -351,6 +351,36 @@ err1:
 ? ? ? ?return NULL;
 ?}
 
+/**
+ * omap_hwmod_mux_get_wake_status - omap hwmod check pad wakeup
+ * @hmux: ? ? ? ? ? ? ?Pads for a hwmod
+ *
+ * Gets the wakeup status of given pad from omap-hwmod.
+ * Returns true if wakeup capability is set and wakeup event occurred.
+ * Returns false if wakeup event has not occurred or pads are not

available.
+ */

arch/arm/mach-omap2/mux.c +22 -0

+bool omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux)
+{
+ ? ? ? int i;
+ ? ? ? unsigned int val;
+ ? ? ? u8 ret = false;
+
+ ? ? ? for (i = 0; i< ? ?hmux->nr_pads; i++) {
+ ? ? ? ? ? ? ? struct omap_device_pad *pad =&hmux->pads[i];
+
+ ? ? ? ? ? ? ? if (pad->flags& ? ?OMAP_DEVICE_PAD_WAKEUP) {
+ ? ? ? ? ? ? ? ? ? ? ? val = omap_mux_read(pad->partition,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pad->mux->reg_offset);
+ ? ? ? ? ? ? ? ? ? ? ? if (val& ? ?OMAP_WAKEUP_EVENT) {
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ret = true;
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? }
+ ? ? ? }
+
+ ? ? ? return ret;
+}
+

?/* Assumes the calling function takes care of locking */ ?void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state) ?{

arch/arm/mach-omap2/mux.h +0 -0

index 2132308..8b2150a 100644
@@ -225,8 +225,21 @@ omap_hwmod_mux_init(struct omap_device_pad *bpads,
 int nr_pads);

? */
?void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state);

arch/arm/mach-omap2/mux.h +7 -0

+/**
+ * omap_hwmod_mux_get_wake_status - omap hwmod check pad wakeup
+ * @hmux: ? ? ? ? ? ? ?Pads for a hwmod
+ *
+ * Called only from omap_hwmod.c, do not use.
+ */
+bool omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux);

?#else

arch/arm/mach-omap2/mux.h +6 -0

+static inline bool
+omap_hwmod_mux_get_wake_status(struct omap_hwmod_mux_info *hmux)
+{
+ ? ? ? return 0;
+}
+

?static inline int omap_mux_init_gpio(int gpio, int val) ?{
? ? ? ?return 0;

arch/arm/mach-omap2/omap_hwmod.c +0 -0

index e751dd9..a8b24d7 100644
@@ -2724,3 +2724,10 @@ int omap_hwmod_no_setup_reset(struct omap_hwmod
 *oh)
 
 ? ? ? ?return 0;

?}

arch/arm/mach-omap2/omap_hwmod.c +7 -0

+
+int omap_hwmod_pad_get_wakeup_status(struct omap_hwmod *oh)
+{
+ ? ? ? if (oh&& ? ?oh->mux)
+ ? ? ? ? ? ? ? return omap_hwmod_mux_get_wake_status(oh->mux);
+ ? ? ? return -EINVAL;
+}

arch/arm/plat-omap/include/plat/omap_hwmod.h +1 -0

index 0e329ca..9a6195c 100644
@@ -607,6 +607,7 @@ u32 omap_hwmod_get_context_loss_count(struct
 omap_hwmod *oh);
 
 ?int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
 
+int omap_hwmod_pad_get_wakeup_status(struct omap_hwmod *oh);

Quoted 3 lines

?/*
? * Chip variant-specific hwmod init routines - XXX should be converted ? * to use initcalls once the initial boot ordering is straightened out

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

Quoted 11 lines · Rajendra Nayak

Quoted 9 lines · Govindraj

Quoted 8 lines · On Sat, Oct 1, 2011 at 8:03 PM, Rajendra Nayak<[email protected]> ?wrote:

Quoted 5 lines · Govindraj.R

Add API to determine IO-PAD wakeup event status for a given hwmod dynamic_mux pad.

Wake up event set will be cleared on pad mux_read.

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Are these api's even getting used in this series?

Used in Tero's irq_chaining patches.

So shouldn't this patch be part of his series instead?

Yes it is. Part of his v9-series.

Quoted 16 lines · Govindraj

Quoted 15 lines · Rajendra Nayak

Quoted 12 lines · Govindraj

Quoted 9 lines · On Sat, Oct 1, 2011 at 8:03 PM, Rajendra Nayak<[email protected]> ?wrote:

Quoted 5 lines · Govindraj.R

Add API to determine IO-PAD wakeup event status for a given hwmod dynamic_mux pad.

Wake up event set will be cleared on pad mux_read.

Are these api's even getting used in this series?

Used in Tero's irq_chaining patches.

Quoted 4 lines

Quoted 1 line

So shouldn't this patch be part of his series instead?

Yes it is. Part of his v9-series.

Then please drop from this series and state the introductory patch (PATCH 0/n) that this series has a dependency on Tero's series.

We had been using traditional 8250 driver as uart console driver prior to omap-serial driver. Since we have omap-serial driver in mainline kernel for some time now it has been used as default uart console driver on omap2+ platforms. Remove 8250 support for omap-uarts.

Serial_in and serial_out override for 8250 serial driver is also removed. Empty fifo read fix is already taken care with omap-serial driver with data ready bit check from LSR reg before reading RX fifo.

Also waiting for THRE(transmit hold reg empty) is done with wait_for_xmitr in omap-serial driver.

Remove headers that were necessary to support 8250 support and remove all config bindings done to keep 8250 backward compatibility while adding omap-serial driver. Remove omap_uart_reset needed for 8250 autoconf.

Signed-off-by Govindraj.R <[email protected]>

Quoted 5 lines · Govindraj.R

We had been using traditional 8250 driver as uart console driver prior to omap-serial driver. Since we have omap-serial driver in mainline kernel for some time now it has been used as default uart console driver on omap2+ platforms. Remove 8250 support for omap-uarts.

Nice to see the this disappearing.

Quoted 2 lines

Serial_in and serial_out override for 8250 serial driver is also removed.

Quoted 2 lines

Empty fifo read fix is already taken care with omap-serial driver with data ready bit check from LSR reg before reading RX fifo.

As stated in the previous review. ?Patches that move code/features should have the removal and the add-back in the same patch. ?Doing so makes it easy for reviewers to see whether it was simply moved, or if it was modified when it was moved, etc.

Quoted 2 lines

Also waiting for THRE(transmit hold reg empty) is done with wait_for_xmitr in omap-serial driver.

Again, remove it here in the patch that adds that support (the errata patch I guess.)

Quoted 6 lines

Remove headers that were necessary to support 8250 support and remove all config bindings done to keep 8250 backward compatibility while adding omap-serial driver. Remove omap_uart_reset needed for 8250 autoconf.

Signed-off-by Govindraj.R <[email protected]>

So basically, this patch should only remove the legacy 8250 support (as the subject says) and everything else should be done in the other relevant patches.

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

Hi Kevin,

Thanks for the review,

Quoted 16 lines · Kevin Hilman

Quoted 5 lines · Govindraj.R

We had been using traditional 8250 driver as uart console driver prior to omap-serial driver. Since we have omap-serial driver in mainline kernel for some time now it has been used as default uart console driver on omap2+ platforms. Remove 8250 support for omap-uarts.

Nice to see the this disappearing.

Quoted 5 lines

Serial_in and serial_out override for 8250 serial driver is also removed.

Empty fifo read fix is already taken care with omap-serial driver with data ready bit check from LSR reg before reading RX fifo.

Quoted 5 lines

As stated in the previous review. ?Patches that move code/features should have the removal and the add-back in the same patch. ?Doing so makes it easy for reviewers to see whether it was simply moved, or if it was modified when it was moved, etc.

Empty fifo read is already taken care in omap-serial.c and is part of mainline code. Nothing to add to omap-serial.c

Quoted 6 lines

Quoted 2 lines

Also waiting for THRE(transmit hold reg empty) is done with wait_for_xmitr in omap-serial driver.

Again, remove it here in the patch that adds that support (the errata patch I guess.)

The errata patch ( [PATCH v6 11/16] ) moves only mdr_errata and force_idle from serial.c to omap-serial.c.

Already handled stuffs and things that already exists with omap-serial.c are removed here.

Quoted 11 lines

Quoted 6 lines

Remove headers that were necessary to support 8250 support and remove all config bindings done to keep 8250 backward compatibility while adding omap-serial driver. Remove omap_uart_reset needed for 8250 autoconf.

Signed-off-by Govindraj.R <[email protected]>

So basically, this patch should only remove the legacy 8250 support (as the subject says) and everything else should be done in the other relevant patches.

Yes removes only the 8250 code. serial_in/serial_out were read/write overrides part of 8250 code.

serial_in/serial_out had these checks for empty fifo read and wait for tx which is already handled with omap-serial.c.

Quoted 22 lines · Govindraj

Hi Kevin,

Thanks for the review,

Quoted 16 lines · Kevin Hilman

Quoted 5 lines · Govindraj.R

We had been using traditional 8250 driver as uart console driver prior to omap-serial driver. Since we have omap-serial driver in mainline kernel for some time now it has been used as default uart console driver on omap2+ platforms. Remove 8250 support for omap-uarts.

Nice to see the this disappearing.

Quoted 5 lines

Serial_in and serial_out override for 8250 serial driver is also removed.

Empty fifo read fix is already taken care with omap-serial driver with data ready bit check from LSR reg before reading RX fifo.

Quoted 8 lines

Quoted 5 lines

As stated in the previous review. ?Patches that move code/features should have the removal and the add-back in the same patch. ?Doing so makes it easy for reviewers to see whether it was simply moved, or if it was modified when it was moved, etc.

Empty fifo read is already taken care in omap-serial.c and is part of mainline code. ?Nothing to add to omap-serial.c

OK, good. ?I guess I missed the 'already' part in your changelog.

Quoted 7 lines

Quoted 6 lines

Quoted 2 lines

Also waiting for THRE(transmit hold reg empty) is done with wait_for_xmitr in omap-serial driver.

Again, remove it here in the patch that adds that support (the errata patch I guess.)

Quoted 5 lines

The errata patch ( [PATCH v6 11/16] ) moves only mdr_errata and force_idle from serial.c to omap-serial.c.

Already handled stuffs and things that already exists with omap-serial.c are removed here.

OK.

Quoted 15 lines

Quoted 11 lines

Quoted 6 lines

Remove headers that were necessary to support 8250 support and remove all config bindings done to keep 8250 backward compatibility while adding omap-serial driver. Remove omap_uart_reset needed for 8250 autoconf.

Signed-off-by Govindraj.R <[email protected]>

So basically, this patch should only remove the legacy 8250 support (as the subject says) and everything else should be done in the other relevant patches.

Yes removes only the 8250 code. serial_in/serial_out were read/write overrides part of 8250 code.

Quoted 2 lines

serial_in/serial_out had these checks for empty fifo read and wait for tx which is already handled with omap-serial.c.

OK, thanks for the clarification.

The changelog could've been a bit more specific for readers not intimately familiar with the driver.

Currently we use a shared irq handler to identify uart activity and then trigger a timer.

Based the timeout value set from sysfs the timer used to expire.

If no uart-activity was detected timer-expiry func sets can_sleep flag. Based on this we will disable the uart clocks in idle path.

Since the clock gating mechanism is outside the uart driver, we currently use this mechanism. In preparation to runtime implementation for omap-serial driver we can cleanup this mechanism and use runtime API's to gate uart clocks.

Also remove timer related info from local uart_state struct and remove the code used to set timeout value from sysfs. Remove un-used function omap_uart_check_wakeup.

Signed-off-by Govindraj.R <[email protected]>

Quoted 2 lines · Govindraj.R

Currently we use a shared irq handler to identify uart activity and then trigger a timer.

OK.

Quoted 1 line

Based the timeout value set from sysfs the timer used to expire.

Please re-phrase as I'm not sure what is being said here.

Quoted 7 lines

If no uart-activity was detected timer-expiry func sets can_sleep flag. Based on this we will disable the uart clocks in idle path.

Since the clock gating mechanism is outside the uart driver, we currently use this mechanism. In preparation to runtime implementation for omap-serial driver we can cleanup this mechanism and use runtime API's to gate uart clocks.

Quoted 5 lines

Also remove timer related info from local uart_state struct and remove the code used to set timeout value from sysfs. Remove un-used function omap_uart_check_wakeup.

Signed-off-by Govindraj.R <[email protected]>

The patch itself fine, and is a well-neede cleanup, but changelog (especially the first part) does not read well.

Removing some of the uart info that is maintained in omap_uart_state struct used for UART PM in serial.c.

Remove omap_uart_state struct dependency from omap_serial_init, omap_serial_init_port, omap_serial_early_init and omap_uart_idle_init functions. And populate the same info in omap_uart_port_info struct used as pdata struct.

Added omap_uart_hwmod_lookup function to look up oh by name used in serial_port_init and omap_serial_early_init functions.

A list of omap_uart_state was maintained one for each uart, the same is removed. Number of uarts available is maintained in num_uarts field, re-use the same in omap_serial_init func to register each uart.

Remove omap_info which used details from omap_uart_state and use a pdata pointer to pass platform specific info to driver.

The mapbase (start_address), membase(io_remap cookie) maintained as part of uart_state struct and pdata struct are removed as this is handled within driver.

Errata field is also moved to pdata.

These changes are done to cleanup serial.c file and prepare for runtime changes.

Signed-off-by Govindraj.R <[email protected]>

---
 arch/arm/mach-omap2/serial.c                  |  132 +++++++++---------------
 arch/arm/plat-omap/include/plat/omap-serial.h |    4 +-
 drivers/tty/serial/omap-serial.c              |   12 ++-
 3 files changed, 61 insertions(+), 87 deletions(-)

arch/arm/mach-omap2/serial.c +3 -10

index c98b9b4..8c43d1c 100644
@@ -68,14 +68,6 @@ struct omap_uart_state {
 	int clocked;
 
 	int regshift;
-	void __iomem *membase;
-	resource_size_t mapbase;
-
-	struct list_head node;
-	struct omap_hwmod *oh;
-	struct platform_device *pdev;
-
-	u32 errata;
 #if defined(CONFIG_ARCH_OMAP3) && defined(CONFIG_PM)
 	int context_valid;
 

@@ -90,7 +82,6 @@ struct omap_uart_state {
 #endif
 };
 
-static LIST_HEAD(uart_list);
 static u8 num_uarts;
 
 static int uart_idle_hwmod(struct omap_device *od)

@@ -143,7 +134,19 @@ static inline void serial_write_reg(struct omap_uart_state *uart, int offset,
 	__raw_writeb(value, uart->membase + offset);
 }
 
-#if defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP3)
+struct omap_hwmod *omap_uart_hwmod_lookup(int num)
+{
+	struct omap_hwmod *oh;

arch/arm/mach-omap2/serial.c +2 -3

 /*
  * Work Around for Errata i202 (3430 - 1.12, 3630 - 1.6)

@@ -357,22 +360,17 @@ int omap_uart_can_sleep(void)
 	return can_sleep;
 }
 
-static void omap_uart_idle_init(struct omap_uart_state *uart)
+static void omap_uart_idle_init(struct omap_uart_port_info *uart,
+				unsigned short num)
 {
-	int ret;
-

arch/arm/mach-omap2/serial.c +5 -19

 	} else if (cpu_is_omap24xx()) {
 		u32 wk_mask = 0;
 		u32 wk_en = PM_WKEN1, wk_st = PM_WKST1;
 
-		switch (uart->num) {
+		switch (num) {
 		case 0:
 			wk_mask = OMAP24XX_ST_UART1_MASK;
 			break;

@@ -421,7 +418,6 @@ static void omap_uart_idle_init(struct omap_uart_state *uart)
 		uart->wk_en = NULL;
 		uart->wk_st = NULL;
 		uart->wk_mask = 0;
-		uart->padconf = 0;
 	}
 }
 

@@ -436,26 +432,13 @@ static void omap_uart_block_sleep(struct omap_uart_state *uart)
 
 static int __init omap_serial_early_init(void)
 {
-	int i = 0;
-
 	do {
-		char oh_name[MAX_UART_HWMOD_NAME_LEN];
 		struct omap_hwmod *oh;
-		struct omap_uart_state *uart;
 
-		snprintf(oh_name, MAX_UART_HWMOD_NAME_LEN,
-			 "uart%d", i + 1);
-		oh = omap_hwmod_lookup(oh_name);
+		oh = omap_uart_hwmod_lookup(num_uarts);
 		if (!oh)
 			break;
 
-		uart = kzalloc(sizeof(struct omap_uart_state), GFP_KERNEL);
-		if (WARN_ON(!uart))
-			return -ENODEV;
-
-		uart->oh = oh;
-		uart->num = i++;
-		list_add_tail(&uart->node, &uart_list);
 		num_uarts++;
 
 		/*

@@ -468,7 +451,7 @@ static int __init omap_serial_early_init(void)
 		 * to determine SoC specific init before omap_device
 		 * is ready.  Therefore, don't allow idle here
 		 */
-		uart->oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;
+		oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;
 	} while (1);
 
 	return 0;

@@ -488,57 +471,47 @@ core_initcall(omap_serial_early_init);
  */
 void __init omap_serial_init_port(struct omap_board_data *bdata)
 {
-	struct omap_uart_state *uart;
 	struct omap_hwmod *oh;
 	struct platform_device *pdev;
-	void *pdata = NULL;
+	char *name = DRIVER_NAME;
+	struct omap_uart_port_info *pdata;

arch/arm/mach-omap2/serial.c +3 -11

 		return;
 
-	list_for_each_entry(uart, &uart_list, node)
-		if (bdata->id == uart->num)
-			break;
-
-	oh = uart->oh;
-	uart->dma_enabled = 0;
-	name = DRIVER_NAME;
+	oh = omap_uart_hwmod_lookup(bdata->id);
+	if (!oh)
+		return;
 
-	omap_up.dma_enabled = uart->dma_enabled;
-	omap_up.uartclk = OMAP24XX_BASE_BAUD * 16;
-	omap_up.mapbase = oh->slaves[0]->addr->pa_start;
-	omap_up.membase = omap_hwmod_get_mpu_rt_va(oh);

arch/arm/mach-omap2/serial.c +3 -2

-	if (WARN_ON(!oh))
-		return;
+	pdata->uartclk = OMAP24XX_BASE_BAUD * 16;
+	pdata->flags = UPF_BOOT_AUTOCONF;
+

arch/arm/mach-omap2/serial.c +1 -12

 	oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);
 
-	uart->regshift = 2;
-	uart->mapbase = oh->slaves[0]->addr->pa_start;
-	uart->membase = omap_hwmod_get_mpu_rt_va(oh);
-	uart->pdev = pdev;
-
-	oh->dev_attr = uart;
-
 	console_lock(); /* in case the earlycon is on the UART */
 
 	/*

@@ -546,23 +519,18 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
 	 * on init.  Now that omap_device is ready, ensure full idle
 	 * before doing omap_device_enable().
 	 */
-	omap_hwmod_idle(uart->oh);
+	omap_hwmod_idle(oh);
 
-	omap_device_enable(uart->pdev);
-	omap_uart_idle_init(uart);
-	omap_hwmod_enable_wakeup(uart->oh);
-	omap_device_idle(uart->pdev);

arch/arm/mach-omap2/serial.c +4 -6

 		device_init_wakeup(&pdev->dev, true);
 
-	/* Enable the MDR1 errata for OMAP3 */
-	if (cpu_is_omap34xx() && !cpu_is_ti816x())
-		uart->errata |= UART_ERRATA_i202_MDR1_ACCESS;
+	kfree(pdata);
 }
 
 /**

@@ -574,11 +542,11 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
  */
 void __init omap_serial_init(void)
 {
-	struct omap_uart_state *uart;
 	struct omap_board_data bdata;
+	u8 i;
 
-	list_for_each_entry(uart, &uart_list, node) {
-		bdata.id = uart->num;
+	for (i = 0; i < num_uarts; i++) {
+		bdata.id = i;
 		bdata.flags = 0;
 		bdata.pads = NULL;
 		bdata.pads_cnt = 0;

arch/arm/plat-omap/include/plat/omap-serial.h +2 -2

index 307cd6f..0f061b4 100644
@@ -59,9 +59,9 @@
 struct omap_uart_port_info {
 	bool			dma_enabled;	/* To specify DMA Mode */
 	unsigned int		uartclk;	/* UART clock rate */
-	void __iomem		*membase;	/* ioremap cookie or NULL */
-	resource_size_t		mapbase;	/* resource base */
 	upf_t			flags;		/* UPF_* flags */
+
+	u32			errata;
 };
 
 struct uart_omap_dma {

drivers/tty/serial/omap-serial.c +3 -2

index 5e713d3..6c2ea54 100644
@@ -1275,10 +1275,16 @@ static int serial_omap_probe(struct platform_device *pdev)
 	up->port.ops = &serial_omap_pops;
 	up->port.line = pdev->id;
 
-	up->port.membase = omap_up_info->membase;
-	up->port.mapbase = omap_up_info->mapbase;
+	up->port.mapbase = mem->start;
+	up->port.membase = ioremap(mem->start, resource_size(mem));
+

Quoted 7 lines · Govindraj.R

Removing some of the uart info that is maintained in omap_uart_state struct used for UART PM in serial.c.

Remove omap_uart_state struct dependency from omap_serial_init, omap_serial_init_port, omap_serial_early_init and omap_uart_idle_init functions. ?And populate the same info in omap_uart_port_info struct used as pdata struct.

IMO, this change doesn't belong in this patch and leads to clutter. ?The rest of the series slowly removes/replaces all the fields from this struct, so the right place to remove it's usage all together is at the end of the series when (if) all the fields are no longer needed (or have been moved.)

Stated differently, IMO, this patch should leave the uart->num and uart->oh and the list_head (uart->node) alone (probably uart->pdev too) and just cleanup the fields that are no longer used. ?Removing num, oh, node here causes churn because you're force to change things here that are then removed in later patches.

Quoted 2 lines

Added omap_uart_hwmod_lookup function to look up oh by name used in serial_port_init and omap_serial_early_init functions.

Because of the above change, you now are doing a hwmod lookup 2 times for every UART. ?Leaving the uart_list and uart->num in place will avoid the need for that change.

Quoted 6 lines

A list of omap_uart_state was maintained one for each uart, the same is removed. ?Number of uarts available is maintained in num_uarts field, re-use the same in omap_serial_init func to register each uart.

Remove omap_info which used details from omap_uart_state and use a pdata pointer to pass platform specific info to driver.

There is no omap_info. ?Did you mean omap_up_info?

Quoted 4 lines

The mapbase (start_address), membase(io_remap cookie) maintained as part of uart_state struct and pdata struct are removed as this is handled within driver.

This part makes sense.

Quoted 1 line

Errata field is also moved to pdata.

Why in this patch instead of the subsequent "Move errata handling from serial.c to omap-serial" patch?

Quoted 2 lines

These changes are done to cleanup serial.c file and prepare for runtime changes.

There are a lot of changes in this patch with very little description as to why, and many appear to be unrelated. ?They should probably be separate patches, or have a better description as to how all the changes are related so they belong in the same patch.

Quoted 42 lines

Signed-off-by Govindraj.R <[email protected]>

---

?arch/arm/mach-omap2/serial.c ? ? ? ? ? ? ? ? ?| ?132 +++++++++---------------
?arch/arm/plat-omap/include/plat/omap-serial.h | ? ?4 +-
?drivers/tty/serial/omap-serial.c ? ? ? ? ? ? ?| ? 12 ++-

?3 files changed, 61 insertions(+), 87 deletions(-)

arch/arm/mach-omap2/serial.c +0 -8

index c98b9b4..8c43d1c 100644
@@ -68,14 +68,6 @@ struct omap_uart_state {
 ? ? ? int clocked;
 
 ? ? ? int regshift;
- ? ? void __iomem *membase;
- ? ? resource_size_t mapbase;
-
- ? ? struct list_head node;
- ? ? struct omap_hwmod *oh;
- ? ? struct platform_device *pdev;
-
- ? ? u32 errata;
 ?#if defined(CONFIG_ARCH_OMAP3) && defined(CONFIG_PM)
 ? ? ? int context_valid;
 

@@ -90,7 +82,6 @@ struct omap_uart_state {

?#endif
?};

-static LIST_HEAD(uart_list);
?static u8 num_uarts;

?static int uart_idle_hwmod(struct omap_device *od)

arch/arm/mach-omap2/serial.c +3 -1

@@ -143,7 +134,19 @@ static inline void serial_write_reg(struct omap_uart_state *uart, int offset,
 ? ? ? __raw_writeb(value, uart->membase + offset);
 ?}
 
-#if defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP3)
+struct omap_hwmod *omap_uart_hwmod_lookup(int num)
+{
+ ? ? struct omap_hwmod *oh;

arch/arm/mach-omap2/serial.c static inline void serial_write_reg(struct omap_uart_state *uart, int offset, · line 149

Quoted 10 lines

arch/arm/mach-omap2/serial.c +10 -0

+ ? ? char oh_name[MAX_UART_HWMOD_NAME_LEN];
+
+ ? ? snprintf(oh_name, MAX_UART_HWMOD_NAME_LEN, "uart%d", num + 1);
+ ? ? oh = omap_hwmod_lookup(oh_name);
+ ? ? WARN(IS_ERR(oh), "Could not lookup hmwod info for %s\n",
+ ? ? ? ? ? ? ? ? ? ? oh_name);
+ ? ? return oh;
+}
+
+#if defined(CONFIG_PM)

The CONFIG_ARCH_OMAP3 part of this #if was dropped with this change with no mention as to why. ?(I understand why it was done, but it's not releveant to $SUBJECT patch so should be a separate patch.)

Quoted 12 lines

?/*
? * Work Around for Errata i202 (3430 - 1.12, 3630 - 1.6)

arch/arm/mach-omap2/serial.c +2 -3

@@ -357,22 +360,17 @@ int omap_uart_can_sleep(void)
 ? ? ? return can_sleep;
 ?}
 
-static void omap_uart_idle_init(struct omap_uart_state *uart)
+static void omap_uart_idle_init(struct omap_uart_port_info *uart,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned short num)
 ?{
- ? ? int ret;
-

arch/arm/mach-omap2/serial.c int omap_uart_can_sleep(void) · line 372

Quoted 10 lines

arch/arm/mach-omap2/serial.c +1 -5

- ? ? uart->can_sleep = 0;
- ? ? omap_uart_smart_idle_enable(uart, 0);
-
 ? ? ? if (cpu_is_omap34xx() && !cpu_is_ti816x()) {
- ? ? ? ? ? ? u32 mod = (uart->num > 1) ? OMAP3430_PER_MOD : CORE_MOD;
+ ? ? ? ? ? ? u32 mod = (num > 1) ? OMAP3430_PER_MOD : CORE_MOD;
 ? ? ? ? ? ? ? u32 wk_mask = 0;
 ? ? ? ? ? ? ? u32 padconf = 0;
 
- ? ? ? ? ? ? /* XXX These PRM accesses do not belong here */

why?

arch/arm/mach-omap2/serial.c static void omap_uart_idle_init(struct omap_uart_state *uart) · line 394

Quoted 12 lines

arch/arm/mach-omap2/serial.c +1 -1

 ? ? ? ? ? ? ? uart->wk_en = OMAP34XX_PRM_REGADDR(mod, PM_WKEN1);
 ? ? ? ? ? ? ? uart->wk_st = OMAP34XX_PRM_REGADDR(mod, PM_WKST1);
- ? ? ? ? ? ? switch (uart->num) {
+ ? ? ? ? ? ? switch (num) {

? ? ? ? ? ? ? case 0:
? ? ? ? ? ? ? ? ? ? ? wk_mask = OMAP3430_ST_UART1_MASK; ? ? ? ? ? ? ? ? ? ? ? padconf = 0x182;

arch/arm/mach-omap2/serial.c +0 -1

@@ -391,12 +389,11 @@ static void omap_uart_idle_init(struct omap_uart_state *uart)
 ? ? ? ? ? ? ? ? ? ? ? break;
 ? ? ? ? ? ? ? }
 ? ? ? ? ? ? ? uart->wk_mask = wk_mask;
- ? ? ? ? ? ? uart->padconf = padconf;

The assignment is removed here, making all the rest of the padconf stuff that remains useless.

However, a subsequent patch removes the mux stuff entirely, so I suggest you just drop this change from here.

Quoted 68 lines

arch/arm/mach-omap2/serial.c +1 -1

 ? ? ? } else if (cpu_is_omap24xx()) {
 ? ? ? ? ? ? ? u32 wk_mask = 0;
 ? ? ? ? ? ? ? u32 wk_en = PM_WKEN1, wk_st = PM_WKST1;
 
- ? ? ? ? ? ? switch (uart->num) {
+ ? ? ? ? ? ? switch (num) {

? ? ? ? ? ? ? case 0:
? ? ? ? ? ? ? ? ? ? ? wk_mask = OMAP24XX_ST_UART1_MASK; ? ? ? ? ? ? ? ? ? ? ? break;

arch/arm/mach-omap2/serial.c +1 -15

@@ -421,7 +418,6 @@ static void omap_uart_idle_init(struct omap_uart_state *uart)
 ? ? ? ? ? ? ? uart->wk_en = NULL;
 ? ? ? ? ? ? ? uart->wk_st = NULL;
 ? ? ? ? ? ? ? uart->wk_mask = 0;
- ? ? ? ? ? ? uart->padconf = 0;
 ? ? ? }
 ?}
 

@@ -436,26 +432,13 @@ static void omap_uart_block_sleep(struct omap_uart_state *uart)
 
 ?static int __init omap_serial_early_init(void)
 ?{
- ? ? int i = 0;
-
 ? ? ? do {
- ? ? ? ? ? ? char oh_name[MAX_UART_HWMOD_NAME_LEN];
 ? ? ? ? ? ? ? struct omap_hwmod *oh;
- ? ? ? ? ? ? struct omap_uart_state *uart;
 
- ? ? ? ? ? ? snprintf(oh_name, MAX_UART_HWMOD_NAME_LEN,
- ? ? ? ? ? ? ? ? ? ? ?"uart%d", i + 1);
- ? ? ? ? ? ? oh = omap_hwmod_lookup(oh_name);
+ ? ? ? ? ? ? oh = omap_uart_hwmod_lookup(num_uarts);
 ? ? ? ? ? ? ? if (!oh)
 ? ? ? ? ? ? ? ? ? ? ? break;
 
- ? ? ? ? ? ? uart = kzalloc(sizeof(struct omap_uart_state), GFP_KERNEL);
- ? ? ? ? ? ? if (WARN_ON(!uart))
- ? ? ? ? ? ? ? ? ? ? return -ENODEV;
-
- ? ? ? ? ? ? uart->oh = oh;
- ? ? ? ? ? ? uart->num = i++;
- ? ? ? ? ? ? list_add_tail(&uart->node, &uart_list);
 ? ? ? ? ? ? ? num_uarts++;
 

? ? ? ? ? ? ? /*

arch/arm/mach-omap2/serial.c +0 -0

@@ -468,7 +451,7 @@ static int __init omap_serial_early_init(void)

? ? ? ? ? ? ? ?* to determine SoC specific init before omap_device ? ? ? ? ? ? ? ?* is ready. ?Therefore, don't allow idle here
? ? ? ? ? ? ? ?*/
- ? ? ? ? ? ? uart->oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;
+ ? ? ? ? ? ? oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET; ? ? ? } while (1);

? ? ? return 0;

arch/arm/mach-omap2/serial.c +0 -0

@@ -488,57 +471,47 @@ core_initcall(omap_serial_early_init);

? */
?void __init omap_serial_init_port(struct omap_board_data *bdata) ?{
- ? ? struct omap_uart_state *uart;
? ? ? struct omap_hwmod *oh;
? ? ? struct platform_device *pdev;
- ? ? void *pdata = NULL;
+ ? ? char *name = DRIVER_NAME;
+ ? ? struct omap_uart_port_info *pdata;
? ? ? u32 pdata_size = 0;
- ? ? char *name;
- ? ? struct omap_uart_port_info omap_up;

Quoted 6 lines

? ? ? if (WARN_ON(!bdata))
? ? ? ? ? ? ? return;
? ? ? if (WARN_ON(bdata->id < 0))
? ? ? ? ? ? ? return;
- ? ? if (WARN_ON(bdata->id >= num_uarts))
+ ? ? if (WARN_ON(bdata->id >= OMAP_MAX_HSUART_PORTS))

why? ?because of early_init, num_uarts is already the max number of UARTs available (based on hwmod probe.)

Quoted 24 lines

? ? ? ? ? ? ? return;

- ? ? list_for_each_entry(uart, &uart_list, node)
- ? ? ? ? ? ? if (bdata->id == uart->num)
- ? ? ? ? ? ? ? ? ? ? break;
-
- ? ? oh = uart->oh;
- ? ? uart->dma_enabled = 0;
- ? ? name = DRIVER_NAME;
+ ? ? oh = omap_uart_hwmod_lookup(bdata->id);
+ ? ? if (!oh)
+ ? ? ? ? ? ? return;

- ? ? omap_up.dma_enabled = uart->dma_enabled;
- ? ? omap_up.uartclk = OMAP24XX_BASE_BAUD * 16;
- ? ? omap_up.mapbase = oh->slaves[0]->addr->pa_start;
- ? ? omap_up.membase = omap_hwmod_get_mpu_rt_va(oh);
- ? ? omap_up.flags = UPF_BOOT_AUTOCONF | UPF_SHARE_IRQ;
+ ? ? pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+ ? ? if (!pdata) {
+ ? ? ? ? ? ? pr_err("Memory allocation for UART pdata failed\n");
+ ? ? ? ? ? ? return;
+ ? ? }

Quoted 3 lines

- ? ? pdata = &omap_up;
? ? ? pdata_size = sizeof(struct omap_uart_port_info);
+ ? ? omap_uart_idle_init(pdata, bdata->id);

Why was this moved here?

ISTR that the order of this call relative to the hwmod/omap_device enable/disable calls below was important, especially in the DEBUG_LL case.

Quoted 9 lines

- ? ? if (WARN_ON(!oh))
- ? ? ? ? ? ? return;
+ ? ? pdata->uartclk = OMAP24XX_BASE_BAUD * 16;
+ ? ? pdata->flags = UPF_BOOT_AUTOCONF;
+
+ ? ? /* Enable the MDR1 errata for OMAP3 */
+ ? ? if (cpu_is_omap34xx() && !cpu_is_ti816x())
+ ? ? ? ? ? ? pdata->errata |= UART_ERRATA_i202_MDR1_ACCESS;

Quoted 6 lines

- ? ? pdev = omap_device_build(name, uart->num, oh, pdata, pdata_size,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ?omap_uart_latency,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ?ARRAY_SIZE(omap_uart_latency), false);
+ ? ? pdev = omap_device_build(name, bdata->id, oh, pdata,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? pdata_size, omap_uart_latency,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ARRAY_SIZE(omap_uart_latency), false);

Note the unecesary whitespace changes in this change.

Quoted 4 lines

? ? ? WARN(IS_ERR(pdev), "Could not build omap_device for %s: %s.\n", ? ? ? ? ? ?name, oh->name);

- ? ? omap_device_disable_idle_on_suspend(pdev);

This should also be a separate patch at the end of the series, and is not related to the changes described in the changelog.

Quoted 29 lines

? ? ? oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);

- ? ? uart->regshift = 2;
- ? ? uart->mapbase = oh->slaves[0]->addr->pa_start;
- ? ? uart->membase = omap_hwmod_get_mpu_rt_va(oh);
- ? ? uart->pdev = pdev;

arch/arm/mach-omap2/serial.c +0 -3

-
- ? ? oh->dev_attr = uart;
-

? ? ? console_lock(); /* in case the earlycon is on the UART */

? ? ? /*

arch/arm/mach-omap2/serial.c +0 -0

@@ -546,23 +519,18 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)

? ? ? ?* on init. ?Now that omap_device is ready, ensure full idle ? ? ? ?* before doing omap_device_enable().
? ? ? ?*/
- ? ? omap_hwmod_idle(uart->oh);
+ ? ? omap_hwmod_idle(oh);

- ? ? omap_device_enable(uart->pdev);
- ? ? omap_uart_idle_init(uart);
- ? ? omap_hwmod_enable_wakeup(uart->oh);
- ? ? omap_device_idle(uart->pdev);
+ ? ? omap_device_enable(pdev);
+ ? ? omap_hwmod_enable_wakeup(oh);

- ? ? omap_uart_block_sleep(uart);
? ? ? console_unlock();

Quoted 4 lines

- ? ? if ((cpu_is_omap34xx() && uart->padconf) ||
- ? ? ? ? (uart->wk_en && uart->wk_mask))
+ ? ? if ((cpu_is_omap34xx() && bdata->pads) ||
+ ? ? ? ? ? ? (pdata->wk_en && pdata->wk_mask))

This change seems to belong as part of the mux patch.

Quoted 53 lines

? ? ? ? ? ? ? device_init_wakeup(&pdev->dev, true);

- ? ? /* Enable the MDR1 errata for OMAP3 */
- ? ? if (cpu_is_omap34xx() && !cpu_is_ti816x())
- ? ? ? ? ? ? uart->errata |= UART_ERRATA_i202_MDR1_ACCESS;
+ ? ? kfree(pdata);
?}

?/**

arch/arm/mach-omap2/serial.c +0 -0

@@ -574,11 +542,11 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)

? */
?void __init omap_serial_init(void)
?{
- ? ? struct omap_uart_state *uart;
? ? ? struct omap_board_data bdata;
+ ? ? u8 i;

- ? ? list_for_each_entry(uart, &uart_list, node) {
- ? ? ? ? ? ? bdata.id = uart->num;
+ ? ? for (i = 0; i < num_uarts; i++) {
+ ? ? ? ? ? ? bdata.id = i;
? ? ? ? ? ? ? bdata.flags = 0;
? ? ? ? ? ? ? bdata.pads = NULL;
? ? ? ? ? ? ? bdata.pads_cnt = 0;

arch/arm/plat-omap/include/plat/omap-serial.h +0 -0

index 307cd6f..0f061b4 100644
@@ -59,9 +59,9 @@
 ?struct omap_uart_port_info {

? ? ? bool ? ? ? ? ? ? ? ? ? ?dma_enabled; ? ?/* To specify DMA Mode */ ? ? ? unsigned int ? ? ? ? ? ?uartclk; ? ? ? ?/* UART clock rate */
- ? ? void __iomem ? ? ? ? ? ?*membase; ? ? ? /* ioremap cookie or NULL */
- ? ? resource_size_t ? ? ? ? mapbase; ? ? ? ?/* resource base */ ? ? ? upf_t ? ? ? ? ? ? ? ? ? flags; ? ? ? ? ?/* UPF_* flags */ +
+ ? ? u32 ? ? ? ? ? ? ? ? ? ? errata;
?};

?struct uart_omap_dma {

drivers/tty/serial/omap-serial.c +3 -2

index 5e713d3..6c2ea54 100644
@@ -1275,10 +1275,16 @@ static int serial_omap_probe(struct platform_device *pdev)
 ? ? ? up->port.ops = &serial_omap_pops;
 ? ? ? up->port.line = pdev->id;
 
- ? ? up->port.membase = omap_up_info->membase;
- ? ? up->port.mapbase = omap_up_info->mapbase;
+ ? ? up->port.mapbase = mem->start;
+ ? ? up->port.membase = ioremap(mem->start, resource_size(mem));
+

drivers/tty/serial/omap-serial.c static int serial_omap_probe(struct platform_device *pdev) · line 1289

Quoted 10 lines

drivers/tty/serial/omap-serial.c +6 -1

+ ? ? if (!up->port.membase) {
+ ? ? ? ? ? ? dev_err(&pdev->dev, "can't ioremap UART\n");
+ ? ? ? ? ? ? ret = -ENOMEM;
+ ? ? ? ? ? ? goto err;
+ ? ? }
+
 ? ? ? up->port.flags = omap_up_info->flags;
- ? ? up->port.irqflags = omap_up_info->irqflags;
 ? ? ? up->port.uartclk = omap_up_info->uartclk;
 ? ? ? up->uart_dma.uart_base = mem->start;

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

Prior to this change rx-pad wakeup was done by writing to rx-pad offset value populated in serial.c idle_init. Now with mux framework support we can use mux_utilities along with hmwod framework to handle io-pad configuration and enable rx-pad wake-up mechanism.

Quoted 4 lines · Govindraj.R

Prior to this change rx-pad wakeup was done by writing to rx-pad offset value populated in serial.c idle_init. Now with mux framework support we can use mux_utilities along with hmwod framework to handle io-pad configuration and enable rx-pad wake-up mechanism.

This patch (along with a few others) have compile errors when CONFIG_PM is not set.

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

In preparation to runtime conversion add missing uart regs to port structure which can be used in context restore. Also ensuring all uart reg info's are part of port structure.

Signed-off-by Govindraj.R <[email protected]>

Quoted 4 lines · Govindraj.R

In preparation to runtime conversion add missing uart regs to port structure which can be used in context restore. Also ensuring all uart reg info's are part of port structure.

Quoted 1 line

Signed-off-by Govindraj.R <[email protected]>

IMO, this should come later in the series to avoid adding bunch of code that gets moved in a subsequent patch.

First, convert to runtime PM (current patch 9/16) Then, add a patch to move context save/restore into driver. Then, add this patch.

Adapts omap-serial driver to use pm_runtime API's.

drivers/tty/serial/omap-serial.c +2 -3

@@ -1065,19 +1123,18 @@ static struct uart_driver serial_omap_reg = {
 	.cons		= OMAP_CONSOLE,
 };
 
-static int
-serial_omap_suspend(struct platform_device *pdev, pm_message_t state)
+static int serial_omap_suspend(struct device *dev)
 {
-	struct uart_omap_port *up = platform_get_drvdata(pdev);
+	struct uart_omap_port *up = dev_get_drvdata(dev);
 
 	if (up)
 		uart_suspend_port(&serial_omap_reg, &up->port);
 	return 0;

drivers/tty/serial/omap-serial.c +35 -0

+static int serial_omap_runtime_suspend(struct device *dev)
+{
+	struct uart_omap_port *up = dev_get_drvdata(dev);
+	struct omap_uart_port_info *pdata = dev->platform_data;
+
+	if (!up)
+		return -EINVAL;
+
+	if (!pdata->enable_wakeup || !pdata->get_context_loss_count)
+		return 0;
+
+	if (pdata->get_context_loss_count)
+		up->context_loss_cnt = pdata->get_context_loss_count(dev);
+
+	if (device_may_wakeup(dev)) {
+		if (!up->wakeups_enabled) {
+			pdata->enable_wakeup(up->pdev, true);
+			up->wakeups_enabled = true;
+		}
+	} else {
+		if (up->wakeups_enabled) {
+			pdata->enable_wakeup(up->pdev, false);
+			up->wakeups_enabled = false;
+		}
+	}
+
+	return 0;
+}
+
+static int serial_omap_runtime_resume(struct device *dev)
+{
+	struct uart_omap_port *up = dev_get_drvdata(dev);
+	struct omap_uart_port_info *pdata = dev->platform_data;
+
+	if (up) {

Quoted 1 line · Govindraj.R

Adapts omap-serial driver to use pm_runtime API's.

[...]

Quoted 14 lines

(quoted diff) +2 -3

@@ -1065,19 +1123,18 @@ static struct uart_driver serial_omap_reg = {
 ? ? ? .cons ? ? ? ? ? = OMAP_CONSOLE,
 ?};
 
-static int
-serial_omap_suspend(struct platform_device *pdev, pm_message_t state)
+static int serial_omap_suspend(struct device *dev)
 ?{
- ? ? struct uart_omap_port *up = platform_get_drvdata(pdev);
+ ? ? struct uart_omap_port *up = dev_get_drvdata(dev);
 
 ? ? ? if (up)
 ? ? ? ? ? ? ? uart_suspend_port(&serial_omap_reg, &up->port);
 ? ? ? return 0;

Review static struct uart_driver serial_omap_reg = { · line 1140

Quoted 10 lines

(quoted diff) +2 -2

 ?}
 
-static int serial_omap_resume(struct platform_device *dev)
+static int serial_omap_resume(struct device *dev)
 ?{
- ? ? struct uart_omap_port *up = platform_get_drvdata(dev);
+ ? ? struct uart_omap_port *up = dev_get_drvdata(dev);
 
 ? ? ? if (up)
 ? ? ? ? ? ? ? uart_resume_port(&serial_omap_reg, &up->port);

These functions need to be wrapped in #ifdef CONFIG_SUSPEND, otherwise, when building with !CONFIG_SUSPEND you'll get :

/work/kernel/omap/pm/drivers/tty/serial/omap-serial.c:1134:12: warning: 'serial_omap_suspend' defined but not used /work/kernel/omap/pm/drivers/tty/serial/omap-serial.c:1150:12: warning: 'serial_omap_resume' defined but not used

[...]

Quoted 43 lines

(quoted diff) +43 -0

+static int serial_omap_runtime_suspend(struct device *dev)
+{
+ ? ? struct uart_omap_port *up = dev_get_drvdata(dev);
+ ? ? struct omap_uart_port_info *pdata = dev->platform_data;
+
+ ? ? if (!up)
+ ? ? ? ? ? ? return -EINVAL;
+
+ ? ? if (!pdata->enable_wakeup || !pdata->get_context_loss_count)
+ ? ? ? ? ? ? return 0;
+
+ ? ? if (pdata->get_context_loss_count)
+ ? ? ? ? ? ? up->context_loss_cnt = pdata->get_context_loss_count(dev);
+
+ ? ? if (device_may_wakeup(dev)) {
+ ? ? ? ? ? ? if (!up->wakeups_enabled) {
+ ? ? ? ? ? ? ? ? ? ? pdata->enable_wakeup(up->pdev, true);
+ ? ? ? ? ? ? ? ? ? ? up->wakeups_enabled = true;
+ ? ? ? ? ? ? }
+ ? ? } else {
+ ? ? ? ? ? ? if (up->wakeups_enabled) {
+ ? ? ? ? ? ? ? ? ? ? pdata->enable_wakeup(up->pdev, false);
+ ? ? ? ? ? ? ? ? ? ? up->wakeups_enabled = false;
+ ? ? ? ? ? ? }
+ ? ? }
+
+ ? ? return 0;
+}
+
+static int serial_omap_runtime_resume(struct device *dev)
+{
+ ? ? struct uart_omap_port *up = dev_get_drvdata(dev);
+ ? ? struct omap_uart_port_info *pdata = dev->platform_data;
+
+ ? ? if (up) {
+ ? ? ? ? ? ? if (pdata->get_context_loss_count) {
+ ? ? ? ? ? ? ? ? ? ? u32 loss_cnt = pdata->get_context_loss_count(dev);
+
+ ? ? ? ? ? ? ? ? ? ? if (up->context_loss_cnt != loss_cnt)
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? serial_omap_restore_context(up);
+ ? ? ? ? ? ? }
+ ? ? }
+

Quoted 2 lines

? ? ? return 0;
?}

Similarily, thse need to be wrapped with #ifdef CONFIG_PM_RUNTIME, otherwise, when !CONFIG_PM_RUNTIME:

/work/kernel/omap/pm/drivers/tty/serial/omap-serial.c:1498:12: warning: 'serial_omap_runtime_suspend' defined but not used /work/kernel/omap/pm/drivers/tty/serial/omap-serial.c:1531:12: warning: 'serial_omap_runtime_resume' defined but not used

drivers/tty/serial/omap-serial.c static int serial_omap_remove(struct platform_device *dev) · line 1481

Quoted 6 lines

drivers/tty/serial/omap-serial.c +6 -0

+static const struct dev_pm_ops serial_omap_dev_pm_ops = {
+ ? ? SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume)
+ ? ? SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? serial_omap_runtime_resume, NULL)
+};
+

Note that you don't need #else parts to the above #ifdefs since the SET_*_OPS() macros used here take care of that.

Adapts omap-serial driver to use pm_runtime API's.

Use runtime runtime API's to handle uart clocks and obtain device_usage statics. Set runtime API's usage to irq_safe so that we can use get_sync from irq context. Auto-suspend for port specific activities and put for reg access. Use device_may_wakeup to check whether uart has wakeup capabilities and then enable uart runtime usage for the uart.

Removing save_context/restore_context functions from serial.c Adding context restore to .runtime_suspend and using reg values from port structure to restore the uart port context based on context_loss_count. Maintain internal state machine using wakeups_enabled field for avoiding repeated enable/disable of uart port wakeup mechanism.

Remove omap_uart_disable_wakeup and modify omap_uart_enable_wakeup to accept pdev and bool value to enable/disable the uart wakeup mechanism after uart clock's are cut.

omap_hwmod_enable_wakeup is used to set
pad wakeup for the uarts. PM_WKEN reg values are left to default. Removed omap_uart_enable/disable_clocks in serial.c now clock handling done with runtime API's.

By default uart autosuspend delay is set to -1 to avoid character loss if uart's are autoidled and woken up on rx pin.

After boot up UART's can be autoidled by setting autosuspendi delay from sysfs.

echo 3000 > /sys/devices/platform/omap/omap_uart.X/power/autosuspend_delay_ms X=0,1,2,3 for UART1/2/3/4. Number of uarts available may vary across omap_soc.

Acked-by Alan Cox <[email protected]>

Signed-off-by Govindraj.R <[email protected]>

Quoted 8 lines · Govindraj.R

Adapts omap-serial driver to use pm_runtime API's.

Use runtime runtime API's to handle uart clocks and obtain device_usage statics. Set runtime API's usage to irq_safe so that we can use get_sync from irq context. Auto-suspend for port specific activities and put for reg access. Use device_may_wakeup to check whether uart has wakeup capabilities and then enable uart runtime usage for the uart.

OK. ?Current patch should do only this part. ?The rest should be separate patches with their own descriptive changelogs.

Quoted 5 lines

Removing save_context/restore_context functions from serial.c Adding context restore to .runtime_suspend and using reg values from port structure to restore the uart port context based on context_loss_count. Maintain internal state machine using wakeups_enabled field for avoiding repeated enable/disable of uart port wakeup mechanism.

This part should be a separate patch that follows.

Quoted 8 lines

Remove omap_uart_disable_wakeup and modify omap_uart_enable_wakeup to accept pdev and bool value to enable/disable the uart wakeup mechanism after uart clock's are cut.

omap_hwmod_enable_wakeup is used to set
pad wakeup for the uarts. PM_WKEN reg values are left to default. Removed omap_uart_enable/disable_clocks in serial.c now clock handling done with runtime API's.

As stated in previous reviews, this wakeup enable/disable needs more description as the functionality is changing compared to current code.

Current version modifies wakeup enable/disable at both power-domain level (PM_WKEN) and at the IO ring.

Updated version modifies wakeups at module-level (SYSCONFIG) and at IO ring using omap_hwmod_enable_wakeup()

IMO, the updated version makes more sense, but needs a description as to why that change in functionality will have equivalent results compared to the existing one.

Quoted 2 lines

By default uart autosuspend delay is set to -1 to avoid character loss if uart's are autoidled and woken up on rx pin.

OK, good.

Quoted 7 lines

After boot up UART's can be autoidled by setting autosuspendi delay from sysfs.

echo 3000 > /sys/devices/platform/omap/omap_uart.X/power/autosuspend_delay_ms X=0,1,2,3 for UART1/2/3/4. Number of uarts available may vary across omap_soc.

Acked-by Alan Cox <[email protected]>

Signed-off-by Govindraj.R <[email protected]>

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

Use device_may_wakeup to check whether uart has wakeup capabilities and then enable uart runtime usage for the uart.