On 6/27/2023 5:16 AM, Thinh Nguyen wrote:
On Mon, Jun 26, 2023, Thinh Nguyen wrote:
On Sat, Jun 24, 2023, Krishna Kurapati PSSNV wrote:
On 6/24/2023 3:57 AM, Thinh Nguyen wrote:
On Wed, Jun 21, 2023, Krishna Kurapati wrote:
On some SoC's like SA8295P where the tertiary controller is host-only
capable, GEVTADDRHI/LO, GEVTSIZ, GEVTCOUNT registers are not accessible.
Trying to access them leads to a crash.
For DRD/Peripheral supported controllers, event buffer setup is done
again in gadget_pullup. Skip setup or cleanup of event buffers if
controller is host-only capable.
Suggested-by: Johan Hovold <johan@xxxxxxxxxx>
Signed-off-by: Krishna Kurapati <quic_kriskura@xxxxxxxxxxx>
---
drivers/usb/dwc3/core.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 32ec05fc242b..e1ebae54454f 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -486,6 +486,11 @@ static void dwc3_free_event_buffers(struct dwc3 *dwc)
static int dwc3_alloc_event_buffers(struct dwc3 *dwc, unsigned int length)
{
struct dwc3_event_buffer *evt;
+ unsigned int hw_mode;
+
+ hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+ if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST)
+ return 0;
This is a little awkward. Returning 0 here indicates that this function
was successful, and the event buffers were allocated based on the
function name. Do this check outside of dwc3_alloc_one_event_buffer()
and specifically set dwc->ev_buf = NULL if that's the case.
Hi Thinh,
Apologies, I didn't understand the comment properly.
I thought we were supposed to return 0 here if we fulfill the goal of this
function (allocate if we are drd/gadget and don't allocate if we are host
mode only).
The name of the function implies that it returns 0 if it allocated the
event buffer. If there are multiple conditions to the function returning
0 here, then we should document it.
If we return a non zero error here, probe would fail as this call will be
done only for host only controllers during probe and nowhere else.
Are you suggesting we move this check to dwc3_alloc_one_event_buffer call
?
Regards,
Krishna,
I'm suggesting to move the check to the caller of
dwc3_alloc_one_event_buffer(). Something like this so that it's
self-documented:
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 0beaab932e7d..bba82792bf6f 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1773,6 +1773,7 @@ static int dwc3_probe(struct platform_device *pdev)
struct resource *res, dwc_res;
void __iomem *regs;
struct dwc3 *dwc;
+ unsigned int hw_mode;
int ret;
dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
@@ -1854,11 +1855,16 @@ static int dwc3_probe(struct platform_device *pdev)
pm_runtime_forbid(dev);
- ret = dwc3_alloc_event_buffers(dwc, DWC3_EVENT_BUFFERS_SIZE);
- if (ret) {
- dev_err(dwc->dev, "failed to allocate event buffers\n");
- ret = -ENOMEM;
- goto err_allow_rpm;
+ hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+ if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST) {
+ dwc->ev_buf = NULL;
+ } else {
+ ret = dwc3_alloc_event_buffers(dwc, DWC3_EVENT_BUFFERS_SIZE);
+ if (ret) {
+ dev_err(dwc->dev, "failed to allocate event buffers\n");
+ ret = -ENOMEM;
+ goto err_allow_rpm;
+ }
}
dwc->edev = dwc3_get_extcon(dwc);
Actually, please ignore. there's already a document there, I missed that
for some reason. What you did is fine. Though, I don't see the condition
for ev_buf = NULL anywhere. Can you add that for clarity?
Thanks,
Thinh
Hi Thinh,
Did you mean adding "dwc->ev_buf = NULL" specifically ?
Regards,
Krishna,
