[alsa-devel] [PATCH v4 1/2] ASoC: cs35l36: Add support for Cirrus CS35L36 Amplifier

[Charles Keepax]

On Thu, Dec 20, 2018 at 01:41:15PM -0600, James Schulman wrote:
> Add driver support for Cirrus Logic CS35L36 boosted
> speaker amplifier
> 
> Signed-off-by: James Schulman <james.schulman at cirrus.com>
> ---
> +struct cs35l36_platform_data {
> +	bool sclk_frc;
> +	bool lrclk_frc;

These two are now unused.

> +	bool multi_amp_mode;
> +	bool dcm_mode;
> +	int ldm_mode_sel;
> +	bool amp_gain_zc;

As is this guy.

> +	bool amp_pcm_inv;
> +	bool pdm_ldm_exit;
> +	bool pdm_ldm_enter;

And these two.

> +	bool imon_pol_inv;
> +	bool vmon_pol_inv;
> +	int boost_ind;
> +	int bst_vctl;
> +	int bst_vctl_sel;
> +	int bst_ipk;
> +	bool extern_boost;
> +	int temp_warn_thld;
> +	struct cs35l36_vpbr_cfg vpbr_config;
> +	struct cs35l36_irq_cfg irq_config;
> +};

> +
> +	/* INT/GPIO Pin Config */
> +	irq_gpio = of_get_child_by_name(np, "cirrus,irq-config");
> +	irq_gpio_config->is_present = irq_gpio ? true : false;

Should this be based off the interrupts property itself? The
defaults for both of the properties in this block seem quite sane
so it seems like people might use the IRQ but not specify this
block.

> +	if (irq_gpio_config->is_present) {
> +		if (of_property_read_u32(irq_gpio, "cirrus,irq-drive-select",
> +					 &val) >= 0)
> +			irq_gpio_config->irq_drv_sel = val;
> +		if (of_property_read_u32(irq_gpio, "cirrus,irq-gpio-select",
> +					 &val) >= 0)
> +			irq_gpio_config->irq_gpio_sel = val;
> +
> +		irq_d = irq_get_irq_data(i2c_client->irq);
> +		if (!irq_d) {
> +			dev_err(&i2c_client->dev, "Invalid IRQ: %d\n",
> +				i2c_client->irq);
> +			return -EINVAL;
> +		}
> +
> +		irq_gpio_config->irq_pol = irqd_get_trigger_type(irq_d);
> +	}

> +
> +	dev_info(&i2c_client->dev, "Cirrus Logic CS35L%d, Revision: %02X\n",
> +		 cs35l36->chip_version, reg_revid >> 8);
> +
> +	ret =  snd_soc_register_component(dev, &soc_component_dev_cs35l36,
> +					  cs35l36_dai, ARRAY_SIZE(cs35l36_dai));

Could use devm_ here.

Fix up those and you can add my:

Reviewed-by: Charles Keepax <ckeepax at opensource.cirrus.com>

Thanks,
Charles