Kevin,

My re-structuring patch would be along similar lines.  There are cople of 
references in the usb.c that need to be re-done (platform specific memorymaps 
for ex) to be generic across platforms.

I am delayed on this patch due to other priorities currently.  Will put in an 
RFC by this week.

regards
swami

________________________________________
From: [email protected] 
[[email protected]] On Behalf Of Kevin 
Hilman [[email protected]]
Sent: Tuesday, September 15, 2009 1:36 AM
To: Sergei Shtylyov
Cc: [email protected]; [email protected]
Subject: Re: [PATCH 1/3] DaVinci: move MUSB platform device to devices.c

Sergei Shtylyov <[email protected]> writes:

> There's absolutely no reason why the DaVinci USB platfrom device should
> be in its own module;

Other than devices*.c getting very cluttered, difficult to maintain
and the root cause of merge conflicts because of so many unrleated
patches wanting to touch the same files.

For this reason, as well as to share more between DMx and DA8xx, I
would like to move to a model where devices.c is split up into usb.c,
mmc.c, etc., so I don't like this change.

> moreover, it will stand in the way of DA8xx's USB
> platfrom device which occupies different region and IRQ -- so, move it
> into devices.c and get rid of usb.c...

I'd rather see both DMx and da8xx consolidated into usb.c.  In your
patches, there is lots of duplication between those files.

Then usb.c would have all the resource and platform_data for all devices
and the interface would be:

  davinci_register_usb()   -- renamed from setup_usb()
  da8xx_register_usb11()
  da8xx_register_usb20()

Kevin

