[PATCH 0/3] qcom smd big endian support

[PATCH 0/3] qcom smd big endian support

[Stephen Boyd]

This set of patches does some tidying in the beginning to prepare
for adding big endian CPU support to the smd code. It builds on a
previous patch to add big endian support to smem.

Stephen Boyd (3):
  soc: qcom: smd: Represent channel layout in structures
  soc: qcom: smd: Use __iowrite32_copy() instead of open-coding it
  soc: qcom: smd: Handle big endian CPUs

 drivers/soc/qcom/smd.c | 221 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 136 insertions(+), 85 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 1/3] soc: qcom: smd: Represent channel layout in structures

[Stephen Boyd]

The rx and tx channel info are laid out in memory next to each
other, and there are two types of channel info structures, byte
based and word based. We have 4 pointers to these info
structures, when we really only need two to point to the
different types of structures. Encapsulate the byte based and word
based tx/rx structures in a "full channel" structure that
describes the layout of memory and reduces the number of pointers
in the smd channel structure by two.

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

diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
index edd9d9a37238..c16547b85e05 100644
--- a/drivers/soc/qcom/smd.c
+++ b/drivers/soc/qcom/smd.c
@@ -65,7 +65,9 @@
  */
 
 struct smd_channel_info;
+struct smd_full_channel_info;
 struct smd_channel_info_word;
+struct smd_full_channel_info_word;
 
 #define SMD_ALLOC_TBL_COUNT	2
 #define SMD_ALLOC_TBL_SIZE	64
@@ -151,10 +153,8 @@ enum smd_channel_state {
  * @name:		name of the channel
  * @state:		local state of the channel
  * @remote_state:	remote state of the channel
- * @tx_info:		byte aligned outgoing channel info
- * @rx_info:		byte aligned incoming channel info
- * @tx_info_word:	word aligned outgoing channel info
- * @rx_info_word:	word aligned incoming channel info
+ * @info:		byte aligned outgoing/incoming channel info
+ * @info_word:		word aligned outgoing/incoming channel info
  * @tx_lock:		lock to make writes to the channel mutually exclusive
  * @fblockread_event:	wakeup event tied to tx fBLOCKREADINTR
  * @tx_fifo:		pointer to the outgoing ring buffer
@@ -175,11 +175,8 @@ struct qcom_smd_channel {
 	enum smd_channel_state state;
 	enum smd_channel_state remote_state;
 
-	struct smd_channel_info *tx_info;
-	struct smd_channel_info *rx_info;
-
-	struct smd_channel_info_word *tx_info_word;
-	struct smd_channel_info_word *rx_info_word;
+	struct smd_full_channel_info *info;
+	struct smd_full_channel_info_word *info_word;
 
 	struct mutex tx_lock;
 	wait_queue_head_t fblockread_event;
@@ -228,6 +225,11 @@ struct smd_channel_info {
 	u32 head;
 };
 
+struct smd_full_channel_info {
+	struct smd_channel_info tx;
+	struct smd_channel_info rx;
+};
+
 /*
  * Format of the smd_info smem items, for word aligned channels.
  */
@@ -245,25 +247,30 @@ struct smd_channel_info_word {
 	u32 head;
 };
 
+struct smd_full_channel_info_word {
+	struct smd_channel_info_word tx;
+	struct smd_channel_info_word rx;
+};
+
 #define GET_RX_CHANNEL_INFO(channel, param) \
-	(channel->rx_info_word ? \
-		channel->rx_info_word->param : \
-		channel->rx_info->param)
+	(channel->info_word ? \
+		channel->info_word->rx.param : \
+		channel->info->rx.param)
 
 #define SET_RX_CHANNEL_INFO(channel, param, value) \
-	(channel->rx_info_word ? \
-		(channel->rx_info_word->param = value) : \
-		(channel->rx_info->param = value))
+	(channel->info_word ? \
+		(channel->info_word->rx.param = value) : \
+		(channel->info->rx.param = value))
 
 #define GET_TX_CHANNEL_INFO(channel, param) \
-	(channel->tx_info_word ? \
-		channel->tx_info_word->param : \
-		channel->tx_info->param)
+	(channel->info_word ? \
+		channel->info_word->tx.param : \
+		channel->info->tx.param)
 
 #define SET_TX_CHANNEL_INFO(channel, param, value) \
-	(channel->tx_info_word ? \
-		(channel->tx_info_word->param = value) : \
-		(channel->tx_info->param = value))
+	(channel->info_word ? \
+		(channel->info_word->tx.param = value) : \
+		(channel->info->tx.param = value))
 
 /**
  * struct qcom_smd_alloc_entry - channel allocation entry
@@ -412,7 +419,7 @@ static size_t qcom_smd_channel_peek(struct qcom_smd_channel *channel,
 	unsigned tail;
 	size_t len;
 
-	word_aligned = channel->rx_info_word != NULL;
+	word_aligned = channel->info_word;
 	tail = GET_RX_CHANNEL_INFO(channel, tail);
 
 	len = min_t(size_t, count, channel->fifo_size - tail);
@@ -627,7 +634,7 @@ static int qcom_smd_write_fifo(struct qcom_smd_channel *channel,
 	unsigned head;
 	size_t len;
 
-	word_aligned = channel->tx_info_word != NULL;
+	word_aligned = channel->info_word;
 	head = GET_TX_CHANNEL_INFO(channel, head);
 
 	len = min_t(size_t, count, channel->fifo_size - head);
@@ -670,7 +677,7 @@ int qcom_smd_send(struct qcom_smd_channel *channel, const void *data, int len)
 	int ret;
 
 	/* Word aligned channels only accept word size aligned data */
-	if (channel->rx_info_word != NULL && len % 4)
+	if (channel->info_word && len % 4)
 		return -EINVAL;
 
 	ret = mutex_lock_interruptible(&channel->tx_lock);
@@ -988,11 +995,9 @@ static struct qcom_smd_channel *qcom_smd_create_channel(struct qcom_smd_edge *ed
 	 * use.
 	 */
 	if (info_size == 2 * sizeof(struct smd_channel_info_word)) {
-		channel->tx_info_word = info;
-		channel->rx_info_word = info + sizeof(struct smd_channel_info_word);
+		channel->info_word = info;
 	} else if (info_size == 2 * sizeof(struct smd_channel_info)) {
-		channel->tx_info = info;
-		channel->rx_info = info + sizeof(struct smd_channel_info);
+		channel->info = info;
 	} else {
 		dev_err(smd->dev,
 			"channel info of size %zu not supported\n", info_size);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 1/3] soc: qcom: smd: Represent channel layout in structures

[Bjorn Andersson]

On Mon 31 Aug 18:39 PDT 2015, Stephen Boyd wrote:

> The rx and tx channel info are laid out in memory next to each
> other, and there are two types of channel info structures, byte
> based and word based. We have 4 pointers to these info
> structures, when we really only need two to point to the
> different types of structures. Encapsulate the byte based and word
> based tx/rx structures in a "full channel" structure that
> describes the layout of memory and reduces the number of pointers
> in the smd channel structure by two.
> 

Saving the extra pointer doesn't feel like worth the change, but
representing the pair of info structs as one sounds like a reasonable
cleanup.

Reviewed-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/soc/qcom/smd.c | 61 +++++++++++++++++++++++++++-----------------------
>  1 file changed, 33 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
> index edd9d9a37238..c16547b85e05 100644
> --- a/drivers/soc/qcom/smd.c
> +++ b/drivers/soc/qcom/smd.c
> @@ -65,7 +65,9 @@
>   */
>  
>  struct smd_channel_info;
> +struct smd_full_channel_info;

I would have called this "smd_channel_info_pair"...

>  struct smd_channel_info_word;
> +struct smd_full_channel_info_word;
>  

Regards,
Bjorn

[PATCH 1/3] soc: qcom: smd: Represent channel layout in structures

[Stephen Boyd]

On 08/31/2015 09:55 PM, Bjorn Andersson wrote:
> On Mon 31 Aug 18:39 PDT 2015, Stephen Boyd wrote:
>
>> The rx and tx channel info are laid out in memory next to each
>> other, and there are two types of channel info structures, byte
>> based and word based. We have 4 pointers to these info
>> structures, when we really only need two to point to the
>> different types of structures. Encapsulate the byte based and word
>> based tx/rx structures in a "full channel" structure that
>> describes the layout of memory and reduces the number of pointers
>> in the smd channel structure by two.
>>
> Saving the extra pointer doesn't feel like worth the change, but
> representing the pair of info structs as one sounds like a reasonable
> cleanup.

Agreed.

>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>  drivers/soc/qcom/smd.c | 61 +++++++++++++++++++++++++++-----------------------
>>  1 file changed, 33 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
>> index edd9d9a37238..c16547b85e05 100644
>> --- a/drivers/soc/qcom/smd.c
>> +++ b/drivers/soc/qcom/smd.c
>> @@ -65,7 +65,9 @@
>>   */
>>  
>>  struct smd_channel_info;
>> +struct smd_full_channel_info;
> I would have called this "smd_channel_info_pair"...

Sure I'll make the change and add your Reviewed-by.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/3] soc: qcom: smd: Use __iowrite32_copy() instead of open-coding it

[Stephen Boyd]

We already have a function to do this and it silences some sparse
warnings along the way.

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

diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
index c16547b85e05..91b7f5668962 100644
--- a/drivers/soc/qcom/smd.c
+++ b/drivers/soc/qcom/smd.c
@@ -371,20 +371,15 @@ static void qcom_smd_channel_set_state(struct qcom_smd_channel *channel,
 /*
  * Copy count bytes of data using 32bit accesses, if that's required.
  */
-static void smd_copy_to_fifo(void __iomem *_dst,
-			     const void *_src,
+static void smd_copy_to_fifo(void __iomem *dst,
+			     const void *src,
 			     size_t count,
 			     bool word_aligned)
 {
-	u32 *dst = (u32 *)_dst;
-	u32 *src = (u32 *)_src;
-
 	if (word_aligned) {
-		count /= sizeof(u32);
-		while (count--)
-			writel_relaxed(*src++, dst++);
+		__iowrite32_copy(dst, src, count / sizeof(u32));
 	} else {
-		memcpy_toio(_dst, _src, count);
+		memcpy_toio(dst, src, count);
 	}
 }
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/3] soc: qcom: smd: Use __iowrite32_copy() instead of open-coding it

[Bjorn Andersson]

On Mon 31 Aug 18:39 PDT 2015, Stephen Boyd wrote:

> We already have a function to do this and it silences some sparse
> warnings along the way.
> 

Didn't know that, thanks. Do you know why there's no equivalent for
transfers in the other direction? Should we hack one up to do the same
simplification in smd_copy_from_fifo()?

Reviewed-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>

Regards,
Bjorn

[PATCH 2/3] soc: qcom: smd: Use __iowrite32_copy() instead of open-coding it

[Stephen Boyd]

On 08/31, Bjorn Andersson wrote:
> On Mon 31 Aug 18:39 PDT 2015, Stephen Boyd wrote:
> 
> > We already have a function to do this and it silences some sparse
> > warnings along the way.
> > 
> 
> Didn't know that, thanks. Do you know why there's no equivalent for
> transfers in the other direction? Should we hack one up to do the same
> simplification in smd_copy_from_fifo()?

Don't know. We can certainly write one although it would be nice
if we had two users. I'll take a look.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/3] soc: qcom: smd: Use __iowrite32_copy() instead of open-coding it

[Stephen Boyd]

On 09/01/2015 11:03 AM, Stephen Boyd wrote:
> On 08/31, Bjorn Andersson wrote:
>> On Mon 31 Aug 18:39 PDT 2015, Stephen Boyd wrote:
>>
>>> We already have a function to do this and it silences some sparse
>>> warnings along the way.
>>>
>> Didn't know that, thanks. Do you know why there's no equivalent for
>> transfers in the other direction? Should we hack one up to do the same
>> simplification in smd_copy_from_fifo()?
> Don't know. We can certainly write one although it would be nice
> if we had two users. I'll take a look.
>

Looks like we can convert smd and bcm47xx_nvram over. Also, we need to
add it or move the readl_relaxed() to a __raw_readl() in
smd_copy_from_fifo() because it's doing byte swapping that we don't want.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Stephen Boyd]

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(-)

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))
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Bjorn Andersson]

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

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

[Stephen Boyd]

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.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project