Re: [PATCHv3] tlv320dac33: Add device tree bindings

[Peter Ujfalusi]

Hi,

On 2018-02-24 22:57, Pavel Machek wrote:
> On Tue 2018-02-06 15:27:22, Mark Brown wrote:
>> On Tue, Feb 06, 2018 at 02:49:06PM +0100, Pavel Machek wrote:
>>> On Tue 2018-02-06 12:11:22, Mark Brown wrote:
>>
>>>> Please submit patches using subject lines reflecting the style for the
>>>> subsystem.  This makes it easier for people to identify relevant
>>>> patches.  Look at what existing commits in the area you're changing are
>>>> doing and make sure your subject lines visually resemble what they're
>>>> doing.
>>
>>> AFAICT this goes to Documentation, not sound.
>>
>> Even if that were the case you're not following the commonly accepted
>> practice there either.
>>
>>> If you have any comments on V1 of this patch, that would be nice, I'd
>>> like to get that merged once we are finished with the bindings.
>>
>> It is the middle of the merge window, please be a bit more patient.
> 
> For the record, patch currently looks like this. If you have some
> comments on the code, I'd like to hear them.
> 
> Best regards,
> diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c
> index 8c71d2f..7302789 100644
> --- a/sound/soc/codecs/tlv320dac33.c
> +++ b/sound/soc/codecs/tlv320dac33.c

...

> @@ -1511,10 +1509,26 @@ static int dac33_i2c_probe(struct i2c_client *client,
>  
>  	i2c_set_clientdata(client, dac33);
>  
> -	dac33->power_gpio = pdata->power_gpio;
> -	dac33->burst_bclkdiv = pdata->burst_bclkdiv;
> -	dac33->keep_bclk = pdata->keep_bclk;
> -	dac33->mode1_latency = pdata->mode1_latency;
> +	if (pdata) {
> +		dac33->power_gpio = pdata->power_gpio;
> +		dac33->burst_bclkdiv = pdata->burst_bclkdiv;
> +		dac33->keep_bclk = pdata->keep_bclk;
> +		dac33->mode1_latency = pdata->mode1_latency;
> +	} else if (np) {
> +		ret = of_get_named_gpio(np, "power-gpios", 0);
> +		if (ret >= 0)
> +			dac33->power_gpio = ret;
> +		else
> +			dac33->power_gpio = -1;

the code checks for (dac33->power_gpio >= 0) in live path, I guess you
can just skip the power_gpio = -1 in case we don't have GPIO
Handling of EPROBE_DEFER might be something we might want for the GPIO.

> +
> +		if (of_property_read_bool(np, "ti,keep-bclk"))
> +			dac33->keep_bclk = true;
> +
> +		of_property_read_u32(np, "ti,burst-bclkdiv", &dac33->burst_bclkdiv);
> +	} else {
> +		dev_err(&client->dev, "Platform data not set\n");
> +		return -ENODEV;
> +	}
>  	if (!dac33->mode1_latency)
>  		dac33->mode1_latency = 10000; /* 10ms */
>  	dac33->irq = client->irq;
> @@ -1580,9 +1594,16 @@ static const struct i2c_device_id tlv320dac33_i2c_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, tlv320dac33_i2c_id);
>  
> +static const struct of_device_id tlv320dac33_of_match[] = {
> +	{ .compatible = "ti,tlv320dac33", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, tlv320dac33_of_match);
> +
>  static struct i2c_driver tlv320dac33_i2c_driver = {
>  	.driver = {
>  		.name = "tlv320dac33-codec",
> +		.of_match_table = of_match_ptr(tlv320dac33_of_match),
>  	},
>  	.probe		= dac33_i2c_probe,
>  	.remove		= dac33_i2c_remove,
> 
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki