Re: [PATCH v7 11/13] slimbus: qcom: Add Qualcomm Slimbus controller driver

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

 



Thanks for your review,

On 23/11/17 16:42, Jonathan Neuschäfer wrote:
Hello Srinivas and Charles,

On Thu, Nov 23, 2017 at 10:07:03AM +0000, Charles Keepax wrote:
On Wed, Nov 15, 2017 at 02:10:41PM +0000, srinivas.kandagatla@xxxxxxxxxx wrote:
From: Sagar Dharia <sdharia@xxxxxxxxxxxxxx>

This controller driver programs manager, interface, and framer
devices for Qualcomm's slimbus HW block.
Manager component currently implements logical address setting,
and messaging interface.
Interface device reports bus synchronization information, and framer
device clocks the bus from the time it's woken up, until clock-pause
is executed by the manager device.

Signed-off-by: Sagar Dharia <sdharia@xxxxxxxxxxxxxx>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
---
[...]
+static void qcom_slim_rxwq(struct work_struct *work)
+{
+	u8 buf[SLIM_MSGQ_BUF_LEN];
+	u8 mc, mt, len;
+	int i, ret;
+	struct qcom_slim_ctrl *ctrl = container_of(work, struct qcom_slim_ctrl,
+						 wd);
+
+	while ((slim_get_current_rxbuf(ctrl, buf)) != -ENODATA) {
+		len = SLIM_HEADER_GET_RL(buf[0]);
+		mt = SLIM_HEADER_GET_MT(buf[0]);
+		mc = SLIM_HEADER_GET_MC(buf[1]);
+		if (mt == SLIM_MSG_MT_CORE &&
+			mc == SLIM_MSG_MC_REPORT_PRESENT) {
+			u8 laddr;
+			struct slim_eaddr ea;
+			u8 e_addr[6];
+
+			for (i = 0; i < 6; i++)
+				e_addr[i] = buf[7-i];
+
+			ea.manf_id = (u16)(e_addr[5] << 8) | e_addr[4];
+			ea.prod_code = (u16)(e_addr[3] << 8) | e_addr[2];
+			ea.dev_index = e_addr[1];
+			ea.instance = e_addr[0];

If we are just bitshifting this out of the bytes does it really
make it much more clear to reverse the byte order first? Feels
like you might as well shift it out of buf directly.

In any case, there is a predefined function to make this code a little
nicer in <asm/byteorder.h>:

	le16_to_cpu(x):  Converts the 16-bit little endian value x to
	                 CPU-endian
	le16_to_cpup(p): Converts the 16-bit little endian value pointed
	                 to by p to CPU-endian

If you use le16_to_cpup, you need to cast your pointer to __le16 *:

	ea.manf_id = le16_to_cpup((__le16 *)&e_addr[4]);

It Looks like I can make use of these apis here, I will give this a go to cleanup this part of the code.

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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux