Hi Sakari, Thanks for the quick review.
On Thu, Nov 29, 2012 at 1:38 AM, Sakari Ailus <[email protected]> wrote: > On Wed, Nov 28, 2012 at 04:25:32PM +0530, Prabhakar Lad wrote: >> From: Manjunath Hadli <[email protected]> >> >> enable PPCR, enbale ISIF out on BCR and disable all events to >> get the correct operation from ISIF. >> >> Signed-off-by: Manjunath Hadli <[email protected]> >> Signed-off-by: Lad, Prabhakar <[email protected]> >> --- >> drivers/media/platform/davinci/vpss.c | 13 +++++++++++++ >> 1 files changed, 13 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/media/platform/davinci/vpss.c >> b/drivers/media/platform/davinci/vpss.c >> index 146e4b0..34ad7bd 100644 >> --- a/drivers/media/platform/davinci/vpss.c >> +++ b/drivers/media/platform/davinci/vpss.c >> @@ -52,9 +52,11 @@ MODULE_AUTHOR("Texas Instruments"); >> #define DM355_VPSSBL_EVTSEL_DEFAULT 0x4 >> >> #define DM365_ISP5_PCCR 0x04 >> +#define DM365_ISP5_BCR 0x08 >> #define DM365_ISP5_INTSEL1 0x10 >> #define DM365_ISP5_INTSEL2 0x14 >> #define DM365_ISP5_INTSEL3 0x18 >> +#define DM365_ISP5_EVTSEL 0x1c >> #define DM365_ISP5_CCDCMUX 0x20 >> #define DM365_ISP5_PG_FRAME_SIZE 0x28 >> #define DM365_VPBE_CLK_CTRL 0x00 >> @@ -357,6 +359,10 @@ void dm365_vpss_set_pg_frame_size(struct >> vpss_pg_frame_size frame_size) >> } >> EXPORT_SYMBOL(dm365_vpss_set_pg_frame_size); >> >> +#define DM365_ISP5_EVTSEL_EVT_DISABLE 0x00000000 >> +#define DM365_ISP5_BCR_ISIF_OUT_ENABLE 0x00000002 >> +#define DM365_ISP5_PCCR_CLK_ENABLE 0x0000007f >> + > > How about defining these next to the register definitions themselves? There > also seems to be one EVTSEL bit defined in the beginning of the first chunk. > Please group these. > Ok I'll move them to the top. These are bit definitions, maybe using BIT() could be better ? > For that matter --- IMO you could just write zero to the register, without > defining it a name, if the purpose of the register is to select something > based on bits that are set. > Ok. >> static int __devinit vpss_probe(struct platform_device *pdev) >> { >> struct resource *r1, *r2; >> @@ -426,9 +432,16 @@ static int __devinit vpss_probe(struct platform_device >> *pdev) >> oper_cfg.hw_ops.enable_clock = dm365_enable_clock; >> oper_cfg.hw_ops.select_ccdc_source = dm365_select_ccdc_source; >> /* Setup vpss interrupts */ >> + isp5_write((isp5_read(DM365_ISP5_PCCR) | >> + DM365_ISP5_PCCR_CLK_ENABLE), DM365_ISP5_PCCR); >> + isp5_write((isp5_read(DM365_ISP5_BCR) | >> + DM365_ISP5_BCR_ISIF_OUT_ENABLE), DM365_ISP5_BCR); >> isp5_write(DM365_ISP5_INTSEL1_DEFAULT, DM365_ISP5_INTSEL1); >> isp5_write(DM365_ISP5_INTSEL2_DEFAULT, DM365_ISP5_INTSEL2); >> isp5_write(DM365_ISP5_INTSEL3_DEFAULT, DM365_ISP5_INTSEL3); >> + /* No event selected */ >> + isp5_write((isp5_read(DM365_ISP5_EVTSEL) | >> + DM365_ISP5_EVTSEL_EVT_DISABLE), DM365_ISP5_EVTSEL); > > What's this? You're reading the value of the register, orring that with zero > and writing it back? :-) > My Bad I'll remove it. Regards, --Prabhakar >> } else >> oper_cfg.hw_ops.clear_wbl_overflow = dm644x_clear_wbl_overflow; >> > > -- > Regards, > > Sakari Ailus > e-mail: [email protected] XMPP: [email protected] _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
