From: Ingo Molnar <mingo@kernel.org>
To: David Herrmann <dh.herrmann@gmail.com>
Cc: "dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>,
Dave Airlie <airlied@gmail.com>,
Daniel Vetter <daniel.vetter@ffwll.ch>,
Tomi Valkeinen <tomi.valkeinen@ti.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
Tom Gundersen <teg@jklm.no>
Subject: Re: [PATCH 02/11] x86: sysfb: remove sysfb when probing real hw
Date: Fri, 24 Jan 2014 10:16:57 +0000 [thread overview]
Message-ID: <20140124101657.GB5755@gmail.com> (raw)
In-Reply-To: <CANq1E4SyciabqonrFMRa8cMSDxkFuvfqexTBMxKPih+XreF_=A@mail.gmail.com>
* David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Thu, Jan 23, 2014 at 6:14 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * David Herrmann <dh.herrmann@gmail.com> wrote:
> >
> >> >> +#ifdef CONFIG_X86_SYSFB
> >> >> +# include <asm/sysfb.h>
> >> >> +#endif
> >> >
> >> > I guess a single space is sufficient?
> >> >
> >> > Better yet, I'd include sysfb.h unconditionally:
> >>
> >> Unconditionally won't work as only x86 has this header. [...]
> >
> > Well, in non-x86 code an #ifdef x86 looks ugly as well - but I guess
> > better than not building.
> >
> >> [...] If there's a way to place a dummy into asm-generic which is
> >> picked if arch/xy/include/asm/ doesn't have the header, let me know.
> >
> > Not that I know of.
> >
> >> But if I include it unconditionally without any fallback, this will
> >> fail on non-x86. And adding the header to all archs seems overkill.
> >
> > So why not drop the x86-ism and rename it to CONFIG_PLATFORM_SYSFB?
> > Some platforms configure it, some don't. Then the prototypes could
> > move into include/linux/sysfb.h or so and would be platform agnostic.
>
> This is almost exactly what patch #6 does. [...]
Indeed - I never got so far down into the series.
> [...] But it also adds ~400 lines of kernel-doc and ~400 lines of
> Documentation/. Given your remarks, I guess I will just split this
> patch into code and docs, so we can just pick it up for stable in
> case patch #1 does not fix all issues.
I have no objections to this form if it's fixed in a later patch and
this one is easier to backport. I just missed that aspect.
Thanks,
Ingo
WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@kernel.org>
To: David Herrmann <dh.herrmann@gmail.com>
Cc: "dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>,
Dave Airlie <airlied@gmail.com>,
Daniel Vetter <daniel.vetter@ffwll.ch>,
Tomi Valkeinen <tomi.valkeinen@ti.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
Tom Gundersen <teg@jklm.no>
Subject: Re: [PATCH 02/11] x86: sysfb: remove sysfb when probing real hw
Date: Fri, 24 Jan 2014 11:16:57 +0100 [thread overview]
Message-ID: <20140124101657.GB5755@gmail.com> (raw)
In-Reply-To: <CANq1E4SyciabqonrFMRa8cMSDxkFuvfqexTBMxKPih+XreF_=A@mail.gmail.com>
* David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Thu, Jan 23, 2014 at 6:14 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * David Herrmann <dh.herrmann@gmail.com> wrote:
> >
> >> >> +#ifdef CONFIG_X86_SYSFB
> >> >> +# include <asm/sysfb.h>
> >> >> +#endif
> >> >
> >> > I guess a single space is sufficient?
> >> >
> >> > Better yet, I'd include sysfb.h unconditionally:
> >>
> >> Unconditionally won't work as only x86 has this header. [...]
> >
> > Well, in non-x86 code an #ifdef x86 looks ugly as well - but I guess
> > better than not building.
> >
> >> [...] If there's a way to place a dummy into asm-generic which is
> >> picked if arch/xy/include/asm/ doesn't have the header, let me know.
> >
> > Not that I know of.
> >
> >> But if I include it unconditionally without any fallback, this will
> >> fail on non-x86. And adding the header to all archs seems overkill.
> >
> > So why not drop the x86-ism and rename it to CONFIG_PLATFORM_SYSFB?
> > Some platforms configure it, some don't. Then the prototypes could
> > move into include/linux/sysfb.h or so and would be platform agnostic.
>
> This is almost exactly what patch #6 does. [...]
Indeed - I never got so far down into the series.
> [...] But it also adds ~400 lines of kernel-doc and ~400 lines of
> Documentation/. Given your remarks, I guess I will just split this
> patch into code and docs, so we can just pick it up for stable in
> case patch #1 does not fix all issues.
I have no objections to this form if it's fixed in a later patch and
this one is easier to backport. I just missed that aspect.
Thanks,
Ingo
next prev parent reply other threads:[~2014-01-24 10:16 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-23 14:14 [PATCH 00/11] SimpleDRM & Sysfb David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` [PATCH 01/11] x86: sysfb: fool-proof CONFIG_X86_SYSFB David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` [PATCH 02/11] x86: sysfb: remove sysfb when probing real hw David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 16:51 ` Ingo Molnar
2014-01-23 16:51 ` Ingo Molnar
2014-01-23 17:07 ` David Herrmann
2014-01-23 17:07 ` David Herrmann
2014-01-23 17:07 ` David Herrmann
2014-01-23 17:14 ` Ingo Molnar
2014-01-23 17:14 ` Ingo Molnar
2014-01-23 19:09 ` David Herrmann
2014-01-23 19:09 ` David Herrmann
2014-01-23 19:09 ` David Herrmann
2014-01-24 10:16 ` Ingo Molnar [this message]
2014-01-24 10:16 ` Ingo Molnar
2014-01-23 14:14 ` [PATCH 03/11] fbdev: efifb: add dev->remove() callback David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` [PATCH 04/11] fbdev: vesafb: " David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` [PATCH 05/11] x86: sysfb: store apertures in simplefb platform-data David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` [PATCH 06/11] video: sysfb: add generic firmware-fb interface David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:14 ` [PATCH 07/11] drm: mgag200: remove redundant fbdev removal David Herrmann
2014-01-23 14:14 ` David Herrmann
2014-01-23 14:15 ` [PATCH 08/11] drm/i915: remove sysfbs early David Herrmann
2014-01-23 14:15 ` David Herrmann
2014-01-23 14:15 ` [PATCH 09/11] drm: add SimpleDRM driver David Herrmann
2014-01-23 14:15 ` David Herrmann
2014-01-23 14:15 ` [PATCH 10/11] drm: simpledrm: add fbdev fallback support David Herrmann
2014-01-23 14:15 ` David Herrmann
2014-01-23 14:15 ` David Herrmann
2014-01-23 14:15 ` [PATCH 11/11] x86/sysfb: allow sysfb+simpledrm combination David Herrmann
2014-01-23 14:15 ` David Herrmann
2014-01-23 14:15 ` David Herrmann
2014-01-27 22:18 ` [PATCH 00/11] SimpleDRM & Sysfb David Herrmann
2014-01-27 22:18 ` David Herrmann
2014-01-27 22:18 ` David Herrmann
2014-02-21 9:56 ` Thierry Reding
2014-02-21 9:56 ` Thierry Reding
2014-02-21 9:56 ` Thierry Reding
2014-03-03 10:12 ` Tomi Valkeinen
2014-03-03 10:12 ` Tomi Valkeinen
2014-03-03 10:12 ` Tomi Valkeinen
2014-03-03 10:29 ` David Herrmann
2014-03-03 10:29 ` David Herrmann
2014-03-03 10:29 ` David Herrmann
2014-03-03 10:45 ` Tomi Valkeinen
2014-03-03 10:45 ` Tomi Valkeinen
2014-03-03 11:09 ` David Herrmann
2014-03-03 11:09 ` David Herrmann
2014-03-03 11:09 ` David Herrmann
2014-03-03 11:22 ` Tomi Valkeinen
2014-03-03 11:22 ` Tomi Valkeinen
2014-03-06 12:16 ` David Herrmann
2014-03-06 12:16 ` David Herrmann
2014-03-06 12:16 ` David Herrmann
2014-03-07 12:44 ` Tomi Valkeinen
2014-03-07 12:44 ` Tomi Valkeinen
2014-03-07 13:05 ` David Herrmann
2014-03-07 13:05 ` David Herrmann
2014-03-07 13:05 ` David Herrmann
2014-03-07 13:52 ` Tomi Valkeinen
2014-03-07 13:52 ` Tomi Valkeinen
2014-03-07 14:06 ` David Herrmann
2014-03-07 14:06 ` David Herrmann
2014-03-07 14:06 ` David Herrmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140124101657.GB5755@gmail.com \
--to=mingo@kernel.org \
--cc=airlied@gmail.com \
--cc=daniel.vetter@ffwll.ch \
--cc=dh.herrmann@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=teg@jklm.no \
--cc=tomi.valkeinen@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.