diff mbox

[3/3] soc: qcom: smd: Handle big endian CPUs

Message ID 1441071594-15941-4-git-send-email-sboyd@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stephen Boyd Sept. 1, 2015, 1:39 a.m. UTC 
The smd structures are always in little endian, but the smd
driver is not capable of being used on big endian CPUs. Annotate
the little endian data members and update the code to do the
proper byte swapping.

Cc: Bjorn Andersson <bjorn.andersson@sonymobile.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/soc/qcom/smd.c | 173 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 112 insertions(+), 61 deletions(-)

Comments

Bjorn Andersson Sept. 1, 2015, 5:06 a.m. UTC | #1 
On Mon 31 Aug 18:39 PDT 2015, Stephen Boyd wrote:

> The smd structures are always in little endian, but the smd
> driver is not capable of being used on big endian CPUs. Annotate
> the little endian data members and update the code to do the
> proper byte swapping.
> 

I think this looks good, but I think it would be better to reference
the 8-bit entries of the info struct as "flags" rather than INFO8s.

Could you please change the name of the macros to
{G,S}ET_{R,T}X_CHANNEL_FLAG() to make the code slightly easier to
follow?

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Sept. 1, 2015, 6:04 p.m. UTC | #2 
On 08/31, Bjorn Andersson wrote:
> On Mon 31 Aug 18:39 PDT 2015, Stephen Boyd wrote:
> 
> > The smd structures are always in little endian, but the smd
> > driver is not capable of being used on big endian CPUs. Annotate
> > the little endian data members and update the code to do the
> > proper byte swapping.
> > 
> 
> I think this looks good, but I think it would be better to reference
> the 8-bit entries of the info struct as "flags" rather than INFO8s.
> 
> Could you please change the name of the macros to
> {G,S}ET_{R,T}X_CHANNEL_FLAG() to make the code slightly easier to

Sure. I also found another missing __le32 marking on the "hdr"
stack structure. I'll test out some smd sending before resending
the patches.
diff mbox

Patch

diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
index 91b7f5668962..53642a874339 100644
--- a/drivers/soc/qcom/smd.c
+++ b/drivers/soc/qcom/smd.c
@@ -212,7 +212,7 @@  struct qcom_smd {
  * Format of the smd_info smem items, for byte aligned channels.
  */
 struct smd_channel_info {
-	u32 state;
+	__le32 state;
 	u8  fDSR;
 	u8  fCTS;
 	u8  fCD;
@@ -221,8 +221,8 @@  struct smd_channel_info {
 	u8  fTAIL;
 	u8  fSTATE;
 	u8  fBLOCKREADINTR;
-	u32 tail;
-	u32 head;
+	__le32 tail;
+	__le32 head;
 };
 
 struct smd_full_channel_info {
@@ -234,17 +234,17 @@  struct smd_full_channel_info {
  * Format of the smd_info smem items, for word aligned channels.
  */
 struct smd_channel_info_word {
-	u32 state;
-	u32 fDSR;
-	u32 fCTS;
-	u32 fCD;
-	u32 fRI;
-	u32 fHEAD;
-	u32 fTAIL;
-	u32 fSTATE;
-	u32 fBLOCKREADINTR;
-	u32 tail;
-	u32 head;
+	__le32 state;
+	__le32 fDSR;
+	__le32 fCTS;
+	__le32 fCD;
+	__le32 fRI;
+	__le32 fHEAD;
+	__le32 fTAIL;
+	__le32 fSTATE;
+	__le32 fBLOCKREADINTR;
+	__le32 tail;
+	__le32 head;
 };
 
 struct smd_full_channel_info_word {
@@ -252,25 +252,73 @@  struct smd_full_channel_info_word {
 	struct smd_channel_info_word rx;
 };
 
-#define GET_RX_CHANNEL_INFO(channel, param) \
-	(channel->info_word ? \
-		channel->info_word->rx.param : \
-		channel->info->rx.param)
-
-#define SET_RX_CHANNEL_INFO(channel, param, value) \
-	(channel->info_word ? \
-		(channel->info_word->rx.param = value) : \
-		(channel->info->rx.param = value))
-
-#define GET_TX_CHANNEL_INFO(channel, param) \
-	(channel->info_word ? \
-		channel->info_word->tx.param : \
-		channel->info->tx.param)
-
-#define SET_TX_CHANNEL_INFO(channel, param, value) \
-	(channel->info_word ? \
-		(channel->info_word->tx.param = value) : \
-		(channel->info->tx.param = value))
+#define GET_RX_CHANNEL_INFO8(channel, param)				     \
+	({								     \
+		BUILD_BUG_ON(sizeof(channel->info->rx.param) != sizeof(u8)); \
+		channel->info_word ?					     \
+			le32_to_cpu(channel->info_word->rx.param) :	     \
+			channel->info->rx.param;			     \
+	})
+
+#define GET_RX_CHANNEL_INFO(channel, param)				      \
+	({								      \
+		BUILD_BUG_ON(sizeof(channel->info->rx.param) != sizeof(u32)); \
+		le32_to_cpu(channel->info_word ?			      \
+			channel->info_word->rx.param :			      \
+			channel->info->rx.param);			      \
+	})
+
+#define SET_RX_CHANNEL_INFO8(channel, param, value)			     \
+	({								     \
+		BUILD_BUG_ON(sizeof(channel->info->rx.param) != sizeof(u8)); \
+		if (channel->info_word)					     \
+			channel->info_word->rx.param = cpu_to_le32(value);   \
+		else							     \
+			channel->info->rx.param = value;		     \
+	})
+
+#define SET_RX_CHANNEL_INFO(channel, param, value)			      \
+	({								      \
+		BUILD_BUG_ON(sizeof(channel->info->rx.param) != sizeof(u32)); \
+		if (channel->info_word)					      \
+			channel->info_word->rx.param = cpu_to_le32(value);   \
+		else							      \
+			channel->info->rx.param = cpu_to_le32(value);	      \
+	})
+
+#define GET_TX_CHANNEL_INFO8(channel, param)				     \
+	({								     \
+		BUILD_BUG_ON(sizeof(channel->info->tx.param) != sizeof(u8)); \
+		channel->info_word ?					     \
+			le32_to_cpu(channel->info_word->tx.param) :          \
+			channel->info->tx.param;			     \
+	})
+
+#define GET_TX_CHANNEL_INFO(channel, param)				      \
+	({								      \
+		BUILD_BUG_ON(sizeof(channel->info->tx.param) != sizeof(u32)); \
+		le32_to_cpu(channel->info_word ?			      \
+			channel->info_word->tx.param :			      \
+			channel->info->tx.param);			      \
+	})
+
+#define SET_TX_CHANNEL_INFO8(channel, param, value)			     \
+	({								     \
+		BUILD_BUG_ON(sizeof(channel->info->tx.param) != sizeof(u8)); \
+		if (channel->info_word)					     \
+			channel->info_word->tx.param = cpu_to_le32(value);   \
+		else							     \
+			channel->info->tx.param = value;		     \
+	})
+
+#define SET_TX_CHANNEL_INFO(channel, param, value)			      \
+	({								      \
+		BUILD_BUG_ON(sizeof(channel->info->tx.param) != sizeof(u32)); \
+		if (channel->info_word)					      \
+			channel->info_word->tx.param = cpu_to_le32(value);   \
+		else							      \
+			channel->info->tx.param = cpu_to_le32(value);	      \
+	})
 
 /**
  * struct qcom_smd_alloc_entry - channel allocation entry
@@ -281,9 +329,9 @@  struct smd_full_channel_info_word {
  */
 struct qcom_smd_alloc_entry {
 	u8 name[20];
-	u32 cid;
-	u32 flags;
-	u32 ref_count;
+	__le32 cid;
+	__le32 flags;
+	__le32 ref_count;
 } __packed;
 
 #define SMD_CHANNEL_FLAGS_EDGE_MASK	0xff
@@ -312,14 +360,14 @@  static void qcom_smd_signal_channel(struct qcom_smd_channel *channel)
 static void qcom_smd_channel_reset(struct qcom_smd_channel *channel)
 {
 	SET_TX_CHANNEL_INFO(channel, state, SMD_CHANNEL_CLOSED);
-	SET_TX_CHANNEL_INFO(channel, fDSR, 0);
-	SET_TX_CHANNEL_INFO(channel, fCTS, 0);
-	SET_TX_CHANNEL_INFO(channel, fCD, 0);
-	SET_TX_CHANNEL_INFO(channel, fRI, 0);
-	SET_TX_CHANNEL_INFO(channel, fHEAD, 0);
-	SET_TX_CHANNEL_INFO(channel, fTAIL, 0);
-	SET_TX_CHANNEL_INFO(channel, fSTATE, 1);
-	SET_TX_CHANNEL_INFO(channel, fBLOCKREADINTR, 0);
+	SET_TX_CHANNEL_INFO8(channel, fDSR, 0);
+	SET_TX_CHANNEL_INFO8(channel, fCTS, 0);
+	SET_TX_CHANNEL_INFO8(channel, fCD, 0);
+	SET_TX_CHANNEL_INFO8(channel, fRI, 0);
+	SET_TX_CHANNEL_INFO8(channel, fHEAD, 0);
+	SET_TX_CHANNEL_INFO8(channel, fTAIL, 0);
+	SET_TX_CHANNEL_INFO8(channel, fSTATE, 1);
+	SET_TX_CHANNEL_INFO8(channel, fBLOCKREADINTR, 0);
 	SET_TX_CHANNEL_INFO(channel, head, 0);
 	SET_TX_CHANNEL_INFO(channel, tail, 0);
 
@@ -357,12 +405,12 @@  static void qcom_smd_channel_set_state(struct qcom_smd_channel *channel,
 
 	dev_dbg(edge->smd->dev, "set_state(%s, %d)\n", channel->name, state);
 
-	SET_TX_CHANNEL_INFO(channel, fDSR, is_open);
-	SET_TX_CHANNEL_INFO(channel, fCTS, is_open);
-	SET_TX_CHANNEL_INFO(channel, fCD, is_open);
+	SET_TX_CHANNEL_INFO8(channel, fDSR, is_open);
+	SET_TX_CHANNEL_INFO8(channel, fCTS, is_open);
+	SET_TX_CHANNEL_INFO8(channel, fCD, is_open);
 
 	SET_TX_CHANNEL_INFO(channel, state, state);
-	SET_TX_CHANNEL_INFO(channel, fSTATE, 1);
+	SET_TX_CHANNEL_INFO8(channel, fSTATE, 1);
 
 	channel->state = state;
 	qcom_smd_signal_channel(channel);
@@ -504,10 +552,10 @@  static bool qcom_smd_channel_intr(struct qcom_smd_channel *channel)
 		need_state_scan = true;
 	}
 	/* Indicate that we have seen any state change */
-	SET_RX_CHANNEL_INFO(channel, fSTATE, 0);
+	SET_RX_CHANNEL_INFO8(channel, fSTATE, 0);
 
 	/* Signal waiting qcom_smd_send() about the interrupt */
-	if (!GET_TX_CHANNEL_INFO(channel, fBLOCKREADINTR))
+	if (!GET_TX_CHANNEL_INFO8(channel, fBLOCKREADINTR))
 		wake_up_interruptible(&channel->fblockread_event);
 
 	/* Don't consume any data until we've opened the channel */
@@ -515,7 +563,7 @@  static bool qcom_smd_channel_intr(struct qcom_smd_channel *channel)
 		goto out;
 
 	/* Indicate that we've seen the new data */
-	SET_RX_CHANNEL_INFO(channel, fHEAD, 0);
+	SET_RX_CHANNEL_INFO8(channel, fHEAD, 0);
 
 	/* Consume data */
 	for (;;) {
@@ -535,10 +583,10 @@  static bool qcom_smd_channel_intr(struct qcom_smd_channel *channel)
 	}
 
 	/* Indicate that we have seen and updated tail */
-	SET_RX_CHANNEL_INFO(channel, fTAIL, 1);
+	SET_RX_CHANNEL_INFO8(channel, fTAIL, 1);
 
 	/* Signal the remote that we've consumed the data (if requested) */
-	if (!GET_RX_CHANNEL_INFO(channel, fBLOCKREADINTR)) {
+	if (!GET_RX_CHANNEL_INFO8(channel, fBLOCKREADINTR)) {
 		/* Ensure ordering of channel info updates */
 		wmb();
 
@@ -685,7 +733,7 @@  int qcom_smd_send(struct qcom_smd_channel *channel, const void *data, int len)
 			goto out;
 		}
 
-		SET_TX_CHANNEL_INFO(channel, fBLOCKREADINTR, 1);
+		SET_TX_CHANNEL_INFO8(channel, fBLOCKREADINTR, 1);
 
 		ret = wait_event_interruptible(channel->fblockread_event,
 				       qcom_smd_get_tx_avail(channel) >= tlen ||
@@ -693,15 +741,15 @@  int qcom_smd_send(struct qcom_smd_channel *channel, const void *data, int len)
 		if (ret)
 			goto out;
 
-		SET_TX_CHANNEL_INFO(channel, fBLOCKREADINTR, 0);
+		SET_TX_CHANNEL_INFO8(channel, fBLOCKREADINTR, 0);
 	}
 
-	SET_TX_CHANNEL_INFO(channel, fTAIL, 0);
+	SET_TX_CHANNEL_INFO8(channel, fTAIL, 0);
 
 	qcom_smd_write_fifo(channel, hdr, sizeof(hdr));
 	qcom_smd_write_fifo(channel, data, len);
 
-	SET_TX_CHANNEL_INFO(channel, fHEAD, 1);
+	SET_TX_CHANNEL_INFO8(channel, fHEAD, 1);
 
 	/* Ensure ordering of channel info updates */
 	wmb();
@@ -1043,6 +1091,7 @@  static void qcom_discover_channels(struct qcom_smd_edge *edge)
 	int ret;
 	int tbl;
 	int i;
+	u32 eflags, cid;
 
 	for (tbl = 0; tbl < SMD_ALLOC_TBL_COUNT; tbl++) {
 		ret = qcom_smem_get(edge->remote_pid,
@@ -1054,6 +1103,7 @@  static void qcom_discover_channels(struct qcom_smd_edge *edge)
 
 		for (i = 0; i < SMD_ALLOC_TBL_SIZE; i++) {
 			entry = &alloc_tbl[i];
+			eflags = le32_to_cpu(entry->flags);
 			if (test_bit(i, edge->allocated[tbl]))
 				continue;
 
@@ -1063,14 +1113,15 @@  static void qcom_discover_channels(struct qcom_smd_edge *edge)
 			if (!entry->name[0])
 				continue;
 
-			if (!(entry->flags & SMD_CHANNEL_FLAGS_PACKET))
+			if (!(eflags & SMD_CHANNEL_FLAGS_PACKET))
 				continue;
 
-			if ((entry->flags & SMD_CHANNEL_FLAGS_EDGE_MASK) != edge->edge_id)
+			if ((eflags & SMD_CHANNEL_FLAGS_EDGE_MASK) != edge->edge_id)
 				continue;
 
-			info_id = smem_items[tbl].info_base_id + entry->cid;
-			fifo_id = smem_items[tbl].fifo_base_id + entry->cid;
+			cid = le32_to_cpu(entry->cid);
+			info_id = smem_items[tbl].info_base_id + cid;
+			fifo_id = smem_items[tbl].fifo_base_id + cid;
 
 			channel = qcom_smd_create_channel(edge, info_id, fifo_id, entry->name);
 			if (IS_ERR(channel))