A few SOLO6x10 patches.
Subsystems (2)
Hi,
I'm attaching a few patches to SOLO6x10 video frame grabber driver. Nothing out of ordinary, an audio bug fix (nobody using SOLO with audio?), an rmmod-related panic and the register access "optimization". Feel free to pick up what you want.
The remaining issue is incorrect SDRAM size reported in certain cases. I still have to investigate it a bit.
Signature · 4 lines
Krzysztof Halasa Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland
PATCH 1/1 SOLO6x10: unmap registers only after free_irq().
Fixes a panic on ARM. Diagnosis by Russell King.
drivers/media/pci/solo6x10/solo6x10-core.c +1 -1
@@ -164,9 +164,9 @@ static void free_solo_dev(struct solo_dev *solo_dev)/* Now cleanup the PCI device */ solo_irq_off (solo_dev, ~0 ); -pci_iounmap (pdev, solo_dev->reg_base);if (pdev->irq)free_irq (pdev->irq, solo_dev); +pci_iounmap (pdev, solo_dev->reg_base); }pci_release_regions (pdev);
Ismael Luceno
maintainerRe: [PATCH] SOLO6x10: unmap registers only after free_irq().
Quoted 18 lines · On Mon, 08 Jun 2015 15:37:15 +0200 [email protected] (Krzysztof Hałasa) wrote:
Fixes a panic on ARM. Diagnosis by Russell King.
Signed-off-by Krzysztof Hałasa <[email protected]>drivers/media/pci/solo6x10/solo6x10-core.c +1
-2@@ -164,9 +164,9 @@ static void free_solo_dev(struct solo_dev *solo_dev)/* Now cleanup the PCI device */ solo_irq_off (solo_dev, ~0 ); -pci_iounmap (pdev, solo_dev->reg_base);if (pdev->irq)free_irq (pdev->irq, solo_dev); +pci_iounmap (pdev, solo_dev->reg_base); }pci_release_regions (pdev); --
Quoted 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/1 SOLO6x10: Fix G.723 minimum audio period count.
In reply to Krzysztof HałasaThe period count is fixed, don't confuse ALSA.
drivers/media/pci/solo6x10/solo6x10-g723.c +6 -7
@@ -48,10 +48,8 @@/* The solo writes to 1k byte pages, 32 pages, in the dma. Each 1k page * is broken down to 20 * 48 byte regions (one for each channel possible) * with the rest of the page being dummy data. */ -#define G723_MAX_BUFFER (G723_PERIOD_BYTES * PERIODS_MAX) +#define PERIODS G723_FDMA_PAGES#define G723_INTR_ORDER 4/* 0 - 4 */ -#define PERIODS_MIN (1 << G723_INTR_ORDER) -#define PERIODS_MAX G723_FDMA_PAGESstruct solo_snd_pcm {int on;
@@ -130,11 +128,11 @@ static const struct snd_pcm_hardware snd_solo_pcm_hw = { .rate_max = SAMPLERATE, .channels_min = 1, .channels_max = 1, - .buffer_bytes_max = G723_MAX_BUFFER, + .buffer_bytes_max = G723_PERIOD_BYTES * PERIODS, .period_bytes_min = G723_PERIOD_BYTES, .period_bytes_max = G723_PERIOD_BYTES, - .periods_min = PERIODS_MIN, - .periods_max = PERIODS_MAX, + .periods_min = PERIODS, + .periods_max = PERIODS, };static int snd_solo_pcm_open (struct snd_pcm_substream *ss)
@@ -340,7 +338,8 @@ static int solo_snd_pcm_init(struct solo_dev *solo_dev) ret =snd_pcm_lib_preallocate_pages_for_all (pcm,SNDRV_DMA_TYPE_CONTINUOUS ,snd_dma_continuous_data (GFP_KERNEL ), -G723_MAX_BUFFER ,G723_MAX_BUFFER ); +G723_PERIOD_BYTES *PERIODS , +G723_PERIOD_BYTES *PERIODS );if (ret <0 )return ret;
Ismael Luceno
maintainerRe: [PATCH] SOLO6x10: Fix G.723 minimum audio period count.
Quoted 47 lines · On Mon, 08 Jun 2015 15:35:05 +0200 [email protected] (Krzysztof Hałasa) wrote:
The period count is fixed, don't confuse ALSA.
Signed-off-by Krzysztof Hałasa <[email protected]>drivers/media/pci/solo6x10/solo6x10-g723.c +0
-0@@ -48,10 +48,8 @@/* The solo writes to 1k byte pages, 32 pages, in the dma. Each 1k page
* is broken down to 20 * 48 byte regions (one for each channel
possible)
* with the rest of the page being dummy data. */
-#defineG723_MAX_BUFFER(G723_PERIOD_BYTES*PERIODS_MAX) +#define PERIODSG723_FDMA_PAGES
#defineG723_INTR_ORDER4 /* 0 - 4 */drivers/media/pci/solo6x10/solo6x10-g723.c +5
-7-#define PERIODS_MIN (1 << G723_INTR_ORDER) -#define PERIODS_MAX G723_FDMA_PAGESstruct solo_snd_pcm {int on;@@ -130,11 +128,11 @@ static const struct snd_pcm_hardware snd_solo_pcm_hw = { .rate_max = SAMPLERATE, .channels_min = 1, .channels_max = 1, - .buffer_bytes_max = G723_MAX_BUFFER, + .buffer_bytes_max = G723_PERIOD_BYTES * PERIODS, .period_bytes_min = G723_PERIOD_BYTES, .period_bytes_max = G723_PERIOD_BYTES, - .periods_min = PERIODS_MIN, - .periods_max = PERIODS_MAX, + .periods_min = PERIODS, + .periods_max = PERIODS, };static int snd_solo_pcm_open (struct snd_pcm_substream *ss)@@ -340,7 +338,8 @@ static int solo_snd_pcm_init(struct solo_dev *solo_dev) ret = snd_pcm_lib_preallocate_pages_for_all(pcm,SNDRV_DMA_TYPE_CONTINUOUS ,snd_dma_continuous_data (GFP_KERNEL ), -G723_MAX_BUFFER ,G723_MAX_BUFFER ); +G723_PERIOD_BYTES *PERIODS , + G723_PERIOD_BYTES * PERIODS);if (ret <0 )return ret; --
Quoted 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/1 SOLO6x10: remove unneeded register locking and barriers.
In reply to Krzysztof Hałasareadl() and writel() are atomic, we don't need the spin lock. Also, flushing posted write buffer isn't required. Especially on read :-)
drivers/media/pci/solo6x10/solo6x10-core.c +0 -1
@@ -483,7 +483,6 @@ static int solo_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) solo_dev->type = id->driver_data; solo_dev->pdev = pdev; -spin_lock_init (&solo_dev->reg_io_lock); ret =v4l2_device_register (&pdev->dev, &solo_dev->v4l2_dev);if (ret) goto fail_probe;
drivers/media/pci/solo6x10/solo6x10.h +1 -25
@@ -201,7 +201,6 @@ struct solo_dev {int nr_ext;u32 irq_mask;u32 motion_mask; -spinlock_t reg_io_lock;struct v4l2_device v4l2_dev;/* tw28xx accounting */
@@ -283,36 +282,13 @@ struct solo_dev {static inline u32 solo_reg_read (struct solo_dev *solo_dev,int reg) { -unsigned long flags; -u32 ret; -u16 val; - -spin_lock_irqsave (&solo_dev->reg_io_lock, flags); - - ret =readl (solo_dev->reg_base + reg); -rmb (); -pci_read_config_word (solo_dev->pdev,PCI_STATUS , &val); -rmb (); - -spin_unlock_irqrestore (&solo_dev->reg_io_lock, flags); - -return ret; +return readl (solo_dev->reg_base + reg); }static inline void solo_reg_write (struct solo_dev *solo_dev,int reg,u32 data) { -unsigned long flags; -u16 val; - -spin_lock_irqsave (&solo_dev->reg_io_lock, flags); -writel (data, solo_dev->reg_base + reg); -wmb (); -pci_read_config_word (solo_dev->pdev,PCI_STATUS , &val); -rmb (); - -spin_unlock_irqrestore (&solo_dev->reg_io_lock, flags); }static inline void solo_irq_on (struct solo_dev *dev,u32 mask)
Ismael Luceno
maintainerRe: [PATCH] SOLO6x10: remove unneeded register locking and barriers.
Quoted 54 lines · On Mon, 08 Jun 2015 15:42:24 +0200 [email protected] (Krzysztof Hałasa) wrote:
readl()andwritel()are atomic, we don't need the spin lock. Also, flushing posted write buffer isn't required. Especially on read :-)Signed-off-by Krzysztof Hałasa <[email protected]>drivers/media/pci/solo6x10/solo6x10-core.c +0
-1@@ -483,7 +483,6 @@ static int solo_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) solo_dev->type = id->driver_data; solo_dev->pdev = pdev; -spin_lock_init (&solo_dev->reg_io_lock); ret =v4l2_device_register (&pdev->dev, &solo_dev->v4l2_dev);if (ret) goto fail_probe;drivers/media/pci/solo6x10/solo6x10.h +1
-19@@ -201,7 +201,6 @@ struct solo_dev {int nr_ext;u32 irq_mask;u32 motion_mask; -spinlock_t reg_io_lock;struct v4l2_device v4l2_dev;/* tw28xx accounting */ @@ -283,36 +282,13 @@ struct solo_dev {static inline u32 solo_reg_read (struct solo_dev *solo_dev,int reg) { -unsigned long flags; -u32 ret; -u16 val; - -spin_lock_irqsave (&solo_dev->reg_io_lock, flags); - - ret =readl (solo_dev->reg_base + reg); -rmb (); -pci_read_config_word (solo_dev->pdev,PCI_STATUS , &val); -rmb (); - -spin_unlock_irqrestore (&solo_dev->reg_io_lock, flags); - -return ret; +return readl (solo_dev->reg_base + reg); }static inline void solo_reg_write (struct solo_dev *solo_dev,int reg,u32 data) { -unsigned long flags; -u16 val; - -spin_lock_irqsave (&solo_dev->reg_io_lock, flags);
drivers/media/pci/solo6x10/solo6x10.h struct solo_dev { · line 294
Quoted 10 lines
drivers/media/pci/solo6x10/solo6x10.h +0
-6-writel (data, solo_dev->reg_base + reg); -wmb (); -pci_read_config_word (solo_dev->pdev,PCI_STATUS , &val); -rmb (); - -spin_unlock_irqrestore (&solo_dev->reg_io_lock, flags); }static inline void solo_irq_on (struct solo_dev *dev,u32 mask)
PATCH 1/1 SOLO6x10: Remove dead code.
In reply to Krzysztof Hałasasolo_dev and pdev cannot be NULL here. It doesn't matter if we initialized the PCI device or not.
drivers/media/pci/solo6x10/solo6x10-core.c +2 -15
@@ -134,23 +134,11 @@ static irqreturn_t solo_isr(int irq, void *data)static void free_solo_dev (struct solo_dev *solo_dev) { -struct pci_dev *pdev; - -if (!solo_dev) -return ; +struct pci_dev *pdev = solo_dev->pdev;if (solo_dev->dev.parent)device_unregister (&solo_dev->dev); - pdev = solo_dev->pdev; - -/* If we never initialized the PCI device, then nothing else -* below here needs cleanup */ -if (!pdev) { -kfree (solo_dev); -return ; - } -if (solo_dev->reg_base) {/* Bring down the sub-devices first */ solo_g723_exit (solo_dev);
@@ -164,8 +152,7 @@ static void free_solo_dev(struct solo_dev *solo_dev)/* Now cleanup the PCI device */ solo_irq_off (solo_dev, ~0 ); -if (pdev->irq) -free_irq (pdev->irq, solo_dev); +free_irq (pdev->irq, solo_dev);pci_iounmap (pdev, solo_dev->reg_base); }
Ismael Luceno
maintainerRe: [PATCH] SOLO6x10: Remove dead code.
Quoted 33 lines · On Mon, 08 Jun 2015 15:50:22 +0200 [email protected] (Krzysztof Hałasa) wrote:
solo_devand pdev cannot be NULL here. It doesn't matter if we initialized the PCI device or not.Signed-off-by Krzysztof Hałasa <[email protected]>drivers/media/pci/solo6x10/solo6x10-core.c +1
-13@@ -134,23 +134,11 @@ static irqreturn_t solo_isr(int irq, void *data)static void free_solo_dev (struct solo_dev *solo_dev) { -struct pci_dev *pdev; - -if (!solo_dev) -return ; +struct pci_dev *pdev = solo_dev->pdev;if (solo_dev->dev.parent)device_unregister (&solo_dev->dev); - pdev = solo_dev->pdev; - -/* If we never initialized the PCI device, then nothing else -* below here needs cleanup */ -if (!pdev) { -kfree (solo_dev); -return ; - } -if (solo_dev->reg_base) {/* Bring down the sub-devices first */ solo_g723_exit (solo_dev);
drivers/media/pci/solo6x10/solo6x10-core.c static void free_solo_dev(struct solo_dev · line 158
Quoted 9 lines
drivers/media/pci/solo6x10/solo6x10-core.c +1
-2@@ -164,8 +152,7 @@ static void free_solo_dev(struct solo_dev *solo_dev)/* Now cleanup the PCI device */ solo_irq_off (solo_dev, ~0 ); -if (pdev->irq) -free_irq (pdev->irq, solo_dev); +free_irq (pdev->irq, solo_dev);pci_iounmap (pdev, solo_dev->reg_base); }
Ismael Luceno
maintainerOn Mon, 08 Jun 2015 15:30:17 +0200
[email protected] (Krzysztof Hałasa) wrote:
<...>
Quoted 1 line
(nobody using SOLO with audio?)
Probably not, the audio quality is pretty bad.