On 24.07.2012 22:39, Timur Tabi wrote:
Daniel Mack wrote:
Yes, I'm aware of this, I just wonder how does your DT note looks like?
Take a look at arch/powerpc/boot/dts/mpc8610_hpcd.dts
Interesting, I didn't know that works without enabling the specific string inside the driver. And that makes me wonder why many of the Wolfson codec have them (like wm8580). I might lack something here, maybe Mark can explain?
And are you ok with the 2nd patch?
Isn't that something that should be done in platform code? It seems very platform-specific for a codec driver.
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?
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?
- /* See if we way to bring the codec out of reset */
- if (np) {
enum of_gpio_flags reset_gpio_flags;Blank line after variable declarations
int reset_gpio = of_get_named_gpio_flags(np, "reset-gpio", 0,&reset_gpio_flags);Can you make this line shorter by using shorter variable names or something?
Right, fixed.
if (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") < 0) {reset_gpio = -EINVAL;}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.
Thanks for the review, Daniel