Re: [PATCH] media: vsp1: tidyup VI6_HGT_LBn_H() macro

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 13, 2019 at 11:55:14AM +0000, Kieran Bingham wrote:
Hi Greg, Laurent, Sasha,

On 12/12/2019 07:33, Greg Kroah-Hartman wrote:
On Wed, Dec 11, 2019 at 09:58:11PM +0000, Kieran Bingham wrote:
Hi Laurent,

+Greg, +Sasha to opine on the merit of whether this should go to stable
trees (for my future learning and understanding more so than this
specific case)

On 11/12/2019 17:58, Laurent Pinchart wrote:
Hello,

On Wed, Dec 11, 2019 at 12:59:57PM +0000, Kieran Bingham wrote:
Hi Morimoto-san,

Thank you for the patch,

Likewise :-)

On 11/12/2019 01:55, Kuninori Morimoto wrote:

From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>

The address of VSP2_VI6_HGT_LBx_H are
	VSP2_VI6_HGT_LB0_H : 0x3428
	VSP2_VI6_HGT_LB1_H : 0x3430
	VSP2_VI6_HGT_LB2_H : 0x3438
	VSP2_VI6_HGT_LB3_H : 0x3440

Thus, VI6_HGT_LBn_H() macro should start from 0x3420 instead of 0x3430.
This patch fixup it.

s/fixup/fixes/


I think this deserves a fixes tag:

Fixes: 26e0ca22c3b8 ("[media] v4l: Renesas R-Car VSP1 driver")

Given that this macro is not used, we could argue that it doesn't fix
anything yet :-) I'd rather avoid having this backported to stable
kernels as it's not useful to have it there, and thus not add a Fixes

I'm sorry - I'm not sure I can agree here, Do you know that no one will
use this macro when they back port the HGT functionality to an LTSI kernel?

We know the Renesas BSP uses LTSI kernels, and the very nature of the
fact that this typo has been spotted by the Renesas BSP team suggests
that they are indeed looking at/using this functionality ...

(Ok, so maybe they will thus apply the fix themselves, but that's not my
point, and if they 'have' to apply the fix - it should be in stable?)

It feels a bit presumptuous to state that we shouldn't fix this because
/we/ don't utilise it yet, when this issue is in mainline regardless ...

Nothing should be in the kernel tree that is not already used by
something in that specific kernel tree.  We don't care about out-of-tree
code, and especially for stable kernel patches, it does not matter in
the least.

So perhaps this patch should actually remove this macro rather than fix it?

If you have out-of-tree code, you are on your own here, sorry.

So no, no backporting of stuff that no one actually uses in the codebase
itself.

Ok understood, It was really the 'the macro exists in the kernel, but is
wrong' part that got me.

Along with the fact that we now have various automated machinery that
would likely pick this patch up and backport it anyway?

(Sasha, is that assumption accurate? Or would you/your system have
identified that this macro is not used?)

No, it would have likely just backported it.

The right thing to do is to just remove the macro if no one is using it.

--
Thanks,
Sasha



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux