* [PATCH 1/3] [media] tw68: make tw68_pci_tbl static and constify
@ 2014-09-04 14:46 Mauro Carvalho Chehab
2014-09-04 14:46 ` [PATCH 2/3] [media] tw68: Remove a sparse warning Mauro Carvalho Chehab
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2014-09-04 14:46 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, Hans Verkuil
drivers/media/pci/tw68/tw68-core.c:72:22: warning: symbol 'tw68_pci_tbl' was not declared. Should it be static?
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
diff --git a/drivers/media/pci/tw68/tw68-core.c b/drivers/media/pci/tw68/tw68-core.c
index baf93af1d764..a6fb48cf7aae 100644
--- a/drivers/media/pci/tw68/tw68-core.c
+++ b/drivers/media/pci/tw68/tw68-core.c
@@ -69,7 +69,7 @@ static atomic_t tw68_instance = ATOMIC_INIT(0);
* the PCI ID database up to date. Note that the entries must be
* added under vendor 0x1797 (Techwell Inc.) as subsystem IDs.
*/
-struct pci_device_id tw68_pci_tbl[] = {
+static const struct pci_device_id tw68_pci_tbl[] = {
{PCI_DEVICE(PCI_VENDOR_ID_TECHWELL, PCI_DEVICE_ID_6800)},
{PCI_DEVICE(PCI_VENDOR_ID_TECHWELL, PCI_DEVICE_ID_6801)},
{PCI_DEVICE(PCI_VENDOR_ID_TECHWELL, PCI_DEVICE_ID_6804)},
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/3] [media] tw68: Remove a sparse warning 2014-09-04 14:46 [PATCH 1/3] [media] tw68: make tw68_pci_tbl static and constify Mauro Carvalho Chehab @ 2014-09-04 14:46 ` Mauro Carvalho Chehab 2014-09-04 14:54 ` Hans Verkuil 2014-09-04 16:20 ` Hans Verkuil 2014-09-04 14:46 ` [PATCH 3/3] [media] tw68: don't assume that pagesize is always 4096 Mauro Carvalho Chehab 2014-09-04 14:50 ` [PATCH 1/3] [media] tw68: make tw68_pci_tbl static and constify Hans Verkuil 2 siblings, 2 replies; 9+ messages in thread From: Mauro Carvalho Chehab @ 2014-09-04 14:46 UTC (permalink / raw) Cc: Mauro Carvalho Chehab, Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil drivers/media/pci/tw68/tw68-video.c:351:9: warning: incorrect type in argument 1 (different base types) drivers/media/pci/tw68/tw68-video.c:351:9: expected unsigned int [unsigned] val drivers/media/pci/tw68/tw68-video.c:351:9: got restricted __le32 [usertype] <noident> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c index 66fae2345fdd..4dd38578cf1b 100644 --- a/drivers/media/pci/tw68/tw68-video.c +++ b/drivers/media/pci/tw68/tw68-video.c @@ -348,7 +348,7 @@ int tw68_video_start_dma(struct tw68_dev *dev, struct tw68_buf *buf) * a new address can be set. */ tw_clearl(TW68_DMAC, TW68_DMAP_EN); - tw_writel(TW68_DMAP_SA, cpu_to_le32(buf->dma)); + tw_writel(TW68_DMAP_SA, (__force u32)cpu_to_le32(buf->dma)); /* Clear any pending interrupts */ tw_writel(TW68_INTSTAT, dev->board_virqmask); /* Enable the risc engine and the fifo */ -- 1.9.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] [media] tw68: Remove a sparse warning 2014-09-04 14:46 ` [PATCH 2/3] [media] tw68: Remove a sparse warning Mauro Carvalho Chehab @ 2014-09-04 14:54 ` Hans Verkuil 2014-09-04 15:05 ` Mauro Carvalho Chehab 2014-09-04 16:20 ` Hans Verkuil 1 sibling, 1 reply; 9+ messages in thread From: Hans Verkuil @ 2014-09-04 14:54 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil I'll need to review this as well. Perhaps tw_writel should expect a __le32? Hans On 09/04/14 16:46, Mauro Carvalho Chehab wrote: > drivers/media/pci/tw68/tw68-video.c:351:9: warning: incorrect type in argument 1 (different base types) > drivers/media/pci/tw68/tw68-video.c:351:9: expected unsigned int [unsigned] val > drivers/media/pci/tw68/tw68-video.c:351:9: got restricted __le32 [usertype] <noident> > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> > > diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c > index 66fae2345fdd..4dd38578cf1b 100644 > --- a/drivers/media/pci/tw68/tw68-video.c > +++ b/drivers/media/pci/tw68/tw68-video.c > @@ -348,7 +348,7 @@ int tw68_video_start_dma(struct tw68_dev *dev, struct tw68_buf *buf) > * a new address can be set. > */ > tw_clearl(TW68_DMAC, TW68_DMAP_EN); > - tw_writel(TW68_DMAP_SA, cpu_to_le32(buf->dma)); > + tw_writel(TW68_DMAP_SA, (__force u32)cpu_to_le32(buf->dma)); > /* Clear any pending interrupts */ > tw_writel(TW68_INTSTAT, dev->board_virqmask); > /* Enable the risc engine and the fifo */ > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] [media] tw68: Remove a sparse warning 2014-09-04 14:54 ` Hans Verkuil @ 2014-09-04 15:05 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 9+ messages in thread From: Mauro Carvalho Chehab @ 2014-09-04 15:05 UTC (permalink / raw) To: Hans Verkuil Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil Em Thu, 04 Sep 2014 16:54:38 +0200 Hans Verkuil <hansverk@cisco.com> escreveu: > I'll need to review this as well. Perhaps tw_writel should expect a __le32? There are several other parts where it is using the address as CPU endian. Anyway, if you have some BE system, then you can test the board there and see what would work. Regards, Mauro > > Hans > > On 09/04/14 16:46, Mauro Carvalho Chehab wrote: > > drivers/media/pci/tw68/tw68-video.c:351:9: warning: incorrect type in argument 1 (different base types) > > drivers/media/pci/tw68/tw68-video.c:351:9: expected unsigned int [unsigned] val > > drivers/media/pci/tw68/tw68-video.c:351:9: got restricted __le32 [usertype] <noident> > > > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> > > > > diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c > > index 66fae2345fdd..4dd38578cf1b 100644 > > --- a/drivers/media/pci/tw68/tw68-video.c > > +++ b/drivers/media/pci/tw68/tw68-video.c > > @@ -348,7 +348,7 @@ int tw68_video_start_dma(struct tw68_dev *dev, struct tw68_buf *buf) > > * a new address can be set. > > */ > > tw_clearl(TW68_DMAC, TW68_DMAP_EN); > > - tw_writel(TW68_DMAP_SA, cpu_to_le32(buf->dma)); > > + tw_writel(TW68_DMAP_SA, (__force u32)cpu_to_le32(buf->dma)); > > /* Clear any pending interrupts */ > > tw_writel(TW68_INTSTAT, dev->board_virqmask); > > /* Enable the risc engine and the fifo */ > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] [media] tw68: Remove a sparse warning 2014-09-04 14:46 ` [PATCH 2/3] [media] tw68: Remove a sparse warning Mauro Carvalho Chehab 2014-09-04 14:54 ` Hans Verkuil @ 2014-09-04 16:20 ` Hans Verkuil 1 sibling, 0 replies; 9+ messages in thread From: Hans Verkuil @ 2014-09-04 16:20 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil On 09/04/2014 04:46 PM, Mauro Carvalho Chehab wrote: > drivers/media/pci/tw68/tw68-video.c:351:9: warning: incorrect type in argument 1 (different base types) > drivers/media/pci/tw68/tw68-video.c:351:9: expected unsigned int [unsigned] val > drivers/media/pci/tw68/tw68-video.c:351:9: got restricted __le32 [usertype] <noident> > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> Nacked-by: Hans Verkuil <hverkuil@xs4all.nl> tw_writel maps to writel which already does cpu_to_le32(), so doing it again is once too many. I'll post a patch that removes the bogus cpu_to_le32(). Hans > > diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c > index 66fae2345fdd..4dd38578cf1b 100644 > --- a/drivers/media/pci/tw68/tw68-video.c > +++ b/drivers/media/pci/tw68/tw68-video.c > @@ -348,7 +348,7 @@ int tw68_video_start_dma(struct tw68_dev *dev, struct tw68_buf *buf) > * a new address can be set. > */ > tw_clearl(TW68_DMAC, TW68_DMAP_EN); > - tw_writel(TW68_DMAP_SA, cpu_to_le32(buf->dma)); > + tw_writel(TW68_DMAP_SA, (__force u32)cpu_to_le32(buf->dma)); > /* Clear any pending interrupts */ > tw_writel(TW68_INTSTAT, dev->board_virqmask); > /* Enable the risc engine and the fifo */ > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] [media] tw68: don't assume that pagesize is always 4096 2014-09-04 14:46 [PATCH 1/3] [media] tw68: make tw68_pci_tbl static and constify Mauro Carvalho Chehab 2014-09-04 14:46 ` [PATCH 2/3] [media] tw68: Remove a sparse warning Mauro Carvalho Chehab @ 2014-09-04 14:46 ` Mauro Carvalho Chehab 2014-09-04 14:53 ` Hans Verkuil 2014-09-04 16:21 ` Hans Verkuil 2014-09-04 14:50 ` [PATCH 1/3] [media] tw68: make tw68_pci_tbl static and constify Hans Verkuil 2 siblings, 2 replies; 9+ messages in thread From: Mauro Carvalho Chehab @ 2014-09-04 14:46 UTC (permalink / raw) Cc: Mauro Carvalho Chehab, Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil This code implicitly assumes that pagesize is 4096, but this is not true on all archs, as the PAGE_SHIFT can be different than 12 on all those architectures: alpha, arc, arm64, cris, frv, hexagon,ia64, m68k, metag, microblaze, mips, openrisc, parisc, powerpc, sh, sparc and tile. The real constrant here seems to be to limit the buffer size to 4MB. So, fix the code to reflect that, in a way that it will keep working with differnt values for PAGE_SIZE. Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c index 4dd38578cf1b..66658accdca9 100644 --- a/drivers/media/pci/tw68/tw68-video.c +++ b/drivers/media/pci/tw68/tw68-video.c @@ -366,7 +366,7 @@ static int tw68_buffer_pages(int size) { size = PAGE_ALIGN(size); size += PAGE_SIZE; /* for non-page-aligned buffers */ - size /= 4096; + size /= PAGE_SIZE; return size; } @@ -376,7 +376,7 @@ static int tw68_buffer_count(unsigned int size, unsigned int count) { unsigned int maxcount; - maxcount = 1024 / tw68_buffer_pages(size); + maxcount = (4096 * 1024 / PAGE_SIZE) / tw68_buffer_pages(size); if (count > maxcount) count = maxcount; return count; -- 1.9.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] [media] tw68: don't assume that pagesize is always 4096 2014-09-04 14:46 ` [PATCH 3/3] [media] tw68: don't assume that pagesize is always 4096 Mauro Carvalho Chehab @ 2014-09-04 14:53 ` Hans Verkuil 2014-09-04 16:21 ` Hans Verkuil 1 sibling, 0 replies; 9+ messages in thread From: Hans Verkuil @ 2014-09-04 14:53 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil I need to review this more closely tomorrow. Hans On 09/04/14 16:46, Mauro Carvalho Chehab wrote: > This code implicitly assumes that pagesize is 4096, but this is > not true on all archs, as the PAGE_SHIFT can be different than > 12 on all those architectures: alpha, arc, arm64, cris, frv, > hexagon,ia64, m68k, metag, microblaze, mips, openrisc, parisc, > powerpc, sh, sparc and tile. > > The real constrant here seems to be to limit the buffer size to > 4MB. > > So, fix the code to reflect that, in a way that it will keep > working with differnt values for PAGE_SIZE. > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> > > diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c > index 4dd38578cf1b..66658accdca9 100644 > --- a/drivers/media/pci/tw68/tw68-video.c > +++ b/drivers/media/pci/tw68/tw68-video.c > @@ -366,7 +366,7 @@ static int tw68_buffer_pages(int size) > { > size = PAGE_ALIGN(size); > size += PAGE_SIZE; /* for non-page-aligned buffers */ > - size /= 4096; > + size /= PAGE_SIZE; > return size; > } > > @@ -376,7 +376,7 @@ static int tw68_buffer_count(unsigned int size, unsigned int count) > { > unsigned int maxcount; > > - maxcount = 1024 / tw68_buffer_pages(size); > + maxcount = (4096 * 1024 / PAGE_SIZE) / tw68_buffer_pages(size); > if (count > maxcount) > count = maxcount; > return count; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] [media] tw68: don't assume that pagesize is always 4096 2014-09-04 14:46 ` [PATCH 3/3] [media] tw68: don't assume that pagesize is always 4096 Mauro Carvalho Chehab 2014-09-04 14:53 ` Hans Verkuil @ 2014-09-04 16:21 ` Hans Verkuil 1 sibling, 0 replies; 9+ messages in thread From: Hans Verkuil @ 2014-09-04 16:21 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil On 09/04/2014 04:46 PM, Mauro Carvalho Chehab wrote: > This code implicitly assumes that pagesize is 4096, but this is > not true on all archs, as the PAGE_SHIFT can be different than > 12 on all those architectures: alpha, arc, arm64, cris, frv, > hexagon,ia64, m68k, metag, microblaze, mips, openrisc, parisc, > powerpc, sh, sparc and tile. > > The real constrant here seems to be to limit the buffer size to > 4MB. > > So, fix the code to reflect that, in a way that it will keep > working with differnt values for PAGE_SIZE. While your fix is OK, I have a better patch that makes the code a lot more readable. I'll post that soon. Regards, Hans > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> > > diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c > index 4dd38578cf1b..66658accdca9 100644 > --- a/drivers/media/pci/tw68/tw68-video.c > +++ b/drivers/media/pci/tw68/tw68-video.c > @@ -366,7 +366,7 @@ static int tw68_buffer_pages(int size) > { > size = PAGE_ALIGN(size); > size += PAGE_SIZE; /* for non-page-aligned buffers */ > - size /= 4096; > + size /= PAGE_SIZE; > return size; > } > > @@ -376,7 +376,7 @@ static int tw68_buffer_count(unsigned int size, unsigned int count) > { > unsigned int maxcount; > > - maxcount = 1024 / tw68_buffer_pages(size); > + maxcount = (4096 * 1024 / PAGE_SIZE) / tw68_buffer_pages(size); > if (count > maxcount) > count = maxcount; > return count; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] [media] tw68: make tw68_pci_tbl static and constify 2014-09-04 14:46 [PATCH 1/3] [media] tw68: make tw68_pci_tbl static and constify Mauro Carvalho Chehab 2014-09-04 14:46 ` [PATCH 2/3] [media] tw68: Remove a sparse warning Mauro Carvalho Chehab 2014-09-04 14:46 ` [PATCH 3/3] [media] tw68: don't assume that pagesize is always 4096 Mauro Carvalho Chehab @ 2014-09-04 14:50 ` Hans Verkuil 2 siblings, 0 replies; 9+ messages in thread From: Hans Verkuil @ 2014-09-04 14:50 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil Acked-by: Hans Verkuil <hans.verkuil@xs4all.nl> Thanks! Hans On 09/04/14 16:46, Mauro Carvalho Chehab wrote: > drivers/media/pci/tw68/tw68-core.c:72:22: warning: symbol 'tw68_pci_tbl' was not declared. Should it be static? > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com> > > diff --git a/drivers/media/pci/tw68/tw68-core.c b/drivers/media/pci/tw68/tw68-core.c > index baf93af1d764..a6fb48cf7aae 100644 > --- a/drivers/media/pci/tw68/tw68-core.c > +++ b/drivers/media/pci/tw68/tw68-core.c > @@ -69,7 +69,7 @@ static atomic_t tw68_instance = ATOMIC_INIT(0); > * the PCI ID database up to date. Note that the entries must be > * added under vendor 0x1797 (Techwell Inc.) as subsystem IDs. > */ > -struct pci_device_id tw68_pci_tbl[] = { > +static const struct pci_device_id tw68_pci_tbl[] = { > {PCI_DEVICE(PCI_VENDOR_ID_TECHWELL, PCI_DEVICE_ID_6800)}, > {PCI_DEVICE(PCI_VENDOR_ID_TECHWELL, PCI_DEVICE_ID_6801)}, > {PCI_DEVICE(PCI_VENDOR_ID_TECHWELL, PCI_DEVICE_ID_6804)}, > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-09-04 16:22 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-04 14:46 [PATCH 1/3] [media] tw68: make tw68_pci_tbl static and constify Mauro Carvalho Chehab 2014-09-04 14:46 ` [PATCH 2/3] [media] tw68: Remove a sparse warning Mauro Carvalho Chehab 2014-09-04 14:54 ` Hans Verkuil 2014-09-04 15:05 ` Mauro Carvalho Chehab 2014-09-04 16:20 ` Hans Verkuil 2014-09-04 14:46 ` [PATCH 3/3] [media] tw68: don't assume that pagesize is always 4096 Mauro Carvalho Chehab 2014-09-04 14:53 ` Hans Verkuil 2014-09-04 16:21 ` Hans Verkuil 2014-09-04 14:50 ` [PATCH 1/3] [media] tw68: make tw68_pci_tbl static and constify Hans Verkuil
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).