diff mbox

davinci: fix DM365 EVM video input mux bits

Message ID 1311046211-2849-1-git-send-email-jon.povey@racelogic.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Jon Povey July 19, 2011, 3:30 a.m. UTC
Video input mux settings for tvp7002 and imager inputs were swapped.
Comment was correct.

Tested on EVM with tvp7002 input.

Signed-off-by: Jon Povey <jon.povey@racelogic.co.uk>
---
As a bugfix maybe this should try to get into 3.0?

 arch/arm/mach-davinci/board-dm365-evm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Comments

Manjunath Hadli July 19, 2011, 5:29 a.m. UTC | #1
Jon,

The fix looks correct. As a part of a different implementation which will go soon upstream, this has been taken care of, where we have used macros 
CPLD_VIDEO_INPUT_MUX_TVP7002    0x1
CPLD_VIDEO_INPUT_MUX_IMAGER     0x2

So, if you can do the same, it will look better.

Thanks,
-Manju


On Tue, Jul 19, 2011 at 09:00:11, Jon Povey wrote:
> Video input mux settings for tvp7002 and imager inputs were swapped.
> Comment was correct.
> 
> Tested on EVM with tvp7002 input.
> 
> Signed-off-by: Jon Povey <jon.povey@racelogic.co.uk>
> ---
> As a bugfix maybe this should try to get into 3.0?
> 
>  arch/arm/mach-davinci/board-dm365-evm.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/board-dm365-evm.c b/arch/arm/mach-davinci/board-dm365-evm.c
> index c67f684..09a87e6 100644
> --- a/arch/arm/mach-davinci/board-dm365-evm.c
> +++ b/arch/arm/mach-davinci/board-dm365-evm.c
> @@ -520,7 +520,7 @@ fail:
>  	 */
>  	if (have_imager()) {
>  		label = "HD imager";
> -		mux |= 1;
> +		mux |= 2;
>  
>  		/* externally mux MMC1/ENET/AIC33 to imager */
>  		mux |= BIT(6) | BIT(5) | BIT(3);
> @@ -540,7 +540,7 @@ fail:
>  		resets &= ~BIT(1);
>  
>  		if (have_tvp7002()) {
> -			mux |= 2;
> +			mux |= 1;
>  			resets &= ~BIT(2);
>  			label = "tvp7002 HD";
>  		} else {
> --
> 1.6.3.3
> 
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source@linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
>
Jon Povey July 19, 2011, 6:28 a.m. UTC | #2
Hadli, Manjunath wrote:
> Jon,
>
> The fix looks correct. As a part of a different
> implementation which will go soon upstream, this has been
> taken care of, where we have used macros
> CPLD_VIDEO_INPUT_MUX_TVP7002    0x1
> CPLD_VIDEO_INPUT_MUX_IMAGER     0x2
>
> So, if you can do the same, it will look better.

If you have a patch you can split out that has your defines
where and how you want them then great.

Otherwise I'd suggest just getting this or something equivalent
in to fix the bug first, and reimplement it later.

--
Jon Povey
jon.povey@racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network
Manjunath Hadli July 19, 2011, 7:25 a.m. UTC | #3
Will not be able to give you a sub-patch as it is combined as part of a larger implementation.

Since the bug fix needs to go into 3.0, I am Ok if you fix the bug, and the macros can be substituted later.

-Manju

On Tue, Jul 19, 2011 at 11:58:19, Jon Povey wrote:
> Hadli, Manjunath wrote:
> > Jon,
> >
> > The fix looks correct. As a part of a different implementation which 
> > will go soon upstream, this has been taken care of, where we have used 
> > macros
> > CPLD_VIDEO_INPUT_MUX_TVP7002    0x1
> > CPLD_VIDEO_INPUT_MUX_IMAGER     0x2
> >
> > So, if you can do the same, it will look better.
> 
> If you have a patch you can split out that has your defines where and how you want them then great.
> 
> Otherwise I'd suggest just getting this or something equivalent in to fix the bug first, and reimplement it later.
> 
> --
> Jon Povey
> jon.povey@racelogic.co.uk
> 
> Racelogic is a limited company registered in England. Registered number 2743719 . 
> Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .
> 
> The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network
> 
> 
>
Sekhar Nori July 19, 2011, 8:56 a.m. UTC | #4
Hi Jon,

On Tue, Jul 19, 2011 at 09:00:11, Jon Povey wrote:
> Video input mux settings for tvp7002 and imager inputs were swapped.
> Comment was correct.
> 
> Tested on EVM with tvp7002 input.
> 
> Signed-off-by: Jon Povey <jon.povey@racelogic.co.uk>

Thanks for the fix.

Since this part of the code hasn't changed since it
was written, this is a good candidate for stable
kernel series. You can just add:

Cc: stable@kernel.org

Also, this way if the fix fails to make it to 3.0,
it can make it into its stable series. I will add
the Cc line when I queue this patch.

> ---
> As a bugfix maybe this should try to get into 3.0?

I am going queue this to Arnd for sending it for v3.0.
He hasn't yet picked up my earlier pull request yet so
keep fingers crossed :)

BTW, I changed the subject to:

"davinci: DM365 EVM: fix video input mux bits"

Nothing really wrong with what you had, just that
I prefer it this way.

Thanks,
Sekhar
Sekhar Nori July 19, 2011, 9 a.m. UTC | #5
Hi Manju,

On Tue, Jul 19, 2011 at 12:55:13, Hadli, Manjunath wrote:
> Will not be able to give you a sub-patch as it is combined as part of a larger implementation.
> 
> Since the bug fix needs to go into 3.0, I am Ok if you fix the bug, and the 
> macros can be substituted later.

I am going to take this as your ack and add:

Acked-by: Manjunath Hadli <manjunath.hadli@ti.com>

while committing.

Thanks,
Sekhar
Manjunath Hadli July 19, 2011, 10:24 a.m. UTC | #6
Sure. Please do.

Thanks and Regards,
-Manju
Russell King - ARM Linux July 19, 2011, 10:55 a.m. UTC | #7
On Tue, Jul 19, 2011 at 02:26:50PM +0530, Nori, Sekhar wrote:
> Since this part of the code hasn't changed since it
> was written, this is a good candidate for stable
> kernel series. You can just add:
> 
> Cc: stable@kernel.org

Be careful when doing that - if you have git mail the patches out for
review, and it picks that up, it'll send it to the stable team as well.
The stable team will then complain "that's not how to submit patches
to them".

So, while you can add "Cc: stable@kernel.org" to the patch, its best
to only add that just before you publish the commit after review, but
before merging.
Sekhar Nori July 20, 2011, 4:40 p.m. UTC | #8
Hi Russell,

On Tue, Jul 19, 2011 at 16:25:32, Russell King - ARM Linux wrote:
> On Tue, Jul 19, 2011 at 02:26:50PM +0530, Nori, Sekhar wrote:
> > Since this part of the code hasn't changed since it
> > was written, this is a good candidate for stable
> > kernel series. You can just add:
> > 
> > Cc: stable@kernel.org
> 
> Be careful when doing that - if you have git mail the patches out for
> review, and it picks that up, it'll send it to the stable team as well.
> The stable team will then complain "that's not how to submit patches
> to them".
> 
> So, while you can add "Cc: stable@kernel.org" to the patch, its best
> to only add that just before you publish the commit after review, but
> before merging.

But its very tough to guess what the last version
ready for merging will be. Review comments can come
at any stage.

Would adding "Cc: stable@kernel.org" to the commit text
but taking care to not actually send mail to stable@kernel.org
help? The stable_kernel_rules.txt document seems to suggest
this is an acceptable way:

"
- To have the patch automatically included in the stable tree, add the tag
     Cc: stable@kernel.org
   in the sign-off area. Once the patch is merged it will be applied to
   the stable tree without anything else needing to be done by the author
   or subsystem maintainer.
"

Thanks,
Sekhar
Russell King - ARM Linux July 20, 2011, 4:48 p.m. UTC | #9
On Wed, Jul 20, 2011 at 10:10:57PM +0530, Nori, Sekhar wrote:
> Hi Russell,
> 
> On Tue, Jul 19, 2011 at 16:25:32, Russell King - ARM Linux wrote:
> > On Tue, Jul 19, 2011 at 02:26:50PM +0530, Nori, Sekhar wrote:
> > > Since this part of the code hasn't changed since it
> > > was written, this is a good candidate for stable
> > > kernel series. You can just add:
> > > 
> > > Cc: stable@kernel.org
> > 
> > Be careful when doing that - if you have git mail the patches out for
> > review, and it picks that up, it'll send it to the stable team as well.
> > The stable team will then complain "that's not how to submit patches
> > to them".
> > 
> > So, while you can add "Cc: stable@kernel.org" to the patch, its best
> > to only add that just before you publish the commit after review, but
> > before merging.
> 
> But its very tough to guess what the last version
> ready for merging will be. Review comments can come
> at any stage.
> 
> Would adding "Cc: stable@kernel.org" to the commit text
> but taking care to not actually send mail to stable@kernel.org
> help? The stable_kernel_rules.txt document seems to suggest
> this is an acceptable way:

Correct.
diff mbox

Patch

diff --git a/arch/arm/mach-davinci/board-dm365-evm.c b/arch/arm/mach-davinci/board-dm365-evm.c
index c67f684..09a87e6 100644
--- a/arch/arm/mach-davinci/board-dm365-evm.c
+++ b/arch/arm/mach-davinci/board-dm365-evm.c
@@ -520,7 +520,7 @@  fail:
 	 */
 	if (have_imager()) {
 		label = "HD imager";
-		mux |= 1;
+		mux |= 2;
 
 		/* externally mux MMC1/ENET/AIC33 to imager */
 		mux |= BIT(6) | BIT(5) | BIT(3);
@@ -540,7 +540,7 @@  fail:
 		resets &= ~BIT(1);
 
 		if (have_tvp7002()) {
-			mux |= 2;
+			mux |= 1;
 			resets &= ~BIT(2);
 			label = "tvp7002 HD";
 		} else {