Re: [PATCH] usb: dwc2: fix "iomem 0x00000000" message by setting iomem parameters in usb_hcd

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

 



Hi,

Heiner Kallweit <hkallweit1@xxxxxxxxx> writes:
> Set the iomem parameters in the usb_hcd to fix this misleading
> message during driver load:
> dwc2 c9100000.usb: irq 22, io mem 0x00000000
>
> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> ---
>  drivers/usb/dwc2/core.h     | 3 ++-
>  drivers/usb/dwc2/hcd.c      | 5 ++++-
>  drivers/usb/dwc2/hcd.h      | 3 ++-
>  drivers/usb/dwc2/platform.c | 2 +-
>  4 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 9548d3e0..b66eaeea 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -1229,7 +1229,8 @@ static inline void dwc2_hcd_connect(struct dwc2_hsotg *hsotg) {}
>  static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force) {}
>  static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
>  static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
> -static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
> +static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
> +				struct resource *res)
>  { return 0; }
>  static inline int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg)
>  { return 0; }
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 911c3b36..2cfbd10e 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -4964,7 +4964,7 @@ static void dwc2_hcd_release(struct dwc2_hsotg *hsotg)
>   * USB bus with the core and calls the hc_driver->start() function. It returns
>   * a negative error on failure.
>   */
> -int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
> +int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq, struct resource *res)
>  {
>  	struct usb_hcd *hcd;
>  	struct dwc2_host_chan *channel;
> @@ -5021,6 +5021,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
>  
>  	hcd->has_tt = 1;
>  
> +	hcd->rsrc_start = res->start;
> +	hcd->rsrc_len = resource_size(res);
> +
>  	((struct wrapper_priv_data *) &hcd->hcd_priv)->hsotg = hsotg;
>  	hsotg->priv = hcd;
>  
> diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
> index 1ed5fa2b..2305b5fb 100644
> --- a/drivers/usb/dwc2/hcd.h
> +++ b/drivers/usb/dwc2/hcd.h
> @@ -521,7 +521,8 @@ static inline u8 dwc2_hcd_is_pipe_out(struct dwc2_hcd_pipe_info *pipe)
>  	return !dwc2_hcd_is_pipe_in(pipe);
>  }
>  
> -extern int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq);
> +extern int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
> +			 struct resource *res);
>  extern void dwc2_hcd_remove(struct dwc2_hsotg *hsotg);
>  
>  /* Transaction Execution Functions */
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 4fc8c603..5ddc8860 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -445,7 +445,7 @@ static int dwc2_driver_probe(struct platform_device *dev)
>  	}
>  
>  	if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) {
> -		retval = dwc2_hcd_init(hsotg, hsotg->irq);
> +		retval = dwc2_hcd_init(hsotg, hsotg->irq, res);

This is a good idea, but there's more work that can come out of this, if
you're interested:

i) devm_ioremap_resource() could be called by the generic layer
ii) devm_request_irq() could be move to the generic layer
iii) dwc2_hcd_init() could also be called generically as long as dr_mode
     is set properly.
iv) dwc2_debugfs_init() could be called generically as well

IOW, dwc2_driver_probe() could be as minimal as:

static int dwc2_driver_probe(struct platform_device *dev)
{
	struct dwc2_hsotg *hsotg;
	struct resource *res;
	int retval;

	hsotg = devm_kzalloc(&dev->dev, sizeof(*hsotg), GFP_KERNEL);
	if (!hsotg)
		return -ENOMEM;

	hsotg->dev = &dev->dev;

	if (!dev->dev.dma_mask)
		dev->dev.dma_mask = &dev->dev.coherent_dma_mask;

	retval = dma_set_coherent_mask(&dev->dev, DMA_BIT_MASK(32));
	if (retval)
		return retval;

	hsotg->res = platform_get_resource(dev, IORESOURCE_MEM, 0);
	hsotg->irq = platform_get_irq(dev, 0);

	retval = dwc2_get_dr_mode(hsotg);
	if (retval)
		return retval;        

	retval = dwc2_get_hwparams(hsotg);
	if (retval)
        	return retval;

	platform_set_drvdata(dev, hsotg);

	retval = dwc2_core_init(hsotg);
	if (retval)
		return retval;

	return 0;
}

dwc2_core_init() needs to implemented, of course, but it could hide all
details about what to do with IRQs and resources and what not. Assuming
you can properly initialize clocks, it could even handle clocks
generically for you.

-- 
balbi

Attachment: signature.asc
Description: PGP signature


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

  Powered by Linux