RE: [PATCH v4] mwifiex: Reduce endian conversion for REG Host Commands

2016-07-08 Thread Amitkumar Karwar 
> From: Prasun Maiti [mailto:prasunmait...@gmail.com]
> Sent: Monday, June 27, 2016 3:43 PM
> To: Amitkumar Karwar
> Cc: Nishant Sarmukadam; Linux Wireless; Linux Next; Linux Kernel; Kalle
> Valo
> Subject: [PATCH v4] mwifiex: Reduce endian conversion for REG Host
> Commands
> 
> For multiple REG Host Commands (e.g HostCmd_CMD_802_11_EEPROM_ACCESS,
> HostCmd_CMD_MAC_REG_ACCESS etc.) "cpu_to_leX"-converted values are saved
> to driver. So, "leX_to_cpu" conversion is required too many times
> afterwards in driver.
> 
> This patch reduces the endian: conversion without saving "cpu_to_leX"
> converted values in driver. This will convert endianness in prepare
> command and command response path.
> 
> Signed-off-by: Prasun Maiti 
> ---
> Changes in v3:
>   - Fixed the following warnings:
>   * sta_ioctl.c:1339:34: warning: comparison of distinct pointer
> types lacks a cast [enabled by default]
>   * sta_cmdresp.c:821:6: warning: comparison of distinct pointer
> types lacks a cast [enabled by default]
> 
> Changes in v4:
>   - Fixed the sparse compilation warnings:
>   * warning: cast from restricted __le32
>   * warning: cast from restricted __le16
> 
>  drivers/net/wireless/marvell/mwifiex/ioctl.h   | 10 +++---
>  drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 28 +++
> -
>  drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 37 +++
> ---
>  drivers/net/wireless/marvell/mwifiex/sta_ioctl.c   | 21 ++--
>  4 files changed, 46 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/ioctl.h
> b/drivers/net/wireless/marvell/mwifiex/ioctl.h
> index a5a48c1..4ce9330 100644
> --- a/drivers/net/wireless/marvell/mwifiex/ioctl.h
> +++ b/drivers/net/wireless/marvell/mwifiex/ioctl.h
> @@ -341,16 +341,16 @@ enum {
>  };
> 
>  struct mwifiex_ds_reg_rw {
> - __le32 type;
> - __le32 offset;
> - __le32 value;
> + u32 type;
> + u32 offset;
> + u32 value;
>  };
> 
>  #define MAX_EEPROM_DATA 256
> 
>  struct mwifiex_ds_read_eeprom {
> - __le16 offset;
> - __le16 byte_count;
> + u16 offset;
> + u16 byte_count;
>   u8 value[MAX_EEPROM_DATA];
>  };
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> index e436574..9df02ba 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> @@ -1130,9 +1130,8 @@ static int mwifiex_cmd_reg_access(struct
> host_cmd_ds_command *cmd,
>   cmd->size = cpu_to_le16(sizeof(*mac_reg) + S_DS_GEN);
>   mac_reg = >params.mac_reg;
>   mac_reg->action = cpu_to_le16(cmd_action);
> - mac_reg->offset =
> - cpu_to_le16((u16) le32_to_cpu(reg_rw->offset));
> - mac_reg->value = reg_rw->value;
> + mac_reg->offset = cpu_to_le16((u16) reg_rw->offset);
> + mac_reg->value = cpu_to_le32(reg_rw->value);
>   break;
>   }
>   case HostCmd_CMD_BBP_REG_ACCESS:
> @@ -1142,9 +1141,8 @@ static int mwifiex_cmd_reg_access(struct
> host_cmd_ds_command *cmd,
>   cmd->size = cpu_to_le16(sizeof(*bbp_reg) + S_DS_GEN);
>   bbp_reg = >params.bbp_reg;
>   bbp_reg->action = cpu_to_le16(cmd_action);
> - bbp_reg->offset =
> - cpu_to_le16((u16) le32_to_cpu(reg_rw->offset));
> - bbp_reg->value = (u8) le32_to_cpu(reg_rw->value);
> + bbp_reg->offset = cpu_to_le16((u16) reg_rw->offset);
> + bbp_reg->value = (u8) reg_rw->value;
>   break;
>   }
>   case HostCmd_CMD_RF_REG_ACCESS:
> @@ -1154,8 +1152,8 @@ static int mwifiex_cmd_reg_access(struct
> host_cmd_ds_command *cmd,
>   cmd->size = cpu_to_le16(sizeof(*rf_reg) + S_DS_GEN);
>   rf_reg = >params.rf_reg;
>   rf_reg->action = cpu_to_le16(cmd_action);
> - rf_reg->offset = cpu_to_le16((u16) le32_to_cpu(reg_rw-
> >offset));
> - rf_reg->value = (u8) le32_to_cpu(reg_rw->value);
> + rf_reg->offset = cpu_to_le16((u16) reg_rw->offset);
> + rf_reg->value = (u8) reg_rw->value;
>   break;
>   }
>   case HostCmd_CMD_PMIC_REG_ACCESS:
> @@ -1165,9 +1163,8 @@ static int mwifiex_cmd_reg_access(struct
> host_cmd_ds_command *cmd,
>   cmd->size = cpu_to_le16(sizeof(*pmic_reg) + S_DS_GEN);
>   pmic_reg = >params.pmic_reg;
>   pmic_reg->action = cpu_to_le16(cmd_action);
> - pmic_reg->offset =
> - cpu_to_le16((u16) le32_to_cpu(reg_rw->offset));
> - pmic_reg->value = (u8) le32_to_cpu(reg_rw->value);
> + pmic_reg->offset = cpu_to_le16((u16) reg_rw->offset);
> + pmic_reg->value = (u8) reg_rw->value;
>   break;
>   }
>   case HostCmd_CMD_CAU_REG_ACCESS:
> @@

RE: [PATCH v4] mwifiex: Reduce endian conversion for REG Host Commands

2016-07-08 Thread Amitkumar Karwar 
> From: linux-wireless-ow...@vger.kernel.org [mailto:linux-wireless-
> ow...@vger.kernel.org] On Behalf Of Kalle Valo
> Sent: Friday, July 08, 2016 7:10 PM
> To: Amitkumar Karwar
> Cc: Prasun Maiti; Nishant Sarmukadam; Linux Wireless; Linux Next; Linux
> Kernel
> Subject: Re: [PATCH v4] mwifiex: Reduce endian conversion for REG Host
> Commands
> 
> Amitkumar Karwar <akar...@marvell.com> writes:
> 
> >> From: Prasun Maiti [mailto:prasunmait...@gmail.com]
> >> Sent: Monday, June 27, 2016 3:43 PM
> >> To: Amitkumar Karwar
> >> Cc: Nishant Sarmukadam; Linux Wireless; Linux Next; Linux Kernel;
> >> Kalle Valo
> >> Subject: [PATCH v4] mwifiex: Reduce endian conversion for REG Host
> >> Commands
> >>
> >> For multiple REG Host Commands (e.g HostCmd_CMD_802_11_EEPROM_ACCESS,
> >> HostCmd_CMD_MAC_REG_ACCESS etc.) "cpu_to_leX"-converted values are
> >> saved to driver. So, "leX_to_cpu" conversion is required too many
> >> times afterwards in driver.
> >>
> >> This patch reduces the endian: conversion without saving "cpu_to_leX"
> >> converted values in driver. This will convert endianness in prepare
> >> command and command response path.
> >>
> >> Signed-off-by: Prasun Maiti <prasunmait...@gmail.com>
> 
> [deleted almost 300 lines of unnecessary quotes]
> 
> > Any specific reason for dropping this patch?
> > I can see the status as "Deferred" on patchwork.
> 
> It's not dropped, "Deferred" means that the patch was postponed for some
> reason and I need to look at the patch more carefully. In this case I
> did it because I was hoping to see an Acked-by from you.

Thanks for clarification. I will ack the patch.

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

Re: [PATCH v4] mwifiex: Reduce endian conversion for REG Host Commands

2016-07-08 Thread Kalle Valo 
Amitkumar Karwar  writes:

>> From: Prasun Maiti [mailto:prasunmait...@gmail.com]
>> Sent: Monday, June 27, 2016 3:43 PM
>> To: Amitkumar Karwar
>> Cc: Nishant Sarmukadam; Linux Wireless; Linux Next; Linux Kernel; Kalle
>> Valo
>> Subject: [PATCH v4] mwifiex: Reduce endian conversion for REG Host
>> Commands
>> 
>> For multiple REG Host Commands (e.g HostCmd_CMD_802_11_EEPROM_ACCESS,
>> HostCmd_CMD_MAC_REG_ACCESS etc.) "cpu_to_leX"-converted values are saved
>> to driver. So, "leX_to_cpu" conversion is required too many times
>> afterwards in driver.
>> 
>> This patch reduces the endian: conversion without saving "cpu_to_leX"
>> converted values in driver. This will convert endianness in prepare
>> command and command response path.
>> 
>> Signed-off-by: Prasun Maiti 

[deleted almost 300 lines of unnecessary quotes]

> Any specific reason for dropping this patch?
> I can see the status as "Deferred" on patchwork.

It's not dropped, "Deferred" means that the patch was postponed for some
reason and I need to look at the patch more carefully. In this case I
did it because I was hoping to see an Acked-by from you.

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

RE: [PATCH v4] mwifiex: Reduce endian conversion for REG Host Commands

2016-07-08 Thread Amitkumar Karwar 
Hi Kalle,

> From: Prasun Maiti [mailto:prasunmait...@gmail.com]
> Sent: Monday, June 27, 2016 3:43 PM
> To: Amitkumar Karwar
> Cc: Nishant Sarmukadam; Linux Wireless; Linux Next; Linux Kernel; Kalle
> Valo
> Subject: [PATCH v4] mwifiex: Reduce endian conversion for REG Host
> Commands
> 
> For multiple REG Host Commands (e.g HostCmd_CMD_802_11_EEPROM_ACCESS,
> HostCmd_CMD_MAC_REG_ACCESS etc.) "cpu_to_leX"-converted values are saved
> to driver. So, "leX_to_cpu" conversion is required too many times
> afterwards in driver.
> 
> This patch reduces the endian: conversion without saving "cpu_to_leX"
> converted values in driver. This will convert endianness in prepare
> command and command response path.
> 
> Signed-off-by: Prasun Maiti 
> ---
> Changes in v3:
>   - Fixed the following warnings:
>   * sta_ioctl.c:1339:34: warning: comparison of distinct pointer
> types lacks a cast [enabled by default]
>   * sta_cmdresp.c:821:6: warning: comparison of distinct pointer
> types lacks a cast [enabled by default]
> 
> Changes in v4:
>   - Fixed the sparse compilation warnings:
>   * warning: cast from restricted __le32
>   * warning: cast from restricted __le16
> 
>  drivers/net/wireless/marvell/mwifiex/ioctl.h   | 10 +++---
>  drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 28 +++
> -
>  drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 37 +++
> ---
>  drivers/net/wireless/marvell/mwifiex/sta_ioctl.c   | 21 ++--
>  4 files changed, 46 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/ioctl.h
> b/drivers/net/wireless/marvell/mwifiex/ioctl.h
> index a5a48c1..4ce9330 100644
> --- a/drivers/net/wireless/marvell/mwifiex/ioctl.h
> +++ b/drivers/net/wireless/marvell/mwifiex/ioctl.h
> @@ -341,16 +341,16 @@ enum {
>  };
> 
>  struct mwifiex_ds_reg_rw {
> - __le32 type;
> - __le32 offset;
> - __le32 value;
> + u32 type;
> + u32 offset;
> + u32 value;
>  };
> 
>  #define MAX_EEPROM_DATA 256
> 
>  struct mwifiex_ds_read_eeprom {
> - __le16 offset;
> - __le16 byte_count;
> + u16 offset;
> + u16 byte_count;
>   u8 value[MAX_EEPROM_DATA];
>  };
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> index e436574..9df02ba 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> @@ -1130,9 +1130,8 @@ static int mwifiex_cmd_reg_access(struct
> host_cmd_ds_command *cmd,
>   cmd->size = cpu_to_le16(sizeof(*mac_reg) + S_DS_GEN);
>   mac_reg = >params.mac_reg;
>   mac_reg->action = cpu_to_le16(cmd_action);
> - mac_reg->offset =
> - cpu_to_le16((u16) le32_to_cpu(reg_rw->offset));
> - mac_reg->value = reg_rw->value;
> + mac_reg->offset = cpu_to_le16((u16) reg_rw->offset);
> + mac_reg->value = cpu_to_le32(reg_rw->value);
>   break;
>   }
>   case HostCmd_CMD_BBP_REG_ACCESS:
> @@ -1142,9 +1141,8 @@ static int mwifiex_cmd_reg_access(struct
> host_cmd_ds_command *cmd,
>   cmd->size = cpu_to_le16(sizeof(*bbp_reg) + S_DS_GEN);
>   bbp_reg = >params.bbp_reg;
>   bbp_reg->action = cpu_to_le16(cmd_action);
> - bbp_reg->offset =
> - cpu_to_le16((u16) le32_to_cpu(reg_rw->offset));
> - bbp_reg->value = (u8) le32_to_cpu(reg_rw->value);
> + bbp_reg->offset = cpu_to_le16((u16) reg_rw->offset);
> + bbp_reg->value = (u8) reg_rw->value;
>   break;
>   }
>   case HostCmd_CMD_RF_REG_ACCESS:
> @@ -1154,8 +1152,8 @@ static int mwifiex_cmd_reg_access(struct
> host_cmd_ds_command *cmd,
>   cmd->size = cpu_to_le16(sizeof(*rf_reg) + S_DS_GEN);
>   rf_reg = >params.rf_reg;
>   rf_reg->action = cpu_to_le16(cmd_action);
> - rf_reg->offset = cpu_to_le16((u16) le32_to_cpu(reg_rw-
> >offset));
> - rf_reg->value = (u8) le32_to_cpu(reg_rw->value);
> + rf_reg->offset = cpu_to_le16((u16) reg_rw->offset);
> + rf_reg->value = (u8) reg_rw->value;
>   break;
>   }
>   case HostCmd_CMD_PMIC_REG_ACCESS:
> @@ -1165,9 +1163,8 @@ static int mwifiex_cmd_reg_access(struct
> host_cmd_ds_command *cmd,
>   cmd->size = cpu_to_le16(sizeof(*pmic_reg) + S_DS_GEN);
>   pmic_reg = >params.pmic_reg;
>   pmic_reg->action = cpu_to_le16(cmd_action);
> - pmic_reg->offset =
> - cpu_to_le16((u16) le32_to_cpu(reg_rw->offset));
> - pmic_reg->value = (u8) le32_to_cpu(reg_rw->value);
> + pmic_reg->offset = cpu_to_le16((u16) reg_rw->offset);
> + pmic_reg->value = (u8) reg_rw->value;
>   break;
>   }
>   case