Skip to content

Commit

Permalink
Merge branch 'net-ipa-a-few-small-fixes'
Browse files Browse the repository at this point in the history
Alex Elder says:

====================
net: ipa: a few small fixes

This series implements some minor bug fixes or improvements.

The first patch removes an apparently unnecessary restriction, which
results in an error on a 32-bit ARM build.

The second makes a definition used for SDM845 match what is used in
the downstream code.

The third just ensures two netdev pointers are only non-null when
valid.

The fourth simplifies a little code, knowing that a called function
never returns an error.

The fifth and sixth just remove some empty/place holder functions.

And the last patch fixes a comment, makes a function private, and
removes an unnecessary double-negation of a Boolean variable.  This
patch produces a warning from checkpatch, indicating that a pair of
parentheses is unnecessary.  I agree with that advice, but it
conflicts with a suggestion from the compiler.  I left the "problem"
in place to avoid the compiler warning.
====================

Link: https://lore.kernel.org/r/20210409180722.1176868-1-elder@linaro.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Apr 10, 2021
2 parents 8859a44 + 602a1c7 commit cbd3125
Show file tree
Hide file tree
Showing 13 changed files with 49 additions and 153 deletions.
54 changes: 6 additions & 48 deletions drivers/net/ipa/gsi.c
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ static void gsi_irq_type_disable(struct gsi *gsi, enum gsi_irq_type_id type_id)
gsi_irq_type_update(gsi, gsi->type_enabled_bitmap & ~BIT(type_id));
}

/* Turn off all GSI interrupts initially */
/* Turn off all GSI interrupts initially; there is no gsi_irq_teardown() */
static void gsi_irq_setup(struct gsi *gsi)
{
/* Disable all interrupt types */
Expand All @@ -217,12 +217,6 @@ static void gsi_irq_setup(struct gsi *gsi)
iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
}

/* Turn off all GSI interrupts when we're all done */
static void gsi_irq_teardown(struct gsi *gsi)
{
/* Nothing to do */
}

/* Event ring commands are performed one at a time. Their completion
* is signaled by the event ring control GSI interrupt type, which is
* only enabled when we issue an event ring command. Only the event
Expand Down Expand Up @@ -786,7 +780,7 @@ static void gsi_channel_trans_quiesce(struct gsi_channel *channel)
}
}

/* Program a channel for use */
/* Program a channel for use; there is no gsi_channel_deprogram() */
static void gsi_channel_program(struct gsi_channel *channel, bool doorbell)
{
size_t size = channel->tre_ring.count * GSI_RING_ELEMENT_SIZE;
Expand Down Expand Up @@ -874,11 +868,6 @@ static void gsi_channel_program(struct gsi_channel *channel, bool doorbell)
/* All done! */
}

static void gsi_channel_deprogram(struct gsi_channel *channel)
{
/* Nothing to do */
}

static int __gsi_channel_start(struct gsi_channel *channel, bool start)
{
struct gsi *gsi = channel->gsi;
Expand Down Expand Up @@ -1623,18 +1612,6 @@ static u32 gsi_event_bitmap_init(u32 evt_ring_max)
return event_bitmap;
}

/* Setup function for event rings */
static void gsi_evt_ring_setup(struct gsi *gsi)
{
/* Nothing to do */
}

/* Inverse of gsi_evt_ring_setup() */
static void gsi_evt_ring_teardown(struct gsi *gsi)
{
/* Nothing to do */
}

/* Setup function for a single channel */
static int gsi_channel_setup_one(struct gsi *gsi, u32 channel_id)
{
Expand Down Expand Up @@ -1684,7 +1661,6 @@ static void gsi_channel_teardown_one(struct gsi *gsi, u32 channel_id)

netif_napi_del(&channel->napi);

gsi_channel_deprogram(channel);
gsi_channel_de_alloc_command(gsi, channel_id);
gsi_evt_ring_reset_command(gsi, evt_ring_id);
gsi_evt_ring_de_alloc_command(gsi, evt_ring_id);
Expand Down Expand Up @@ -1759,7 +1735,6 @@ static int gsi_channel_setup(struct gsi *gsi)
u32 mask;
int ret;

gsi_evt_ring_setup(gsi);
gsi_irq_enable(gsi);

mutex_lock(&gsi->mutex);
Expand Down Expand Up @@ -1819,7 +1794,6 @@ static int gsi_channel_setup(struct gsi *gsi)
mutex_unlock(&gsi->mutex);

gsi_irq_disable(gsi);
gsi_evt_ring_teardown(gsi);

return ret;
}
Expand Down Expand Up @@ -1848,15 +1822,13 @@ static void gsi_channel_teardown(struct gsi *gsi)
mutex_unlock(&gsi->mutex);

