RE: [PATCH 1/2] ASoC: sma1307: Add driver for Iron Device SMA1307

[Ki-Seok Jo <kiseok.jo@xxxxxxxxxxxxxx>]

> > +static int sma1307_reset_get(struct snd_kcontrol *kcontrol,
> > +			     struct snd_ctl_elem_value *ucontrol) {
> > +	struct snd_soc_component *component =
> > +	    snd_soc_kcontrol_component(kcontrol);
> > +	struct sma1307_priv *sma1307 = 
> > +snd_soc_component_get_drvdata(component);
> > +
> > +	ucontrol->value.integer.value[0] = (int)sma1307->reset;
> > +	dev_dbg(sma1307->dev, "%s:  ready\n", __func__);

> Drop all such simple function success messages.

Even though the logs were set using dev_dbg, is there still an issue?


> > +
> > +	return true;
> > +}
> > +
> > +static int sma1307_binary_mode_get(struct snd_kcontrol *kcontrol,
> > +				   struct snd_ctl_elem_value *ucontrol) {
> > +	struct sma1307_priv *sma1307 = snd_kcontrol_chip(kcontrol);
> > +
> > +	ucontrol->value.enumerated.item[0] = (unsigned 
> > +int)sma1307->binary_mode;
> > +
> > +	dev_dbg(sma1307->dev,
> > +		 "%s: binary mode is %s\n",
> > +		 __func__, sma1307_binary_mode_text[sma1307->binary_mode]);

> Why do you debug every sound-API call?

I added it for debugging purposes to verify when and if it is called during actual operation. Is it a problem if it is only called in debug mode?

> > +
> > +static void sma1307_setting_loaded(struct sma1307_priv *sma1307, char 
> > +*file)

> This was never properly build and tested. That's a string, but you pass here 'struct sma1307_setting_file'.

> Do not send code which does not pass W=1 builds, smatch and sparse checks.

> > Also, thagt's a const char *.

I performed static analysis with the C=2 option, but I will check using the W=1 option.

This function is configured to not operate if the file is missing. Typically, it will not function without the file, but it can be used if I provide the file as needed. Should such a function be avoided?

> > +
> > +	sma1307->attr_grp = &sma1307_attr_group;
> > +	ret = sysfs_create_group(sma1307->kobj, sma1307->attr_grp);

> You need to document sysfs ABI.

I was unaware that documentation for sysfs was required. I will read Documentation/admin-guide/sysfs-rules.rst and work on creating the documentation. Additionally, I would appreciate any further information you can provide.

Thank you for your help. I will proceed with the documentation step by step. Could you provide more information about sparse? I tried using it but encountered difficulties, so I only proceeded with the C=2 option.

Best regards,
Kiseok Jo