Re: [PATCH/RFC] pxa_set_udc_parent

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

 



Daniel Ribeiro <drwyrm@xxxxxxxxx> writes:
> The UDC may be connected to an external USB transceiver on SPI or I2c, this
> patch allows setting the pxa2xx_udc parent to assert proper suspend/resume
> ordering.
>
> Signed-off-by: Daniel Ribeiro <drwyrm@xxxxxxxxx>
>
> ---
>  arch/arm/mach-pxa/devices.c          |   15 ++++++++++-----
>  arch/arm/mach-pxa/include/mach/udc.h |    1 +
>  arch/arm/mach-pxa/pxa25x.c           |    1 -
>  arch/arm/mach-pxa/pxa27x.c           |    1 -
>  4 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c
> index d245e59..db0228d 100644
> --- a/arch/arm/mach-pxa/devices.c
> +++ b/arch/arm/mach-pxa/devices.c
> @@ -15,6 +15,7 @@
>  #include <mach/camera.h>
>  #include <mach/audio.h>
>  #include <mach/pxa3xx_nand.h>
> +#include <mach/hardware.h>
>  
>  #include "devices.h"
>  #include "generic.h"
> @@ -71,12 +72,18 @@ void __init pxa_set_mci_info(struct pxamci_platform_data *info)
>  	pxa_register_device(&pxa_device_mci, info);
>  }
>  
> -
> -static struct pxa2xx_udc_mach_info pxa_udc_info;
> +void __init pxa_set_udc_parent(struct device *parent_dev)
> +{
> +	pxa25x_device_udc.dev.parent = parent_dev;
> +	pxa27x_device_udc.dev.parent = parent_dev;
> +}
>  
>  void __init pxa_set_udc_info(struct pxa2xx_udc_mach_info *info)
>  {
> -	memcpy(&pxa_udc_info, info, sizeof *info);
> +	if (cpu_is_pxa27x())
> +		pxa_register_device(&pxa27x_device_udc, info);
> +	else if (cpu_is_pxa25x())
> +		pxa_register_device(&pxa25x_device_udc, info);
>  }
>  
>  static struct resource pxa2xx_udc_resources[] = {
> @@ -100,7 +107,6 @@ struct platform_device pxa25x_device_udc = {
>  	.resource	= pxa2xx_udc_resources,
>  	.num_resources	= ARRAY_SIZE(pxa2xx_udc_resources),
>  	.dev		=  {
> -		.platform_data	= &pxa_udc_info,
>  		.dma_mask	= &udc_dma_mask,
>  	}
>  };
> @@ -111,7 +117,6 @@ struct platform_device pxa27x_device_udc = {
>  	.resource	= pxa2xx_udc_resources,
>  	.num_resources	= ARRAY_SIZE(pxa2xx_udc_resources),
>  	.dev		=  {
> -		.platform_data	= &pxa_udc_info,
>  		.dma_mask	= &udc_dma_mask,
>  	}
>  };
> diff --git a/arch/arm/mach-pxa/include/mach/udc.h b/arch/arm/mach-pxa/include/mach/udc.h
> index 2f82332..6378f30 100644
> --- a/arch/arm/mach-pxa/include/mach/udc.h
> +++ b/arch/arm/mach-pxa/include/mach/udc.h
> @@ -5,4 +5,5 @@
>  #include <asm/mach/udc_pxa2xx.h>
>  
>  extern void pxa_set_udc_info(struct pxa2xx_udc_mach_info *info);
> +extern void pxa_set_udc_parent(struct device *parent_dev);
>  
> diff --git a/arch/arm/mach-pxa/pxa25x.c b/arch/arm/mach-pxa/pxa25x.c
> index 77c2693..228ed74 100644
> --- a/arch/arm/mach-pxa/pxa25x.c
> +++ b/arch/arm/mach-pxa/pxa25x.c
> @@ -321,7 +321,6 @@ void __init pxa26x_init_irq(void)
>  #endif
>  
>  static struct platform_device *pxa25x_devices[] __initdata = {
> -	&pxa25x_device_udc,
>  	&pxa_device_ffuart,
>  	&pxa_device_btuart,
>  	&pxa_device_stuart,
> diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
> index a425ec7..7dd8bcf 100644
> --- a/arch/arm/mach-pxa/pxa27x.c
> +++ b/arch/arm/mach-pxa/pxa27x.c
> @@ -346,7 +346,6 @@ void __init pxa27x_set_i2c_power_info(struct i2c_pxa_platform_data *info)
>  }
>  
>  static struct platform_device *devices[] __initdata = {
> -	&pxa27x_device_udc,
>  	&pxa_device_ffuart,
>  	&pxa_device_btuart,
>  	&pxa_device_stuart,
> -- 
> tg: (577c9c4..) pxa/set_udc_parent (depends on: master)
> total: 0 errors, 0 warnings, 61 lines checked
>
> pxa_set_udc_parent.patch has no obvious style problems and is ready for submission.

I cross-posted that to linux-usb mailing list, where IMO this matter should be
also discussed. Just for my understanding, would you explain to me why the order
is important ?

My naive view was that when suspend is triggered, you have either :
 (1)
   - pxa27x_udc suspend() called => udc is disabled
   - transceiver suspend() called => lines are disconnected
 (2)
   - transceiver suspend() called => line are disconnected
     => pxa27x_udc doesn't receive anymore date
     => packet loss, but that's acceptable since we disable udc anyway
   - pxa27x_udc suspend() called => udc is disabled

I don't see why order is important in that case. I understand the conceptual
idea, I just don't understand why.

Cheers.

--
Robert
--
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