diff mbox series

[1/3] media: qcom: camss: vfe: Stop spamming logs with version

Message ID 20250429180828.950219-4-krzysztof.kozlowski@linaro.org
State Superseded
Headers show
Series [1/3] media: qcom: camss: vfe: Stop spamming logs with version | expand

Commit Message

Krzysztof Kozlowski April 29, 2025, 6:08 p.m. UTC
Camss drivers spam kernel dmesg with 64 useless messages during boot:

  qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
  qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0

All of these messages are the same, so it makes no sense to print same
information 32 times.

The driver does not use read version at all, so if it was needed for any
real debugging purpose it would be provided via debugfs interface.
However even then printing this is pointless, because version of
hardware block is deducible from the compatible.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../media/platform/qcom/camss/camss-vfe-17x.c |  1 -
 .../media/platform/qcom/camss/camss-vfe-4-1.c |  1 -
 .../media/platform/qcom/camss/camss-vfe-4-7.c |  1 -
 .../media/platform/qcom/camss/camss-vfe-4-8.c |  1 -
 .../media/platform/qcom/camss/camss-vfe-480.c |  1 -
 .../media/platform/qcom/camss/camss-vfe-680.c |  1 -
 .../media/platform/qcom/camss/camss-vfe-780.c |  1 -
 drivers/media/platform/qcom/camss/camss-vfe.c | 22 -------------------
 drivers/media/platform/qcom/camss/camss-vfe.h |  8 -------
 9 files changed, 37 deletions(-)

Comments

Krzysztof Kozlowski April 29, 2025, 6:11 p.m. UTC | #1
On 29/04/2025 20:08, Krzysztof Kozlowski wrote:
> Camss drivers spam kernel dmesg with 64 useless messages during boot:
> 
>   qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
>   qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
> 
> All of these messages are the same, so it makes no sense to print same
> information 32 times.
> 
> The driver does not use read version at all, so if it was needed for any
> real debugging purpose it would be provided via debugfs interface.
> However even then printing this is pointless, because version of
> hardware block is deducible from the compatible.
This is how the dmesg looks after camss:


qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:3 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2
qcom-camss acb7000.isp: VFE:0 HW Version = 3.0.2


Best regards,
Krzysztof
Bryan O'Donoghue April 29, 2025, 7:40 p.m. UTC | #2
On 29/04/2025 19:08, Krzysztof Kozlowski wrote:
> -	dev_dbg(vfe->camss->dev, "VFE:%d HW Version = %u.%u.%u\n",
> -		vfe->id, gen, rev, step);

Please just change to dev_dbg_once() instead of entirely removing.

Same comment with the other patches.

---
bod
Krzysztof Kozlowski April 30, 2025, 6:15 a.m. UTC | #3
On 29/04/2025 21:40, Bryan O'Donoghue wrote:
> On 29/04/2025 19:08, Krzysztof Kozlowski wrote:
>> -	dev_dbg(vfe->camss->dev, "VFE:%d HW Version = %u.%u.%u\n",
>> -		vfe->id, gen, rev, step);
> 
> Please just change to dev_dbg_once() instead of entirely removing.

Why?

This is entirely useless message, isn't it? Version is deducible from
the compatible.

> 
> Same comment with the other patches.
> 
> ---
> bod


Best regards,
Krzysztof
Johan Hovold April 30, 2025, 7:25 a.m. UTC | #4
On Tue, Apr 29, 2025 at 08:08:29PM +0200, Krzysztof Kozlowski wrote:
> Camss drivers spam kernel dmesg with 64 useless messages during boot:
> 
>   qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
>   qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
> 
> All of these messages are the same, so it makes no sense to print same
> information 32 times.

It's even worse then that (several hundred messages during use) and I
sent fixes for these regressions a few weeks ago:

	https://lore.kernel.org/lkml/20250407104828.3833-1-johan+linaro@kernel.org/
	https://lore.kernel.org/lkml/20250407085125.21325-1-johan+linaro@kernel.org/

Unfortunately, it seems Bryan missed that this was a regression that
should be fixed in 6.15 and only included them in a pull request for 6.16:

	https://lore.kernel.org/all/20250410233039.77093-1-bod@kernel.org/

Bryan, has your PR been merged? Can you try to get my fixes into 6.15
since this is a regression in 6.15-rc1?

Johan
Krzysztof Kozlowski April 30, 2025, 8:19 a.m. UTC | #5
On 30/04/2025 09:25, Johan Hovold wrote:
> On Tue, Apr 29, 2025 at 08:08:29PM +0200, Krzysztof Kozlowski wrote:
>> Camss drivers spam kernel dmesg with 64 useless messages during boot:
>>
>>   qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
>>   qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
>>
>> All of these messages are the same, so it makes no sense to print same
>> information 32 times.
> 
> It's even worse then that (several hundred messages during use) and I
> sent fixes for these regressions a few weeks ago:
> 
> 	https://lore.kernel.org/lkml/20250407104828.3833-1-johan+linaro@kernel.org/
> 	https://lore.kernel.org/lkml/20250407085125.21325-1-johan+linaro@kernel.org/

Oh damn...

I developed this on top of next, so already with your fixes included,
but - following standard kernel coding practice that drivers should be
silent on success - I think even debug messages are not needed here.

There is really no point in printing (even as debug) version of hw block
EVERY TIME I boot the hardware. It does not change, does it?

If anyone wants to know it and cannot deduce from compatible, then add
debugfs interface.


Best regards,
Krzysztof
Bryan O'Donoghue April 30, 2025, 8:24 a.m. UTC | #6
On 30/04/2025 08:25, Johan Hovold wrote:
> Unfortunately, it seems Bryan missed that this was a regression that
> should be fixed in 6.15 and only included them in a pull request for 6.16:
> 
> 	https://lore.kernel.org/all/20250410233039.77093-1-bod@kernel.org/
> 
> Bryan, has your PR been merged? Can you try to get my fixes into 6.15
> since this is a regression in 6.15-rc1?

Let me see.. there's a -fixes branch, I think I should be able to PR 
anything with a Fixes: tag there.

---
bod
Bryan O'Donoghue April 30, 2025, 8:30 a.m. UTC | #7
On 30/04/2025 09:19, Krzysztof Kozlowski wrote:
> If anyone wants to know it and cannot deduce from compatible, then add
> debugfs interface.

dev_dbg(); isn't too offensive really IMO but if it really bothers you 
switching to debugfs would be fine.

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/e62825fc2ed737ab88085567f0947306a2a0da9b

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/ff0d7d980ec8192b459b5926b85a105917746d91

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/3580ffcbe507036c35e8f21e293f018fbb62d8bf

https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/cd88d924eb55f5dfeb2283e6e0eef37d5bd4c1c4

---
bod
Krzysztof Kozlowski April 30, 2025, 8:33 a.m. UTC | #8
On 30/04/2025 10:30, Bryan O'Donoghue wrote:
> On 30/04/2025 09:19, Krzysztof Kozlowski wrote:
>> If anyone wants to know it and cannot deduce from compatible, then add
>> debugfs interface.
> 
> dev_dbg(); isn't too offensive really IMO but if it really bothers you 
> switching to debugfs would be fine.

Yes, please. Dmesg should be only contain issues or some useful
debugging data. Probe success is not useful. It duplicates sysfs and
tracing. Version of hardware - well, I am sure it duplicates the compatible.

> 
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/e62825fc2ed737ab88085567f0947306a2a0da9b
> 
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/ff0d7d980ec8192b459b5926b85a105917746d91
> 
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/3580ffcbe507036c35e8f21e293f018fbb62d8bf
> 
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/cd88d924eb55f5dfeb2283e6e0eef37d5bd4c1c4
> 
> ---
> bod


Best regards,
Krzysztof
Johan Hovold April 30, 2025, 12:31 p.m. UTC | #9
On Wed, Apr 30, 2025 at 10:19:13AM +0200, Krzysztof Kozlowski wrote:
> On 30/04/2025 09:25, Johan Hovold wrote:
> > On Tue, Apr 29, 2025 at 08:08:29PM +0200, Krzysztof Kozlowski wrote:
> >> Camss drivers spam kernel dmesg with 64 useless messages during boot:
> >>
> >>   qcom-camss acb7000.isp: VFE:1 HW Version = 3.0.2
> >>   qcom-camss acb7000.isp: VFE:2 HW Version = 2.4.0
> >>
> >> All of these messages are the same, so it makes no sense to print same
> >> information 32 times.
> > 
> > It's even worse then that (several hundred messages during use) and I
> > sent fixes for these regressions a few weeks ago:
> > 
> > 	https://lore.kernel.org/lkml/20250407104828.3833-1-johan+linaro@kernel.org/
> > 	https://lore.kernel.org/lkml/20250407085125.21325-1-johan+linaro@kernel.org/
> 
> Oh damn...
> 
> I developed this on top of next, so already with your fixes included,
> but - following standard kernel coding practice that drivers should be
> silent on success - I think even debug messages are not needed here.