gsi_irq_disable(gsi);
gsi_evt_ring_teardown(gsi);
}

/* Setup function for GSI. GSI firmware must be loaded and initialized */
int gsi_setup(struct gsi *gsi)
{
struct device *dev = gsi->dev;
u32 val;
int ret;

/* Here is where we first touch the GSI hardware */
val = ioread32(gsi->virt + GSI_GSI_STATUS_OFFSET);
Expand All @@ -1865,7 +1837,7 @@ int gsi_setup(struct gsi *gsi)
return -EIO;
}

gsi_irq_setup(gsi);
gsi_irq_setup(gsi); /* No matching teardown required */

val = ioread32(gsi->virt + GSI_GSI_HW_PARAM_2_OFFSET);

Expand Down Expand Up @@ -1899,18 +1871,13 @@ int gsi_setup(struct gsi *gsi)
/* Writing 1 indicates IRQ interrupts; 0 would be MSI */
iowrite32(1, gsi->virt + GSI_CNTXT_INTSET_OFFSET);

ret = gsi_channel_setup(gsi);
if (ret)
gsi_irq_teardown(gsi);

return ret;
return gsi_channel_setup(gsi);
}

/* Inverse of gsi_setup() */
void gsi_teardown(struct gsi *gsi)
{
gsi_channel_teardown(gsi);
gsi_irq_teardown(gsi);
}

/* Initialize a channel's event ring */
Expand Down Expand Up @@ -1952,7 +1919,7 @@ static void gsi_channel_evt_ring_exit(struct gsi_channel *channel)
gsi_evt_ring_id_free(gsi, evt_ring_id);
}

/* Init function for event rings */
/* Init function for event rings; there is no gsi_evt_ring_exit() */
static void gsi_evt_ring_init(struct gsi *gsi)
{
u32 evt_ring_id = 0;
Expand All @@ -1964,12 +1931,6 @@ static void gsi_evt_ring_init(struct gsi *gsi)
while (++evt_ring_id < GSI_EVT_RING_COUNT_MAX);
}

/* Inverse of gsi_evt_ring_init() */
static void gsi_evt_ring_exit(struct gsi *gsi)
{
/* Nothing to do */
}

static bool gsi_channel_data_valid(struct gsi *gsi,
const struct ipa_gsi_endpoint_data *data)
{
Expand Down Expand Up @@ -2114,7 +2075,7 @@ static int gsi_channel_init(struct gsi *gsi, u32 count,
/* IPA v4.2 requires the AP to allocate channels for the modem */
modem_alloc = gsi->version == IPA_VERSION_4_2;

gsi_evt_ring_init(gsi);
gsi_evt_ring_init(gsi); /* No matching exit required */

/* The endpoint data array is indexed by endpoint name */
for (i = 0; i < count; i++) {
Expand Down Expand Up @@ -2148,7 +2109,6 @@ static int gsi_channel_init(struct gsi *gsi, u32 count,
}
gsi_channel_exit_one(&gsi->channel[data->channel_id]);
}
gsi_evt_ring_exit(gsi);

return ret;
}
Expand All @@ -2162,8 +2122,6 @@ static void gsi_channel_exit(struct gsi *gsi)
gsi_channel_exit_one(&gsi->channel[channel_id]);
while (channel_id--);
gsi->modem_channel_bitmap = 0;

gsi_evt_ring_exit(gsi);
}

/* Init function for GSI. GSI hardware does not need to be "ready" */
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/ipa/gsi_trans.c
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ int gsi_trans_pool_init(struct gsi_trans_pool *pool, size_t size, u32 count,
void *virt;

#ifdef IPA_VALIDATE
if (!size || size % 8)
if (!size)
return -EINVAL;
if (count < max_alloc)
return -EINVAL;
Expand Down Expand Up @@ -141,7 +141,7 @@ int gsi_trans_pool_init_dma(struct device *dev, struct gsi_trans_pool *pool,
void *virt;

#ifdef IPA_VALIDATE
if (!size || size % 8)
if (!size)
return -EINVAL;
if (count < max_alloc)
return -EINVAL;
Expand Down
1 change: 1 addition & 0 deletions drivers/net/ipa/ipa_data-v3.5.1.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = {
.status_enable = true,
.tx = {
.seq_type = IPA_SEQ_2_PASS_SKIP_LAST_UC,
.seq_rep_type = IPA_SEQ_REP_DMA_PARSER,
.status_endpoint =
IPA_ENDPOINT_MODEM_AP_RX,
},
Expand Down
6 changes: 3 additions & 3 deletions drivers/net/ipa/ipa_endpoint.c
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ int ipa_endpoint_modem_exception_reset_all(struct ipa *ipa)
/* We need one command per modem TX endpoint. We can get an upper
* bound on that by assuming all initialized endpoints are modem->IPA.
* That won't happen, and we could be more precise, but this is fine
* for now. We need to end the transaction with a "tag process."
* for now. End the transaction with commands to clear the pipeline.
*/
count = hweight32(initialized) + ipa_cmd_pipeline_clear_count();
trans = ipa_cmd_trans_alloc(ipa, count);
Expand Down Expand Up @@ -1755,7 +1755,7 @@ int ipa_endpoint_config(struct ipa *ipa)

/* Make sure it's pointing in the right direction */
endpoint = &ipa->endpoint[endpoint_id];
if ((endpoint_id < rx_base) != !!endpoint->toward_ipa) {
if ((endpoint_id < rx_base) != endpoint->toward_ipa) {
dev_err(dev, "endpoint id %u wrong direction\n",
endpoint_id);
ret = -EINVAL;
Expand Down Expand Up @@ -1791,7 +1791,7 @@ static void ipa_endpoint_init_one(struct ipa *ipa, enum ipa_endpoint_name name,
ipa->initialized |= BIT(endpoint->endpoint_id);
}

void ipa_endpoint_exit_one(struct ipa_endpoint *endpoint)
static void ipa_endpoint_exit_one(struct ipa_endpoint *endpoint)
{
endpoint->ipa->initialized &= ~BIT(endpoint->endpoint_id);

Expand Down
2 changes: 0 additions & 2 deletions drivers/net/ipa/ipa_endpoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ int ipa_endpoint_modem_exception_reset_all(struct ipa *ipa);

int ipa_endpoint_skb_tx(struct ipa_endpoint *endpoint, struct sk_buff *skb);

void ipa_endpoint_exit_one(struct ipa_endpoint *endpoint);

int ipa_endpoint_enable_one(struct ipa_endpoint *endpoint);
void ipa_endpoint_disable_one(struct ipa_endpoint *endpoint);

Expand Down
29 changes: 9 additions & 20 deletions drivers/net/ipa/ipa_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,21 +147,21 @@ int ipa_setup(struct ipa *ipa)
if (ret)
goto err_endpoint_teardown;

ret = ipa_mem_setup(ipa);
ret = ipa_mem_setup(ipa); /* No matching teardown required */
if (ret)
goto err_command_disable;

ret = ipa_table_setup(ipa);
ret = ipa_table_setup(ipa); /* No matching teardown required */
if (ret)
goto err_mem_teardown;
goto err_command_disable;

/* Enable the exception handling endpoint, and tell the hardware
* to use it by default.
*/
exception_endpoint = ipa->name_map[IPA_ENDPOINT_AP_LAN_RX];
ret = ipa_endpoint_enable_one(exception_endpoint);
if (ret)
goto err_table_teardown;
goto err_command_disable;

ipa_endpoint_default_route_set(ipa, exception_endpoint->endpoint_id);

Expand All @@ -179,10 +179,6 @@ int ipa_setup(struct ipa *ipa)
err_default_route_clear:
ipa_endpoint_default_route_clear(ipa);
ipa_endpoint_disable_one(exception_endpoint);
err_table_teardown:
ipa_table_teardown(ipa);
err_mem_teardown:
ipa_mem_teardown(ipa);
err_command_disable:
ipa_endpoint_disable_one(command_endpoint);
err_endpoint_teardown:
Expand Down Expand Up @@ -211,8 +207,6 @@ static void ipa_teardown(struct ipa *ipa)
ipa_endpoint_default_route_clear(ipa);
exception_endpoint = ipa->name_map[IPA_ENDPOINT_AP_LAN_RX];
ipa_endpoint_disable_one(exception_endpoint);
ipa_table_teardown(ipa);
ipa_mem_teardown(ipa);
command_endpoint = ipa->name_map[IPA_ENDPOINT_AP_COMMAND_TX];
ipa_endpoint_disable_one(command_endpoint);
ipa_endpoint_teardown(ipa);
Expand Down Expand Up @@ -480,23 +474,20 @@ static int ipa_config(struct ipa *ipa, const struct ipa_data *data)
if (ret)
goto err_endpoint_deconfig;

ipa_table_config(ipa);
ipa_table_config(ipa); /* No deconfig required */

/* Assign resource limitation to each group */
/* Assign resource limitation to each group; no deconfig required */
ret = ipa_resource_config(ipa, data->resource_data);
if (ret)
goto err_table_deconfig;
goto err_mem_deconfig;

ret = ipa_modem_config(ipa);
if (ret)
goto err_resource_deconfig;
goto err_mem_deconfig;

return 0;

err_resource_deconfig:
ipa_resource_deconfig(ipa);
err_table_deconfig:
ipa_table_deconfig(ipa);
err_mem_deconfig:
ipa_mem_deconfig(ipa);
err_endpoint_deconfig:
ipa_endpoint_deconfig(ipa);
Expand All @@ -514,8 +505,6 @@ static int ipa_config(struct ipa *ipa, const struct ipa_data *data)
static void ipa_deconfig(struct ipa *ipa)
{
ipa_modem_deconfig(ipa);
ipa_resource_deconfig(ipa);
ipa_table_deconfig(ipa);
ipa_mem_deconfig(ipa);
ipa_endpoint_deconfig(ipa);
ipa_hardware_deconfig(ipa);
Expand Down
9 changes: 3 additions & 6 deletions drivers/net/ipa/ipa_mem.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0

/* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.
* Copyright (C) 2019-2020 Linaro Ltd.
* Copyright (C) 2019-2021 Linaro Ltd.
*/

#include <linux/types.h>
Expand Down Expand Up @@ -53,6 +53,8 @@ ipa_mem_zero_region_add(struct gsi_trans *trans, const struct ipa_mem *mem)
* The AP informs the modem where its portions of memory are located
* in a QMI exchange that occurs at modem startup.
*
* There is no need for a matching ipa_mem_teardown() function.
*
* Return: 0 if successful, or a negative error code
*/
int ipa_mem_setup(struct ipa *ipa)
Expand Down Expand Up @@ -97,11 +99,6 @@ int ipa_mem_setup(struct ipa *ipa)
return 0;
}

void ipa_mem_teardown(struct ipa *ipa)
{
/* Nothing to do */
}

#ifdef IPA_VALIDATE

static bool ipa_mem_valid(struct ipa *ipa, enum ipa_mem_id mem_id)
Expand Down
5 changes: 2 additions & 3 deletions drivers/net/ipa/ipa_mem.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */

/* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.
* Copyright (C) 2019-2020 Linaro Ltd.
* Copyright (C) 2019-2021 Linaro Ltd.
*/
#ifndef _IPA_MEM_H_
#define _IPA_MEM_H_
Expand Down Expand Up @@ -88,8 +88,7 @@ struct ipa_mem {
int ipa_mem_config(struct ipa *ipa);
void ipa_mem_deconfig(struct ipa *ipa);

int ipa_mem_setup(struct ipa *ipa);
void ipa_mem_teardown(struct ipa *ipa);
int ipa_mem_setup(struct ipa *ipa); /* No ipa_mem_teardown() needed */

int ipa_mem_zero_modem(struct ipa *ipa);

Expand Down
Loading

0 comments on commit cbd3125

Please sign in to comment.