[PATCH] ASoC: rt5640: add device tree support

Stephen Warren swarren at wwwdotorg.org
Thu Jun 13 02:56:46 EST 2013


On 06/12/2013 10:46 AM, Mark Brown wrote:
> On Tue, Jun 11, 2013 at 02:40:40PM -0600, Stephen Warren wrote:
> 
>> +- realtek,ldo1-en-gpios : The GPIO that controls the CODEC's
>> LDO1_EN pin.
> 
> Why gpios and not gpio?

For some reason, GPIO properties have always been named "gpios" rather
than "gpio", even when only a single entry is expected. I don't really
understand why, but I've been asked (or seen others asked) to
s/gpio/gpios/ in DT bindings before. I explicitly CC'd Grant and Rob
here in case they can shed any light.

>> -	if (rt5640->pdata.ldo1_en) { +	if
>> (gpio_is_valid(rt5640->pdata.ldo1_en)) { ret =
>> devm_gpio_request_one(&i2c->dev, rt5640->pdata.ldo1_en, 
>> GPIOF_OUT_INIT_HIGH,
> 
> Unfortunately gpio_is_valid() is unhelpful for platform data since
> often zero is a valid GPIO but it's also the default "do nothing"
> platform data.  It's therefore better to either include a check for
> non-zero as well or have code that takes a zero in the platform
> data and sets it to a negative value instead.

I think people filling in platform data should simply be required to
put a valid/correct value in that field. There aren't any users of
this, so it's not like adding a new field where
backwards-compatibility might be a concern (and even then, updating
all users of this platform data type wouldn't be hard), and it should
be obvious if you get it wrong.


More information about the devicetree-discuss mailing list