* Re: [PATCH] ocfb: remove incorrect __iomem annotation
From: Tomi Valkeinen @ 2015-09-24 10:54 UTC (permalink / raw)
To: linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 300 bytes --]
On 17/08/15 11:10, Christoph Hellwig wrote:
> Make sparse happy:
Well, I think someone will anyway be unhappy:
- memset_io() is used to clear fb_virt.
- fb_virt is assigned to fbdev->info.screen_base.
I don't know if it's better or worse to have the __iomem in ocfb.c...
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 28/38] video/omap: remove invalid check
From: Tomi Valkeinen @ 2015-09-24 10:48 UTC (permalink / raw)
To: Andrzej Hajda, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski,
Jean-Christophe Plagniol-Villard, linux-fbdev, linux-omap
In-Reply-To: <1442842450-29769-29-git-send-email-a.hajda@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]
On 21/09/15 16:34, Andrzej Hajda wrote:
> regno is unsigned so it cannot be negative.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> drivers/video/fbdev/omap/omapfb_main.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/video/fbdev/omap/omapfb_main.c b/drivers/video/fbdev/omap/omapfb_main.c
> index 1fb3ea3..393ae1b 100644
> --- a/drivers/video/fbdev/omap/omapfb_main.c
> +++ b/drivers/video/fbdev/omap/omapfb_main.c
> @@ -276,11 +276,6 @@ static int _setcolreg(struct fb_info *info, u_int regno, u_int red, u_int green,
> if (r != 0)
> break;
>
> - if (regno < 0) {
> - r = -EINVAL;
> - break;
> - }
> -
> if (regno < 16) {
> u16 pal;
> pal = ((red >> (16 - var->red.length)) <<
>
Thanks, queued for 4.4.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: fbdev: mb862xx: Fix module autoload for OF platform driver
From: Tomi Valkeinen @ 2015-09-24 10:18 UTC (permalink / raw)
To: Luis de Bethencourt, linux-kernel
Cc: Jean-Christophe Plagniol-Villard, Wolfram Sang, linux-fbdev
In-Reply-To: <20150918184609.GA32729@goodgumbo.baconseed.org>
[-- Attachment #1: Type: text/plain, Size: 1253 bytes --]
On 18/09/15 21:46, Luis de Bethencourt wrote:
> This platform driver has a OF device ID table but the OF module
> alias information is not created so module autoloading won't work.
>
> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> ---
>
> Hello,
>
> This patch adds the missing MODULE_DEVICE_TABLE() for OF to export
> that information so modules have the correct aliases built-in and
> autoloading works correctly.
>
> A longer explanation by Javier Canillas can be found here:
> https://lkml.org/lkml/2015/7/30/519
>
> Thanks,
> Luis
>
> drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> index 9b8bebd..f9ec5c0 100644
> --- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> +++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> @@ -831,6 +831,7 @@ static struct of_device_id of_platform_mb862xx_tbl[] = {
> { .compatible = "fujitsu,coral", },
> { /* end */ }
> };
> +MODULE_DEVICE_TABLE(of, of_platform_mb862xx_tbl);
>
> static struct platform_driver of_platform_mb862xxfb_driver = {
> .driver = {
>
Thanks, queued for 4.3 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] omapdss: acx565akm: Fix module autoload for OF platform driver
From: Tomi Valkeinen @ 2015-09-24 10:17 UTC (permalink / raw)
To: Luis de Bethencourt, linux-kernel
Cc: Jean-Christophe Plagniol-Villard, linux-omap, linux-fbdev
In-Reply-To: <20150918184457.GA32636@goodgumbo.baconseed.org>
[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]
On 18/09/15 21:44, Luis de Bethencourt wrote:
> This platform driver has a OF device ID table but the OF module
> alias information is not created so module autoloading won't work.
>
> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> ---
>
> Hello,
>
> This patch adds the missing MODULE_DEVICE_TABLE() for OF to export
> that information so modules have the correct aliases built-in and
> autoloading works correctly.
>
> A longer explanation by Javier Canillas can be found here:
> https://lkml.org/lkml/2015/7/30/519
>
> Thanks,
> Luis
>
> drivers/video/fbdev/omap2/displays-new/panel-sony-acx565akm.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/video/fbdev/omap2/displays-new/panel-sony-acx565akm.c b/drivers/video/fbdev/omap2/displays-new/panel-sony-acx565akm.c
> index 90cbc4c..c581231 100644
> --- a/drivers/video/fbdev/omap2/displays-new/panel-sony-acx565akm.c
> +++ b/drivers/video/fbdev/omap2/displays-new/panel-sony-acx565akm.c
> @@ -898,6 +898,7 @@ static const struct of_device_id acx565akm_of_match[] = {
> { .compatible = "omapdss,sony,acx565akm", },
> {},
> };
> +MODULE_DEVICE_TABLE(of, acx565akm_of_match);
>
> static struct spi_driver acx565akm_driver = {
> .driver = {
>
Thanks, queued for 4.3 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: broadsheetfb: fix memory leak
From: Tomi Valkeinen @ 2015-09-24 10:12 UTC (permalink / raw)
To: Sudip Mukherjee, Jean-Christophe Plagniol-Villard
Cc: linux-kernel, linux-fbdev
In-Reply-To: <1442574992-31654-1-git-send-email-sudipm.mukherjee@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]
On 18/09/15 14:16, Sudip Mukherjee wrote:
> On the error path we have missed releasing the firmware.
>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> drivers/video/fbdev/broadsheetfb.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/broadsheetfb.c b/drivers/video/fbdev/broadsheetfb.c
> index 0e5fde1..ac1f41e 100644
> --- a/drivers/video/fbdev/broadsheetfb.c
> +++ b/drivers/video/fbdev/broadsheetfb.c
> @@ -752,7 +752,7 @@ static ssize_t broadsheet_loadstore_waveform(struct device *dev,
> if ((fw_entry->size < 8*1024) || (fw_entry->size > 64*1024)) {
> dev_err(dev, "Invalid waveform\n");
> err = -EINVAL;
> - goto err_failed;
> + goto err_fw;
> }
>
> mutex_lock(&(par->io_lock));
> @@ -762,13 +762,15 @@ static ssize_t broadsheet_loadstore_waveform(struct device *dev,
> mutex_unlock(&(par->io_lock));
> if (err < 0) {
> dev_err(dev, "Failed to store broadsheet waveform\n");
> - goto err_failed;
> + goto err_fw;
> }
>
> dev_info(dev, "Stored broadsheet waveform, size %zd\n", fw_entry->size);
>
> return len;
>
> +err_fw:
> + release_firmware(fw_entry);
> err_failed:
> return err;
> }
This looks like correct fix, but where is the firmware released when
there is no error?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2] video: fbdev: fsl: Fix the sleep function for FSL DIU module
From: Tomi Valkeinen @ 2015-09-24 10:07 UTC (permalink / raw)
To: Wang Dongsheng
Cc: Scott Wood, Jin Jason, linuxppc-dev@lists.ozlabs.org,
linux-fbdev@vger.kernel.org, Timur Tabi
In-Reply-To: <SN1PR0301MB1616D30A83E8A5C6FEE6A44A9D430@SN1PR0301MB1616.namprd03.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 443 bytes --]
On 24/09/15 13:02, Wang Dongsheng wrote:
> Hi Tomi,
>
> Could you apply this patch?
>
>>> For deep sleep, the diu module will power off, when wake up from the
>>> deep sleep, the registers need to be reinitialized.
>>>
>>> Signed-off-by: Jason Jin<Jason.Jin@freescale.com>
>>> Signed-off-by: Wang Dongsheng<dongsheng.wang@freescale.com>
>>
>> Acked-by: Timur Tabi <timur@tabi.org>
Thanks, queued for 4.3 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH v2] video: fbdev: fsl: Fix the sleep function for FSL DIU module
From: Wang Dongsheng @ 2015-09-24 10:02 UTC (permalink / raw)
To: tomi.valkeinen@ti.com
Cc: Scott Wood, Jin Jason, linuxppc-dev@lists.ozlabs.org,
linux-fbdev@vger.kernel.org, Timur Tabi
In-Reply-To: <55CEB5CB.4060803@tabi.org>
Hi Tomi,
Could you apply this patch?
> > For deep sleep, the diu module will power off, when wake up from the
> > deep sleep, the registers need to be reinitialized.
> >
> > Signed-off-by: Jason Jin<Jason.Jin@freescale.com>
> > Signed-off-by: Wang Dongsheng<dongsheng.wang@freescale.com>
>
> Acked-by: Timur Tabi <timur@tabi.org>
Regards,
-Dongsheng
^ permalink raw reply
* [PATCH] radeonfb: Deinline large functions
From: Denys Vlasenko @ 2015-09-23 17:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Denys Vlasenko, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Richard Purdie, Chris Wilson, David Airlie, Alex Deucher,
Ben Skeggs, Zhang Rui, Len Brown, Jesse Barnes, Dave Airlie,
linux-kernel, linux-fbdev
With this .config: http://busybox.net/~vda/kernel_config,
after uninlining these functions have sizes and callsite counts
as follows:
__OUTPLLP: 61 bytes, 12 callsites
__INPLL: 79 bytes, 150 callsites
__OUTPLL: 82 bytes, 138 callsites
_OUTREGP: 101 bytes, 8 callsites
_radeon_msleep: 66 bytes, 18 callsites
_radeon_fifo_wait: 83 bytes, 24 callsites
_radeon_engine_idle: 92 bytes, 10 callsites
radeon_engine_flush: 105 bytes, 2 callsites
radeon_pll_errata_after_index_slow: 31 bytes, 11 callsites
radeon_pll_errata_after_data_slow: 91 bytes, 9 callsites
radeon_pll_errata_after_FOO functions are split into two parts:
the inlined part which checks corresponding rinfo->errata bit,
and out-of-line part which performs workaround magic per se.
Reduction in code size is about 49,500 bytes:
text data bss dec hex filename
85789648 22294616 20627456 128711720 7abfc28 vmlinux.before
85740176 22294680 20627456 128662312 7ab3b28 vmlinux
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: David Airlie <airlied@linux.ie>
Cc: Alex Deucher <alexdeucher@gmail.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Dave Airlie <airlied@redhat.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
---
Resending. Ben, please let me know if you aren't the right person for this.
drivers/video/fbdev/aty/radeon_base.c | 133 ++++++++++++++++++++++++++++++-
drivers/video/fbdev/aty/radeonfb.h | 144 ++++------------------------------
2 files changed, 146 insertions(+), 131 deletions(-)
diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index 26d80a4..bd9b6ca 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -282,9 +282,138 @@ static int backlight = 1;
static int backlight = 0;
#endif
-/*
- * prototypes
+/* Note about this function: we have some rare cases where we must not schedule,
+ * this typically happen with our special "wake up early" hook which allows us to
+ * wake up the graphic chip (and thus get the console back) before everything else
+ * on some machines that support that mechanism. At this point, interrupts are off
+ * and scheduling is not permitted
*/
+void _radeon_msleep(struct radeonfb_info *rinfo, unsigned long ms)
+{
+ if (rinfo->no_schedule || oops_in_progress)
+ mdelay(ms);
+ else
+ msleep(ms);
+}
+
+void radeon_pll_errata_after_index_slow(struct radeonfb_info *rinfo)
+{
+ /* Called if (rinfo->errata & CHIP_ERRATA_PLL_DUMMYREADS) is set */
+ (void)INREG(CLOCK_CNTL_DATA);
+ (void)INREG(CRTC_GEN_CNTL);
+}
+
+void radeon_pll_errata_after_data_slow(struct radeonfb_info *rinfo)
+{
+ if (rinfo->errata & CHIP_ERRATA_PLL_DELAY) {
+ /* we can't deal with posted writes here ... */
+ _radeon_msleep(rinfo, 5);
+ }
+ if (rinfo->errata & CHIP_ERRATA_R300_CG) {
+ u32 save, tmp;
+ save = INREG(CLOCK_CNTL_INDEX);
+ tmp = save & ~(0x3f | PLL_WR_EN);
+ OUTREG(CLOCK_CNTL_INDEX, tmp);
+ tmp = INREG(CLOCK_CNTL_DATA);
+ OUTREG(CLOCK_CNTL_INDEX, save);
+ }
+}
+
+void _OUTREGP(struct radeonfb_info *rinfo, u32 addr, u32 val, u32 mask)
+{
+ unsigned long flags;
+ unsigned int tmp;
+
+ spin_lock_irqsave(&rinfo->reg_lock, flags);
+ tmp = INREG(addr);
+ tmp &= (mask);
+ tmp |= (val);
+ OUTREG(addr, tmp);
+ spin_unlock_irqrestore(&rinfo->reg_lock, flags);
+}
+
+u32 __INPLL(struct radeonfb_info *rinfo, u32 addr)
+{
+ u32 data;
+
+ OUTREG8(CLOCK_CNTL_INDEX, addr & 0x0000003f);
+ radeon_pll_errata_after_index(rinfo);
+ data = INREG(CLOCK_CNTL_DATA);
+ radeon_pll_errata_after_data(rinfo);
+ return data;
+}
+
+void __OUTPLL(struct radeonfb_info *rinfo, unsigned int index, u32 val)
+{
+ OUTREG8(CLOCK_CNTL_INDEX, (index & 0x0000003f) | 0x00000080);
+ radeon_pll_errata_after_index(rinfo);
+ OUTREG(CLOCK_CNTL_DATA, val);
+ radeon_pll_errata_after_data(rinfo);
+}
+
+void __OUTPLLP(struct radeonfb_info *rinfo, unsigned int index,
+ u32 val, u32 mask)
+{
+ unsigned int tmp;
+
+ tmp = __INPLL(rinfo, index);
+ tmp &= (mask);
+ tmp |= (val);
+ __OUTPLL(rinfo, index, tmp);
+}
+
+void _radeon_fifo_wait(struct radeonfb_info *rinfo, int entries)
+{
+ int i;
+
+ for (i=0; i<2000000; i++) {
+ if ((INREG(RBBM_STATUS) & 0x7f) >= entries)
+ return;
+ udelay(1);
+ }
+ printk(KERN_ERR "radeonfb: FIFO Timeout !\n");
+}
+
+void radeon_engine_flush(struct radeonfb_info *rinfo)
+{
+ int i;
+
+ /* Initiate flush */
+ OUTREGP(DSTCACHE_CTLSTAT, RB2D_DC_FLUSH_ALL,
+ ~RB2D_DC_FLUSH_ALL);
+
+ /* Ensure FIFO is empty, ie, make sure the flush commands
+ * has reached the cache
+ */
+ _radeon_fifo_wait(rinfo, 64);
+
+ /* Wait for the flush to complete */
+ for (i=0; i < 2000000; i++) {
+ if (!(INREG(DSTCACHE_CTLSTAT) & RB2D_DC_BUSY))
+ return;
+ udelay(1);
+ }
+ printk(KERN_ERR "radeonfb: Flush Timeout !\n");
+}
+
+void _radeon_engine_idle(struct radeonfb_info *rinfo)
+{
+ int i;
+
+ /* ensure FIFO is empty before waiting for idle */
+ _radeon_fifo_wait(rinfo, 64);
+
+ for (i=0; i<2000000; i++) {
+ if (((INREG(RBBM_STATUS) & GUI_ACTIVE)) = 0) {
+ radeon_engine_flush(rinfo);
+ return;
+ }
+ udelay(1);
+ }
+ printk(KERN_ERR "radeonfb: Idle Timeout !\n");
+}
+
+
static void radeon_unmap_ROM(struct radeonfb_info *rinfo, struct pci_dev *dev)
{
diff --git a/drivers/video/fbdev/aty/radeonfb.h b/drivers/video/fbdev/aty/radeonfb.h
index cb84604..85b80cd 100644
--- a/drivers/video/fbdev/aty/radeonfb.h
+++ b/drivers/video/fbdev/aty/radeonfb.h
@@ -370,20 +370,7 @@ struct radeonfb_info {
* IO macros
*/
-/* Note about this function: we have some rare cases where we must not schedule,
- * this typically happen with our special "wake up early" hook which allows us to
- * wake up the graphic chip (and thus get the console back) before everything else
- * on some machines that support that mechanism. At this point, interrupts are off
- * and scheduling is not permitted
- */
-static inline void _radeon_msleep(struct radeonfb_info *rinfo, unsigned long ms)
-{
- if (rinfo->no_schedule || oops_in_progress)
- mdelay(ms);
- else
- msleep(ms);
-}
-
+void _radeon_msleep(struct radeonfb_info *rinfo, unsigned long ms);
#define INREG8(addr) readb((rinfo->mmio_base)+addr)
#define OUTREG8(addr,val) writeb(val, (rinfo->mmio_base)+addr)
@@ -392,19 +379,7 @@ static inline void _radeon_msleep(struct radeonfb_info *rinfo, unsigned long ms)
#define INREG(addr) readl((rinfo->mmio_base)+addr)
#define OUTREG(addr,val) writel(val, (rinfo->mmio_base)+addr)
-static inline void _OUTREGP(struct radeonfb_info *rinfo, u32 addr,
- u32 val, u32 mask)
-{
- unsigned long flags;
- unsigned int tmp;
-
- spin_lock_irqsave(&rinfo->reg_lock, flags);
- tmp = INREG(addr);
- tmp &= (mask);
- tmp |= (val);
- OUTREG(addr, tmp);
- spin_unlock_irqrestore(&rinfo->reg_lock, flags);
-}
+void _OUTREGP(struct radeonfb_info *rinfo, u32 addr, u32 val, u32 mask);
#define OUTREGP(addr,val,mask) _OUTREGP(rinfo, addr, val,mask)
@@ -425,64 +400,24 @@ static inline void _OUTREGP(struct radeonfb_info *rinfo, u32 addr,
* possible exception to this rule is the call to unblank(), which may
* be done at irq time if an oops is in progress.
*/
+void radeon_pll_errata_after_index_slow(struct radeonfb_info *rinfo);
static inline void radeon_pll_errata_after_index(struct radeonfb_info *rinfo)
{
- if (!(rinfo->errata & CHIP_ERRATA_PLL_DUMMYREADS))
- return;
-
- (void)INREG(CLOCK_CNTL_DATA);
- (void)INREG(CRTC_GEN_CNTL);
+ if (rinfo->errata & CHIP_ERRATA_PLL_DUMMYREADS)
+ radeon_pll_errata_after_index_slow(rinfo);
}
+void radeon_pll_errata_after_data_slow(struct radeonfb_info *rinfo);
static inline void radeon_pll_errata_after_data(struct radeonfb_info *rinfo)
{
- if (rinfo->errata & CHIP_ERRATA_PLL_DELAY) {
- /* we can't deal with posted writes here ... */
- _radeon_msleep(rinfo, 5);
- }
- if (rinfo->errata & CHIP_ERRATA_R300_CG) {
- u32 save, tmp;
- save = INREG(CLOCK_CNTL_INDEX);
- tmp = save & ~(0x3f | PLL_WR_EN);
- OUTREG(CLOCK_CNTL_INDEX, tmp);
- tmp = INREG(CLOCK_CNTL_DATA);
- OUTREG(CLOCK_CNTL_INDEX, save);
- }
-}
-
-static inline u32 __INPLL(struct radeonfb_info *rinfo, u32 addr)
-{
- u32 data;
-
- OUTREG8(CLOCK_CNTL_INDEX, addr & 0x0000003f);
- radeon_pll_errata_after_index(rinfo);
- data = INREG(CLOCK_CNTL_DATA);
- radeon_pll_errata_after_data(rinfo);
- return data;
-}
-
-static inline void __OUTPLL(struct radeonfb_info *rinfo, unsigned int index,
- u32 val)
-{
-
- OUTREG8(CLOCK_CNTL_INDEX, (index & 0x0000003f) | 0x00000080);
- radeon_pll_errata_after_index(rinfo);
- OUTREG(CLOCK_CNTL_DATA, val);
- radeon_pll_errata_after_data(rinfo);
-}
-
-
-static inline void __OUTPLLP(struct radeonfb_info *rinfo, unsigned int index,
- u32 val, u32 mask)
-{
- unsigned int tmp;
-
- tmp = __INPLL(rinfo, index);
- tmp &= (mask);
- tmp |= (val);
- __OUTPLL(rinfo, index, tmp);
+ if (rinfo->errata & (CHIP_ERRATA_PLL_DELAY|CHIP_ERRATA_R300_CG))
+ radeon_pll_errata_after_data_slow(rinfo);
}
+u32 __INPLL(struct radeonfb_info *rinfo, u32 addr);
+void __OUTPLL(struct radeonfb_info *rinfo, unsigned int index, u32 val);
+void __OUTPLLP(struct radeonfb_info *rinfo, unsigned int index,
+ u32 val, u32 mask);
#define INPLL(addr) __INPLL(rinfo, addr)
#define OUTPLL(index, val) __OUTPLL(rinfo, index, val)
@@ -532,58 +467,9 @@ static inline u32 radeon_get_dstbpp(u16 depth)
* 2D Engine helper routines
*/
-static inline void _radeon_fifo_wait(struct radeonfb_info *rinfo, int entries)
-{
- int i;
-
- for (i=0; i<2000000; i++) {
- if ((INREG(RBBM_STATUS) & 0x7f) >= entries)
- return;
- udelay(1);
- }
- printk(KERN_ERR "radeonfb: FIFO Timeout !\n");
-}
-
-static inline void radeon_engine_flush (struct radeonfb_info *rinfo)
-{
- int i;
-
- /* Initiate flush */
- OUTREGP(DSTCACHE_CTLSTAT, RB2D_DC_FLUSH_ALL,
- ~RB2D_DC_FLUSH_ALL);
-
- /* Ensure FIFO is empty, ie, make sure the flush commands
- * has reached the cache
- */
- _radeon_fifo_wait (rinfo, 64);
-
- /* Wait for the flush to complete */
- for (i=0; i < 2000000; i++) {
- if (!(INREG(DSTCACHE_CTLSTAT) & RB2D_DC_BUSY))
- return;
- udelay(1);
- }
- printk(KERN_ERR "radeonfb: Flush Timeout !\n");
-}
-
-
-static inline void _radeon_engine_idle(struct radeonfb_info *rinfo)
-{
- int i;
-
- /* ensure FIFO is empty before waiting for idle */
- _radeon_fifo_wait (rinfo, 64);
-
- for (i=0; i<2000000; i++) {
- if (((INREG(RBBM_STATUS) & GUI_ACTIVE)) = 0) {
- radeon_engine_flush (rinfo);
- return;
- }
- udelay(1);
- }
- printk(KERN_ERR "radeonfb: Idle Timeout !\n");
-}
-
+void _radeon_fifo_wait(struct radeonfb_info *rinfo, int entries);
+void radeon_engine_flush(struct radeonfb_info *rinfo);
+void _radeon_engine_idle(struct radeonfb_info *rinfo);
#define radeon_engine_idle() _radeon_engine_idle(rinfo)
#define radeon_fifo_wait(entries) _radeon_fifo_wait(rinfo,entries)
--
1.8.1.4
^ permalink raw reply related
* Re: [PATCH v3 04/12] backlight: pwm_bl: remove useless call to pwm_set_period
From: Lee Jones @ 2015-09-22 22:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1442828009-6241-5-git-send-email-boris.brezillon@free-electrons.com>
On Mon, 21 Sep 2015, Boris Brezillon wrote:
> The PWM period will be set when calling pwm_config. Remove this useless
> call to pwm_set_period, which might mess up with the initial PWM state
> once we have added proper support for PWM init state retrieval.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> drivers/video/backlight/pwm_bl.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
Acked-by: Lee Jones <lee.jones@linaro.org>
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index ae498c1..71944f8 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -293,12 +293,14 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> * period, parsed from the DT, in the PWM device. For the non-DT case,
> * set the period from platform data if it has not already been set
> * via the PWM lookup table.
> + * FIXME: This assignment should be dropped as soon as all the boards
> + * have moved to the PWM lookup table approach. The same goes for the
> + * pb->period field which should be replaced by
> + * pwm_get_default_period() calls.
> */
> pb->period = pwm_get_default_period(pb->pwm);
> - if (!pb->period && (data->pwm_period_ns > 0)) {
> + if (!pb->period && (data->pwm_period_ns > 0))
> pb->period = data->pwm_period_ns;
> - pwm_set_period(pb->pwm, data->pwm_period_ns);
> - }
>
> pb->lth_brightness = data->lth_brightness * (pb->period / pb->scale);
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v3 01/12] pwm: introduce default period and polarity concepts
From: Lee Jones @ 2015-09-22 21:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1442828009-6241-2-git-send-email-boris.brezillon@free-electrons.com>
On Mon, 21 Sep 2015, Boris Brezillon wrote:
> When requested by a user, the PWM is assigned a default period and polarity
> extracted from the DT, the platform data or statically set by the driver.
> Those default values are currently stored in the period and polarity
> fields of the pwm_device struct, but they will be stored somewhere else
> once we have introduced the architecture allowing for hardware state
> retrieval.
>
> The pwm_set_default_polarity and pwm_set_default_period should only be
> used by PWM drivers or the PWM core infrastructure to specify the
> default period and polarity values.
>
> PWM users might call the pwm_get_default_period to query the default
> period value. There is currently no helper to query the default
> polarity, but it might be added later on if there is a need for it.
>
> This patch also modifies all the places where the default helpers should
> be used in place of the standard ones.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> drivers/leds/leds-pwm.c | 2 +-
> drivers/pwm/core.c | 14 +++++++-------
> drivers/pwm/pwm-pxa.c | 2 +-
> drivers/pwm/pwm-sun4i.c | 3 ++-
> drivers/regulator/pwm-regulator.c | 4 ++--
> drivers/video/backlight/lm3630a_bl.c | 4 ++--
> drivers/video/backlight/pwm_bl.c | 2 +-
Acked-by: Lee Jones <lee.jones@linaro.org>
> drivers/video/fbdev/ssd1307fb.c | 2 +-
> include/linux/pwm.h | 17 +++++++++++++++++
> 9 files changed, 34 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
> index 1d07e3e..2c564d1 100644
> --- a/drivers/leds/leds-pwm.c
> +++ b/drivers/leds/leds-pwm.c
> @@ -125,7 +125,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> if (led_data->can_sleep)
> INIT_WORK(&led_data->work, led_pwm_work);
>
> - led_data->period = pwm_get_period(led_data->pwm);
> + led_data->period = pwm_get_default_period(led_data->pwm);
> if (!led_data->period && (led->pwm_period_ns > 0))
> led_data->period = led->pwm_period_ns;
>
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 3f9df3e..732375d 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -146,12 +146,12 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
> if (IS_ERR(pwm))
> return pwm;
>
> - pwm_set_period(pwm, args->args[1]);
> + pwm_set_default_period(pwm, args->args[1]);
>
> if (args->args[2] & PWM_POLARITY_INVERTED)
> - pwm_set_polarity(pwm, PWM_POLARITY_INVERSED);
> + pwm_set_default_polarity(pwm, PWM_POLARITY_INVERSED);
> else
> - pwm_set_polarity(pwm, PWM_POLARITY_NORMAL);
> + pwm_set_default_polarity(pwm, PWM_POLARITY_NORMAL);
>
> return pwm;
> }
> @@ -172,7 +172,7 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
> if (IS_ERR(pwm))
> return pwm;
>
> - pwm_set_period(pwm, args->args[1]);
> + pwm_set_default_period(pwm, args->args[1]);
>
> return pwm;
> }
> @@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip,
> pwm->chip = chip;
> pwm->pwm = chip->base + i;
> pwm->hwpwm = i;
> - pwm->polarity = polarity;
> + pwm_set_default_polarity(pwm, polarity);
>
> radix_tree_insert(&pwm_tree, pwm->pwm, pwm);
> }
> @@ -730,8 +730,8 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
> if (IS_ERR(pwm))
> goto out;
>
> - pwm_set_period(pwm, chosen->period);
> - pwm_set_polarity(pwm, chosen->polarity);
> + pwm_set_default_period(pwm, chosen->period);
> + pwm_set_default_polarity(pwm, chosen->polarity);
>
> out:
> mutex_unlock(&pwm_lookup_lock);
> diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
> index cb2f702..65b80aa 100644
> --- a/drivers/pwm/pwm-pxa.c
> +++ b/drivers/pwm/pwm-pxa.c
> @@ -160,7 +160,7 @@ pxa_pwm_of_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
> if (IS_ERR(pwm))
> return pwm;
>
> - pwm_set_period(pwm, args->args[0]);
> + pwm_set_default_period(pwm, args->args[0]);
>
> return pwm;
> }
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index cd9dde5..a364fb7 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -333,7 +333,8 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
> val = sun4i_pwm_readl(pwm, PWM_CTRL_REG);
> for (i = 0; i < pwm->chip.npwm; i++)
> if (!(val & BIT_CH(PWM_ACT_STATE, i)))
> - pwm->chip.pwms[i].polarity = PWM_POLARITY_INVERSED;
> + pwm_set_default_polarity(&pwm->chip.pwms[i],
> + PWM_POLARITY_INVERSED);
> clk_disable_unprepare(pwm->clk);
>
> return 0;
> diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
> index fc3166d..cc549b7 100644
> --- a/drivers/regulator/pwm-regulator.c
> +++ b/drivers/regulator/pwm-regulator.c
> @@ -56,7 +56,7 @@ static int pwm_regulator_set_voltage_sel(struct regulator_dev *rdev,
> int dutycycle;
> int ret;
>
> - pwm_reg_period = pwm_get_period(drvdata->pwm);
> + pwm_reg_period = pwm_get_default_period(drvdata->pwm);
>
> dutycycle = (pwm_reg_period *
> drvdata->duty_cycle_table[selector].dutycycle) / 100;
> @@ -114,7 +114,7 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
> {
> struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
> unsigned int ramp_delay = rdev->constraints->ramp_delay;
> - unsigned int period = pwm_get_period(drvdata->pwm);
> + unsigned int period = pwm_get_default_period(drvdata->pwm);
> int duty_cycle;
> int ret;
>
> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
> index 35fe482..449ebc3 100644
> --- a/drivers/video/backlight/lm3630a_bl.c
> +++ b/drivers/video/backlight/lm3630a_bl.c
> @@ -162,7 +162,7 @@ static int lm3630a_intr_config(struct lm3630a_chip *pchip)
>
> static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
> {
> - unsigned int period = pwm_get_period(pchip->pwmd);
> + unsigned int period = pwm_get_default_period(pchip->pwmd);
> unsigned int duty = br * period / br_max;
>
> pwm_config(pchip->pwmd, duty, period);
> @@ -425,7 +425,7 @@ static int lm3630a_probe(struct i2c_client *client,
> return PTR_ERR(pchip->pwmd);
> }
> }
> - pchip->pwmd->period = pdata->pwm_period;
> + pwm_set_default_period(pchip->pwmd, pdata->pwm_period);
>
> /* interrupt enable : irq 0 is not allowed */
> pchip->irq = client->irq;
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index eff379b..ae498c1 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -294,7 +294,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> * set the period from platform data if it has not already been set
> * via the PWM lookup table.
> */
> - pb->period = pwm_get_period(pb->pwm);
> + pb->period = pwm_get_default_period(pb->pwm);
> if (!pb->period && (data->pwm_period_ns > 0)) {
> pb->period = data->pwm_period_ns;
> pwm_set_period(pb->pwm, data->pwm_period_ns);
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 93f4c90..ab3daf0 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -294,7 +294,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
> return PTR_ERR(par->pwm);
> }
>
> - par->pwm_period = pwm_get_period(par->pwm);
> + par->pwm_period = pwm_get_default_period(par->pwm);
> /* Enable the PWM */
> pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
> pwm_enable(par->pwm);
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index d681f68..31239a9 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -115,11 +115,22 @@ static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
> pwm->period = period;
> }
>
> +static inline void pwm_set_default_period(struct pwm_device *pwm,
> + unsigned int period)
> +{
> + pwm_set_period(pwm, period);
> +}
> +
> static inline unsigned int pwm_get_period(const struct pwm_device *pwm)
> {
> return pwm ? pwm->period : 0;
> }
>
> +static inline unsigned int pwm_get_default_period(const struct pwm_device *pwm)
> +{
> + return pwm_get_period(pwm);
> +}
> +
> static inline void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty)
> {
> if (pwm)
> @@ -136,6 +147,12 @@ static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm)
> */
> int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity);
>
> +static inline void pwm_set_default_polarity(struct pwm_device *pwm,
> + enum pwm_polarity polarity)
> +{
> + pwm_set_polarity(pwm, polarity);
> +}
> +
> static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
> {
> return pwm ? pwm->polarity : PWM_POLARITY_NORMAL;
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types
From: Jacek Anaszewski @ 2015-09-22 9:46 UTC (permalink / raw)
To: Andrzej Hajda
Cc: David Howells, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
linux-kernel, brcm80211-dev-list, devel, dev, dri-devel,
intel-gfx, linux-api, linux-arm-kernel, linux-cachefs, linux-clk,
linux-crypto, linux-fbdev, linux-input, linux-leds, linux-media,
linux-mips, linux-mm, linux-omap, linux-rdma, linux-serial,
linux-sh, linux-usb, linux-wireless, lustre-devel, netdev,
rtc-linux
In-Reply-To: <56011BB9.5030004@samsung.com>
On 09/22/2015 11:13 AM, Andrzej Hajda wrote:
> On 09/21/2015 03:42 PM, David Howells wrote:
>> Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>>
>>> Semantic patch finds comparisons of types:
>>> unsigned < 0
>>> unsigned >= 0
>>> The former is always false, the latter is always true.
>>> Such comparisons are useless, so theoretically they could be
>>> safely removed, but their presence quite often indicates bugs.
>>
>> Or someone has left them in because they don't matter and there's the
>> possibility that the type being tested might be or become signed under some
>> circumstances. If the comparison is useless, I'd expect the compiler to just
>> discard it - for such cases your patch is pointless.
>>
>> If I have, for example:
>>
>> unsigned x;
>>
>> if (x = 0 || x > 27)
>> give_a_range_error();
>>
>> I will write this as:
>>
>> unsigned x;
>>
>> if (x <= 0 || x > 27)
>> give_a_range_error();
>>
>> because it that gives a way to handle x being changed to signed at some point
>> in the future for no cost. In which case, your changing the <= to an =
>> "because the < part of the case is useless" is arguably wrong.
>
> This is why I have not checked for such cases - I have skipped checks of type
> unsigned <= 0
> exactly for the reasons above.
>
> However I have left two other checks as they seems to me more suspicious - they
> are always true or false. But as Dmitry and Andrew pointed out Linus have quite
> strong opinion against removing range checks in such cases as he finds it
> clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37.
Dropped 30/38 and 31/38 from LED tree then.
--
Best Regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types
From: Andrzej Hajda @ 2015-09-22 9:13 UTC (permalink / raw)
To: David Howells
Cc: linux-mips, linux-fbdev, linux-sh, brcm80211-dev-list, dri-devel,
linux-mm, linux-clk, linux-leds, Marek Szyprowski, devel,
linux-rdma, linux-cachefs, linux-serial, linux-input, linux-media,
dev, rtc-linux, Bartlomiej Zolnierkiewicz, intel-gfx, linux-omap,
linux-arm-kernel, Andrzej Hajda, linux-api, linux-usb,
linux-wireless, linux-kernel, linux-crypto, netdev, lustre-devel
In-Reply-To: <17571.1442842945@warthog.procyon.org.uk>
On 09/21/2015 03:42 PM, David Howells wrote:
> Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>
>> Semantic patch finds comparisons of types:
>> unsigned < 0
>> unsigned >= 0
>> The former is always false, the latter is always true.
>> Such comparisons are useless, so theoretically they could be
>> safely removed, but their presence quite often indicates bugs.
>
> Or someone has left them in because they don't matter and there's the
> possibility that the type being tested might be or become signed under some
> circumstances. If the comparison is useless, I'd expect the compiler to just
> discard it - for such cases your patch is pointless.
>
> If I have, for example:
>
> unsigned x;
>
> if (x = 0 || x > 27)
> give_a_range_error();
>
> I will write this as:
>
> unsigned x;
>
> if (x <= 0 || x > 27)
> give_a_range_error();
>
> because it that gives a way to handle x being changed to signed at some point
> in the future for no cost. In which case, your changing the <= to an =
> "because the < part of the case is useless" is arguably wrong.
This is why I have not checked for such cases - I have skipped checks of type
unsigned <= 0
exactly for the reasons above.
However I have left two other checks as they seems to me more suspicious - they
are always true or false. But as Dmitry and Andrew pointed out Linus have quite
strong opinion against removing range checks in such cases as he finds it
clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37.
Regards
Andrzej
>
> David
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v3 01/12] pwm: introduce default period and polarity concepts
From: Jacek Anaszewski @ 2015-09-22 6:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1442828009-6241-2-git-send-email-boris.brezillon@free-electrons.com>
Hi Boris,
On 09/21/2015 11:33 AM, Boris Brezillon wrote:
> When requested by a user, the PWM is assigned a default period and polarity
> extracted from the DT, the platform data or statically set by the driver.
> Those default values are currently stored in the period and polarity
> fields of the pwm_device struct, but they will be stored somewhere else
> once we have introduced the architecture allowing for hardware state
> retrieval.
>
> The pwm_set_default_polarity and pwm_set_default_period should only be
> used by PWM drivers or the PWM core infrastructure to specify the
> default period and polarity values.
>
> PWM users might call the pwm_get_default_period to query the default
> period value. There is currently no helper to query the default
> polarity, but it might be added later on if there is a need for it.
>
> This patch also modifies all the places where the default helpers should
> be used in place of the standard ones.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> drivers/leds/leds-pwm.c | 2 +-
> drivers/pwm/core.c | 14 +++++++-------
> drivers/pwm/pwm-pxa.c | 2 +-
> drivers/pwm/pwm-sun4i.c | 3 ++-
> drivers/regulator/pwm-regulator.c | 4 ++--
> drivers/video/backlight/lm3630a_bl.c | 4 ++--
> drivers/video/backlight/pwm_bl.c | 2 +-
> drivers/video/fbdev/ssd1307fb.c | 2 +-
> include/linux/pwm.h | 17 +++++++++++++++++
> 9 files changed, 34 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
> index 1d07e3e..2c564d1 100644
> --- a/drivers/leds/leds-pwm.c
> +++ b/drivers/leds/leds-pwm.c
> @@ -125,7 +125,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> if (led_data->can_sleep)
> INIT_WORK(&led_data->work, led_pwm_work);
>
> - led_data->period = pwm_get_period(led_data->pwm);
> + led_data->period = pwm_get_default_period(led_data->pwm);
> if (!led_data->period && (led->pwm_period_ns > 0))
> led_data->period = led->pwm_period_ns;
>
Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>
--
Best Regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH] backlight: pwm: free pwm requested by legacy API on error path
From: Lee Jones @ 2015-09-21 23:23 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Jingoo Han, Thierry Reding, linux-pwm, linux-fbdev
In-Reply-To: <1434292334-22312-1-git-send-email-vladimir_zapolskiy@mentor.com>
On Sun, 14 Jun 2015, Vladimir Zapolskiy wrote:
> If pwm is requested by legacy pwm_request() and if the following
> backlight_device_register() call fails, add pwm_free() clean-up.
>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> ---
> drivers/video/backlight/pwm_bl.c | 2 ++
> 1 file changed, 2 insertions(+)
Applied, thanks.
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 9991cdb..a691247 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -307,6 +307,8 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> if (IS_ERR(bl)) {
> dev_err(&pdev->dev, "failed to register backlight\n");
> ret = PTR_ERR(bl);
> + if (pb->legacy)
> + pwm_free(pb->pwm);
> goto err_alloc;
> }
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] backlight: pwm: reject legacy pwm request for device defined in dt
From: Lee Jones @ 2015-09-21 23:22 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Jingoo Han, Thierry Reding, linux-pwm, linux-fbdev
In-Reply-To: <1434292172-22129-1-git-send-email-vladimir_zapolskiy@mentor.com>
On Sun, 14 Jun 2015, Vladimir Zapolskiy wrote:
> Platform PWM backlight data provided by board's device tree should be
> complete enough to successfully request a pwm device using pwm_get()
> API. This change fixes a bug, when an arbitrary (first found) PWM is
> connected to a "pwm-backlight" compatible device, when explicit PWM
> device reference is not given.
>
> Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
> already describes "pwms" as a required property, instead of blind
> selection of a potentially wrong PWM reject legacy PWM device
> registration request, leave legacy API only for non-dt cases.
>
> Based on initial implementation done by Dmitry Eremin-Solenikov.
>
> Reported-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> Acked-by: Thierry Reding <thierry.reding@gmail.com>
> ---
> drivers/video/backlight/pwm_bl.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
Applied, thanks.
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 57cb9ec..9991cdb 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -271,15 +271,16 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> }
>
> pb->pwm = devm_pwm_get(&pdev->dev, NULL);
> - if (IS_ERR(pb->pwm)) {
> + if (IS_ERR(pb->pwm) && !pdev->dev.of_node) {
> dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
> pb->legacy = true;
> pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
> - if (IS_ERR(pb->pwm)) {
> - dev_err(&pdev->dev, "unable to request legacy PWM\n");
> - ret = PTR_ERR(pb->pwm);
> - goto err_alloc;
> - }
> + }
> +
> + if (IS_ERR(pb->pwm)) {
> + dev_err(&pdev->dev, "unable to request PWM\n");
> + ret = PTR_ERR(pb->pwm);
> + goto err_alloc;
> }
>
> dev_dbg(&pdev->dev, "got pwm for backlight\n");
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Heiko Stübner @ 2015-09-21 22:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1442828009-6241-1-git-send-email-boris.brezillon@free-electrons.com>
Hi Boris,
Am Montag, 21. September 2015, 11:33:17 schrieb Boris Brezillon:
> This series adds support for atomic PWM update, or IOW, the capability
> to update all the parameters of a PWM device (enabled/disabled, period,
> duty and polarity) in one go.
I gave this v3 a spin on a rk3288-veyron device with the pwm-regulator
supplying vdd_logic. Looks really nice and runs fine, so the series
Tested-by: Heiko Stuebner <heiko@sntech.de>
^ permalink raw reply
* Re: [PATCH v3 11/12] regulator: pwm: implement ->enable(), ->disable() and ->is_enabled methods
From: Mark Brown @ 2015-09-21 21:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1442828009-6241-12-git-send-email-boris.brezillon@free-electrons.com>
[-- Attachment #1: Type: text/plain, Size: 349 bytes --]
On Mon, Sep 21, 2015 at 11:33:28AM +0200, Boris Brezillon wrote:
> Implement the ->enable(), ->disable() and ->is_enabled methods and remove
> the PWM call in ->set_voltage_sel().
I'll apply this so it definitely makes it into mainline, I'm happy to
provide a tag for this so it can be merged into the PWM tree if the rest
of the series is ready.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH v3 12/12] regulator: pwm: properly initialize the ->state field
From: Mark Brown @ 2015-09-21 21:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1442828009-6241-13-git-send-email-boris.brezillon@free-electrons.com>
[-- Attachment #1: Type: text/plain, Size: 360 bytes --]
On Mon, Sep 21, 2015 at 11:33:29AM +0200, Boris Brezillon wrote:
> The ->state field is currently initialized to 0, thus referencing the
> voltage selector at index 0, which might not reflect the current voltage
> value.
> If possible, retrieve the current voltage selector from the PWM state, else
> return -EINVAL.
Acked-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH v3 01/12] pwm: introduce default period and polarity concepts
From: Robert Jarzmik @ 2015-09-21 18:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1442828009-6241-2-git-send-email-boris.brezillon@free-electrons.com>
Boris Brezillon <boris.brezillon@free-electrons.com> writes:
> When requested by a user, the PWM is assigned a default period and polarity
> extracted from the DT, the platform data or statically set by the driver.
> Those default values are currently stored in the period and polarity
> fields of the pwm_device struct, but they will be stored somewhere else
> once we have introduced the architecture allowing for hardware state
> retrieval.
>
> The pwm_set_default_polarity and pwm_set_default_period should only be
> used by PWM drivers or the PWM core infrastructure to specify the
> default period and polarity values.
>
> PWM users might call the pwm_get_default_period to query the default
> period value. There is currently no helper to query the default
> polarity, but it might be added later on if there is a need for it.
>
> This patch also modifies all the places where the default helpers should
> be used in place of the standard ones.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
For pwm-pxa.c :
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Cheers.
--
Robert
^ permalink raw reply
* Re: [PATCH] backlight: pwm: reject legacy pwm request for device defined in dt
From: Vladimir Zapolskiy @ 2015-09-21 15:16 UTC (permalink / raw)
To: Lee Jones, Thierry Reding; +Cc: Jingoo Han, linux-pwm, linux-fbdev
In-Reply-To: <1434292172-22129-1-git-send-email-vladimir_zapolskiy@mentor.com>
Lee, Thierry,
On 14.06.2015 17:29, Vladimir Zapolskiy wrote:
> Platform PWM backlight data provided by board's device tree should be
> complete enough to successfully request a pwm device using pwm_get()
> API. This change fixes a bug, when an arbitrary (first found) PWM is
> connected to a "pwm-backlight" compatible device, when explicit PWM
> device reference is not given.
>
> Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
> already describes "pwms" as a required property, instead of blind
> selection of a potentially wrong PWM reject legacy PWM device
> registration request, leave legacy API only for non-dt cases.
>
> Based on initial implementation done by Dmitry Eremin-Solenikov.
>
> Reported-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> Acked-by: Thierry Reding <thierry.reding@gmail.com>
> ---
> drivers/video/backlight/pwm_bl.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 57cb9ec..9991cdb 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -271,15 +271,16 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> }
>
> pb->pwm = devm_pwm_get(&pdev->dev, NULL);
> - if (IS_ERR(pb->pwm)) {
> + if (IS_ERR(pb->pwm) && !pdev->dev.of_node) {
> dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
> pb->legacy = true;
> pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
> - if (IS_ERR(pb->pwm)) {
> - dev_err(&pdev->dev, "unable to request legacy PWM\n");
> - ret = PTR_ERR(pb->pwm);
> - goto err_alloc;
> - }
> + }
> +
> + if (IS_ERR(pb->pwm)) {
> + dev_err(&pdev->dev, "unable to request PWM\n");
> + ret = PTR_ERR(pb->pwm);
> + goto err_alloc;
> }
>
> dev_dbg(&pdev->dev, "got pwm for backlight\n");
>
could you please apply this reviewed and acked change?
Thank you in advance.
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH] backlight: pwm: free pwm requested by legacy API on error path
From: Vladimir Zapolskiy @ 2015-09-21 15:15 UTC (permalink / raw)
To: Lee Jones, Jingoo Han, Thierry Reding; +Cc: linux-pwm, linux-fbdev
In-Reply-To: <1434292334-22312-1-git-send-email-vladimir_zapolskiy@mentor.com>
Lee, Thierry,
On 14.06.2015 17:32, Vladimir Zapolskiy wrote:
> If pwm is requested by legacy pwm_request() and if the following
> backlight_device_register() call fails, add pwm_free() clean-up.
>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> ---
> drivers/video/backlight/pwm_bl.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 9991cdb..a691247 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -307,6 +307,8 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> if (IS_ERR(bl)) {
> dev_err(&pdev->dev, "failed to register backlight\n");
> ret = PTR_ERR(bl);
> + if (pb->legacy)
> + pwm_free(pb->pwm);
> goto err_alloc;
> }
>
>
could you please apply this trivial fix?
Thank you in advance.
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types
From: David Howells @ 2015-09-21 13:42 UTC (permalink / raw)
To: Andrzej Hajda
Cc: dhowells-H+wXaHxf7aLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-cachefs-H+wXaHxf7aLQT0dZR+AlfA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-mips-6z/3iImG2C8G8FEW9MqTrA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-sh-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
lustre-devel-aLEFhgZF4x6X6Mz3xDxJMA
In-Reply-To: <1442842450-29769-1-git-send-email-a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Andrzej Hajda <a.hajda@samsung.com> wrote:
> Semantic patch finds comparisons of types:
> unsigned < 0
> unsigned >= 0
> The former is always false, the latter is always true.
> Such comparisons are useless, so theoretically they could be
> safely removed, but their presence quite often indicates bugs.
Or someone has left them in because they don't matter and there's the
possibility that the type being tested might be or become signed under some
circumstances. If the comparison is useless, I'd expect the compiler to just
discard it - for such cases your patch is pointless.
If I have, for example:
unsigned x;
if (x = 0 || x > 27)
give_a_range_error();
I will write this as:
unsigned x;
if (x <= 0 || x > 27)
give_a_range_error();
because it that gives a way to handle x being changed to signed at some point
in the future for no cost. In which case, your changing the <= to an =
"because the < part of the case is useless" is arguably wrong.
David
^ permalink raw reply
* [PATCH 28/38] video/omap: remove invalid check
From: Andrzej Hajda @ 2015-09-21 13:34 UTC (permalink / raw)
To: linux-kernel
Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
Tomi Valkeinen, Jean-Christophe Plagniol-Villard, linux-fbdev,
linux-omap
In-Reply-To: <1442842450-29769-1-git-send-email-a.hajda@samsung.com>
regno is unsigned so it cannot be negative.
The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/video/fbdev/omap/omapfb_main.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/video/fbdev/omap/omapfb_main.c b/drivers/video/fbdev/omap/omapfb_main.c
index 1fb3ea3..393ae1b 100644
--- a/drivers/video/fbdev/omap/omapfb_main.c
+++ b/drivers/video/fbdev/omap/omapfb_main.c
@@ -276,11 +276,6 @@ static int _setcolreg(struct fb_info *info, u_int regno, u_int red, u_int green,
if (r != 0)
break;
- if (regno < 0) {
- r = -EINVAL;
- break;
- }
-
if (regno < 16) {
u16 pal;
pal = ((red >> (16 - var->red.length)) <<
--
1.9.1
^ permalink raw reply related
* [PATCH v3 12/12] regulator: pwm: properly initialize the ->state field
From: Boris Brezillon @ 2015-09-21 9:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1442828009-6241-1-git-send-email-boris.brezillon@free-electrons.com>
The ->state field is currently initialized to 0, thus referencing the
voltage selector at index 0, which might not reflect the current voltage
value.
If possible, retrieve the current voltage selector from the PWM state, else
return -EINVAL.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/regulator/pwm-regulator.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index 9ffdbd6..449e3b3 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -41,10 +41,35 @@ struct pwm_voltages {
/**
* Voltage table call-backs
*/
+static void pwm_regulator_init_state(struct regulator_dev *rdev)
+{
+ struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
+ struct pwm_state pwm_state;
+ unsigned int dutycycle;
+ int i;
+
+ pwm_get_state(drvdata->pwm, &pwm_state);
+
+ if (!pwm_state.period)
+ return;
+
+ dutycycle = (pwm_state.duty_cycle * 100) / pwm_state.period;
+
+ for (i = 0; i < rdev->desc->n_voltages; i++) {
+ if (dutycycle = drvdata->duty_cycle_table[i].dutycycle) {
+ drvdata->state = i;
+ return;
+ }
+ }
+}
+
static int pwm_regulator_get_voltage_sel(struct regulator_dev *rdev)
{
struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
+ if (drvdata->state < 0)
+ pwm_regulator_init_state(rdev);
+
return drvdata->state;
}
@@ -211,6 +236,7 @@ static int pwm_regulator_init_table(struct platform_device *pdev,
return ret;
}
+ drvdata->state = -EINVAL;
drvdata->duty_cycle_table = duty_cycle_table;
pwm_regulator_desc.ops = &pwm_regulator_voltage_table_ops;
pwm_regulator_desc.n_voltages = length / sizeof(*duty_cycle_table);
--
1.9.1
^ permalink raw reply related
* [PATCH v3 11/12] regulator: pwm: implement ->enable(), ->disable() and ->is_enabled methods
From: Boris Brezillon @ 2015-09-21 9:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1442828009-6241-1-git-send-email-boris.brezillon@free-electrons.com>
Implement the ->enable(), ->disable() and ->is_enabled methods and remove
the PWM call in ->set_voltage_sel().
This is particularly important for critical regulators tagged as always-on,
because not claiming the PWM (and its dependencies) might lead to
unpredictable behavior (like a system hang because the PWM clk is only
claimed when the PWM device is enabled).
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/regulator/pwm-regulator.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index cc549b7..9ffdbd6 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -69,12 +69,6 @@ static int pwm_regulator_set_voltage_sel(struct regulator_dev *rdev,
drvdata->state = selector;
- ret = pwm_enable(drvdata->pwm);
- if (ret) {
- dev_err(&rdev->dev, "Failed to enable PWM\n");
- return ret;
- }
-
return 0;
}
@@ -89,6 +83,29 @@ static int pwm_regulator_list_voltage(struct regulator_dev *rdev,
return drvdata->duty_cycle_table[selector].uV;
}
+static int pwm_regulator_enable(struct regulator_dev *dev)
+{
+ struct pwm_regulator_data *drvdata = rdev_get_drvdata(dev);
+
+ return pwm_enable(drvdata->pwm);
+}
+
+static int pwm_regulator_disable(struct regulator_dev *dev)
+{
+ struct pwm_regulator_data *drvdata = rdev_get_drvdata(dev);
+
+ pwm_disable(drvdata->pwm);
+
+ return 0;
+}
+
+static int pwm_regulator_is_enabled(struct regulator_dev *dev)
+{
+ struct pwm_regulator_data *drvdata = rdev_get_drvdata(dev);
+
+ return pwm_is_enabled(drvdata->pwm);
+}
+
/**
* Continuous voltage call-backs
*/
@@ -144,11 +161,17 @@ static struct regulator_ops pwm_regulator_voltage_table_ops = {
.get_voltage_sel = pwm_regulator_get_voltage_sel,
.list_voltage = pwm_regulator_list_voltage,
.map_voltage = regulator_map_voltage_iterate,
+ .enable = pwm_regulator_enable,
+ .disable = pwm_regulator_disable,
+ .is_enabled = pwm_regulator_is_enabled,
};
static struct regulator_ops pwm_regulator_voltage_continuous_ops = {
.get_voltage = pwm_regulator_get_voltage,
.set_voltage = pwm_regulator_set_voltage,
+ .enable = pwm_regulator_enable,
+ .disable = pwm_regulator_disable,
+ .is_enabled = pwm_regulator_is_enabled,
};
static struct regulator_desc pwm_regulator_desc = {
--
1.9.1
^ 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