Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751972AbdDAK4f (ORCPT ); Sat, 1 Apr 2017 06:56:35 -0400 Received: from saturn.retrosnub.co.uk ([178.18.118.26]:41064 "EHLO saturn.retrosnub.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751590AbdDAK4c (ORCPT ); Sat, 1 Apr 2017 06:56:32 -0400 Subject: Re: [PATCH v5 2/2] iio: Aspeed ADC To: Xo Wang , Joel Stanley References: <20170328215259.31622-1-raltherr@google.com> <20170328215259.31622-2-raltherr@google.com> Cc: Rick Altherr , Rob Herring , Raveendra Padasalagi , Lars-Peter Clausen , Fabrice Gasnier , Scott Branden , linux-iio@vger.kernel.org, Zhiyong Tao , Michael Turquette , Stephen Boyd , Linux Kernel Mailing List , William Breathitt Gray , linux-clk@vger.kernel.org, Quentin Schulz , Akinobu Mita , Geert Uytterhoeven , Andreas Klinger , Peter Meerwald-Stadler , Hartmut Knaack , Crestez Dan Leonard , OpenBMC Maillist From: Jonathan Cameron Message-ID: <661ef6ee-ca1a-7188-7e11-3e0084c54082@kernel.org> Date: Sat, 1 Apr 2017 11:56:29 +0100 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: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 15418 Lines: 403 On 31/03/17 21:57, Xo Wang wrote: > On Tue, Mar 28, 2017 at 7:33 PM, Joel Stanley wrote: >> On Wed, Mar 29, 2017 at 8:22 AM, Rick Altherr wrote: >>> Aspeed BMC SoCs include a 16 channel, 10-bit ADC. Low and high threshold >>> interrupts are supported by the hardware but are not currently implemented. >>> >>> Signed-off-by: Rick Altherr >> >> Reviewed-by: Joel Stanley >> > > Tested-by: Xo Wang > > Tested on a Zaius OpenPOWER server running a AST2500 BMC alongside configs: > > CONFIG_IIO=y > CONFIG_ASPEED_ADC=y > CONFIG_SENSORS_IIO_HWMON=y > > as well as the appropriate device tree entries. Thanks Xo and Joel, Testing and reviewing is sometimes a thankless task so it is always nice when several people do these for a new driver! Jonathan > >>> --- >>> >>> Changes in v5: >>> - Return EINVAL for unimplemented read/write channel infos. >>> - Return EPERM for write attempts to read-only channel infos. >>> >>> Changes in v4: >>> - Avoid copying per-model data to per-instance data >>> >>> Changes in v3: >>> - Drop model numbers from description as same IP is used in every generation >>> - Remove unused macros >>> - Shorten macro prefix to just ASPEED_ >>> - Remove extraneous whitespace >>> - Rename 2nd argument of ASPEED_CHAN() to clarify purpose >>> - Use existing macros in place of magic constants >>> - Remove unnecessary logging in failure conditions during probe >>> - Disable ADC hardware during remove() >>> - Add a NULL terminator to of_match_table >>> - Add per-model data to accomodate slight variations (vref, min/max sample rate) >>> - Added missing Kconfig depends on COMMON_CLK >>> >>> Changes in v2: >>> - Rewritten as an IIO device >>> - Renamed register macros to describe the register's purpose >>> - Replaced awkward reading of 16-bit data registers with readw() >>> - Added Kconfig dependency on COMPILE_TEST >>> >>> drivers/iio/adc/Kconfig | 11 ++ >>> drivers/iio/adc/Makefile | 1 + >>> drivers/iio/adc/aspeed_adc.c | 294 +++++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 306 insertions(+) >>> create mode 100644 drivers/iio/adc/aspeed_adc.c >>> >>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig >>> index 2268a6fb9865..236d05f99e80 100644 >>> --- a/drivers/iio/adc/Kconfig >>> +++ b/drivers/iio/adc/Kconfig >>> @@ -130,6 +130,17 @@ config AD799X >>> To compile this driver as a module, choose M here: the module will be >>> called ad799x. >>> >>> +config ASPEED_ADC >>> + tristate "Aspeed ADC" >>> + depends on ARCH_ASPEED || COMPILE_TEST >>> + depends on COMMON_CLK >>> + help >>> + If you say yes here you get support for the ADC included in Aspeed >>> + BMC SoCs. >>> + >>> + To compile this driver as a module, choose M here: the module will be >>> + called aspeed_adc. >>> + >>> config AT91_ADC >>> tristate "Atmel AT91 ADC" >>> depends on ARCH_AT91 >>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile >>> index 73dbe399f894..306f10ffca74 100644 >>> --- a/drivers/iio/adc/Makefile >>> +++ b/drivers/iio/adc/Makefile >>> @@ -14,6 +14,7 @@ obj-$(CONFIG_AD7791) += ad7791.o >>> obj-$(CONFIG_AD7793) += ad7793.o >>> obj-$(CONFIG_AD7887) += ad7887.o >>> obj-$(CONFIG_AD799X) += ad799x.o >>> +obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o >>> obj-$(CONFIG_AT91_ADC) += at91_adc.o >>> obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o >>> obj-$(CONFIG_AXP288_ADC) += axp288_adc.o >>> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c >>> new file mode 100644 >>> index 000000000000..2283ed202194 >>> --- /dev/null >>> +++ b/drivers/iio/adc/aspeed_adc.c >>> @@ -0,0 +1,294 @@ >>> +/* >>> + * Aspeed AST2400/2500 ADC >>> + * >>> + * Copyright (C) 2017 Google, Inc. >>> + * >>> + * This program is free software; you can redistribute it and/or modify it >>> + * under the terms and conditions of the GNU General Public License, >>> + * version 2, as published by the Free Software Foundation. >>> + * >>> + */ >>> + >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +#include >>> +#include >>> + >>> +#define ASPEED_RESOLUTION_BITS 10 >>> +#define ASPEED_CLOCKS_PER_SAMPLE 12 >>> + >>> +#define ASPEED_REG_ENGINE_CONTROL 0x00 >>> +#define ASPEED_REG_INTERRUPT_CONTROL 0x04 >>> +#define ASPEED_REG_VGA_DETECT_CONTROL 0x08 >>> +#define ASPEED_REG_CLOCK_CONTROL 0x0C >>> +#define ASPEED_REG_MAX 0xC0 >>> + >>> +#define ASPEED_OPERATION_MODE_POWER_DOWN (0x0 << 1) >>> +#define ASPEED_OPERATION_MODE_STANDBY (0x1 << 1) >>> +#define ASPEED_OPERATION_MODE_NORMAL (0x7 << 1) >>> + >>> +#define ASPEED_ENGINE_ENABLE BIT(0) >>> + >>> +struct aspeed_adc_model_data { >>> + const char *model_name; >>> + unsigned int min_sampling_rate; // Hz >>> + unsigned int max_sampling_rate; // Hz >>> + unsigned int vref_voltage; // mV >>> +}; >>> + >>> +struct aspeed_adc_data { >>> + struct device *dev; >>> + void __iomem *base; >>> + spinlock_t clk_lock; >>> + struct clk_hw *clk_prescaler; >>> + struct clk_hw *clk_scaler; >>> +}; >>> + >>> +#define ASPEED_CHAN(_idx, _data_reg_addr) { \ >>> + .type = IIO_VOLTAGE, \ >>> + .indexed = 1, \ >>> + .channel = (_idx), \ >>> + .address = (_data_reg_addr), \ >>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ >>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \ >>> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \ >>> +} >>> + >>> +static const struct iio_chan_spec aspeed_adc_iio_channels[] = { >>> + ASPEED_CHAN(0, 0x10), >>> + ASPEED_CHAN(1, 0x12), >>> + ASPEED_CHAN(2, 0x14), >>> + ASPEED_CHAN(3, 0x16), >>> + ASPEED_CHAN(4, 0x18), >>> + ASPEED_CHAN(5, 0x1A), >>> + ASPEED_CHAN(6, 0x1C), >>> + ASPEED_CHAN(7, 0x1E), >>> + ASPEED_CHAN(8, 0x20), >>> + ASPEED_CHAN(9, 0x22), >>> + ASPEED_CHAN(10, 0x24), >>> + ASPEED_CHAN(11, 0x26), >>> + ASPEED_CHAN(12, 0x28), >>> + ASPEED_CHAN(13, 0x2A), >>> + ASPEED_CHAN(14, 0x2C), >>> + ASPEED_CHAN(15, 0x2E), >>> +}; >>> + >>> +static int aspeed_adc_read_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, >>> + int *val, int *val2, long mask) >>> +{ >>> + struct aspeed_adc_data *data = iio_priv(indio_dev); >>> + const struct aspeed_adc_model_data *model_data = >>> + of_device_get_match_data(data->dev); >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_RAW: >>> + *val = readw(data->base + chan->address); >>> + return IIO_VAL_INT; >>> + >>> + case IIO_CHAN_INFO_SCALE: >>> + *val = model_data->vref_voltage; >>> + *val2 = ASPEED_RESOLUTION_BITS; >>> + return IIO_VAL_FRACTIONAL_LOG2; >>> + >>> + case IIO_CHAN_INFO_SAMP_FREQ: >>> + *val = clk_get_rate(data->clk_scaler->clk) / >>> + ASPEED_CLOCKS_PER_SAMPLE; >>> + return IIO_VAL_INT; >>> + >>> + default: >>> + return -EINVAL; >>> + } >>> +} >>> + >>> +static int aspeed_adc_write_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, >>> + int val, int val2, long mask) >>> +{ >>> + struct aspeed_adc_data *data = iio_priv(indio_dev); >>> + const struct aspeed_adc_model_data *model_data = >>> + of_device_get_match_data(data->dev); >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_SAMP_FREQ: >>> + if (val < model_data->min_sampling_rate || >>> + val > model_data->max_sampling_rate) >>> + return -EINVAL; >>> + >>> + clk_set_rate(data->clk_scaler->clk, >>> + val * ASPEED_CLOCKS_PER_SAMPLE); >>> + return 0; >>> + >>> + case IIO_CHAN_INFO_SCALE: >>> + case IIO_CHAN_INFO_RAW: >>> + /* >>> + * Technically, these could be written but the only reasons >>> + * for doing so seem better handled in userspace. EPERM is >>> + * returned to signal this is a policy choice rather than a >>> + * hardware limitation. >>> + */ >>> + return -EPERM; >>> + >>> + default: >>> + return -EINVAL; >>> + } >>> +} >>> + >>> +static int aspeed_adc_reg_access(struct iio_dev *indio_dev, >>> + unsigned int reg, unsigned int writeval, >>> + unsigned int *readval) >>> +{ >>> + struct aspeed_adc_data *data = iio_priv(indio_dev); >>> + >>> + if (!readval || reg % 4 || reg > ASPEED_REG_MAX) >>> + return -EINVAL; >>> + >>> + *readval = readl(data->base + reg); >>> + return 0; >>> +} >>> + >>> +static const struct iio_info aspeed_adc_iio_info = { >>> + .driver_module = THIS_MODULE, >>> + .read_raw = aspeed_adc_read_raw, >>> + .write_raw = aspeed_adc_write_raw, >>> + .debugfs_reg_access = aspeed_adc_reg_access, >>> +}; >>> + >>> +static int aspeed_adc_probe(struct platform_device *pdev) >>> +{ >>> + struct iio_dev *indio_dev; >>> + struct aspeed_adc_data *data; >>> + const struct aspeed_adc_model_data *model_data; >>> + struct resource *res; >>> + const char *clk_parent_name; >>> + int ret; >>> + u32 adc_engine_control_reg_val; >>> + >>> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*data)); >>> + if (!indio_dev) >>> + return -ENOMEM; >>> + >>> + data = iio_priv(indio_dev); >>> + data->dev = &pdev->dev; >>> + >>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> + data->base = devm_ioremap_resource(&pdev->dev, res); >>> + if (IS_ERR(data->base)) >>> + return PTR_ERR(data->base); >>> + >>> + /* Register ADC clock prescaler with source specified by device tree. */ >>> + spin_lock_init(&data->clk_lock); >>> + clk_parent_name = of_clk_get_parent_name(pdev->dev.of_node, 0); >>> + >>> + data->clk_prescaler = clk_hw_register_divider( >>> + &pdev->dev, "prescaler", clk_parent_name, 0, >>> + data->base + ASPEED_REG_CLOCK_CONTROL, >>> + 17, 15, 0, &data->clk_lock); >>> + if (IS_ERR(data->clk_prescaler)) >>> + return PTR_ERR(data->clk_prescaler); >>> + >>> + /* >>> + * Register ADC clock scaler downstream from the prescaler. Allow rate >>> + * setting to adjust the prescaler as well. >>> + */ >>> + data->clk_scaler = clk_hw_register_divider( >>> + &pdev->dev, "scaler", "prescaler", >>> + CLK_SET_RATE_PARENT, >>> + data->base + ASPEED_REG_CLOCK_CONTROL, >>> + 0, 10, 0, &data->clk_lock); >>> + if (IS_ERR(data->clk_scaler)) { >>> + ret = PTR_ERR(data->clk_scaler); >>> + goto scaler_error; >>> + } >>> + >>> + /* Start all channels in normal mode. */ >>> + clk_prepare_enable(data->clk_scaler->clk); >>> + adc_engine_control_reg_val = GENMASK(31, 16) | >>> + ASPEED_OPERATION_MODE_NORMAL | ASPEED_ENGINE_ENABLE; >>> + writel(adc_engine_control_reg_val, >>> + data->base + ASPEED_REG_ENGINE_CONTROL); >>> + >>> + model_data = of_device_get_match_data(&pdev->dev); >>> + indio_dev->name = model_data->model_name; >>> + indio_dev->dev.parent = &pdev->dev; >>> + indio_dev->info = &aspeed_adc_iio_info; >>> + indio_dev->modes = INDIO_DIRECT_MODE; >>> + indio_dev->channels = aspeed_adc_iio_channels; >>> + indio_dev->num_channels = ARRAY_SIZE(aspeed_adc_iio_channels); >>> + >>> + ret = iio_device_register(indio_dev); >>> + if (ret) >>> + goto iio_register_error; >>> + >>> + return 0; >>> + >>> +iio_register_error: >>> + writel(ASPEED_OPERATION_MODE_POWER_DOWN, >>> + data->base + ASPEED_REG_ENGINE_CONTROL); >>> + clk_disable_unprepare(data->clk_scaler->clk); >>> + clk_hw_unregister_divider(data->clk_scaler); >>> + >>> +scaler_error: >>> + clk_hw_unregister_divider(data->clk_prescaler); >>> + return ret; >>> +} >>> + >>> +static int aspeed_adc_remove(struct platform_device *pdev) >>> +{ >>> + struct iio_dev *indio_dev = platform_get_drvdata(pdev); >>> + struct aspeed_adc_data *data = iio_priv(indio_dev); >>> + >>> + iio_device_unregister(indio_dev); >>> + writel(ASPEED_OPERATION_MODE_POWER_DOWN, >>> + data->base + ASPEED_REG_ENGINE_CONTROL); >>> + clk_disable_unprepare(data->clk_scaler->clk); >>> + clk_hw_unregister_divider(data->clk_scaler); >>> + clk_hw_unregister_divider(data->clk_prescaler); >>> + >>> + return 0; >>> +} >>> + >>> +static const struct aspeed_adc_model_data ast2400_model_data = { >>> + .model_name = "ast2400-adc", >>> + .vref_voltage = 2500, // mV >>> + .min_sampling_rate = 10000, >>> + .max_sampling_rate = 500000, >>> +}; >>> + >>> +static const struct aspeed_adc_model_data ast2500_model_data = { >>> + .model_name = "ast2500-adc", >>> + .vref_voltage = 1800, // mV >>> + .min_sampling_rate = 1, >>> + .max_sampling_rate = 1000000, >>> +}; >>> + >>> +static const struct of_device_id aspeed_adc_matches[] = { >>> + { .compatible = "aspeed,ast2400-adc", .data = &ast2400_model_data }, >>> + { .compatible = "aspeed,ast2500-adc", .data = &ast2500_model_data }, >>> + {}, >>> +}; >>> +MODULE_DEVICE_TABLE(of, aspeed_adc_matches); >>> + >>> +static struct platform_driver aspeed_adc_driver = { >>> + .probe = aspeed_adc_probe, >>> + .remove = aspeed_adc_remove, >>> + .driver = { >>> + .name = KBUILD_MODNAME, >>> + .of_match_table = aspeed_adc_matches, >>> + } >>> +}; >>> + >>> +module_platform_driver(aspeed_adc_driver); >>> + >>> +MODULE_AUTHOR("Rick Altherr "); >>> +MODULE_DESCRIPTION("Aspeed AST2400/2500 ADC Driver"); >>> +MODULE_LICENSE("GPL"); >>> -- >>> 2.12.2.564.g063fe858b8-goog >>> > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >