Re: [PATCH 2/6] rsi: use enum for FSM states

[amit karwar]

On Tue, Jun 13, 2017 at 12:17 PM, Kalle Valo <kv...@codeaurora.org> wrote:
> Amitkumar Karwar <amitkar...@gmail.com> writes:
>
>> Currently macros are used for FSM states. We will replace
>> it with enum so that new state can be added easily without
>> worrying about macro value.
>>
>> Signed-off-by: Amitkumar Karwar <amit.kar...@redpinesignals.com>
>
> From and Signed-off-by does not match (gmail.com vs redpinesignals.com).
> I suggest that you add a separate From line to the commit log containing
> redpinesignals.com, this way you can still submit patches via gmail.com.

Sure. I will follow this for my future patches.

>
> Also your name in patchwork is not capitalised (patchwork is idiotic and
> takes the name from patchwork user data and not from the patch):
>
>   [1/6] rsi: add usb RS9113 chipset support2017-06-02 
> amit karwar  New
>   [2/6] rsi: use enum for FSM states   2017-06-02 
> amit karwar  New
>   [3/6] rsi: Register interrupt handler before firmware load   2017-06-02 
> amit karwar  New
>   [4/6] rsi: receive path enhancement for RS9113   2017-06-02 
> amit karwar  New
>   [5/6] rsi: configure new boot parameters to device   2017-06-02 
> amit karwar  New
>   [6/6] rsi: add tx frame for common device configuration  2017-06-02 
> amit karwar  New
>
> To fix that could you please register to patchwork and capitalise your
> name correctly? You have only one chance, patchwork doesn't allow users
> edit the name afterwards, so don't do any typos :)

Thanks. I have registered with patchwork and corrected my name.

Regards,
Amitkumar Karwar

Re: git repo with Redpine changes

[amit karwar]

On Thu, Jun 1, 2017 at 7:49 PM, Alexey Brodkin
<alexey.brod...@synopsys.com> wrote:
> Hi Amit!
>
> On Thu, 2017-06-01 at 19:24 +0530, amit karwar wrote:
>> On Tue, May 23, 2017 at 10:56 PM, Alexey Brodkin
>> <alexey.brod...@synopsys.com> wrote:
>>
>> I have posted a firmware image for new firmware loading mechanism to
>> community. There are some patches for SDIO specific driver
>> enhancements and fixes. I am sending them in couple of days. With
>> those patches you will be able to see wireless interface bring up,
>> connection in open network and data path working.
>
> May I have any reference to that post/image?

Here is the link:
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/commit/?id=37857004a430e96dc837db7f967b6d0279053de8

Regards,
Amitkumar Karwar

Re: git repo with Redpine changes

[amit karwar]

On Tue, May 23, 2017 at 10:56 PM, Alexey Brodkin
<alexey.brod...@synopsys.com> wrote:
> Hi Amit!
>
> On Tue, 2017-05-23 at 17:00 +0530, amit karwar wrote:
>> On Wed, May 17, 2017 at 1:41 PM, Alexey Brodkin
>> <alexey.brod...@synopsys.com> wrote:
>> >
>> > Hello Amitkumar, Prameela,
>> >
>> > I was lucky enough to get hold of RS9113 Evaluation Kit and now I'm trying
>> > to get it working with my board via USB and SDIO interfaces but seeing 
>> > issues here and there:
>> >  1) In case of SDIO I see this on interface up with vanilla 4.11.1 kernel:
>> > --->8
>> > # ifconfig wlan0 up
>> > rsi_91x: Sending RX filter frame
>> > rsi_91x: rsi_core_qos_processor: Queue number = 4
>> > rsi_91x: rsi_sdio_host_intf_write_pkt: Successfully written onto card
>> > rsi_91x: rsi_set_vap_capabilities: Sending VAP capabilities frame
>> > rsi_91x: rsi_mac80211_conf_tx: Conf queue 0, aifs: 2, cwmin: 15 cwmax: 
>> > 1023, txop: 0
>> > rsi_91x: rsi_mac80211_conf_tx: Conf queue 1, aifs: 2, cwmin: 15 cwmax: 
>> > 1023, txop: 0
>> > rsi_91x: rsi_mac80211_conf_tx: Conf queue 2, aifs: 2, cwmin: 15 cwmax: 
>> > 1023, txop: 0
>> > # rsi_91x: rsi_mac80211_conf_tx: Conf queue 3, aifs: 2, cwmin: 15 cwmax: 
>> > 1023, txop: 0
>> > rsi_91x: rsi_channel_change: Set channel: 2412 MHz type: 416 channel_no 1
>> > rsi_91x: rsi_set_channel: Sending scan req frame
>> > rsi_91x: rsi_mac80211_config: Configuring Power
>> > rsi_91x: rsi_config_power: Set tx power: 20 dBM
>> > rsi_91x: rsi_send_radio_params_update: Sending Radio Params update frame
>> > rsi_91x: rsi_core_qos_processor: Queue number = 4
>> > rsi_91x: rsi_interrupt_handler: Intr_status = 8 8 4
>> > rsi_91x: Pkt pending interrupt
>> > rsi_91x: rsi_mgmt_pkt_recv: Msg Len: 0, Msg Type:1
>> > rsi_91x: rsi_handle_ta_confirm_type: Invalid TA confirm pkt received
>> > rsi_91x: rsi_sdio_host_intf_write_pkt: Successfully written onto card
>> > rsi_91x: rsi_core_qos_processor: Queue number = 4
>> > rsi_91x: rsi_sdio_host_intf_write_pkt: Successfully written onto card
>> > rsi_91x: rsi_core_qos_processor: Queue number = 4
>> > rsi_91x: rsi_interrupt_handler: Intr_status = 8 8 4
>> > rsi_91x: Pkt pending interrupt
>> > rsi_91x: rsi_mgmt_pkt_recv: Msg Len: 0, Msg Type:1
>> > rsi_91x: rsi_handle_ta_confirm_type: Invalid TA confirm pkt received
>> > rsi_91x: rsi_interrupt_handler: Intr_status = 8 8 4
>> > rsi_91x: Pkt pending interrupt
>> > rsi_91x: rsi_mgmt_pkt_recv: Msg Len: 321, Msg Type:2
>> > rsi_91x: rsi_sdio_host_intf_write_pkt: Successfully written onto card
>> > rsi_91x: rsi_core_qos_processor: Queue number = 255
>> > rsi_91x: rsi_core_qos_processor: No More Pkt
>> > rsi_91x: rsi_interrupt_handler: Intr_status = c 8 4
>> > rsi_91x: Pkt pending interrupt
>> > rsi_91x: rsi_mgmt_pkt_recv: Msg Len: 299, Msg Type:2
>> > rsi_91x: rsi_interrupt_handler: ==> FIRMWARE Assert <==
>
> Any hint on what's wrong here?
>
> Note firmware was successfully loaded on probe a bit earlier.
> And here I used a firmware blob from linux-firmware git repo,
> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/rsi_91x.fw
>
>> > rsi_91x: rsi_interrupt_handler: Firmware Status is 0x65
>> > rsi_91x: rsi_core_qos_processor: Queue number = 255
>> > rsi_91x: rsi_core_qos_processor: No More Pkt
>> > --->8
>> >
>> > and you recent patch series for nwe firmware loading I see something a bit
>> > different:
>> > --->8
>> > mmc_host mmc0: Bus speed (slot 0) = 5000Hz (slot req 2500Hz, 
>> > actual 2500HZ div = 1)
>> > mmc0: new SDIO card at address fffd
>> > rsi_91x: rsi_probe: Init function called
>> > rsi_91x: rsi_init_sdio_interface: Enabled the interface
>> > mmc_host mmc0: Bus speed (slot 0) = 5000Hz (slot req 5000Hz, 
>> > actual 5000HZ div = 0)
>> > rsi_91x: rsi_setblocklength: Setting the block length
>> > rsi_91x: rsi_setblocklength: Operational blk length is 256
>> > rsi_91x: rsi_init_sdio_interface: Setup card succesfully
>> > rsi_91x: rsi_init_sdio_slave_regs: Initialzing SDIO read start level
>> > rsi_91x: rsi_init_sdio_slave_regs: Initialzing FIFO ctrl registers
>> > rsi_91x: rsi_sdio_master_access_msword: MASTER_ACCESS_MSBYTE:0x5
>

Re: [v3 10/11] rsi: Add new firmware loading method

[amit karwar]

