lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <661ef6ee-ca1a-7188-7e11-3e0084c54082@kernel.org>
Date:   Sat, 1 Apr 2017 11:56:29 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Xo Wang <xow@...gle.com>, Joel Stanley <joel@....id.au>
Cc:     Rick Altherr <raltherr@...gle.com>, Rob Herring <robh@...nel.org>,
        Raveendra Padasalagi <raveendra.padasalagi@...adcom.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Fabrice Gasnier <fabrice.gasnier@...com>,
        Scott Branden <scott.branden@...adcom.com>,
        linux-iio@...r.kernel.org, Zhiyong Tao <zhiyong.tao@...iatek.com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...eaurora.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        William Breathitt Gray <vilhelm.gray@...il.com>,
        linux-clk@...r.kernel.org,
        Quentin Schulz <quentin.schulz@...e-electrons.com>,
        Akinobu Mita <akinobu.mita@...il.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Andreas Klinger <ak@...klinger.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Hartmut Knaack <knaack.h@....de>,
        Crestez Dan Leonard <leonard.crestez@...el.com>,
        OpenBMC Maillist <openbmc@...ts.ozlabs.org>
Subject: Re: [PATCH v5 2/2] iio: Aspeed ADC

On 31/03/17 21:57, Xo Wang wrote:
> On Tue, Mar 28, 2017 at 7:33 PM, Joel Stanley <joel@....id.au> wrote:
>> On Wed, Mar 29, 2017 at 8:22 AM, Rick Altherr <raltherr@...gle.com> 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 <raltherr@...gle.com>
>>
>> Reviewed-by: Joel Stanley <joel@....id.au>
>>
> 
> Tested-by: Xo Wang <xow@...gle.com>
> 
> 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 <asm/io.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/err.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_platform.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/spinlock.h>
>>> +#include <linux/types.h>
>>> +
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/driver.h>
>>> +
>>> +#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 <raltherr@...gle.com>");
>>> +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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