* [PATCH v2 00/10] ALSA: oxfw: code refactoring for quirks specific to ASICs
@ 2021-05-15 7:11 Takashi Sakamoto
2021-05-15 7:11 ` Takashi Sakamoto
` (9 more replies)
0 siblings, 10 replies; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-15 7:11 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, clemens
Hi,
This patchset is reveised version of my initial post:
* https://lore.kernel.org/alsa-devel/20210514085435.92807-1-o-takashi@sakamocchi.jp/
Changes from v1:
* revert Kconfig fix
* fix detection of mackie models
* add quirk flag for blocking transmission
Regards
Takashi Sakamoto (10):
Revert "ALSA: bebob/oxfw: fix Kconfig entry for Mackie d.2 Pro"
ALSA: oxfw: code refactoring for existent device entry with
specifier_id and version
ALSA: oxfw: code refactoring to detect mackie models
ALSA: oxfw: add explicit device entry for Loud Technologies Tapco
Link.FireWire 4x6
ALSA: oxfw: add explicit device entry for Loud Technologies Mackie
Onyx Sattelite
ALSA: oxfw: add comment for the type of ASICs
ALSA: oxfw: code refactoring for jumbo-payload quirk in OXFW970
ALSA: firewire-lib: code refactoring for jumbo payload quirk
ALSA: oxfw; code refactoring for wrong_dbs quirk
ALSA: oxfw: add quirk flag for blocking transmission method
sound/firewire/Kconfig | 4 +-
sound/firewire/amdtp-stream.c | 7 +-
sound/firewire/bebob/bebob.c | 2 +-
sound/firewire/oxfw/oxfw-stream.c | 25 +++---
sound/firewire/oxfw/oxfw.c | 137 ++++++++++++++----------------
sound/firewire/oxfw/oxfw.h | 12 ++-
6 files changed, 98 insertions(+), 89 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 01/10] Revert "ALSA: bebob/oxfw: fix Kconfig entry for Mackie d.2 Pro"
2021-05-15 7:11 [PATCH v2 00/10] ALSA: oxfw: code refactoring for quirks specific to ASICs Takashi Sakamoto
@ 2021-05-15 7:11 ` Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 02/10] ALSA: oxfw: code refactoring for existent device entry with specifier_id and version Takashi Sakamoto
` (8 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-15 7:11 UTC (permalink / raw)
To: tiwai; +Cc: clemens, alsa-devel, stable
This reverts commit 0edabdfe89581669609eaac5f6a8d0ae6fe95e7f.
I've explained that optional FireWire card for d.2 is also built-in to
d.2 Pro, however it's wrong. The optional card uses DM1000 ASIC and has
'Mackie DJ Mixer' in its model name of configuration ROM. On the other
hand, built-in FireWire card for d.2 Pro and d.4 Pro uses OXFW971 ASIC
and has 'd.Pro' in its model name according to manuals and user
experiences. The former card is not the card for d.2 Pro. They are similar
in appearance but different internally.
Fixes: 0edabdfe8958 ("ALSA: bebob/oxfw: fix Kconfig entry for Mackie d.2 Pro")
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/firewire/Kconfig | 4 ++--
sound/firewire/bebob/bebob.c | 2 +-
sound/firewire/oxfw/oxfw.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/firewire/Kconfig b/sound/firewire/Kconfig
index 9897bd26a438..def1f3d5ecf5 100644
--- a/sound/firewire/Kconfig
+++ b/sound/firewire/Kconfig
@@ -38,7 +38,7 @@ config SND_OXFW
* Mackie(Loud) Onyx 1640i (former model)
* Mackie(Loud) Onyx Satellite
* Mackie(Loud) Tapco Link.Firewire
- * Mackie(Loud) d.4 pro
+ * Mackie(Loud) d.2 pro/d.4 pro (built-in FireWire card with OXFW971 ASIC)
* Mackie(Loud) U.420/U.420d
* TASCAM FireOne
* Stanton Controllers & Systems 1 Deck/Mixer
@@ -84,7 +84,7 @@ config SND_BEBOB
* PreSonus FIREBOX/FIREPOD/FP10/Inspire1394
* BridgeCo RDAudio1/Audio5
* Mackie Onyx 1220/1620/1640 (FireWire I/O Card)
- * Mackie d.2 (FireWire Option) and d.2 Pro
+ * Mackie d.2 (optional FireWire card with DM1000 ASIC)
* Stanton FinalScratch 2 (ScratchAmp)
* Tascam IF-FW/DM
* Behringer XENIX UFX 1204/1604
diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c
index daeecfa8b9aa..90e98a6d1546 100644
--- a/sound/firewire/bebob/bebob.c
+++ b/sound/firewire/bebob/bebob.c
@@ -387,7 +387,7 @@ static const struct ieee1394_device_id bebob_id_table[] = {
SND_BEBOB_DEV_ENTRY(VEN_BRIDGECO, 0x00010049, &spec_normal),
/* Mackie, Onyx 1220/1620/1640 (Firewire I/O Card) */
SND_BEBOB_DEV_ENTRY(VEN_MACKIE2, 0x00010065, &spec_normal),
- // Mackie, d.2 (Firewire option card) and d.2 Pro (the card is built-in).
+ // Mackie, d.2 (optional Firewire card with DM1000).
SND_BEBOB_DEV_ENTRY(VEN_MACKIE1, 0x00010067, &spec_normal),
/* Stanton, ScratchAmp */
SND_BEBOB_DEV_ENTRY(VEN_STANTON, 0x00000001, &spec_normal),
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 9eea25c46dc7..5490637d278a 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -355,7 +355,7 @@ static const struct ieee1394_device_id oxfw_id_table[] = {
* Onyx-i series (former models): 0x081216
* Mackie Onyx Satellite: 0x00200f
* Tapco LINK.firewire 4x6: 0x000460
- * d.4 pro: Unknown
+ * d.2 pro/d.4 pro (built-in card): Unknown
* U.420: Unknown
* U.420d: Unknown
*/
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 01/10] Revert "ALSA: bebob/oxfw: fix Kconfig entry for Mackie d.2 Pro"
@ 2021-05-15 7:11 ` Takashi Sakamoto
0 siblings, 0 replies; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-15 7:11 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, clemens, stable
This reverts commit 0edabdfe89581669609eaac5f6a8d0ae6fe95e7f.
I've explained that optional FireWire card for d.2 is also built-in to
d.2 Pro, however it's wrong. The optional card uses DM1000 ASIC and has
'Mackie DJ Mixer' in its model name of configuration ROM. On the other
hand, built-in FireWire card for d.2 Pro and d.4 Pro uses OXFW971 ASIC
and has 'd.Pro' in its model name according to manuals and user
experiences. The former card is not the card for d.2 Pro. They are similar
in appearance but different internally.
Fixes: 0edabdfe8958 ("ALSA: bebob/oxfw: fix Kconfig entry for Mackie d.2 Pro")
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/firewire/Kconfig | 4 ++--
sound/firewire/bebob/bebob.c | 2 +-
sound/firewire/oxfw/oxfw.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/firewire/Kconfig b/sound/firewire/Kconfig
index 9897bd26a438..def1f3d5ecf5 100644
--- a/sound/firewire/Kconfig
+++ b/sound/firewire/Kconfig
@@ -38,7 +38,7 @@ config SND_OXFW
* Mackie(Loud) Onyx 1640i (former model)
* Mackie(Loud) Onyx Satellite
* Mackie(Loud) Tapco Link.Firewire
- * Mackie(Loud) d.4 pro
+ * Mackie(Loud) d.2 pro/d.4 pro (built-in FireWire card with OXFW971 ASIC)
* Mackie(Loud) U.420/U.420d
* TASCAM FireOne
* Stanton Controllers & Systems 1 Deck/Mixer
@@ -84,7 +84,7 @@ config SND_BEBOB
* PreSonus FIREBOX/FIREPOD/FP10/Inspire1394
* BridgeCo RDAudio1/Audio5
* Mackie Onyx 1220/1620/1640 (FireWire I/O Card)
- * Mackie d.2 (FireWire Option) and d.2 Pro
+ * Mackie d.2 (optional FireWire card with DM1000 ASIC)
* Stanton FinalScratch 2 (ScratchAmp)
* Tascam IF-FW/DM
* Behringer XENIX UFX 1204/1604
diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c
index daeecfa8b9aa..90e98a6d1546 100644
--- a/sound/firewire/bebob/bebob.c
+++ b/sound/firewire/bebob/bebob.c
@@ -387,7 +387,7 @@ static const struct ieee1394_device_id bebob_id_table[] = {
SND_BEBOB_DEV_ENTRY(VEN_BRIDGECO, 0x00010049, &spec_normal),
/* Mackie, Onyx 1220/1620/1640 (Firewire I/O Card) */
SND_BEBOB_DEV_ENTRY(VEN_MACKIE2, 0x00010065, &spec_normal),
- // Mackie, d.2 (Firewire option card) and d.2 Pro (the card is built-in).
+ // Mackie, d.2 (optional Firewire card with DM1000).
SND_BEBOB_DEV_ENTRY(VEN_MACKIE1, 0x00010067, &spec_normal),
/* Stanton, ScratchAmp */
SND_BEBOB_DEV_ENTRY(VEN_STANTON, 0x00000001, &spec_normal),
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 9eea25c46dc7..5490637d278a 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -355,7 +355,7 @@ static const struct ieee1394_device_id oxfw_id_table[] = {
* Onyx-i series (former models): 0x081216
* Mackie Onyx Satellite: 0x00200f
* Tapco LINK.firewire 4x6: 0x000460
- * d.4 pro: Unknown
+ * d.2 pro/d.4 pro (built-in card): Unknown
* U.420: Unknown
* U.420d: Unknown
*/
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 02/10] ALSA: oxfw: code refactoring for existent device entry with specifier_id and version
2021-05-15 7:11 [PATCH v2 00/10] ALSA: oxfw: code refactoring for quirks specific to ASICs Takashi Sakamoto
2021-05-15 7:11 ` Takashi Sakamoto
@ 2021-05-15 7:11 ` Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 03/10] ALSA: oxfw: code refactoring to detect mackie models Takashi Sakamoto
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-15 7:11 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, clemens
All of the devices known to be based on OXFW ASICs have the same layout
of configuration ROM, in which unit directory includes vendor, model,
specifier_id and version immediate values. Especially, the pair of
specifier_id and version is fixed to represent AV/C general protocol.
This commit refactors device entries to fulfil with these 4 elements.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/firewire/oxfw/oxfw.c | 82 ++++++++++++--------------------------
1 file changed, 25 insertions(+), 57 deletions(-)
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 5490637d278a..7be999c61730 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -320,36 +320,24 @@ static const struct compat_info lacie_speakers = {
.model_name = "FireWire Speakers",
};
+#define OXFW_DEV_ENTRY(vendor, model, data) \
+{ \
+ .match_flags = IEEE1394_MATCH_VENDOR_ID | \
+ IEEE1394_MATCH_MODEL_ID | \
+ IEEE1394_MATCH_SPECIFIER_ID | \
+ IEEE1394_MATCH_VERSION, \
+ .vendor_id = vendor, \
+ .model_id = model, \
+ .specifier_id = SPECIFIER_1394TA, \
+ .version = VERSION_AVC, \
+ .driver_data = (kernel_ulong_t)data, \
+}
+
static const struct ieee1394_device_id oxfw_id_table[] = {
- {
- .match_flags = IEEE1394_MATCH_VENDOR_ID |
- IEEE1394_MATCH_MODEL_ID |
- IEEE1394_MATCH_SPECIFIER_ID |
- IEEE1394_MATCH_VERSION,
- .vendor_id = VENDOR_GRIFFIN,
- .model_id = 0x00f970,
- .specifier_id = SPECIFIER_1394TA,
- .version = VERSION_AVC,
- .driver_data = (kernel_ulong_t)&griffin_firewave,
- },
- {
- .match_flags = IEEE1394_MATCH_VENDOR_ID |
- IEEE1394_MATCH_MODEL_ID |
- IEEE1394_MATCH_SPECIFIER_ID |
- IEEE1394_MATCH_VERSION,
- .vendor_id = VENDOR_LACIE,
- .model_id = 0x00f970,
- .specifier_id = SPECIFIER_1394TA,
- .version = VERSION_AVC,
- .driver_data = (kernel_ulong_t)&lacie_speakers,
- },
- /* Behringer,F-Control Audio 202 */
- {
- .match_flags = IEEE1394_MATCH_VENDOR_ID |
- IEEE1394_MATCH_MODEL_ID,
- .vendor_id = VENDOR_BEHRINGER,
- .model_id = 0x00fc22,
- },
+ OXFW_DEV_ENTRY(VENDOR_GRIFFIN, 0x00f970, &griffin_firewave),
+ OXFW_DEV_ENTRY(VENDOR_LACIE, 0x00f970, &lacie_speakers),
+ // Behringer,F-Control Audio 202.
+ OXFW_DEV_ENTRY(VENDOR_BEHRINGER, 0x00fc22, NULL),
/*
* Any Mackie(Loud) models (name string/model id):
* Onyx-i series (former models): 0x081216
@@ -367,34 +355,14 @@ static const struct ieee1394_device_id oxfw_id_table[] = {
.specifier_id = SPECIFIER_1394TA,
.version = VERSION_AVC,
},
- /* TASCAM, FireOne */
- {
- .match_flags = IEEE1394_MATCH_VENDOR_ID |
- IEEE1394_MATCH_MODEL_ID,
- .vendor_id = VENDOR_TASCAM,
- .model_id = 0x800007,
- },
- /* Stanton, Stanton Controllers & Systems 1 Mixer (SCS.1m) */
- {
- .match_flags = IEEE1394_MATCH_VENDOR_ID |
- IEEE1394_MATCH_MODEL_ID,
- .vendor_id = OUI_STANTON,
- .model_id = 0x001000,
- },
- /* Stanton, Stanton Controllers & Systems 1 Deck (SCS.1d) */
- {
- .match_flags = IEEE1394_MATCH_VENDOR_ID |
- IEEE1394_MATCH_MODEL_ID,
- .vendor_id = OUI_STANTON,
- .model_id = 0x002000,
- },
- // APOGEE, duet FireWire
- {
- .match_flags = IEEE1394_MATCH_VENDOR_ID |
- IEEE1394_MATCH_MODEL_ID,
- .vendor_id = OUI_APOGEE,
- .model_id = 0x01dddd,
- },
+ // TASCAM, FireOne.
+ OXFW_DEV_ENTRY(VENDOR_TASCAM, 0x800007, NULL),
+ // Stanton, Stanton Controllers & Systems 1 Mixer (SCS.1m).
+ OXFW_DEV_ENTRY(OUI_STANTON, 0x001000, NULL),
+ // Stanton, Stanton Controllers & Systems 1 Deck (SCS.1d).
+ OXFW_DEV_ENTRY(OUI_STANTON, 0x002000, NULL),
+ // APOGEE, duet FireWire.
+ OXFW_DEV_ENTRY(OUI_APOGEE, 0x01dddd, NULL),
{ }
};
MODULE_DEVICE_TABLE(ieee1394, oxfw_id_table);
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 03/10] ALSA: oxfw: code refactoring to detect mackie models
2021-05-15 7:11 [PATCH v2 00/10] ALSA: oxfw: code refactoring for quirks specific to ASICs Takashi Sakamoto
2021-05-15 7:11 ` Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 02/10] ALSA: oxfw: code refactoring for existent device entry with specifier_id and version Takashi Sakamoto
@ 2021-05-15 7:11 ` Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 04/10] ALSA: oxfw: add explicit device entry for Loud Technologies Tapco Link.FireWire 4x6 Takashi Sakamoto
` (6 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-15 7:11 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, clemens
This commit changes condition statement to call mackie models detection
just for the device entry. Additionally, comment is added for Onyx 1640i.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/firewire/oxfw/oxfw.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 7be999c61730..2af72951ebf8 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -245,7 +245,7 @@ static int oxfw_probe(struct fw_unit *unit,
{
struct snd_oxfw *oxfw;
- if (entry->vendor_id == VENDOR_LOUD && !detect_loud_models(unit))
+ if (entry->vendor_id == VENDOR_LOUD && entry->model_id == 0 && !detect_loud_models(unit))
return -ENODEV;
/* Allocate this independent of sound card instance. */
@@ -341,6 +341,7 @@ static const struct ieee1394_device_id oxfw_id_table[] = {
/*
* Any Mackie(Loud) models (name string/model id):
* Onyx-i series (former models): 0x081216
+ * Onyx 1640i: 0x001640
* Mackie Onyx Satellite: 0x00200f
* Tapco LINK.firewire 4x6: 0x000460
* d.2 pro/d.4 pro (built-in card): Unknown
@@ -352,6 +353,7 @@ static const struct ieee1394_device_id oxfw_id_table[] = {
IEEE1394_MATCH_SPECIFIER_ID |
IEEE1394_MATCH_VERSION,
.vendor_id = VENDOR_LOUD,
+ .model_id = 0,
.specifier_id = SPECIFIER_1394TA,
.version = VERSION_AVC,
},
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 04/10] ALSA: oxfw: add explicit device entry for Loud Technologies Tapco Link.FireWire 4x6
2021-05-15 7:11 [PATCH v2 00/10] ALSA: oxfw: code refactoring for quirks specific to ASICs Takashi Sakamoto
` (2 preceding siblings ...)
2021-05-15 7:11 ` [PATCH v2 03/10] ALSA: oxfw: code refactoring to detect mackie models Takashi Sakamoto
@ 2021-05-15 7:11 ` Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 05/10] ALSA: oxfw: add explicit device entry for Loud Technologies Mackie Onyx Sattelite Takashi Sakamoto
` (5 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-15 7:11 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, clemens
Loud Technologies Tapco Link.FireWire 4x6 is identified as the model
with OXFW970 ASIC.
This commit adds explicit entry for the model.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/firewire/oxfw/oxfw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 2af72951ebf8..998f0da6fd5b 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -47,7 +47,6 @@ static bool detect_loud_models(struct fw_unit *unit)
"Onyx 1640i",
"d.Pro",
"Mackie Onyx Satellite",
- "Tapco LINK.firewire 4x6",
"U.420"};
char model[32];
int err;
@@ -338,12 +337,13 @@ static const struct ieee1394_device_id oxfw_id_table[] = {
OXFW_DEV_ENTRY(VENDOR_LACIE, 0x00f970, &lacie_speakers),
// Behringer,F-Control Audio 202.
OXFW_DEV_ENTRY(VENDOR_BEHRINGER, 0x00fc22, NULL),
+ // Loud Technologies, Tapco Link.FireWire 4x6.
+ OXFW_DEV_ENTRY(VENDOR_LOUD, 0x000460, NULL),
/*
* Any Mackie(Loud) models (name string/model id):
* Onyx-i series (former models): 0x081216
* Onyx 1640i: 0x001640
* Mackie Onyx Satellite: 0x00200f
- * Tapco LINK.firewire 4x6: 0x000460
* d.2 pro/d.4 pro (built-in card): Unknown
* U.420: Unknown
* U.420d: Unknown
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 05/10] ALSA: oxfw: add explicit device entry for Loud Technologies Mackie Onyx Sattelite
2021-05-15 7:11 [PATCH v2 00/10] ALSA: oxfw: code refactoring for quirks specific to ASICs Takashi Sakamoto
` (3 preceding siblings ...)
2021-05-15 7:11 ` [PATCH v2 04/10] ALSA: oxfw: add explicit device entry for Loud Technologies Tapco Link.FireWire 4x6 Takashi Sakamoto
@ 2021-05-15 7:11 ` Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 06/10] ALSA: oxfw: add comment for the type of ASICs Takashi Sakamoto
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-15 7:11 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, clemens
Loud Technologies Mackie Onyx Satellite is identified as the model with
OXFW970 ASIC.
This commit adds explicit entry for the model.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/firewire/oxfw/oxfw.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 998f0da6fd5b..e851149c5280 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -46,7 +46,6 @@ static bool detect_loud_models(struct fw_unit *unit)
"Onyx-i",
"Onyx 1640i",
"d.Pro",
- "Mackie Onyx Satellite",
"U.420"};
char model[32];
int err;
@@ -339,15 +338,14 @@ static const struct ieee1394_device_id oxfw_id_table[] = {
OXFW_DEV_ENTRY(VENDOR_BEHRINGER, 0x00fc22, NULL),
// Loud Technologies, Tapco Link.FireWire 4x6.
OXFW_DEV_ENTRY(VENDOR_LOUD, 0x000460, NULL),
- /*
- * Any Mackie(Loud) models (name string/model id):
- * Onyx-i series (former models): 0x081216
- * Onyx 1640i: 0x001640
- * Mackie Onyx Satellite: 0x00200f
- * d.2 pro/d.4 pro (built-in card): Unknown
- * U.420: Unknown
- * U.420d: Unknown
- */
+ // Loud Technologies, Mackie Onyx Satellite.
+ OXFW_DEV_ENTRY(VENDOR_LOUD, MODEL_SATELLITE, NULL),
+ // Any Mackie(Loud) models (name string/model id):
+ // Onyx-i series (former models): 0x081216
+ // Onyx 1640i: 0x001640
+ // d.2 pro/d.4 pro (built-in card): Unknown
+ // U.420: Unknown
+ // U.420d: Unknown
{
.match_flags = IEEE1394_MATCH_VENDOR_ID |
IEEE1394_MATCH_SPECIFIER_ID |
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 06/10] ALSA: oxfw: add comment for the type of ASICs
2021-05-15 7:11 [PATCH v2 00/10] ALSA: oxfw: code refactoring for quirks specific to ASICs Takashi Sakamoto
` (4 preceding siblings ...)
2021-05-15 7:11 ` [PATCH v2 05/10] ALSA: oxfw: add explicit device entry for Loud Technologies Mackie Onyx Sattelite Takashi Sakamoto
@ 2021-05-15 7:11 ` Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 07/10] ALSA: oxfw: code refactoring for jumbo-payload quirk in OXFW970 Takashi Sakamoto
` (3 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-15 7:11 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, clemens
ALSA OXFW supports two types of ASICS; OXFW970 and OXFW971. The former
is known to have a quirk we call 'jumbo payload' that some isochronous
cycles are skipped to transfer isochronous packets during handling
asynchronous transaction. The quirk seems to correspond to firmware
initially delivered by Oxford Semiconductor since the quirk is not
confirmed for Mackie Onyx Satellite in which the revised firmware is
available. The quirk is not confirmed in the latter.
This commit adds code comment to describe the quirk.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/firewire/oxfw/oxfw.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index e851149c5280..9a9c84bc811a 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -332,14 +332,30 @@ static const struct compat_info lacie_speakers = {
}
static const struct ieee1394_device_id oxfw_id_table[] = {
+ //
+ // OXFW970 devices:
+ // Initial firmware has a quirk to postpone isoc packet transmission during finishing async
+ // transaction. As a result, several isochronous cycles are skipped to transfer the packets
+ // and the audio data frames which should have been transferred during the cycles are put
+ // into packet at the first isoc cycle after the postpone. Furthermore, the value of SYT
+ // field in CIP header is not reliable as synchronization timing,
+ //
OXFW_DEV_ENTRY(VENDOR_GRIFFIN, 0x00f970, &griffin_firewave),
OXFW_DEV_ENTRY(VENDOR_LACIE, 0x00f970, &lacie_speakers),
- // Behringer,F-Control Audio 202.
+ // Behringer,F-Control Audio 202. The value of SYT field is not reliable at all.
OXFW_DEV_ENTRY(VENDOR_BEHRINGER, 0x00fc22, NULL),
- // Loud Technologies, Tapco Link.FireWire 4x6.
+ // Loud Technologies, Tapco Link.FireWire 4x6. The value of SYT field is always 0xffff.
OXFW_DEV_ENTRY(VENDOR_LOUD, 0x000460, NULL),
- // Loud Technologies, Mackie Onyx Satellite.
+ // Loud Technologies, Mackie Onyx Satellite. Although revised version of firmware is
+ // installed to avoid the postpone, the value of SYT field is always 0xffff.
OXFW_DEV_ENTRY(VENDOR_LOUD, MODEL_SATELLITE, NULL),
+ // Miglia HarmonyAudio. Not yet identified.
+
+ //
+ // OXFW971 devices:
+ // The value of SYT field in CIP header is enough reliable. Both of blocking and non-blocking
+ // transmission methods are available.
+ //
// Any Mackie(Loud) models (name string/model id):
// Onyx-i series (former models): 0x081216
// Onyx 1640i: 0x001640
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 07/10] ALSA: oxfw: code refactoring for jumbo-payload quirk in OXFW970
2021-05-15 7:11 [PATCH v2 00/10] ALSA: oxfw: code refactoring for quirks specific to ASICs Takashi Sakamoto
` (5 preceding siblings ...)
2021-05-15 7:11 ` [PATCH v2 06/10] ALSA: oxfw: add comment for the type of ASICs Takashi Sakamoto
@ 2021-05-15 7:11 ` Takashi Sakamoto
2021-05-17 9:11 ` Takashi Iwai
2021-05-15 7:11 ` [PATCH v2 08/10] ALSA: firewire-lib: code refactoring for jumbo payload quirk Takashi Sakamoto
` (2 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-15 7:11 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, clemens
This commit adds enumeration to describe quirks of OXFW ASICs.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/firewire/oxfw/oxfw-stream.c | 20 +++++++-------------
sound/firewire/oxfw/oxfw.c | 3 +++
sound/firewire/oxfw/oxfw.h | 7 +++++++
3 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/sound/firewire/oxfw/oxfw-stream.c b/sound/firewire/oxfw/oxfw-stream.c
index 80c9dc13f1b5..33a7d0f308f1 100644
--- a/sound/firewire/oxfw/oxfw-stream.c
+++ b/sound/firewire/oxfw/oxfw-stream.c
@@ -153,12 +153,18 @@ static int init_stream(struct snd_oxfw *oxfw, struct amdtp_stream *stream)
struct cmp_connection *conn;
enum cmp_direction c_dir;
enum amdtp_stream_direction s_dir;
+ enum cip_flags flags = CIP_NONBLOCKING;
int err;
if (stream == &oxfw->tx_stream) {
conn = &oxfw->out_conn;
c_dir = CMP_OUTPUT;
s_dir = AMDTP_IN_STREAM;
+
+ if (oxfw->quirks & SND_OXFW_QUIRK_JUMBO_PAYLOAD)
+ flags |= CIP_JUMBO_PAYLOAD;
+ if (oxfw->wrong_dbs)
+ flags |= CIP_WRONG_DBS;
} else {
conn = &oxfw->in_conn;
c_dir = CMP_INPUT;
@@ -169,24 +175,12 @@ static int init_stream(struct snd_oxfw *oxfw, struct amdtp_stream *stream)
if (err < 0)
return err;
- err = amdtp_am824_init(stream, oxfw->unit, s_dir, CIP_NONBLOCKING);
+ err = amdtp_am824_init(stream, oxfw->unit, s_dir, flags);
if (err < 0) {
cmp_connection_destroy(conn);
return err;
}
- /*
- * OXFW starts to transmit packets with non-zero dbc.
- * OXFW postpone transferring packets till handling any asynchronous
- * packets. As a result, next isochronous packet includes more data
- * blocks than IEC 61883-6 defines.
- */
- if (stream == &oxfw->tx_stream) {
- oxfw->tx_stream.flags |= CIP_JUMBO_PAYLOAD;
- if (oxfw->wrong_dbs)
- oxfw->tx_stream.flags |= CIP_WRONG_DBS;
- }
-
return 0;
}
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 9a9c84bc811a..90a66e1312fe 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -86,6 +86,9 @@ static int name_card(struct snd_oxfw *oxfw)
goto end;
be32_to_cpus(&firmware);
+ if (firmware >> 20 == 0x970)
+ oxfw->quirks |= SND_OXFW_QUIRK_JUMBO_PAYLOAD;
+
/* to apply card definitions */
if (oxfw->entry->vendor_id == VENDOR_GRIFFIN ||
oxfw->entry->vendor_id == VENDOR_LACIE) {
diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h
index fa2d7f9e2dc3..9e1c12546ab5 100644
--- a/sound/firewire/oxfw/oxfw.h
+++ b/sound/firewire/oxfw/oxfw.h
@@ -32,6 +32,12 @@
#include "../amdtp-am824.h"
#include "../cmp.h"
+enum snd_oxfw_quirk {
+ // Postpone transferring packets during handling asynchronous transaction. As a result,
+ // next isochronous packet includes more events than one packet can include.
+ SND_OXFW_QUIRK_JUMBO_PAYLOAD = 0x01,
+};
+
/* This is an arbitrary number for convinience. */
#define SND_OXFW_STREAM_FORMAT_ENTRIES 10
struct snd_oxfw {
@@ -43,6 +49,7 @@ struct snd_oxfw {
bool registered;
struct delayed_work dwork;
+ enum snd_oxfw_quirk quirks;
bool wrong_dbs;
bool has_output;
bool has_input;
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 08/10] ALSA: firewire-lib: code refactoring for jumbo payload quirk
2021-05-15 7:11 [PATCH v2 00/10] ALSA: oxfw: code refactoring for quirks specific to ASICs Takashi Sakamoto
` (6 preceding siblings ...)
2021-05-15 7:11 ` [PATCH v2 07/10] ALSA: oxfw: code refactoring for jumbo-payload quirk in OXFW970 Takashi Sakamoto
@ 2021-05-15 7:11 ` Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 09/10] ALSA: oxfw; code refactoring for wrong_dbs quirk Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 10/10] ALSA: oxfw: add quirk flag for blocking transmission method Takashi Sakamoto
9 siblings, 0 replies; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-15 7:11 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, clemens
A new macro is added to describe the maximum number of cycles to accept
cycle skip by jumbo payload quirk.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/firewire/amdtp-stream.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index e0faa6601966..d78f86e12e76 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -64,6 +64,11 @@
#define IT_PKT_HEADER_SIZE_CIP 8 // For 2 CIP header.
#define IT_PKT_HEADER_SIZE_NO_CIP 0 // Nothing.
+// The initial firmware of OXFW970 can postpone transmission of packet during finishing
+// asynchronous transaction. This module accepts 5 cycles to skip as maximum to avoid buffer
+// overrun. Actual device can skip more, then this module stops the packet streaming.
+#define IR_JUMBO_PAYLOAD_MAX_SKIP_CYCLES 5
+
static void pcm_period_work(struct work_struct *work);
/**
@@ -316,7 +321,7 @@ unsigned int amdtp_stream_get_max_payload(struct amdtp_stream *s)
unsigned int cip_header_size = 0;
if (s->flags & CIP_JUMBO_PAYLOAD)
- multiplier = 5;
+ multiplier = IR_JUMBO_PAYLOAD_MAX_SKIP_CYCLES;
if (!(s->flags & CIP_NO_HEADER))
cip_header_size = sizeof(__be32) * 2;
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 09/10] ALSA: oxfw; code refactoring for wrong_dbs quirk
2021-05-15 7:11 [PATCH v2 00/10] ALSA: oxfw: code refactoring for quirks specific to ASICs Takashi Sakamoto
` (7 preceding siblings ...)
2021-05-15 7:11 ` [PATCH v2 08/10] ALSA: firewire-lib: code refactoring for jumbo payload quirk Takashi Sakamoto
@ 2021-05-15 7:11 ` Takashi Sakamoto
2021-05-17 9:11 ` Takashi Iwai
2021-05-15 7:11 ` [PATCH v2 10/10] ALSA: oxfw: add quirk flag for blocking transmission method Takashi Sakamoto
9 siblings, 1 reply; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-15 7:11 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, clemens
A new entry is added to the quirk enumeration for wrong_dbs quirk to
obsolete structure member.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/firewire/oxfw/oxfw-stream.c | 2 +-
sound/firewire/oxfw/oxfw.c | 2 +-
sound/firewire/oxfw/oxfw.h | 3 ++-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/firewire/oxfw/oxfw-stream.c b/sound/firewire/oxfw/oxfw-stream.c
index 33a7d0f308f1..9edd8515f586 100644
--- a/sound/firewire/oxfw/oxfw-stream.c
+++ b/sound/firewire/oxfw/oxfw-stream.c
@@ -163,7 +163,7 @@ static int init_stream(struct snd_oxfw *oxfw, struct amdtp_stream *stream)
if (oxfw->quirks & SND_OXFW_QUIRK_JUMBO_PAYLOAD)
flags |= CIP_JUMBO_PAYLOAD;
- if (oxfw->wrong_dbs)
+ if (oxfw->quirks & SND_OXFW_QUIRK_WRONG_DBS)
flags |= CIP_WRONG_DBS;
} else {
conn = &oxfw->in_conn;
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 90a66e1312fe..966697dace47 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -176,7 +176,7 @@ static int detect_quirks(struct snd_oxfw *oxfw)
* value in 'dbs' field of CIP header against its format information.
*/
if (vendor == VENDOR_LOUD && model == MODEL_SATELLITE)
- oxfw->wrong_dbs = true;
+ oxfw->quirks |= SND_OXFW_QUIRK_WRONG_DBS;
return 0;
}
diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h
index 9e1c12546ab5..1b0c53802569 100644
--- a/sound/firewire/oxfw/oxfw.h
+++ b/sound/firewire/oxfw/oxfw.h
@@ -36,6 +36,8 @@ enum snd_oxfw_quirk {
// Postpone transferring packets during handling asynchronous transaction. As a result,
// next isochronous packet includes more events than one packet can include.
SND_OXFW_QUIRK_JUMBO_PAYLOAD = 0x01,
+ // The dbs field of CIP header in tx packet is wrong.
+ SND_OXFW_QUIRK_WRONG_DBS = 0x02,
};
/* This is an arbitrary number for convinience. */
@@ -50,7 +52,6 @@ struct snd_oxfw {
struct delayed_work dwork;
enum snd_oxfw_quirk quirks;
- bool wrong_dbs;
bool has_output;
bool has_input;
u8 *tx_stream_formats[SND_OXFW_STREAM_FORMAT_ENTRIES];
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 10/10] ALSA: oxfw: add quirk flag for blocking transmission method
2021-05-15 7:11 [PATCH v2 00/10] ALSA: oxfw: code refactoring for quirks specific to ASICs Takashi Sakamoto
` (8 preceding siblings ...)
2021-05-15 7:11 ` [PATCH v2 09/10] ALSA: oxfw; code refactoring for wrong_dbs quirk Takashi Sakamoto
@ 2021-05-15 7:11 ` Takashi Sakamoto
9 siblings, 0 replies; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-15 7:11 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, clemens
Stanton SCS.1m and Apogee Duet FireWire use blocking transmission method
unlike the other models.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/firewire/oxfw/oxfw-stream.c | 7 ++++++-
sound/firewire/oxfw/oxfw.c | 14 +++++++++++---
sound/firewire/oxfw/oxfw.h | 2 ++
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/sound/firewire/oxfw/oxfw-stream.c b/sound/firewire/oxfw/oxfw-stream.c
index 9edd8515f586..b17ebf09fbf9 100644
--- a/sound/firewire/oxfw/oxfw-stream.c
+++ b/sound/firewire/oxfw/oxfw-stream.c
@@ -153,9 +153,14 @@ static int init_stream(struct snd_oxfw *oxfw, struct amdtp_stream *stream)
struct cmp_connection *conn;
enum cmp_direction c_dir;
enum amdtp_stream_direction s_dir;
- enum cip_flags flags = CIP_NONBLOCKING;
+ enum cip_flags flags;
int err;
+ if (!(oxfw->quirks & SND_OXFW_QUIRK_BLOCKING_TRANSMISSION))
+ flags = CIP_NONBLOCKING;
+ else
+ flags = CIP_BLOCKING;
+
if (stream == &oxfw->tx_stream) {
conn = &oxfw->out_conn;
c_dir = CMP_OUTPUT;
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 966697dace47..59bffa32636c 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -23,6 +23,8 @@
#define OUI_APOGEE 0x0003db
#define MODEL_SATELLITE 0x00200f
+#define MODEL_SCS1M 0x001000
+#define MODEL_DUET_FW 0x01dddd
#define SPECIFIER_1394TA 0x00a02d
#define VERSION_AVC 0x010001
@@ -144,13 +146,19 @@ static int detect_quirks(struct snd_oxfw *oxfw)
* messages.
*/
if (oxfw->entry->vendor_id == OUI_STANTON) {
- /* No physical MIDI ports. */
+ if (oxfw->entry->model_id == MODEL_SCS1M)
+ oxfw->quirks |= SND_OXFW_QUIRK_BLOCKING_TRANSMISSION;
+
+ // No physical MIDI ports.
oxfw->midi_input_ports = 0;
oxfw->midi_output_ports = 0;
return snd_oxfw_scs1x_add(oxfw);
}
+ if (oxfw->entry->vendor_id == OUI_APOGEE && oxfw->entry->model_id == MODEL_DUET_FW)
+ oxfw->quirks |= SND_OXFW_QUIRK_BLOCKING_TRANSMISSION;
+
/*
* TASCAM FireOne has physical control and requires a pair of additional
* MIDI ports.
@@ -377,11 +385,11 @@ static const struct ieee1394_device_id oxfw_id_table[] = {
// TASCAM, FireOne.
OXFW_DEV_ENTRY(VENDOR_TASCAM, 0x800007, NULL),
// Stanton, Stanton Controllers & Systems 1 Mixer (SCS.1m).
- OXFW_DEV_ENTRY(OUI_STANTON, 0x001000, NULL),
+ OXFW_DEV_ENTRY(OUI_STANTON, MODEL_SCS1M, NULL),
// Stanton, Stanton Controllers & Systems 1 Deck (SCS.1d).
OXFW_DEV_ENTRY(OUI_STANTON, 0x002000, NULL),
// APOGEE, duet FireWire.
- OXFW_DEV_ENTRY(OUI_APOGEE, 0x01dddd, NULL),
+ OXFW_DEV_ENTRY(OUI_APOGEE, MODEL_DUET_FW, NULL),
{ }
};
MODULE_DEVICE_TABLE(ieee1394, oxfw_id_table);
diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h
index 1b0c53802569..6a0fe0fa2f1a 100644
--- a/sound/firewire/oxfw/oxfw.h
+++ b/sound/firewire/oxfw/oxfw.h
@@ -38,6 +38,8 @@ enum snd_oxfw_quirk {
SND_OXFW_QUIRK_JUMBO_PAYLOAD = 0x01,
// The dbs field of CIP header in tx packet is wrong.
SND_OXFW_QUIRK_WRONG_DBS = 0x02,
+ // Blocking transmission mode is used.
+ SND_OXFW_QUIRK_BLOCKING_TRANSMISSION = 0x04,
};
/* This is an arbitrary number for convinience. */
--
2.27.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 07/10] ALSA: oxfw: code refactoring for jumbo-payload quirk in OXFW970
2021-05-15 7:11 ` [PATCH v2 07/10] ALSA: oxfw: code refactoring for jumbo-payload quirk in OXFW970 Takashi Sakamoto
@ 2021-05-17 9:11 ` Takashi Iwai
2021-05-18 1:45 ` Takashi Sakamoto
0 siblings, 1 reply; 15+ messages in thread
From: Takashi Iwai @ 2021-05-17 9:11 UTC (permalink / raw)
To: Takashi Sakamoto; +Cc: alsa-devel, clemens
On Sat, 15 May 2021 09:11:09 +0200,
Takashi Sakamoto wrote:
> --- a/sound/firewire/oxfw/oxfw.h
> +++ b/sound/firewire/oxfw/oxfw.h
> @@ -32,6 +32,12 @@
> #include "../amdtp-am824.h"
> #include "../cmp.h"
>
> +enum snd_oxfw_quirk {
> + // Postpone transferring packets during handling asynchronous transaction. As a result,
> + // next isochronous packet includes more events than one packet can include.
> + SND_OXFW_QUIRK_JUMBO_PAYLOAD = 0x01,
> +};
> +
> /* This is an arbitrary number for convinience. */
> #define SND_OXFW_STREAM_FORMAT_ENTRIES 10
> struct snd_oxfw {
> @@ -43,6 +49,7 @@ struct snd_oxfw {
> bool registered;
> struct delayed_work dwork;
>
> + enum snd_oxfw_quirk quirks;
Declaring the field as this enum type for bit flags isn't really
right, IMO. Usually an enum *type* is used for storing only the
enumerated values as-is, but the actual code (in a later patch) stores
the combination of the defined values as bits.
That is, if a field is defined with an enum type, readers and
compilers may believe that only the defined values are stored there,
while the code doesn't follow that, which is a confusing situation.
I see that a similar pattern is used already in the firewire code, but
I don't think this justifies to introduce it to yet another place.
thanks,
Takashi
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 09/10] ALSA: oxfw; code refactoring for wrong_dbs quirk
2021-05-15 7:11 ` [PATCH v2 09/10] ALSA: oxfw; code refactoring for wrong_dbs quirk Takashi Sakamoto
@ 2021-05-17 9:11 ` Takashi Iwai
0 siblings, 0 replies; 15+ messages in thread
From: Takashi Iwai @ 2021-05-17 9:11 UTC (permalink / raw)
To: Takashi Sakamoto; +Cc: alsa-devel, clemens
On Sat, 15 May 2021 09:11:11 +0200,
Takashi Sakamoto wrote:
>
> A new entry is added to the quirk enumeration for wrong_dbs quirk to
> obsolete structure member.
>
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Just a nitpick: the subject is wrongly with a semicolon instead of a
colon.
thanks,
Takashi
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 07/10] ALSA: oxfw: code refactoring for jumbo-payload quirk in OXFW970
2021-05-17 9:11 ` Takashi Iwai
@ 2021-05-18 1:45 ` Takashi Sakamoto
0 siblings, 0 replies; 15+ messages in thread
From: Takashi Sakamoto @ 2021-05-18 1:45 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, clemens
On Mon, May 17, 2021 at 11:11:07AM +0200, Takashi Iwai wrote:
> On Sat, 15 May 2021 09:11:09 +0200,
> Takashi Sakamoto wrote:
> > --- a/sound/firewire/oxfw/oxfw.h
> > +++ b/sound/firewire/oxfw/oxfw.h
> > @@ -32,6 +32,12 @@
> > #include "../amdtp-am824.h"
> > #include "../cmp.h"
> >
> > +enum snd_oxfw_quirk {
> > + // Postpone transferring packets during handling asynchronous transaction. As a result,
> > + // next isochronous packet includes more events than one packet can include.
> > + SND_OXFW_QUIRK_JUMBO_PAYLOAD = 0x01,
> > +};
> > +
> > /* This is an arbitrary number for convinience. */
> > #define SND_OXFW_STREAM_FORMAT_ENTRIES 10
> > struct snd_oxfw {
> > @@ -43,6 +49,7 @@ struct snd_oxfw {
> > bool registered;
> > struct delayed_work dwork;
> >
> > + enum snd_oxfw_quirk quirks;
>
> Declaring the field as this enum type for bit flags isn't really
> right, IMO. Usually an enum *type* is used for storing only the
> enumerated values as-is, but the actual code (in a later patch) stores
> the combination of the defined values as bits.
> That is, if a field is defined with an enum type, readers and
> compilers may believe that only the defined values are stored there,
> while the code doesn't follow that, which is a confusing situation.
>
> I see that a similar pattern is used already in the firewire code, but
> I don't think this justifies to introduce it to yet another place.
The concern is in the category of human practice, and heuristics, in my
opinion.
I check C language specification and it says that enumeration-constant
has type int, and enumerated type shall be compatible with char, a signed
integer type, or an unsigned integer type and the choice of type is
implementation-defined. The assignment of ORed enumeration-constant (int)
to enumerated type (int with 32 bit storage in most System V ABIs) is not
forbidden past and future though the specification mentions about its
warnings in the annex.
Nevertheless, the practical point should be respected as well. I'll
prepare take 3 patchset including fix for some issued points.
Thanks
Takashi Sakamoto
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-05-18 1:46 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-15 7:11 [PATCH v2 00/10] ALSA: oxfw: code refactoring for quirks specific to ASICs Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 01/10] Revert "ALSA: bebob/oxfw: fix Kconfig entry for Mackie d.2 Pro" Takashi Sakamoto
2021-05-15 7:11 ` Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 02/10] ALSA: oxfw: code refactoring for existent device entry with specifier_id and version Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 03/10] ALSA: oxfw: code refactoring to detect mackie models Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 04/10] ALSA: oxfw: add explicit device entry for Loud Technologies Tapco Link.FireWire 4x6 Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 05/10] ALSA: oxfw: add explicit device entry for Loud Technologies Mackie Onyx Sattelite Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 06/10] ALSA: oxfw: add comment for the type of ASICs Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 07/10] ALSA: oxfw: code refactoring for jumbo-payload quirk in OXFW970 Takashi Sakamoto
2021-05-17 9:11 ` Takashi Iwai
2021-05-18 1:45 ` Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 08/10] ALSA: firewire-lib: code refactoring for jumbo payload quirk Takashi Sakamoto
2021-05-15 7:11 ` [PATCH v2 09/10] ALSA: oxfw; code refactoring for wrong_dbs quirk Takashi Sakamoto
2021-05-17 9:11 ` Takashi Iwai
2021-05-15 7:11 ` [PATCH v2 10/10] ALSA: oxfw: add quirk flag for blocking transmission method Takashi Sakamoto
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.