Re: [PATCH v1 2/8] usb: xhci: xhci_bulk_tx: Don't "BUG" when comparing addresses on Octeon

[Stefan Roese]

On 19.08.20 15:49, Daniel Schwierzeck wrote:

Am Montag, den 17.08.2020, 15:06 +0200 schrieb Stefan Roese:

Octeon uses mapped addresses for virtual and physical memory. Its not
that easy to calculate the resulting addresses here. So lets remove
this BUG_ON() for Octeon in xhci_bulk_tx().

Signed-off-by: Stefan Roese 
Cc: Bin Meng 
Cc: Marek Vasut 
---

  drivers/usb/host/xhci-ring.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 092ed6eaf1..a762177c57 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -726,8 +726,11 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
  
  	BUG_ON(TRB_TO_SLOT_ID(field) != slot_id);

BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
-   BUG_ON(*(void **)(uintptr_t)le64_to_cpu(event->trans_event.buffer) -
-   buffer > (size_t)length);
+   if (!IS_ENABLED(CONFIG_ARCH_OCTEON)) {
+   BUG_ON(*(void **)(uintptr_t)le64_to_cpu(
+  event->trans_event.buffer) - buffer >
+  (size_t)length);
+   }


why not remove the check? Or add at least a generic Kconfig option for
special handling on archs with memory remapping. Arch or SoC specific
config options in generic code are bad ;)


Yes, I agree. I was also thinking about removing this check completely.
I'm not 100% sure, but I think that some (most?) of these BUG_ON()
macros have been removed from the Linux kernel.

To not make too many changes, I'll change this patch in the next
version to just remove this one BUG_ON().

Thanks,
Stefan

Re: [PATCH v1 2/8] usb: xhci: xhci_bulk_tx: Don't "BUG" when comparing addresses on Octeon

[Daniel Schwierzeck]

Am Montag, den 17.08.2020, 15:06 +0200 schrieb Stefan Roese:
> Octeon uses mapped addresses for virtual and physical memory. Its not
> that easy to calculate the resulting addresses here. So lets remove
> this BUG_ON() for Octeon in xhci_bulk_tx().
> 
> Signed-off-by: Stefan Roese 
> Cc: Bin Meng 
> Cc: Marek Vasut 
> ---
> 
>  drivers/usb/host/xhci-ring.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 092ed6eaf1..a762177c57 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -726,8 +726,11 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
> pipe,
>  
>   BUG_ON(TRB_TO_SLOT_ID(field) != slot_id);
>   BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
> - BUG_ON(*(void **)(uintptr_t)le64_to_cpu(event->trans_event.buffer) -
> - buffer > (size_t)length);
> + if (!IS_ENABLED(CONFIG_ARCH_OCTEON)) {
> + BUG_ON(*(void **)(uintptr_t)le64_to_cpu(
> +event->trans_event.buffer) - buffer >
> +(size_t)length);
> + }

why not remove the check? Or add at least a generic Kconfig option for
special handling on archs with memory remapping. Arch or SoC specific
config options in generic code are bad ;)

>  
>   record_transfer_result(udev, event, length);
>   xhci_acknowledge_event(ctrl);
-- 
- Daniel

[PATCH v1 2/8] usb: xhci: xhci_bulk_tx: Don't "BUG" when comparing addresses on Octeon

[Stefan Roese]

Octeon uses mapped addresses for virtual and physical memory. Its not
that easy to calculate the resulting addresses here. So lets remove
this BUG_ON() for Octeon in xhci_bulk_tx().

Signed-off-by: Stefan Roese 
Cc: Bin Meng 
Cc: Marek Vasut 
---

 drivers/usb/host/xhci-ring.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 092ed6eaf1..a762177c57 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -726,8 +726,11 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
 
BUG_ON(TRB_TO_SLOT_ID(field) != slot_id);
BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
-   BUG_ON(*(void **)(uintptr_t)le64_to_cpu(event->trans_event.buffer) -
-   buffer > (size_t)length);
+   if (!IS_ENABLED(CONFIG_ARCH_OCTEON)) {
+   BUG_ON(*(void **)(uintptr_t)le64_to_cpu(
+  event->trans_event.buffer) - buffer >
+  (size_t)length);
+   }
 
record_transfer_result(udev, event, length);
xhci_acknowledge_event(ctrl);
-- 
2.28.0