From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: "Noralf Trønnes" <noralf@tronnes.org>
Cc: devel@driverdev.osuosl.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-fbdev@vger.kernel.org,
Nishad Kamdar <nishadkamdar@gmail.com>,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v1 3/5] fbtft: Drop useless #ifdef CONFIG_OF and dead code
Date: Wed, 20 Nov 2019 17:28:50 +0200 [thread overview]
Message-ID: <20191120152850.GQ32742@smile.fi.intel.com> (raw)
In-Reply-To: <676c3f0e-81b7-d9be-8b37-a38eb3d762fb@tronnes.org>
On Wed, Nov 20, 2019 at 04:04:17PM +0100, Noralf Trønnes wrote:
> Den 20.11.2019 15.43, skrev Noralf Trønnes:
> > Den 20.11.2019 10.57, skrev Andy Shevchenko:
> >> First of all there is no need to guard GPIO request by CONFIG_OF.
> >> It works for everybody independently on resource provider. While here,
> >> rename the function to reflect the above.
> >>
> >> Moreover, since we have a global dependency to OF, the rest of
> >> conditional compilation is no-op, i.e. it's always be true.
> >>
> >> Due to above drop useless #ifdef CONFIG_OF and therefore dead code.
> >>
> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> ---
> >> drivers/staging/fbtft/fbtft-core.c | 19 ++-----------------
> >> 1 file changed, 2 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
> >
> > <snip>
> >
> >> @@ -1184,17 +1176,10 @@ static struct fbtft_platform_data *fbtft_probe_dt(struct device *dev)
> >> pdata->display.backlight = 1;
> >> if (of_find_property(node, "init", NULL))
> >> pdata->display.fbtftops.init_display = fbtft_init_display_dt;
> >> - pdata->display.fbtftops.request_gpios = fbtft_request_gpios_dt;
> >> + pdata->display.fbtftops.request_gpios = fbtft_request_gpios;
> >
> > You can ditch the .request_gpios callback and call fbtft_request_gpios()
> > directly in fbtft_register_framebuffer(). That will make it safe to drop
> > the OF dependency, otherwise .request_gpios will be NULL in the non-DT
> > case. This is one of the bugs that follwed the gpio refactoring.
>
> Really difficult to read this fbtft code (that I wrote...).
> The NULL deref can only happen when dev->platform_data is set. That
> can't happen, in mainline at least, now that fbtft_device is gone.
Hmm... If I read code correctly this patch doesn't change this logic. We have
non-NULL ->request_gpios() in case of pdata != NULL if and only if supplier
gives it to us.
The above assignment happens only for DT case (fbtft_properties_read() is
guarded against non-DT, okay non-fwnode, cases).
> > You can also ditch the .request_gpios_match callback if you want, it
> > isn't called anymore (it is set in fb_agm1264k-fl).
I guess both improvements can be done later since they are not affecting the
logic in this series.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
next prev parent reply other threads:[~2019-11-20 15:29 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-20 9:57 [PATCH v1 1/5] fbtft: Make sure string is NULL terminated Andy Shevchenko
2019-11-20 9:57 ` [PATCH v1 2/5] fbtft: Describe function parameters in kernel-doc Andy Shevchenko
2019-11-20 9:57 ` [PATCH v1 3/5] fbtft: Drop useless #ifdef CONFIG_OF and dead code Andy Shevchenko
2019-11-20 14:43 ` Noralf Trønnes
2019-11-20 15:04 ` Noralf Trønnes
2019-11-20 15:28 ` Andy Shevchenko [this message]
2019-11-20 9:57 ` [PATCH v1 4/5] fbtft: Make use of device property API Andy Shevchenko
2019-11-20 9:57 ` [PATCH v1 5/5] fbtft: Drop OF dependency Andy Shevchenko
2019-11-21 10:18 ` [PATCH v1 1/5] fbtft: Make sure string is NULL terminated Steven Price
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=20191120152850.GQ32742@smile.fi.intel.com \
--to=andriy.shevchenko@linux.intel.com \
--cc=devel@driverdev.osuosl.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=nishadkamdar@gmail.com \
--cc=noralf@tronnes.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).