[omap3isp,v2,5/9] omap3isp: Remove unused CBUFF register access
Commit Message
Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
---
drivers/media/video/isp/isp.c | 2 --
drivers/media/video/isp/isp.h | 1 -
drivers/media/video/isp/ispreg.h | 25 -------------------------
3 files changed, 0 insertions(+), 28 deletions(-)
Comments
Hi Sergio,
I have one comment below.
On Mon, Nov 15, 2010 at 03:29:57PM +0100, ext Sergio Aguirre wrote:
> Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
> ---
> drivers/media/video/isp/isp.c | 2 --
> drivers/media/video/isp/isp.h | 1 -
> drivers/media/video/isp/ispreg.h | 25 -------------------------
> 3 files changed, 0 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/media/video/isp/isp.c b/drivers/media/video/isp/isp.c
> index a5c02ba..f266e7c 100644
> --- a/drivers/media/video/isp/isp.c
> +++ b/drivers/media/video/isp/isp.c
> @@ -86,7 +86,6 @@ static const struct isp_res_mapping isp_res_maps[] = {
> {
> .isp_rev = ISP_REVISION_2_0,
> .map = 1 << OMAP3_ISP_IOMEM_MAIN |
> - 1 << OMAP3_ISP_IOMEM_CBUFF |
> 1 << OMAP3_ISP_IOMEM_CCP2 |
> 1 << OMAP3_ISP_IOMEM_CCDC |
> 1 << OMAP3_ISP_IOMEM_HIST |
> @@ -100,7 +99,6 @@ static const struct isp_res_mapping isp_res_maps[] = {
> {
> .isp_rev = ISP_REVISION_15_0,
> .map = 1 << OMAP3_ISP_IOMEM_MAIN |
> - 1 << OMAP3_ISP_IOMEM_CBUFF |
> 1 << OMAP3_ISP_IOMEM_CCP2 |
> 1 << OMAP3_ISP_IOMEM_CCDC |
> 1 << OMAP3_ISP_IOMEM_HIST |
> diff --git a/drivers/media/video/isp/isp.h b/drivers/media/video/isp/isp.h
> index edc029c..b8f63e2 100644
> --- a/drivers/media/video/isp/isp.h
> +++ b/drivers/media/video/isp/isp.h
> @@ -56,7 +56,6 @@
>
> enum isp_mem_resources {
> OMAP3_ISP_IOMEM_MAIN,
> - OMAP3_ISP_IOMEM_CBUFF,
> OMAP3_ISP_IOMEM_CCP2,
> OMAP3_ISP_IOMEM_CCDC,
> OMAP3_ISP_IOMEM_HIST,
> diff --git a/drivers/media/video/isp/ispreg.h b/drivers/media/video/isp/ispreg.h
> index 8e4324f..c080980 100644
> --- a/drivers/media/video/isp/ispreg.h
> +++ b/drivers/media/video/isp/ispreg.h
> @@ -37,11 +37,6 @@
> #define OMAP3ISP_REG_BASE OMAP3430_ISP_BASE
> #define OMAP3ISP_REG(offset) (OMAP3ISP_REG_BASE + (offset))
>
> -#define OMAP3ISP_CBUFF_REG_OFFSET 0x0100
> -#define OMAP3ISP_CBUFF_REG_BASE (OMAP3ISP_REG_BASE + \
> - OMAP3ISP_CBUFF_REG_OFFSET)
> -#define OMAP3ISP_CBUFF_REG(offset) (OMAP3ISP_CBUFF_REG_BASE + (offset))
> -
> #define OMAP3ISP_CCP2_REG_OFFSET 0x0400
> #define OMAP3ISP_CCP2_REG_BASE (OMAP3ISP_REG_BASE + \
> OMAP3ISP_CCP2_REG_OFFSET)
> @@ -244,26 +239,6 @@
> #define ISP_CSIB_SYSCONFIG ISPCCP2_SYSCONFIG
> #define ISP_CSIA_SYSCONFIG ISPCSI2_SYSCONFIG
>
> -/* ISP_CBUFF Registers */
> -
> -#define ISP_CBUFF_SYSCONFIG (0x010)
> -#define ISP_CBUFF_IRQENABLE (0x01C)
> -
> -#define ISP_CBUFF0_CTRL (0x020)
> -#define ISP_CBUFF1_CTRL (0x024)
> -
> -#define ISP_CBUFF0_START (0x040)
> -#define ISP_CBUFF1_START (0x044)
> -
> -#define ISP_CBUFF0_END (0x050)
> -#define ISP_CBUFF1_END (0x054)
> -
> -#define ISP_CBUFF0_WINDOWSIZE (0x060)
> -#define ISP_CBUFF1_WINDOWSIZE (0x064)
> -
> -#define ISP_CBUFF0_THRESHOLD (0x070)
> -#define ISP_CBUFF1_THRESHOLD (0x074)
> -
No need to remove the registers from header file. We're not using them
on current version, but it doesn't mean we won't use ever. :)
Br,
David Cohen
> /* CCDC module register offset */
>
> #define ISPCCDC_PID (0x000)
> --
> 1.7.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
--
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
Hi David,
On Friday 19 November 2010 11:19:44 David Cohen wrote:
> On Mon, Nov 15, 2010 at 03:29:57PM +0100, ext Sergio Aguirre wrote:
[snip]
> > @@ -244,26 +239,6 @@
> >
> > #define ISP_CSIB_SYSCONFIG ISPCCP2_SYSCONFIG
> > #define ISP_CSIA_SYSCONFIG ISPCSI2_SYSCONFIG
> >
> > -/* ISP_CBUFF Registers */
> > -
> > -#define ISP_CBUFF_SYSCONFIG (0x010)
> > -#define ISP_CBUFF_IRQENABLE (0x01C)
> > -
> > -#define ISP_CBUFF0_CTRL (0x020)
> > -#define ISP_CBUFF1_CTRL (0x024)
> > -
> > -#define ISP_CBUFF0_START (0x040)
> > -#define ISP_CBUFF1_START (0x044)
> > -
> > -#define ISP_CBUFF0_END (0x050)
> > -#define ISP_CBUFF1_END (0x054)
> > -
> > -#define ISP_CBUFF0_WINDOWSIZE (0x060)
> > -#define ISP_CBUFF1_WINDOWSIZE (0x064)
> > -
> > -#define ISP_CBUFF0_THRESHOLD (0x070)
> > -#define ISP_CBUFF1_THRESHOLD (0x074)
> > -
>
> No need to remove the registers from header file. We're not using them
> on current version, but it doesn't mean we won't use ever. :)
I would have made the same comment for other registers, but we're not using
the CBUFF module at all here, with no plans to use it in the future. It might
not be worth it keeping the register definitions. I have no strong feeling
about it, I'm fine with both choices.
Hi Laurent and David,
> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Friday, November 19, 2010 4:33 AM
> To: David Cohen
> Cc: Aguirre, Sergio; linux-media@vger.kernel.org
> Subject: Re: [omap3isp][PATCH v2 5/9] omap3isp: Remove unused CBUFF
> register access
>
> Hi David,
>
> On Friday 19 November 2010 11:19:44 David Cohen wrote:
> > On Mon, Nov 15, 2010 at 03:29:57PM +0100, ext Sergio Aguirre wrote:
>
> [snip]
>
> > > @@ -244,26 +239,6 @@
> > >
> > > #define ISP_CSIB_SYSCONFIG ISPCCP2_SYSCONFIG
> > > #define ISP_CSIA_SYSCONFIG ISPCSI2_SYSCONFIG
> > >
> > > -/* ISP_CBUFF Registers */
> > > -
> > > -#define ISP_CBUFF_SYSCONFIG (0x010)
> > > -#define ISP_CBUFF_IRQENABLE (0x01C)
> > > -
> > > -#define ISP_CBUFF0_CTRL (0x020)
> > > -#define ISP_CBUFF1_CTRL (0x024)
> > > -
> > > -#define ISP_CBUFF0_START (0x040)
> > > -#define ISP_CBUFF1_START (0x044)
> > > -
> > > -#define ISP_CBUFF0_END (0x050)
> > > -#define ISP_CBUFF1_END (0x054)
> > > -
> > > -#define ISP_CBUFF0_WINDOWSIZE (0x060)
> > > -#define ISP_CBUFF1_WINDOWSIZE (0x064)
> > > -
> > > -#define ISP_CBUFF0_THRESHOLD (0x070)
> > > -#define ISP_CBUFF1_THRESHOLD (0x074)
> > > -
> >
> > No need to remove the registers from header file. We're not using them
> > on current version, but it doesn't mean we won't use ever. :)
>
> I would have made the same comment for other registers, but we're not
> using
> the CBUFF module at all here, with no plans to use it in the future. It
> might
> not be worth it keeping the register definitions. I have no strong feeling
> about it, I'm fine with both choices.
David,
IMO, we should not introduce dead code/unusued defines in the first omap3isp
upstream version. I think it's already quite hard to review for somebody
outside the omap3isp development team.
Having all this just in case will most probably end up in bulk, as we
might never implement the CBUFF HW block, as Laurent mentions.
I'll be more biased on all this being re-added if we end up implementing a ispcbuff submodule.
Regards,
Sergio
>
> --
> Regards,
>
> Laurent Pinchart
--
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
On Fri, Nov 19, 2010 at 04:54:01PM +0100, ext Aguirre, Sergio wrote:
> Hi Laurent and David,
Hi Sergio,
>
> > -----Original Message-----
> > From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> > Sent: Friday, November 19, 2010 4:33 AM
> > To: David Cohen
> > Cc: Aguirre, Sergio; linux-media@vger.kernel.org
> > Subject: Re: [omap3isp][PATCH v2 5/9] omap3isp: Remove unused CBUFF
> > register access
> >
> > Hi David,
> >
> > On Friday 19 November 2010 11:19:44 David Cohen wrote:
> > > On Mon, Nov 15, 2010 at 03:29:57PM +0100, ext Sergio Aguirre wrote:
> >
> > [snip]
> >
> > > > @@ -244,26 +239,6 @@
> > > >
> > > > #define ISP_CSIB_SYSCONFIG ISPCCP2_SYSCONFIG
> > > > #define ISP_CSIA_SYSCONFIG ISPCSI2_SYSCONFIG
> > > >
> > > > -/* ISP_CBUFF Registers */
> > > > -
> > > > -#define ISP_CBUFF_SYSCONFIG (0x010)
> > > > -#define ISP_CBUFF_IRQENABLE (0x01C)
> > > > -
> > > > -#define ISP_CBUFF0_CTRL (0x020)
> > > > -#define ISP_CBUFF1_CTRL (0x024)
> > > > -
> > > > -#define ISP_CBUFF0_START (0x040)
> > > > -#define ISP_CBUFF1_START (0x044)
> > > > -
> > > > -#define ISP_CBUFF0_END (0x050)
> > > > -#define ISP_CBUFF1_END (0x054)
> > > > -
> > > > -#define ISP_CBUFF0_WINDOWSIZE (0x060)
> > > > -#define ISP_CBUFF1_WINDOWSIZE (0x064)
> > > > -
> > > > -#define ISP_CBUFF0_THRESHOLD (0x070)
> > > > -#define ISP_CBUFF1_THRESHOLD (0x074)
> > > > -
> > >
> > > No need to remove the registers from header file. We're not using them
> > > on current version, but it doesn't mean we won't use ever. :)
> >
> > I would have made the same comment for other registers, but we're not
> > using
> > the CBUFF module at all here, with no plans to use it in the future. It
> > might
> > not be worth it keeping the register definitions. I have no strong feeling
> > about it, I'm fine with both choices.
>
> David,
>
> IMO, we should not introduce dead code/unusued defines in the first omap3isp
> upstream version. I think it's already quite hard to review for somebody
> outside the omap3isp development team.
>
> Having all this just in case will most probably end up in bulk, as we
> might never implement the CBUFF HW block, as Laurent mentions.
That's a good point. I see no problem in removing it in that case.
Br,
David
>
> I'll be more biased on all this being re-added if we end up implementing a ispcbuff submodule.
>
> Regards,
> Sergio
>
>
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> --
> 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
--
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
@@ -86,7 +86,6 @@ static const struct isp_res_mapping isp_res_maps[] = {
{
.isp_rev = ISP_REVISION_2_0,
.map = 1 << OMAP3_ISP_IOMEM_MAIN |
- 1 << OMAP3_ISP_IOMEM_CBUFF |
1 << OMAP3_ISP_IOMEM_CCP2 |
1 << OMAP3_ISP_IOMEM_CCDC |
1 << OMAP3_ISP_IOMEM_HIST |
@@ -100,7 +99,6 @@ static const struct isp_res_mapping isp_res_maps[] = {
{
.isp_rev = ISP_REVISION_15_0,
.map = 1 << OMAP3_ISP_IOMEM_MAIN |
- 1 << OMAP3_ISP_IOMEM_CBUFF |
1 << OMAP3_ISP_IOMEM_CCP2 |
1 << OMAP3_ISP_IOMEM_CCDC |
1 << OMAP3_ISP_IOMEM_HIST |
@@ -56,7 +56,6 @@
enum isp_mem_resources {
OMAP3_ISP_IOMEM_MAIN,
- OMAP3_ISP_IOMEM_CBUFF,
OMAP3_ISP_IOMEM_CCP2,
OMAP3_ISP_IOMEM_CCDC,
OMAP3_ISP_IOMEM_HIST,
@@ -37,11 +37,6 @@
#define OMAP3ISP_REG_BASE OMAP3430_ISP_BASE
#define OMAP3ISP_REG(offset) (OMAP3ISP_REG_BASE + (offset))
-#define OMAP3ISP_CBUFF_REG_OFFSET 0x0100
-#define OMAP3ISP_CBUFF_REG_BASE (OMAP3ISP_REG_BASE + \
- OMAP3ISP_CBUFF_REG_OFFSET)
-#define OMAP3ISP_CBUFF_REG(offset) (OMAP3ISP_CBUFF_REG_BASE + (offset))
-
#define OMAP3ISP_CCP2_REG_OFFSET 0x0400
#define OMAP3ISP_CCP2_REG_BASE (OMAP3ISP_REG_BASE + \
OMAP3ISP_CCP2_REG_OFFSET)
@@ -244,26 +239,6 @@
#define ISP_CSIB_SYSCONFIG ISPCCP2_SYSCONFIG
#define ISP_CSIA_SYSCONFIG ISPCSI2_SYSCONFIG
-/* ISP_CBUFF Registers */
-
-#define ISP_CBUFF_SYSCONFIG (0x010)
-#define ISP_CBUFF_IRQENABLE (0x01C)
-
-#define ISP_CBUFF0_CTRL (0x020)
-#define ISP_CBUFF1_CTRL (0x024)
-
-#define ISP_CBUFF0_START (0x040)
-#define ISP_CBUFF1_START (0x044)
-
-#define ISP_CBUFF0_END (0x050)
-#define ISP_CBUFF1_END (0x054)
-
-#define ISP_CBUFF0_WINDOWSIZE (0x060)
-#define ISP_CBUFF1_WINDOWSIZE (0x064)
-
-#define ISP_CBUFF0_THRESHOLD (0x070)
-#define ISP_CBUFF1_THRESHOLD (0x074)
-
/* CCDC module register offset */
#define ISPCCDC_PID (0x000)