[PATCH v1 1/2] firmware: sysfb: Unorphan sysfb files

Andy Shevchenko posted 2 patches 3 months, 1 week ago
[PATCH v1 1/2] firmware: sysfb: Unorphan sysfb files
Posted by Andy Shevchenko 3 months, 1 week ago
The commit d391c5827107 ("drivers/firmware: move x86 Generic
System Framebuffers support") moved some code to the common
folders and effectively orphaned it without any reason. Put
it back under DRM MISC record.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8dc0f6609d1f..c2c7aa594160 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7973,10 +7973,12 @@ T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
 F:	Documentation/devicetree/bindings/display/
 F:	Documentation/devicetree/bindings/gpu/
 F:	Documentation/gpu/
+F:	drivers/firmware/sysfb*.c
 F:	drivers/gpu/drm/
 F:	drivers/gpu/vga/
 F:	rust/kernel/drm/
 F:	include/drm/drm
+F:	include/linux/sysfb.h
 F:	include/linux/vga*
 F:	include/uapi/drm/
 X:	drivers/gpu/drm/amd/
-- 
2.47.2
Re: [PATCH v1 1/2] firmware: sysfb: Unorphan sysfb files
Posted by Javier Martinez Canillas 3 months, 1 week ago
Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:

Hello Andy,

> The commit d391c5827107 ("drivers/firmware: move x86 Generic
> System Framebuffers support") moved some code to the common
> folders and effectively orphaned it without any reason. Put
> it back under DRM MISC record.
>

What do you mean that it was "orphaned without any reason" ? There were no
regex matchs for the old file paths in MAINTAINERS either before that commit.

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  MAINTAINERS | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8dc0f6609d1f..c2c7aa594160 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7973,10 +7973,12 @@ T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
>  F:	Documentation/devicetree/bindings/display/
>  F:	Documentation/devicetree/bindings/gpu/
>  F:	Documentation/gpu/
> +F:	drivers/firmware/sysfb*.c

I would prefer these to be in the "DRM DRIVER FOR FIRMWARE FRAMEBUFFERS"
entry instead of "DRM DRIVERS" since the former is what has most of the
code for the sysfb infrastructure.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH v1 1/2] firmware: sysfb: Unorphan sysfb files
Posted by Andy Shevchenko 3 months, 1 week ago
On Fri, Jun 27, 2025 at 10:50:48AM +0200, Javier Martinez Canillas wrote:
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
> 
> > The commit d391c5827107 ("drivers/firmware: move x86 Generic
> > System Framebuffers support") moved some code to the common
> > folders and effectively orphaned it without any reason. Put
> > it back under DRM MISC record.
> 
> What do you mean that it was "orphaned without any reason" ? There were no
> regex matchs for the old file paths in MAINTAINERS either before that commit.

There were maintainers for that code before. The change in question dropped that.

[((bf44e8cecc03...))]$ scripts/get_maintainer.pl -f arch/x86/kernel/sysfb*
Thomas Gleixner <tglx@linutronix.de> (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
Ingo Molnar <mingo@redhat.com> (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
Borislav Petkov <bp@alien8.de> (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
x86@kernel.org (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
"H. Peter Anvin" <hpa@zytor.com> (reviewer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
linux-kernel@vger.kernel.org (open list:X86 ARCHITECTURE (32-BIT AND 64-BIT))

[((d391c5827107...))]$ scripts/get_maintainer.pl -f drivers/firmware/sysfb*
linux-kernel@vger.kernel.org (open list)

See the difference?

...

> > +F:	drivers/firmware/sysfb*.c

> I would prefer these to be in the "DRM DRIVER FOR FIRMWARE FRAMEBUFFERS"
> entry instead of "DRM DRIVERS" since the former is what has most of the
> code for the sysfb infrastructure.

Then do it, please, fix the above.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 1/2] firmware: sysfb: Unorphan sysfb files
Posted by Javier Martinez Canillas 3 months, 1 week ago
Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:

> On Fri, Jun 27, 2025 at 10:50:48AM +0200, Javier Martinez Canillas wrote:
>> Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
>> 
>> > The commit d391c5827107 ("drivers/firmware: move x86 Generic
>> > System Framebuffers support") moved some code to the common
>> > folders and effectively orphaned it without any reason. Put
>> > it back under DRM MISC record.
>> 
>> What do you mean that it was "orphaned without any reason" ? There were no
>> regex matchs for the old file paths in MAINTAINERS either before that commit.
>
> There were maintainers for that code before. The change in question dropped that.
>
> [((bf44e8cecc03...))]$ scripts/get_maintainer.pl -f arch/x86/kernel/sysfb*
> Thomas Gleixner <tglx@linutronix.de> (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
> Ingo Molnar <mingo@redhat.com> (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
> Borislav Petkov <bp@alien8.de> (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
> x86@kernel.org (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
> "H. Peter Anvin" <hpa@zytor.com> (reviewer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
> linux-kernel@vger.kernel.org (open list:X86 ARCHITECTURE (32-BIT AND 64-BIT))
>
> [((d391c5827107...))]$ scripts/get_maintainer.pl -f drivers/firmware/sysfb*
> linux-kernel@vger.kernel.org (open list)
>

That's just because there is an entry for arch/x86/. The problem then is
that there isn't an entry for drivers/firmware. It was orphaned then just
because it was moved to a directory that has no entry in MAINTAINERS.

> See the difference?
>

There is no need to have such a rude tone.

> ...
>
>> > +F:	drivers/firmware/sysfb*.c
>
>> I would prefer these to be in the "DRM DRIVER FOR FIRMWARE FRAMEBUFFERS"
>> entry instead of "DRM DRIVERS" since the former is what has most of the
>> code for the sysfb infrastructure.
>
> Then do it, please, fix the above.
>

Part of the review process is to give feedback to patch authors. I don't
understand why you expect me to fix an issue you brought up just because
I ask you to rework your patch a little.

> -- 
> With Best Regards,
> Andy Shevchenko
>
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH v1 1/2] firmware: sysfb: Unorphan sysfb files
Posted by Andy Shevchenko 3 months, 1 week ago
On Fri, Jun 27, 2025 at 11:19:34AM +0200, Javier Martinez Canillas wrote:
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
> > On Fri, Jun 27, 2025 at 10:50:48AM +0200, Javier Martinez Canillas wrote:
> >> Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:

> >> > The commit d391c5827107 ("drivers/firmware: move x86 Generic
> >> > System Framebuffers support") moved some code to the common
> >> > folders and effectively orphaned it without any reason. Put
> >> > it back under DRM MISC record.
> >> 
> >> What do you mean that it was "orphaned without any reason" ? There were no
> >> regex matchs for the old file paths in MAINTAINERS either before that commit.
> >
> > There were maintainers for that code before. The change in question dropped that.
> >
> > [((bf44e8cecc03...))]$ scripts/get_maintainer.pl -f arch/x86/kernel/sysfb*
> > Thomas Gleixner <tglx@linutronix.de> (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
> > Ingo Molnar <mingo@redhat.com> (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
> > Borislav Petkov <bp@alien8.de> (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
> > x86@kernel.org (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
> > "H. Peter Anvin" <hpa@zytor.com> (reviewer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
> > linux-kernel@vger.kernel.org (open list:X86 ARCHITECTURE (32-BIT AND 64-BIT))
> >
> > [((d391c5827107...))]$ scripts/get_maintainer.pl -f drivers/firmware/sysfb*
> > linux-kernel@vger.kernel.org (open list)
> 
> That's just because there is an entry for arch/x86/. The problem then is
> that there isn't an entry for drivers/firmware. It was orphaned then just
> because it was moved to a directory that has no entry in MAINTAINERS.
> 
> > See the difference?
> 
> There is no need to have such a rude tone.

It wasn't meant to be rude, sorry. The point is that any change in
drivers/firmware/sysfb* and respective include are not visible to (any)
maintainers, they just might be sent for a luck of somebody to pick
them up by browsing the LKML for such things.

...

> >> > +F:	drivers/firmware/sysfb*.c
> >
> >> I would prefer these to be in the "DRM DRIVER FOR FIRMWARE FRAMEBUFFERS"
> >> entry instead of "DRM DRIVERS" since the former is what has most of the
> >> code for the sysfb infrastructure.
> >
> > Then do it, please, fix the above.
> 
> Part of the review process is to give feedback to patch authors. I don't
> understand why you expect me to fix an issue you brought up just because
> I ask you to rework your patch a little.

In my humble opinion, the author of the patch that makes the problem appear
can help to fix that as well. Are my expectations too high?

In any case, this was an ad-hoc patch due to the second one, so this one
may be considered as a administrative bug report.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 1/2] firmware: sysfb: Unorphan sysfb files
Posted by Javier Martinez Canillas 3 months, 1 week ago
Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:

[...]

>> 
>> That's just because there is an entry for arch/x86/. The problem then is
>> that there isn't an entry for drivers/firmware. It was orphaned then just
>> because it was moved to a directory that has no entry in MAINTAINERS.
>> 
>> > See the difference?
>> 
>> There is no need to have such a rude tone.
>
> It wasn't meant to be rude, sorry. The point is that any change in

No worries.

> drivers/firmware/sysfb* and respective include are not visible to (any)
> maintainers, they just might be sent for a luck of somebody to pick
> them up by browsing the LKML for such things.
>

Right. But get_maintainer.pl still reports I think the correct people to Cc:

./scripts/get_maintainer.pl -f -- drivers/firmware/sysfb*
Thomas Zimmermann <tzimmermann@suse.de> (commit_signer:4/4=100%,authored:2/4=50%,added_lines:11/43=26%,removed_lines:5/11=45%,commit_signer:1/1=100%,authored:1/1=100%,added_lines:1/1=100%,removed_lines:30/30=100%)
Javier Martinez Canillas <javierm@redhat.com> (commit_signer:4/4=100%,authored:1/4=25%,added_lines:19/43=44%,commit_signer:1/1=100%)
Alex Deucher <alexander.deucher@amd.com> (commit_signer:3/4=75%,authored:1/4=25%,added_lines:13/43=30%,removed_lines:6/11=55%)
Tzung-Bi Shih <tzungbi@kernel.org> (commit_signer:1/4=25%)
linux-kernel@vger.kernel.org (open list)

In my opinion both Thomas and me have much more context and knowledge of
the sysfb codebase than the x86 maintainers. It was just for historical
reasons that the sysfb code ended in the arch/x86/ sub-directory.

But you are correct that dri-devel at least should also be in the Cc list.

> ...
>
>> >> > +F:	drivers/firmware/sysfb*.c
>> >
>> >> I would prefer these to be in the "DRM DRIVER FOR FIRMWARE FRAMEBUFFERS"
>> >> entry instead of "DRM DRIVERS" since the former is what has most of the
>> >> code for the sysfb infrastructure.
>> >
>> > Then do it, please, fix the above.
>> 
>> Part of the review process is to give feedback to patch authors. I don't
>> understand why you expect me to fix an issue you brought up just because
>> I ask you to rework your patch a little.
>
> In my humble opinion, the author of the patch that makes the problem appear
> can help to fix that as well. Are my expectations too high?
>
> In any case, this was an ad-hoc patch due to the second one, so this one
> may be considered as a administrative bug report.
>

That's OK, but it wasn't framed as a bug report but as a patch and that's why
I gave my feedback. But I'll post a patch and add a Reported-by tag from you.

Thomas, I think we can then only merge patch #2 and I will take care of #1.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH v1 1/2] firmware: sysfb: Unorphan sysfb files
Posted by Andy Shevchenko 3 months, 1 week ago
On Fri, Jun 27, 2025 at 12:33:31PM +0200, Javier Martinez Canillas wrote:
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:

[...]

> >> That's just because there is an entry for arch/x86/. The problem then is
> >> that there isn't an entry for drivers/firmware. It was orphaned then just
> >> because it was moved to a directory that has no entry in MAINTAINERS.
> >> 
> >> > See the difference?
> >> 
> >> There is no need to have such a rude tone.
> >
> > It wasn't meant to be rude, sorry. The point is that any change in
> 
> No worries.
> 
> > drivers/firmware/sysfb* and respective include are not visible to (any)
> > maintainers, they just might be sent for a luck of somebody to pick
> > them up by browsing the LKML for such things.
> >
> 
> Right. But get_maintainer.pl still reports I think the correct people to Cc:
> 
> ./scripts/get_maintainer.pl -f -- drivers/firmware/sysfb*
> Thomas Zimmermann <tzimmermann@suse.de> (commit_signer:4/4=100%,authored:2/4=50%,added_lines:11/43=26%,removed_lines:5/11=45%,commit_signer:1/1=100%,authored:1/1=100%,added_lines:1/1=100%,removed_lines:30/30=100%)
> Javier Martinez Canillas <javierm@redhat.com> (commit_signer:4/4=100%,authored:1/4=25%,added_lines:19/43=44%,commit_signer:1/1=100%)
> Alex Deucher <alexander.deucher@amd.com> (commit_signer:3/4=75%,authored:1/4=25%,added_lines:13/43=30%,removed_lines:6/11=55%)
> Tzung-Bi Shih <tzungbi@kernel.org> (commit_signer:1/4=25%)
> linux-kernel@vger.kernel.org (open list)

The problem is deeper. This is default behaviour of get_maintainer digging
into the Git history. In the past people were complaining (a lot in some cases)
that they were included in the threads by a mistake because they made cosmetic
patches or the treewide change while not being interested _at all_ in looking
after the certain file / driver. So, with --no-git-fallback it gives nothing,
expect LKML.

> In my opinion both Thomas and me have much more context and knowledge of
> the sysfb codebase than the x86 maintainers. It was just for historical
> reasons that the sysfb code ended in the arch/x86/ sub-directory.
> 
> But you are correct that dri-devel at least should also be in the Cc list.

See also above. Depending on the options it may still give bad result w.o.
explicit mention in the MAINTAINERS (or via glob).

...

> >> >> > +F:	drivers/firmware/sysfb*.c
> >> >
> >> >> I would prefer these to be in the "DRM DRIVER FOR FIRMWARE FRAMEBUFFERS"
> >> >> entry instead of "DRM DRIVERS" since the former is what has most of the
> >> >> code for the sysfb infrastructure.
> >> >
> >> > Then do it, please, fix the above.
> >> 
> >> Part of the review process is to give feedback to patch authors. I don't
> >> understand why you expect me to fix an issue you brought up just because
> >> I ask you to rework your patch a little.
> >
> > In my humble opinion, the author of the patch that makes the problem appear
> > can help to fix that as well. Are my expectations too high?
> >
> > In any case, this was an ad-hoc patch due to the second one, so this one
> > may be considered as a administrative bug report.
> 
> That's OK, but it wasn't framed as a bug report but as a patch and that's why
> I gave my feedback. But I'll post a patch and add a Reported-by tag from you.

Sure, thanks ahead!

> Thomas, I think we can then only merge patch #2 and I will take care of #1.

I just sent a v2 without the first patch.

Thanks for review!

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 1/2] firmware: sysfb: Unorphan sysfb files
Posted by Javier Martinez Canillas 3 months, 1 week ago
Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:

> On Fri, Jun 27, 2025 at 12:33:31PM +0200, Javier Martinez Canillas wrote:

[...]

>
> The problem is deeper. This is default behaviour of get_maintainer digging
> into the Git history. In the past people were complaining (a lot in some cases)
> that they were included in the threads by a mistake because they made cosmetic
> patches or the treewide change while not being interested _at all_ in looking
> after the certain file / driver. So, with --no-git-fallback it gives nothing,
> expect LKML.
>
>> In my opinion both Thomas and me have much more context and knowledge of
>> the sysfb codebase than the x86 maintainers. It was just for historical
>> reasons that the sysfb code ended in the arch/x86/ sub-directory.
>> 
>> But you are correct that dri-devel at least should also be in the Cc list.
>
> See also above. Depending on the options it may still give bad result w.o.
> explicit mention in the MAINTAINERS (or via glob).
>

Sure, I was not saying that is not worth it.

> ...
>
>> >> >> > +F:	drivers/firmware/sysfb*.c
>> >> >
>> >> >> I would prefer these to be in the "DRM DRIVER FOR FIRMWARE FRAMEBUFFERS"
>> >> >> entry instead of "DRM DRIVERS" since the former is what has most of the
>> >> >> code for the sysfb infrastructure.
>> >> >
>> >> > Then do it, please, fix the above.
>> >> 
>> >> Part of the review process is to give feedback to patch authors. I don't
>> >> understand why you expect me to fix an issue you brought up just because
>> >> I ask you to rework your patch a little.
>> >
>> > In my humble opinion, the author of the patch that makes the problem appear
>> > can help to fix that as well. Are my expectations too high?
>> >
>> > In any case, this was an ad-hoc patch due to the second one, so this one
>> > may be considered as a administrative bug report.
>> 
>> That's OK, but it wasn't framed as a bug report but as a patch and that's why
>> I gave my feedback. But I'll post a patch and add a Reported-by tag from you.
>
> Sure, thanks ahead!
>

Patch sent:

https://lore.kernel.org/dri-devel/20250627113328.2703491-1-javierm@redhat.com/T/#u

>> Thomas, I think we can then only merge patch #2 and I will take care of #1.
>
> I just sent a v2 without the first patch.
>
> Thanks for review!
>

You are welcome.

> -- 
> With Best Regards,
> Andy Shevchenko
>
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat