* Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset
From: Dan Carpenter @ 2015-03-11 9:23 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Sudip Mukherjee, teddy.wang, Greg KH, devel, linux-fbdev,
linux-kernel
In-Reply-To: <CAA5enKaWynLvLUN+GCdphOOMmqX+cAj3kA3owjKbng66CK=pqg@mail.gmail.com>
On Wed, Mar 11, 2015 at 09:11:52AM +0000, Lorenzo Stoakes wrote:
> On 11 March 2015 at 08:54, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > When I see a patch like this, then I worry, "What if the Sparse
> > annotations are wrong? The patch description doesn't say anything about
> > that." After review then I think the annotations are correct so that's
> > fine.
>
> How do you mean? I was careful to check what sparse was referring to,
> then investigate how memset should be used with pointers with a
> __iomem qualifier. I'd like to be able to improve my patch
> descriptions going forward as best I can :)
>
Yes. The patch is correct. I wasn't asking you to redo it. From later
patches it's actually clear that you know that this change is a bugfix
and a behavior change. But we get a lot of patches where people just
randomly change things to please Sparse and it maybe silences a warning
but it's not correct. I can think of a few recentish examples where
people used standard struct types which hold __iomem or __user pointers
but they used them in non-standard ways so the pointers were actually
normal kernel pointers.
I guess the rule here is that the patch should explain the effect of the
bugfix for the user. Often you won't know the effect, but it's a
helpful thing to think about.
> > Btw, do you have this hardware? Are you able to test these changes?
>
> Unfortunately not, I am trying to keep these changes as simple code
> fixes that ought not to affect actual hardware behaviour as I can
> (though of course you can never be entirely sure that's the case!)
That's fine. I was just wondering. It affects how paranoid I am when I
review the code.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset
From: Lorenzo Stoakes @ 2015-03-11 9:11 UTC (permalink / raw)
To: Dan Carpenter
Cc: Sudip Mukherjee, teddy.wang, Greg KH, devel, linux-fbdev,
linux-kernel
In-Reply-To: <20150311085433.GU10964@mwanda>
On 11 March 2015 at 08:54, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> When I see a patch like this, then I worry, "What if the Sparse
> annotations are wrong? The patch description doesn't say anything about
> that." After review then I think the annotations are correct so that's
> fine.
How do you mean? I was careful to check what sparse was referring to,
then investigate how memset should be used with pointers with a
__iomem qualifier. I'd like to be able to improve my patch
descriptions going forward as best I can :)
> Btw, do you have this hardware? Are you able to test these changes?
Unfortunately not, I am trying to keep these changes as simple code
fixes that ought not to affect actual hardware behaviour as I can
(though of course you can never be entirely sure that's the case!)
I suspect that Sudip must have some real hardware, is this the case
Sudip? If it isn't too presumptuous of me to ask, perhaps you might be
able to check patches that are successfully merged into
staging-testing?
Best,
--
Lorenzo Stoakes
https:/ljs.io
^ permalink raw reply
* Re: [PATCH 6/6] staging: sm750fb: Spinlock and unlock in the same block
From: Dan Carpenter @ 2015-03-11 9:09 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: sudipm.mukherjee, teddy.wang, gregkh, devel, linux-fbdev,
linux-kernel
In-Reply-To: <1426037325-8392-6-git-send-email-lstoakes@gmail.com>
On Wed, Mar 11, 2015 at 01:28:45AM +0000, Lorenzo Stoakes wrote:
> -static inline void myspin_lock(spinlock_t * sl){
> - struct lynx_share * share;
> - share = container_of(sl,struct lynx_share,slock);
> - if(share->dual){
> - spin_lock(sl);
> - }
> -}
Yes, good. We all hate locking wrappers but these are worse than
normal.
> + /* if not use spin_lock,system will die if user load driver
> + * and immediatly unload driver frequently (dual)*/
> + if (share->dual) {
> + spin_lock(&share->slock);
> + share->accel.de_fillrect(&share->accel,
> + base,pitch,Bpp,
> + region->dx,region->dy,
> + region->width,region->height,
> + color,rop);
> + spin_unlock(&share->slock);
> + } else
> + share->accel.de_fillrect(&share->accel,
> + base,pitch,Bpp,
> + region->dx,region->dy,
> + region->width,region->height,
> + color,rop);
> }
No. You've made the code uglier to work around Sparse stupidness. Also
the braces are not according to kernel style.
if (share->dual)
spin_lock(&share->slock);
share->accel.de_fillrect(&share->accel,
base,pitch,Bpp,
region->dx,region->dy,
region->width,region->height,
color,rop);
if (share->dual)
spin_unlock(&share->slock);
Sparse will still complain but no one cares.
regards,
^ permalink raw reply
* Re: [PATCH 3/3] staging: sm7xxfb: change return of sm7xx_vga_setup
From: Sudip Mukherjee @ 2015-03-11 8:59 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Greg Kroah-Hartman, devel, linux-fbdev, linux-kernel
In-Reply-To: <20150309140414.GA11015@mwanda>
On Mon, Mar 09, 2015 at 05:04:14PM +0300, Dan Carpenter wrote:
> On Mon, Mar 09, 2015 at 07:23:43PM +0530, Sudip Mukherjee wrote:
<snip>
> > > Hm... That's a good question.
> > >
> > > I suspect we should just go with fb_get_mode() and do it quickly. This
> > > driver is so new in the kernel that there are no users to get upset.
> >
> > i am afraid that is not correct. this driver was in staging from long
> > back, and Greg removed it from staging as no one was working with it.
> > But after that removal Lemote Yeeloong laptop of Steven Rostedt
> > stopped working. (https://lkml.org/lkml/2014/11/20/698) , so i guess
> > there will be many such users.
>
> Hm... I suspect we should still change it so it matches the other FB
> drivers but I don't know.
>
> Perhaps Greg has an opinion?
maybe, i should make the change and then we can request Steven Rostedt
to test it on his Lemote Yeeloong. if nothing breaks, then alright,
else revert the patch.
regards
sudip
>
> regards,
> dan carpenter
>
^ permalink raw reply
* Re: [PATCH 6/6] staging: sm750fb: correct integer comparison
From: Sudip Mukherjee @ 2015-03-11 8:56 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Greg Kroah-Hartman, devel, linux-fbdev, linux-kernel
In-Reply-To: <20150311075743.GS10964@mwanda>
On Wed, Mar 11, 2015 at 10:57:43AM +0300, Dan Carpenter wrote:
> On Tue, Mar 10, 2015 at 10:46:57PM +0530, Sudip Mukherjee wrote:
> >
> > - while((opt = strsep(&src,":")) != NULL && *opt != NULL){
> > + while((opt = strsep(&src,":")) != NULL && *opt != 0){
>
> Use '\0' for the NUL char.
i got a doubt while making that patch, so i checked lib/string.c and
saw both 0 and '\0' are used.
regards
sudip
>
> regards,
> dan carpenter
^ permalink raw reply
* Re: [PATCH 4/6] staging: sm750fb: Expose hw712_fillrect externally
From: Dan Carpenter @ 2015-03-11 8:56 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: sudipm.mukherjee, teddy.wang, gregkh, devel, linux-fbdev,
linux-kernel
In-Reply-To: <1426037325-8392-4-git-send-email-lstoakes@gmail.com>
On Wed, Mar 11, 2015 at 01:28:43AM +0000, Lorenzo Stoakes wrote:
> This patch adds a reference to hw712_fillrect which is not used elsewhere in the driver,
> but appears to be an alternative to the hw_fillrect method. This patch fixes the following sparse warning:-
>
> drivers/staging/sm750fb/sm750_accel.c:95:5: warning: symbol 'hw712_fillrect' was not declared. Should it be static?
>
Just delete it.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset
From: Dan Carpenter @ 2015-03-11 8:54 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: sudipm.mukherjee, teddy.wang, gregkh, devel, linux-fbdev,
linux-kernel
In-Reply-To: <1426037325-8392-1-git-send-email-lstoakes@gmail.com>
On Wed, Mar 11, 2015 at 01:28:40AM +0000, Lorenzo Stoakes wrote:
> This patch uses memset_io instead of memset when using memset on __iomem
> qualified pointers. This fixes the following sparse warnings:-
>
> drivers/staging/sm750fb/sm750.c:489:17: warning: incorrect type in argument 1 (different address spaces)
> drivers/staging/sm750fb/sm750.c:490:17: warning: incorrect type in argument 1 (different address spaces)
> drivers/staging/sm750fb/sm750.c:501:17: warning: incorrect type in argument 1 (different address spaces)
> drivers/staging/sm750fb/sm750.c:502:17: warning: incorrect type in argument 1 (different address spaces)
> drivers/staging/sm750fb/sm750.c:833:5: warning: incorrect type in argument 1 (different address spaces)
> drivers/staging/sm750fb/sm750.c:1154:9: warning: incorrect type in argument 1 (different address spaces)
>
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
When I see a patch like this, then I worry, "What if the Sparse
annotations are wrong? The patch description doesn't say anything about
that." After review then I think the annotations are correct so that's
fine.
Btw, do you have this hardware? Are you able to test these changes?
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 6/6] staging: sm750fb: correct integer comparison
From: Dan Carpenter @ 2015-03-11 7:57 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: Greg Kroah-Hartman, devel, linux-fbdev, linux-kernel
In-Reply-To: <1426007817-1884-7-git-send-email-sudipm.mukherjee@gmail.com>
On Tue, Mar 10, 2015 at 10:46:57PM +0530, Sudip Mukherjee wrote:
> fixed the build warning about comparison of pointer and integer.
> end of string was being compared to NULL.
>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> drivers/staging/sm750fb/sm750.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index 021b863..5532a28 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -1000,7 +1000,7 @@ static void sm750fb_setup(struct lynx_share * share,char * src)
> goto NO_PARAM;
> }
>
> - while((opt = strsep(&src,":")) != NULL && *opt != NULL){
> + while((opt = strsep(&src,":")) != NULL && *opt != 0){
Use '\0' for the NUL char.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 03/15] fbdev: aty128fb: replace PPC_OF with PPC
From: Tomi Valkeinen @ 2015-03-11 7:51 UTC (permalink / raw)
To: Kevin Hao
Cc: linux-fbdev, linuxppc-dev, Paul Mackerras,
Jean-Christophe Plagniol-Villard
In-Reply-To: <20150311043811.GF28582@pek-khao-d1.corp.ad.wrs.com>
[-- Attachment #1: Type: text/plain, Size: 911 bytes --]
On 11/03/15 06:38, Kevin Hao wrote:
> On Tue, Mar 10, 2015 at 02:23:12PM +0200, Tomi Valkeinen wrote:
>>> I just sent out a v2 [1] a few hours earlier with some minor updates. We plan
>>> to merge this patch series via the powerpc tree in 4.1 cycle if I can collect
>>> all the acks from the corresponding driver maintainers.
>>
>> Fbdev changes look ok to me.
>
> Hi Tomi,
>
> I assume that I can add a "Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>"
> for all the following patches, right?
> http://patchwork.ozlabs.org/patch/443890/
> http://patchwork.ozlabs.org/patch/443891/
> http://patchwork.ozlabs.org/patch/443892/
> http://patchwork.ozlabs.org/patch/443893/
> http://patchwork.ozlabs.org/patch/443894/
> http://patchwork.ozlabs.org/patch/443895/
> http://patchwork.ozlabs.org/patch/443897/
Yes, that's right. Sorry for not being clear on the ack.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/6] staging: sm750fb: correct incompatible pointer type
From: Greg Kroah-Hartman @ 2015-03-11 7:43 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: devel, linux-fbdev, linux-kernel
In-Reply-To: <20150311072842.GA3550@sudip-PC>
On Wed, Mar 11, 2015 at 12:58:42PM +0530, Sudip Mukherjee wrote:
> On Tue, Mar 10, 2015 at 09:17:54PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Mar 10, 2015 at 09:11:00PM +0100, Greg Kroah-Hartman wrote:
> > > On Tue, Mar 10, 2015 at 10:46:55PM +0530, Sudip Mukherjee wrote:
> > > > we were getting build warnings about assignment of incompatible
> <snip>
> > >
> > > That's a nice cleanup, but it's not even the correct cleanup.
> >
> > Oops, sorry, it is the correct cleanup, I was looking at the '*'
> > placement, not the const part.
> >
> > but the patch needs to be redone, I'll just go fix up the build warnings
> > for now...
>
> thanks Greg for your patience and time to redo this patch. I saw you
> broke it into two - three different patches.
> But just one doubt, was your compiler showing any warning for the
> patch - "Staging: sm750fb: provide error path for
> hw_sm750le_setBLANK()" , commit id -
> e74ac550298ec4635cc32e99f966568a808fd370 . my compiler didnot show
> any warning for that.
> mine is gcc 4.7.3, is it time to update?
Yes it is, that's a really old version of gcc. Also please wrap your
email lines at 72 columns :)
> thanks again, and sorry that you had to do fix this. I should have
> made the patches in a more proper way.
Not a problem, it's part of the learning process.
greg k-h
^ permalink raw reply
* Re: [PATCH 4/6] staging: sm750fb: correct incompatible pointer type
From: Sudip Mukherjee @ 2015-03-11 7:40 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: devel, linux-fbdev, linux-kernel
In-Reply-To: <20150310201754.GA8122@kroah.com>
On Tue, Mar 10, 2015 at 09:17:54PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Mar 10, 2015 at 09:11:00PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Mar 10, 2015 at 10:46:55PM +0530, Sudip Mukherjee wrote:
> > > we were getting build warnings about assignment of incompatible
<snip>
> >
> > That's a nice cleanup, but it's not even the correct cleanup.
>
> Oops, sorry, it is the correct cleanup, I was looking at the '*'
> placement, not the const part.
>
> but the patch needs to be redone, I'll just go fix up the build warnings
> for now...
thanks Greg for your patience and time to redo this patch. I saw you broke it into two - three different patches.
But just one doubt, was your compiler showing any warning for the patch - "Staging: sm750fb: provide error path for
hw_sm750le_setBLANK()" , commit id - e74ac550298ec4635cc32e99f966568a808fd370 . my compiler didnot show any warning for that.
mine is gcc 4.7.3, is it time to update?
thanks again, and sorry that you had to do fix this. I should have made the patches in a more proper way.
regards
sudip
>
> greg k-h
^ permalink raw reply
* Re: [PATCH 03/15] fbdev: aty128fb: replace PPC_OF with PPC
From: Kevin Hao @ 2015-03-11 4:38 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-fbdev, linuxppc-dev, Paul Mackerras,
Jean-Christophe Plagniol-Villard
In-Reply-To: <54FEE230.6030006@ti.com>
[-- Attachment #1: Type: text/plain, Size: 792 bytes --]
On Tue, Mar 10, 2015 at 02:23:12PM +0200, Tomi Valkeinen wrote:
> > I just sent out a v2 [1] a few hours earlier with some minor updates. We plan
> > to merge this patch series via the powerpc tree in 4.1 cycle if I can collect
> > all the acks from the corresponding driver maintainers.
>
> Fbdev changes look ok to me.
Hi Tomi,
I assume that I can add a "Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>"
for all the following patches, right?
http://patchwork.ozlabs.org/patch/443890/
http://patchwork.ozlabs.org/patch/443891/
http://patchwork.ozlabs.org/patch/443892/
http://patchwork.ozlabs.org/patch/443893/
http://patchwork.ozlabs.org/patch/443894/
http://patchwork.ozlabs.org/patch/443895/
http://patchwork.ozlabs.org/patch/443897/
Thanks,
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH v2] video: mxsfb: Make sure axi clock is enabled when accessing registers
From: Liu Ying @ 2015-03-11 2:58 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-fbdev, Peter Chen, Jean-Christophe Plagniol-Villard,
Fabio Estevam, Greg Kroah-Hartman, linux-kernel, stable
In-Reply-To: <54FEDD5D.4000104@ti.com>
On Tue, Mar 10, 2015 at 02:02:37PM +0200, Tomi Valkeinen wrote:
> On 04/03/15 09:06, Liu Ying wrote:
> > The LCDIF engines embedded in i.MX6sl and i.MX6sx SoCs need the axi clock
> > as the engine's system clock. The clock should be enabled when accessing
> > LCDIF registers, otherwise the kernel would hang up. We should also keep
> > the clock being enabled when the engine is being active to scan out frames
>
> The text above is a bit confusing. Maybe just "... also keep the clock
> enabled when..."
Okay.
>
> > from memory. This patch makes sure the axi clock is enabled when accessing
> > registers so that the kernel hang up issue can be fixed.
> >
> > Reported-by: Peter Chen <peter.chen@freescale.com>
> > Tested-by: Peter Chen <peter.chen@freescale.com>
> > Cc: <stable@vger.kernel.org> # 3.19+
> > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> > ---
> > v1->v2:
> > * Add 'Tested-by: Peter Chen <peter.chen@freescale.com>' tag.
> > * Add 'Cc: <stable@vger.kernel.org> # 3.19+' tag.
> >
> > drivers/video/fbdev/mxsfb.c | 70 ++++++++++++++++++++++++++++++++++++---------
> > 1 file changed, 56 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
> > index f8ac4a4..a8cf3b2 100644
> > --- a/drivers/video/fbdev/mxsfb.c
> > +++ b/drivers/video/fbdev/mxsfb.c
> > @@ -316,6 +316,18 @@ static int mxsfb_check_var(struct fb_var_screeninfo *var,
> > return 0;
> > }
> >
> > +static inline void mxsfb_enable_axi_clk(struct mxsfb_info *host)
> > +{
> > + if (host->clk_axi)
> > + clk_prepare_enable(host->clk_axi);
> > +}
> > +
> > +static inline void mxsfb_disable_axi_clk(struct mxsfb_info *host)
> > +{
> > + if (host->clk_axi)
> > + clk_disable_unprepare(host->clk_axi);
> > +}
> > +
> > static void mxsfb_enable_controller(struct fb_info *fb_info)
> > {
> > struct mxsfb_info *host = to_imxfb_host(fb_info);
> > @@ -333,14 +345,13 @@ static void mxsfb_enable_controller(struct fb_info *fb_info)
> > }
> > }
> >
> > - if (host->clk_axi)
> > - clk_prepare_enable(host->clk_axi);
> > -
> > if (host->clk_disp_axi)
> > clk_prepare_enable(host->clk_disp_axi);
> > clk_prepare_enable(host->clk);
> > clk_set_rate(host->clk, PICOS2KHZ(fb_info->var.pixclock) * 1000U);
> >
> > + mxsfb_enable_axi_clk(host);
> > +
>
> Is there some reason to move the clk enable to a different place here?
Moving it to here reflects better that we need to enable it when accessing the
registers.
Another reason is weak, perhaps. We've got an unannounced new SoC(not yet get
upstreamed). It has a LCDIF embedded. The pixel clock(host->clk) and the axi
clock are derived from a same clock gate. And, the clock gate is defined with
the flag CLK_SET_RATE_GATE, which means it must be gated across rate change.
So, we need to move it beneath clk_set_rate() for the pixel clock sooner or
later.
>
> > /* if it was disabled, re-enable the mode again */
> > writel(CTRL_DOTCLK_MODE, host->base + LCDC_CTRL + REG_SET);
> >
> > @@ -380,11 +391,11 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
> > reg = readl(host->base + LCDC_VDCTRL4);
> > writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
> >
> > + mxsfb_disable_axi_clk(host);
> > +
> > clk_disable_unprepare(host->clk);
> > if (host->clk_disp_axi)
> > clk_disable_unprepare(host->clk_disp_axi);
> > - if (host->clk_axi)
> > - clk_disable_unprepare(host->clk_axi);
>
> And same here for disable.
This is to make sure the clock disable order is exactly reversed, comparing to
the clock enable order.
>
> > host->enabled = 0;
> >
> > @@ -421,6 +432,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
> > mxsfb_disable_controller(fb_info);
> > }
> >
> > + mxsfb_enable_axi_clk(host);
> > +
> > /* clear the FIFOs */
> > writel(CTRL1_FIFO_CLEAR, host->base + LCDC_CTRL1 + REG_SET);
> >
> > @@ -438,6 +451,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
> > ctrl |= CTRL_SET_WORD_LENGTH(3);
> > switch (host->ld_intf_width) {
> > case STMLCDIF_8BIT:
> > + mxsfb_disable_axi_clk(host);
> > dev_err(&host->pdev->dev,
> > "Unsupported LCD bus width mapping\n");
> > return -EINVAL;
> > @@ -451,6 +465,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
> > writel(CTRL1_SET_BYTE_PACKAGING(0x7), host->base + LCDC_CTRL1);
> > break;
> > default:
> > + mxsfb_disable_axi_clk(host);
> > dev_err(&host->pdev->dev, "Unhandled color depth of %u\n",
> > fb_info->var.bits_per_pixel);
> > return -EINVAL;
> > @@ -504,6 +519,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
> > fb_info->fix.line_length * fb_info->var.yoffset,
> > host->base + host->devdata->next_buf);
> >
> > + mxsfb_disable_axi_clk(host);
> > +
> > if (reenable)
> > mxsfb_enable_controller(fb_info);
> >
> > @@ -582,10 +599,16 @@ static int mxsfb_pan_display(struct fb_var_screeninfo *var,
> >
> > offset = fb_info->fix.line_length * var->yoffset;
> >
> > + if (!host->enabled)
> > + mxsfb_enable_axi_clk(host);
> > +
> > /* update on next VSYNC */
> > writel(fb_info->fix.smem_start + offset,
> > host->base + host->devdata->next_buf);
> >
> > + if (!host->enabled)
> > + mxsfb_disable_axi_clk(host);
> > +
>
> Why do you check for host->enabled here, but not elsewhere?
We need this check here to make sure the axi clock reference count is no greater
than 1. Looking at the context of mxsfb_set_par(), mxsfb_restore_mode() and
mxsfb_probe() closely, the check is not necessary. Regarding mxsfb_shutdown(),
since the system is shutting down, I assume the reference count is not that
important.
Regards,
Liu Ying
>
> Tomi
>
>
^ permalink raw reply
* [PATCH 6/6] staging: sm750fb: Spinlock and unlock in the same block
From: Lorenzo Stoakes @ 2015-03-11 1:28 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh
Cc: linux-fbdev, devel, linux-kernel, Lorenzo Stoakes
In-Reply-To: <1426037325-8392-1-git-send-email-lstoakes@gmail.com>
This patch combines spinlock locks and unlocks together in the same block rather
than occurring in separate blocks preventing a possible deadlock. This fixes the
following sparse warnings:-
drivers/staging/sm750fb/sm750.c:218:22: warning: context imbalance in 'lynxfb_ops_fillrect' - different lock contexts for basic block
drivers/staging/sm750fb/sm750.c:241:22: warning: context imbalance in 'lynxfb_ops_copyarea' - different lock contexts for basic block
drivers/staging/sm750fb/sm750.c:282:22: warning: context imbalance in 'lynxfb_ops_imageblit' - different lock contexts for basic block
Unfortunately this change involves code (and comment) duplication.
Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 76 +++++++++++++++++++++++------------------
1 file changed, 43 insertions(+), 33 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 3e36b6a..58c7518 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -56,23 +56,6 @@ static char * g_settings = NULL;
static int g_dualview = 0;
static char * g_option = NULL;
-/* if not use spin_lock,system will die if user load driver
- * and immediatly unload driver frequently (dual)*/
-static inline void myspin_lock(spinlock_t * sl){
- struct lynx_share * share;
- share = container_of(sl,struct lynx_share,slock);
- if(share->dual){
- spin_lock(sl);
- }
-}
-
-static inline void myspin_unlock(spinlock_t * sl){
- struct lynx_share * share;
- share = container_of(sl,struct lynx_share,slock);
- if(share->dual){
- spin_unlock(sl);
- }
-}
static const struct fb_videomode lynx750_ext[] = {
/* 1024x600-60 VESA [1.71:1] */
{NULL, 60, 1024, 600, 20423, 144, 40, 18, 1, 104, 3,
@@ -209,13 +192,22 @@ static void lynxfb_ops_fillrect(struct fb_info* info,const struct fb_fillrect* r
color = (Bpp = 1)?region->color:((u32*)info->pseudo_palette)[region->color];
rop = ( region->rop != ROP_COPY ) ? HW_ROP2_XOR:HW_ROP2_COPY;
- myspin_lock(&share->slock);
- share->accel.de_fillrect(&share->accel,
- base,pitch,Bpp,
- region->dx,region->dy,
- region->width,region->height,
- color,rop);
- myspin_unlock(&share->slock);
+ /* if not use spin_lock,system will die if user load driver
+ * and immediatly unload driver frequently (dual)*/
+ if (share->dual) {
+ spin_lock(&share->slock);
+ share->accel.de_fillrect(&share->accel,
+ base,pitch,Bpp,
+ region->dx,region->dy,
+ region->width,region->height,
+ color,rop);
+ spin_unlock(&share->slock);
+ } else
+ share->accel.de_fillrect(&share->accel,
+ base,pitch,Bpp,
+ region->dx,region->dy,
+ region->width,region->height,
+ color,rop);
}
static void lynxfb_ops_copyarea(struct fb_info * info,const struct fb_copyarea * region)
@@ -233,12 +225,20 @@ static void lynxfb_ops_copyarea(struct fb_info * info,const struct fb_copyarea *
pitch = info->fix.line_length;
Bpp = info->var.bits_per_pixel >> 3;
- myspin_lock(&share->slock);
- share->accel.de_copyarea(&share->accel,
- base,pitch,region->sx,region->sy,
- base,pitch,Bpp,region->dx,region->dy,
- region->width,region->height,HW_ROP2_COPY);
- myspin_unlock(&share->slock);
+ /* if not use spin_lock,system will die if user load driver
+ * and immediatly unload driver frequently (dual)*/
+ if (share->dual) {
+ spin_lock(&share->slock);
+ share->accel.de_copyarea(&share->accel,
+ base,pitch,region->sx,region->sy,
+ base,pitch,Bpp,region->dx,region->dy,
+ region->width,region->height,HW_ROP2_COPY);
+ spin_unlock(&share->slock);
+ } else
+ share->accel.de_copyarea(&share->accel,
+ base,pitch,region->sx,region->sy,
+ base,pitch,Bpp,region->dx,region->dy,
+ region->width,region->height,HW_ROP2_COPY);
}
static void lynxfb_ops_imageblit(struct fb_info*info,const struct fb_image* image)
@@ -272,14 +272,24 @@ static void lynxfb_ops_imageblit(struct fb_info*info,const struct fb_image* imag
}
return;
_do_work:
- myspin_lock(&share->slock);
- share->accel.de_imageblit(&share->accel,
+ /* if not use spin_lock,system will die if user load driver
+ * and immediatly unload driver frequently (dual)*/
+ if (share->dual) {
+ spin_lock(&share->slock);
+ share->accel.de_imageblit(&share->accel,
+ image->data,image->width>>3,0,
+ base,pitch,Bpp,
+ image->dx,image->dy,
+ image->width,image->height,
+ fgcol,bgcol,HW_ROP2_COPY);
+ spin_unlock(&share->slock);
+ } else
+ share->accel.de_imageblit(&share->accel,
image->data,image->width>>3,0,
base,pitch,Bpp,
image->dx,image->dy,
image->width,image->height,
fgcol,bgcol,HW_ROP2_COPY);
- myspin_unlock(&share->slock);
}
static int lynxfb_ops_pan_display(struct fb_var_screeninfo *var,
--
2.3.2
^ permalink raw reply related
* [PATCH 5/6] staging: sm750fb: Fix __iomem pointer types
From: Lorenzo Stoakes @ 2015-03-11 1:28 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh
Cc: linux-fbdev, devel, linux-kernel, Lorenzo Stoakes
In-Reply-To: <1426037325-8392-1-git-send-email-lstoakes@gmail.com>
This patch annotates pointers as referring to I/O mapped memory where they ought
to be, removes now unnecessary ugly casts, eliminates an incorrect deref on I/O
mapped memory by using iowrite16 instead, and updates the pointer arithmetic
accordingly to take into account that the pointers are now byte-sized. This
fixes the following sparse warnings:-
drivers/staging/sm750fb/sm750_cursor.c:113:19: warning: cast removes address space of expression
drivers/staging/sm750fb/sm750_cursor.c:204:19: warning: cast removes address space of expression
Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
drivers/staging/sm750fb/sm750_cursor.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750_cursor.c b/drivers/staging/sm750fb/sm750_cursor.c
index 6cceef1..c2ff3bd 100644
--- a/drivers/staging/sm750fb/sm750_cursor.c
+++ b/drivers/staging/sm750fb/sm750_cursor.c
@@ -98,7 +98,7 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
int i,j,count,pitch,offset;
u8 color,mask,opr;
u16 data;
- u16 * pbuffer,*pstart;
+ void __iomem * pbuffer,*pstart;
/* in byte*/
pitch = cursor->w >> 3;
@@ -106,11 +106,11 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
/* in byte */
count = pitch * cursor->h;
- /* in ushort */
- offset = cursor->maxW * 2 / 8 / 2;
+ /* in byte */
+ offset = cursor->maxW * 2 / 8;
data = 0;
- pstart = (u16 *)cursor->vstart;
+ pstart = cursor->vstart;
pbuffer = pstart;
/*
@@ -161,7 +161,7 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
}
}
#endif
- *pbuffer = data;
+ iowrite16(data, pbuffer);
/* assume pitch is 1,2,4,8,...*/
#if 0
@@ -174,7 +174,7 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
pstart += offset;
pbuffer = pstart;
}else{
- pbuffer++;
+ pbuffer += sizeof(u16);
}
}
@@ -189,7 +189,7 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
int i,j,count,pitch,offset;
u8 color, mask;
u16 data;
- u16 * pbuffer,*pstart;
+ void __iomem * pbuffer,*pstart;
/* in byte*/
pitch = cursor->w >> 3;
@@ -197,11 +197,11 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
/* in byte */
count = pitch * cursor->h;
- /* in ushort */
- offset = cursor->maxW * 2 / 8 / 2;
+ /* in byte */
+ offset = cursor->maxW * 2 / 8;
data = 0;
- pstart = (u16 *)cursor->vstart;
+ pstart = cursor->vstart;
pbuffer = pstart;
for(i=0;i<count;i++)
@@ -234,7 +234,7 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
data |= ((color & (1<<j))?1:2)<<(j*2);
}
#endif
- *pbuffer = data;
+ iowrite16(data, pbuffer);
/* assume pitch is 1,2,4,8,...*/
if(!(i&(pitch-1)))
@@ -244,7 +244,7 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
pstart += offset;
pbuffer = pstart;
}else{
- pbuffer++;
+ pbuffer += sizeof(u16);
}
}
--
2.3.2
^ permalink raw reply related
* [PATCH 4/6] staging: sm750fb: Expose hw712_fillrect externally
From: Lorenzo Stoakes @ 2015-03-11 1:28 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh
Cc: linux-fbdev, devel, linux-kernel, Lorenzo Stoakes
In-Reply-To: <1426037325-8392-1-git-send-email-lstoakes@gmail.com>
This patch adds a reference to hw712_fillrect which is not used elsewhere in the driver,
but appears to be an alternative to the hw_fillrect method. This patch fixes the following sparse warning:-
drivers/staging/sm750fb/sm750_accel.c:95:5: warning: symbol 'hw712_fillrect' was not declared. Should it be static?
Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
drivers/staging/sm750fb/sm750_accel.c | 2 +-
drivers/staging/sm750fb/sm750_accel.h | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index 4aa763b..6521c3b 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -92,7 +92,7 @@ void hw_set2dformat(struct lynx_accel * accel,int fmt)
/* seems sm712 RectFill command is broken,so need use BitBlt to
* replace it. */
-static int hw712_fillrect(struct lynx_accel * accel,
+int hw712_fillrect(struct lynx_accel * accel,
u32 base,u32 pitch,u32 Bpp,
u32 x,u32 y,u32 width,u32 height,
u32 color,u32 rop)
diff --git a/drivers/staging/sm750fb/sm750_accel.h b/drivers/staging/sm750fb/sm750_accel.h
index 3ee0bd8..51a9367 100644
--- a/drivers/staging/sm750fb/sm750_accel.h
+++ b/drivers/staging/sm750fb/sm750_accel.h
@@ -238,11 +238,16 @@ void hw_set2dformat(struct lynx_accel * accel,int fmt);
void hw_de_init(struct lynx_accel * accel);
+int hw712_fillrect(struct lynx_accel * accel,
+ u32 base,u32 pitch,u32 Bpp,
+ u32 x,u32 y,u32 width,u32 height,
+ u32 color,u32 rop);
+
int hw_fillrect(struct lynx_accel * accel,
u32 base,u32 pitch,u32 Bpp,
u32 x,u32 y,u32 width,u32 height,
u32 color,u32 rop);
int hw_copyarea(
struct lynx_accel * accel,
unsigned int sBase, /* Address of source: offset in frame buffer */
--
2.3.2
^ permalink raw reply related
* [PATCH 3/6] staging: sm750fb: Make internal functions static
From: Lorenzo Stoakes @ 2015-03-11 1:28 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh
Cc: linux-fbdev, devel, linux-kernel, Lorenzo Stoakes
In-Reply-To: <1426037325-8392-1-git-send-email-lstoakes@gmail.com>
This patch declares externally unavailable functions static. This fixes the
following sparse warnings:-
drivers/staging/sm750fb/ddk750_swi2c.c:223:6: warning: symbol 'swI2CStart' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:234:6: warning: symbol 'swI2CStop' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:252:6: warning: symbol 'swI2CWriteByte' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:320:15: warning: symbol 'swI2CReadByte' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:361:6: warning: symbol 'swI2CInit_SM750LE' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_hwi2c.c:63:6: warning: symbol 'hwI2CWaitTXDone' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_hwi2c.c:93:14: warning: symbol 'hwI2CWriteData' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_hwi2c.c:160:14: warning: symbol 'hwI2CReadData' was not declared. Should it be static?
Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
drivers/staging/sm750fb/ddk750_hwi2c.c | 6 +++---
drivers/staging/sm750fb/ddk750_swi2c.c | 10 +++++-----
drivers/staging/sm750fb/sm750_accel.c | 2 +-
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_hwi2c.c b/drivers/staging/sm750fb/ddk750_hwi2c.c
index 84dfb6f..7826376 100644
--- a/drivers/staging/sm750fb/ddk750_hwi2c.c
+++ b/drivers/staging/sm750fb/ddk750_hwi2c.c
@@ -60,7 +60,7 @@ void hwI2CClose(void)
}
-long hwI2CWaitTXDone(void)
+static long hwI2CWaitTXDone(void)
{
unsigned int timeout;
@@ -90,7 +90,7 @@ long hwI2CWaitTXDone(void)
* Return Value:
* Total number of bytes those are actually written.
*/
-unsigned int hwI2CWriteData(
+static unsigned int hwI2CWriteData(
unsigned char deviceAddress,
unsigned int length,
unsigned char *pBuffer
@@ -157,7 +157,7 @@ unsigned int hwI2CWriteData(
* Return Value:
* Total number of actual bytes read from the slave device
*/
-unsigned int hwI2CReadData(
+static unsigned int hwI2CReadData(
unsigned char deviceAddress,
unsigned int length,
unsigned char *pBuffer
diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
index 1249759..516f5bb 100644
--- a/drivers/staging/sm750fb/ddk750_swi2c.c
+++ b/drivers/staging/sm750fb/ddk750_swi2c.c
@@ -220,7 +220,7 @@ static void swI2CAck(void)
/*
* This function sends the start command to the slave device
*/
-void swI2CStart(void)
+static void swI2CStart(void)
{
/* Start I2C */
swI2CSDA(1);
@@ -231,7 +231,7 @@ void swI2CStart(void)
/*
* This function sends the stop command to the slave device
*/
-void swI2CStop(void)
+static void swI2CStop(void)
{
/* Stop the I2C */
swI2CSCL(1);
@@ -249,7 +249,7 @@ void swI2CStop(void)
* 0 - Success
* -1 - Fail to write byte
*/
-long swI2CWriteByte(unsigned char data)
+static long swI2CWriteByte(unsigned char data)
{
unsigned char value = data;
int i;
@@ -317,7 +317,7 @@ long swI2CWriteByte(unsigned char data)
* Return Value:
* One byte data read from the Slave device
*/
-unsigned char swI2CReadByte(unsigned char ack)
+static unsigned char swI2CReadByte(unsigned char ack)
{
int i;
unsigned char data = 0;
@@ -358,7 +358,7 @@ unsigned char swI2CReadByte(unsigned char ack)
* -1 - Fail to initialize the i2c
* 0 - Success
*/
-long swI2CInit_SM750LE(
+static long swI2CInit_SM750LE(
unsigned char i2cClkGPIO,
unsigned char i2cDataGPIO
)
diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index 6521c3b..4aa763b 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -92,7 +92,7 @@ void hw_set2dformat(struct lynx_accel * accel,int fmt)
/* seems sm712 RectFill command is broken,so need use BitBlt to
* replace it. */
-int hw712_fillrect(struct lynx_accel * accel,
+static int hw712_fillrect(struct lynx_accel * accel,
u32 base,u32 pitch,u32 Bpp,
u32 x,u32 y,u32 width,u32 height,
u32 color,u32 rop)
--
2.3.2
^ permalink raw reply related
* [PATCH 2/6] staging: sm750fb: Fix non-ANSI function declarations
From: Lorenzo Stoakes @ 2015-03-11 1:28 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh
Cc: linux-fbdev, devel, linux-kernel, Lorenzo Stoakes
In-Reply-To: <1426037325-8392-1-git-send-email-lstoakes@gmail.com>
Fixes Function declarations which expect no parameters to have a parameter list consisting of void. This fixes the following sparse warnings:-
drivers/staging/sm750fb/sm750_hw.c:584:23: warning: non-ANSI function declaration of function 'hw_sm750le_deWait'
drivers/staging/sm750fb/sm750_hw.c:601:21: warning: non-ANSI function declaration of function 'hw_sm750_deWait'
9,13d7
drivers/staging/sm750fb/ddk750_chip.c:14:33: warning: non-ANSI function declaration of function 'getChipType'
drivers/staging/sm750fb/ddk750_chip.c:94:27: warning: non-ANSI function declaration of function 'getChipClock'
drivers/staging/sm750fb/ddk750_chip.c:235:31: warning: non-ANSI function declaration of function 'ddk750_getVMSize'
drivers/staging/sm750fb/ddk750_power.c:18:27: warning: non-ANSI function declaration of function 'getPowerMode'
drivers/staging/sm750fb/ddk750_display.c:276:24: warning: non-ANSI function declaration of function 'ddk750_initDVIDisp'
19,22d12
drivers/staging/sm750fb/ddk750_sii164.c:37:34: warning: non-ANSI function declaration of function 'sii164GetVendorID'
drivers/staging/sm750fb/ddk750_sii164.c:54:34: warning: non-ANSI function declaration of function 'sii164GetDeviceID'
drivers/staging/sm750fb/ddk750_dvi.c:65:31: warning: non-ANSI function declaration of function 'dviGetVendorID'
drivers/staging/sm750fb/ddk750_dvi.c:85:31: warning: non-ANSI function declaration of function 'dviGetDeviceID'
Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
drivers/staging/sm750fb/ddk750_chip.c | 6 +++---
drivers/staging/sm750fb/ddk750_display.c | 2 +-
drivers/staging/sm750fb/ddk750_dvi.c | 4 ++--
drivers/staging/sm750fb/ddk750_power.c | 2 +-
drivers/staging/sm750fb/ddk750_sii164.c | 4 ++--
drivers/staging/sm750fb/sm750_hw.c | 4 ++--
6 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index b71169e..3c77207 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -11,7 +11,7 @@ typedef struct _pllcalparam{
pllcalparam;
-logical_chip_type_t getChipType()
+logical_chip_type_t getChipType(void)
{
unsigned short physicalID;
char physicalRev;
@@ -91,7 +91,7 @@ unsigned int getPllValue(clock_type_t clockType, pll_value_t *pPLL)
}
-unsigned int getChipClock()
+unsigned int getChipClock(void)
{
pll_value_t pll;
#if 1
@@ -232,7 +232,7 @@ void setMasterClock(unsigned int frequency)
}
-unsigned int ddk750_getVMSize()
+unsigned int ddk750_getVMSize(void)
{
unsigned int reg;
unsigned int data;
diff --git a/drivers/staging/sm750fb/ddk750_display.c b/drivers/staging/sm750fb/ddk750_display.c
index a282a94..c84196a 100644
--- a/drivers/staging/sm750fb/ddk750_display.c
+++ b/drivers/staging/sm750fb/ddk750_display.c
@@ -273,7 +273,7 @@ void ddk750_setLogicalDispOut(disp_output_t output)
}
-int ddk750_initDVIDisp()
+int ddk750_initDVIDisp(void)
{
/* Initialize DVI. If the dviInit fail and the VendorID or the DeviceID are
not zeroed, then set the failure flag. If it is zeroe, it might mean
diff --git a/drivers/staging/sm750fb/ddk750_dvi.c b/drivers/staging/sm750fb/ddk750_dvi.c
index 1c083e7..f5932bb 100644
--- a/drivers/staging/sm750fb/ddk750_dvi.c
+++ b/drivers/staging/sm750fb/ddk750_dvi.c
@@ -62,7 +62,7 @@ int dviInit(
* Output:
* Vendor ID
*/
-unsigned short dviGetVendorID()
+unsigned short dviGetVendorID(void)
{
dvi_ctrl_device_t *pCurrentDviCtrl;
@@ -82,7 +82,7 @@ unsigned short dviGetVendorID()
* Output:
* Device ID
*/
-unsigned short dviGetDeviceID()
+unsigned short dviGetDeviceID(void)
{
dvi_ctrl_device_t *pCurrentDviCtrl;
diff --git a/drivers/staging/sm750fb/ddk750_power.c b/drivers/staging/sm750fb/ddk750_power.c
index 98dfcbd..cbb9767 100644
--- a/drivers/staging/sm750fb/ddk750_power.c
+++ b/drivers/staging/sm750fb/ddk750_power.c
@@ -15,7 +15,7 @@ void ddk750_setDPMS(DPMS_t state)
}
}
-unsigned int getPowerMode()
+unsigned int getPowerMode(void)
{
if(getChipType() = SM750LE)
return 0;
diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index faf8250..bdd7742 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -34,7 +34,7 @@ static char *gDviCtrlChipName = "Silicon Image SiI 164";
* Output:
* Vendor ID
*/
-unsigned short sii164GetVendorID()
+unsigned short sii164GetVendorID(void)
{
unsigned short vendorID;
@@ -51,7 +51,7 @@ unsigned short sii164GetVendorID()
* Output:
* Device ID
*/
-unsigned short sii164GetDeviceID()
+unsigned short sii164GetDeviceID(void)
{
unsigned short deviceID;
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index c44a50b..3050847 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -581,7 +581,7 @@ void hw_sm750_initAccel(struct lynx_share * share)
share->accel.de_init(&share->accel);
}
-int hw_sm750le_deWait()
+int hw_sm750le_deWait(void)
{
int i=0x10000000;
while(i--){
@@ -598,7 +598,7 @@ int hw_sm750le_deWait()
}
-int hw_sm750_deWait()
+int hw_sm750_deWait(void)
{
int i=0x10000000;
while(i--){
--
2.3.2
^ permalink raw reply related
* [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset
From: Lorenzo Stoakes @ 2015-03-11 1:28 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh
Cc: linux-fbdev, devel, linux-kernel, Lorenzo Stoakes
This patch uses memset_io instead of memset when using memset on __iomem
qualified pointers. This fixes the following sparse warnings:-
drivers/staging/sm750fb/sm750.c:489:17: warning: incorrect type in argument 1 (different address spaces)
drivers/staging/sm750fb/sm750.c:490:17: warning: incorrect type in argument 1 (different address spaces)
drivers/staging/sm750fb/sm750.c:501:17: warning: incorrect type in argument 1 (different address spaces)
drivers/staging/sm750fb/sm750.c:502:17: warning: incorrect type in argument 1 (different address spaces)
drivers/staging/sm750fb/sm750.c:833:5: warning: incorrect type in argument 1 (different address spaces)
drivers/staging/sm750fb/sm750.c:1154:9: warning: incorrect type in argument 1 (different address spaces)
Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index aa0888c..3e36b6a 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -486,8 +486,8 @@ static int lynxfb_resume(struct pci_dev* pdev)
par = info->par;
crtc = &par->crtc;
cursor = &crtc->cursor;
- memset(cursor->vstart, 0x0, cursor->size);
- memset(crtc->vScreen,0x0,crtc->vidmem_size);
+ memset_io(cursor->vstart, 0x0, cursor->size);
+ memset_io(crtc->vScreen,0x0,crtc->vidmem_size);
lynxfb_ops_set_par(info);
fb_set_suspend(info, 0);
}
@@ -498,8 +498,8 @@ static int lynxfb_resume(struct pci_dev* pdev)
par = info->par;
crtc = &par->crtc;
cursor = &crtc->cursor;
- memset(cursor->vstart, 0x0, cursor->size);
- memset(crtc->vScreen,0x0,crtc->vidmem_size);
+ memset_io(cursor->vstart, 0x0, cursor->size);
+ memset_io(crtc->vScreen,0x0,crtc->vidmem_size);
lynxfb_ops_set_par(info);
fb_set_suspend(info, 0);
}
@@ -830,7 +830,7 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
crtc->cursor.share = share;
- memset(crtc->cursor.vstart, 0, crtc->cursor.size);
+ memset_io(crtc->cursor.vstart, 0, crtc->cursor.size);
if(!g_hwcursor){
lynxfb_ops.fb_cursor = NULL;
crtc->cursor.disable(&crtc->cursor);
@@ -1151,7 +1151,7 @@ static int lynxfb_pci_probe(struct pci_dev * pdev,
}
#endif
- memset(share->pvMem,0,share->vidmem_size);
+ memset_io(share->pvMem,0,share->vidmem_size);
pr_info("sm%3x mmio address = %p\n",share->devid,share->pvReg);
--
2.3.2
^ permalink raw reply related
* Fwd: [PATCH] fbdev: udl: fix oops on USB unplug due to missing get_vblank_counter function
From: Adam Richter @ 2015-03-10 22:11 UTC (permalink / raw)
To: linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 1861 bytes --]
[This is a third submission of a message that vger first bounched from
my Yahoo account, and then
from gmail when I failed to disable HTML formatting. I apologize if
it is a duplicate or triplicate. -Adam]
Hello, Bernie.
In Linux-4.0-rc3, the DisplayLink driver gets a kernel memory fault when its
USB device is unplugged, specifically when vblank_disable_and_save in
linux-4.0-rc3/drivers/gpu/drm/drm_irq.c attempts to call the NULL function
pointer dev->driver_get_vblank_counter.
linux-4.0-rc3/Documentation/DocBook/drm.tmpl says of that field, "Otherwise
drivers can use the drm_vblank_count helper function to handle this
operation." Trying that seems to have eliminated the oops when I unplug the
device, although I do see these lines in the console log at device driver
initialization time that I think are probably fine, but which I should
mention in case the indicate that I did the wrong the thing:
[drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[drm] No driver support for vblank timestamp query.
If that doesn't look like a problem to you, then I suggest you apply the
patch, appended below and also included as an attachment, and push the
change upstream. The patch just adds one line that appears in six other
framebuffer drivers to initialize the driver_get_vblank_counter drm_driver
function.
Thank you for maintaining the USB DisplayLink driver, and thanks for your
attention to this.
Adam Richter
--- drivers/gpu/drm/udl/udl_drv.c.orig 2015-03-09 22:10:14.292335527 -0700
+++ drivers/gpu/drm/udl/udl_drv.c 2015-03-09 22:22:14.148342842 -0700
@@ -41,6 +41,7 @@ static struct drm_driver driver = {
.load = udl_driver_load,
.unload = udl_driver_unload,
.set_busid = udl_driver_set_busid,
+ .get_vblank_counter = drm_vblank_count,
/* gem hooks */
.gem_free_object = udl_gem_free_object,
[-- Attachment #2: displaylink.diff --]
[-- Type: text/plain, Size: 398 bytes --]
--- drivers/gpu/drm/udl/udl_drv.c.orig 2015-03-09 22:10:14.292335527 -0700
+++ drivers/gpu/drm/udl/udl_drv.c 2015-03-09 22:22:14.148342842 -0700
@@ -41,6 +41,7 @@ static struct drm_driver driver = {
.load = udl_driver_load,
.unload = udl_driver_unload,
.set_busid = udl_driver_set_busid,
+ .get_vblank_counter = drm_vblank_count,
/* gem hooks */
.gem_free_object = udl_gem_free_object,
^ permalink raw reply
* Re: [PATCH 4/6] staging: sm750fb: correct incompatible pointer type
From: Greg Kroah-Hartman @ 2015-03-10 20:17 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: devel, linux-fbdev, linux-kernel
In-Reply-To: <20150310201100.GA17629@kroah.com>
On Tue, Mar 10, 2015 at 09:11:00PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Mar 10, 2015 at 10:46:55PM +0530, Sudip Mukherjee wrote:
> > we were getting build warnings about assignment of incompatible
> > pointer types. some of the function definitions were having wrong
> > return type or arguments.
> >
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
> > drivers/staging/sm750fb/sm750.h | 11 ++++++-----
> > drivers/staging/sm750fb/sm750_accel.c | 4 ++--
> > drivers/staging/sm750fb/sm750_accel.h | 4 ++--
> > 3 files changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> > index d39968c..edb955a 100644
> > --- a/drivers/staging/sm750fb/sm750.h
> > +++ b/drivers/staging/sm750fb/sm750.h
> > @@ -23,7 +23,7 @@ struct lynx_accel{
> > volatile unsigned char __iomem * dpPortBase;
> >
> > /* function fointers */
> > - int (*de_init)(struct lynx_accel *);
> > + void (*de_init)(struct lynx_accel *);
>
> That's ok, but:
>
> >
> > int (*de_wait)(void);/* see if hardware ready to work */
> >
> > @@ -34,8 +34,8 @@ struct lynx_accel{
> > u32,u32,u32,u32,
> > u32,u32,u32,u32);
> >
> > - int (*de_imageblit)(struct lynx_accel *,const char *,u32,u32,u32,
> > - u32,u32,u32,u32,u32,u32,u32,u32,u32);
> > + int (*de_imageblit)(struct lynx_accel *, const char *, u32, u32, u32,
> > + u32, u32, u32, u32, u32, u32, u32, u32, u32);
> >
>
>
> This isn't fixing an error.
>
> > };
> >
> > @@ -120,8 +120,9 @@ struct lynxfb_crtc{
> > int(*proc_setColReg)(struct lynxfb_crtc*,ushort,ushort,ushort,ushort);
> > void (*clear)(struct lynxfb_crtc*);
> > /* pan display */
> > - int(*proc_panDisplay)(struct lynxfb_crtc*, struct fb_var_screeninfo*,
> > - struct fb_info*);
> > + int (*proc_panDisplay)(struct lynxfb_crtc*,
> > + const struct fb_var_screeninfo*,
> > + const struct fb_info*);
>
> That's a nice cleanup, but it's not even the correct cleanup.
Oops, sorry, it is the correct cleanup, I was looking at the '*'
placement, not the const part.
but the patch needs to be redone, I'll just go fix up the build warnings
for now...
greg k-h
^ permalink raw reply
* Re: [PATCH 0/6] staging: sm750fb: fixed all build warnings
From: Greg Kroah-Hartman @ 2015-03-10 20:13 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: devel, linux-fbdev, linux-kernel
In-Reply-To: <1426007817-1884-1-git-send-email-sudipm.mukherjee@gmail.com>
On Tue, Mar 10, 2015 at 10:46:51PM +0530, Sudip Mukherjee wrote:
> Hi Greg,
> all the build warnings have been taken care of in this series.
> some of the patches will generate a few checkpatch warning and for some I tried
> to address few of the chcekpatch warnings while modifying them.
Don't mix what you do in a single patch please.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 4/6] staging: sm750fb: correct incompatible pointer type
From: Greg Kroah-Hartman @ 2015-03-10 20:11 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: devel, linux-fbdev, linux-kernel
In-Reply-To: <1426007817-1884-5-git-send-email-sudipm.mukherjee@gmail.com>
On Tue, Mar 10, 2015 at 10:46:55PM +0530, Sudip Mukherjee wrote:
> we were getting build warnings about assignment of incompatible
> pointer types. some of the function definitions were having wrong
> return type or arguments.
>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> drivers/staging/sm750fb/sm750.h | 11 ++++++-----
> drivers/staging/sm750fb/sm750_accel.c | 4 ++--
> drivers/staging/sm750fb/sm750_accel.h | 4 ++--
> 3 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> index d39968c..edb955a 100644
> --- a/drivers/staging/sm750fb/sm750.h
> +++ b/drivers/staging/sm750fb/sm750.h
> @@ -23,7 +23,7 @@ struct lynx_accel{
> volatile unsigned char __iomem * dpPortBase;
>
> /* function fointers */
> - int (*de_init)(struct lynx_accel *);
> + void (*de_init)(struct lynx_accel *);
That's ok, but:
>
> int (*de_wait)(void);/* see if hardware ready to work */
>
> @@ -34,8 +34,8 @@ struct lynx_accel{
> u32,u32,u32,u32,
> u32,u32,u32,u32);
>
> - int (*de_imageblit)(struct lynx_accel *,const char *,u32,u32,u32,
> - u32,u32,u32,u32,u32,u32,u32,u32,u32);
> + int (*de_imageblit)(struct lynx_accel *, const char *, u32, u32, u32,
> + u32, u32, u32, u32, u32, u32, u32, u32, u32);
>
This isn't fixing an error.
> };
>
> @@ -120,8 +120,9 @@ struct lynxfb_crtc{
> int(*proc_setColReg)(struct lynxfb_crtc*,ushort,ushort,ushort,ushort);
> void (*clear)(struct lynxfb_crtc*);
> /* pan display */
> - int(*proc_panDisplay)(struct lynxfb_crtc*, struct fb_var_screeninfo*,
> - struct fb_info*);
> + int (*proc_panDisplay)(struct lynxfb_crtc*,
> + const struct fb_var_screeninfo*,
> + const struct fb_info*);
That's a nice cleanup, but it's not even the correct cleanup.
Please just fix the specific warning, don't mix what you are doing in
one patch. You aren't documenting this in the changelog information, so
I have to reject it, sorry.
greg k-h
^ permalink raw reply
* [PATCH 6/6] staging: sm750fb: correct integer comparison
From: Sudip Mukherjee @ 2015-03-10 17:28 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-fbdev, devel, linux-kernel, Sudip Mukherjee
In-Reply-To: <1426007817-1884-1-git-send-email-sudipm.mukherjee@gmail.com>
fixed the build warning about comparison of pointer and integer.
end of string was being compared to NULL.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/staging/sm750fb/sm750.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 021b863..5532a28 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -1000,7 +1000,7 @@ static void sm750fb_setup(struct lynx_share * share,char * src)
goto NO_PARAM;
}
- while((opt = strsep(&src,":")) != NULL && *opt != NULL){
+ while((opt = strsep(&src,":")) != NULL && *opt != 0){
pr_err("opt=%s\n",opt);
pr_err("src=%s\n",src);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 5/6] staging: sm750fb: fix mixed declarations
From: Sudip Mukherjee @ 2015-03-10 17:28 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-fbdev, devel, linux-kernel, Sudip Mukherjee
In-Reply-To: <1426007817-1884-1-git-send-email-sudipm.mukherjee@gmail.com>
we were getting build warning about mixed declaration. the variable
is now declared at the beginning of the block.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/staging/sm750fb/sm750.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 87029b6..021b863 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -1175,6 +1175,7 @@ ALLOC_FB:
else
{
struct lynxfb_par * par;
+ int errno;
pr_info("framebuffer #%d alloc okay\n",fbidx);
share->fbinfo[fbidx] = info[fbidx];
par = info[fbidx]->par;
@@ -1191,7 +1192,7 @@ ALLOC_FB:
/* register frame buffer*/
pr_info("Ready to register framebuffer #%d.\n",fbidx);
- int errno = register_framebuffer(info[fbidx]);
+ errno = register_framebuffer(info[fbidx]);
if (errno < 0) {
pr_err("Failed to register fb_info #%d. err %d\n",fbidx, errno);
if(fbidx = 0)
--
1.8.1.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox