From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1163925AbdD0SMM convert rfc822-to-8bit (ORCPT ); Thu, 27 Apr 2017 14:12:12 -0400 Received: from mx6-14.smtp.antispamcloud.com ([95.211.2.226]:35469 "EHLO mx6-14.smtp.antispamcloud.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S939557AbdD0SLv (ORCPT ); Thu, 27 Apr 2017 14:11:51 -0400 X-Greylist: delayed 3746 seconds by postgrey-1.27 at vger.kernel.org; Thu, 27 Apr 2017 14:11:51 EDT Subject: Re: [PATCH] iio:ad5064: Add support for ltc2633 and similar devices To: Lars-Peter Clausen , Jonathan Cameron , References: <1493199875-11912-1-git-send-email-mike.looijmans@topic.nl> <20b926f2-58ee-52d8-6520-a99e4640125d@kernel.org> <646b634f-c0ad-f39c-a395-02461f5e8352@metafoo.de> CC: , , , From: Mike Looijmans Organization: Topic Message-ID: <68f2f512-ec3e-c8ef-bf4a-5448f3b81f6b@topic.nl> Date: Thu, 27 Apr 2017 19:09:18 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <646b634f-c0ad-f39c-a395-02461f5e8352@metafoo.de> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8BIT X-Originating-IP: [85.150.144.104] X-EXCLAIMER-MD-CONFIG: 9833cda7-5b21-4d34-9a38-8d025ddc3664 X-EXCLAIMER-MD-BIFURCATION-INSTANCE: 0 X-Originating-IP: 37.74.225.130 X-SpamExperts-Domain: topic.nl X-SpamExperts-Username: 37.74.225.128/28 Authentication-Results: antispamcloud.com; auth=pass smtp.auth=37.74.225.128/28@topic.nl X-SpamExperts-Outgoing-Class: ham X-SpamExperts-Outgoing-Evidence: SB/global_tokens (1.48498491552e-05) X-Recommended-Action: accept X-Filter-ID: s0sct1PQhAABKnZB5plbIbbvfIHzQjPVmPLZeVYSu3xU9luQrU+8/8qthi+0Jd/W/95+6ZE6dI+d FNTaLLKrTD4PkK8WOoKuw32u9iEtZmisSr42qUwvK7U11fJ5smF5bE7GwnNXTE7JFCR1ojdwNIYU APp+8D0Yc6zwHLA8vb1FO9weOBB5jKmUQpbnrzZsFpivhm43NKrjkrK1UE5oqWbjO41FyBEqIaDu dcVplPGNaieKxw/vh63RdlaW2Zj1hcQD6Dxo9kokFZNwiacNPxeTCKssqlJePCDBHWfCYUvWOATT vnG+QUCcfrSs6FZc7tZ3vKwSN7Mv0HNteAusUZ8BuYwGqmI8Tx/Ism0pnCcmlWACu0WC7nIKSPYV aBAANeo6Hr4UJMIZEVOGlXeApA3I626B4k1VCg2pmPRu/tsfyK263qfUbK2DcIzvTAPCrnFLmOGc 2IjZAW0dTqZzwr3upx1n/FUf/NwbTI+uRoXlWvXEyVXmV1DcCGMoc6ig/SVaKi0jvtxxX4zA29pu 2ONYQFX+rhdRbxagFZk9QeU7sEKSfUWHty4XO0JvVoTKX2XX9bIsGDSYq5OAASmskVp/3TgIM4oj TBFr687rGjvGLWY+MgHYxfRtKtuAsj1l++DuIQUs/5JJj4C/n4CILr8EQbc1Bj2DWCrO5gJHZOZc RojozDt311EWh69LdsB7wRQnLmK5phjNWLSiBrApMgZ24itrInQFZPBtVxKxu3Jc5ZM17tXcNFnA aL+DkvzApG7M2OICIeicNQTOE0P6Qm9R4BJZ58JiPXkJGM8Rdws0NlDES5AoumH5pq0HbaC6nLDT BoRcmmOGfDwYGCxPFiuiJCh7gG91UBKPkwAvDYACbNU02VhqpxibdbLtzo8ts2d2itxp1vJgddxZ p4ySQBjyg3uWkt7I/OFV4ntayDMV10E2JxcfvZtyUGGivBKut8C9mOBdONdnsxgsk1D2p+313tCN ohmVb4CX5VdiYdUQ6/0l9rz9oPESCv9cUJlSjCBfb2nmbliF8YJYtEQzr6A9PZUdQfdj9WA2TnVq EtESTTsH9NeHFjLOqWWDPnzF X-Report-Abuse-To: spam@quarantine2.antispamcloud.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 27-04-17 11:16, Lars-Peter Clausen wrote: > On 04/27/2017 07:52 AM, Jonathan Cameron wrote: >> On 26/04/17 10:44, Mike Looijmans wrote: >>> The Linear Technology LTC2631, LTC2633 and LTC2635 are very similar >>> to the AD5064 device, in particular the LTC2627. >>> >>> This patch adds support for those devices. Only the LTC2633 has been >>> tested, which is the 2-channel variant. The LTC2631 is the 1-channel, >>> and the LTC2635 the 4-channel version. The actual DAC resolution depends >>> on the exact chip type and can be 12, 10 or 8 bits, using the upper bits >>> so this has no effect on the register map. The internal reference is set >>> to 2.5V on "L" versions, and it's 4.096V for "H" versions. >>> >>> Datasheets: >>> LTC2631: http://www.linear.com/docs/26553 >>> LTC2633: http://www.linear.com/docs/39529 >>> LTC2635: http://www.linear.com/docs/28754 >>> >>> Signed-off-by: Mike Looijmans > > Ah, its always good if somebody manages to clear an item from your TODO list > before you :) Glad to be able to reverse the roles this time :) >> Looks fine to me, but I'd like to give time for Lars to take a look >> as it is his driver. > > Patch looks good, but I'd prefer it if we had different entries in the > device table for different resolutions. So you don't have to manually shift > the output value by 12 when you are using the 8-bit version. This is how it > has been done for other DAC drivers and it would be good to stay consistent. Doing so would add 18 entries (3 channels, 2 ranges, 3 resolutions) instead of a mere 6. That's why I refrained from it. > > Maybe even include the reset-to-midscale/reset-to-gnd designator of the part > number in the compatible string. > > http://cds.linear.com/docs/en/datasheet/2631fc.pdf#page=3 And that would make the number of entries to add 36. I think if we go that path, we should make things parameterizable, e.g. have channel count, bit resolution, reset state and range as devicetree parameters, so that the "compatible" string needn't hold all information. It would impact existing trees though. >> Jonathan >>> --- >>> drivers/iio/dac/Kconfig | 3 ++- >>> drivers/iio/dac/ad5064.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++-- >>> 2 files changed, 58 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig >>> index d3084028..31ffb67 100644 >>> --- a/drivers/iio/dac/Kconfig >>> +++ b/drivers/iio/dac/Kconfig >>> @@ -13,7 +13,8 @@ config AD5064 >>> AD5045, AD5064, AD5064-1, AD5065, AD5625, AD5625R, AD5627, AD5627R, >>> AD5628, AD5629R, AD5645R, AD5647R, AD5648, AD5665, AD5665R, AD5666, >>> AD5667, AD5667R, AD5668, AD5669R, LTC2606, LTC2607, LTC2609, LTC2616, >>> - LTC2617, LTC2619, LTC2626, LTC2627, LTC2629 Digital to Analog Converter. >>> + LTC2617, LTC2619, LTC2626, LTC2627, LTC2629, LTC2631, LTC2633, LTC2635 >>> + Digital to Analog Converter. >>> >>> To compile this driver as a module, choose M here: the >>> module will be called ad5064. >>> diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c >>> index 6803e4a..b440180 100644 >>> --- a/drivers/iio/dac/ad5064.c >>> +++ b/drivers/iio/dac/ad5064.c >>> @@ -2,8 +2,8 @@ >>> * AD5024, AD5025, AD5044, AD5045, AD5064, AD5064-1, AD5065, AD5625, AD5625R, >>> * AD5627, AD5627R, AD5628, AD5629R, AD5645R, AD5647R, AD5648, AD5665, AD5665R, >>> * AD5666, AD5667, AD5667R, AD5668, AD5669R, LTC2606, LTC2607, LTC2609, LTC2616, >>> - * LTC2617, LTC2619, LTC2626, LTC2627, LTC2629 Digital to analog converters >>> - * driver >>> + * LTC2617, LTC2619, LTC2626, LTC2627, LTC2629, LTC2631, LTC2633, LTC2635 >>> + * Digital to analog converters driver >>> * >>> * Copyright 2011 Analog Devices Inc. >>> * >>> @@ -168,6 +168,12 @@ enum ad5064_type { >>> ID_LTC2626, >>> ID_LTC2627, >>> ID_LTC2629, >>> + ID_LTC2631_L, >>> + ID_LTC2631_H, >>> + ID_LTC2633_L, >>> + ID_LTC2633_H, >>> + ID_LTC2635_L, >>> + ID_LTC2635_H, >>> }; >>> >>> static int ad5064_write(struct ad5064_state *st, unsigned int cmd, >>> @@ -724,6 +730,48 @@ static int ad5064_write_raw(struct iio_dev *indio_dev, >>> .num_channels = 4, >>> .regmap_type = AD5064_REGMAP_LTC, >>> }, >>> + [ID_LTC2631_L] = { >>> + .shared_vref = true, >>> + .internal_vref = 2500000, >>> + .channels = ltc2627_channels, >>> + .num_channels = 1, >>> + .regmap_type = AD5064_REGMAP_LTC, >>> + }, >>> + [ID_LTC2631_H] = { >>> + .shared_vref = true, >>> + .internal_vref = 4096000, >>> + .channels = ltc2627_channels, >>> + .num_channels = 1, >>> + .regmap_type = AD5064_REGMAP_LTC, >>> + }, >>> + [ID_LTC2633_L] = { >>> + .shared_vref = true, >>> + .internal_vref = 2500000, >>> + .channels = ltc2627_channels, >>> + .num_channels = 2, >>> + .regmap_type = AD5064_REGMAP_LTC, >>> + }, >>> + [ID_LTC2633_H] = { >>> + .shared_vref = true, >>> + .internal_vref = 4096000, >>> + .channels = ltc2627_channels, >>> + .num_channels = 2, >>> + .regmap_type = AD5064_REGMAP_LTC, >>> + }, >>> + [ID_LTC2635_L] = { >>> + .shared_vref = true, >>> + .internal_vref = 2500000, >>> + .channels = ltc2627_channels, >>> + .num_channels = 4, >>> + .regmap_type = AD5064_REGMAP_LTC, >>> + }, >>> + [ID_LTC2635_H] = { >>> + .shared_vref = true, >>> + .internal_vref = 4096000, >>> + .channels = ltc2627_channels, >>> + .num_channels = 4, >>> + .regmap_type = AD5064_REGMAP_LTC, >>> + }, >>> }; >>> >>> static inline unsigned int ad5064_num_vref(struct ad5064_state *st) >>> @@ -982,6 +1030,12 @@ static int ad5064_i2c_remove(struct i2c_client *i2c) >>> {"ltc2626", ID_LTC2626}, >>> {"ltc2627", ID_LTC2627}, >>> {"ltc2629", ID_LTC2629}, >>> + {"ltc2631-l", ID_LTC2631_L}, >>> + {"ltc2631-h", ID_LTC2631_H}, >>> + {"ltc2633-l", ID_LTC2633_L}, >>> + {"ltc2633-h", ID_LTC2633_H}, >>> + {"ltc2635-l", ID_LTC2635_L}, >>> + {"ltc2635-h", ID_LTC2635_H}, >>> {} >>> }; >>> MODULE_DEVICE_TABLE(i2c, ad5064_i2c_ids); >>> >> > -- Mike Looijmans Kind regards, Mike Looijmans System Expert TOPIC Products Materiaalweg 4, NL-5681 RJ Best Postbus 440, NL-5680 AK Best Telefoon: +31 (0) 499 33 69 79 E-mail: mike.looijmans@topicproducts.com Website: www.topicproducts.com Please consider the environment before printing this e-mail