[{"id":1768123,"web_url":"http://patchwork.ozlabs.org/comment/1768123/","msgid":"","list_archive_url":null,"date":"2017-09-13T20:07:40","subject":"Re: [U-Boot] [U-Boot,1/8] adc: Add driver for Rockchip Saradc","submitter":{"id":53488,"url":"http://patchwork.ozlabs.org/api/people/53488/","name":"Philipp Tomsich","email":"philipp.tomsich@theobroma-systems.com"},"content":"> The ADC can support some channels signal-ended some bits Successive Approximation\n> Register (SAR) A/D Converter, like 6-channel and 10-bit. It converts the analog\n> input signal into some bits binary digital codes.\n> \n> Signed-off-by: David Wu \n> ---\n> drivers/adc/Kconfig | 9 ++\n> drivers/adc/Makefile | 1 +\n> drivers/adc/rockchip-saradc.c | 188 ++++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 198 insertions(+)\n> create mode 100644 drivers/adc/rockchip-saradc.c\n> \n\nAcked-by: Philipp Tomsich ","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xstBy1TSBz9rxj\n\tfor ;\n\tThu, 14 Sep 2017 06:13:14 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid D6741C2232C; Wed, 13 Sep 2017 20:09:50 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 4CA74C222A0;\n\tWed, 13 Sep 2017 20:08:36 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 5018BC220B1; Wed, 13 Sep 2017 20:07:47 +0000 (UTC)","from mail.theobroma-systems.com (vegas.theobroma-systems.com\n\t[144.76.126.164])\n\tby lists.denx.de (Postfix) with ESMTPS id 7C054C22168\n\tfor ; Wed, 13 Sep 2017 20:07:44 +0000 (UTC)","from 89-104-28-141.customer.bnet.at ([89.104.28.141]:62926\n\thelo=vpn-10-11-0-14.lan) by mail.theobroma-systems.com with esmtpsa\n\t(TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80)\n\t(envelope-from )\n\tid 1dsDwi-0004RU-Ku; Wed, 13 Sep 2017 22:07:40 +0200"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=0.0 required=5.0 tests=none autolearn=unavailable\n\tautolearn_force=no version=3.4.0","MIME-Version":"1.0","From":"Philipp Tomsich ","To":"David Wu ","In-Reply-To":"<1505297379-12638-2-git-send-email-david.wu@rock-chips.com>","References":"<1505297379-12638-2-git-send-email-david.wu@rock-chips.com>","Message-Id":"","Date":"Wed, 13 Sep 2017 22:07:40 +0200","Cc":"huangtao@rock-chips.com, linux-rockchip@lists.infradead.org,\n\tzhangqing@rock-chips.com, u-boot@lists.denx.de, p.marczak@samsung.com,\n\tDavid Wu , andy.yan@rock-chips.com,\n\tchenjh@rock-chips.com","Subject":"Re: [U-Boot] [U-Boot,1/8] adc: Add driver for Rockchip Saradc","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1768156,"web_url":"http://patchwork.ozlabs.org/comment/1768156/","msgid":"","list_archive_url":null,"date":"2017-09-13T20:40:27","subject":"Re: [U-Boot] [U-Boot,1/8] adc: Add driver for Rockchip Saradc","submitter":{"id":53488,"url":"http://patchwork.ozlabs.org/api/people/53488/","name":"Philipp Tomsich","email":"philipp.tomsich@theobroma-systems.com"},"content":"On Wed, 13 Sep 2017, David Wu wrote:\n\n> The ADC can support some channels signal-ended some bits Successive Approximation\n> Register (SAR) A/D Converter, like 6-channel and 10-bit. It converts the analog\n> input signal into some bits binary digital codes.\n>\n> Signed-off-by: David Wu \n\nReviewed-by: Philipp Tomsich \n\nPlease see below for requested changes.\n\n> ---\n> drivers/adc/Kconfig | 9 ++\n> drivers/adc/Makefile | 1 +\n> drivers/adc/rockchip-saradc.c | 188 ++++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 198 insertions(+)\n> create mode 100644 drivers/adc/rockchip-saradc.c\n>\n> diff --git a/drivers/adc/Kconfig b/drivers/adc/Kconfig\n> index e5335f7..830fe0f 100644\n> --- a/drivers/adc/Kconfig\n> +++ b/drivers/adc/Kconfig\n> @@ -20,6 +20,15 @@ config ADC_EXYNOS\n> \t - 12-bit resolution\n> \t - 600 KSPS of sample rate\n>\n> +config SARADC_ROCKCHIP\n> +\tbool \"Enable Rockchip SARADC driver\"\n> +\thelp\n> +\t This enables driver for Rockchip SARADC.\n> +\t It provides:\n> +\t - 2~6 analog input channels\n> +\t - 1O-bit resolution\n> +\t - 1MSPS of sample rate\n> +\n> config ADC_SANDBOX\n> \tbool \"Enable Sandbox ADC test driver\"\n> \thelp\n> diff --git a/drivers/adc/Makefile b/drivers/adc/Makefile\n> index cebf26d..4b5aa69 100644\n> --- a/drivers/adc/Makefile\n> +++ b/drivers/adc/Makefile\n> @@ -8,3 +8,4 @@\n> obj-$(CONFIG_ADC) += adc-uclass.o\n> obj-$(CONFIG_ADC_EXYNOS) += exynos-adc.o\n> obj-$(CONFIG_ADC_SANDBOX) += sandbox.o\n> +obj-$(CONFIG_SARADC_ROCKCHIP) += rockchip-saradc.o\n\nDo you feel strongly about the \"SARADC_ROCKCHIP\" or would \"ADC_ROCKCHIP\" \nbe correct as well? I don't care either way, but this is the first entry \nhere that does not start with CONFIG_ADC_, so I am wondering...\n\n> diff --git a/drivers/adc/rockchip-saradc.c b/drivers/adc/rockchip-saradc.c\n> new file mode 100644\n> index 0000000..5c7c3d9\n> --- /dev/null\n> +++ b/drivers/adc/rockchip-saradc.c\n> @@ -0,0 +1,188 @@\n> +/*\n> + * (C) Copyright 2017, Fuzhou Rockchip Electronics Co., Ltd\n> + *\n> + * SPDX-License-Identifier:\tGPL-2.0+\n> + *\n> + * Rockchip Saradc driver for U-Boot\n\nShould this be SARADC (all uppercase)?\n\n> + */\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n\nPlease see https://www.denx.de/wiki/U-Boot/CodingStyle for the include \norder. Please revise.\n\n@Simon: yes, I am quick study ;-)\n\n> +\n> +#define SARADC_DATA\t\t\t0x00\n> +\n> +#define SARADC_STAS\t\t\t0x04\n\nPlease see https://www.denx.de/wiki/U-Boot/CodingStyle for the guidelines \non using structures for I/O accesses. Please revise.\n\n> +#define SARADC_STAS_BUSY\t\tBIT(0)\n> +\n> +#define SARADC_CTRL\t\t\t0x08\n> +#define SARADC_CTRL_POWER_CTRL\t\tBIT(3)\n> +#define SARADC_CTRL_CHN_MASK\t\t0x7\n> +#define SARADC_CTRL_IRQ_STATUS\t\tBIT(6)\n> +#define SARADC_CTRL_IRQ_ENABLE\t\tBIT(5)\n> +\n> +#define SARADC_DLY_PU_SOC\t\t0x0c\n> +\n> +#define SARADC_TIMEOUT\t\t\t(100 * 1000)\n> +\n> +struct rockchip_saradc_data {\n> +\tint\t\t\t\tnum_bits;\n> +\tint\t\t\t\tnum_channels;\n> +\tunsigned long\t\t\tclk_rate;\n> +};\n> +\n> +struct rockchip_saradc_priv {\n> +\tfdt_addr_t\t\t\t\tregs;\n> +\tint \t\t\t\t\tactive_channel;\n> +\tconst struct rockchip_saradc_data\t*data;\n> +};\n> +\n> +int rockchip_saradc_channel_data(struct udevice *dev, int channel,\n> +\t\t\t unsigned int *data)\n> +{\n> +\tstruct rockchip_saradc_priv *priv = dev_get_priv(dev);\n> +\n> +\tif (channel != priv->active_channel) {\n> +\t\terror(\"Requested channel is not active!\");\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tif ((readl(priv->regs + SARADC_CTRL) & SARADC_CTRL_IRQ_STATUS) != SARADC_CTRL_IRQ_STATUS)\n> +\t\treturn -EBUSY;\n> +\n> +\t/* Read value */\n> +\t*data = readl(priv->regs + SARADC_DATA);\n> +\t*data &= (1 << priv->data->num_bits) - 1;\n\nThis recomputes the data_mask (from below).\nCan we just use the data_mask again?\n\n> +\n> +\t/* Power down adc */\n> +\twritel(0, priv->regs + SARADC_CTRL);\n> +\n> +\treturn 0;\n> +}\n> +\n> +int rockchip_saradc_start_channel(struct udevice *dev, int channel)\n> +{\n> +\tstruct rockchip_saradc_priv *priv = dev_get_priv(dev);\n> +\n> +\tif (channel < 0 || channel >= priv->data->num_channels) {\n> +\t\terror(\"Requested channel is invalid!\");\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\t/* 8 clock periods as delay between power up and start cmd */\n> +\twritel(8, priv->regs + SARADC_DLY_PU_SOC);\n> +\n> +\t/* Select the channel to be used and trigger conversion */\n> +\twritel(SARADC_CTRL_POWER_CTRL\n> +\t\t\t| (channel & SARADC_CTRL_CHN_MASK) | SARADC_CTRL_IRQ_ENABLE,\n\nThis line does not match the style guideline: it is too wide and the \noperator should be before the line-break.\n\nDid you run checkpatch or use patman?\n\n> +\t\t priv->regs + SARADC_CTRL);\n> +\n> +\tpriv->active_channel = channel;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int rockchip_saradc_stop(struct udevice *dev)\n> +{\n> +\tstruct rockchip_saradc_priv *priv = dev_get_priv(dev);\n> +\n> +\t/* Power down adc */\n> +\twritel(0, priv->regs + SARADC_CTRL);\n> +\n> +\tpriv->active_channel = -1;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int rockchip_saradc_probe(struct udevice *dev)\n> +{\n> +\tstruct rockchip_saradc_priv *priv = dev_get_priv(dev);\n> +\tstruct clk clk;\n> +\tint ret;\n> +\n> +\tret = clk_get_by_index(dev, 0, &clk);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = clk_set_rate(&clk, priv->data->clk_rate);\n> +\tif (IS_ERR_VALUE(ret))\n> +\t\treturn ret;\n> +\n> +\tpriv->active_channel = -1;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int rockchip_saradc_ofdata_to_platdata(struct udevice *dev)\n> +{\n> +\tstruct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);\n> +\tstruct rockchip_saradc_priv *priv = dev_get_priv(dev);\n> +\tstruct rockchip_saradc_data *data =\n> +\t\t\t\t\t(struct rockchip_saradc_data *)dev_get_driver_data(dev);\n> +\n> +\tpriv->regs = devfdt_get_addr(dev);\n\nShouldn't this be dev_read_addr?\n\n> +\tif (priv->regs == FDT_ADDR_T_NONE) {\n> +\t\terror(\"Dev: %s - can't get address!\", dev->name);\n> +\t\treturn -ENODATA;\n> +\t}\n> +\n> +\tpriv->data = data;\n> +\tuc_pdata->data_mask = (1 << priv->data->num_bits) - 1;;\n> +\tuc_pdata->data_format = ADC_DATA_FORMAT_BIN;\n> +\tuc_pdata->data_timeout_us = SARADC_TIMEOUT / 5;\n> +\tuc_pdata->channel_mask = (1 << priv->data->num_channels) - 1;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static const struct adc_ops rockchip_saradc_ops = {\n> +\t.start_channel = rockchip_saradc_start_channel,\n> +\t.channel_data = rockchip_saradc_channel_data,\n> +\t.stop = rockchip_saradc_stop,\n> +};\n> +\n> +static const struct rockchip_saradc_data saradc_data = {\n> +\t.num_bits = 10,\n> +\t.num_channels = 3,\n> +\t.clk_rate = 1000000,\n> +};\n> +\n> +static const struct rockchip_saradc_data rk3066_tsadc_data = {\n> +\t.num_bits = 12,\n> +\t.num_channels = 2,\n> +\t.clk_rate = 50000,\n> +};\n> +\n> +static const struct rockchip_saradc_data rk3399_saradc_data = {\n> +\t.num_bits = 10,\n> +\t.num_channels = 6,\n> +\t.clk_rate = 1000000,\n> +};\n> +\n> +static const struct udevice_id rockchip_saradc_ids[] = {\n> +\t{\n> +\t\t.compatible = \"rockchip,saradc\",\n> +\t\t.data = (ulong)&saradc_data,\n> +\t},\n> +\t{\n> +\t\t.compatible = \"rockchip,rk3066-tsadc\",\n> +\t\t.data = (ulong)&rk3066_tsadc_data,\n> +\t}, {\n> +\t\t.compatible = \"rockchip,rk3399-saradc\",\n> +\t\t.data = (ulong)&rk3399_saradc_data,\n> +\t},\n> +\t{ }\n> +};\n> +\n> +U_BOOT_DRIVER(rockchip_saradc) = {\n> +\t.name\t\t= \"rockchip_saradc\",\n> +\t.id\t\t= UCLASS_ADC,\n> +\t.of_match\t= rockchip_saradc_ids,\n> +\t.ops\t\t= &rockchip_saradc_ops,\n> +\t.probe\t\t= rockchip_saradc_probe,\n> +\t.ofdata_to_platdata = rockchip_saradc_ofdata_to_platdata,\n> +\t.priv_auto_alloc_size = sizeof(struct rockchip_saradc_priv),\n> +};\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xstpj4tsTz9ryv\n\tfor ;\n\tThu, 14 Sep 2017 06:40:45 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 6BEA1C223EB; Wed, 13 Sep 2017 20:40:37 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 65A06C22168;\n\tWed, 13 Sep 2017 20:40:34 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid C57E1C22168; Wed, 13 Sep 2017 20:40:32 +0000 (UTC)","from mail.theobroma-systems.com (vegas.theobroma-systems.com\n\t[144.76.126.164])\n\tby lists.denx.de (Postfix) with ESMTPS id 794B6C21FD8\n\tfor ; Wed, 13 Sep 2017 20:40:32 +0000 (UTC)","from [86.59.122.178] (port=60370 helo=android.lan)\n\tby mail.theobroma-systems.com with esmtps\n\t(TLS1.2:RSA_AES_128_CBC_SHA1:128)\n\t(Exim 4.80) (envelope-from )\n\tid 1dsEST-0005Zu-8J; Wed, 13 Sep 2017 22:40:29 +0200","from [10.11.0.14] (helo=vpn-10-11-0-14.lan)\n\tby android.lan with esmtp (Exim 4.84_2)\n\t(envelope-from )\n\tid 1dsESS-00084B-QQ; Wed, 13 Sep 2017 22:40:28 +0200"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=0.0 required=5.0 tests=none autolearn=unavailable\n\tautolearn_force=no version=3.4.0","Date":"Wed, 13 Sep 2017 22:40:27 +0200 (CEST)","From":"Philipp Tomsich ","X-X-Sender":"ptomsich@vpn-10-11-0-14.lan","To":"David Wu ","In-Reply-To":"<1505297379-12638-2-git-send-email-david.wu@rock-chips.com>","Message-ID":"","References":"<1505297379-12638-2-git-send-email-david.wu@rock-chips.com>","User-Agent":"Alpine 2.21 (OSX 202 2017-01-01)","MIME-Version":"1.0","Cc":"huangtao@rock-chips.com, linux-rockchip@lists.infradead.org,\n\tzhangqing@rock-chips.com, u-boot@lists.denx.de, p.marczak@samsung.com,\n\tandy.yan@rock-chips.com, chenjh@rock-chips.com","Subject":"Re: [U-Boot] [U-Boot,1/8] adc: Add driver for Rockchip Saradc","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}}]