Skip to content

Wilc stability issues #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fedepell merged 26 commits into from
Jan 16, 2024
Merged

Wilc stability issues #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7653aa5
wilc1000: fix possible access to NULL value (from upstream)
fedepell Dec 17, 2023
f542495
wilc: make hw mac structure aligned to prevent sporadic bus error (fr…
fedepell Dec 17, 2023
9c2aa70
wilc: minor fix to identation to make logic more understandable
fedepell Dec 17, 2023
173c73b
wilc: fix possible double free (from upstream)
fedepell Dec 17, 2023
7946d31
wilc: add small delay when waking up chip (from upstream)
fedepell Dec 17, 2023
576b99d
wilc: skip isr if in closing stage (from upstream)
fedepell Dec 17, 2023
07058ba
wilc: on error do not allow going to sleep (from upstream)
fedepell Dec 17, 2023
c9e5d5f
wilc: slight readapt of cleanup logic on error (partially from upstream)
fedepell Dec 17, 2023
461c435
wilc: add a waiting period especially on deinit
fedepell Dec 17, 2023
7f4f22d
nulling bus_data when freeing sdio_priv
fedefrancescon Dec 18, 2023
f2c69cc
added some error check
fedefrancescon Dec 19, 2023
bcf9e8d
fix build issues
fedefrancescon Dec 19, 2023
5a6760a
Merge pull request #1 from fedefrancescon:wilc_stability_issues
fedefrancescon Dec 19, 2023
3a1d70f
avoid errors on RSSI if driver has not yet been initialized
Dec 19, 2023
c2ff900
added missing reset options (not sure they're really really needed)
Dec 19, 2023
2187e89
added wilc powerdown on deinit
Dec 19, 2023
27b6b66
removed sleeps
Dec 19, 2023
9dd7b23
%% original patch: 0001-Disable_noisy_WILC1000.patch
fedepell Oct 30, 2021
a4acb1c
Revert "%% original patch: 0001-Disable_noisy_WILC1000.patch"
Dec 20, 2023
96103b2
added reset for `wilc->close`
Dec 20, 2023
49e2558
removed duplicated property init (already in wilc_wlan_init)
Jan 3, 2024
634bae8
CHECK: wilc_wlan_deinitialize now respects the order of operations as…
Jan 5, 2024
b3c730b
CHECK: the wilc->initialized flag should be set within wilc_wlan_init…
Jan 5, 2024
059a594
missing initialized flag reset
Jan 16, 2024
8a8e1bf
made some error message clearer
Jan 16, 2024
9f87329
Merge pull request #2 from fedefrancescon/nulling-priv-sdio
fedepell Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions drivers/staging/wilc1000/bt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ int wilc_bt_power_down(struct wilc *wilc, int source)

mutex_lock(&wilc->cs);

pr_info("source: %s, current bus status Wifi: %d, BT: %d\n",
pr_info("POWERING DOWN: source: %s, current bus status Wifi: %d, BT: %d\n",
(source == DEV_WIFI ? "Wifi" : "BT"),
wilc->power.status[DEV_WIFI],
wilc->power.status[DEV_BT]);
Expand All @@ -327,6 +327,7 @@ int wilc_bt_power_down(struct wilc *wilc, int source)
release_bus(wilc, WILC_BUS_RELEASE_ONLY, source);
if (ret) {
mutex_unlock(&wilc->cs);
pr_err("deinit failed ret: 0x%0x\n", ret);
return ret;
}
}
Expand All @@ -346,7 +347,7 @@ int wilc_bt_power_up(struct wilc *wilc, int source)

mutex_lock(&wilc->cs);

pr_debug("source: %s, current bus status Wifi: %d, BT: %d\n",
pr_info("POWERING UP: source: %s, current bus status Wifi: %d, BT: %d\n",
(source == DEV_WIFI ? "Wifi" : "BT"),
wilc->power.status[DEV_WIFI],
wilc->power.status[DEV_BT]);
Expand Down
6 changes: 6 additions & 0 deletions drivers/staging/wilc1000/hif.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1819,12 +1819,18 @@ int wilc_get_rssi(struct wilc_vif *vif, s8 *rssi_level)
{
struct wid wid;
int result;
struct wilc *wilc = vif->wilc;

if (!rssi_level) {
PRINT_ER(vif->ndev, "RSS pointer value is null\n");
return -EFAULT;
}

if (!wilc->initialized) {
netdev_info(vif->ndev, "Driver not initialized\n");
return 0;
}

wid.id = WID_RSSI;
wid.type = WID_CHAR;
wid.size = sizeof(char);
Expand Down
44 changes: 24 additions & 20 deletions drivers/staging/wilc1000/netdev.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <linux/interrupt.h>
#include <net/ip.h>
#include <linux/module.h>
#include <linux/delay.h>

#include "netdev.h"
#include "cfg80211.h"
Expand Down Expand Up @@ -744,13 +745,17 @@ static void wilc_wlan_deinitialize(struct net_device *dev)
struct wilc_vif *vif = netdev_priv(dev);
Copy link

@fedefrancescon fedefrancescon Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this call safe? null check could be ignored for this one?

Copy link

@fedefrancescon fedefrancescon Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better looking at this function probably the NULL check is pointless as, presumably dev is already checked

struct wilc *wl = vif->wilc;

if (!wl) {
PRINT_ER(dev, "wl is NULL\n");
return;
}

if (wl->initialized) {
PRINT_INFO(vif->ndev, INIT_DBG, "Deinitializing wilc ...\n");

if (!wl) {
PRINT_ER(dev, "wl is NULL\n");
return;
}
ret = wilc_wlan_stop(wl, vif);
if (ret != 0)
PRINT_ER(dev, "failed in wlan_stop\n");

PRINT_D(vif->ndev, INIT_DBG, "destroy aging timer\n");

Expand All @@ -765,21 +770,18 @@ static void wilc_wlan_deinitialize(struct net_device *dev)
mutex_unlock(&wl->hif_cs);
}
}
complete(&wl->txq_event);
deinit_irq(dev);

PRINT_INFO(vif->ndev, INIT_DBG, "Deinitializing Threads\n");
wlan_deinitialize_threads(dev);
PRINT_INFO(vif->ndev, INIT_DBG, "Deinitializing IRQ\n");
deinit_irq(dev);

ret = wilc_wlan_stop(wl, vif);
if (ret != 0)
PRINT_ER(dev, "failed in wlan_stop\n");

PRINT_INFO(vif->ndev, INIT_DBG, "Deinitializing WILC Wlan\n");
wilc_wlan_cleanup(dev);

wl->initialized = false;
wl->close = 0;
wl->quit = 0;

PRINT_INFO(dev, INIT_DBG, "wilc deinitialization Done\n");
} else {
Expand Down Expand Up @@ -828,8 +830,6 @@ static int wilc_wlan_initialize(struct net_device *dev, struct wilc_vif *vif)

if (!wl->initialized) {
wl->mac_status = WILC_MAC_STATUS_INIT;
wl->close = 0;
wl->initialized = 0;

ret = wilc_wlan_init(dev);
if (ret) {
Expand Down Expand Up @@ -871,9 +871,11 @@ static int wilc_wlan_initialize(struct net_device *dev, struct wilc_vif *vif)
ret = wilc_start_firmware(dev);
if (ret) {
PRINT_ER(dev, "Failed to start firmware\n");
goto fail_irq_enable;
goto fail_fw_start;
}

wl->initialized = true;

if (cfg_get(vif, 1, WID_FIRMWARE_VERSION, 1, 0)) {
int size;
char firmware_ver[50];
Expand All @@ -893,7 +895,6 @@ static int wilc_wlan_initialize(struct net_device *dev, struct wilc_vif *vif)
goto fail_fw_start;
}

wl->initialized = true;
return 0;

fail_fw_start:
Expand All @@ -910,6 +911,7 @@ static int wilc_wlan_initialize(struct net_device *dev, struct wilc_vif *vif)
fail_wilc_wlan:
wilc_wlan_cleanup(dev);
PRINT_ER(dev, "WLAN initialization FAILED\n");
wl->initialized = false;
} else {
PRINT_WRN(vif->ndev, INIT_DBG, "wilc already initialized\n");
}
Expand All @@ -928,7 +930,7 @@ static int wilc_mac_open(struct net_device *ndev)
{
struct wilc_vif *vif = netdev_priv(ndev);
struct wilc *wl = vif->wilc;
unsigned char mac_add[ETH_ALEN] = {0};
unsigned char mac_add[ETH_ALEN] __aligned(2) = { 0 };
int ret = 0;

if (!wl || !wl->dev) {
Expand Down Expand Up @@ -999,7 +1001,7 @@ static int wilc_set_mac_addr(struct net_device *dev, void *p)
struct wilc_vif *vif = netdev_priv(dev);
struct sockaddr *addr = (struct sockaddr *)p;
struct wilc *wilc = vif->wilc;
unsigned char mac_addr[6] = {0};
unsigned char mac_addr[6] __aligned(2) = { 0 };
struct wilc_vif *tmp_vif;
int srcu_idx;

Expand Down Expand Up @@ -1164,17 +1166,20 @@ static int wilc_mac_close(struct net_device *ndev)
if (vif->ndev) {
netif_stop_queue(vif->ndev);

handle_connect_cancel(vif);
handle_connect_cancel(vif);

if (!recovery_on)
wilc_deinit_host_int(vif->ndev);
if (!recovery_on)
wilc_deinit_host_int(vif->ndev);
}

if (wl->open_ifcs == 0) {
PRINT_INFO(ndev, GENERIC_DBG, "Deinitializing wilc\n");
wl->close = 1;

wilc_wlan_deinitialize(ndev);

PRINT_INFO(ndev, GENERIC_DBG, "Powering down wilc\n");
wilc_bt_power_down(wl, DEV_WIFI);
}

vif->mac_opened = 0;
Expand Down Expand Up @@ -1266,7 +1271,6 @@ void wilc_netdev_cleanup(struct wilc *wilc)
#endif
wilc_sysfs_exit();
wlan_deinit_locks(wilc);
kfree(wilc->bus_data);
wiphy_unregister(wilc->wiphy);
pr_info("Freeing wiphy\n");
wiphy_free(wilc->wiphy);
Expand Down
54 changes: 52 additions & 2 deletions drivers/staging/wilc1000/sdio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,18 +221,22 @@ static int wilc_sdio_probe(struct sdio_func *func,
wilc_netdev_cleanup(wilc);
free:
kfree(sdio_priv);
wilc->bus_data = NULL;
return ret;
}

static void wilc_sdio_remove(struct sdio_func *func)
{
struct wilc *wilc = sdio_get_drvdata(func);
struct wilc_sdio *sdio_priv = wilc->bus_data;

if (!IS_ERR(wilc->rtc_clk))
clk_disable_unprepare(wilc->rtc_clk);

wilc_netdev_cleanup(wilc);
wilc_bt_deinit();
kfree(sdio_priv);
wilc->bus_data = NULL;
}

static int wilc_sdio_reset(struct wilc *wilc)
Expand All @@ -257,6 +261,9 @@ static int wilc_sdio_reset(struct wilc *wilc)
static bool wilc_sdio_is_init(struct wilc *wilc)
{
struct wilc_sdio *sdio_priv = wilc->bus_data;
if (!sdio_priv) {
return false;
}

return sdio_priv->is_init;
}
Expand Down Expand Up @@ -410,6 +417,11 @@ static int wilc_sdio_write_reg(struct wilc *wilc, u32 addr, u32 data)
struct wilc_sdio *sdio_priv = wilc->bus_data;
int ret;

if (!sdio_priv) {
dev_err(&func->dev, "%s: 'bus_data' not initialized\n", __func__);
return -1;
}

cpu_to_le32s(&data);

if (addr >= 0xf0 && addr <= 0xff) { /* only vendor specific registers */
Expand Down Expand Up @@ -459,6 +471,11 @@ static int wilc_sdio_write(struct wilc *wilc, u32 addr, u8 *buf, u32 size)
{
struct sdio_func *func = dev_to_sdio_func(wilc->dev);
struct wilc_sdio *sdio_priv = wilc->bus_data;
if (!sdio_priv) {
dev_err(&func->dev, "%s: 'bus_data' not initialized\n", __func__);
return -1;
}

u32 block_size = sdio_priv->block_size;
struct sdio_cmd53 cmd;
int nblk, nleft, ret;
Expand Down Expand Up @@ -533,6 +550,11 @@ static int wilc_sdio_read_reg(struct wilc *wilc, u32 addr, u32 *data)
struct sdio_func *func = dev_to_sdio_func(wilc->dev);
struct wilc_sdio *sdio_priv = wilc->bus_data;
int ret;

if (!sdio_priv) {
dev_err(&func->dev, "%s: 'bus_data' not initialized\n", __func__);
return -1;
}

if (addr >= 0xf0 && addr <= 0xff) { /* only vendor specific registers */
struct sdio_cmd52 cmd;
Expand Down Expand Up @@ -580,9 +602,15 @@ static int wilc_sdio_read(struct wilc *wilc, u32 addr, u8 *buf, u32 size)
{
struct sdio_func *func = dev_to_sdio_func(wilc->dev);
struct wilc_sdio *sdio_priv = wilc->bus_data;
u32 block_size = sdio_priv->block_size;
struct sdio_cmd53 cmd;
int nblk, nleft, ret;
u32 block_size;

if (!sdio_priv) {
dev_err(&func->dev, "%s: 'bus_data' not initialized\n", __func__);
return -1;
}
block_size = sdio_priv->block_size;

cmd.read_write = 0;
if (addr > 0) {
Expand Down Expand Up @@ -660,7 +688,9 @@ static int wilc_sdio_deinit(struct wilc *wilc)
struct sdio_func *func = dev_to_sdio_func(wilc->dev);
struct wilc_sdio *sdio_priv = wilc->bus_data;

sdio_priv->is_init = false;
if (sdio_priv) {
sdio_priv->is_init = false;
}

pm_runtime_put_sync_autosuspend(mmc_dev(func->card->host));
wilc_wlan_power(wilc, false);
Expand All @@ -672,6 +702,11 @@ static int wilc_sdio_init(struct wilc *wilc, bool resume)
{
struct sdio_func *func = dev_to_sdio_func(wilc->dev);
struct wilc_sdio *sdio_priv = wilc->bus_data;
if (!sdio_priv) {
dev_err(&func->dev, "%s: 'bus_data' not initialized\n", __func__);
return -1;
}

struct sdio_cmd52 cmd;
int loop, ret;
u32 chipid;
Expand Down Expand Up @@ -830,6 +865,11 @@ static int wilc_sdio_read_int(struct wilc *wilc, u32 *int_status)
{
struct sdio_func *func = dev_to_sdio_func(wilc->dev);
struct wilc_sdio *sdio_priv = wilc->bus_data;
if (!sdio_priv) {
dev_err(&func->dev, "%s: 'bus_data' not initialized\n", __func__);
return -1;
}

u32 tmp;
struct sdio_cmd52 cmd;
u32 irq_flags;
Expand Down Expand Up @@ -881,6 +921,11 @@ static int wilc_sdio_clear_int_ext(struct wilc *wilc, u32 val)
struct wilc_sdio *sdio_priv = wilc->bus_data;
int ret;
u32 reg = 0;

if (!sdio_priv) {
dev_err(&func->dev, "%s: 'bus_data' not initialized\n", __func__);
return -1;
}

if (wilc->chip == WILC_1000) {
if (sdio_priv->irq_gpio)
Expand Down Expand Up @@ -970,6 +1015,11 @@ static int wilc_sdio_sync_ext(struct wilc *wilc, int nint)
{
struct sdio_func *func = dev_to_sdio_func(wilc->dev);
struct wilc_sdio *sdio_priv = wilc->bus_data;
if (!sdio_priv) {
dev_err(&func->dev, "%s: 'bus_data' not initialized\n", __func__);
return -1;
}

u32 reg;
int ret, i;

Expand Down
4 changes: 4 additions & 0 deletions drivers/staging/wilc1000/spi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,18 +162,22 @@ static int wilc_bus_probe(struct spi_device *spi)
wilc_netdev_cleanup(wilc);
free:
kfree(spi_priv);
wilc->bus_data = NULL;
return ret;
}

static int wilc_bus_remove(struct spi_device *spi)
{
struct wilc *wilc = spi_get_drvdata(spi);
struct wilc_spi *spi_priv = wilc->bus_data;

if (!IS_ERR(wilc->rtc_clk))
clk_disable_unprepare(wilc->rtc_clk);

wilc_netdev_cleanup(wilc);
wilc_bt_deinit();
kfree(spi_priv);
wilc->bus_data = NULL;
return 0;
}

Expand Down
Loading