On Wed, May 24, 2017 at 1:57 PM, Kalle Valo  wrote:
> Amitkumar Karwar  writes:
>
>> From: Prameela Rani Garnepudi 
>>
>> The older firmware loading method has been deprecated and not in use
>> for any chipets. New method is introduced which works based on soft
>> boot loader. In this method, complete RAM image and FLASH image are
>> present in the flash. Before loading the functional firmware, host
>> issues boot loader commands to verify whether firmware to load is
>> different from the current functional firmware. If not, firmware
>> upgrade progresses and boot loader will switch to the new functional
>> firmware.
>>
>> "rs9113_wlan_qspi.rps" is the firmware filename used in this patch.
>
> I don't know if you saw the discussion with the wil6210 firmware, but
> the linux-firmware maintainers prefer to have a subdirectory for
> firmware files. I'm planning to apply this series now, but you can send
> a followup patch ASAP which changes the rsi driver to use a subdirectory
> for the firmware files?

I have just sent a follow up patch.

> Normally these kind of breaks in user space interface are not allowed,
> and instead the requirement is that the driver has a transition period
> from the old firmware interface to the new one. But I'm making an
> exception here as, to my knowledge, the upstream rsi driver is not that
> widely used and I want to get the driver development going on again.
>

Right. Driver is not currently much used.
Thanks for accepting the changes.

Regards,
Amitkumar Karwar

Re: [v2 10/11] rsi: Add new firmware loading method

[amit karwar]

On Fri, May 12, 2017 at 2:15 PM, Kalle Valo  wrote:
> Amitkumar Karwar  writes:
>
>> From: Prameela Rani Garnepudi 
>>
>> The older firmware loading method has been deprecated and not in use
>> for any chipets. New method is introduced which works based on soft
>> boot loader. In this method, complete RAM image and FLASH image are
>> present in the flash. Before loading the functional firmware, host
>> issues boot loader commands to verify whether firmware to load is
>> different from the current functional firmware. If not, firmware
>> upgrade progresses and boot loader will switch to the new functional
>> firmware.
>>
>> Signed-off-by: Prameela Rani Garnepudi 
>> Signed-off-by: Amitkumar Karwar 
>
> Please document in the commit log the new firmware filenames used by
> this patch.
>

Sure. This has been taken care in updated version.

Regards,
Amitkumar

Re: [v2 07/11] rsi: Add usb multi-byte read operation

[amit karwar]

On Fri, May 12, 2017 at 12:05 AM, Kalle Valo  wrote:
> Amitkumar Karwar  writes:
>
>> From: Prameela Rani Garnepudi 
>>
>> USB multibyte read will be used in the new firmware loading method
>> for RS9113 chipset.
>>
>> Signed-off-by: Prameela Rani Garnepudi 
>> Signed-off-by: Amitkumar Karwar 
>
> [...]
>
>> +static int rsi_usb_read_register_multiple(struct rsi_hw *adapter, u32 addr,
>> +   u8 *data, u16 count)
>> +{
>> + struct rsi_91x_usbdev *dev = (struct rsi_91x_usbdev *)adapter->rsi_dev;
>> + u8 *buf;
>> + u16 transfer;
>> + int status;
>> +
>> + if (!addr)
>> + return -EINVAL;
>> +
>> + buf = kzalloc(4096, GFP_KERNEL);
>> + if (!buf)
>> + return -ENOMEM;
>> +
>> + while (count) {
>> + transfer = min_t(u16, count, 4096);
>
> A minor thing, no need to resend just because of this. But a define for
> 4096 would be nice.
>

I have defined a macro for 4096 in v3 series.

Regards,
Amitkumar Karwar

Re: [v2 05/11] rsi: Remove unnecessary buffer allocation

[amit karwar]

On Fri, May 12, 2017 at 1:58 PM, Arend van Spriel
 wrote:
> On 5/11/2017 8:28 PM, Kalle Valo wrote:
>>
>> Amitkumar Karwar  writes:
>>
>>> From: Prameela Rani Garnepudi 
>>>
>>> In functions usb read register and usb write register, dynamic allocation
>>> of 4 bytes is used. This is removed as it is unncessary for local
>>> variable
>>> and for such small data.
>>>
>>> Signed-off-by: Prameela Rani Garnepudi 
>>> Signed-off-by: Amitkumar Karwar 
>>> ---
>>>   drivers/net/wireless/rsi/rsi_91x_usb.c | 18 --
>>>   1 file changed, 4 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c
>>> b/drivers/net/wireless/rsi/rsi_91x_usb.c
>>> index 73b01a8..8eb7407 100644
>>> --- a/drivers/net/wireless/rsi/rsi_91x_usb.c
>>> +++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
>>> @@ -157,12 +157,8 @@ static int rsi_usb_reg_read(struct usb_device
>>> *usbdev,
>>> u16 *value,
>>> u16 len)
>>>   {
>>> -   u8 *buf;
>>> -   int status = -ENOMEM;
>>> -
>>> -   buf  = kmalloc(0x04, GFP_KERNEL);
>>> -   if (!buf)
>>> -   return status;
>>> +   u8 buf[4];
>>> +   int status;
>>> status = usb_control_msg(usbdev,
>>>  usb_rcvctrlpipe(usbdev, 0),
>>
>>
>> Recently I got a patch to orinoco_usb which did exactly the opposite
>> (unless I'm missing something):
>>
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git/commit/?id=2f6ae79cb04bb7f9b4be3f1c32b6fda35bf976bc
>>
>> The documentation for usb_control_msg() does not mention anything if
>> it's possible to use stack memory, but AFAIU it's not possible to use
>> stack memory with DMA. Can anyone clarify?
>
>
> After private message I sent to Kalle here my public response :-p
> According to Greg this has been a USB core requirement for a long, long time
> (see below).
>

Thanks Arend.
I have dropped this patch in V3 series.

Regards,
Amitkumar Karwar

Re: [PATCH 1/3] rsi: Rename the file rsi_91x_pkt.c to rsi_91x_hal.c

[amit karwar]

Hi Kvalle,

On Tue, Apr 4, 2017 at 10:30 AM, Prameela Rani Garnepudi
 wrote:
> The file rsi_91x_hal.c will contain new firmware loading method for
> RSI 9113 chipset. So this file will have device specific operations.
> As the file 'rsi_91x_pkt.c' contains code for preparing device
> (frimware understandable) specific descriptors for the transmit frames,
> it is moved to 'rsi_91x_hal.c'
>
> Signed-off-by: Prameela Rani Garnepudi 
> ---
>  drivers/net/wireless/rsi/Makefile  |   2 +-
>  drivers/net/wireless/rsi/rsi_91x_hal.c | 215 
> +
>  drivers/net/wireless/rsi/rsi_91x_pkt.c | 215 
> -
>  3 files changed, 216 insertions(+), 216 deletions(-)
>  create mode 100644 drivers/net/wireless/rsi/rsi_91x_hal.c
>  delete mode 100644 drivers/net/wireless/rsi/rsi_91x_pkt.c
>
> diff --git a/drivers/net/wireless/rsi/Makefile 
> b/drivers/net/wireless/rsi/Makefile
> index 25828b6..a475c81 100644
> --- a/drivers/net/wireless/rsi/Makefile
> +++ b/drivers/net/wireless/rsi/Makefile
> @@ -2,7 +2,7 @@ rsi_91x-y   += rsi_91x_main.o
>  rsi_91x-y  += rsi_91x_core.o
>  rsi_91x-y  += rsi_91x_mac80211.o
>  rsi_91x-y  += rsi_91x_mgmt.o
> -rsi_91x-y  += rsi_91x_pkt.o
> +rsi_91x-y  += rsi_91x_hal.o
>  rsi_91x-$(CONFIG_RSI_DEBUGFS)  += rsi_91x_debugfs.o
>
>  rsi_usb-y  += rsi_91x_usb.o rsi_91x_usb_ops.o
> diff --git a/drivers/net/wireless/rsi/rsi_91x_hal.c 
> b/drivers/net/wireless/rsi/rsi_91x_hal.c
> new file mode 100644
> index 000..02920c9
> --- /dev/null
> +++ b/drivers/net/wireless/rsi/rsi_91x_hal.c
> @@ -0,0 +1,215 @@
> +/**
> + * Copyright (c) 2014 Redpine Signals Inc.
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include "rsi_mgmt.h"
> +
> +/**
> + * rsi_send_data_pkt() - This function sends the recieved data packet from
> + *  driver to device.
> + * @common: Pointer to the driver private structure.
> + * @skb: Pointer to the socket buffer structure.
> + *
> + * Return: status: 0 on success, -1 on failure.
> + */
> +int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
> +{
> +   struct rsi_hw *adapter = common->priv;
> +   struct ieee80211_hdr *tmp_hdr;
> +   struct ieee80211_tx_info *info;
> +   struct skb_info *tx_params;
> +   struct ieee80211_bss_conf *bss;
> +   int status;
> +   u8 ieee80211_size = MIN_802_11_HDR_LEN;
> +   u8 extnd_size;
> +   __le16 *frame_desc;
> +   u16 seq_num;
> +
> +   info = IEEE80211_SKB_CB(skb);
> +   bss = >control.vif->bss_conf;
> +   tx_params = (struct skb_info *)info->driver_data;
> +
> +   if (!bss->assoc) {
> +   status = -EINVAL;
> +   goto err;
> +   }
> +
> +   tmp_hdr = (struct ieee80211_hdr *)>data[0];
> +   seq_num = (le16_to_cpu(tmp_hdr->seq_ctrl) >> 4);
> +
> +   extnd_size = ((uintptr_t)skb->data & 0x3);
> +
> +   if ((FRAME_DESC_SZ + extnd_size) > skb_headroom(skb)) {
> +   rsi_dbg(ERR_ZONE, "%s: Unable to send pkt\n", __func__);
> +   status = -ENOSPC;
> +   goto err;
> +   }
> +
> +   skb_push(skb, (FRAME_DESC_SZ + extnd_size));
> +   frame_desc = (__le16 *)>data[0];
> +   memset((u8 *)frame_desc, 0, FRAME_DESC_SZ);
> +
> +   if (ieee80211_is_data_qos(tmp_hdr->frame_control)) {
> +   ieee80211_size += 2;
> +   frame_desc[6] |= cpu_to_le16(BIT(12));
> +   }
> +
> +   if ((!(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT)) &&
> +   (common->secinfo.security_enable)) {
> +   if (rsi_is_cipher_wep(common))
> +   ieee80211_size += 4;
> +   else
> +   ieee80211_size += 8;
> +   frame_desc[6] |= cpu_to_le16(BIT(15));
> +   }
> +
> +   frame_desc[0] = cpu_to_le16((skb->len - FRAME_DESC_SZ) |
> +   (RSI_WIFI_DATA_Q << 12));
> +   frame_desc[2] = cpu_to_le16((extnd_size) | (ieee80211_size) << 8);
> +
> +   if (common->min_rate != 0x) {
> +  

Re: [PATCH] MAINTAINERS: drop akarwar from mwifiex

[amit karwar]

On Thu, Apr 13, 2017 at 6:47 PM, Kalle Valo  wrote:
> Brian Norris  writes:
>
>> His email is bouncing, and I expect he's not doing this work any more.
>>
>> Cc: Amitkumar Karwar 
>> Cc: Nishant Sarmukadam 
>> Cc: Ganapathi Bhat 
>> Cc: Xinming Hu 
>> Signed-off-by: Brian Norris 
>> ---
>> Or alternatively, we can update his email address. I just don't want to keep
>> seeing bounced emails :)
>
> Me neither :) So Amitkumar, which one do you prefer?
>

Updating email address would be fine. Please replace
akar...@marvell.com with amitkar...@gmail.com

Regards,
Amitkumar Karwar

Re: [RFC PATCH] Revert "mwifiex: fix system hang problem after resume"

[amit karwar]

On Sat, Apr 1, 2017 at 1:51 AM, Brian Norris  wrote:
> This reverts commit 437322ea2a36d112e20aa7282c869bf924b3a836.
>
> This above-mentioned "fix" does not actually do anything to prevent a
> race condition. It simply papers over it so that the issue doesn't
> appear.
>
> If this is a real problem, it should be explained better than the above
> commit does, and an alternative, non-racy solution should be found.
>
> For further reason to revert this: there's ot reason we can't try
> resetting the card when it's *actually* stuck in host-sleep mode. So
> instead, this is unnecessarily creating scenarios where we can't recover
> Wifi.
>
> Cc: Amitkumar Karwar 
> Signed-off-by: Brian Norris 
> ---
> Amit, please take a look. AIUI, your "fix" is wrong, and quite racy. If you
> still think it's needed, can you please propose an alternative? Or at least
> explain more why this is needed? Thanks.
>

I agree. Fix just covers the issue. We need to investigate why system
hangs when card reset is attempted in host sleep activated scenario.

Acked-by: Amitkumar Karwar 

Regards,
Amitkumar Karwar