| Message ID | 1434996780-34535-2-git-send-email-benzh@chromium.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9bfde72157036f4eaa44f3e8982217ce1b3e14b6 |
| Headers | show |
On 6/22/15 8:13 PM, Ben Zhang wrote: > This patch makes the driver use the unified device property API > so that platform data can be provided by Device Tree, ACPI > or board files. > > Signed-off-by: Ben Zhang <benzh@chromium.org> > --- > sound/soc/codecs/rt5677.c | 57 +++++++++++++++++++++-------------------------- > 1 file changed, 25 insertions(+), 32 deletions(-) > > diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c > index c166217..69e45d8 100644 > --- a/sound/soc/codecs/rt5677.c > +++ b/sound/soc/codecs/rt5677.c > @@ -20,6 +20,7 @@ > #include <linux/platform_device.h> > #include <linux/spi/spi.h> > #include <linux/firmware.h> > +#include <linux/property.h> > #include <sound/core.h> > #include <sound/pcm.h> > #include <sound/pcm_params.h> > @@ -5018,27 +5019,29 @@ static const struct i2c_device_id rt5677_i2c_id[] = { > }; > MODULE_DEVICE_TABLE(i2c, rt5677_i2c_id); > > -static int rt5677_parse_dt(struct rt5677_priv *rt5677, struct device_node *np) > +static void rt5677_read_device_properties(struct rt5677_priv *rt5677, > + struct device *dev) > { > - rt5677->pdata.in1_diff = of_property_read_bool(np, > - "realtek,in1-differential"); > - rt5677->pdata.in2_diff = of_property_read_bool(np, > - "realtek,in2-differential"); > - rt5677->pdata.lout1_diff = of_property_read_bool(np, > - "realtek,lout1-differential"); > - rt5677->pdata.lout2_diff = of_property_read_bool(np, > - "realtek,lout2-differential"); > - rt5677->pdata.lout3_diff = of_property_read_bool(np, > - "realtek,lout3-differential"); > - > - of_property_read_u8_array(np, "realtek,gpio-config", > - rt5677->pdata.gpio_config, RT5677_GPIO_NUM); > - > - of_property_read_u32(np, "realtek,jd1-gpio", &rt5677->pdata.jd1_gpio); > - of_property_read_u32(np, "realtek,jd2-gpio", &rt5677->pdata.jd2_gpio); > - of_property_read_u32(np, "realtek,jd3-gpio", &rt5677->pdata.jd3_gpio); > - > - return 0; > + rt5677->pdata.in1_diff = device_property_read_bool(dev, > + "realtek,in1-differential"); Shouldn't it be device_property_present() ? thanks for starting this transition, this will be very useful for ACPI-based solutions. -Pierre > + rt5677->pdata.in2_diff = device_property_read_bool(dev, > + "realtek,in2-differential"); > + rt5677->pdata.lout1_diff = device_property_read_bool(dev, > + "realtek,lout1-differential"); > + rt5677->pdata.lout2_diff = device_property_read_bool(dev, > + "realtek,lout2-differential"); > + rt5677->pdata.lout3_diff = device_property_read_bool(dev, > + "realtek,lout3-differential"); > + > + device_property_read_u8_array(dev, "realtek,gpio-config", > + rt5677->pdata.gpio_config, RT5677_GPIO_NUM); > + > + device_property_read_u32(dev, "realtek,jd1-gpio", > + &rt5677->pdata.jd1_gpio); > + device_property_read_u32(dev, "realtek,jd2-gpio", > + &rt5677->pdata.jd2_gpio); > + device_property_read_u32(dev, "realtek,jd3-gpio", > + &rt5677->pdata.jd3_gpio); > } > > static struct regmap_irq rt5677_irqs[] = { > @@ -5121,18 +5124,8 @@ static int rt5677_i2c_probe(struct i2c_client *i2c, > > if (pdata) > rt5677->pdata = *pdata; > - > - if (i2c->dev.of_node) { > - ret = rt5677_parse_dt(rt5677, i2c->dev.of_node); > - if (ret) { > - dev_err(&i2c->dev, "Failed to parse device tree: %d\n", > - ret); > - return ret; > - } > - } else { > - rt5677->pow_ldo2 = -EINVAL; > - rt5677->reset_pin = -EINVAL; > - } > + else > + rt5677_read_device_properties(rt5677, &i2c->dev); > > /* pow-ldo2 and reset are optional. The codec pins may be statically > * connected on the board without gpios. If the gpio device property >
On Tue, Jun 23, 2015 at 12:18:54AM +0200, Pierre-Louis Bossart wrote: > On 6/22/15 8:13 PM, Ben Zhang wrote: > >+ rt5677->pdata.in1_diff = device_property_read_bool(dev, > >+ "realtek,in1-differential"); > Shouldn't it be device_property_present() ? At least on the DT side they should be equivalent - I don't know if ACPI works differently? > thanks for starting this transition, this will be very useful for ACPI-based > solutions. Have the Intel audio people been speaking to the UEFI forum about using the new registration process for these properties? More from interest than anything else.
On Mon, Jun 22, 2015 at 3:50 PM, Mark Brown <broonie@kernel.org> wrote: > On Tue, Jun 23, 2015 at 12:18:54AM +0200, Pierre-Louis Bossart wrote: >> On 6/22/15 8:13 PM, Ben Zhang wrote: > >> >+ rt5677->pdata.in1_diff = device_property_read_bool(dev, >> >+ "realtek,in1-differential"); > >> Shouldn't it be device_property_present() ? > > At least on the DT side they should be equivalent - I don't know if ACPI > works differently? I think they are equivalent on the ACPI side as well. device_property_read_bool is defined as a wrapper of device_property_present in include/linux/property.h. I tested that the function returns true iff an entry for the property exists in the _DSD. For example in1_diff is true when: Name (_DSD, Package () { ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package () { Package () { "realtek,in1-differential", 1 }, .... } }) >> thanks for starting this transition, this will be very useful for ACPI-based >> solutions. > > Have the Intel audio people been speaking to the UEFI forum about using > the new registration process for these properties? More from interest > than anything else.
On 6/22/15, 3:50 PM, "Mark Brown" <broonie@kernel.org> wrote: >On Tue, Jun 23, 2015 at 12:18:54AM +0200, Pierre-Louis Bossart wrote: >> On 6/22/15 8:13 PM, Ben Zhang wrote: > >> >+ rt5677->pdata.in1_diff = device_property_read_bool(dev, >> >+ "realtek,in1-differential"); > >> Shouldn't it be device_property_present() ? > >At least on the DT side they should be equivalent - I don't know if ACPI >works differently? > >> thanks for starting this transition, this will be very useful for >>ACPI-based >> solutions. > >Have the Intel audio people been speaking to the UEFI forum about using >the new registration process for these properties? More from interest >than anything else. Yes (although "audio people" is a rather broad term at Intel :-)
On Mon, Jun 22, 2015 at 05:05:37PM -0700, Darren Hart wrote: > On 6/22/15, 3:50 PM, "Mark Brown" <broonie@kernel.org> wrote: > >Have the Intel audio people been speaking to the UEFI forum about using > >the new registration process for these properties? More from interest > >than anything else. > Yes (although "audio people" is a rather broad term at Intel :-) It is, as is ACPI people, and these are among the issues here. Is this stuff going to get used as a trial run of the new process? It'd be good to know if we should be pushing people towards it.
diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c index c166217..69e45d8 100644 --- a/sound/soc/codecs/rt5677.c +++ b/sound/soc/codecs/rt5677.c @@ -20,6 +20,7 @@ #include <linux/platform_device.h> #include <linux/spi/spi.h> #include <linux/firmware.h> +#include <linux/property.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> @@ -5018,27 +5019,29 @@ static const struct i2c_device_id rt5677_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, rt5677_i2c_id); -static int rt5677_parse_dt(struct rt5677_priv *rt5677, struct device_node *np) +static void rt5677_read_device_properties(struct rt5677_priv *rt5677, + struct device *dev) { - rt5677->pdata.in1_diff = of_property_read_bool(np, - "realtek,in1-differential"); - rt5677->pdata.in2_diff = of_property_read_bool(np, - "realtek,in2-differential"); - rt5677->pdata.lout1_diff = of_property_read_bool(np, - "realtek,lout1-differential"); - rt5677->pdata.lout2_diff = of_property_read_bool(np, - "realtek,lout2-differential"); - rt5677->pdata.lout3_diff = of_property_read_bool(np, - "realtek,lout3-differential"); - - of_property_read_u8_array(np, "realtek,gpio-config", - rt5677->pdata.gpio_config, RT5677_GPIO_NUM); - - of_property_read_u32(np, "realtek,jd1-gpio", &rt5677->pdata.jd1_gpio); - of_property_read_u32(np, "realtek,jd2-gpio", &rt5677->pdata.jd2_gpio); - of_property_read_u32(np, "realtek,jd3-gpio", &rt5677->pdata.jd3_gpio); - - return 0; + rt5677->pdata.in1_diff = device_property_read_bool(dev, + "realtek,in1-differential"); + rt5677->pdata.in2_diff = device_property_read_bool(dev, + "realtek,in2-differential"); + rt5677->pdata.lout1_diff = device_property_read_bool(dev, + "realtek,lout1-differential"); + rt5677->pdata.lout2_diff = device_property_read_bool(dev, + "realtek,lout2-differential"); + rt5677->pdata.lout3_diff = device_property_read_bool(dev, + "realtek,lout3-differential"); + + device_property_read_u8_array(dev, "realtek,gpio-config", + rt5677->pdata.gpio_config, RT5677_GPIO_NUM); + + device_property_read_u32(dev, "realtek,jd1-gpio", + &rt5677->pdata.jd1_gpio); + device_property_read_u32(dev, "realtek,jd2-gpio", + &rt5677->pdata.jd2_gpio); + device_property_read_u32(dev, "realtek,jd3-gpio", + &rt5677->pdata.jd3_gpio); } static struct regmap_irq rt5677_irqs[] = { @@ -5121,18 +5124,8 @@ static int rt5677_i2c_probe(struct i2c_client *i2c, if (pdata) rt5677->pdata = *pdata; - - if (i2c->dev.of_node) { - ret = rt5677_parse_dt(rt5677, i2c->dev.of_node); - if (ret) { - dev_err(&i2c->dev, "Failed to parse device tree: %d\n", - ret); - return ret; - } - } else { - rt5677->pow_ldo2 = -EINVAL; - rt5677->reset_pin = -EINVAL; - } + else + rt5677_read_device_properties(rt5677, &i2c->dev); /* pow-ldo2 and reset are optional. The codec pins may be statically * connected on the board without gpios. If the gpio device property
This patch makes the driver use the unified device property API so that platform data can be provided by Device Tree, ACPI or board files. Signed-off-by: Ben Zhang <benzh@chromium.org> --- sound/soc/codecs/rt5677.c | 57 +++++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 32 deletions(-)