Ah, ok, it's based on my fixes and just removing the debug printks.

I think that should be made more clear in the commit message as debug
printks are not enabled by default and the rules for those are less
strict.

> There is really no point in printing (even as debug) version of hw block
> EVERY TIME I boot the hardware. It does not change, does it?

Well, it doesn't hurt to print it once on probe if this is something
read out from the hardware, but clearly printing it hundreds of times
during use make that debug statement pretty useless.

Johan
Johan Hovold April 30, 2025, 12:34 p.m. UTC | #10
On Wed, Apr 30, 2025 at 09:24:12AM +0100, Bryan O'Donoghue wrote:
> On 30/04/2025 08:25, Johan Hovold wrote:
> > Unfortunately, it seems Bryan missed that this was a regression that
> > should be fixed in 6.15 and only included them in a pull request for 6.16:
> > 
> > 	https://lore.kernel.org/all/20250410233039.77093-1-bod@kernel.org/
> > 
> > Bryan, has your PR been merged? Can you try to get my fixes into 6.15
> > since this is a regression in 6.15-rc1?
> 
> Let me see.. there's a -fixes branch, I think I should be able to PR 
> anything with a Fixes: tag there.

I see now that you added CC stable tags to my commits so the fixes
should trickle back to 6.15 eventually, but for issues introduced in
-rc1 you should try to get them fixed in the same development cycle.

Johan
Johan Hovold May 20, 2025, 7:53 a.m. UTC | #11
On Tue, May 20, 2025 at 08:06:22AM +0200, Krzysztof Kozlowski wrote:
> On 30/04/2025 10:33, Krzysztof Kozlowski wrote:
> > On 30/04/2025 10:30, Bryan O'Donoghue wrote:
> >> On 30/04/2025 09:19, Krzysztof Kozlowski wrote:
> >>> If anyone wants to know it and cannot deduce from compatible, then add
> >>> debugfs interface.
> >>
> >> dev_dbg(); isn't too offensive really IMO but if it really bothers you 
> >> switching to debugfs would be fine.
> > 
> > Yes, please. Dmesg should be only contain issues or some useful
> > debugging data. Probe success is not useful. It duplicates sysfs and
> > tracing. Version of hardware - well, I am sure it duplicates the compatible.
> 
> To recall: kernel coding style is also clear here:
> "When drivers are working properly they are quiet,"

That's clear and well known (or should be).

> and kernel debugging guide as well:
> "In almost all cases the debug statements shouldn't be upstreamed, as a
> working driver is supposed to be silent."

But this is a very recent addition and questionable when read in
isolation since debug statements are not printed by default. The
preceding sentences do qualify this:

	Permanent debug statements have to be useful for a developer to
	troubleshoot driver misbehavior. Judging that is a bit more of
	an art than a science...

> So I really do not get why this driver deserved exception. Nevertheless
> I think we agreed that these logs can go away, thus I just sent a v2
> with a bit extended commit msg.

Spamming the logs as the driver currently does is clearly broken and
should be fixed. Keeping a hw version dev_dbg() is generally perfectly
fine, though.

Johan
Johan Hovold May 20, 2025, 8:23 a.m. UTC | #12
On Tue, May 20, 2025 at 10:02:32AM +0200, Krzysztof Kozlowski wrote:
> On 20/05/2025 09:53, Johan Hovold wrote:

> > Spamming the logs as the driver currently does is clearly broken and
> > should be fixed. Keeping a hw version dev_dbg() is generally perfectly
> > fine, though.

> My main argument, expressed in the commit msg to which no one objected,
> is that this debug is 100% useless: deducible from the compatible,
> always known upfront, always the same.

To me that deduction does not seem straightforward, at least not without
access to internal qualcomm docs, for example:

	compatible = "qcom,sc8280xp-camss";

        qcom-camss ac5a000.camss: VFE:0 HW Version = 1.2.2
	qcom-camss ac5a000.camss: VFE:1 HW Version = 1.2.2
        qcom-camss ac5a000.camss: VFE:2 HW Version = 1.2.2
        qcom-camss ac5a000.camss: VFE:3 HW Version = 1.2.2
        qcom-camss ac5a000.camss: VFE:4 HW Version = 1.3.0
        qcom-camss ac5a000.camss: VFE:5 HW Version = 1.3.0
        qcom-camss ac5a000.camss: VFE:6 HW Version = 1.3.0
        qcom-camss ac5a000.camss: VFE:7 HW Version = 1.3.0

Whether the hw version is actually useful to anyone debugging this
driver I can't say, but keeping it printed *once* seems perfectly
alright if someone wants to keep it (e.g. as we have a long history of
working around hw bugs based on revision information like this).

Johan
Krzysztof Kozlowski May 20, 2025, 8:51 a.m. UTC | #13
On 20/05/2025 10:44, Bryan O'Donoghue wrote:
> On 20/05/2025 09:30, Krzysztof Kozlowski wrote:
>> On 20/05/2025 10:23, Johan Hovold wrote:
>>> On Tue, May 20, 2025 at 10:02:32AM +0200, Krzysztof Kozlowski wrote:
>>>> On 20/05/2025 09:53, Johan Hovold wrote:
>>>
>>>>> Spamming the logs as the driver currently does is clearly broken and
>>>>> should be fixed. Keeping a hw version dev_dbg() is generally perfectly
>>>>> fine, though.
>>>
>>>> My main argument, expressed in the commit msg to which no one objected,
>>>> is that this debug is 100% useless: deducible from the compatible,
>>>> always known upfront, always the same.
>>>
>>> To me that deduction does not seem straightforward, at least not without
>>> access to internal qualcomm docs, for example:
>>>
>>> 	compatible = "qcom,sc8280xp-camss";
>>>
>>>          qcom-camss ac5a000.camss: VFE:0 HW Version = 1.2.2
>>> 	qcom-camss ac5a000.camss: VFE:1 HW Version = 1.2.2
>>>          qcom-camss ac5a000.camss: VFE:2 HW Version = 1.2.2
>>>          qcom-camss ac5a000.camss: VFE:3 HW Version = 1.2.2
>>>          qcom-camss ac5a000.camss: VFE:4 HW Version = 1.3.0
>>>          qcom-camss ac5a000.camss: VFE:5 HW Version = 1.3.0
>>>          qcom-camss ac5a000.camss: VFE:6 HW Version = 1.3.0
>>>          qcom-camss ac5a000.camss: VFE:7 HW Version = 1.3.0
>>>
>>
>> I understand that deduction is not straightforward, but it is also a
>> fixed one, meaning it will be always sc8280xp -> (vFOO, vBAR), thus the
>> only usefulness of above is to map each compatible to pair of two HW
>> versions. This can be done via debugfs interface once and stored in
>> public docs. No need to do that mapping every time driver probes and my
>> patches drop nice chunk of code, including indirect function calls.
>>
>> At least so far no one objected that same compatible maps to same pairs
>> of HW versions.
>>
>>> Whether the hw version is actually useful to anyone debugging this
>>> driver I can't say, but keeping it printed *once* seems perfectly
>>> alright if someone wants to keep it (e.g. as we have a long history of
>>> working around hw bugs based on revision information like this).
>>
>> Now if you claim that one needs access to qcom docs to deduce it, I
>> claim this version would be useful only to qcom people (or
>> qcom-NDA-access-to-HPG) folks.
>>
>>
>> Best regards,
>> Krzysztof
> 
> I find the debug prints useful in that I know the hardware block has 
> been powered on, clocked etc. I agree the number of those prints seems 
> excessive.
> 
> The reason it is printed out multiple time is the blocks get powered on/off.
> 
> Personally I agree with Johan - it would be nice/useful to print it out 
> once with DEBUG on, so that we know we have successfully powered-on and 

That's opposite to what coding style asks. Success should be silent.

> identified the blocks once.

Last time you suggested to print it once, so this is contradictory. If
you need simple probe success (or component bind) confirmation, then
fortunately we already have infrastructure for that: sysfs and tracing.

This log should either say something useful or not be there at all.
Printing same version 4 times is not useful at all, considering all my
previous arguments that this maps to compatible 1-to-1.

> 
> Doing it over and over again is excessive as failure to power-on will 
> surely produce error messages anyway.


Best regards,
Krzysztof
Bryan O'Donoghue May 20, 2025, 8:58 a.m. UTC | #14
On 20/05/2025 09:51, Krzysztof Kozlowski wrote:
>           qcom-camss ac5a000.camss: VFE:0 HW Version = 1.2.2
>           qcom-camss ac5a000.camss: VFE:1 HW Version = 1.2.2
>           qcom-camss ac5a000.camss: VFE:2 HW Version = 1.2.2
>           qcom-camss ac5a000.camss: VFE:3 HW Version = 1.2.2
>           qcom-camss ac5a000.camss: VFE:4 HW Version = 1.3.0
>           qcom-camss ac5a000.camss: VFE:5 HW Version = 1.3.0
>           qcom-camss ac5a000.camss: VFE:6 HW Version = 1.3.0
>           qcom-camss ac5a000.camss: VFE:7 HW Version = 1.3.0

This prints the hardware version of eight distinct hardware blocks VFE 
index increases.

TBH I still find this useful when debugging hardware.

My personal preference is to print it once on boot and skip subsequent. 
Which I think is perfectly reasonable for DEBUG scenario.

---
bod
Krzysztof Kozlowski May 20, 2025, 9:16 a.m. UTC | #15
On 20/05/2025 10:58, Bryan O'Donoghue wrote:
> On 20/05/2025 09:51, Krzysztof Kozlowski wrote:
>>           qcom-camss ac5a000.camss: VFE:0 HW Version = 1.2.2
>>           qcom-camss ac5a000.camss: VFE:1 HW Version = 1.2.2
>>           qcom-camss ac5a000.camss: VFE:2 HW Version = 1.2.2
>>           qcom-camss ac5a000.camss: VFE:3 HW Version = 1.2.2
>>           qcom-camss ac5a000.camss: VFE:4 HW Version = 1.3.0
>>           qcom-camss ac5a000.camss: VFE:5 HW Version = 1.3.0
>>           qcom-camss ac5a000.camss: VFE:6 HW Version = 1.3.0
>>           qcom-camss ac5a000.camss: VFE:7 HW Version = 1.3.0
> 
> This prints the hardware version of eight distinct hardware blocks VFE 
> index increases.
> 
> TBH I still find this useful when debugging hardware.

How? Can you respond to actual arguments repeated 5 times that this is
fixed and always known?

If this is always known, in what way this is useful?

