On Mon, Nov 02, 2026 at 08:45:18PM -0600, Will Porter wrote: > Hi Christian, > > Thanks for RFC V2, and for folding everything in so quickly. > > On Mon, Nov 02, 2026 at 01:32:46AM +0100, Christian Ruppert wrote: > > Makes sense. Added to RFC V2 (in patch 10 where the cache is added). > > Please double-check if the logic reflects what you had in mind. > > Yes, that is what I had in mind: a put of the cached value still > writes it, but returns 0, so alsactl restore sends no change events. > On an iD14 MkI, a put of the held value sent no change event on a > write-only switch, a volume or a matrix control. A put of a new value > on the write-only switch sent one. > > One question on the same function. A FORCE_SET control without CACHE > now reads the device before each write; in RFC v1 the put skipped > that read. If the device refuses the read, the put fails before it > writes. No table uses FORCE_SET without CACHE today, so nothing > breaks now. > > Should that combination work? If yes, the put could skip the read > when the control has no cache slot, as the diff below does. If not, > a comment or a check in snd_audient_add_ctrl() would say so, and you > can drop the diff. I guess that depends how we define force_set. In my mind it's literally force set, i.e. for write-only values we must explicitly specify both FORCE_SET and CACHE. The reasoning is that I don't see how write-only controls can work correctly without cache. But force-setting uncached read-write controls might be of value if the set operation as such triggers an event inside a device, even if the value is the same as the one just read back on some volatile control. I imagine this could become useful for certain enumeration types, for example to load a preset. The question is now what the put function should return in such a case and until we encounter one it's rather hypothetical to say what's preferable: - We could systematically return 1 because an event was triggered in the device. If you think this is the right thing to do, feel free to weave your diff into patch 10. I don't think it makes sense to reiterate RFC V3 for such a small change and I'll test the full patch set anwyay. - We could return 1 if the value is changed and 0 if it isn't which is the current behaviour and designed to be consistent with FORCE_SET in case we do have a cache. I imagine (cached) write-only controls are more probable to trigger events on writing than volatile read-write controls so this seemed the reasonable choice. > > I don't think this is necessary as it should be rather obvious to > > everyone touching the driver that the u32 argument can't carry more > > than 4 bytes. Added a comment to the parameter definition. > > Fine with me. > > checkpatch --strict on the 11 patches gives no errors or warnings, > only CHECK-level style notes, mostly unneeded parentheses (04, 05, > 07, 10) and alignment (01, 05). I'll leave those to your discretion. Not sure if we need to reiterate because of those, Takashi please advise. I would like to keep the parentheses for readability if possible and the alignment issues can be fixed little by little when we touch the file over time. Or otherwise, you can also weave in fixes if you prefer. > iD14 patches on RFC V2 > > I rebased my iD14 patch series on RFC V2. Three of my patches are now > part of yours, so seven remain: > > 1. a step size for integer controls > 2. enumerated controls > 3. a separate mixer map for the iD14 MkI > 4. iD14 MkI monitor and routing controls > 5. iD14 MkI mixer matrix > 6. iD14 MkI write-only controls > 7. the iD14 MkI FU 12 control Is there a reason 4, 5 and 6 are separate? They should all be one big update in the device definition tables without any code changes, right? > All 18 patches build one by one with W=1 without warnings. > On an iD14 MkI (7.2.11 stable), the controls passed the same read and > write checks as on RFC v1. > > Next steps > > As agreed, I'll send the final series: your 11 patches, then the > seven iD14 patches. Two questions first: > > - Your four mixer_evo fixes ("ALSA: mixer_evo: minor fixes and tlv > addition") are still open on Patchwork. Should they still go in > separately, before this series? Then I'd base the final series on > sound for-next once Takashi applies them. Or should I put them at > the front of the final series? In principle they are bugfixes and the intention was to keep them small enough so they can still squeeze into 7.4. If Takashi decides that it's already too late for this type of fixes I'm happy remove the ones which are fixed by our new patch set anyway and resubmit the remaining ones at the beginning of our serie. Happy to review and test an RFC of the complete series until he's back and takes a decision. I expect your seven patches aren't massively impacted by the four pending fixes so starting to review them as soon as you're ready would allow for one or two iterations if required, without losing any time while waiting. > - If the diff below looks right to you, shall I fold it into patch > 10 when I send, or would you prefer to take it into your own tree > first? See above. If you think it makes sense to apply it please just fold it in, I don't think we need a full iteration for this. > Thanks, > Will > > -- >8 -- > From: Will Porter > Subject: [RFC PATCH] ALSA: mixer_audient: Skip the read for an uncached FORCE_SET put > > A FORCE_SET put without a cache slot reads the device before the > write, and fails without writing if that read fails. Skip the read > in that case, as RFC v1 did. Puts on cached controls keep their > change reporting. > > Also fix a typo in a comment and drop a blank line. > > Assisted-by: Claude:claude-opus-5-5 > Signed-off-by: Will Porter > --- > Proposed change to patch 10. > > diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c > index 483ab8f..33c2d1f 100644 > --- a/sound/usb/mixer_audient.c > +++ b/sound/usb/mixer_audient.c > @@ -329,11 +329,12 @@ static int snd_audient_raw_set_one(struct usb_mixer_interface *const mixer, > int ret; > u32 val; > > - if (force) { > + if (force || > + (!slot && (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET))) { > val = !value; /* just make it different */ > } else { > ret = snd_audient_raw_get_one(mixer, ctype, slot, cidx, &val); > - if (ret == -ENODATA) /* FORCED_SET and not cached yet */ > + if (ret == -ENODATA) /* FORCE_SET and not cached yet */ > val = !value; /* just make it different */ > else if (ret < 0) > return ret; > @@ -352,7 +353,6 @@ static int snd_audient_raw_set_one(struct usb_mixer_interface *const mixer, > slot->value = value; > slot->valid = true; > } > - > } > > return val != value; -- Christian Ruppert This message is digitally signed with GnuPG. GPG/PGP Key fingerprint = 4492 2196 75F0 EAA0 981E B661 1476 F17A 975F 587B