Moderated list (Was: Re: [BUG] New Kernel Bugs)
At Wed, 14 Nov 2007 04:01:31 -0800 (PST), David Miller wrote: > > From: David Miller <[EMAIL PROTECTED]> > Date: Wed, 14 Nov 2007 03:56:57 -0800 (PST) > > > The fact that it farts at me every time I post to this thread. > > See? I got another one and I have received at least 10 of the > following over the past 2 days. > > That's rediculious. > > And because a human adds the whitelist this is always going to > happen to someone when they start posting to the alsa list for > the first time. ... if you give too many recipients in your post. That is often really annoying thing to me, together with keeping the unrelated subject line ;) I personally don't care whether it's a moderated or open list. We chose it simply due to too bad S/N ratio at that time. So, if the current list annoys your or many others and the list management on vger is so good, it'd be basically a good move, of course. I'll appreciate it. The only confusion would be the change of ML address, but we can do it slowly, too. Takashi - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Moderated list (Was: Re: [BUG] New Kernel Bugs)
At Wed, 14 Nov 2007 13:21:30 +0100,
Rene Herman wrote:
>
> On 14-11-07 09:25, Takashi Iwai wrote:
>
> > At Wed, 14 Nov 2007 04:01:31 -0800 (PST),
> > David Miller wrote:
> >> From: David Miller <[EMAIL PROTECTED]>
> >> Date: Wed, 14 Nov 2007 03:56:57 -0800 (PST)
> >>
> >>> The fact that it farts at me every time I post to this thread.
> >> See? I got another one and I have received at least 10 of the
> >> following over the past 2 days.
> >>
> >> That's rediculious.
> >>
> >> And because a human adds the whitelist this is always going to
> >> happen to someone when they start posting to the alsa list for
> >> the first time.
> >
> > ... if you give too many recipients in your post. That is often
> > really annoying thing to me, together with keeping the unrelated
> > subject line ;)
> >
> > I personally don't care whether it's a moderated or open list.
> > We chose it simply due to too bad S/N ratio at that time. So, if the
> > current list annoys your or many others and the list management on
> > vger is so good, it'd be basically a good move, of course. I'll
> > appreciate it.
> >
> > The only confusion would be the change of ML address, but we can do it
> > slowly, too.
>
> I'd love the lists at vger. Amazing spam-filtering. I'd like to request the
> name [EMAIL PROTECTED] (and [EMAIL PROTECTED] if at all
> possible so we can open that one up as well) though.
I think alsa-user can stay as is. It's no place for dragging many
other addresses like alsa-devel.
BTW, I also prefer keeping the name [EMAIL PROTECTED] It's been so.
> There wouldn't need to be a forced ML address change if Jaroslov would then
> just rewrite alsa-{devel,[EMAIL PROTECTED] to vger.kernel.org same as
> he did for alsa-devel and does for alsa-user to @lists.sf.net.
If it works, then I'm for it, too.
thanks,
Takashi
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [BUG] New Kernel Bugs
At Thu, 15 Nov 2007 14:17:27 +0100, Olivier Galibert wrote: > > On Thu, Nov 15, 2007 at 06:59:34AM +0100, Rene Herman wrote: > > Totally unrelated indeed so why are spouting crap? If the kohab list has a > > problem take it up with them but keep ALSA out of it. alsa-devel has only > > ever moderated out spam -- nothing else. > > That is incorrect. Hopefully it is the case now though, since my > experience of the subject was years ago. Yeah, it was really years ago that we once switched to the open list. Funny that people never forget such a thing :) Takashi - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [very-RFC 0/8] TSN driver for the kernel
On Mon, 20 Jun 2016 17:21:26 +0200, Richard Cochran wrote: > > On Mon, Jun 20, 2016 at 02:31:48PM +0200, Richard Cochran wrote: > > Where is this "audio_time" program of which you speak? > > Never mind, found it in alsa-lib. > > I still would appreciate an answer to my other questions, though... Currently HD-audio (both ASoC and legacy ones) are the only drivers providing the link timestamp. In the recent code, it's PCM get_time_info ops, so you can easily grep it. HTH, Takashi
Re: [alsa-devel] [very-RFC 0/8] TSN driver for the kernel
On Tue, 21 Jun 2016 08:38:57 +0200, Richard Cochran wrote: > > On Tue, Jun 21, 2016 at 07:54:32AM +0200, Takashi Iwai wrote: > > > I still would appreciate an answer to my other questions, though... > > > > Currently HD-audio (both ASoC and legacy ones) are the only drivers > > providing the link timestamp. In the recent code, it's PCM > > get_time_info ops, so you can easily grep it. > > Yes, I found that myself, thanks. > > > HTH, > > No it doesn't help me, because I asked three questions, and none were > about the link timestamp. ?? The extended audio timpestamp is essentially to return the link timestamp. Just the term has changed along time... Takashi
Re: 4.3-rc3 Regression: NFS access stall by commit 6ae459bdaaee
On Tue, 29 Sep 2015 02:35:04 +0200, Pravin Shelar wrote: > > On Mon, Sep 28, 2015 at 6:12 AM, Takashi Iwai wrote: > > [I resent this since the previous mail didn't go out properly, as it > > seems; apologies if you already read it, please disregard] > > > > Hi, > > > > I noticed that NFS access from my workstation slowed down drastically, > > almost stalls, with the fresh 4.3-rc3. There are no particular kernel > > errors / warnings. > > > > Then I performed git section, and it leaded to the commit: > > 6ae459bdaaeebc632b16e54dcbabb490c6931d61 > > skbuff: Fix skb checksum flag on skb pull > > > > Reverting this commit from 4.3-rc3 fixed the issue indeed. > > > > Could you take a look at this? I added Trond to Cc in case he might > > already know of it. > > > I send out fix for similar issue. Can you try the posted patch. > https://patchwork.ozlabs.org/patch/523632/ Yes, the patch fixes the problem, thanks. Feel free to take my tested-by tag: Tested-by: Takashi Iwai But I guess the real fix is only the first chunk and the latter is nothing but a cleanup? If so, it'd be better to split it. Takashi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
S3 resume fails with r8169 (RTL8168evl/8111evl)
Hi, we're facing a problem where the network can't be resumed after S3 with r8169 driver. The chip is with RTL_GIGA_MAC_VER_34, 8168evl/8111evl. The link is up and detected at S3 resume, but there is no proceed after that. NetworkManager gave up after timeout. During S3 resume, there is no interrupt count increase, too. S4 works fine. It seems that only S3 suffers. Reloading r8169 module after S3 resume also results in the failure. Once when this happens, it can't even read the data properly, all iomap reads via RTL_R*() seem returning 0xff. The machine is a Baytrail-based small box. The problem is seen from all kernel versions I tested since 3.0.* to 4.2-final. The dmesg snippet regarding r8169 is attached below. Please let me know if you need any specific information. thanks, Takashi [0.00] Initializing cgroup subsys cpuset [0.00] Initializing cgroup subsys cpu [0.00] Initializing cgroup subsys cpuacct [0.00] Linux version 4.2.0-3.g7b8109d-default (geeko@buildhost) (gcc version 5.1.1 20150713 [gcc-5-branch revision 225736] (SUSE Linux) ) #1 SMP Mon Aug 31 11:17:08 UTC 2015 (7b8109d) [0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-4.2.0-3.g7b8109d-default root=UUID=d2af3e85-8ae1-4c53-abd4-4fdbffdd567e ro resume=/dev/sda2 splash=silent quiet showopts [6.896861] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded [6.896881] r8169 :01:00.0: can't disable ASPM; OS doesn't have ASPM control [6.897692] r8169 :01:00.0 eth0: RTL8168evl/8111evl at 0xc9662000, 00:80:64:cd:84:ca, XID 0c900880 IRQ 91 [6.897700] r8169 :01:00.0 eth0: jumbo features [frames: 9200 bytes, tx checksumming: ko] [8.719631] r8169 :01:00.0 eth0: link down [8.719697] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready [8.719720] r8169 :01:00.0 eth0: link down [ 11.337389] r8169 :01:00.0 eth0: link up [ 11.337406] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready [ 61.642477] PM: Syncing filesystems ... done. [ 61.960567] PM: Preparing system for sleep (mem) [ 67.064302] Freezing user space processes ... (elapsed 0.001 seconds) done. [ 67.065983] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ 67.067756] PM: Suspending system (mem) [ 67.067803] Suspending console(s) (use no_console_suspend to debug) [ 67.068637] serial 00:02: disabled [ 67.070188] sd 0:0:0:0: [sda] Synchronizing SCSI cache [ 67.070264] sd 0:0:0:0: [sda] Stopping disk [ 67.132282] PM: suspend of devices complete after 64.052 msecs [ 67.147893] PM: late suspend of devices complete after 15.570 msecs [ 67.148575] r8169 :01:00.0: System wakeup enabled by ACPI [ 67.148823] xhci_hcd :00:14.0: System wakeup enabled by ACPI [ 67.163503] PM: noirq suspend of devices complete after 15.574 msecs [ 67.163549] ACPI: Preparing to enter system sleep state S3 [ 67.164057] PM: Saving platform NVS memory [ 67.164067] Disabling non-boot CPUs ... [ 67.164372] Broke affinity for irq 88 [ 67.164376] Broke affinity for irq 91 [ 67.165412] smpboot: CPU 1 is now offline [ 67.166298] ACPI: Low-level resume complete [ 67.166386] PM: Restoring platform NVS memory [ 67.166848] Enabling non-boot CPUs ... [ 67.166974] x86: Booting SMP configuration: [ 67.166976] smpboot: Booting Node 0 Processor 1 APIC 0x2 [ 67.175206] cache: parent cpu1 should not be sleeping [ 67.175550] CPU1 is up [ 67.176020] ACPI: Waking up from system sleep state S3 [ 67.177427] acpi LNXPOWER:02: Turning OFF [ 67.177602] acpi LNXPOWER:01: Turning OFF [ 67.177749] acpi LNXPOWER:00: Turning OFF [ 67.192966] xhci_hcd :00:14.0: System wakeup disabled by ACPI [ 67.193241] PM: noirq resume of devices complete after 15.433 msecs [ 67.265799] PM: early resume of devices complete after 72.349 msecs [ 67.266866] rtc_cmos 00:00: System wakeup disabled by ACPI [ 67.268925] serial 00:02: activated [ 67.269217] r8169 :01:00.0: System wakeup disabled by ACPI [ 67.269831] r8169 :01:00.0 eth0: rtl_phyar_cond == 1 (loop: 20, delay: 25). [ 67.270466] r8169 :01:00.0 eth0: rtl_phyar_cond == 1 (loop: 20, delay: 25). [ 67.943469] r8169 :01:00.0 eth0: rtl_phyar_cond == 1 (loop: 20, delay: 25). [ 68.763630] r8169 :01:00.0 eth0: rtl_phy_reset_cond == 1 (loop: 100, delay: 1). [ 68.22] r8169 :01:00.0 eth0: rtl_chipcmd_cond == 1 (loop: 100, delay: 100). [ 68.779259] r8169 :01:00.0 eth0: rtl_csiar_cond == 1 (loop: 100, delay: 10). [ 68.907638] r8169 :01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100). [ 68.907640] r8169 :01:00.0 eth0: link up -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
At Wed, 20 May 2015 11:46:31 +0200,
Marcel Holtmann wrote:
>
> Hi Oliver,
>
> >> The data is cached in RAM. More specifically, the former loaded
> >> firmware files are reloaded and saved at suspend for each device
> >> object. See fw_pm_notify() in firmware_class.c.
> >
> > OK, this may be a stupid idea, but do we know the firmware
> > was successfully loaded in the first place?
> > Also btusb is in the habit of falling back to a generic
> > firmware in some places. It seems to me that caching
> > firmware is conceptually not enough, but we'd also need
> > to record the absence of firmware images.
>
> in a lot of cases the firmware is optional. The device will operate fine
> without the firmware. There are a few devices where the firmware is required,
> but for many it just contains patches.
>
> It would be nice if we could tell request_firmware() if it is optional or
> mandatory firmware. Or if it should just cache the status of a missing
> firmware as well.
OK, below is a quick hack to record the failed f/w files, too.
Not sure whether this helps, though. Proper tests are appreciated.
Takashi
---
From: Takashi Iwai
Subject: [PATCH] firmware: cache failed firmwares, too
Signed-off-by: Takashi Iwai
---
drivers/base/firmware_class.c | 33 -
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 171841ad1008..a15af7289c94 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -1035,6 +1035,8 @@ _request_firmware_prepare(struct firmware **firmware_p,
const char *name,
firmware->priv = buf;
if (ret > 0) {
+ if (buf->size == -1UL)
+ return -ENOENT; /* already recorded as failure */
ret = sync_cached_firmware_buf(buf);
if (!ret) {
fw_set_page_data(buf, firmware);
@@ -1047,17 +1049,12 @@ _request_firmware_prepare(struct firmware **firmware_p,
const char *name,
return 1; /* need to load */
}
-static int assign_firmware_buf(struct firmware *fw, struct device *device,
+static void assign_firmware_buf(struct firmware *fw, struct device *device,
unsigned int opt_flags)
{
struct firmware_buf *buf = fw->priv;
mutex_lock(&fw_lock);
- if (!buf->size || is_fw_load_aborted(buf)) {
- mutex_unlock(&fw_lock);
- return -ENOENT;
- }
-
/*
* add firmware name into devres list so that we can auto cache
* and uncache firmware for device.
@@ -1079,9 +1076,9 @@ static int assign_firmware_buf(struct firmware *fw,
struct device *device,
}
/* pass the pages buffer to driver at the last minute */
- fw_set_page_data(buf, fw);
+ if (buf->size != -1UL)
+ fw_set_page_data(buf, fw);
mutex_unlock(&fw_lock);
- return 0;
}
/* called from request_firmware() and request_firmware_work_func() */
@@ -1124,6 +1121,9 @@ _request_firmware(const struct firmware **firmware_p,
const char *name,
ret = fw_get_filesystem_firmware(device, fw->priv);
if (ret) {
+ struct firmware_buf *buf = fw->priv;
+
+ buf->size = -1UL; /* failed */
if (!(opt_flags & FW_OPT_NO_WARN))
dev_warn(device,
"Direct firmware load for %s failed with error
%d\n",
@@ -1132,12 +1132,12 @@ _request_firmware(const struct firmware **firmware_p,
const char *name,
dev_warn(device, "Falling back to user helper\n");
ret = fw_load_from_user_helper(fw, name, device,
opt_flags, timeout);
+ if (ret)
+ buf->size = -1UL; /* failed */
}
}
- if (!ret)
- ret = assign_firmware_buf(fw, device, opt_flags);
-
+ assign_firmware_buf(fw, device, opt_flags);
usermodehelper_read_unlock();
out:
@@ -1435,17 +1435,8 @@ static void __async_dev_cache_fw_image(void *fw_entry,
async_cookie_t cookie)
{
struct fw_cache_entry *fce = fw_entry;
- struct firmware_cache *fwc = &fw_cache;
- int ret;
-
- ret = cache_firmware(fce->name);
- if (ret) {
- spin_lock(&fwc->name_lock);
- list_del(&fce->list);
- spin_unlock(&fwc->name_lock);
- free_fw_cache_entry(fce);
- }
+ cache_firmware(fce->name);
}
/* called with dev->devres_lock held */
--
2.4.1
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
At Wed, 20 May 2015 16:42:44 -0700, Laura Abbott wrote: > > On 05/20/2015 05:44 AM, Takashi Iwai wrote: > > At Wed, 20 May 2015 11:46:31 +0200, > > Marcel Holtmann wrote: > >> > >> Hi Oliver, > >> > >>>> The data is cached in RAM. More specifically, the former loaded > >>>> firmware files are reloaded and saved at suspend for each device > >>>> object. See fw_pm_notify() in firmware_class.c. > >>> > >>> OK, this may be a stupid idea, but do we know the firmware > >>> was successfully loaded in the first place? > >>> Also btusb is in the habit of falling back to a generic > >>> firmware in some places. It seems to me that caching > >>> firmware is conceptually not enough, but we'd also need > >>> to record the absence of firmware images. > >> > >> in a lot of cases the firmware is optional. The device will operate fine > >> without the firmware. There are a few devices where the firmware is > >> required, but for many it just contains patches. > >> > >> It would be nice if we could tell request_firmware() if it is optional or > >> mandatory firmware. Or if it should just cache the status of a missing > >> firmware as well. > > > > OK, below is a quick hack to record the failed f/w files, too. > > Not sure whether this helps, though. Proper tests are appreciated. > > > > > > This doesn't quite work. We end up with the name on fw_names but > the firmware isn't actually on the firmware cache list. > > If request_firmware fails to get the firmware from the filesystem, > release firmware will be called which is going to free the > firmware_buf which has been marked as failed anyway. The only > way to make this work would be to always piggy back and increase > the ref so it always stays around. But this also marks the firmware > as a permanent failure. There would need to be a hook somewhere > to force a cache drop, else there would be no way to add new > firmware to a running system without a reboot. > > Perhaps we split the difference: keep a list of firmware images > that failed to load in the past and if one is requested during > a time when usermodehelper isn't available, silently return an > error? This way, if correct firmware is loaded at a regular time > the item can be removed from the list. Well, IMO, it's way too much expectation for the generic f/w loader. The driver itself must know already which should be really loaded. The fact is that it's the driver who calls the function that might not work in the resume path. So the driver can deal with such exceptions at best. This can be either delaying the f/w loading via proper UMH lock (like my former patch or your patch) or avoiding the f/w request of non-existing files that the driver already knows of. Takashi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
At Thu, 21 May 2015 14:07:11 +0200,
Marcel Holtmann wrote:
>
> Hi Takashi,
>
> >> The data is cached in RAM. More specifically, the former loaded
> >> firmware files are reloaded and saved at suspend for each device
> >> object. See fw_pm_notify() in firmware_class.c.
> >
> > OK, this may be a stupid idea, but do we know the firmware
> > was successfully loaded in the first place?
> > Also btusb is in the habit of falling back to a generic
> > firmware in some places. It seems to me that caching
> > firmware is conceptually not enough, but we'd also need
> > to record the absence of firmware images.
>
> in a lot of cases the firmware is optional. The device will operate fine
> without the firmware. There are a few devices where the firmware is
> required, but for many it just contains patches.
>
> It would be nice if we could tell request_firmware() if it is optional
> or mandatory firmware. Or if it should just cache the status of a
> missing firmware as well.
> >>>
> >>> OK, below is a quick hack to record the failed f/w files, too.
> >>> Not sure whether this helps, though. Proper tests are appreciated.
> >>>
> >>>
> >>
> >> This doesn't quite work. We end up with the name on fw_names but
> >> the firmware isn't actually on the firmware cache list.
> >>
> >> If request_firmware fails to get the firmware from the filesystem,
> >> release firmware will be called which is going to free the
> >> firmware_buf which has been marked as failed anyway. The only
> >> way to make this work would be to always piggy back and increase
> >> the ref so it always stays around. But this also marks the firmware
> >> as a permanent failure. There would need to be a hook somewhere
> >> to force a cache drop, else there would be no way to add new
> >> firmware to a running system without a reboot.
> >>
> >> Perhaps we split the difference: keep a list of firmware images
> >> that failed to load in the past and if one is requested during
> >> a time when usermodehelper isn't available, silently return an
> >> error? This way, if correct firmware is loaded at a regular time
> >> the item can be removed from the list.
> >
> > Well, IMO, it's way too much expectation for the generic f/w loader.
> > The driver itself must know already which should be really loaded.
> > The fact is that it's the driver who calls the function that might not
> > work in the resume path. So the driver can deal with such exceptions
> > at best.
>
> I keep repeating myself here. From the driver point of view it goes
> via probe() callback of the USB driver. So the driver does not
> know. For the driver it looks like a brand new device. There are
> platforms that might decide to just kill the power to the USB bus
> where the Bluetooth controller sits on. It gets the power back on
> resume. However this means it is a brand new device at that
> point. So the driver should not have to remember everything.
Then avoiding the failed firmware is no solution, indeed.
If it's a new probe, it should be never executed during resume.
That is, either freeze the work like Laura's patch or explicitly allow
the UMH lock wait like my patch. Laura's patch has a merit that it's
much simpler. OTOH, if you want to keep the changes only in
request_firmware() call, you can think of changes like my patch; a
revised version is attached below.
Takashi
---
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 171841ad1008..87157f557263 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -97,21 +97,6 @@ static inline long firmware_loading_timeout(void)
return loading_timeout > 0 ? loading_timeout * HZ : MAX_JIFFY_OFFSET;
}
-/* firmware behavior options */
-#define FW_OPT_UEVENT (1U << 0)
-#define FW_OPT_NOWAIT (1U << 1)
-#ifdef CONFIG_FW_LOADER_USER_HELPER
-#define FW_OPT_USERHELPER (1U << 2)
-#else
-#define FW_OPT_USERHELPER 0
-#endif
-#ifdef CONFIG_FW_LOADER_USER_HELPER_FALLBACK
-#define FW_OPT_FALLBACKFW_OPT_USERHELPER
-#else
-#define FW_OPT_FALLBACK0
-#endif
-#define FW_OPT_NO_WARN (1U << 3)
-
struct firmware_cache {
/* firmware_buf instance will be added into the below list */
spinlock_t lock;
@@ -1085,8 +1070,7 @@ static int assign_firmware_buf(struct firmware *fw,
struct device *device,
}
/* called from request_firmware() and request_firmware_work_func() */
-static int
-_request_firmware(const struct firmware **firmware_p, const char *name,
+int _request_firmware(const struct firmware **firmware_p, const char *name,
struct device *device, unsigned int opt_flags)
{
struct firmware *fw;
@@ -1099,13 +1083,15 @@ _request_firmware(const struct firmware **firmware_p,
const char *name,
if (!name || name[0] == '\0')
return -EINVAL;
+ /* Need to pin this module until return */
+ _
Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
At Thu, 21 May 2015 10:18:08 -0400 (EDT), Alan Stern wrote: > > On Thu, 21 May 2015, Takashi Iwai wrote: > > > Then avoiding the failed firmware is no solution, indeed. > > If it's a new probe, it should be never executed during resume. > > Can you expand this comment? What's wrong with probing during resume? Well, if the probe requires the access to a user-space file, it can't be done during resume. That's the very problem we're seeing now. The firmware loader can't help much alone if it's a new device object. > The USB stack does carry out probes during resume under certain > circumstances. A driver lacking a reset_resume callback is one of > those circumstances. So, having a proper reset_resume in btusb would help in the end? Takashi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
At Thu, 21 May 2015 11:26:17 -0400 (EDT), Alan Stern wrote: > > On Thu, 21 May 2015, Takashi Iwai wrote: > > > At Thu, 21 May 2015 10:18:08 -0400 (EDT), > > Alan Stern wrote: > > > > > > On Thu, 21 May 2015, Takashi Iwai wrote: > > > > > > > Then avoiding the failed firmware is no solution, indeed. > > > > If it's a new probe, it should be never executed during resume. > > > > > > Can you expand this comment? What's wrong with probing during resume? > > > > Well, if the probe requires the access to a user-space file, it can't > > be done during resume. That's the very problem we're seeing now. > > The firmware loader can't help much alone if it's a new device > > object. > > But the same thing happens during early boot, if the driver is built > into the kernel. When the probe occurs, userspace isn't up and running > yet, so the firmware loader can't do anything. > > Why should probe during resume be any worse than probe during early > boot? The early boot has initrd, so the files can be there. But the resume has no way to fetch the file except for cached data. Takashi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
At Thu, 21 May 2015 19:27:41 +0200, Arend van Spriel wrote: > > On 05/21/15 17:35, Takashi Iwai wrote: > > At Thu, 21 May 2015 11:26:17 -0400 (EDT), > > Alan Stern wrote: > >> > >> On Thu, 21 May 2015, Takashi Iwai wrote: > >> > >>> At Thu, 21 May 2015 10:18:08 -0400 (EDT), > >>> Alan Stern wrote: > >>>> > >>>> On Thu, 21 May 2015, Takashi Iwai wrote: > >>>> > >>>>> Then avoiding the failed firmware is no solution, indeed. > >>>>> If it's a new probe, it should be never executed during resume. > >>>> > >>>> Can you expand this comment? What's wrong with probing during resume? > >>> > >>> Well, if the probe requires the access to a user-space file, it can't > >>> be done during resume. That's the very problem we're seeing now. > >>> The firmware loader can't help much alone if it's a new device > >>> object. > >> > >> But the same thing happens during early boot, if the driver is built > >> into the kernel. When the probe occurs, userspace isn't up and running > >> yet, so the firmware loader can't do anything. > >> > >> Why should probe during resume be any worse than probe during early > >> boot? > > > > The early boot has initrd, so the files can be there. But the resume > > has no way to fetch the file except for cached data. > > but initrd is optional so without initrd it is pretty much the same. User can build the firmware into the kernel. Takashi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
At Thu, 21 May 2015 13:37:56 -0400 (EDT), Alan Stern wrote: > > On Thu, 21 May 2015, Takashi Iwai wrote: > > > At Thu, 21 May 2015 11:26:17 -0400 (EDT), > > Alan Stern wrote: > > > > > > On Thu, 21 May 2015, Takashi Iwai wrote: > > > > > > > At Thu, 21 May 2015 10:18:08 -0400 (EDT), > > > > Alan Stern wrote: > > > > > > > > > > On Thu, 21 May 2015, Takashi Iwai wrote: > > > > > > > > > > > Then avoiding the failed firmware is no solution, indeed. > > > > > > If it's a new probe, it should be never executed during resume. > > > > > > > > > > Can you expand this comment? What's wrong with probing during resume? > > > > > > > > Well, if the probe requires the access to a user-space file, it can't > > > > be done during resume. That's the very problem we're seeing now. > > > > The firmware loader can't help much alone if it's a new device > > > > object. > > > > > > But the same thing happens during early boot, if the driver is built > > > into the kernel. When the probe occurs, userspace isn't up and running > > > yet, so the firmware loader can't do anything. > > > > > > Why should probe during resume be any worse than probe during early > > > boot? > > > > The early boot has initrd, so the files can be there. But the resume > > has no way to fetch the file except for cached data. > > I suppose USB could delay re-probing until userspace is running again, > if we knew when that was. But it would be awkward and prone to races. > It also would leave a user-visible window of time during which the > device does not exist, which we want to avoid. (This may not matter > for bluetooth, but it does matter for other kinds of devices.) Right. > I would prefer to solve this problem in a different way, if possible. Well, we're back in square again :) But, before going further the discussion in loop again, I'd like to know which firmware file actually hits. Is it a non-existing firmware? Or is it a firmware that should have been cached? In the latter case, why it isn't used? Takashi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH] x86: remove vmalloc.h from asm/io.h
At Fri, 29 May 2015 19:18:47 +1000, Stephen Rothwell wrote: > > Nothing in asm/io.h uses anything from vmalloc.h, so remove the include > and fix up the build problems in an allmodconfig (64 bit and 32 bit) > build. > > This may be the place where x86 builds get vmalloc.h implicitly included > and that tends to hide places where vmalloc() et al are added to files > but the include of vmalloc.h is forgotten. > > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: "H. Peter Anvin" > Cc: > Cc: Konrad Rzeszutek Wilk > Cc: Boris Ostrovsky > Cc: David Vrabel > Cc: Anton Vorontsov > Cc: Colin Cross > Cc: Kees Cook > Cc: Tony Luck > Cc: "Rafael J. Wysocki" > Cc: Len Brown > Cc: Kristen Carlson Accardi > Cc: Viresh Kumar > Cc: Vinod Koul > Cc: "K. Y. Srinivasan" > Cc: Haiyang Zhang > Cc: Hiral Patel > Cc: Suma Ramars > Cc: Brian Uchino > Cc: "James E.J. Bottomley" > Cc: Jaroslav Kysela > Cc: Takashi Iwai For the sound bits, Acked-by: Takashi Iwai thanks, Takashi > Cc: Andrew Morton > Suggested-by: David Miller > Signed-off-by: Stephen Rothwell > > --- > > Based in Linus' tree of today. > > There are probably more places that need vmalloc.h included, but this > passes 64 bit and 32 bit allmodconfig builds, so is a place to start. > > Dave Miller suggested that I start this journey. > > arch/x86/include/asm/io.h | 2 -- > arch/x86/kernel/crash.c| 1 + > arch/x86/kernel/machine_kexec_64.c | 1 + > arch/x86/mm/pageattr-test.c| 1 + > arch/x86/mm/pageattr.c | 1 + > arch/x86/xen/p2m.c | 1 + > drivers/acpi/apei/erst.c | 1 + > drivers/cpufreq/intel_pstate.c | 1 + > drivers/dma/mic_x100_dma.c | 1 + > drivers/net/hyperv/netvsc.c| 1 + > drivers/net/hyperv/rndis_filter.c | 1 + > drivers/scsi/fnic/fnic_debugfs.c | 1 + > drivers/scsi/fnic/fnic_trace.c | 1 + > sound/pci/asihpi/hpioctl.c | 1 + > 14 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h > index 34a5b93704d3..5791e7ace9db 100644 > --- a/arch/x86/include/asm/io.h > +++ b/arch/x86/include/asm/io.h > @@ -197,8 +197,6 @@ extern void set_iounmap_nonlazy(void); > > #include > > -#include > - > /* > * Convert a virtual cached pointer to an uncached pointer > */ > diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c > index c76d3e37c6e1..e068d6683dba 100644 > --- a/arch/x86/kernel/crash.c > +++ b/arch/x86/kernel/crash.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > > #include > #include > diff --git a/arch/x86/kernel/machine_kexec_64.c > b/arch/x86/kernel/machine_kexec_64.c > index 415480d3ea84..11546b462fa6 100644 > --- a/arch/x86/kernel/machine_kexec_64.c > +++ b/arch/x86/kernel/machine_kexec_64.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > > #include > #include > diff --git a/arch/x86/mm/pageattr-test.c b/arch/x86/mm/pageattr-test.c > index 6629f397b467..8ff686aa7e8c 100644 > --- a/arch/x86/mm/pageattr-test.c > +++ b/arch/x86/mm/pageattr-test.c > @@ -9,6 +9,7 @@ > #include > #include > #include > +#include > > #include > #include > diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c > index 89af288ec674..bedfc794b4ba 100644 > --- a/arch/x86/mm/pageattr.c > +++ b/arch/x86/mm/pageattr.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > > #include > #include > diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c > index b47124d4cd67..8b7f18e200aa 100644 > --- a/arch/x86/xen/p2m.c > +++ b/arch/x86/xen/p2m.c > @@ -67,6 +67,7 @@ > #include > #include > #include > +#include > > #include > #include > diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c > index ed65e9c4b5b0..3670bbab57a3 100644 > --- a/drivers/acpi/apei/erst.c > +++ b/drivers/acpi/apei/erst.c > @@ -35,6 +35,7 @@ > #include > #include > #include > +#include > #include > > #include "apei-internal.h" > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c > index 6414661ac1c4..2ba53f4f6af2 100644 > --- a/drivers/cpufreq/intel_pstate.c > +++ b/drivers/cpufreq/intel_pstate.c > @@ -26,6 +26,7 @@ > #include > #include > #include > +#include > #include > > #include > diff --git a/drivers/dma/mic_x100_dma.c b/drivers/dma/mic_x100_dma.c > index 6de2e677be04..74d9db05a
Re: [alsa-devel] [trivial PATCH] treewide: Align function definition open/close braces
On Mon, 18 Dec 2017 01:28:44 +0100, Joe Perches wrote: > > Some functions definitions have either the initial open brace and/or > the closing brace outside of column 1. > > Move those braces to column 1. > > This allows various function analyzers like gnu complexity to work > properly for these modified functions. > > Miscellanea: > > o Remove extra trailing ; and blank line from xfs_agf_verify > > Signed-off-by: Joe Perches > --- > git diff -w shows no difference other than the above 'Miscellanea' > > (this is against -next, but it applies against Linus' tree > with a couple offsets) > > arch/x86/include/asm/atomic64_32.h | 2 +- > drivers/acpi/custom_method.c | 2 +- > drivers/acpi/fan.c | 2 +- > drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +- > drivers/media/i2c/msp3400-kthreads.c | 2 +- > drivers/message/fusion/mptsas.c | 2 +- > drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c | 2 +- > drivers/net/wireless/ath/ath9k/xmit.c| 2 +- > drivers/platform/x86/eeepc-laptop.c | 2 +- > drivers/rtc/rtc-ab-b5ze-s3.c | 2 +- > drivers/scsi/dpt_i2o.c | 2 +- > drivers/scsi/sym53c8xx_2/sym_glue.c | 2 +- > fs/locks.c | 2 +- > fs/ocfs2/stack_user.c| 2 +- > fs/xfs/libxfs/xfs_alloc.c| 5 ++--- > fs/xfs/xfs_export.c | 2 +- > kernel/audit.c | 6 +++--- > kernel/trace/trace_printk.c | 4 ++-- > lib/raid6/sse2.c | 14 +++--- > sound/soc/fsl/fsl_dma.c | 2 +- For sound bits, Acked-by: Takashi Iwai thanks, Takashi
[PATCH] hv/netvsc: Fix NULL dereference at single queue mode fallback
The recent commit 916c5e1413be ("hv/netvsc: fix handling of fallback
to single queue mode") tried to fix the fallback behavior to a single
queue mode, but it changed the function to return zero incorrectly,
while the function should return an object pointer. Eventually this
leads to a NULL dereference at the callers that expect non-NULL
value.
Fix it by returning the proper net_device object.
Fixes: 916c5e1413be ("hv/netvsc: fix handling of fallback to single queue mode")
Cc:
Signed-off-by: Takashi Iwai
---
drivers/net/hyperv/rndis_filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/rndis_filter.c
b/drivers/net/hyperv/rndis_filter.c
index 408ece27131c..2a5209f23f29 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1338,7 +1338,7 @@ struct netvsc_device *rndis_filter_device_add(struct
hv_device *dev,
/* setting up multiple channels failed */
net_device->max_chn = 1;
net_device->num_chn = 1;
- return 0;
+ return net_device;
err_dev_remv:
rndis_filter_device_remove(dev, net_device);
--
2.18.0
Re: [PATCH] hv/netvsc: Fix NULL dereference at single queue mode fallback
On Tue, 14 Aug 2018 19:29:32 +0200,
David Miller wrote:
>
> From: Takashi Iwai
> Date: Tue, 14 Aug 2018 19:10:50 +0200
>
> > The recent commit 916c5e1413be ("hv/netvsc: fix handling of fallback
> > to single queue mode") tried to fix the fallback behavior to a single
> > queue mode, but it changed the function to return zero incorrectly,
> > while the function should return an object pointer. Eventually this
> > leads to a NULL dereference at the callers that expect non-NULL
> > value.
> >
> > Fix it by returning the proper net_device object.
> >
> > Fixes: 916c5e1413be ("hv/netvsc: fix handling of fallback to single queue
> > mode")
> > Signed-off-by: Takashi Iwai
>
> Applied and queued up for -stable.
>
> Please do not put explicit "CC: stable" notations in networking patches, I
> queue
> up and submit networking patches to -stable explicitly.
OK, noted for the next time. Thanks!
Takashi
Re: [PATCH 20/22] sound: pci: avoid string overflow warnings
On Fri, 14 Jul 2017 14:07:12 +0200,
Arnd Bergmann wrote:
>
> With gcc-7, we get various warnings about a possible string overflow:
>
> sound/pci/rme9652/hdspm.c: In function 'snd_hdspm_create_alsa_devices':
> sound/pci/rme9652/hdspm.c:2123:17: error: ' MIDIoverMADI' directive writing
> 13 bytes into a region of size between 1 and 32 [-Werror=format-overflow=]
> sound/pci/pcxhr/pcxhr.c: In function 'pcxhr_probe':
> sound/pci/pcxhr/pcxhr.c:1647:28: error: ' [PCM #' directive writing 7 bytes
> into a region of size between 1 and 32 [-Werror=format-overflow=]
> sound/pci/mixart/mixart.c: In function 'snd_mixart_probe':
> sound/pci/mixart/mixart.c:1353:28: error: ' [PCM #' directive writing 7 bytes
> into a region of size between 1 and 32 [-Werror=format-overflow=]
>sprintf(card->shortname, "%s [PCM #%d]", mgr->shortname, i);
> ^~
> sound/pci/mixart/mixart.c:1353:28: note: using the range [-2147483648,
> 2147483647] for directive argument
> sound/pci/mixart/mixart.c:1353:3: note: 'sprintf' output between 10 and 51
> bytes into a destination of size 32
>sprintf(card->shortname, "%s [PCM #%d]", mgr->shortname, i);
>^~~
> sound/pci/mixart/mixart.c:1354:27: error: ' [PCM #' directive writing 7 bytes
> into a region of size between 1 and 80 [-Werror=format-overflow=]
>sprintf(card->longname, "%s [PCM #%d]", mgr->longname, i);
>^~
> sound/pci/mixart/mixart.c:1354:27: note: using the range [-2147483648,
> 2147483647] for directive argument
> sound/pci/mixart/mixart.c:1354:3: note: 'sprintf' output between 10 and 99
> bytes into a destination of size 80
>
> I have checked these all and found that the driver-private
> shortname strings for mixart and pcxhr are longer than necessary,
> and making them shorter will be safe while also making it clear
> that no overflow can happen when they get passed as a substring
> into the card shortname.
>
> For hdspm, we have a local buffer of the same size as its substring.
> In this case, making the buffer a little longer is safe as the
> functions that take it as an argument all use length checking and
> the strings we pass into it are actually short enough.
>
> Signed-off-by: Arnd Bergmann
Thanks for the patch. I have seen it but ignored, so far, as not sure
which action is the best. An alternative solution is to use
snprintf() blindly, for example.
For mixart, it's even better to drop mgr->shortname[] and longname[]
assignment. The shortname is the fixed string, and the longname is
used only at copying to card->longname, so we can create a string
there from the scratch.
Takashi
> ---
> sound/pci/mixart/mixart.h | 4 ++--
> sound/pci/pcxhr/pcxhr.h | 4 ++--
> sound/pci/rme9652/hdspm.c | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/sound/pci/mixart/mixart.h b/sound/pci/mixart/mixart.h
> index 426743871540..c8309e327663 100644
> --- a/sound/pci/mixart/mixart.h
> +++ b/sound/pci/mixart/mixart.h
> @@ -75,8 +75,8 @@ struct mixart_mgr {
> struct mem_area mem[2];
>
> /* share the name */
> - char shortname[32]; /* short name of this soundcard */
> - char longname[80]; /* name of this soundcard */
> + char shortname[16]; /* short name of this soundcard */
> + char longname[40]; /* name of this soundcard */
>
> /* one and only blocking message or notification may be pending */
> u32 pending_event;
> diff --git a/sound/pci/pcxhr/pcxhr.h b/sound/pci/pcxhr/pcxhr.h
> index 9e39e509a3ef..4909a43ce3d9 100644
> --- a/sound/pci/pcxhr/pcxhr.h
> +++ b/sound/pci/pcxhr/pcxhr.h
> @@ -75,8 +75,8 @@ struct pcxhr_mgr {
> unsigned long port[3];
>
> /* share the name */
> - char shortname[32]; /* short name of this soundcard */
> - char longname[96]; /* name of this soundcard */
> + char shortname[16]; /* short name of this soundcard */
> + char longname[40]; /* name of this soundcard */
>
> struct pcxhr_rmh *prmh;
>
> diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c
> index 254c3d040118..a1cbf5938a0e 100644
> --- a/sound/pci/rme9652/hdspm.c
> +++ b/sound/pci/rme9652/hdspm.c
> @@ -2061,7 +2061,7 @@ static int snd_hdspm_create_midi(struct snd_card *card,
>struct hdspm *hdspm, int id)
> {
> int err;
> - char buf[32];
> + char buf[64];
>
> hdspm->midi[id].id = id;
> hdspm->midi[id].hdspm = hdspm;
> --
> 2.9.0
>
>
Re: [alsa-devel] [PATCH 04/11] sound/hal2: switch to dma_alloc_attrs
On Fri, 16 Jun 2017 09:17:09 +0200,
Christoph Hellwig wrote:
>
> Use dma_alloc_attrs directly instead of the dma_alloc_noncoherent wrapper.
>
> Signed-off-by: Christoph Hellwig
Should I take this one through sound git tree, or would you prefer
taking all through another?
In the latter case,
Reviewed-by: Takashi Iwai
thanks,
Takashi
> ---
> sound/mips/hal2.c | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/sound/mips/hal2.c b/sound/mips/hal2.c
> index 00fc9241d266..17a78a93e885 100644
> --- a/sound/mips/hal2.c
> +++ b/sound/mips/hal2.c
> @@ -461,15 +461,15 @@ static int hal2_alloc_dmabuf(struct hal2_codec *codec)
> int count = H2_BUF_SIZE / H2_BLOCK_SIZE;
> int i;
>
> - codec->buffer = dma_alloc_noncoherent(NULL, H2_BUF_SIZE,
> - &buffer_dma, GFP_KERNEL);
> + codec->buffer = dma_alloc_attrs(NULL, H2_BUF_SIZE, &buffer_dma,
> + GFP_KERNEL, DMA_ATTR_NON_CONSISTENT);
> if (!codec->buffer)
> return -ENOMEM;
> - desc = dma_alloc_noncoherent(NULL, count * sizeof(struct hal2_desc),
> - &desc_dma, GFP_KERNEL);
> + desc = dma_alloc_attrs(NULL, count * sizeof(struct hal2_desc),
> +&desc_dma, GFP_KERNEL, DMA_ATTR_NON_CONSISTENT);
> if (!desc) {
> - dma_free_noncoherent(NULL, H2_BUF_SIZE,
> - codec->buffer, buffer_dma);
> + dma_free_attrs(NULL, H2_BUF_SIZE, codec->buffer, buffer_dma,
> +DMA_ATTR_NON_CONSISTENT);
> return -ENOMEM;
> }
> codec->buffer_dma = buffer_dma;
> @@ -490,10 +490,10 @@ static int hal2_alloc_dmabuf(struct hal2_codec *codec)
>
> static void hal2_free_dmabuf(struct hal2_codec *codec)
> {
> - dma_free_noncoherent(NULL, codec->desc_count * sizeof(struct hal2_desc),
> - codec->desc, codec->desc_dma);
> - dma_free_noncoherent(NULL, H2_BUF_SIZE, codec->buffer,
> - codec->buffer_dma);
> + dma_free_attrs(NULL, codec->desc_count * sizeof(struct hal2_desc),
> +codec->desc, codec->desc_dma, DMA_ATTR_NON_CONSISTENT);
> + dma_free_attrs(NULL, H2_BUF_SIZE, codec->buffer, codec->buffer_dma,
> +DMA_ATTR_NON_CONSISTENT);
> }
>
> static struct snd_pcm_hardware hal2_pcm_hw = {
> --
> 2.11.0
>
> ___
> Alsa-devel mailing list
> alsa-de...@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
Re: [alsa-devel] [PATCH 04/11] sound/hal2: switch to dma_alloc_attrs
On Fri, 16 Jun 2017 10:51:47 +0200, Christoph Hellwig wrote: > > On Fri, Jun 16, 2017 at 10:49:56AM +0200, Takashi Iwai wrote: > > On Fri, 16 Jun 2017 09:17:09 +0200, > > Christoph Hellwig wrote: > > > > > > Use dma_alloc_attrs directly instead of the dma_alloc_noncoherent wrapper. > > > > > > Signed-off-by: Christoph Hellwig > > > > Should I take this one through sound git tree, or would you prefer > > taking all through another? > > Feel free to take it through the sound tree. OK, applied now. Thanks! Takashi
[PATCH] sky2: Disable MSI on ASUS P6T
The onboard sky2 NIC on ASUS P6T WS PRO doesn't work after PM resume
due to the infamous IRQ problem. Disabling MSI works around it, so
let's add it to the blacklist.
Unfortunately the BIOS on the machine doesn't fill the standard
DMI_SYS_* entry, so we pick up DMI_BOARD_* entries instead.
BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1142496
Reported-and-tested-by: Marcus Seyfarth
Signed-off-by: Takashi Iwai
---
drivers/net/ethernet/marvell/sky2.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/marvell/sky2.c
b/drivers/net/ethernet/marvell/sky2.c
index f518312ffe69..a01c75ede871 100644
--- a/drivers/net/ethernet/marvell/sky2.c
+++ b/drivers/net/ethernet/marvell/sky2.c
@@ -4924,6 +4924,13 @@ static const struct dmi_system_id msi_blacklist[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "P5W DH Deluxe"),
},
},
+ {
+ .ident = "ASUS P6T",
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "ASUSTeK Computer INC."),
+ DMI_MATCH(DMI_BOARD_NAME, "P6T"),
+ },
+ },
{}
};
--
2.16.4
Re: regression in iwlwifi: page fault in iwl_dbg_tlv_alloc_region() (commit ba8f6f4ae254)
On Tue, 12 Jan 2021 16:46:21 +0100,
Kalle Valo wrote:
>
> Takashi Iwai writes:
>
> > On Tue, 12 Jan 2021 13:45:33 +0100,
> > Kalle Valo wrote:
> >>
> >> Takashi Iwai writes:
> >>
> >> > On Tue, 12 Jan 2021 12:33:14 +0100,
> >> > Kalle Valo wrote:
> >> >>
> >> >> (adding luca)
> >> >>
> >> >> Michal Kubecek writes:
> >> >>
> >> >> > FYI, there is a regression in iwlwifi driver caused by commit
> >> >> > ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device memory")
> >> >> > reported at
> >> >> >
> >> >> > https://bugzilla.kernel.org/show_bug.cgi?id=210733
> >> >> > https://bugzilla.suse.com/show_bug.cgi?id=1180344
> >> >> >
> >> >> > The problem seems to be an attempt to write terminating null character
> >> >> > into a string which may be read only. There is also a proposed fix.
> >> >>
> >> >> Can someone submit a proper patch, please? See instructions below how to
> >> >> submit.
> >> >>
> >> >> And please add Fixes tag to the commit log:
> >> >>
> >> >> Fixes: ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device memory")
> >> >
> >> > OK, I'll do it for my own
> >>
> >> Thanks.
> >>
> >> > but really I hoped that someone would have reacted on the bugzilla
> >> > report before the official patch submission. So far no one from the
> >> > upstream devs showed interest in the bug at all, unfortunately.
> >>
> >> Bugzilla is problematic as I don't know if anyone tracks it actively, at
> >> least I don't have time for that. I recommend reporting all wireless
> >> issues to mailing lists to make sure everyone see it.
> >
> > I share your feeling as a subsystem maintainer, but at the same time,
> > I see it's a big problem if the whole bugzilla reports are just
> > silently ignored. If it's a void, shouldn't we rather shut it down?
>
> I'm all for shutting down bugzilla.kernel.org as silent bug reports are
> frustrating the users. But I don't know what others would think about
> that, maybe some subsystems use it actively?
Yes, I'm still checking bugzilla.kernel.org for sound bug reports.
Not always promptly reacting like the distro bugzilla, but it's
regularly scanned and covered in the best effort basis.
Graphics people already moved out of bugzilla to gitlab Issues in
their own gitlab.freedesktop.org. Not sure about others.
> At least there should be a big warning for wireless bugs.
Maybe we can ask Konstantin about that at least for wireless
components?
> > And, although a mailing list is fine to report via only some texts or
> > some patches, it's not suitable to attach large logs or such, which is
> > often essential for debugging. Thanks to lore, the archivability is
> > no longer a problem with ML recently, but the lack of data assignment
> > is another problem, IMO.
>
> Sure, I fully agree on the benefits of a bug tracker. The issue here is
> the lack of people managing bug reports, not the tool.
Yeah, it's not only about wireless but true in general: handling bug
reports is a tough task and often annoying. Moreover, everyone of
course wants to believe that their driver is bug free :)
Takashi
Re: [PATCH 1/2] iwlwifi: dbg: Don't touch the tlv data
On Tue, 12 Jan 2021 16:48:56 +0100,
Kalle Valo wrote:
>
> Takashi Iwai writes:
>
> > The commit ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device
> > memory") added a termination of name string just to be sure, and this
> > seems causing a regression, a GPF triggered at firmware loading.
> > Basically we shouldn't modify the firmware data that may be provided
> > as read-only.
> >
> > This patch drops the code that caused the regression and keep the tlv
> > data as is.
> >
> > Fixes: ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device memory")
> > BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1180344
> > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=210733
> > Signed-off-by: Takashi Iwai
>
> I'm planning to queue this to v5.11. Should I add cc stable?
Yes, it hits 5.10.y.
> Luca, can I have your ack?
It'd be great if this fix goes in quickly.
BTW, I thought network people don't want to have Cc-to-stable in the
patch, so I didn't put it by myself. Is this rule still valid?
thanks,
Takashi
Re: [PATCH 2/2] iwlwifi: dbg: Mark ucode tlv data as const
On Tue, 12 Jan 2021 16:50:54 +0100, Kalle Valo wrote: > > Takashi Iwai writes: > > > The ucode TLV data may be read-only and should be treated as const > > pointers, but currently a few code forcibly cast to the writable > > pointer unnecessarily. This gave developers a wrong impression as if > > it can be modified, resulting in crashing regressions already a couple > > of times. > > > > This patch adds the const prefix to those cast pointers, so that such > > attempt can be caught more easily in future. > > > > Signed-off-by: Takashi Iwai > > So this need to go to -next, right? Yes, this isn't urgently needed for 5.11. > Does this depend on patch 1 or can > this be applied independently? It depends on the first patch, otherwise you'll get the warning in the code changing the const data (it must warn -- that's the purpose of this change :) So, if applying to a separate branch is difficult, applying together for 5.11 would be an option. thanks, Takashi
Re: [PATCH v1 2/2] isa: Make the remove callback for isa drivers return void
On Thu, 21 Jan 2021 21:48:12 +0100, Uwe Kleine-König wrote: > > The driver core ignores the return value of the remove callback, so > don't give isa drivers the chance to provide a value. > > Adapt all isa_drivers with a remove callbacks accordingly; they all > return 0 unconditionally anyhow. > > Signed-off-by: Uwe Kleine-König For the sound/* changes: Reviewed-by: Takashi Iwai BTW, how will we take the patches? Judging from the LOCs, sound/* are mostly affected, so I may merge them via sound.git tree, if other people have no objection. thanks, Takashi
Re: [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void
On Thu, 26 Nov 2020 17:59:50 +0100,
Uwe Kleine-König wrote:
>
> The driver core ignores the return value of struct device_driver::remove
> because there is only little that can be done. For the shutdown callback
> it's ps3_system_bus_shutdown() which ignores the return value.
>
> To simplify the quest to make struct device_driver::remove return void,
> let struct ps3_system_bus_driver::remove return void, too. All users
> already unconditionally return 0, this commit makes it obvious that
> returning an error code is a bad idea and ensures future users behave
> accordingly.
>
> Signed-off-by: Uwe Kleine-König
For the sound bit:
Acked-by: Takashi Iwai
thanks,
Takashi
> ---
> arch/powerpc/include/asm/ps3.h | 4 ++--
> arch/powerpc/platforms/ps3/system-bus.c | 5 ++---
> drivers/block/ps3disk.c | 3 +--
> drivers/block/ps3vram.c | 3 +--
> drivers/char/ps3flash.c | 3 +--
> drivers/net/ethernet/toshiba/ps3_gelic_net.c | 3 +--
> drivers/ps3/ps3-lpm.c| 3 +--
> drivers/ps3/ps3-vuart.c | 10 --
> drivers/scsi/ps3rom.c| 3 +--
> drivers/usb/host/ehci-ps3.c | 4 +---
> drivers/usb/host/ohci-ps3.c | 4 +---
> drivers/video/fbdev/ps3fb.c | 4 +---
> sound/ppc/snd_ps3.c | 3 +--
> 13 files changed, 18 insertions(+), 34 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ps3.h b/arch/powerpc/include/asm/ps3.h
> index cb89e4bf55ce..e646c7f218bc 100644
> --- a/arch/powerpc/include/asm/ps3.h
> +++ b/arch/powerpc/include/asm/ps3.h
> @@ -378,8 +378,8 @@ struct ps3_system_bus_driver {
> enum ps3_match_sub_id match_sub_id;
> struct device_driver core;
> int (*probe)(struct ps3_system_bus_device *);
> - int (*remove)(struct ps3_system_bus_device *);
> - int (*shutdown)(struct ps3_system_bus_device *);
> + void (*remove)(struct ps3_system_bus_device *);
> + void (*shutdown)(struct ps3_system_bus_device *);
> /* int (*suspend)(struct ps3_system_bus_device *, pm_message_t); */
> /* int (*resume)(struct ps3_system_bus_device *); */
> };
> diff --git a/arch/powerpc/platforms/ps3/system-bus.c
> b/arch/powerpc/platforms/ps3/system-bus.c
> index c62aaa29a9d5..b431f41c6cb5 100644
> --- a/arch/powerpc/platforms/ps3/system-bus.c
> +++ b/arch/powerpc/platforms/ps3/system-bus.c
> @@ -382,7 +382,6 @@ static int ps3_system_bus_probe(struct device *_dev)
>
> static int ps3_system_bus_remove(struct device *_dev)
> {
> - int result = 0;
> struct ps3_system_bus_device *dev = ps3_dev_to_system_bus_dev(_dev);
> struct ps3_system_bus_driver *drv;
>
> @@ -393,13 +392,13 @@ static int ps3_system_bus_remove(struct device *_dev)
> BUG_ON(!drv);
>
> if (drv->remove)
> - result = drv->remove(dev);
> + drv->remove(dev);
> else
> dev_dbg(&dev->core, "%s:%d %s: no remove method\n",
> __func__, __LINE__, drv->core.name);
>
> pr_debug(" <- %s:%d: %s\n", __func__, __LINE__, dev_name(&dev->core));
> - return result;
> + return 0;
> }
>
> static void ps3_system_bus_shutdown(struct device *_dev)
> diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
> index 7b55811c2a81..ba3ece56cbb3 100644
> --- a/drivers/block/ps3disk.c
> +++ b/drivers/block/ps3disk.c
> @@ -507,7 +507,7 @@ static int ps3disk_probe(struct ps3_system_bus_device
> *_dev)
> return error;
> }
>
> -static int ps3disk_remove(struct ps3_system_bus_device *_dev)
> +static void ps3disk_remove(struct ps3_system_bus_device *_dev)
> {
> struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
> struct ps3disk_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
> @@ -526,7 +526,6 @@ static int ps3disk_remove(struct ps3_system_bus_device
> *_dev)
> kfree(dev->bounce_buf);
> kfree(priv);
> ps3_system_bus_set_drvdata(_dev, NULL);
> - return 0;
> }
>
> static struct ps3_system_bus_driver ps3disk = {
> diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
> index 1088798c8dd0..b71d28372ef3 100644
> --- a/drivers/block/ps3vram.c
> +++ b/drivers/block/ps3vram.c
> @@ -797,7 +797,7 @@ static int ps3vram_probe(struct ps3_system_bus_device
> *dev)
> return error;
> }
>
> -static int ps3vram_remove(struct ps3_system_bus_device *dev)
> +static void ps3vram_remove(struct ps3_system_bus_device *dev)
> {
> struc
Re: [PATCH 1/2] ALSA: ppc: drop if block with always false condition
On Thu, 26 Nov 2020 17:59:49 +0100, Uwe Kleine-König wrote: > > The remove callback is only called for devices that were probed > successfully before. As the matching probe function cannot complete > without error if dev->match_id != PS3_MATCH_ID_SOUND, we don't have to > check this here. > > Signed-off-by: Uwe Kleine-König Applied this one now. Thanks. Takashi
Re: [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void
On Wed, 02 Dec 2020 13:14:06 +0100, Michael Ellerman wrote: > > Uwe Kleine-König writes: > > Hello Michael, > > > > On Sat, Nov 28, 2020 at 09:48:30AM +0100, Takashi Iwai wrote: > >> On Thu, 26 Nov 2020 17:59:50 +0100, > >> Uwe Kleine-König wrote: > >> > > >> > The driver core ignores the return value of struct device_driver::remove > >> > because there is only little that can be done. For the shutdown callback > >> > it's ps3_system_bus_shutdown() which ignores the return value. > >> > > >> > To simplify the quest to make struct device_driver::remove return void, > >> > let struct ps3_system_bus_driver::remove return void, too. All users > >> > already unconditionally return 0, this commit makes it obvious that > >> > returning an error code is a bad idea and ensures future users behave > >> > accordingly. > >> > > >> > Signed-off-by: Uwe Kleine-König > >> > >> For the sound bit: > >> Acked-by: Takashi Iwai > > > > assuming that you are the one who will apply this patch: Note that it > > depends on patch 1 that Takashi already applied to his tree. So you > > either have to wait untils patch 1 appears in some tree that you merge > > before applying, or you have to take patch 1, too. (With Takashi > > optinally dropping it then.) > > Thanks. I've picked up both patches. > > If Takashi doesn't want to rebase his tree to drop patch 1 that's OK, it > will just arrive in mainline via two paths, but git should handle it. Yeah, I'd like to avoid rebasing, so let's get it merge from both trees. git can handle such a case gracefully. thanks, Takashi
Re: [PATCH v2 0/3] isa: Make the remove callback for isa drivers return void
On Fri, 22 Jan 2021 10:24:46 +0100, Uwe Kleine-König wrote: > > Hello, > > changes since v1 > (https://lore.kernel.org/r/20210121204812.402589-1-...@kleine-koenig.org): > - fix build failure, found by the kernel test robot >(my grep missed the struct isa_driver embedded in struct >radio_isa_driver). For this I introduced patch 2 and adapted patch 3 >a bit. An interdiff is below. > - Added Acks/Reviews by Guenter Roeck, William Breathitt Gray, Marc > Kleine-Budde, Wolfram Sang and Takashi Iwai > > The base-commit I specified (by hand *sigh*) in v1 was broken, must be a > cut-n-paste-error, sorry for that. > > Takashi suggested to take this series via sound.git given that this is > the most affected tree. This is fine for me. Otherwise I can also > provide an immutable branch. For both variants we still need a few acks > though. Now I applied all three patches to my sound.git tree. It's found in isa-void-remove-callback signed tag, in case anyone else wants to merge in. The branch is based on 5.11-rc4. git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git tags/isa-void-remove-callback thanks, Takashi
Re: [alsa-devel] [PATCH 17/18] ALSA: hal2: pass struct device to DMA API functions
On Fri, 01 Feb 2019 09:48:00 +0100, Christoph Hellwig wrote: > > The DMA API generally relies on a struct device to work properly, and > only barely works without one for legacy reasons. Pass the easily > available struct device from the platform_device to remedy this. > > Signed-off-by: Christoph Hellwig Looks good to me: Reviewed-by: Takashi Iwai Shall I take this one through sound git tree or all through yours? thanks, Takashi
Re: [alsa-devel] [PATCH 18/18] ALSA: pass struct device to DMA API functions
On Fri, 01 Feb 2019 09:48:01 +0100, Christoph Hellwig wrote: > > The DMA API generally relies on a struct device to work properly, and > only barely works without one for legacy reasons. Pass the easily > available struct device from the platform_device to remedy this. > > Also use GFP_KERNEL instead of GFP_USER as the gfp_t for the memory > allocation, as we should treat this allocation as a normal kernel one. > > Signed-off-by: Christoph Hellwig Reviewed-by: Takashi Iwai thanks, Takashi
Re: [alsa-devel] don't pass a NULL struct device to DMA API functions
On Fri, 01 Feb 2019 09:47:43 +0100,
Christoph Hellwig wrote:
>
> We still have a few drivers which pass a NULL struct device pointer
> to DMA API functions, which generally is a bad idea as the API
> implementations rely on the device not only for ops selection, but
> also the dma mask and various other attributes.
>
> This series contains all easy conversions to pass a struct device,
> besides that there also is some arch code that needs separate handling,
> a driver that should not use the DMA API at all, and one that is
> a complete basket case to be deal with separately.
Actually there are a bunch of ISA sound drivers that still call
allocators with NULL device.
The patch below should address it, although it's only compile-tested.
thanks,
Takashi
-- 8< --
From: Takashi Iwai
Subject: [PATCH] ALSA: isa: Avoid passing NULL to memory allocators
We used to pass NULL to memory allocators for ISA devices due to
historical reasons. But we prefer rather a proper device object to be
assigned, so let's fix it by replacing snd_dma_isa_data() call with
card->dev reference, and kill snd_dma_isa_data() definition.
Signed-off-by: Takashi Iwai
---
Documentation/sound/kernel-api/writing-an-alsa-driver.rst | 10 +-
include/sound/memalloc.h | 1 -
sound/isa/ad1816a/ad1816a_lib.c | 2 +-
sound/isa/cmi8330.c | 2 +-
sound/isa/es1688/es1688_lib.c | 2 +-
sound/isa/es18xx.c| 2 +-
sound/isa/gus/gus_pcm.c | 4 ++--
sound/isa/sb/sb16_main.c | 2 +-
sound/isa/sb/sb8_main.c | 2 +-
sound/isa/sscape.c| 7 ---
sound/isa/wss/wss_lib.c | 2 +-
11 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst
b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst
index 7c2f2032d30a..6b154dbb02cc 100644
--- a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst
+++ b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst
@@ -3520,14 +3520,14 @@ allocator will try to get an area as large as possible
within the
given size.
The second argument (type) and the third argument (device pointer) are
-dependent on the bus. In the case of the ISA bus, pass
-:c:func:`snd_dma_isa_data()` as the third argument with
+dependent on the bus. For normal devices, pass the device pointer
+(typically identical as ``card->dev``) to the third argument with
``SNDRV_DMA_TYPE_DEV`` type. For the continuous buffer unrelated to the
bus can be pre-allocated with ``SNDRV_DMA_TYPE_CONTINUOUS`` type and the
``snd_dma_continuous_data(GFP_KERNEL)`` device pointer, where
-``GFP_KERNEL`` is the kernel allocation flag to use. For the PCI
-scatter-gather buffers, use ``SNDRV_DMA_TYPE_DEV_SG`` with
-``snd_dma_pci_data(pci)`` (see the `Non-Contiguous Buffers`_
+``GFP_KERNEL`` is the kernel allocation flag to use. For the
+scatter-gather buffers, use ``SNDRV_DMA_TYPE_DEV_SG`` with the device
+pointer (see the `Non-Contiguous Buffers`_
section).
Once the buffer is pre-allocated, you can use the allocator in the
diff --git a/include/sound/memalloc.h b/include/sound/memalloc.h
index af3fa577fa06..1ac0dd82a916 100644
--- a/include/sound/memalloc.h
+++ b/include/sound/memalloc.h
@@ -37,7 +37,6 @@ struct snd_dma_device {
};
#define snd_dma_pci_data(pci) (&(pci)->dev)
-#define snd_dma_isa_data() NULL
#define snd_dma_continuous_data(x) ((struct device *)(__force unsigned
long)(x))
diff --git a/sound/isa/ad1816a/ad1816a_lib.c b/sound/isa/ad1816a/ad1816a_lib.c
index 61e8c7e524db..94b381a78e9e 100644
--- a/sound/isa/ad1816a/ad1816a_lib.c
+++ b/sound/isa/ad1816a/ad1816a_lib.c
@@ -693,7 +693,7 @@ int snd_ad1816a_pcm(struct snd_ad1816a *chip, int device)
snd_ad1816a_init(chip);
snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV,
- snd_dma_isa_data(),
+ chip->card->dev,
64*1024, chip->dma1 > 3 ||
chip->dma2 > 3 ? 128*1024 : 64*1024);
chip->pcm = pcm;
diff --git a/sound/isa/cmi8330.c b/sound/isa/cmi8330.c
index 7e5aa06414c4..1868b73aa49c 100644
--- a/sound/isa/cmi8330.c
+++ b/sound/isa/cmi8330.c
@@ -470,7 +470,7 @@ static int snd_cmi8330_pcm(struct snd_card *card, struct
snd_cmi8330 *chip)
snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE,
&chip->streams[SNDRV_PCM_STREAM_CAPTURE].ops);
snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV,
- snd_dma_isa_data(),
+
Re: [alsa-devel] [PATCH 17/18] ALSA: hal2: pass struct device to DMA API functions
On Fri, 01 Feb 2019 17:09:06 +0100, Christoph Hellwig wrote: > > On Fri, Feb 01, 2019 at 02:12:45PM +0100, Takashi Iwai wrote: > > Shall I take this one through sound git tree or all through yours? > > Feel free to merge the sound bits through your tree! Alright, merged both patches 17 and 18 now. thanks, Takashi
Re: [alsa-devel] don't pass a NULL struct device to DMA API functions
On Fri, 01 Feb 2019 17:09:57 +0100, Christoph Hellwig wrote: > > On Fri, Feb 01, 2019 at 02:16:08PM +0100, Takashi Iwai wrote: > > Actually there are a bunch of ISA sound drivers that still call > > allocators with NULL device. > > > > The patch below should address it, although it's only compile-tested. > > Oh, I missed these "indirect" calls. This looks good to me: > > Reviewed-by: Christoph Hellwig OK, merged this one to for-next branch now as well. thanks, Takashi
Re: 2.6.15-rc6: known regressions in the kernel Bugzilla
At Wed, 21 Dec 2005 23:15:57 -0800, Greg KH wrote: > > On Thu, Dec 22, 2005 at 02:13:20AM +0100, Adrian Bunk wrote: > > The following bug in the kernel Bugzilla contains a regressions in > > 2.6.15-rc without a patch: > > - #5760 No sound with snd_intel8x0 & ALi M5455 chipset > > (kobject_register failed) > > We put code in the kobjet to report when callers fail, due to problems > in them, and the kobject code is blamed... > > Looks like a sound driver issue, nothing has changed in the kobject > code. But there is no relevant change in sound stuff between 2.6.15-rc4 and rc6 (except for minor fix of OSS drivers). If it really worked with 2.6.15-rc4, something else got broken. Takashi - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regression in iwlwifi: page fault in iwl_dbg_tlv_alloc_region() (commit ba8f6f4ae254)
On Tue, 12 Jan 2021 12:33:14 +0100,
Kalle Valo wrote:
>
> (adding luca)
>
> Michal Kubecek writes:
>
> > FYI, there is a regression in iwlwifi driver caused by commit
> > ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device memory")
> > reported at
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=210733
> > https://bugzilla.suse.com/show_bug.cgi?id=1180344
> >
> > The problem seems to be an attempt to write terminating null character
> > into a string which may be read only. There is also a proposed fix.
>
> Can someone submit a proper patch, please? See instructions below how to
> submit.
>
> And please add Fixes tag to the commit log:
>
> Fixes: ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device memory")
OK, I'll do it for my own, but really I hoped that someone would have
reacted on the bugzilla report before the official patch submission.
So far no one from the upstream devs showed interest in the bug at
all, unfortunately.
thanks,
Takashi
Re: regression in iwlwifi: page fault in iwl_dbg_tlv_alloc_region() (commit ba8f6f4ae254)
On Tue, 12 Jan 2021 13:45:33 +0100,
Kalle Valo wrote:
>
> Takashi Iwai writes:
>
> > On Tue, 12 Jan 2021 12:33:14 +0100,
> > Kalle Valo wrote:
> >>
> >> (adding luca)
> >>
> >> Michal Kubecek writes:
> >>
> >> > FYI, there is a regression in iwlwifi driver caused by commit
> >> > ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device memory")
> >> > reported at
> >> >
> >> > https://bugzilla.kernel.org/show_bug.cgi?id=210733
> >> > https://bugzilla.suse.com/show_bug.cgi?id=1180344
> >> >
> >> > The problem seems to be an attempt to write terminating null character
> >> > into a string which may be read only. There is also a proposed fix.
> >>
> >> Can someone submit a proper patch, please? See instructions below how to
> >> submit.
> >>
> >> And please add Fixes tag to the commit log:
> >>
> >> Fixes: ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device memory")
> >
> > OK, I'll do it for my own
>
> Thanks.
>
> > but really I hoped that someone would have reacted on the bugzilla
> > report before the official patch submission. So far no one from the
> > upstream devs showed interest in the bug at all, unfortunately.
>
> Bugzilla is problematic as I don't know if anyone tracks it actively, at
> least I don't have time for that. I recommend reporting all wireless
> issues to mailing lists to make sure everyone see it.
I share your feeling as a subsystem maintainer, but at the same time,
I see it's a big problem if the whole bugzilla reports are just
silently ignored. If it's a void, shouldn't we rather shut it down?
And, although a mailing list is fine to report via only some texts or
some patches, it's not suitable to attach large logs or such, which is
often essential for debugging. Thanks to lore, the archivability is
no longer a problem with ML recently, but the lack of data assignment
is another problem, IMO.
thanks,
Takashi
[PATCH 1/2] iwlwifi: dbg: Don't touch the tlv data
The commit ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device
memory") added a termination of name string just to be sure, and this
seems causing a regression, a GPF triggered at firmware loading.
Basically we shouldn't modify the firmware data that may be provided
as read-only.
This patch drops the code that caused the regression and keep the tlv
data as is.
Fixes: ba8f6f4ae254 ("iwlwifi: dbg: add dumping special device memory")
BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1180344
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=210733
Signed-off-by: Takashi Iwai
---
As an alternative fix, the debug print could be with the printf length
specifier to limit the size to IWL_FW_INIT_MAX_NAME, as found in the
bugzilla entries above, too. I chose a simpler (partial) revert here
instead.
drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
index a654147d3cd6..a80a35a7740f 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
@@ -180,13 +180,6 @@ static int iwl_dbg_tlv_alloc_region(struct iwl_trans
*trans,
if (le32_to_cpu(tlv->length) < sizeof(*reg))
return -EINVAL;
- /* For safe using a string from FW make sure we have a
-* null terminator
-*/
- reg->name[IWL_FW_INI_MAX_NAME - 1] = 0;
-
- IWL_DEBUG_FW(trans, "WRT: parsing region: %s\n", reg->name);
-
if (id >= IWL_FW_INI_MAX_REGION_ID) {
IWL_ERR(trans, "WRT: Invalid region id %u\n", id);
return -EINVAL;
--
2.26.2
[PATCH 2/2] iwlwifi: dbg: Mark ucode tlv data as const
The ucode TLV data may be read-only and should be treated as const
pointers, but currently a few code forcibly cast to the writable
pointer unnecessarily. This gave developers a wrong impression as if
it can be modified, resulting in crashing regressions already a couple
of times.
This patch adds the const prefix to those cast pointers, so that such
attempt can be caught more easily in future.
Signed-off-by: Takashi Iwai
---
.../net/wireless/intel/iwlwifi/iwl-dbg-tlv.c | 50 ++-
.../net/wireless/intel/iwlwifi/iwl-dbg-tlv.h | 2 +-
drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 2 +-
3 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
index a80a35a7740f..12c49fe8608a 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
@@ -61,7 +61,8 @@ dbg_ver_table[IWL_DBG_TLV_TYPE_NUM] = {
[IWL_DBG_TLV_TYPE_TRIGGER] = {.min_ver = 1, .max_ver = 1,},
};
-static int iwl_dbg_tlv_add(struct iwl_ucode_tlv *tlv, struct list_head *list)
+static int iwl_dbg_tlv_add(const struct iwl_ucode_tlv *tlv,
+ struct list_head *list)
{
u32 len = le32_to_cpu(tlv->length);
struct iwl_dbg_tlv_node *node;
@@ -76,9 +77,9 @@ static int iwl_dbg_tlv_add(struct iwl_ucode_tlv *tlv, struct
list_head *list)
return 0;
}
-static bool iwl_dbg_tlv_ver_support(struct iwl_ucode_tlv *tlv)
+static bool iwl_dbg_tlv_ver_support(const struct iwl_ucode_tlv *tlv)
{
- struct iwl_fw_ini_header *hdr = (void *)&tlv->data[0];
+ const struct iwl_fw_ini_header *hdr = (const void *)&tlv->data[0];
u32 type = le32_to_cpu(tlv->type);
u32 tlv_idx = type - IWL_UCODE_TLV_DEBUG_BASE;
u32 ver = le32_to_cpu(hdr->version);
@@ -91,9 +92,9 @@ static bool iwl_dbg_tlv_ver_support(struct iwl_ucode_tlv *tlv)
}
static int iwl_dbg_tlv_alloc_debug_info(struct iwl_trans *trans,
- struct iwl_ucode_tlv *tlv)
+ const struct iwl_ucode_tlv *tlv)
{
- struct iwl_fw_ini_debug_info_tlv *debug_info = (void *)tlv->data;
+ const struct iwl_fw_ini_debug_info_tlv *debug_info = (const void
*)tlv->data;
if (le32_to_cpu(tlv->length) != sizeof(*debug_info))
return -EINVAL;
@@ -105,9 +106,9 @@ static int iwl_dbg_tlv_alloc_debug_info(struct iwl_trans
*trans,
}
static int iwl_dbg_tlv_alloc_buf_alloc(struct iwl_trans *trans,
- struct iwl_ucode_tlv *tlv)
+ const struct iwl_ucode_tlv *tlv)
{
- struct iwl_fw_ini_allocation_tlv *alloc = (void *)tlv->data;
+ const struct iwl_fw_ini_allocation_tlv *alloc = (const void *)tlv->data;
u32 buf_location;
u32 alloc_id;
@@ -145,9 +146,9 @@ static int iwl_dbg_tlv_alloc_buf_alloc(struct iwl_trans
*trans,
}
static int iwl_dbg_tlv_alloc_hcmd(struct iwl_trans *trans,
- struct iwl_ucode_tlv *tlv)
+ const struct iwl_ucode_tlv *tlv)
{
- struct iwl_fw_ini_hcmd_tlv *hcmd = (void *)tlv->data;
+ const struct iwl_fw_ini_hcmd_tlv *hcmd = (const void *)tlv->data;
u32 tp = le32_to_cpu(hcmd->time_point);
if (le32_to_cpu(tlv->length) <= sizeof(*hcmd))
@@ -169,9 +170,9 @@ static int iwl_dbg_tlv_alloc_hcmd(struct iwl_trans *trans,
}
static int iwl_dbg_tlv_alloc_region(struct iwl_trans *trans,
- struct iwl_ucode_tlv *tlv)
+ const struct iwl_ucode_tlv *tlv)
{
- struct iwl_fw_ini_region_tlv *reg = (void *)tlv->data;
+ const struct iwl_fw_ini_region_tlv *reg = (const void *)tlv->data;
struct iwl_ucode_tlv **active_reg;
u32 id = le32_to_cpu(reg->id);
u32 type = le32_to_cpu(reg->type);
@@ -214,9 +215,10 @@ static int iwl_dbg_tlv_alloc_region(struct iwl_trans
*trans,
}
static int iwl_dbg_tlv_alloc_trigger(struct iwl_trans *trans,
-struct iwl_ucode_tlv *tlv)
+const struct iwl_ucode_tlv *tlv)
{
- struct iwl_fw_ini_trigger_tlv *trig = (void *)tlv->data;
+ const struct iwl_fw_ini_trigger_tlv *trig = (const void *)tlv->data;
+ struct iwl_fw_ini_trigger_tlv *dup_trig;
u32 tp = le32_to_cpu(trig->time_point);
struct iwl_ucode_tlv *dup = NULL;
int ret;
@@ -237,8 +239,8 @@ static int iwl_dbg_tlv_alloc_trigger(struct iwl_trans
*trans,
GFP_KERNEL);
if (!dup)
return -ENOMEM;
- trig = (void *)dup->data;
- trig->occurrences = cpu_to_le32(-1);
+ dup_trig = (void *)dup->
[PATCH 0/2] iwlwifi: Fix a crash at loading
Hi, this is the fix for the recently introduced regression of iwlwifi driver (since 5.10), a crash at the module loading [1][2]. The first patch actually fixes the bug, and this should go to 5.11-rc. The second patch is an enhancement to make pointers const for safety. Takashi [1] https://bugzilla.suse.com/show_bug.cgi?id=1180344 [2] https://bugzilla.kernel.org/show_bug.cgi?id=210733 === Takashi Iwai (2): iwlwifi: dbg: Don't touch the tlv data iwlwifi: dbg: Mark ucode tlv data as const .../net/wireless/intel/iwlwifi/iwl-dbg-tlv.c | 57 +-- .../net/wireless/intel/iwlwifi/iwl-dbg-tlv.h | 2 +- drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 2 +- 3 files changed, 28 insertions(+), 33 deletions(-) -- 2.26.2
Re: [PATCH 2/8] ALSA: pcm: use krealloc_array()
On Tue, 27 Oct 2020 13:17:19 +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski > > Use the helper that checks for overflows internally instead of manually > calculating the size of the new array. > > Signed-off-by: Bartosz Golaszewski Reviewed-by: Takashi Iwai thanks, Takashi
Re: [PATCH 0/3] Modernize tasklet callback API
On Tue, 11 Aug 2020 23:33:13 +0200, Kees Cook wrote: > > On Mon, Aug 03, 2020 at 02:16:15PM +0530, Allen wrote: > > Here's the series re-based on top of 5.8 > > https://github.com/allenpais/tasklets/tree/V3 > > Great! > > > Let me know how you would want these to be reviewed. > > Was a Coccinelle script used for any of these conversions? I wonder if > it'd be easier to do a single treewide patch for the more mechanical > changes. > > And, actually, I still think the "prepare" patches should just be > collapsed into the actual "covert" patches -- there are only a few. > > After those, yeah, I think getting these sent to their respective > maintainers is the next step. > > > Also, I was thinking if removing tasklets completely could be a task > > on KSPP wiki. If yes, I did like to take ownership of that task. I have a > > couple of ideas in mind, which could be discussed in a separate email. > > Sure! I will add it to the tracker. Here's for the refactoring: > https://github.com/KSPP/linux/issues/30 > > and here's for the removal: > https://github.com/KSPP/linux/issues/94 > > if you can added details/examples of how they should be removed, that'd > help other folks too, if they wanted to jump in. :) I have a patch set to convert the remaining tasklet usage in sound drivers to either the threaded IRQ or the work, but it wasn't submitted / merged for 5.8 due to the obvious conflict with your API changes. Each conversion is rather simple, but it's always a question of the nature of each tasklet usage which alternative is the best fit. FWIW, the current version is found in test/kill-tasklet branch of sound git tree git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git thanks, Takashi
Re: [net-next v4 10/12] ASoC: SOF: Introduce descriptors for SOF client
On Tue, 26 May 2020 15:15:26 +0200,
Pierre-Louis Bossart wrote:
>
>
>
> On 5/24/20 1:35 AM, Greg KH wrote:
> > On Sat, May 23, 2020 at 02:41:51PM -0500, Pierre-Louis Bossart wrote:
> >>
> >>
> >> On 5/23/20 1:23 AM, Greg KH wrote:
> >>> On Fri, May 22, 2020 at 09:29:57AM -0500, Pierre-Louis Bossart wrote:
> This is not an hypothetical case, we've had this recurring problem when a
> PCI device creates an audio card represented as a platform device. When
> the
> card registration fails, typically due to configuration issues, the PCI
> probe still completes.
> >>>
> >>> Then fix that problem there. The audio card should not be being created
> >>> as a platform device, as that is not what it is. And even if it was,
> >>> the probe should not complete, it should clean up after itself and error
> >>> out.
> >>
> >> Did you mean 'the PCI probe should not complete and error out'?
> >
> > Yes.
> >
> >> If yes, that's yet another problem... During the PCI probe, we start a
> >> workqueue and return success to avoid blocking everything.
> >
> > That's crazy.
> >
> >> And only 'later' do we actually create the card. So that's two levels
> >> of probe that cannot report a failure. I didn't come up with this
> >> design, IIRC this is due to audio-DRM dependencies and it's been used
> >> for 10+ years.
> >
> > Then if the probe function fails, it needs to unwind everything itself
> > and unregister the device with the PCI subsystem so that things work
> > properly. If it does not do that today, that's a bug.
> >
> > What kind of crazy dependencies cause this type of "requirement"?
>
> I think it is related to the request_module("i915") in
> snd_hdac_i915_init(), and possibly other firmware download.
>
> Adding Takashi for more details.
Right, there are a few levels of complexity there. The HD-audio
PCI controller driver, for example, is initialized in an async way
with a work. It loads the firmware files with
request_firmware_nowait() and also binds itself as a component master
with the DRM graphics driver via component framework.
Currently it has no way to unwind the PCI binding itself at the error
path, though. In theory it should be possible to unregister the PCI
from the driver itself in the work context, but it failed in the
earlier experiments, hence the driver sets itself in a disabled state
instead. Maybe worth to try again.
But, to be noted, all belonging sub-devices aren't instantiated but
deleted at the error path. Only the main PCI binding is kept in a
disabled state just as a place holder until it's unbound explicitly.
Takashi
Re: BUG: unable to handle kernel paging request in dqput
On Sat, 26 Sep 2020 22:48:15 +0200,
syzbot wrote:
>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit:98477740 Merge branch 'rcu/urgent' of git://git.kernel.org..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=1793087590
> kernel config: https://syzkaller.appspot.com/x/.config?x=af502ec9a451c9fc
> dashboard link: https://syzkaller.appspot.com/bug?extid=f816042a7ae2225f25ba
> compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/
> c2443155a0fb245c8f17f2c1c72b6ea391e86e81)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=133783ab90
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13bb597390
>
> The issue was bisected to:
>
> commit 1d0f953086f090a022f2c0e1448300c15372db46
> Author: Ioan-Adrian Ratiu
> Date: Wed Jan 4 22:37:46 2017 +
>
> ALSA: usb-audio: Fix irq/process data synchronization
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=133362c390
> final oops: https://syzkaller.appspot.com/x/report.txt?x=10b362c390
> console output: https://syzkaller.appspot.com/x/log.txt?x=173362c390
This commit looks really irrelevant from the Oops code path.
It must be a different reason.
thanks,
Takashi
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+f816042a7ae2225f2...@syzkaller.appspotmail.com
> Fixes: 1d0f953086f0 ("ALSA: usb-audio: Fix irq/process data synchronization")
>
> EXT4-fs (loop0): mounted filesystem without journal. Opts: ,errors=continue
> Quota error (device loop0): qtree_write_dquot: Error -1622674347 occurred
> while creating quota
> Quota error (device loop0): dq_insert_tree: Quota tree root isn't allocated!
> Quota error (device loop0): qtree_write_dquot: Error -5 occurred while
> creating quota
> BUG: unable to handle page fault for address: fbfff3e8feac
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x) - not-present page
> PGD 21ffe5067 P4D 21ffe5067 PUD 21ffe4067 PMD 0
> Oops: [#1] PREEMPT SMP KASAN
> CPU: 1 PID: 6845 Comm: syz-executor636 Not tainted 5.9.0-rc6-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:bytes_is_nonzero mm/kasan/generic.c:91 [inline]
> RIP: 0010:memory_is_nonzero mm/kasan/generic.c:108 [inline]
> RIP: 0010:memory_is_poisoned_n mm/kasan/generic.c:134 [inline]
> RIP: 0010:memory_is_poisoned mm/kasan/generic.c:165 [inline]
> RIP: 0010:check_memory_region_inline mm/kasan/generic.c:183 [inline]
> RIP: 0010:check_memory_region+0x80/0x2f0 mm/kasan/generic.c:192
> Code: 01 00 00 00 00 fc ff df 4d 01 ea 4d 89 d6 4d 29 ce 49 83 fe 10 7f 2d 4d
> 85 f6 0f 84 ab 01 00 00 4c 89 cb 4c 29 d3 0f 1f 40 00 <45> 0f b6 19 45 84 db
> 0f 85 f3 01 00 00 49 ff c1 48 ff c3 75 eb e9
> RSP: 0018:c90001fdf9d0 EFLAGS: 00010293
> RAX: 2234ff0efb3ccc01 RBX: fffe RCX: 81e00c97
> RDX: RSI: 0004 RDI: 9f47f565
> RBP: 9f47f455 R08: dc00 R09: fbfff3e8feac
> R10: fbfff3e8feae R11: R12: 13e8feac
> R13: dc01 R14: 0002 R15: 9f47f565
> FS: 009ac880() GS:8880ae90() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: fbfff3e8feac CR3: 9f30a000 CR4: 001506e0
> DR0: DR1: DR2:
> DR3: DR6: fffe0ff0 DR7: 0400
> Call Trace:
> instrument_atomic_read include/linux/instrumented.h:56 [inline]
> atomic_read include/asm-generic/atomic-instrumented.h:27 [inline]
> dqput+0x77/0x770 fs/quota/dquot.c:770
> dqput_all fs/quota/dquot.c:397 [inline]
> __dquot_initialize+0x9e6/0xc30 fs/quota/dquot.c:1530
> ext4_xattr_set+0x9b/0x300 fs/ext4/xattr.c:2474
> __vfs_setxattr+0x3be/0x400 fs/xattr.c:177
> __vfs_setxattr_noperm+0x11e/0x4b0 fs/xattr.c:208
> vfs_setxattr+0xde/0x270 fs/xattr.c:283
> setxattr+0x167/0x350 fs/xattr.c:548
> path_setxattr+0x109/0x1c0 fs/xattr.c:567
> __do_sys_setxattr fs/xattr.c:582 [inline]
> __se_sys_setxattr fs/xattr.c:578 [inline]
> __x64_sys_setxattr+0xb7/0xd0 fs/xattr.c:578
> do_syscall_64+0x31/0x70 arch/x86/entry/common.c:46
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x4447e9
> Code: 8d d7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48
> 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f
> 83 5b d7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:7ffcd4905408 EFLAGS: 0246 ORIG_RAX: 00bc
> RAX: ffda RBX: 0030656c69662f2e RCX: 004447e9
> RDX: RSI: 2140 RDI: 20c0
> RBP: 006cf018 R08: R09:
> R10: R11: 0246 R12: 004023d0
> R13: 00402460 R14: 000
