RE: [PATCH 1/2] staging: dwc2: when dma is disabled, clear dev->dma_mask

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Paul Zimmerman
> Sent: Monday, April 29, 2013 6:27 PM
> 
> > From: Matthijs Kooijman [mailto:matthijs@xxxxxxxx]
> > Sent: Monday, April 29, 2013 7:33 AM
> >
> > Instead of just setting the dma_mask to the empty mask, the pointer to
> > the mask is cleared altogether. This is needed because
> > usb_create_shared_hcd sets hcd->self.uses_dma based on
> > dev->dma_mask != NULL, not by looking at the actual mask itself.
> >
> > This change thus makes hcd->self.uses_dma have the correct value when
> > not using dma even though a dma_mask was set up by the platform or pci
> > driver (which in practice should only occur when manually disabling
> > dma, assuming that a dma_mask is normally only setup when dma is
> > supported and enabled). This in turn prevents usb core from doing some
> > dma-specific stuff that isn't needed (though everything seemed to work
> > with the wrong value of uses_dma as well).
> >
> > Signed-off-by: Matthijs Kooijman <matthijs@xxxxxxxx>
> > ---
> >  drivers/staging/dwc2/hcd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/dwc2/hcd.c b/drivers/staging/dwc2/hcd.c
> > index 7727112..ac34d4a 100644
> > --- a/drivers/staging/dwc2/hcd.c
> > +++ b/drivers/staging/dwc2/hcd.c
> > @@ -2810,7 +2810,7 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
> >  			dev_warn(hsotg->dev,
> >  				 "can't enable workaround for >2GB RAM\n");
> >  	} else {
> > -		dma_set_mask(hsotg->dev, 0);
> > +		hsotg->dev->dma_mask = NULL;
> >  		dma_set_coherent_mask(hsotg->dev, 0);
> >  	}
> 
> I see a problem after applying this patch. If I insert the driver
> modules with dwc2_module_params.dma_enable = 0, remove the modules,
> then insert them again with dwc2_module_params.dma_enable = 1, then
> the DMA does not get enabled. I see the message "dma_mask not set,
> disabling DMA" from the other patch. So setting the
> hsotg->dev->dma_mask pointer to NULL seems to be irreversible.
> 
> It seems like this shouldn't happen. I think hsotg->dev should get
> deleted when the module is removed, and recreated when it is inserted
> again. But maybe I just don't understand the device model.

Hi Matthijs,

Can you try the additional patch below on top of your original patch?
This fixes the problem for me with the PCI driver, but I want to make
sure it works for the platform device as well.

-- 
Paul

diff --git a/drivers/staging/dwc2/core.h b/drivers/staging/dwc2/core.h
index fc075a7..c505b67 100644
--- a/drivers/staging/dwc2/core.h
+++ b/drivers/staging/dwc2/core.h
@@ -294,6 +294,7 @@ struct dwc2_core_params {
  */
 struct dwc2_hsotg {
 	struct device *dev;
+	u64 *dma_mask_save;
 	void __iomem *regs;
 	struct dwc2_core_params *core_params;
 	u32 hwcfg1;
diff --git a/drivers/staging/dwc2/hcd.c b/drivers/staging/dwc2/hcd.c
index c35583f..1f02f55 100644
--- a/drivers/staging/dwc2/hcd.c
+++ b/drivers/staging/dwc2/hcd.c
@@ -2668,6 +2668,10 @@ static void dwc2_hcd_free(struct dwc2_hsotg *hsotg)
 	kfree(hsotg->core_params);
 	hsotg->core_params = NULL;
 	del_timer(&hsotg->wkp_timer);
+
+	/* Restore dma_mask pointer if we NULLed it out in dwc2_hcd_init() */
+	if (!hsotg->dev->dma_mask)
+		hsotg->dev->dma_mask = hsotg->dma_mask_save;
 }
 
 static void dwc2_hcd_release(struct dwc2_hsotg *hsotg)
@@ -2819,6 +2823,7 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
 			dev_warn(hsotg->dev,
 				 "can't enable workaround for >2GB RAM\n");
 	} else {
+		hsotg->dma_mask_save = hsotg->dev->dma_mask;
 		hsotg->dev->dma_mask = NULL;
 		dma_set_coherent_mask(hsotg->dev, 0);
 	}


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux