Clean up xhci-mem.c a bit using latest and greatest Linux kernel
features.
Changelog v2:
- used dma_pool_zalloc() instead of open coding it (Mathias)
Andy Shevchenko (7):
xhci: mem: Carefully calculate size for memory allocations
xhci: mem: Use dma_poll_zalloc() instead of explicit memset()
xhci: mem: Get rid of redundant 'else'
xhci: mem: Drop useless return:s
xhci: mem: Use while (i--) pattern to clean up
xhci: mem: Replace explicit castings with appropriate specifiers
xhci: mem: Join string literals back
drivers/usb/host/xhci-mem.c | 83 +++++++++++++++----------------------
1 file changed, 33 insertions(+), 50 deletions(-)
--
2.39.1
For easy grepping on debug purposes join string literals back in
the messages.
No functional change.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/usb/host/xhci-mem.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 67ac02d177b5..7e106bd804ca 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -607,8 +607,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
int ret;
struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
- xhci_dbg(xhci, "Allocating %u streams and %u "
- "stream context array entries.\n",
+ xhci_dbg(xhci, "Allocating %u streams and %u stream context array entries.\n",
num_streams, num_stream_ctxs);
if (xhci->cmd_ring_reserved_trbs == MAX_RSVD_CMD_TRBS) {
xhci_dbg(xhci, "Command ring has no reserved TRBs available\n");
@@ -1950,8 +1949,7 @@ static void xhci_set_hc_event_deq(struct xhci_hcd *xhci, struct xhci_interrupter
deq = xhci_trb_virt_to_dma(ir->event_ring->deq_seg,
ir->event_ring->dequeue);
if (!deq)
- xhci_warn(xhci, "WARN something wrong with SW event ring "
- "dequeue ptr.\n");
+ xhci_warn(xhci, "WARN something wrong with SW event ring dequeue ptr.\n");
/* Update HC event ring dequeue pointer */
temp = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
temp &= ERST_PTR_MASK;
@@ -1960,8 +1958,7 @@ static void xhci_set_hc_event_deq(struct xhci_hcd *xhci, struct xhci_interrupter
*/
temp &= ~ERST_EHB;
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
- "// Write event ring dequeue pointer, "
- "preserving EHB bit");
+ "// Write event ring dequeue pointer, preserving EHB bit");
xhci_write_64(xhci, ((u64) deq & (u64) ~ERST_PTR_MASK) | temp,
&ir->ir_set->erst_dequeue);
}
@@ -1994,8 +1991,7 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
} else if (major_revision <= 0x02) {
rhub = &xhci->usb2_rhub;
} else {
- xhci_warn(xhci, "Ignoring unknown port speed, "
- "Ext Cap %p, revision = 0x%x\n",
+ xhci_warn(xhci, "Ignoring unknown port speed, Ext Cap %p, revision = 0x%x\n",
addr, major_revision);
/* Ignoring port protocol we can't understand. FIXME */
return;
@@ -2010,9 +2006,8 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
port_offset = XHCI_EXT_PORT_OFF(temp);
port_count = XHCI_EXT_PORT_COUNT(temp);
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
- "Ext Cap %p, port offset = %u, "
- "count = %u, revision = 0x%x",
- addr, port_offset, port_count, major_revision);
+ "Ext Cap %p, port offset = %u, count = %u, revision = 0x%x",
+ addr, port_offset, port_count, major_revision);
/* Port count includes the current port offset */
if (port_offset == 0 || (port_offset + port_count - 1) > num_ports)
/* WTF? "Valid values are ‘1’ to MaxPorts" */
@@ -2069,10 +2064,8 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
struct xhci_port *hw_port = &xhci->hw_ports[i];
/* Duplicate entry. Ignore the port if the revisions differ. */
if (hw_port->rhub) {
- xhci_warn(xhci, "Duplicate port entry, Ext Cap %p,"
- " port %u\n", addr, i);
- xhci_warn(xhci, "Port was marked as USB %u, "
- "duplicated as USB %u\n",
+ xhci_warn(xhci, "Duplicate port entry, Ext Cap %p, port %u\n", addr, i);
+ xhci_warn(xhci, "Port was marked as USB %u, duplicated as USB %u\n",
hw_port->rhub->maj_rev, major_revision);
/* Only adjust the roothub port counts if we haven't
* found a similar duplicate.
@@ -2411,8 +2404,8 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
val = readl(&xhci->cap_regs->db_off);
val &= DBOFF_MASK;
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
- "// Doorbell array is located at offset 0x%x"
- " from cap regs base addr", val);
+ "// Doorbell array is located at offset 0x%x from cap regs base addr",
+ val);
xhci->dba = (void __iomem *) xhci->cap_regs + val;
/* Set ir_set to interrupt register set 0 */
--
2.39.1
There is no need to have explicit castings when we have specific pointer extensions
Replace the explicit castings with appropriate specifiers.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/usb/host/xhci-mem.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index b8c1465f8d23..67ac02d177b5 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -666,8 +666,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
cur_ring->cycle_state;
stream_info->stream_ctx_array[cur_stream].stream_ring =
cpu_to_le64(addr);
- xhci_dbg(xhci, "Setting stream %d ring ptr to 0x%08llx\n",
- cur_stream, (unsigned long long) addr);
+ xhci_dbg(xhci, "Setting stream %d ring ptr to 0x%08llx\n", cur_stream, addr);
ret = xhci_update_stream_mapping(cur_ring, mem_flags);
if (ret) {
@@ -977,16 +976,14 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
if (!dev->out_ctx)
goto fail;
- xhci_dbg(xhci, "Slot %d output ctx = 0x%llx (dma)\n", slot_id,
- (unsigned long long)dev->out_ctx->dma);
+ xhci_dbg(xhci, "Slot %d output ctx = 0x%pad (dma)\n", slot_id, &dev->out_ctx->dma);
/* Allocate the (input) device context for address device command */
dev->in_ctx = xhci_alloc_container_ctx(xhci, XHCI_CTX_TYPE_INPUT, flags);
if (!dev->in_ctx)
goto fail;
- xhci_dbg(xhci, "Slot %d input ctx = 0x%llx (dma)\n", slot_id,
- (unsigned long long)dev->in_ctx->dma);
+ xhci_dbg(xhci, "Slot %d input ctx = 0x%pad (dma)\n", slot_id, &dev->in_ctx->dma);
/* Initialize the cancellation and bandwidth list for each ep */
for (i = 0; i < 31; i++) {
@@ -2351,8 +2348,8 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
goto fail;
xhci->dcbaa->dma = dma;
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
- "// Device context base array address = 0x%llx (DMA), %p (virt)",
- (unsigned long long)xhci->dcbaa->dma, xhci->dcbaa);
+ "// Device context base array address = 0x%pad (DMA), %p (virt)",
+ &xhci->dcbaa->dma, xhci->dcbaa);
xhci_write_64(xhci, dma, &xhci->op_regs->dcbaa_ptr);
/*
@@ -2393,8 +2390,8 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
goto fail;
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"Allocated command ring at %p", xhci->cmd_ring);
- xhci_dbg_trace(xhci, trace_xhci_dbg_init, "First segment DMA is 0x%llx",
- (unsigned long long)xhci->cmd_ring->first_seg->dma);
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "First segment DMA is 0x%pad",
+ &xhci->cmd_ring->first_seg->dma);
/* Set the address in the Command Ring Control register */
val_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
--
2.39.1
Carefully calculate size for memory allocations, i.e. with help
of size_mul() macro from overflow.h.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/usb/host/xhci-mem.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index d0a9467aa5fc..c385513ad00b 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -9,6 +9,7 @@
*/
#include <linux/usb.h>
+#include <linux/overflow.h>
#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/dmapool.h>
@@ -568,7 +569,7 @@ static struct xhci_stream_ctx *xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
gfp_t mem_flags)
{
struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
- size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs;
+ size_t size = size_mul(sizeof(struct xhci_stream_ctx), num_stream_ctxs);
if (size > MEDIUM_STREAM_ARRAY_SIZE)
return dma_alloc_coherent(dev, size,
@@ -1660,7 +1661,7 @@ static int scratchpad_alloc(struct xhci_hcd *xhci, gfp_t flags)
goto fail_sp;
xhci->scratchpad->sp_array = dma_alloc_coherent(dev,
- num_sp * sizeof(u64),
+ size_mul(sizeof(u64), num_sp),
&xhci->scratchpad->sp_dma, flags);
if (!xhci->scratchpad->sp_array)
goto fail_sp2;
@@ -1799,7 +1800,7 @@ int xhci_alloc_erst(struct xhci_hcd *xhci,
struct xhci_segment *seg;
struct xhci_erst_entry *entry;
- size = sizeof(struct xhci_erst_entry) * evt_ring->num_segs;
+ size = size_mul(sizeof(struct xhci_erst_entry), evt_ring->num_segs);
erst->entries = dma_alloc_coherent(xhci_to_hcd(xhci)->self.sysdev,
size, &erst->erst_dma_addr, flags);
if (!erst->entries)
@@ -1830,7 +1831,7 @@ xhci_free_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
if (!ir)
return;
- erst_size = sizeof(struct xhci_erst_entry) * (ir->erst.num_entries);
+ erst_size = sizeof(struct xhci_erst_entry) * ir->erst.num_entries;
if (ir->erst.entries)
dma_free_coherent(dev, erst_size,
ir->erst.entries,
--
2.39.1
In the snippets like the following
if (...)
return / goto / break / continue ...;
else
...
the 'else' is redundant. Get rid of it.
While at it, make if-chain sorted from testing bigger values to smaller.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/usb/host/xhci-mem.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 4ffa6495878d..357883256a5a 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -572,14 +572,11 @@ static struct xhci_stream_ctx *xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
size_t size = size_mul(sizeof(struct xhci_stream_ctx), num_stream_ctxs);
if (size > MEDIUM_STREAM_ARRAY_SIZE)
- return dma_alloc_coherent(dev, size,
- dma, mem_flags);
- else if (size <= SMALL_STREAM_ARRAY_SIZE)
- return dma_pool_zalloc(xhci->small_streams_pool,
- mem_flags, dma);
+ return dma_alloc_coherent(dev, size, dma, mem_flags);
+ if (size > SMALL_STREAM_ARRAY_SIZE)
+ return dma_pool_zalloc(xhci->medium_streams_pool, mem_flags, dma);
else
- return dma_pool_zalloc(xhci->medium_streams_pool,
- mem_flags, dma);
+ return dma_pool_zalloc(xhci->small_streams_pool, mem_flags, dma);
}
struct xhci_ring *xhci_dma_to_transfer_ring(
@@ -1399,8 +1396,9 @@ static u32 xhci_get_max_esit_payload(struct usb_device *udev,
if ((udev->speed >= USB_SPEED_SUPER_PLUS) &&
USB_SS_SSP_ISOC_COMP(ep->ss_ep_comp.bmAttributes))
return le32_to_cpu(ep->ssp_isoc_ep_comp.dwBytesPerInterval);
+
/* SuperSpeed or SuperSpeedPlus Isoc ep with less than 48k per esit */
- else if (udev->speed >= USB_SPEED_SUPER)
+ if (udev->speed >= USB_SPEED_SUPER)
return le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval);
max_packet = usb_endpoint_maxp(&ep->desc);
--
2.39.1
Use more natural while (i--) patter to clean up allocated resources.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/usb/host/xhci-mem.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index fa0c4ac2ca7f..b8c1465f8d23 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1679,11 +1679,10 @@ static int scratchpad_alloc(struct xhci_hcd *xhci, gfp_t flags)
return 0;
fail_sp4:
- for (i = i - 1; i >= 0; i--) {
+ while (i--)
dma_free_coherent(dev, xhci->page_size,
xhci->scratchpad->sp_buffers[i],
xhci->scratchpad->sp_array[i]);
- }
kfree(xhci->scratchpad->sp_buffers);
--
2.39.1
When function returns void and we have if-else-if chain, there is
no need to explicitly call return. Drop them and indent lines better.
While at it, make if-chain sorted from testing bigger values to smaller.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/usb/host/xhci-mem.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 357883256a5a..fa0c4ac2ca7f 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -544,14 +544,11 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci,
size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs;
if (size > MEDIUM_STREAM_ARRAY_SIZE)
- dma_free_coherent(dev, size,
- stream_ctx, dma);
- else if (size <= SMALL_STREAM_ARRAY_SIZE)
- return dma_pool_free(xhci->small_streams_pool,
- stream_ctx, dma);
+ dma_free_coherent(dev, size, stream_ctx, dma);
+ else if (size > SMALL_STREAM_ARRAY_SIZE)
+ dma_pool_free(xhci->medium_streams_pool, stream_ctx, dma);
else
- return dma_pool_free(xhci->medium_streams_pool,
- stream_ctx, dma);
+ dma_pool_free(xhci->small_streams_pool, stream_ctx, dma);
}
/*
--
2.39.1
On 8.2.2023 17.11, Andy Shevchenko wrote:
> Clean up xhci-mem.c a bit using latest and greatest Linux kernel
> features.
>
> Changelog v2:
> - used dma_pool_zalloc() instead of open coding it (Mathias)
>
> Andy Shevchenko (7):
> xhci: mem: Carefully calculate size for memory allocations
> xhci: mem: Use dma_poll_zalloc() instead of explicit memset()
> xhci: mem: Get rid of redundant 'else'
> xhci: mem: Drop useless return:s
> xhci: mem: Use while (i--) pattern to clean up
> xhci: mem: Replace explicit castings with appropriate specifiers
> xhci: mem: Join string literals back
>
> drivers/usb/host/xhci-mem.c | 83 +++++++++++++++----------------------
> 1 file changed, 33 insertions(+), 50 deletions(-)
>
Added to queue, thanks
-Mathias