From: Andrey Utkin <andrey.utkin@corp.bluecherry.net>
To: "Krzysztof Hałasa" <khalasa@piap.pl>,
linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
"Mauro Carvalho Chehab" <mchehab@kernel.org>,
"Hans Verkuil" <hans.verkuil@cisco.com>,
"Ismael Luceno" <ismael@iodev.co.uk>
Cc: Bluecherry Maintainers <maintainers@bluecherrydvr.com>,
andrey_utkin@fastmail.com
Subject: solo6010 modprobe lockup since e1ceb25a (v4.3 regression)
Date: Thu, 15 Sep 2016 16:04:41 +0300 [thread overview]
Message-ID: <20160915130441.ji3f3jiiebsnsbct@acer> (raw)
Hi Krzysztof,
Me and one more solo6010 board user experience machine lockup when
solo6x10 module is loaded on kernel series starting with 4.3 (despite
solo6110 board probes just fine on all kernels). That is, 3.16, 3.18,
4.1 and 4.2 are tested and fine, and 4.3, 4.4, and others up to current
linux-next are bad.
So regression slipped in between 4.2 and 4.3. The diff between
stable/linux-4.2.y and ...-4.3.y (which were tested) is not large, and
my suspect fell on ripoff of register writing procedures complexity,
which was introduced in e1ceb25a (see below). Reversion of that fixes
lockup. However, if, on top of reversion of e1ceb25a, i drop barrier
stuff and pci_read_config... (see
https://github.com/bluecherrydvr/linux/commit/d59aaf3), leaving the
spinlock stuff, it locks up again. This is a matter in which I'm not
quite qualified, so I have no idea what that code copes with and why
this workaround works for solo6010. For now I think I'll tell the
customer to use kernel with e1ceb25a reverted, but for upstream fix, I'm
interested in more in-depth investigation. I'll be able to provide dmesg
logs a bit later.
The breaking commit is quoted below.
commit e1ceb25a1569ce5b61b9c496dd32d038ba8cb936
Author: Krzysztof Hałasa <khalasa@piap.pl>
Date: Mon Jun 8 10:42:24 2015 -0300
[media] SOLO6x10: remove unneeded register locking and barriers
readl() and writel() 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 <khalasa@piap.pl>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/pci/solo6x10/solo6x10-core.c b/drivers/media/pci/solo6x10/solo6x10-core.c
index 84627e6..9c948b1 100644
--- a/drivers/media/pci/solo6x10/solo6x10-core.c
+++ b/drivers/media/pci/solo6x10/solo6x10-core.c
@@ -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;
diff --git a/drivers/media/pci/solo6x10/solo6x10.h b/drivers/media/pci/solo6x10/solo6x10.h
index 1ca54b0..27423d7 100644
--- a/drivers/media/pci/solo6x10/solo6x10.h
+++ b/drivers/media/pci/solo6x10/solo6x10.h
@@ -199,7 +199,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 */
@@ -281,36 +280,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)
next reply other threads:[~2016-09-15 13:06 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-15 13:04 Andrey Utkin [this message]
2016-09-15 13:15 ` solo6010 modprobe lockup since e1ceb25a (v4.3 regression) Hans Verkuil
2016-09-15 13:19 ` Andrey Utkin
2016-09-15 13:25 ` Hans Verkuil
2016-09-15 13:58 ` One Thousand Gnomes
2016-09-21 13:16 ` Krzysztof Hałasa
2016-09-21 13:45 ` Andrey Utkin
2016-09-22 8:51 ` Krzysztof Hałasa
2016-09-22 15:23 ` Andrey Utkin
2016-09-26 5:38 ` Krzysztof Hałasa
2016-09-26 9:18 ` Andrey Utkin
2016-09-27 5:27 ` Krzysztof Hałasa
2016-09-27 7:40 ` Andrey Utkin
2016-09-27 11:33 ` Krzysztof Hałasa
2016-09-27 14:22 ` Andrey Utkin
2016-09-28 5:21 ` Krzysztof Hałasa
2016-10-24 19:32 ` Mauro Carvalho Chehab
2016-10-24 20:56 ` Andrey Utkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160915130441.ji3f3jiiebsnsbct@acer \
--to=andrey.utkin@corp.bluecherry.net \
--cc=andrey_utkin@fastmail.com \
--cc=hans.verkuil@cisco.com \
--cc=ismael@iodev.co.uk \
--cc=khalasa@piap.pl \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=maintainers@bluecherrydvr.com \
--cc=mchehab@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox