| Message ID | 1260895054-13232-1-git-send-email-m-karicheri2@ti.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers |
Return-path: <linux-media-owner@vger.kernel.org> Envelope-to: mchehab@infradead.org Delivery-date: Tue, 15 Dec 2009 16:37:41 +0000 Received: from bombadil.infradead.org [18.85.46.34] by pedra with IMAP (fetchmail-6.3.6) for <mchehab@localhost> (single-drop); Tue, 15 Dec 2009 14:39:34 -0200 (BRST) Received: from vger.kernel.org ([209.132.180.67]) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1NKaP7-0008CK-1w; Tue, 15 Dec 2009 16:37:41 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759342AbZLOQhj (ORCPT <rfc822; kmpark@infradead.org> + 1 other); Tue, 15 Dec 2009 11:37:39 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754340AbZLOQhj (ORCPT <rfc822;linux-media-outgoing>); Tue, 15 Dec 2009 11:37:39 -0500 Received: from devils.ext.ti.com ([198.47.26.153]:37903 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752953AbZLOQhi (ORCPT <rfc822;linux-media@vger.kernel.org>); Tue, 15 Dec 2009 11:37:38 -0500 Received: from dlep34.itg.ti.com ([157.170.170.115]) by devils.ext.ti.com (8.13.7/8.13.7) with ESMTP id nBFGbbEl006466 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 15 Dec 2009 10:37:37 -0600 Received: from legion.dal.design.ti.com (localhost [127.0.0.1]) by dlep34.itg.ti.com (8.13.7/8.13.7) with ESMTP id nBFGbaO2025638; Tue, 15 Dec 2009 10:37:36 -0600 (CST) Received: from gt516km11.gt.design.ti.com (gt516km11.gt.design.ti.com [158.218.100.179]) by legion.dal.design.ti.com (8.11.7p1+Sun/8.11.7) with ESMTP id nBFGbaZ06923; Tue, 15 Dec 2009 10:37:36 -0600 (CST) Received: from gt516km11.gt.design.ti.com (localhost.localdomain [127.0.0.1]) by gt516km11.gt.design.ti.com (8.13.1/8.13.1) with ESMTP id nBFGbZri013256; Tue, 15 Dec 2009 11:37:35 -0500 Received: (from a0868495@localhost) by gt516km11.gt.design.ti.com (8.13.1/8.13.1/Submit) id nBFGbYSX013253; Tue, 15 Dec 2009 11:37:34 -0500 From: m-karicheri2@ti.com To: linux-media@vger.kernel.org, hverkuil@xs4all.nl, khilman@deeprootsystems.com Cc: davinci-linux-open-source@linux.davincidsp.com, Muralidharan Karicheri <m-karicheri2@ti.com> Subject: [PATCH - v3 4/4] DaVinci - vpfe-capture-converting ccdc drivers to platform driver Date: Tue, 15 Dec 2009 11:37:31 -0500 Message-Id: <1260895054-13232-1-git-send-email-m-karicheri2@ti.com> X-Mailer: git-send-email 1.6.0.4 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org |
Commit Message
m-karicheri2@ti.com
Dec. 15, 2009, 4:37 p.m. UTC
From: Muralidharan Karicheri <m-karicheri2@ti.com> This combines the two patches sent earlier to change the clock configuration and converting ccdc drivers to platform drivers. This has updated comments against v2 of these patches. Two new clocks "master" and "slave" are defined for ccdc driver as per comments from Kevin Hilman. This adds platform code for ccdc driver on DM355 and DM6446. Reviewed-by: Vaibhav Hiremath <hvaibhav@ti.com> Reviewed-by: Kevin Hilman <khilman@deeprootsystems.com> Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com> --- Applies to linux-davinci tree arch/arm/mach-davinci/dm355.c | 41 ++++++++++++++++++++++++++++----------- arch/arm/mach-davinci/dm644x.c | 20 ++++++++++++++++++- 2 files changed, 48 insertions(+), 13 deletions(-)
Comments
m-karicheri2@ti.com writes: > From: Muralidharan Karicheri <m-karicheri2@ti.com> > > This combines the two patches sent earlier to change the clock configuration > and converting ccdc drivers to platform drivers. This has updated comments > against v2 of these patches. Two new clocks "master" and "slave" are defined for ccdc driver > as per comments from Kevin Hilman. > > This adds platform code for ccdc driver on DM355 and DM6446. > > Reviewed-by: Vaibhav Hiremath <hvaibhav@ti.com> > Reviewed-by: Kevin Hilman <khilman@deeprootsystems.com> > > Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com> > --- > Applies to linux-davinci tree > arch/arm/mach-davinci/dm355.c | 41 ++++++++++++++++++++++++++++----------- > arch/arm/mach-davinci/dm644x.c | 20 ++++++++++++++++++- > 2 files changed, 48 insertions(+), 13 deletions(-) > > diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c > index 2244e8c..a9ea761 100644 > --- a/arch/arm/mach-davinci/dm355.c > +++ b/arch/arm/mach-davinci/dm355.c > @@ -378,6 +378,8 @@ static struct davinci_clk dm355_clks[] = { > CLK(NULL, "timer3", &timer3_clk), > CLK(NULL, "rto", &rto_clk), > CLK(NULL, "usb", &usb_clk), > + CLK("dm355_ccdc", "master", &vpss_master_clk), > + CLK("dm355_ccdc", "slave", &vpss_slave_clk), I still don't understand why you have to add new entries here and can't simply rename the existing CLK nodes using vpss_*_clk. Same comment for dm644x.c changes. Kevin > CLK(NULL, NULL, NULL), > }; > > @@ -665,6 +667,17 @@ static struct platform_device dm355_asp1_device = { > .resource = dm355_asp1_resources, > }; > > +static void dm355_ccdc_setup_pinmux(void) > +{ > + davinci_cfg_reg(DM355_VIN_PCLK); > + davinci_cfg_reg(DM355_VIN_CAM_WEN); > + davinci_cfg_reg(DM355_VIN_CAM_VD); > + davinci_cfg_reg(DM355_VIN_CAM_HD); > + davinci_cfg_reg(DM355_VIN_YIN_EN); > + davinci_cfg_reg(DM355_VIN_CINL_EN); > + davinci_cfg_reg(DM355_VIN_CINH_EN); > +} > + > static struct resource dm355_vpss_resources[] = { > { > /* VPSS BL Base address */ > @@ -701,6 +714,10 @@ static struct resource vpfe_resources[] = { > .end = IRQ_VDINT1, > .flags = IORESOURCE_IRQ, > }, > +}; > + > +static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32); > +static struct resource dm355_ccdc_resource[] = { > /* CCDC Base address */ > { > .flags = IORESOURCE_MEM, > @@ -708,8 +725,18 @@ static struct resource vpfe_resources[] = { > .end = 0x01c70600 + 0x1ff, > }, > }; > +static struct platform_device dm355_ccdc_dev = { > + .name = "dm355_ccdc", > + .id = -1, > + .num_resources = ARRAY_SIZE(dm355_ccdc_resource), > + .resource = dm355_ccdc_resource, > + .dev = { > + .dma_mask = &vpfe_capture_dma_mask, > + .coherent_dma_mask = DMA_BIT_MASK(32), > + .platform_data = dm355_ccdc_setup_pinmux, > + }, > +}; > > -static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32); > static struct platform_device vpfe_capture_dev = { > .name = CAPTURE_DRV_NAME, > .id = -1, > @@ -860,17 +887,7 @@ static int __init dm355_init_devices(void) > davinci_cfg_reg(DM355_INT_EDMA_CC); > platform_device_register(&dm355_edma_device); > platform_device_register(&dm355_vpss_device); > - /* > - * setup Mux configuration for vpfe input and register > - * vpfe capture platform device > - */ > - davinci_cfg_reg(DM355_VIN_PCLK); > - davinci_cfg_reg(DM355_VIN_CAM_WEN); > - davinci_cfg_reg(DM355_VIN_CAM_VD); > - davinci_cfg_reg(DM355_VIN_CAM_HD); > - davinci_cfg_reg(DM355_VIN_YIN_EN); > - davinci_cfg_reg(DM355_VIN_CINL_EN); > - davinci_cfg_reg(DM355_VIN_CINH_EN); > + platform_device_register(&dm355_ccdc_dev); > platform_device_register(&vpfe_capture_dev); > > return 0; > diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c > index e65e29e..e5f1ee9 100644 > --- a/arch/arm/mach-davinci/dm644x.c > +++ b/arch/arm/mach-davinci/dm644x.c > @@ -315,6 +315,8 @@ struct davinci_clk dm644x_clks[] = { > CLK(NULL, "timer0", &timer0_clk), > CLK(NULL, "timer1", &timer1_clk), > CLK("watchdog", NULL, &timer2_clk), > + CLK("dm644x_ccdc", "master", &vpss_master_clk), > + CLK("dm644x_ccdc", "slave", &vpss_slave_clk), > CLK(NULL, NULL, NULL), > }; > > @@ -612,6 +614,11 @@ static struct resource vpfe_resources[] = { > .end = IRQ_VDINT1, > .flags = IORESOURCE_IRQ, > }, > +}; > + > +static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32); > +static struct resource dm644x_ccdc_resource[] = { > + /* CCDC Base address */ > { > .start = 0x01c70400, > .end = 0x01c70400 + 0xff, > @@ -619,7 +626,17 @@ static struct resource vpfe_resources[] = { > }, > }; > > -static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32); > +static struct platform_device dm644x_ccdc_dev = { > + .name = "dm644x_ccdc", > + .id = -1, > + .num_resources = ARRAY_SIZE(dm644x_ccdc_resource), > + .resource = dm644x_ccdc_resource, > + .dev = { > + .dma_mask = &vpfe_capture_dma_mask, > + .coherent_dma_mask = DMA_BIT_MASK(32), > + }, > +}; > + > static struct platform_device vpfe_capture_dev = { > .name = CAPTURE_DRV_NAME, > .id = -1, > @@ -772,6 +789,7 @@ static int __init dm644x_init_devices(void) > platform_device_register(&dm644x_edma_device); > platform_device_register(&dm644x_emac_device); > platform_device_register(&dm644x_vpss_device); > + platform_device_register(&dm644x_ccdc_dev); > platform_device_register(&vpfe_capture_dev); > > return 0; > -- > 1.6.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> CLK(NULL, "rto", &rto_clk), >> CLK(NULL, "usb", &usb_clk), >> + CLK("dm355_ccdc", "master", &vpss_master_clk), >> + CLK("dm355_ccdc", "slave", &vpss_slave_clk), > >I still don't understand why you have to add new entries here and >can't simply rename the existing CLK nodes using vpss_*_clk. > [MK] This will allow multiple drivers define their own clocks derived from these. ccdc driver is not the only driver using these clocks. Your earlier suggestion was to use as follows :- - CLK(NULL, "vpss_master", &vpss_master_clk), - CLK(NULL, "vpss_slave", &vpss_slave_clk), + CLK("vpfe-capture", "master", &vpss_master_clk), + CLK("vpfe-capture", "slave", &vpss_slave_clk), I am not sure if the following will work so that it can be used across multiple drivers. + CLK(NULL, "master", &vpss_master_clk), + CLK(NULL, "slave", &vpss_slave_clk), If yes, I can re-do this patch. Please confirm. >Same comment for dm644x.c changes. > >Kevin > >> CLK(NULL, NULL, NULL), >> }; >> >> @@ -665,6 +667,17 @@ static struct platform_device dm355_asp1_device = { >> .resource = dm355_asp1_resources, >> }; >> >> +static void dm355_ccdc_setup_pinmux(void) >> +{ >> + davinci_cfg_reg(DM355_VIN_PCLK); >> + davinci_cfg_reg(DM355_VIN_CAM_WEN); >> + davinci_cfg_reg(DM355_VIN_CAM_VD); >> + davinci_cfg_reg(DM355_VIN_CAM_HD); >> + davinci_cfg_reg(DM355_VIN_YIN_EN); >> + davinci_cfg_reg(DM355_VIN_CINL_EN); >> + davinci_cfg_reg(DM355_VIN_CINH_EN); >> +} >> + >> static struct resource dm355_vpss_resources[] = { >> { >> /* VPSS BL Base address */ >> @@ -701,6 +714,10 @@ static struct resource vpfe_resources[] = { >> .end = IRQ_VDINT1, >> .flags = IORESOURCE_IRQ, >> }, >> +}; >> + >> +static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32); >> +static struct resource dm355_ccdc_resource[] = { >> /* CCDC Base address */ >> { >> .flags = IORESOURCE_MEM, >> @@ -708,8 +725,18 @@ static struct resource vpfe_resources[] = { >> .end = 0x01c70600 + 0x1ff, >> }, >> }; >> +static struct platform_device dm355_ccdc_dev = { >> + .name = "dm355_ccdc", >> + .id = -1, >> + .num_resources = ARRAY_SIZE(dm355_ccdc_resource), >> + .resource = dm355_ccdc_resource, >> + .dev = { >> + .dma_mask = &vpfe_capture_dma_mask, >> + .coherent_dma_mask = DMA_BIT_MASK(32), >> + .platform_data = dm355_ccdc_setup_pinmux, >> + }, >> +}; >> >> -static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32); >> static struct platform_device vpfe_capture_dev = { >> .name = CAPTURE_DRV_NAME, >> .id = -1, >> @@ -860,17 +887,7 @@ static int __init dm355_init_devices(void) >> davinci_cfg_reg(DM355_INT_EDMA_CC); >> platform_device_register(&dm355_edma_device); >> platform_device_register(&dm355_vpss_device); >> - /* >> - * setup Mux configuration for vpfe input and register >> - * vpfe capture platform device >> - */ >> - davinci_cfg_reg(DM355_VIN_PCLK); >> - davinci_cfg_reg(DM355_VIN_CAM_WEN); >> - davinci_cfg_reg(DM355_VIN_CAM_VD); >> - davinci_cfg_reg(DM355_VIN_CAM_HD); >> - davinci_cfg_reg(DM355_VIN_YIN_EN); >> - davinci_cfg_reg(DM355_VIN_CINL_EN); >> - davinci_cfg_reg(DM355_VIN_CINH_EN); >> + platform_device_register(&dm355_ccdc_dev); >> platform_device_register(&vpfe_capture_dev); >> >> return 0; >> diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach- >davinci/dm644x.c >> index e65e29e..e5f1ee9 100644 >> --- a/arch/arm/mach-davinci/dm644x.c >> +++ b/arch/arm/mach-davinci/dm644x.c >> @@ -315,6 +315,8 @@ struct davinci_clk dm644x_clks[] = { >> CLK(NULL, "timer0", &timer0_clk), >> CLK(NULL, "timer1", &timer1_clk), >> CLK("watchdog", NULL, &timer2_clk), >> + CLK("dm644x_ccdc", "master", &vpss_master_clk), >> + CLK("dm644x_ccdc", "slave", &vpss_slave_clk), >> CLK(NULL, NULL, NULL), >> }; >> >> @@ -612,6 +614,11 @@ static struct resource vpfe_resources[] = { >> .end = IRQ_VDINT1, >> .flags = IORESOURCE_IRQ, >> }, >> +}; >> + >> +static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32); >> +static struct resource dm644x_ccdc_resource[] = { >> + /* CCDC Base address */ >> { >> .start = 0x01c70400, >> .end = 0x01c70400 + 0xff, >> @@ -619,7 +626,17 @@ static struct resource vpfe_resources[] = { >> }, >> }; >> >> -static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32); >> +static struct platform_device dm644x_ccdc_dev = { >> + .name = "dm644x_ccdc", >> + .id = -1, >> + .num_resources = ARRAY_SIZE(dm644x_ccdc_resource), >> + .resource = dm644x_ccdc_resource, >> + .dev = { >> + .dma_mask = &vpfe_capture_dma_mask, >> + .coherent_dma_mask = DMA_BIT_MASK(32), >> + }, >> +}; >> + >> static struct platform_device vpfe_capture_dev = { >> .name = CAPTURE_DRV_NAME, >> .id = -1, >> @@ -772,6 +789,7 @@ static int __init dm644x_init_devices(void) >> platform_device_register(&dm644x_edma_device); >> platform_device_register(&dm644x_emac_device); >> platform_device_register(&dm644x_vpss_device); >> + platform_device_register(&dm644x_ccdc_dev); >> platform_device_register(&vpfe_capture_dev); >> >> return 0; >> -- >> 1.6.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
"Karicheri, Muralidharan" <m-karicheri2@ti.com> writes: >>> CLK(NULL, "rto", &rto_clk), >>> CLK(NULL, "usb", &usb_clk), >>> + CLK("dm355_ccdc", "master", &vpss_master_clk), >>> + CLK("dm355_ccdc", "slave", &vpss_slave_clk), >> >>I still don't understand why you have to add new entries here and >>can't simply rename the existing CLK nodes using vpss_*_clk. >> > > [MK] This will allow multiple drivers define their own clocks derived from > these. ccdc driver is not the only driver using these clocks. OK, but that still doesn't answer why you need multiple CLK() nodes. Who else is using the clocks? > Your earlier suggestion was to use as follows :- > > - CLK(NULL, "vpss_master", &vpss_master_clk), > - CLK(NULL, "vpss_slave", &vpss_slave_clk), > + CLK("vpfe-capture", "master", &vpss_master_clk), > + CLK("vpfe-capture", "slave", &vpss_slave_clk), > > I am not sure if the following will work so that it can be used across > multiple drivers. > > + CLK(NULL, "master", &vpss_master_clk), > + CLK(NULL, "slave", &vpss_slave_clk), > > If yes, I can re-do this patch. Please confirm. No, this will not work. You need a dev_id field so that matching is done using the struct device. My original suggestion was when you had the VPFE driver doing the clk_get(). Now that it's in CCDC, maybe it should look like this. - CLK(NULL, "vpss_master", &vpss_master_clk), - CLK(NULL, "vpss_slave", &vpss_slave_clk), + CLK("ccdc", "master", &vpss_master_clk), + CLK("ccdc", "slave", &vpss_slave_clk), Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
display, resizer drivers etc... Murali Karicheri Software Design Engineer Texas Instruments Inc. Germantown, MD 20874 phone: 301-407-9583 email: m-karicheri2@ti.com >-----Original Message----- >From: Kevin Hilman [mailto:khilman@deeprootsystems.com] >Sent: Wednesday, January 06, 2010 11:04 AM >To: Karicheri, Muralidharan >Cc: linux-media@vger.kernel.org; hverkuil@xs4all.nl; davinci-linux-open- >source@linux.davincidsp.com >Subject: Re: [PATCH - v3 4/4] DaVinci - vpfe-capture-converting ccdc >drivers to platform driver > >"Karicheri, Muralidharan" <m-karicheri2@ti.com> writes: > >>>> CLK(NULL, "rto", &rto_clk), >>>> CLK(NULL, "usb", &usb_clk), >>>> + CLK("dm355_ccdc", "master", &vpss_master_clk), >>>> + CLK("dm355_ccdc", "slave", &vpss_slave_clk), >>> >>>I still don't understand why you have to add new entries here and >>>can't simply rename the existing CLK nodes using vpss_*_clk. >>> >> >> [MK] This will allow multiple drivers define their own clocks derived >from >> these. ccdc driver is not the only driver using these clocks. > >OK, but that still doesn't answer why you need multiple CLK() nodes. > >Who else is using the clocks? > >> Your earlier suggestion was to use as follows :- >> >> - CLK(NULL, "vpss_master", &vpss_master_clk), >> - CLK(NULL, "vpss_slave", &vpss_slave_clk), >> + CLK("vpfe-capture", "master", &vpss_master_clk), >> + CLK("vpfe-capture", "slave", &vpss_slave_clk), >> >> I am not sure if the following will work so that it can be used across >> multiple drivers. >> >> + CLK(NULL, "master", &vpss_master_clk), >> + CLK(NULL, "slave", &vpss_slave_clk), >> >> If yes, I can re-do this patch. Please confirm. > >No, this will not work. You need a dev_id field so that matching >is done using the struct device. > >My original suggestion was when you had the VPFE driver doing the >clk_get(). Now that it's in CCDC, maybe it should look like this. > >- CLK(NULL, "vpss_master", &vpss_master_clk), >- CLK(NULL, "vpss_slave", &vpss_slave_clk), >+ CLK("ccdc", "master", &vpss_master_clk), >+ CLK("ccdc", "slave", &vpss_slave_clk), > >Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
In the future, please do not top-post. Inline replies are preferred so context can be followed. I've moved your reply into context below with some more comments... "Karicheri, Muralidharan" <m-karicheri2@ti.com> writes: >>-----Original Message----- >>From: Kevin Hilman [mailto:khilman@deeprootsystems.com] >>Sent: Wednesday, January 06, 2010 11:04 AM >>To: Karicheri, Muralidharan >>Cc: linux-media@vger.kernel.org; hverkuil@xs4all.nl; davinci-linux-open- >>source@linux.davincidsp.com >>Subject: Re: [PATCH - v3 4/4] DaVinci - vpfe-capture-converting ccdc >>drivers to platform driver >> >>"Karicheri, Muralidharan" <m-karicheri2@ti.com> writes: >> >>>>> CLK(NULL, "rto", &rto_clk), >>>>> CLK(NULL, "usb", &usb_clk), >>>>> + CLK("dm355_ccdc", "master", &vpss_master_clk), >>>>> + CLK("dm355_ccdc", "slave", &vpss_slave_clk), >>>> >>>>I still don't understand why you have to add new entries here and >>>>can't simply rename the existing CLK nodes using vpss_*_clk. >>>> >>> >>> [MK] This will allow multiple drivers define their own clocks derived >>from >>> these. ccdc driver is not the only driver using these clocks. >> >>OK, but that still doesn't answer why you need multiple CLK() nodes. >> >>Who else is using the clocks? >> > > display, resizer drivers etc... OK, I'm not extremely familar with the whole video architecture here, but are all of these drivers expected to be doing clk_get() and clk_enable()? I thought the point of moving the clocks into the CCDC driver was so that the clock management was done in a single, shared space. Kevin >>> Your earlier suggestion was to use as follows :- >>> >>> - CLK(NULL, "vpss_master", &vpss_master_clk), >>> - CLK(NULL, "vpss_slave", &vpss_slave_clk), >>> + CLK("vpfe-capture", "master", &vpss_master_clk), >>> + CLK("vpfe-capture", "slave", &vpss_slave_clk), >>> >>> I am not sure if the following will work so that it can be used across >>> multiple drivers. >>> >>> + CLK(NULL, "master", &vpss_master_clk), >>> + CLK(NULL, "slave", &vpss_slave_clk), >>> >>> If yes, I can re-do this patch. Please confirm. >> >>No, this will not work. You need a dev_id field so that matching >>is done using the struct device. >> >>My original suggestion was when you had the VPFE driver doing the >>clk_get(). Now that it's in CCDC, maybe it should look like this. >> >>- CLK(NULL, "vpss_master", &vpss_master_clk), >>- CLK(NULL, "vpss_slave", &vpss_slave_clk), >>+ CLK("ccdc", "master", &vpss_master_clk), >>+ CLK("ccdc", "slave", &vpss_slave_clk), >> >>Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Kevin, > >OK, I'm not extremely familar with the whole video architecture here, >but are all of these drivers expected to be doing clk_get() and >clk_enable()? > [MK]Many IPs on DaVinci VPFE would require vpss master clock. So it is better to do the way I have done in my patch. So it is expected that clk_get, clk_enable etc are called from other drivers as well. >I thought the point of moving the clocks into the CCDC driver was so that >the clock management was done in a single, shared space. > [MK] No. The CCDC IP is used across DaVinci and OMAP SOCs. The clock is named differently on OMAP, but the IP requires two clocks. So we named them as "master" and "slave" as a generic name. OMAP, patform code will be mapping master and slave clocks to their respective clocks. We had discussed this in the email chain. Murali >Kevin > >>>> Your earlier suggestion was to use as follows :- >>>> >>>> - CLK(NULL, "vpss_master", &vpss_master_clk), >>>> - CLK(NULL, "vpss_slave", &vpss_slave_clk), >>>> + CLK("vpfe-capture", "master", &vpss_master_clk), >>>> + CLK("vpfe-capture", "slave", &vpss_slave_clk), >>>> >>>> I am not sure if the following will work so that it can be used across >>>> multiple drivers. >>>> >>>> + CLK(NULL, "master", &vpss_master_clk), >>>> + CLK(NULL, "slave", &vpss_slave_clk), >>>> >>>> If yes, I can re-do this patch. Please confirm. >>> >>>No, this will not work. You need a dev_id field so that matching >>>is done using the struct device. >>> >>>My original suggestion was when you had the VPFE driver doing the >>>clk_get(). Now that it's in CCDC, maybe it should look like this. >>> >>>- CLK(NULL, "vpss_master", &vpss_master_clk), >>>- CLK(NULL, "vpss_slave", &vpss_slave_clk), >>>+ CLK("ccdc", "master", &vpss_master_clk), >>>+ CLK("ccdc", "slave", &vpss_slave_clk), >>> >>>Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
"Karicheri, Muralidharan" <m-karicheri2@ti.com> writes: > Kevin, > >> >>OK, I'm not extremely familar with the whole video architecture here, >>but are all of these drivers expected to be doing clk_get() and >>clk_enable()? >> > > [MK]Many IPs on DaVinci VPFE would require vpss master clock. So > it is better to do the way I have done in my patch. So it is expected > that clk_get, clk_enable etc are called from other drivers as well. OK, then you are expecting to add clkdev nodes for the other devices as well. That's ok. However, you still haven't answered my original question. AFAICT, there are no users of the clkdev nodes "vpss_master" and "vpss_slave". Why not remove those and replace them with your new nodes instead of leaving them and adding new ones? Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Kevin, Can I remove it through a separate patch? This patch is already merged in Hans tree. Murali Karicheri Software Design Engineer Texas Instruments Inc. Germantown, MD 20874 phone: 301-407-9583 email: m-karicheri2@ti.com >-----Original Message----- >From: Kevin Hilman [mailto:khilman@deeprootsystems.com] >Sent: Thursday, January 07, 2010 2:44 PM >To: Karicheri, Muralidharan >Cc: linux-media@vger.kernel.org; hverkuil@xs4all.nl; davinci-linux-open- >source@linux.davincidsp.com >Subject: Re: [PATCH - v3 4/4] DaVinci - vpfe-capture-converting ccdc >drivers to platform driver > >"Karicheri, Muralidharan" <m-karicheri2@ti.com> writes: > >> Kevin, >> >>> >>>OK, I'm not extremely familar with the whole video architecture here, >>>but are all of these drivers expected to be doing clk_get() and >>>clk_enable()? >>> >> >> [MK]Many IPs on DaVinci VPFE would require vpss master clock. So >> it is better to do the way I have done in my patch. So it is expected >> that clk_get, clk_enable etc are called from other drivers as well. > >OK, then you are expecting to add clkdev nodes for the other devices >as well. That's ok. > >However, you still haven't answered my original question. AFAICT, >there are no users of the clkdev nodes "vpss_master" and "vpss_slave". >Why not remove those and replace them with your new nodes instead of >leaving them and adding new ones? > >Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
"Karicheri, Muralidharan" <m-karicheri2@ti.com> writes: > Can I remove it through a separate patch? This patch is already merged in Hans tree. Hmm, arch patches should not be merged yet as I have not ack'd them. Kevin >>-----Original Message----- >>From: Kevin Hilman [mailto:khilman@deeprootsystems.com] >>Sent: Thursday, January 07, 2010 2:44 PM >>To: Karicheri, Muralidharan >>Cc: linux-media@vger.kernel.org; hverkuil@xs4all.nl; davinci-linux-open- >>source@linux.davincidsp.com >>Subject: Re: [PATCH - v3 4/4] DaVinci - vpfe-capture-converting ccdc >>drivers to platform driver >> >>"Karicheri, Muralidharan" <m-karicheri2@ti.com> writes: >> >>> Kevin, >>> >>>> >>>>OK, I'm not extremely familar with the whole video architecture here, >>>>but are all of these drivers expected to be doing clk_get() and >>>>clk_enable()? >>>> >>> >>> [MK]Many IPs on DaVinci VPFE would require vpss master clock. So >>> it is better to do the way I have done in my patch. So it is expected >>> that clk_get, clk_enable etc are called from other drivers as well. >> >>OK, then you are expecting to add clkdev nodes for the other devices >>as well. That's ok. >> >>However, you still haven't answered my original question. AFAICT, >>there are no users of the clkdev nodes "vpss_master" and "vpss_slave". >>Why not remove those and replace them with your new nodes instead of >>leaving them and adding new ones? >> >>Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Arch patches are not usually merged in Hans tree. Murali Karicheri Software Design Engineer Texas Instruments Inc. Germantown, MD 20874 phone: 301-407-9583 email: m-karicheri2@ti.com >-----Original Message----- >From: Kevin Hilman [mailto:khilman@deeprootsystems.com] >Sent: Thursday, January 07, 2010 4:50 PM >To: Karicheri, Muralidharan >Cc: linux-media@vger.kernel.org; hverkuil@xs4all.nl; davinci-linux-open- >source@linux.davincidsp.com >Subject: Re: [PATCH - v3 4/4] DaVinci - vpfe-capture-converting ccdc >drivers to platform driver > >"Karicheri, Muralidharan" <m-karicheri2@ti.com> writes: > >> Can I remove it through a separate patch? This patch is already merged in >Hans tree. > >Hmm, arch patches should not be merged yet as I have not ack'd them. > >Kevin > > >>>-----Original Message----- >>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com] >>>Sent: Thursday, January 07, 2010 2:44 PM >>>To: Karicheri, Muralidharan >>>Cc: linux-media@vger.kernel.org; hverkuil@xs4all.nl; davinci-linux-open- >>>source@linux.davincidsp.com >>>Subject: Re: [PATCH - v3 4/4] DaVinci - vpfe-capture-converting ccdc >>>drivers to platform driver >>> >>>"Karicheri, Muralidharan" <m-karicheri2@ti.com> writes: >>> >>>> Kevin, >>>> >>>>> >>>>>OK, I'm not extremely familar with the whole video architecture here, >>>>>but are all of these drivers expected to be doing clk_get() and >>>>>clk_enable()? >>>>> >>>> >>>> [MK]Many IPs on DaVinci VPFE would require vpss master clock. So >>>> it is better to do the way I have done in my patch. So it is expected >>>> that clk_get, clk_enable etc are called from other drivers as well. >>> >>>OK, then you are expecting to add clkdev nodes for the other devices >>>as well. That's ok. >>> >>>However, you still haven't answered my original question. AFAICT, >>>there are no users of the clkdev nodes "vpss_master" and "vpss_slave". >>>Why not remove those and replace them with your new nodes instead of >>>leaving them and adding new ones? >>> >>>Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: davinci-linux-open-source-bounces@linux.davincidsp.com > [mailto:davinci-linux-open-source-bounces@linux.davincidsp.com] On > Behalf Of Karicheri, Muralidharan > Sent: Friday, January 08, 2010 4:55 AM > To: Kevin Hilman > Cc: davinci-linux-open-source@linux.davincidsp.com; linux- > media@vger.kernel.org > Subject: RE: [PATCH - v3 4/4] DaVinci - vpfe-capture-converting ccdc > drivers to platform driver > > Arch patches are not usually merged in Hans tree. > [Hiremath, Vaibhav] Hi Kevin and Murali, Sorry for jumping into this discussion so late, Can we use clk_add_alias() function exported by clkdev.c file here? With this board specific file can define aliases for all required platform_data keeping CLK() entry generic. Thanks, Vaibhav > Murali Karicheri > Software Design Engineer > Texas Instruments Inc. > Germantown, MD 20874 > phone: 301-407-9583 > email: m-karicheri2@ti.com > > >-----Original Message----- > >From: Kevin Hilman [mailto:khilman@deeprootsystems.com] > >Sent: Thursday, January 07, 2010 4:50 PM > >To: Karicheri, Muralidharan > >Cc: linux-media@vger.kernel.org; hverkuil@xs4all.nl; davinci-linux- > open- > >source@linux.davincidsp.com > >Subject: Re: [PATCH - v3 4/4] DaVinci - vpfe-capture-converting > ccdc > >drivers to platform driver > > > >"Karicheri, Muralidharan" <m-karicheri2@ti.com> writes: > > > >> Can I remove it through a separate patch? This patch is already > merged in > >Hans tree. > > > >Hmm, arch patches should not be merged yet as I have not ack'd > them. > > > >Kevin > > > > > >>>-----Original Message----- > >>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com] > >>>Sent: Thursday, January 07, 2010 2:44 PM > >>>To: Karicheri, Muralidharan > >>>Cc: linux-media@vger.kernel.org; hverkuil@xs4all.nl; davinci- > linux-open- > >>>source@linux.davincidsp.com > >>>Subject: Re: [PATCH - v3 4/4] DaVinci - vpfe-capture-converting > ccdc > >>>drivers to platform driver > >>> > >>>"Karicheri, Muralidharan" <m-karicheri2@ti.com> writes: > >>> > >>>> Kevin, > >>>> > >>>>> > >>>>>OK, I'm not extremely familar with the whole video architecture > here, > >>>>>but are all of these drivers expected to be doing clk_get() and > >>>>>clk_enable()? > >>>>> > >>>> > >>>> [MK]Many IPs on DaVinci VPFE would require vpss master clock. > So > >>>> it is better to do the way I have done in my patch. So it is > expected > >>>> that clk_get, clk_enable etc are called from other drivers as > well. > >>> > >>>OK, then you are expecting to add clkdev nodes for the other > devices > >>>as well. That's ok. > >>> > >>>However, you still haven't answered my original question. > AFAICT, > >>>there are no users of the clkdev nodes "vpss_master" and > "vpss_slave". > >>>Why not remove those and replace them with your new nodes instead > of > >>>leaving them and adding new ones? > >>> > >>>Kevin > _______________________________________________ > Davinci-linux-open-source mailing list > Davinci-linux-open-source@linux.davincidsp.com > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open- > source -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
"Hiremath, Vaibhav" <hvaibhav@ti.com> writes: >> > [Hiremath, Vaibhav] Hi Kevin and Murali, > > Sorry for jumping into this discussion so late, > > Can we use clk_add_alias() function exported by clkdev.c file here? > With this board specific file can define aliases for all required > platform_data keeping CLK() entry generic. Yes, this would be a good use case clk_add_alias() Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Kevin Hilman [mailto:khilman@deeprootsystems.com] > Sent: Friday, January 08, 2010 8:40 PM > To: Hiremath, Vaibhav > Cc: Karicheri, Muralidharan; davinci-linux-open- > source@linux.davincidsp.com; linux-media@vger.kernel.org > Subject: Re: [PATCH - v3 4/4] DaVinci - vpfe-capture-converting ccdc > drivers to platform driver > > "Hiremath, Vaibhav" <hvaibhav@ti.com> writes: > > >> > > [Hiremath, Vaibhav] Hi Kevin and Murali, > > > > Sorry for jumping into this discussion so late, > > > > Can we use clk_add_alias() function exported by clkdev.c file > here? > > With this board specific file can define aliases for all required > > platform_data keeping CLK() entry generic. > > Yes, this would be a good use case clk_add_alias() > [Hiremath, Vaibhav] Thanks Kevin, actually I am already using the clk_add_alias in AM3517 which used same VPFE_Capture driver. But I was not sure whether this is acceptable or not, since nobody in the kernel is using this API. Thanks for conforming/clarifying; I will submit the patch now for this. Thanks, Vaibhav > Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c index 2244e8c..a9ea761 100644 --- a/arch/arm/mach-davinci/dm355.c +++ b/arch/arm/mach-davinci/dm355.c @@ -378,6 +378,8 @@ static struct davinci_clk dm355_clks[] = { CLK(NULL, "timer3", &timer3_clk), CLK(NULL, "rto", &rto_clk), CLK(NULL, "usb", &usb_clk), + CLK("dm355_ccdc", "master", &vpss_master_clk), + CLK("dm355_ccdc", "slave", &vpss_slave_clk), CLK(NULL, NULL, NULL), }; @@ -665,6 +667,17 @@ static struct platform_device dm355_asp1_device = { .resource = dm355_asp1_resources, }; +static void dm355_ccdc_setup_pinmux(void) +{ + davinci_cfg_reg(DM355_VIN_PCLK); + davinci_cfg_reg(DM355_VIN_CAM_WEN); + davinci_cfg_reg(DM355_VIN_CAM_VD); + davinci_cfg_reg(DM355_VIN_CAM_HD); + davinci_cfg_reg(DM355_VIN_YIN_EN); + davinci_cfg_reg(DM355_VIN_CINL_EN); + davinci_cfg_reg(DM355_VIN_CINH_EN); +} + static struct resource dm355_vpss_resources[] = { { /* VPSS BL Base address */ @@ -701,6 +714,10 @@ static struct resource vpfe_resources[] = { .end = IRQ_VDINT1, .flags = IORESOURCE_IRQ, }, +}; + +static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32); +static struct resource dm355_ccdc_resource[] = { /* CCDC Base address */ { .flags = IORESOURCE_MEM, @@ -708,8 +725,18 @@ static struct resource vpfe_resources[] = { .end = 0x01c70600 + 0x1ff, }, }; +static struct platform_device dm355_ccdc_dev = { + .name = "dm355_ccdc", + .id = -1, + .num_resources = ARRAY_SIZE(dm355_ccdc_resource), + .resource = dm355_ccdc_resource, + .dev = { + .dma_mask = &vpfe_capture_dma_mask, + .coherent_dma_mask = DMA_BIT_MASK(32), + .platform_data = dm355_ccdc_setup_pinmux, + }, +}; -static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32); static struct platform_device vpfe_capture_dev = { .name = CAPTURE_DRV_NAME, .id = -1, @@ -860,17 +887,7 @@ static int __init dm355_init_devices(void) davinci_cfg_reg(DM355_INT_EDMA_CC); platform_device_register(&dm355_edma_device); platform_device_register(&dm355_vpss_device); - /* - * setup Mux configuration for vpfe input and register - * vpfe capture platform device - */ - davinci_cfg_reg(DM355_VIN_PCLK); - davinci_cfg_reg(DM355_VIN_CAM_WEN); - davinci_cfg_reg(DM355_VIN_CAM_VD); - davinci_cfg_reg(DM355_VIN_CAM_HD); - davinci_cfg_reg(DM355_VIN_YIN_EN); - davinci_cfg_reg(DM355_VIN_CINL_EN); - davinci_cfg_reg(DM355_VIN_CINH_EN); + platform_device_register(&dm355_ccdc_dev); platform_device_register(&vpfe_capture_dev); return 0; diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c index e65e29e..e5f1ee9 100644 --- a/arch/arm/mach-davinci/dm644x.c +++ b/arch/arm/mach-davinci/dm644x.c @@ -315,6 +315,8 @@ struct davinci_clk dm644x_clks[] = { CLK(NULL, "timer0", &timer0_clk), CLK(NULL, "timer1", &timer1_clk), CLK("watchdog", NULL, &timer2_clk), + CLK("dm644x_ccdc", "master", &vpss_master_clk), + CLK("dm644x_ccdc", "slave", &vpss_slave_clk), CLK(NULL, NULL, NULL), }; @@ -612,6 +614,11 @@ static struct resource vpfe_resources[] = { .end = IRQ_VDINT1, .flags = IORESOURCE_IRQ, }, +}; + +static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32); +static struct resource dm644x_ccdc_resource[] = { + /* CCDC Base address */ { .start = 0x01c70400, .end = 0x01c70400 + 0xff, @@ -619,7 +626,17 @@ static struct resource vpfe_resources[] = { }, }; -static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32); +static struct platform_device dm644x_ccdc_dev = { + .name = "dm644x_ccdc", + .id = -1, + .num_resources = ARRAY_SIZE(dm644x_ccdc_resource), + .resource = dm644x_ccdc_resource, + .dev = { + .dma_mask = &vpfe_capture_dma_mask, + .coherent_dma_mask = DMA_BIT_MASK(32), + }, +}; + static struct platform_device vpfe_capture_dev = { .name = CAPTURE_DRV_NAME, .id = -1, @@ -772,6 +789,7 @@ static int __init dm644x_init_devices(void) platform_device_register(&dm644x_edma_device); platform_device_register(&dm644x_emac_device); platform_device_register(&dm644x_vpss_device); + platform_device_register(&dm644x_ccdc_dev); platform_device_register(&vpfe_capture_dev); return 0;