> 
> My personal preference is to print it once on boot and skip subsequent. 
> Which I think is perfectly reasonable for DEBUG scenario.
What are you responding to? Before you said you find it useful for
knowing each block power up and down?



Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-17x.c b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
index e5ee7e717b3b..e0d12c3f6015 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-17x.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
@@ -577,7 +577,6 @@  static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
 
 const struct vfe_hw_ops vfe_ops_170 = {
 	.global_reset = vfe_global_reset,
-	.hw_version = vfe_hw_version,
 	.isr_read = vfe_isr_read,
 	.isr = vfe_isr,
 	.pm_domain_off = vfe_pm_domain_off,
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
index 901677293d97..7620ce42b49b 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
@@ -993,7 +993,6 @@  static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
 
 const struct vfe_hw_ops vfe_ops_4_1 = {
 	.global_reset = vfe_global_reset,
-	.hw_version = vfe_hw_version,
 	.isr_read = vfe_isr_read,
 	.isr = vfe_isr,
 	.pm_domain_off = vfe_4_1_pm_domain_off,
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
index 76729607db02..b3b6ccb4748e 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
@@ -1145,7 +1145,6 @@  static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
 
 const struct vfe_hw_ops vfe_ops_4_7 = {
 	.global_reset = vfe_global_reset,
-	.hw_version = vfe_hw_version,
 	.isr_read = vfe_isr_read,
 	.isr = vfe_isr,
 	.pm_domain_off = vfe_pm_domain_off,
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
index b2f7d855d8dd..5a4b4f486aca 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
@@ -1135,7 +1135,6 @@  static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
 
 const struct vfe_hw_ops vfe_ops_4_8 = {
 	.global_reset = vfe_global_reset,
-	.hw_version = vfe_hw_version,
 	.isr_read = vfe_isr_read,
 	.isr = vfe_isr,
 	.pm_domain_off = vfe_pm_domain_off,
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-480.c b/drivers/media/platform/qcom/camss/camss-vfe-480.c
index 4feea590a47b..edd92308af62 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-480.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-480.c
@@ -278,7 +278,6 @@  static void vfe_buf_done_480(struct vfe_device *vfe, int port_id)
 const struct vfe_hw_ops vfe_ops_480 = {
 	.enable_irq = vfe_enable_irq,
 	.global_reset = vfe_global_reset,
-	.hw_version = vfe_hw_version,
 	.isr = vfe_isr,
 	.isr_read = vfe_isr_read,
 	.reg_update = vfe_reg_update,
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-680.c b/drivers/media/platform/qcom/camss/camss-vfe-680.c
index 99036e7c1e76..96a927acc6bb 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-680.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-680.c
@@ -227,7 +227,6 @@  static inline void vfe_reg_update_clear(struct vfe_device *vfe,
 
 const struct vfe_hw_ops vfe_ops_680 = {
 	.global_reset = vfe_global_reset,
-	.hw_version = vfe_hw_version,
 	.isr = vfe_isr,
 	.pm_domain_off = vfe_pm_domain_off,
 	.pm_domain_on = vfe_pm_domain_on,
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-780.c b/drivers/media/platform/qcom/camss/camss-vfe-780.c
index b9812d70f91b..e5023eb7ad60 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-780.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-780.c
@@ -142,7 +142,6 @@  static int vfe_halt(struct vfe_device *vfe)
 
 const struct vfe_hw_ops vfe_ops_780 = {
 	.global_reset = vfe_global_reset,
-	.hw_version = vfe_hw_version,
 	.isr = vfe_isr,
 	.pm_domain_off = vfe_pm_domain_off,
 	.pm_domain_on = vfe_pm_domain_on,
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 4bca6c3abaff..1ae523219525 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -415,26 +415,6 @@  static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code,
 	return 0;
 }
 
-/*
- * vfe_hw_version - Process write master done interrupt
- * @vfe: VFE Device
- *
- * Return vfe hw version
- */
-u32 vfe_hw_version(struct vfe_device *vfe)
-{
-	u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION);
-
-	u32 gen = (hw_version >> HW_VERSION_GENERATION) & 0xF;
-	u32 rev = (hw_version >> HW_VERSION_REVISION) & 0xFFF;
-	u32 step = (hw_version >> HW_VERSION_STEPPING) & 0xFFFF;
-
-	dev_dbg(vfe->camss->dev, "VFE:%d HW Version = %u.%u.%u\n",
-		vfe->id, gen, rev, step);
-
-	return hw_version;
-}
-
 /*
  * vfe_buf_done - Process write master done interrupt
  * @vfe: VFE Device
@@ -1088,8 +1068,6 @@  int vfe_get(struct vfe_device *vfe)
 		vfe_reset_output_maps(vfe);
 
 		vfe_init_outputs(vfe);
-
-		vfe->res->hw_ops->hw_version(vfe);
 	} else {
 		ret = vfe_check_clock_rates(vfe);
 		if (ret < 0)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
index a23f666be753..1553ca89bd86 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.h
+++ b/drivers/media/platform/qcom/camss/camss-vfe.h
@@ -101,7 +101,6 @@  struct vfe_device;
 struct vfe_hw_ops {
 	void (*enable_irq)(struct vfe_device *vfe);
 	void (*global_reset)(struct vfe_device *vfe);
-	u32 (*hw_version)(struct vfe_device *vfe);
 	irqreturn_t (*isr)(int irq, void *dev);
 	void (*isr_read)(struct vfe_device *vfe, u32 *value0, u32 *value1);
 	void (*pm_domain_off)(struct vfe_device *vfe);
@@ -259,13 +258,6 @@  void vfe_put(struct vfe_device *vfe);
  */
 bool vfe_is_lite(struct vfe_device *vfe);
 
-/*
- * vfe_hw_version - Process write master done interrupt
- * @vfe: VFE Device
- *
- * Return vfe hw version
- */
-u32 vfe_hw_version(struct vfe_device *vfe);
 /*
  * vfe_enable - Enable streaming on VFE line
  * @line: VFE line