> While at it, add 'davinci_' prefix to setup_usb(), remove its duplicate
> declaration in common.h, and rename 'usb_dev' to 'davinci_usb_device' to
> match the naming pattern established for devices.c...
>
> Signed-off-by: Sergei Shtylyov <[email protected]>
>
> ---
> The patch is against the recent DaVinci tree...
>
>  arch/arm/mach-davinci/usb.c                 |  110 
> ----------------------------
>  arch/arm/mach-davinci/Makefile              |    2
>  arch/arm/mach-davinci/board-dm355-evm.c     |    2
>  arch/arm/mach-davinci/board-dm355-leopard.c |    2
>  arch/arm/mach-davinci/board-dm644x-evm.c    |    2
>  arch/arm/mach-davinci/board-sffsdr.c        |    2
>  arch/arm/mach-davinci/devices.c             |   94 +++++++++++++++++++++++
>  arch/arm/mach-davinci/include/mach/common.h |    5 -
>  8 files changed, 100 insertions(+), 119 deletions(-)
>
> Index: linux-davinci/arch/arm/mach-davinci/Makefile
> ===================================================================
> --- linux-davinci.orig/arch/arm/mach-davinci/Makefile
> +++ linux-davinci/arch/arm/mach-davinci/Makefile
> @@ -5,7 +5,7 @@
>
>  # Common objects
>  obj-y                        := time.o clock.o serial.o io.o psc.o \
> -                        gpio.o dma.o usb.o common.o sram.o
> +                        gpio.o dma.o common.o sram.o
>
>  obj-$(CONFIG_DAVINCI_MUX)            += mux.o
>
> Index: linux-davinci/arch/arm/mach-davinci/board-dm355-evm.c
> ===================================================================
> --- linux-davinci.orig/arch/arm/mach-davinci/board-dm355-evm.c
> +++ linux-davinci/arch/arm/mach-davinci/board-dm355-evm.c
> @@ -275,7 +275,7 @@ static __init void dm355_evm_init(void)
>       gpio_request(2, "usb_id_toggle");
>       gpio_direction_output(2, USB_ID_VALUE);
>       /* irlml6401 switches over 1A in under 8 msec */
> -     setup_usb(500, 8);
> +     davinci_setup_usb(500, 8);
>
>       davinci_setup_mmc(0, &dm355evm_mmc_config);
>       davinci_setup_mmc(1, &dm355evm_mmc_config);
> Index: linux-davinci/arch/arm/mach-davinci/board-dm355-leopard.c
> ===================================================================
> --- linux-davinci.orig/arch/arm/mach-davinci/board-dm355-leopard.c
> +++ linux-davinci/arch/arm/mach-davinci/board-dm355-leopard.c
> @@ -271,7 +271,7 @@ static __init void dm355_leopard_init(vo
>       gpio_request(2, "usb_id_toggle");
>       gpio_direction_output(2, USB_ID_VALUE);
>       /* irlml6401 switches over 1A in under 8 msec */
> -     setup_usb(500, 8);
> +     davinci_setup_usb(500, 8);
>
>       davinci_setup_mmc(0, &dm355leopard_mmc_config);
>       davinci_setup_mmc(1, &dm355leopard_mmc_config);
> Index: linux-davinci/arch/arm/mach-davinci/board-dm644x-evm.c
> ===================================================================
> --- linux-davinci.orig/arch/arm/mach-davinci/board-dm644x-evm.c
> +++ linux-davinci/arch/arm/mach-davinci/board-dm644x-evm.c
> @@ -409,7 +409,7 @@ evm_u35_setup(struct i2c_client *client,
>       /* irlml6401 switches over 1A, in under 8 msec;
>        * now it can be managed by nDRV_VBUS ...
>        */
> -     setup_usb(500, 8);
> +     davinci_setup_usb(500, 8);
>
>       return 0;
>  }
> Index: linux-davinci/arch/arm/mach-davinci/board-sffsdr.c
> ===================================================================
> --- linux-davinci.orig/arch/arm/mach-davinci/board-sffsdr.c
> +++ linux-davinci/arch/arm/mach-davinci/board-sffsdr.c
> @@ -165,7 +165,7 @@ static __init void davinci_sffsdr_init(v
>       davinci_serial_init(&uart_config);
>       soc_info->emac_pdata->phy_mask = SFFSDR_PHY_MASK;
>       soc_info->emac_pdata->mdio_max_freq = SFFSDR_MDIO_FREQUENCY;
> -     setup_usb(0, 0); /* We support only peripheral mode. */
> +     davinci_setup_usb(0, 0); /* We support only peripheral mode. */
>
>       /* mux VLYNQ pins */
>       davinci_cfg_reg(DM644X_VLYNQEN);
> Index: linux-davinci/arch/arm/mach-davinci/devices.c
> ===================================================================
> --- linux-davinci.orig/arch/arm/mach-davinci/devices.c
> +++ linux-davinci/arch/arm/mach-davinci/devices.c
> @@ -15,6 +15,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/io.h>
> +#include <linux/usb/musb.h>
>
>  #include <asm/mach/map.h>
>
> @@ -28,6 +29,7 @@
>  #include <mach/time.h>
>
>  #define DAVINCI_I2C_BASE          0x01C21000
> +#define DAVINCI_USB_OTG_BASE      0x01C64000
>  #define DAVINCI_MMCSD0_BASE       0x01E10000
>  #define DM355_MMCSD0_BASE         0x01E11000
>  #define DM355_MMCSD1_BASE         0x01E00000
> @@ -62,6 +64,98 @@ void __init davinci_init_i2c(struct davi
>       (void) platform_device_register(&davinci_i2c_device);
>  }
>
> +#if defined(CONFIG_USB_MUSB_HDRC) || defined(CONFIG_USB_MUSB_HDRC_MODULE)
> +
> +static struct musb_hdrc_eps_bits musb_eps[] = {
> +     { "ep1_tx", 8, },
> +     { "ep1_rx", 8, },
> +     { "ep2_tx", 8, },
> +     { "ep2_rx", 8, },
> +     { "ep3_tx", 5, },
> +     { "ep3_rx", 5, },
> +     { "ep4_tx", 5, },
> +     { "ep4_rx", 5, },
> +};
> +
> +static struct musb_hdrc_config musb_config = {
> +     .multipoint     = true,
> +     .dyn_fifo       = true,
> +     .soft_con       = true,
> +     .dma            = true,
> +
> +     .num_eps        = 5,
> +     .dma_channels   = 8,
> +     .ram_bits       = 10,
> +     .eps_bits       = musb_eps,
> +};
> +
> +static struct musb_hdrc_platform_data usb_data = {
> +#if defined(CONFIG_USB_MUSB_OTG)
> +     /* OTG requires a Mini-AB connector */
> +     .mode           = MUSB_OTG,
> +#elif defined(CONFIG_USB_MUSB_PERIPHERAL)
> +     .mode           = MUSB_PERIPHERAL,
> +#elif defined(CONFIG_USB_MUSB_HOST)
> +     .mode           = MUSB_HOST,
> +#endif
> +     .clock          = "usb",
> +     .config         = &musb_config,
> +};
> +
> +static struct resource usb_resources[] = {
> +     {
> +             /* physical address */
> +             .start          = DAVINCI_USB_OTG_BASE,
> +             .end            = DAVINCI_USB_OTG_BASE + 0x5ff,
> +             .flags          = IORESOURCE_MEM,
> +     },
> +     {
> +             .start          = IRQ_USBINT,
> +             .flags          = IORESOURCE_IRQ,
> +     },
> +     {
> +             /* placeholder for the dedicated CPPI IRQ */
> +             .flags          = IORESOURCE_IRQ,
> +     },
> +};
> +
> +static u64 usb_dmamask = DMA_BIT_MASK(32);
> +
> +static struct platform_device davinci_usb_device = {
> +     .name           = "musb_hdrc",
> +     .id             = -1,
> +     .dev = {
> +             .platform_data          = &usb_data,
> +             .dma_mask               = &usb_dmamask,
> +             .coherent_dma_mask      = DMA_BIT_MASK(32),
> +     },
> +     .resource       = usb_resources,
> +     .num_resources  = ARRAY_SIZE(usb_resources),
> +};
> +
> +void __init davinci_setup_usb(unsigned mA, unsigned potpgt_msec)
> +{
> +     usb_data.power = mA / 2;
> +     usb_data.potpgt = potpgt_msec / 2;
> +
> +     if (cpu_is_davinci_dm646x()) {
> +             /* Override the defaults as DM6467 uses different IRQs. */
> +             davinci_usb_device.resource[1].start = IRQ_DM646X_USBINT;
> +             davinci_usb_device.resource[2].start = IRQ_DM646X_USBDMAINT;
> +     } else  /* other devices don't have dedicated CPPI IRQ */
> +             davinci_usb_device.num_resources = 2;
> +
> +     platform_device_register(&davinci_usb_device);
> +}
> +
> +#else
> +
> +void __init davinci_setup_usb(unsigned mA, unsigned potpgt_msec)
> +{
> +}
> +
> +#endif       /* CONFIG_USB_MUSB_HDRC */
> +
>  #if  defined(CONFIG_MMC_DAVINCI) || defined(CONFIG_MMC_DAVINCI_MODULE)
>
>  static u64 mmcsd0_dma_mask = DMA_BIT_MASK(32);
> Index: linux-davinci/arch/arm/mach-davinci/include/mach/common.h
> ===================================================================
> --- linux-davinci.orig/arch/arm/mach-davinci/include/mach/common.h
> +++ linux-davinci/arch/arm/mach-davinci/include/mach/common.h
> @@ -21,10 +21,7 @@ extern void __iomem *davinci_intc_base;
>  extern int davinci_intc_type;
>
>  /* parameters describe VBUS sourcing for host mode */
> -extern void setup_usb(unsigned mA, unsigned potpgt_msec);
> -
> -/* parameters describe VBUS sourcing for host mode */
> -extern void setup_usb(unsigned mA, unsigned potpgt_msec);
> +void davinci_setup_usb(unsigned mA, unsigned potpgt_msec);
>
>  struct davinci_timer_instance {
>       void __iomem    *base;
> Index: linux-davinci/arch/arm/mach-davinci/usb.c
> ===================================================================
> --- linux-davinci.orig/arch/arm/mach-davinci/usb.c
> +++ /dev/null
> @@ -1,110 +0,0 @@
> -/*
> - * USB
> - */
> -#include <linux/kernel.h>
> -#include <linux/module.h>
> -#include <linux/init.h>
> -#include <linux/platform_device.h>
> -#include <linux/dma-mapping.h>
> -
> -#include <linux/usb/musb.h>
> -#include <linux/usb/otg.h>
> -
> -#include <mach/common.h>
> -#include <mach/hardware.h>
> -#include <mach/irqs.h>
> -#include <mach/cputype.h>
> -
> -#define DAVINCI_USB_OTG_BASE 0x01C64000
> -
> -#if defined(CONFIG_USB_MUSB_HDRC) || defined(CONFIG_USB_MUSB_HDRC_MODULE)
> -static struct musb_hdrc_eps_bits musb_eps[] = {
> -     { "ep1_tx", 8, },
> -     { "ep1_rx", 8, },
> -     { "ep2_tx", 8, },
> -     { "ep2_rx", 8, },
> -     { "ep3_tx", 5, },
> -     { "ep3_rx", 5, },
> -     { "ep4_tx", 5, },
> -     { "ep4_rx", 5, },
> -};
> -
> -static struct musb_hdrc_config musb_config = {
> -     .multipoint     = true,
> -     .dyn_fifo       = true,
> -     .soft_con       = true,
> -     .dma            = true,
> -
> -     .num_eps        = 5,
> -     .dma_channels   = 8,
> -     .ram_bits       = 10,
> -     .eps_bits       = musb_eps,
> -};
> -
> -static struct musb_hdrc_platform_data usb_data = {
> -#if defined(CONFIG_USB_MUSB_OTG)
> -     /* OTG requires a Mini-AB connector */
> -     .mode           = MUSB_OTG,
> -#elif defined(CONFIG_USB_MUSB_PERIPHERAL)
> -     .mode           = MUSB_PERIPHERAL,
> -#elif defined(CONFIG_USB_MUSB_HOST)
> -     .mode           = MUSB_HOST,
> -#endif
> -     .clock          = "usb",
> -     .config         = &musb_config,
> -};
> -
> -static struct resource usb_resources[] = {
> -     {
> -             /* physical address */
> -             .start          = DAVINCI_USB_OTG_BASE,
> -             .end            = DAVINCI_USB_OTG_BASE + 0x5ff,
> -             .flags          = IORESOURCE_MEM,
> -     },
> -     {
> -             .start          = IRQ_USBINT,
> -             .flags          = IORESOURCE_IRQ,
> -     },
> -     {
> -             /* placeholder for the dedicated CPPI IRQ */
> -             .flags          = IORESOURCE_IRQ,
> -     },
> -};
> -
> -static u64 usb_dmamask = DMA_BIT_MASK(32);
> -
> -static struct platform_device usb_dev = {
> -     .name           = "musb_hdrc",
> -     .id             = -1,
> -     .dev = {
> -             .platform_data          = &usb_data,
> -             .dma_mask               = &usb_dmamask,
> -             .coherent_dma_mask      = DMA_BIT_MASK(32),
> -     },
> -     .resource       = usb_resources,
> -     .num_resources  = ARRAY_SIZE(usb_resources),
> -};
> -
> -void __init setup_usb(unsigned mA, unsigned potpgt_msec)
> -{
> -     usb_data.power = mA / 2;
> -     usb_data.potpgt = potpgt_msec / 2;
> -
> -     if (cpu_is_davinci_dm646x()) {
> -             /* Override the defaults as DM6467 uses different IRQs. */
> -             usb_dev.resource[1].start = IRQ_DM646X_USBINT;
> -             usb_dev.resource[2].start = IRQ_DM646X_USBDMAINT;
> -     } else  /* other devices don't have dedicated CPPI IRQ */
> -             usb_dev.num_resources = 2;
> -
> -     platform_device_register(&usb_dev);
> -}
> -
> -#else
> -
> -void __init setup_usb(unsigned mA, unsigned potpgt_msec)
> -{
> -}
> -
> -#endif  /* CONFIG_USB_MUSB_HDRC */
> -
>
>
> _______________________________________________
> Davinci-linux-open-source mailing list
> [email protected]
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to