Re: [PATCH 2/2] usb: xhci: set page size to the xHCI-supported size

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 8.1.2025 16.28, Niklas Neronin wrote:
> The current xHCI driver does not validate whether a page size of 4096
> bytes is supported. Address the issue by setting the page size to the
> value supported by the xHCI controller, as read from the Page Size
> register.
> 
> Additionally, this commit removes unnecessary debug messages and instead
> prints the supported and used page size once.
> 
> The xHCI controller supports page sizes of (2^{(n+12)}) bytes, where 'n'
> is the Page Size Bit. Only one page size is supported, with a maximum
> page size of 128 KB.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> ---
>   drivers/usb/host/xhci-mem.c | 28 ++++++++++++----------------
>   drivers/usb/host/xhci.h     |  8 ++++----
>   2 files changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 66584aafc513..6828b75ad77b 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -1953,7 +1953,6 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>   	xhci->interrupters = NULL;
>   
>   	xhci->page_size = 0;
> -	xhci->page_shift = 0;
>   	xhci->usb2_rhub.bus_state.bus_suspended = 0;
>   	xhci->usb3_rhub.bus_state.bus_suspended = 0;
>   }
> @@ -2372,6 +2371,16 @@ xhci_create_secondary_interrupter(struct usb_hcd *hcd, unsigned int segs,
>   }
>   EXPORT_SYMBOL_GPL(xhci_create_secondary_interrupter);
>   
> +static void xhci_hcd_page_size(struct xhci_hcd *xhci)
> +{
> +	u32 page_shift;
> +
> +	page_shift = readl(&xhci->op_regs->page_size) & XHCI_PAGE_SIZE_MASK;

Should we check that page_shift value makes sense here?

We used to hardcode page_size to 4k, and don't really know if all xHC vendors
have a sane op_regs->page_size value.
Register read might return 0 or 0xffffffff in some power states.

To avoid regression we could add:
if (!in_range(page_shift, 0x1, 0x8000))
	page_shift = 0x1;

Otherwise it looks good

-Mathias