Kevin, Thanks for reviewing this. See below for my response.
Murali Karicheri Software Design Engineer Texas Instruments Inc. Germantown, MD 20874 email: [email protected] <snip> >> >> #define VIDCH2CLK (BIT(10)) >> #define VIDCH3CLK (BIT(11)) >> +#define VIDCH1CLK (BIT(4)) >> +#define TVP7002_INPUT (BIT(4)) >> +#define TVP5147_INPUT (~BIT(4)) >> +#define VPIF_INPUT_ONE_CHANNEL (BIT(5)) >> +#define VPIF_INPUT_TWO_CHANNEL (~BIT(5)) > >Hmm, I think the usage of the ~BIT(x) definitions are not exactly >clear. > Is it not clear that for selecting one channel mode you need to set bit 5 and two channel mode requires it to be reset from this? This looks very obvious for me. >I'd rather just see the single BIT(x) definition, and code the >use of them differently... Other way is to define it as Logical OR of all BITs except BIT5. Is it what you looking for? This will be unnecessary IMHO. > >> +#define TVP5147_CH0 "tvp514x-0" >> +#define TVP5147_CH1 "tvp514x-1" >> + <Snip> >> + */ >> +static int setup_vpif_input_path(int channel, const char *sub_dev_name) >> +{ >> + int err = 0; >> + int val; >> + >> + /* for channel 1, we don't do anything */ >> + if (channel != 0) >> + return 0; >> + >> + if (NULL == cpld_client) >> + return -EFAULT; >> + >> + val = i2c_smbus_read_byte(cpld_client); >> + if (val < 0) >> + return val; >> + >> + if (!strcmp(sub_dev_name, TVP5147_CH0) || >> + !strcmp(sub_dev_name, TVP5147_CH1)) > >Hmm, shouldn't this be '&&' instead of '||'. > No. You want to do this for either TVP5147_CH0 or TVP5147_CH1. How can this be && since the strcmp returns zero when there is a match? >> + val &= TVP5147_INPUT; > >rather: > > val &= ~TVP7002_INPUT; /* tvp5147 input */ > TVP5147_INPUT is defined as a 0 for bit4. So val &= TVP5147_INPUT is correct. >> + else >> + val |= TVP7002_INPUT; >> + >> + err = i2c_smbus_write_byte(cpld_client, val); >> + if (err) >> + return err; >> + return 0; >> +} >> + >> +/** >> + * setup_vpif_input_channel_mode() >> + * @mux_mode: mux mode. 0 - 1 channel or (1) - 2 channel >> + * >> + * This will setup input mode to one channel (TVP7002) or 2 channel >(TVP5147) >> + */ >> +static int setup_vpif_input_channel_mode(int mux_mode) >> +{ >> + int err = 0; >> + int val; >> + u32 value; >> + void __iomem *base = IO_ADDRESS(DAVINCI_SYSTEM_MODULE_BASE); > >Use ioremap() + readl/writel instead of IO_ADDRESS() + >__raw_read/__raw_write. Ok. I will change this. > >> + val = i2c_smbus_read_byte(cpld_client); >> + if (val < 0) >> + return val; >> + >> + value = __raw_readl(base + VSCLKDIS_OFFSET); >> + if (mux_mode) { >> + val &= VPIF_INPUT_TWO_CHANNEL; >> + value |= VIDCH1CLK; >> + } else { >> + val |= VPIF_INPUT_ONE_CHANNEL; >> + value &= ~VIDCH1CLK; >> + } >> + __raw_writel(value, base + VSCLKDIS_OFFSET); >> + >> + err = i2c_smbus_write_byte(cpld_client, val); >> + if (err) >> + return err; >> + >> + return 0; >> +} >> + >> +static struct tvp514x_platform_data tvp5146_pdata = { >> + .clk_polarity = 0, >> + .hs_polarity = 1, >> + .vs_polarity = 1 >> +}; >> + >> +#define TVP514X_STD_ALL (V4L2_STD_NTSC | V4L2_STD_PAL) >> + >> +static struct vpif_subdev_info vpif_capture_sdev_info[] = { >> + { >> + .name = TVP5147_CH0, >> + .board_info = { >> + I2C_BOARD_INFO("tvp5146", 0x5d), >> + .platform_data = &tvp5146_pdata, >> + }, >> + .input = INPUT_CVBS_VI2B, >> + .output = OUTPUT_10BIT_422_EMBEDDED_SYNC, >> + .can_route = 1, >> + .vpif_if = { >> + .if_type = VPIF_IF_BT656, >> + .hd_pol = 1, >> + .vd_pol = 1, >> + .fid_pol = 0, >> + }, >> + }, >> + { >> + .name = TVP5147_CH1, >> + .board_info = { >> + I2C_BOARD_INFO("tvp5146", 0x5c), >> + .platform_data = &tvp5146_pdata, >> + }, >> + .input = INPUT_SVIDEO_VI2C_VI1C, >> + .output = OUTPUT_10BIT_422_EMBEDDED_SYNC, >> + .can_route = 1, >> + .vpif_if = { >> + .if_type = VPIF_IF_BT656, >> + .hd_pol = 1, >> + .vd_pol = 1, >> + .fid_pol = 0, >> + }, >> + }, >> +}; >> + >> +static const struct vpif_input dm6467_ch0_inputs[] = { >> + { >> + .input = { >> + .index = 0, >> + .name = "Composite", >> + .type = V4L2_INPUT_TYPE_CAMERA, >> + .std = TVP514X_STD_ALL, >> + }, >> + .subdev_name = TVP5147_CH0, >> + }, >> +}; >> + >> +static const struct vpif_input dm6467_ch1_inputs[] = { >> + { >> + .input = { >> + .index = 0, >> + .name = "S-Video", >> + .type = V4L2_INPUT_TYPE_CAMERA, >> + .std = TVP514X_STD_ALL, >> + }, >> + .subdev_name = TVP5147_CH1, >> + }, >> +}; >> + >> +static struct vpif_capture_config dm646x_vpif_capture_cfg = { >> + .setup_input_path = setup_vpif_input_path, >> + .setup_input_channel_mode = setup_vpif_input_channel_mode, >> + .subdev_info = vpif_capture_sdev_info, >> + .subdev_count = ARRAY_SIZE(vpif_capture_sdev_info), >> + .chan_config[0] = { >> + .inputs = dm6467_ch0_inputs, >> + .input_count = ARRAY_SIZE(dm6467_ch0_inputs), >> + }, >> + .chan_config[1] = { >> + .inputs = dm6467_ch1_inputs, >> + .input_count = ARRAY_SIZE(dm6467_ch1_inputs), >> + }, >> +}; >> + >> static void __init evm_init_i2c(void) >> { >> davinci_init_i2c(&i2c_pdata); >> @@ -453,7 +617,8 @@ static __init void evm_init(void) >> >> soc_info->emac_pdata->phy_mask = DM646X_EVM_PHY_MASK; >> soc_info->emac_pdata->mdio_max_freq = DM646X_EVM_MDIO_FREQUENCY; >> - dm646x_setup_vpif(&dm646x_vpif_config); >> + dm646x_setup_vpif(&dm646x_vpif_display_config, >> + &dm646x_vpif_capture_cfg); >> } >> >> static __init void davinci_dm646x_evm_irq_init(void) >> diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach- >davinci/dm646x.c >> index fc02f22..4ee3b6f 100644 >> --- a/arch/arm/mach-davinci/dm646x.c >> +++ b/arch/arm/mach-davinci/dm646x.c >> @@ -613,9 +613,23 @@ static u64 vpif_dma_mask = DMA_BIT_MASK(32); >> static struct resource vpif_resource[] = { >> { >> .start = DAVINCI_VPIF_BASE, >> - .end = DAVINCI_VPIF_BASE + 0x03fff, >> + .end = DAVINCI_VPIF_BASE + 0x03ff, >> .flags = IORESOURCE_MEM, >> + } >> +}; >> + >> +static struct platform_device vpif_dev = { >> + .name = "vpif", >> + .id = -1, >> + .dev = { >> + .dma_mask = &vpif_dma_mask, >> + .coherent_dma_mask = DMA_BIT_MASK(32), >> }, >> + .resource = vpif_resource, >> + .num_resources = ARRAY_SIZE(vpif_resource), >> +}; >> + >> +static struct resource vpif_display_resource[] = { >> { >> .start = IRQ_DM646X_VP_VERTINT2, >> .end = IRQ_DM646X_VP_VERTINT2, >> @@ -633,10 +647,34 @@ static struct platform_device vpif_display_dev = { >> .id = -1, >> .dev = { >> .dma_mask = &vpif_dma_mask, >> - .coherent_dma_mask = DMA_32BIT_MASK, >> + .coherent_dma_mask = DMA_BIT_MASK(32), >> }, >> - .resource = vpif_resource, >> - .num_resources = ARRAY_SIZE(vpif_resource), >> + .resource = vpif_display_resource, >> + .num_resources = ARRAY_SIZE(vpif_display_resource), >> +}; >> + >> +static struct resource vpif_capture_resource[] = { >> + { >> + .start = IRQ_DM646X_VP_VERTINT0, >> + .end = IRQ_DM646X_VP_VERTINT0, >> + .flags = IORESOURCE_IRQ, >> + }, >> + { >> + .start = IRQ_DM646X_VP_VERTINT1, >> + .end = IRQ_DM646X_VP_VERTINT1, >> + .flags = IORESOURCE_IRQ, >> + }, >> +}; >> + >> +static struct platform_device vpif_capture_dev = { >> + .name = "vpif_capture", >> + .id = -1, >> + .dev = { >> + .dma_mask = &vpif_dma_mask, >> + .coherent_dma_mask = DMA_BIT_MASK(32), >> + }, >> + .resource = vpif_capture_resource, >> + .num_resources = ARRAY_SIZE(vpif_capture_resource), >> }; >> >> static struct resource ide_resources[] = { >> @@ -853,7 +891,8 @@ void __init dm646x_init_mcasp1(struct >snd_platform_data *pdata) >> platform_device_register(&dm646x_dit_device); >> } >> >> -void dm646x_setup_vpif(struct vpif_config *config) >> +void dm646x_setup_vpif(struct vpif_display_config *display_config, >> + struct vpif_capture_config *capture_config) >> { >> unsigned int value; >> void __iomem *base = IO_ADDRESS(DAVINCI_SYSTEM_MODULE_BASE); >> @@ -871,8 +910,11 @@ void dm646x_setup_vpif(struct vpif_config *config) >> davinci_cfg_reg(DM646X_PTSOMUX_DISABLE); >> davinci_cfg_reg(DM646X_PTSIMUX_DISABLE); >> >> - vpif_display_dev.dev.platform_data = config; >> + vpif_display_dev.dev.platform_data = display_config; >> + vpif_capture_dev.dev.platform_data = capture_config; >> + platform_device_register(&vpif_dev); >> platform_device_register(&vpif_display_dev); >> + platform_device_register(&vpif_capture_dev); >> } >> >> void __init dm646x_init(void) >> diff --git a/arch/arm/mach-davinci/include/mach/dm646x.h b/arch/arm/mach- >davinci/include/mach/dm646x.h >> index 737b549..b639142 100644 >> --- a/arch/arm/mach-davinci/include/mach/dm646x.h >> +++ b/arch/arm/mach-davinci/include/mach/dm646x.h >> @@ -14,6 +14,8 @@ >> #include <mach/hardware.h> >> #include <mach/emac.h> >> #include <mach/asp.h> >> +#include <linux/i2c.h> >> +#include <linux/videodev2.h> >> >> #define DM646X_EMAC_BASE (0x01C80000) >> #define DM646X_EMAC_CNTRL_OFFSET (0x0000) >> @@ -24,30 +26,64 @@ >> >> #define DM646X_ATA_REG_BASE (0x01C66000) >> >> -struct vpif_output { >> - u16 id; >> - const char *name; >> +enum vpif_if_type { >> + VPIF_IF_BT656, >> + VPIF_IF_BT1120, >> + VPIF_IF_RAW_BAYER >> +}; >> + >> +struct vpif_interface { >> + enum vpif_if_type if_type; >> + unsigned hd_pol:1; >> + unsigned vd_pol:1; >> + unsigned fid_pol:1; >> }; >> >> struct vpif_subdev_info { >> - unsigned short addr; >> const char *name; >> + struct i2c_board_info board_info; >> + u32 input; >> + u32 output; >> + unsigned can_route:1; >> + struct vpif_interface vpif_if; >> }; >> >> -struct vpif_config { >> +struct vpif_display_config { >> int (*set_clock)(int, int); >> - const struct vpif_subdev_info *subdevinfo; >> + struct vpif_subdev_info *subdevinfo; >> int subdev_count; >> const char **output; >> int output_count; >> const char *card_name; >> }; >> >> +struct vpif_input { >> + struct v4l2_input input; >> + const char *subdev_name; >> +}; >> + >> +#define VPIF_CAPTURE_MAX_CHANNELS 2 >> + >> +struct vpif_capture_chan_config { >> + const struct vpif_input *inputs; >> + int input_count; >> +}; >> + >> +struct vpif_capture_config { >> + int (*setup_input_channel_mode)(int); >> + int (*setup_input_path)(int, const char *); >> + struct vpif_capture_chan_config >chan_config[VPIF_CAPTURE_MAX_CHANNELS]; >> + struct vpif_subdev_info *subdev_info; >> + int subdev_count; >> + const char *card_name; >> +}; >> + >> void __init dm646x_init(void); >> void __init dm646x_init_ide(void); >> void __init dm646x_init_mcasp0(struct snd_platform_data *pdata); >> void __init dm646x_init_mcasp1(struct snd_platform_data *pdata); >> void dm646x_video_init(void); >> -void dm646x_setup_vpif(struct vpif_config *config); >> +void dm646x_setup_vpif(struct vpif_display_config *, >> + struct vpif_capture_config *); >> >> #endif /* __ASM_ARCH_DM646X_H */ >> -- >> 1.6.0.4 _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
