Linux Sound subsystem development
 help / color / mirror / Atom feed
From: Amin Dandache <amin@s7u.net>
To: Takashi Iwai <tiwai@suse.de>
Cc: linux-sound@vger.kernel.org
Subject: Re: [PATCH] rename s1810c* to presonus_studio to get s1824c working
Date: Thu, 27 Feb 2025 10:53:40 +0100	[thread overview]
Message-ID: <ecdb2314-fb78-47d6-96d0-dcd4a51ee397@s7u.net> (raw)
In-Reply-To: <87mse77n1t.wl-tiwai@suse.de>

Hi,


Yes, the previous file was named after the exact model 1810c.
I would not like to put the 1824c into the file as it suggest its just 
one model.
It feels like monkey patching for me.

Also it's the same series: Presonus Studio.

And yes it's just a new device ID, but there are some features missing 
like SPDIF which i could not test yet, also if every function is adopted 
by the 1810c.

I just tested the basic functionality and it works perfect for analog 
signals.

In the future there could be some differences in the handling for the 
models, so i tried to prepare that and rename it to a global name that 
supports more devices inside and more cases for the devices.
like the mixer_scarlett.c which supports all focusrite scarlett devices 
in one file.

I will refactor the proper subject and try put the signed-off-by tag, as 
this is my first commit i have to learn a little bit new.


Best Regards,
Amin

On 27.02.25 10:29, Takashi Iwai wrote:
> On Thu, 27 Feb 2025 07:43:08 +0100,
> Amin Dandache wrote:
>> ---
>>   sound/usb/Makefile                            |   2 +-
>>   sound/usb/format.c                            |  12 +-
>>   ...mixer_s1810c.c => mixer_presonus_studio.c} | 247 +++++++++---------
>>   sound/usb/mixer_presonus_studio.h             |   8 +
>>   sound/usb/mixer_quirks.c                      |  12 +-
>>   sound/usb/mixer_s1810c.h                      |   7 -
>>   sound/usb/quirks.c                            |   9 +-
>>   7 files changed, 161 insertions(+), 136 deletions(-)
>>   rename sound/usb/{mixer_s1810c.c => mixer_presonus_studio.c} (59%)
>>   create mode 100644 sound/usb/mixer_presonus_studio.h
>>   delete mode 100644 sound/usb/mixer_s1810c.h
> First off, could you give the reason why you must rename the whole
> stuff?  Is it just to add the support of a new device ID 194f:010d?
> If so, we don't have to rename everything, but just keep using the
> same function as is.  You can put simply a comment that the function
> supports both devices, instead.
>
> And, the patch should be formatted properly. You need a proper subject
> prefix (e.g. "ALSA: usb-audio: ....."), then put more detailed patch
> description texts.
>
> Last but not least, most importantly, we need your Signed-off-by tag.
> It's a legal requirement.
>
> Could you address the above and resubmit?
>
>
> thanks,
>
> Takashi

  reply	other threads:[~2025-02-27 10:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-27  6:43 [PATCH] rename s1810c* to presonus_studio to get s1824c working Amin Dandache
2025-02-27  9:29 ` Takashi Iwai
2025-02-27  9:53   ` Amin Dandache [this message]
2025-02-27 10:04     ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ecdb2314-fb78-47d6-96d0-dcd4a51ee397@s7u.net \
    --to=amin@s7u.net \
    --cc=linux-sound@vger.kernel.org \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox