From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonathan Cameron Subject: Re: [PATCH v5 1/2] iio: ad5755: add support for dt bindings Date: Sat, 12 Mar 2016 09:25:42 +0000 Message-ID: <56E3E096.1040605@kernel.org> References: <1457705561-12253-1-git-send-email-sean.nyekjaer@prevas.dk> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1457705561-12253-1-git-send-email-sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org> Sender: linux-iio-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Sean Nyekjaer , linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Lars-Peter Clausen List-Id: devicetree@vger.kernel.org On 11/03/16 14:12, Sean Nyekjaer wrote: > Devicetree can provide platform data > > Signed-off-by: Sean Nyekjaer Looks good to me. Would like an Ack / reviewed by from Lars and a device tree ack for the bindings. No particular rush though given it has missed the current merge window now anyway. Please do cc the driver author and maintainers for the device tree bindings on relevant patches. Both will probably pick it up from the list, but they may well read emails addressed directly to them earlier! I'll add cc's for the binding in a mo. Jonathan > --- > Changes since v4: > - added lookups tables instead of large switchcases > > Changes since v3: > - replaced '_' with '-' > - Now used actual values instead of register values. > > Changes since v2: > - removed defines from DT > > drivers/iio/dac/ad5755.c | 186 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 185 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c > index bfb350a..b380803 100644 > --- a/drivers/iio/dac/ad5755.c > +++ b/drivers/iio/dac/ad5755.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -109,6 +110,49 @@ enum ad5755_type { > ID_AD5737, > }; > > +static const int ad5755_dcdc_freq_table[][2] = { > + { 250000, AD5755_DC_DC_FREQ_250kHZ }, > + { 410000, AD5755_DC_DC_FREQ_410kHZ }, > + { 650000, AD5755_DC_DC_FREQ_650kHZ } > +}; > + > +static const int ad5755_dcdc_maxv_table[][2] = { > + { 23000000, AD5755_DC_DC_MAXV_23V }, > + { 24500000, AD5755_DC_DC_MAXV_24V5 }, > + { 27000000, AD5755_DC_DC_MAXV_27V }, > + { 29500000, AD5755_DC_DC_MAXV_29V5 }, > +}; > + > +static const int ad5755_slew_rate_table[][2] = { > + { 64000, AD5755_SLEW_RATE_64k }, > + { 32000, AD5755_SLEW_RATE_32k }, > + { 16000, AD5755_SLEW_RATE_16k }, > + { 8000, AD5755_SLEW_RATE_8k }, > + { 4000, AD5755_SLEW_RATE_4k }, > + { 2000, AD5755_SLEW_RATE_2k }, > + { 1000, AD5755_SLEW_RATE_1k }, > + { 500, AD5755_SLEW_RATE_500 }, > + { 250, AD5755_SLEW_RATE_250 }, > + { 125, AD5755_SLEW_RATE_125 }, > + { 64, AD5755_SLEW_RATE_64 }, > + { 32, AD5755_SLEW_RATE_32 }, > + { 16, AD5755_SLEW_RATE_16 }, > + { 8, AD5755_SLEW_RATE_8 }, > + { 4, AD5755_SLEW_RATE_4 }, > + { 0, AD5755_SLEW_RATE_0_5 }, > +}; > + > +static const int ad5755_slew_step_table[][2] = { > + { 256, AD5755_SLEW_STEP_SIZE_256 }, > + { 128, AD5755_SLEW_STEP_SIZE_128 }, > + { 64, AD5755_SLEW_STEP_SIZE_64 }, > + { 32, AD5755_SLEW_STEP_SIZE_32 }, > + { 16, AD5755_SLEW_STEP_SIZE_16 }, > + { 4, AD5755_SLEW_STEP_SIZE_4 }, > + { 2, AD5755_SLEW_STEP_SIZE_2 }, > + { 1, AD5755_SLEW_STEP_SIZE_1 }, > +}; > + > static int ad5755_write_unlocked(struct iio_dev *indio_dev, > unsigned int reg, unsigned int val) > { > @@ -556,6 +600,129 @@ static const struct ad5755_platform_data ad5755_default_pdata = { > }, > }; > > +#ifdef CONFIG_OF > +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev) > +{ > + struct device_node *np = dev->of_node; > + struct device_node *pp; > + struct ad5755_platform_data *pdata; > + unsigned int tmp; > + unsigned int tmparray[3]; > + int devnr, i; > + > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + return NULL; > + > + pdata->ext_dc_dc_compenstation_resistor = > + of_property_read_bool(np, "adi,ext-dc-dc-compenstation-resistor"); > + > + if (!of_property_read_u32(np, "adi,dc-dc-phase", &tmp)) > + pdata->dc_dc_phase = tmp; > + else > + pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE; > + > + pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ; > + if (!of_property_read_u32(np, "adi,dc-dc-freq-hz", &tmp)) { > + for (i = 0; i < ARRAY_SIZE(ad5755_dcdc_freq_table); i++) { > + if (tmp == ad5755_dcdc_freq_table[i][0]) { > + pdata->dc_dc_freq = ad5755_dcdc_freq_table[i][1]; > + break; > + } > + } > + > + if (i == ARRAY_SIZE(ad5755_dcdc_freq_table)) { > + dev_err(dev, > + "adi,dc-dc-freq out of range selecting 410kHz"); > + } > + } > + > + pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V; > + if (!of_property_read_u32(np, "adi,dc-dc-max-microvolt", &tmp)) { > + for (i = 0; i < ARRAY_SIZE(ad5755_dcdc_maxv_table); i++) { > + if (tmp == ad5755_dcdc_maxv_table[i][0]) { > + pdata->dc_dc_maxv = ad5755_dcdc_maxv_table[i][1]; > + break; > + } > + } > + if (i == ARRAY_SIZE(ad5755_dcdc_maxv_table)) { > + dev_err(dev, > + "adi,dc-dc-maxv out of range selecting 23V"); > + } > + } > + > + devnr = 0; > + for_each_child_of_node(np, pp) { > + if (devnr > AD5755_NUM_CHANNELS) { > + dev_err(dev, > + "There is to many channels defined in DT\n"); > + goto error_out; > + } > + > + if (!of_property_read_u32(pp, "adi,mode", &tmp)) > + pdata->dac[devnr].mode = tmp; > + else > + pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA; > + > + pdata->dac[devnr].ext_current_sense_resistor = > + of_property_read_bool(pp, "adi,ext-current-sense-resistor"); > + > + pdata->dac[devnr].enable_voltage_overrange = > + of_property_read_bool(pp, "adi,enable-voltage-overrange"); > + > + if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) { > + pdata->dac[devnr].slew.enable = tmparray[0]; > + > + pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k; > + for (i = 0; i < ARRAY_SIZE(ad5755_slew_rate_table); i++) { > + if (tmparray[1] == ad5755_slew_rate_table[i][0]) { > + pdata->dac[devnr].slew.rate = > + ad5755_slew_rate_table[i][1]; > + break; > + } > + } > + if (i == ARRAY_SIZE(ad5755_slew_rate_table)) { > + dev_err(dev, > + "channel %d slew rate out of range selecting 64kHz", > + devnr); > + } > + > + pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1; > + for (i = 0; i < ARRAY_SIZE(ad5755_slew_step_table); i++) { > + if (tmparray[2] == ad5755_slew_step_table[i][0]) { > + pdata->dac[devnr].slew.step_size = > + ad5755_slew_step_table[i][1]; > + break; > + } > + } > + if (i == ARRAY_SIZE(ad5755_slew_step_table)) { > + dev_err(dev, > + "channel %d slew step size out of range selecting 1 LSB", > + devnr); > + } > + } else { > + pdata->dac[devnr].slew.enable = false; > + pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k; > + pdata->dac[devnr].slew.step_size = > + AD5755_SLEW_STEP_SIZE_1; > + } > + devnr++; > + } > + > + return pdata; > + > + error_out: > + devm_kfree(dev, pdata); > + return NULL; > +} > +#else > +static > +struct adf4350_platform_data *adf4350_parse_dt(struct device *dev) > +{ > + return NULL; > +} > +#endif > + > static int ad5755_probe(struct spi_device *spi) > { > enum ad5755_type type = spi_get_device_id(spi)->driver_data; > @@ -583,8 +750,15 @@ static int ad5755_probe(struct spi_device *spi) > indio_dev->modes = INDIO_DIRECT_MODE; > indio_dev->num_channels = AD5755_NUM_CHANNELS; > > - if (!pdata) > + if (spi->dev.of_node) > + pdata = ad5755_parse_dt(&spi->dev); > + else > + pdata = spi->dev.platform_data; > + > + if (!pdata) { > + dev_warn(&spi->dev, "no platform data? using default\n"); > pdata = &ad5755_default_pdata; > + } > > ret = ad5755_init_channels(indio_dev, pdata); > if (ret) > @@ -607,6 +781,16 @@ static const struct spi_device_id ad5755_id[] = { > }; > MODULE_DEVICE_TABLE(spi, ad5755_id); > > +static const struct of_device_id ad5755_of_match[] = { > + { .compatible = "adi,ad5755" }, > + { .compatible = "adi,ad5755-1" }, > + { .compatible = "adi,ad5757" }, > + { .compatible = "adi,ad5735" }, > + { .compatible = "adi,ad5737" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ad5755_of_match); > + > static struct spi_driver ad5755_driver = { > .driver = { > .name = "ad5755", > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from saturn.retrosnub.co.uk ([178.18.118.26]:43094 "EHLO saturn.retrosnub.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751650AbcCLJZo (ORCPT ); Sat, 12 Mar 2016 04:25:44 -0500 Subject: Re: [PATCH v5 1/2] iio: ad5755: add support for dt bindings To: Sean Nyekjaer , linux-iio@vger.kernel.org References: <1457705561-12253-1-git-send-email-sean.nyekjaer@prevas.dk> Cc: devicetree@vger.kernel.org, Lars-Peter Clausen From: Jonathan Cameron Message-ID: <56E3E096.1040605@kernel.org> Date: Sat, 12 Mar 2016 09:25:42 +0000 MIME-Version: 1.0 In-Reply-To: <1457705561-12253-1-git-send-email-sean.nyekjaer@prevas.dk> Content-Type: text/plain; charset=windows-1252 Sender: linux-iio-owner@vger.kernel.org List-Id: linux-iio@vger.kernel.org On 11/03/16 14:12, Sean Nyekjaer wrote: > Devicetree can provide platform data > > Signed-off-by: Sean Nyekjaer Looks good to me. Would like an Ack / reviewed by from Lars and a device tree ack for the bindings. No particular rush though given it has missed the current merge window now anyway. Please do cc the driver author and maintainers for the device tree bindings on relevant patches. Both will probably pick it up from the list, but they may well read emails addressed directly to them earlier! I'll add cc's for the binding in a mo. Jonathan > --- > Changes since v4: > - added lookups tables instead of large switchcases > > Changes since v3: > - replaced '_' with '-' > - Now used actual values instead of register values. > > Changes since v2: > - removed defines from DT > > drivers/iio/dac/ad5755.c | 186 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 185 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c > index bfb350a..b380803 100644 > --- a/drivers/iio/dac/ad5755.c > +++ b/drivers/iio/dac/ad5755.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -109,6 +110,49 @@ enum ad5755_type { > ID_AD5737, > }; > > +static const int ad5755_dcdc_freq_table[][2] = { > + { 250000, AD5755_DC_DC_FREQ_250kHZ }, > + { 410000, AD5755_DC_DC_FREQ_410kHZ }, > + { 650000, AD5755_DC_DC_FREQ_650kHZ } > +}; > + > +static const int ad5755_dcdc_maxv_table[][2] = { > + { 23000000, AD5755_DC_DC_MAXV_23V }, > + { 24500000, AD5755_DC_DC_MAXV_24V5 }, > + { 27000000, AD5755_DC_DC_MAXV_27V }, > + { 29500000, AD5755_DC_DC_MAXV_29V5 }, > +}; > + > +static const int ad5755_slew_rate_table[][2] = { > + { 64000, AD5755_SLEW_RATE_64k }, > + { 32000, AD5755_SLEW_RATE_32k }, > + { 16000, AD5755_SLEW_RATE_16k }, > + { 8000, AD5755_SLEW_RATE_8k }, > + { 4000, AD5755_SLEW_RATE_4k }, > + { 2000, AD5755_SLEW_RATE_2k }, > + { 1000, AD5755_SLEW_RATE_1k }, > + { 500, AD5755_SLEW_RATE_500 }, > + { 250, AD5755_SLEW_RATE_250 }, > + { 125, AD5755_SLEW_RATE_125 }, > + { 64, AD5755_SLEW_RATE_64 }, > + { 32, AD5755_SLEW_RATE_32 }, > + { 16, AD5755_SLEW_RATE_16 }, > + { 8, AD5755_SLEW_RATE_8 }, > + { 4, AD5755_SLEW_RATE_4 }, > + { 0, AD5755_SLEW_RATE_0_5 }, > +}; > + > +static const int ad5755_slew_step_table[][2] = { > + { 256, AD5755_SLEW_STEP_SIZE_256 }, > + { 128, AD5755_SLEW_STEP_SIZE_128 }, > + { 64, AD5755_SLEW_STEP_SIZE_64 }, > + { 32, AD5755_SLEW_STEP_SIZE_32 }, > + { 16, AD5755_SLEW_STEP_SIZE_16 }, > + { 4, AD5755_SLEW_STEP_SIZE_4 }, > + { 2, AD5755_SLEW_STEP_SIZE_2 }, > + { 1, AD5755_SLEW_STEP_SIZE_1 }, > +}; > + > static int ad5755_write_unlocked(struct iio_dev *indio_dev, > unsigned int reg, unsigned int val) > { > @@ -556,6 +600,129 @@ static const struct ad5755_platform_data ad5755_default_pdata = { > }, > }; > > +#ifdef CONFIG_OF > +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev) > +{ > + struct device_node *np = dev->of_node; > + struct device_node *pp; > + struct ad5755_platform_data *pdata; > + unsigned int tmp; > + unsigned int tmparray[3]; > + int devnr, i; > + > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + return NULL; > + > + pdata->ext_dc_dc_compenstation_resistor = > + of_property_read_bool(np, "adi,ext-dc-dc-compenstation-resistor"); > + > + if (!of_property_read_u32(np, "adi,dc-dc-phase", &tmp)) > + pdata->dc_dc_phase = tmp; > + else > + pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE; > + > + pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ; > + if (!of_property_read_u32(np, "adi,dc-dc-freq-hz", &tmp)) { > + for (i = 0; i < ARRAY_SIZE(ad5755_dcdc_freq_table); i++) { > + if (tmp == ad5755_dcdc_freq_table[i][0]) { > + pdata->dc_dc_freq = ad5755_dcdc_freq_table[i][1]; > + break; > + } > + } > + > + if (i == ARRAY_SIZE(ad5755_dcdc_freq_table)) { > + dev_err(dev, > + "adi,dc-dc-freq out of range selecting 410kHz"); > + } > + } > + > + pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V; > + if (!of_property_read_u32(np, "adi,dc-dc-max-microvolt", &tmp)) { > + for (i = 0; i < ARRAY_SIZE(ad5755_dcdc_maxv_table); i++) { > + if (tmp == ad5755_dcdc_maxv_table[i][0]) { > + pdata->dc_dc_maxv = ad5755_dcdc_maxv_table[i][1]; > + break; > + } > + } > + if (i == ARRAY_SIZE(ad5755_dcdc_maxv_table)) { > + dev_err(dev, > + "adi,dc-dc-maxv out of range selecting 23V"); > + } > + } > + > + devnr = 0; > + for_each_child_of_node(np, pp) { > + if (devnr > AD5755_NUM_CHANNELS) { > + dev_err(dev, > + "There is to many channels defined in DT\n"); > + goto error_out; > + } > + > + if (!of_property_read_u32(pp, "adi,mode", &tmp)) > + pdata->dac[devnr].mode = tmp; > + else > + pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA; > + > + pdata->dac[devnr].ext_current_sense_resistor = > + of_property_read_bool(pp, "adi,ext-current-sense-resistor"); > + > + pdata->dac[devnr].enable_voltage_overrange = > + of_property_read_bool(pp, "adi,enable-voltage-overrange"); > + > + if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) { > + pdata->dac[devnr].slew.enable = tmparray[0]; > + > + pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k; > + for (i = 0; i < ARRAY_SIZE(ad5755_slew_rate_table); i++) { > + if (tmparray[1] == ad5755_slew_rate_table[i][0]) { > + pdata->dac[devnr].slew.rate = > + ad5755_slew_rate_table[i][1]; > + break; > + } > + } > + if (i == ARRAY_SIZE(ad5755_slew_rate_table)) { > + dev_err(dev, > + "channel %d slew rate out of range selecting 64kHz", > + devnr); > + } > + > + pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1; > + for (i = 0; i < ARRAY_SIZE(ad5755_slew_step_table); i++) { > + if (tmparray[2] == ad5755_slew_step_table[i][0]) { > + pdata->dac[devnr].slew.step_size = > + ad5755_slew_step_table[i][1]; > + break; > + } > + } > + if (i == ARRAY_SIZE(ad5755_slew_step_table)) { > + dev_err(dev, > + "channel %d slew step size out of range selecting 1 LSB", > + devnr); > + } > + } else { > + pdata->dac[devnr].slew.enable = false; > + pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k; > + pdata->dac[devnr].slew.step_size = > + AD5755_SLEW_STEP_SIZE_1; > + } > + devnr++; > + } > + > + return pdata; > + > + error_out: > + devm_kfree(dev, pdata); > + return NULL; > +} > +#else > +static > +struct adf4350_platform_data *adf4350_parse_dt(struct device *dev) > +{ > + return NULL; > +} > +#endif > + > static int ad5755_probe(struct spi_device *spi) > { > enum ad5755_type type = spi_get_device_id(spi)->driver_data; > @@ -583,8 +750,15 @@ static int ad5755_probe(struct spi_device *spi) > indio_dev->modes = INDIO_DIRECT_MODE; > indio_dev->num_channels = AD5755_NUM_CHANNELS; > > - if (!pdata) > + if (spi->dev.of_node) > + pdata = ad5755_parse_dt(&spi->dev); > + else > + pdata = spi->dev.platform_data; > + > + if (!pdata) { > + dev_warn(&spi->dev, "no platform data? using default\n"); > pdata = &ad5755_default_pdata; > + } > > ret = ad5755_init_channels(indio_dev, pdata); > if (ret) > @@ -607,6 +781,16 @@ static const struct spi_device_id ad5755_id[] = { > }; > MODULE_DEVICE_TABLE(spi, ad5755_id); > > +static const struct of_device_id ad5755_of_match[] = { > + { .compatible = "adi,ad5755" }, > + { .compatible = "adi,ad5755-1" }, > + { .compatible = "adi,ad5757" }, > + { .compatible = "adi,ad5735" }, > + { .compatible = "adi,ad5737" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ad5755_of_match); > + > static struct spi_driver ad5755_driver = { > .driver = { > .name = "ad5755", >