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

[Heiner Kallweit]

Am 25.01.2017 um 22:28 schrieb John Youn:
> On 1/15/2017 12:37 PM, Heiner Kallweit wrote:
>> Set the iomem parameters in the usb_hcd to fix this misleading
>> message during driver load:
>> dwc2 c910.usb: irq 22, io mem 0x
>>
>> Signed-off-by: Heiner Kallweit 
>> ---
>>  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);
> 
> You should be able to get the same from hsotg->dev instead of adding a
> paramter.
> 
> In fact, looks like the irq parameter is not needed either since it is 
> already stored in hsotg and can anyway get it from dev as well.
> 
> John
> 
Indeed. I'll send a v2 for the patch and another one for getting rid of
the irq function parameter.

Heiner

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

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

[John Youn]

On 1/15/2017 12:37 PM, Heiner Kallweit wrote:
> Set the iomem parameters in the usb_hcd to fix this misleading
> message during driver load:
> dwc2 c910.usb: irq 22, io mem 0x
>
> Signed-off-by: Heiner Kallweit 
> ---
>  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);

You should be able to get the same from hsotg->dev instead of adding a
paramter.

In fact, looks like the irq parameter is not needed either since it is 
already stored in hsotg and can anyway get it from dev as well.

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

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

[John Youn]

On 1/25/2017 12:37 PM, Heiner Kallweit wrote:
> Am 24.01.2017 um 09:46 schrieb Felipe Balbi:
>>
>> Hi,
>>
>> John Youn  writes:
 John Youn  writes:
> @@ -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_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, sizeof(*hsotg), GFP_KERNEL);
if (!hsotg)
return -ENOMEM;

hsotg->dev = >dev;

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

retval = dma_set_coherent_mask(>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)

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

[Heiner Kallweit]

Am 24.01.2017 um 09:46 schrieb Felipe Balbi:
> 
> Hi,
> 
> John Youn  writes:
>>> John Youn  writes:
 @@ -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_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, sizeof(*hsotg), GFP_KERNEL);
>>> if (!hsotg)
>>> return -ENOMEM;
>>>
>>> hsotg->dev = >dev;
>>>
>>> if (!dev->dev.dma_mask)
>>> dev->dev.dma_mask = >dev.coherent_dma_mask;
>>>
>>> retval = dma_set_coherent_mask(>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
>>> 

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

[Felipe Balbi]

Hi,

John Youn  writes:
>> John Youn  writes:
>>> @@ -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_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, sizeof(*hsotg), GFP_KERNEL);
>>  if (!hsotg)
>>  return -ENOMEM;
>>
>>  hsotg->dev = >dev;
>>
>>  if (!dev->dev.dma_mask)
>>  dev->dev.dma_mask = >dev.coherent_dma_mask;
>>
>>  retval = dma_set_coherent_mask(>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.
>>
> I see what you mean. I'm just a little confused about the term "generic" 
> here:
> For me something generic has more than one user. Here we 

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

[John Youn]

On 1/23/2017 3:50 AM, Felipe Balbi wrote:
>
> Hi,
>
> John Youn  writes:
>> @@ -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_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, sizeof(*hsotg), GFP_KERNEL);
>   if (!hsotg)
>   return -ENOMEM;
>
>   hsotg->dev = >dev;
>
>   if (!dev->dev.dma_mask)
>   dev->dev.dma_mask = >dev.coherent_dma_mask;
>
>   retval = dma_set_coherent_mask(>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.
>
 I see what you mean. I'm just a little confused about the term "generic" 
 here:
 For me something generic has more than one user. Here we seem to speak just
 about factoring out some code from the probe function, or?

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

[John Youn]

On 1/17/2017 12:13 AM, Felipe Balbi wrote:
>
> Hi,
>
> Heiner Kallweit  writes:
>> Am 16.01.2017 um 15:05 schrieb Felipe Balbi:
>>>
>>> Hi,
>>>
>>> Heiner Kallweit  writes:
 Set the iomem parameters in the usb_hcd to fix this misleading
 message during driver load:
 dwc2 c910.usb: irq 22, io mem 0x

 Signed-off-by: Heiner Kallweit 
 ---
  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_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, sizeof(*hsotg), GFP_KERNEL);
>>> if (!hsotg)
>>> return -ENOMEM;
>>>
>>> hsotg->dev = >dev;
>>>
>>> if (!dev->dev.dma_mask)
>>> dev->dev.dma_mask = >dev.coherent_dma_mask;
>>>
>>> retval = dma_set_coherent_mask(>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 

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

[Heiner Kallweit]

Am 17.01.2017 um 21:08 schrieb Heiner Kallweit:
> Am 17.01.2017 um 09:11 schrieb Felipe Balbi:
>>
>> Hi,
>>
>> Heiner Kallweit  writes:
>>> Am 16.01.2017 um 15:05 schrieb Felipe Balbi:

 Hi,

 Heiner Kallweit  writes:
> Set the iomem parameters in the usb_hcd to fix this misleading
> message during driver load:
> dwc2 c910.usb: irq 22, io mem 0x
>
> Signed-off-by: Heiner Kallweit 
> ---
>  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_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, sizeof(*hsotg), GFP_KERNEL);
if (!hsotg)
return -ENOMEM;

hsotg->dev = >dev;

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

retval = dma_set_coherent_mask(>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;

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

[Heiner Kallweit]

Am 17.01.2017 um 09:11 schrieb Felipe Balbi:
> 
> Hi,
> 
> Heiner Kallweit  writes:
>> Am 16.01.2017 um 15:05 schrieb Felipe Balbi:
>>>
>>> Hi,
>>>
>>> Heiner Kallweit  writes:
 Set the iomem parameters in the usb_hcd to fix this misleading
 message during driver load:
 dwc2 c910.usb: irq 22, io mem 0x

 Signed-off-by: Heiner Kallweit 
 ---
  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_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, sizeof(*hsotg), GFP_KERNEL);
>>> if (!hsotg)
>>> return -ENOMEM;
>>>
>>> hsotg->dev = >dev;
>>>
>>> if (!dev->dev.dma_mask)
>>> dev->dev.dma_mask = >dev.coherent_dma_mask;
>>>
>>> retval = dma_set_coherent_mask(>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;
>>> }
>>>

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

[Felipe Balbi]

Hi,

Heiner Kallweit  writes:
> Am 16.01.2017 um 15:05 schrieb Felipe Balbi:
>> 
>> Hi,
>> 
>> Heiner Kallweit  writes:
>>> Set the iomem parameters in the usb_hcd to fix this misleading
>>> message during driver load:
>>> dwc2 c910.usb: irq 22, io mem 0x
>>>
>>> Signed-off-by: Heiner Kallweit 
>>> ---
>>>  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_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, sizeof(*hsotg), GFP_KERNEL);
>>  if (!hsotg)
>>  return -ENOMEM;
>> 
>>  hsotg->dev = >dev;
>> 
>>  if (!dev->dev.dma_mask)
>>  dev->dev.dma_mask = >dev.coherent_dma_mask;
>> 
>>  retval = dma_set_coherent_mask(>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 

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

[Heiner Kallweit]

Am 16.01.2017 um 15:05 schrieb Felipe Balbi:
> 
> Hi,
> 
> Heiner Kallweit  writes:
>> Set the iomem parameters in the usb_hcd to fix this misleading
>> message during driver load:
>> dwc2 c910.usb: irq 22, io mem 0x
>>
>> Signed-off-by: Heiner Kallweit 
>> ---
>>  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_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, sizeof(*hsotg), GFP_KERNEL);
>   if (!hsotg)
>   return -ENOMEM;
> 
>   hsotg->dev = >dev;
> 
>   if (!dev->dev.dma_mask)
>   dev->dev.dma_mask = >dev.coherent_dma_mask;
> 
>   retval = dma_set_coherent_mask(>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.
> 
I see what you mean. I'm just a little confused 

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

[Felipe Balbi]

Hi,

Heiner Kallweit  writes:
> Set the iomem parameters in the usb_hcd to fix this misleading
> message during driver load:
> dwc2 c910.usb: irq 22, io mem 0x
>
> Signed-off-by: Heiner Kallweit 
> ---
>  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_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, sizeof(*hsotg), GFP_KERNEL);
if (!hsotg)
return -ENOMEM;

hsotg->dev = >dev;

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

retval = dma_set_coherent_mask(>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


signature.asc
Description: PGP signature

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

[Heiner Kallweit]

Set the iomem parameters in the usb_hcd to fix this misleading
message during driver load:
dwc2 c910.usb: irq 22, io mem 0x

Signed-off-by: Heiner Kallweit 
---
 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_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);
if (retval) {
if (hsotg->gadget_enabled)
dwc2_hsotg_remove(hsotg);
-- 
2.11.0

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