Re: [PATCH 1/2] ALSA: ASoC: add DT bindings for cs4270

[Timur Tabi <timur@freescale.com>]

Daniel Mack wrote:

> Sure, the code I have at the moment does it that way, but the idea is to
> have little to no platform specific code. And also, if anything in the
> system needs to know when and how to drive the reset line, it should be
> the driver, right?

Maybe.  We do have a lot of similar code in the boot loader, but of course
that means that it's a static configuration.

>> I have no problem using the CS4270
>> on my board, and I don't need this feature.
> 
> Because you care for that either in the bootloader or the platform code
> I believe?

I don't have any platform code that initializes the codec.

Of course, that doesn't mean that you shouldn't need any.  I'm not
fundamentally opposed to your patch, I just don't have any context to go by.

Also, I have a gut feeling that if someone else needs to do the same
thing, then this code:

devm_gpio_request_one(&i2c_client->dev, reset_gpio,
	reset_gpio_flags & OF_GPIO_ACTIVE_LOW ?
	GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
	"cs4270 reset")

won't work for him, because it's not generic enough.

>> I don't see where you test whether the reset-gpio property is present.  It
>> won't be present in my device tree.
> 
> The idea is that reset_gpio will be < 0 in that case, and so
> devm_gpio_request_one() fails. So your platform should be ok and no
> further checks are necessary.

Well, I would prefer that you check for the property before making any
calls that use it.

Also, I just noticed a typo here:

	/* See if we way to bring the codec out of reset */

-- 
Timur Tabi
Linux kernel developer at Freescale