From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 26/39] fireworks: Add command/response functionality into hwdep interface Date: Fri, 28 Feb 2014 07:58:32 +0100 Message-ID: References: <1393558072-25926-1-git-send-email-o-takashi@sakamocchi.jp> <1393558072-25926-27-git-send-email-o-takashi@sakamocchi.jp> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id AC6022651A2 for ; Fri, 28 Feb 2014 07:58:32 +0100 (CET) In-Reply-To: <1393558072-25926-27-git-send-email-o-takashi@sakamocchi.jp> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Takashi Sakamoto Cc: alsa-devel@alsa-project.org, clemens@ladisch.de, ffado-devel@lists.sf.net List-Id: alsa-devel@alsa-project.org At Fri, 28 Feb 2014 12:27:39 +0900, Takashi Sakamoto wrote: > > This commit adds two functionality for hwdep interface, adds two parameters for > this driver, add a node for proc interface. > > To receive responses from devices, this driver already allocate own callback > into private address area in host controller. This means no one can allocate > its own callback to the address. So this driver must give a way for user > applications to receive responses. > > This commit adds a functionality to receive responses via hwdep interface. The > application can receive responses to read from this interface. To achieve this, > this commit adds a buffer to queue responses. The default size of this buffer is > 1024 bytes. This size can be changed to give preferrable size to > 'resp_buf_size' parameter for this driver. The application should notice rest > of space in this buffer because this driver don't push responses when this > buffer has no space. > > Additionaly, this commit adds a functionality to transmit commands via hwdep > interface. The application can transmit commands to write into this interface. > I note that the application can transmit one command at once, but can receive > as many responses as possible untill the user-buffer is full. > > When using these interfaces, the application must keep maximum number of > sequence number in command within the number in firewire.h because this driver > uses this number to distinguish the response is against the command by the > application or this driver. > > Usually responses against commands which the application transmits are pushed > into this buffer. But to enable 'resp_buf_debug' parameter for this driver, all > responses are pushed into the buffer. When using this mode, I reccomend to > expand the size of buffer. > > Finally this commit adds a new node into proc interface to output status of the > buffer. > > Signed-off-by: Takashi Sakamoto > --- > include/uapi/sound/firewire.h | 18 +++ > sound/firewire/fireworks/fireworks.c | 17 +++ > sound/firewire/fireworks/fireworks.h | 22 +-- > sound/firewire/fireworks/fireworks_command.c | 6 +- > sound/firewire/fireworks/fireworks_hwdep.c | 129 ++++++++++++++--- > sound/firewire/fireworks/fireworks_proc.c | 19 +++ > sound/firewire/fireworks/fireworks_transaction.c | 176 ++++++++++++++++++++--- > 7 files changed, 340 insertions(+), 47 deletions(-) > > diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h > index fc9afb2..7f4c419 100644 > --- a/include/uapi/sound/firewire.h > +++ b/include/uapi/sound/firewire.h > @@ -7,6 +7,7 @@ > > #define SNDRV_FIREWIRE_EVENT_LOCK_STATUS 0x000010cc > #define SNDRV_FIREWIRE_EVENT_DICE_NOTIFICATION 0xd1ce004e > +#define SNDRV_FIREWIRE_EVENT_EFW_RESPONSE 0x4e617475 > > struct snd_firewire_event_common { > unsigned int type; /* SNDRV_FIREWIRE_EVENT_xxx */ > @@ -22,10 +23,27 @@ struct snd_firewire_event_dice_notification { > unsigned int notification; /* DICE-specific bits */ > }; > > +#define SND_EFW_TRANSACTION_SEQNUM_MAX ((uint32_t)(BIT(28) - 1)) > +/* each field should be in big endian */ > +struct snd_efw_transaction { > + uint32_t length; > + uint32_t version; > + uint32_t seqnum; > + uint32_t category; > + uint32_t command; > + uint32_t status; > + uint32_t params[0]; > +}; > +struct snd_firewire_event_efw_response { > + unsigned int type; > + uint32_t response[0]; /* some responses */ > +}; > + > union snd_firewire_event { > struct snd_firewire_event_common common; > struct snd_firewire_event_lock_status lock_status; > struct snd_firewire_event_dice_notification dice_notification; > + struct snd_firewire_event_efw_response efw_response; > }; > > > diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c > index 38986c0..7846285 100644 > --- a/sound/firewire/fireworks/fireworks.c > +++ b/sound/firewire/fireworks/fireworks.c > @@ -24,6 +24,8 @@ MODULE_LICENSE("GPL v2"); > static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; > static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR; > static bool enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE_PNP; > +unsigned int resp_buf_size = 1024; > +bool resp_buf_debug = false; > > module_param_array(index, int, NULL, 0444); > MODULE_PARM_DESC(index, "card index"); > @@ -31,6 +33,10 @@ module_param_array(id, charp, NULL, 0444); > MODULE_PARM_DESC(id, "ID string"); > module_param_array(enable, bool, NULL, 0444); > MODULE_PARM_DESC(enable, "enable Fireworks sound card"); > +module_param(resp_buf_size, uint, 0444); > +MODULE_PARM_DESC(resp_buf_size, "response buffer size (default 1024)"); > +module_param(resp_buf_debug, bool, 0444); > +MODULE_PARM_DESC(resp_buf_debug, "store all responses to buffer"); > > static DEFINE_MUTEX(devices_mutex); > static unsigned int devices_used; > @@ -165,6 +171,7 @@ efw_probe(struct fw_unit *unit, > { > struct snd_card *card; > struct snd_efw *efw; > + void *resp_buf; > int card_index, err; > > mutex_lock(&devices_mutex); > @@ -178,6 +185,13 @@ efw_probe(struct fw_unit *unit, > goto end; > } > > + /* prepare response buffer */ > + resp_buf = kzalloc(resp_buf_size, GFP_KERNEL); Better to have a sanity check of resp_buf_size value. You can't trust a value given by a user. Takashi