[{"id":1768118,"web_url":"http://patchwork.ozlabs.org/comment/1768118/","msgid":"<E1dsDwg-0004RU-PI@mail.theobroma-systems.com>","list_archive_url":null,"date":"2017-09-13T20:07:38","subject":"Re: [U-Boot] [U-Boot,2/5] power: pmic: rk816: support rk816 pmic","submitter":{"id":53488,"url":"http://patchwork.ozlabs.org/api/people/53488/","name":"Philipp Tomsich","email":"philipp.tomsich@theobroma-systems.com"},"content":"> From: Elaine Zhang <zhangqing@rock-chips.com>\n> \n> Add Rockchip pmic rk816 support.\n> \n> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>\n> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>\n> ---\n> \n>  drivers/power/pmic/rk8xx.c      |   1 +\n>  drivers/power/regulator/rk8xx.c | 212 ++++++++++++++++++++++++++++++++--------\n>  include/power/rk8xx_pmic.h      |   9 +-\n>  3 files changed, 182 insertions(+), 40 deletions(-)\n> \n\nAcked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>","headers":{"Return-Path":"<u-boot-bounces@lists.denx.de>","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=<UNKNOWN>)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xst7t2BRTz9rxj\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 14 Sep 2017 06:10:34 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid DD0D2C22212; Wed, 13 Sep 2017 20:08:16 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 20B5EC221AD;\n\tWed, 13 Sep 2017 20:07:50 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 6C095C21E2F; Wed, 13 Sep 2017 20:07:44 +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 82567C21E9B\n\tfor <u-boot@lists.denx.de>; Wed, 13 Sep 2017 20:07:41 +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 <philipp.tomsich@theobroma-systems.com>)\n\tid 1dsDwg-0004RU-PI; Wed, 13 Sep 2017 22:07:38 +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 <philipp.tomsich@theobroma-systems.com>","To":"Kever Yang <kever.yang@rock-chips.com>","In-Reply-To":"<1505297094-5273-2-git-send-email-kever.yang@rock-chips.com>","References":"<1505297094-5273-2-git-send-email-kever.yang@rock-chips.com>","Message-Id":"<E1dsDwg-0004RU-PI@mail.theobroma-systems.com>","Date":"Wed, 13 Sep 2017 22:07:38 +0200","Cc":"u-boot@lists.denx.de, Elaine Zhang <zhangqing@rock-chips.com>,\n\tJacob Chen <jacob-chen@iotwrt.com>","Subject":"Re: [U-Boot] [U-Boot,2/5] power: pmic: rk816: support rk816 pmic","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion <u-boot.lists.denx.de>","List-Unsubscribe":"<https://lists.denx.de/options/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=unsubscribe>","List-Archive":"<http://lists.denx.de/pipermail/u-boot/>","List-Post":"<mailto:u-boot@lists.denx.de>","List-Help":"<mailto:u-boot-request@lists.denx.de?subject=help>","List-Subscribe":"<https://lists.denx.de/listinfo/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=subscribe>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" <u-boot-bounces@lists.denx.de>"}},{"id":1768137,"web_url":"http://patchwork.ozlabs.org/comment/1768137/","msgid":"<alpine.OSX.2.21.1709132209360.52090@vpn-10-11-0-14.lan>","list_archive_url":null,"date":"2017-09-13T20:22:29","subject":"Re: [U-Boot] [U-Boot,2/5] power: pmic: rk816: support rk816 pmic","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, Kever Yang wrote:\n\n> From: Elaine Zhang <zhangqing@rock-chips.com>\n>\n> Add Rockchip pmic rk816 support.\n\nCould you summarise some of the key-features of the RK816 here? Just a few \nwords, so we know how it's different from the others and what it's used \nfor (e.g. \"PMIC matching the RK3xxx\").\n\n>\n> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>\n> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>\n> ---\n>\n> drivers/power/pmic/rk8xx.c      |   1 +\n> drivers/power/regulator/rk8xx.c | 212 ++++++++++++++++++++++++++++++++--------\n> include/power/rk8xx_pmic.h      |   9 +-\n> 3 files changed, 182 insertions(+), 40 deletions(-)\n>\n> diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c\n> index eb3ec0f..0fdea95 100644\n> --- a/drivers/power/pmic/rk8xx.c\n> +++ b/drivers/power/pmic/rk8xx.c\n> @@ -100,6 +100,7 @@ static struct dm_pmic_ops rk8xx_ops = {\n>\n> static const struct udevice_id rk8xx_ids[] = {\n> \t{ .compatible = \"rockchip,rk808\" },\n> +\t{ .compatible = \"rockchip,rk816\" },\n> \t{ .compatible = \"rockchip,rk818\" },\n> \t{ }\n> };\n> diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c\n> index 76fc2ef..cf3566e 100644\n> --- a/drivers/power/regulator/rk8xx.c\n> +++ b/drivers/power/regulator/rk8xx.c\n> @@ -48,6 +48,21 @@ static const struct rk8xx_reg_info rk808_buck[] = {\n> \t{ 1800000, 100000, REG_BUCK4_ON_VSEL, RK808_BUCK4_VSEL_MASK, },\n> };\n>\n> +static const struct rk8xx_reg_info rk816_buck[] = {\n> +\t/* buck 1 */\n> +\t{ 712500, 12500, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n> +\t{ 1800000, 200000, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n> +\t{ 2300000, 0, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n> +\t/* buck 2 */\n> +\t{ 712500, 12500, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n> +\t{ 1800000, 200000, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n> +\t{ 2300000, 0, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n> +\t/* buck 3 */\n> +\t{ 712500, 12500, -1, RK818_BUCK_VSEL_MASK, },\n> +\t/* buck 4 */\n> +\t{ 800000, 100000, REG_BUCK4_ON_VSEL, RK818_BUCK4_VSEL_MASK, },\n> +};\n> +\n> static const struct rk8xx_reg_info rk818_buck[] = {\n> \t{ 712500, 12500, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n> \t{ 712500, 12500, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n> @@ -67,6 +82,15 @@ static const struct rk8xx_reg_info rk808_ldo[] = {\n> \t{ 1800000, 100000, REG_LDO8_ON_VSEL, RK808_LDO_VSEL_MASK, },\n> };\n>\n> +static const struct rk8xx_reg_info rk816_ldo[] = {\n> +\t{ 800000, 100000, REG_LDO1_ON_VSEL, RK818_LDO_VSEL_MASK, },\n> +\t{ 800000, 100000, REG_LDO2_ON_VSEL, RK818_LDO_VSEL_MASK, },\n> +\t{ 800000, 100000, REG_LDO3_ON_VSEL, RK818_LDO_VSEL_MASK, },\n> +\t{ 800000, 100000, REG_LDO4_ON_VSEL, RK818_LDO_VSEL_MASK, },\n> +\t{ 800000, 100000, REG_LDO5_ON_VSEL, RK818_LDO_VSEL_MASK, },\n> +\t{ 800000, 100000, REG_LDO6_ON_VSEL, RK818_LDO_VSEL_MASK, },\n> +};\n> +\n> static const struct rk8xx_reg_info rk818_ldo[] = {\n> \t{ 1800000, 100000, REG_LDO1_ON_VSEL, RK818_LDO_VSEL_MASK, },\n> \t{ 1800000, 100000, REG_LDO2_ON_VSEL, RK818_LDO_VSEL_MASK, },\n> @@ -88,10 +112,24 @@ static const uint rk818_chrg_shutdown_vsel_array[] = {\n> };\n>\n> static const struct rk8xx_reg_info *get_buck_reg(struct udevice *pmic,\n> -\t\t\t\t\t     int num)\n> +\t\t\t\t\t\t int num, int uvolt)\n> {\n> \tstruct rk8xx_priv *priv = dev_get_priv(pmic);\n> +\n> \tswitch (priv->variant) {\n> +\tcase RK816_ID:\n> +\t\tswitch (num) {\n> +\t\tcase 0:\n> +\t\tcase 1:\n> +\t\t\tif (uvolt <= 1450000)\n> +\t\t\t\treturn &rk816_buck[num * 3 + 0];\n> +\t\t\telse if (uvolt <= 2200000)\n> +\t\t\t\treturn &rk816_buck[num * 3 + 1];\n> +\t\t\telse\n> +\t\t\t\treturn &rk816_buck[num * 3 + 2];\n> +\t\tdefault:\n> +\t\t\treturn &rk816_buck[num + 4];\n> +\t\t}\n\nI am very concerned with this driver turning into a large switch \nstatement: this really is not the way driver_data is supposed to be used.\n\nCan we have a single path through this function and use driver_data to \nparameterize the cut-offs? As an alternative, we'll need to start \nsplitting this into separate drivers.\n\n> \tcase RK818_ID:\n> \t\treturn &rk818_buck[num];\n> \tdefault:\n> @@ -101,7 +139,7 @@ static const struct rk8xx_reg_info *get_buck_reg(struct udevice *pmic,\n>\n> static int _buck_set_value(struct udevice *pmic, int buck, int uvolt)\n> {\n> -\tconst struct rk8xx_reg_info *info = get_buck_reg(pmic, buck - 1);\n> +\tconst struct rk8xx_reg_info *info = get_buck_reg(pmic, buck, uvolt);\n> \tint mask = info->vsel_mask;\n> \tint val;\n>\n> @@ -114,23 +152,76 @@ static int _buck_set_value(struct udevice *pmic, int buck, int uvolt)\n> \treturn pmic_clrsetbits(pmic, info->vsel_reg, mask, val);\n> }\n>\n> +static int _buck_get_enable(struct udevice *pmic, int buck)\n> +{\n> +\tstruct rk8xx_priv *priv = dev_get_priv(pmic);\n> +\tuint mask = 0;\n> +\tint ret = 0;\n> +\n> +\tswitch (priv->variant) {\n> +\tcase RK816_ID:\n> +\t\tif (buck >= 4) {\n> +\t\t\tmask = 1 << (buck - 4);\n> +\t\t\tret = pmic_reg_read(pmic, RK816_REG_DCDC_EN2);\n> +\t\t} else {\n> +\t\t\tmask = 1 << buck;\n> +\t\t\tret = pmic_reg_read(pmic, RK816_REG_DCDC_EN1);\n> +\t\t}\n> +\t\tbreak;\n> +\tcase RK808_ID:\n> +\tcase RK818_ID:\n> +\t\tmask = 1 << buck;\n> +\t\tret = pmic_reg_read(pmic, REG_DCDC_EN);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\t\tbreak;\n> +\t}\n> +\treturn ret & mask ? true : false;\n> +}\n\nAgain, the RK816 appears to be a significantly different device.\n\n> +\n> +\n> static int _buck_set_enable(struct udevice *pmic, int buck, bool enable)\n> {\n> -\tuint mask;\n> +\tuint mask, value, en_reg;\n> \tint ret;\n> +\tstruct rk8xx_priv *priv = dev_get_priv(pmic);\n>\n> -\tbuck--;\n> -\tmask = 1 << buck;\n> -\tif (enable) {\n> -\t\tret = pmic_clrsetbits(pmic, REG_DCDC_ILMAX, 0, 3 << (buck * 2));\n> -\t\tif (ret)\n> -\t\t\treturn ret;\n> -\t\tret = pmic_clrsetbits(pmic, REG_DCDC_UV_ACT, 1 << buck, 0);\n> -\t\tif (ret)\n> -\t\t\treturn ret;\n> +\tswitch (priv->variant) {\n> +\tcase RK816_ID:\n> +\t\tif (buck >= 4) {\n> +\t\t\tbuck -= 4;\n> +\t\t\ten_reg = RK816_REG_DCDC_EN2;\n> +\t\t} else {\n> +\t\t\ten_reg = RK816_REG_DCDC_EN1;\n> +\t\t}\n> +\t\tif (enable)\n> +\t\t\tvalue = ((1 << buck) | (1 << (buck + 4)));\n> +\t\telse\n> +\t\t\tvalue = ((0 << buck) | (1 << (buck + 4)));\n> +\t\tret = pmic_reg_write(pmic, en_reg, value);\n> +\t\tbreak;\n> +\n> +\tcase RK808_ID:\n> +\tcase RK818_ID:\n> +\t\tmask = 1 << buck;\n> +\t\tif (enable) {\n> +\t\t\tret = pmic_clrsetbits(pmic, REG_DCDC_ILMAX,\n> +\t\t\t\t\t      0, 3 << (buck * 2));\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\t\t\tret = pmic_clrsetbits(pmic, REG_DCDC_UV_ACT,\n> +\t\t\t\t\t      1 << buck, 0);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\t\t}\n> +\t\tret = pmic_clrsetbits(pmic, REG_DCDC_EN, mask,\n> +\t\t\t\t      enable ? mask : 0);\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tret = -EINVAL;\n> \t}\n>\n> -\treturn pmic_clrsetbits(pmic, REG_DCDC_EN, mask, enable ? mask : 0);\n> +\treturn ret;\n> }\n>\n> #ifdef ENABLE_DRIVER\n\nThis define doesn't make any sense: I see that it's always been here, but \nstill: it is just a wrapper around a \"#ifndef CONFIG_SPL_BUILD\". Can we \nuse the #ifndef CONFIG_SPL_BUILD directly?\n\n> @@ -138,7 +229,10 @@ static const struct rk8xx_reg_info *get_ldo_reg(struct udevice *pmic,\n> \t\t\t\t\t     int num)\n> {\n> \tstruct rk8xx_priv *priv = dev_get_priv(pmic);\n> +\n> \tswitch (priv->variant) {\n> +\tcase RK816_ID:\n> +\t\treturn &rk816_ldo[num];\n> \tcase RK818_ID:\n> \t\treturn &rk818_ldo[num];\n> \tdefault:\n> @@ -146,10 +240,70 @@ static const struct rk8xx_reg_info *get_ldo_reg(struct udevice *pmic,\n> \t}\n> }\n>\n> +static int _ldo_get_enable(struct udevice *pmic, int ldo)\n> +{\n> +\tstruct rk8xx_priv *priv = dev_get_priv(pmic);\n> +\tuint mask = 0;\n> +\tint ret = 0;\n> +\n> +\tswitch (priv->variant) {\n> +\tcase RK816_ID:\n> +\t\tif (ldo >= 4) {\n> +\t\t\tmask = 1 << (ldo - 4);\n> +\t\t\tret = pmic_reg_read(pmic, RK816_REG_LDO_EN2);\n> +\t\t} else {\n> +\t\t\tmask = 1 << ldo;\n> +\t\t\tret = pmic_reg_read(pmic, RK816_REG_LDO_EN1);\n> +\t\t}\n\nSame comment as above.\n\n> +\t\tbreak;\n> +\tcase RK808_ID:\n> +\tcase RK818_ID:\n> +\t\tmask = 1 << ldo;\n> +\t\tret = pmic_reg_read(pmic, REG_LDO_EN);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\t\tbreak;\n> +\t}\n> +\treturn ret & mask ? true : false;\n> +}\n> +\n> +\n> +static int _ldo_set_enable(struct udevice *pmic, int ldo, bool enable)\n> +{\n> +\tstruct rk8xx_priv *priv = dev_get_priv(pmic);\n> +\tuint mask, value, en_reg;\n> +\tint ret = 0;\n> +\n> +\tswitch (priv->variant) {\n> +\tcase RK816_ID:\n> +\t\tif (ldo >= 4) {\n> +\t\t\tldo -= 4;\n> +\t\t\ten_reg = RK816_REG_LDO_EN2;\n> +\t\t} else {\n> +\t\t\ten_reg = RK816_REG_LDO_EN1;\n> +\t\t}\n> +\t\tif (enable)\n> +\t\t\tvalue = ((1 << ldo) | (1 << (ldo + 4)));\n> +\t\telse\n> +\t\t\tvalue = ((0 << ldo) | (1 << (ldo + 4)));\n> +\n> +\t\tret = pmic_reg_write(pmic, en_reg, value);\n> +\t\tbreak;\n\nAgain, it looks as if this should be a separate driver.\n\n> +\tcase RK808_ID:\n> +\tcase RK818_ID:\n> +\t\tmask = 1 << ldo;\n> +\t\tret = pmic_clrsetbits(pmic, REG_LDO_EN, mask,\n> +\t\t\t\t       enable ? mask : 0);\n> +\t\tbreak;\n> +\t}\n> +\n> +\treturn ret;\n> +}\n> +\n> static int buck_get_value(struct udevice *dev)\n> {\n> \tint buck = dev->driver_data - 1;\n> -\tconst struct rk8xx_reg_info *info = get_buck_reg(dev->parent, buck);\n> +\tconst struct rk8xx_reg_info *info = get_buck_reg(dev->parent, buck, 0);\n> \tint mask = info->vsel_mask;\n> \tint ret, val;\n>\n> @@ -165,14 +319,14 @@ static int buck_get_value(struct udevice *dev)\n>\n> static int buck_set_value(struct udevice *dev, int uvolt)\n> {\n> -\tint buck = dev->driver_data;\n> +\tint buck = dev->driver_data - 1;\n>\n> \treturn _buck_set_value(dev->parent, buck, uvolt);\n> }\n>\n> static int buck_set_enable(struct udevice *dev, bool enable)\n> {\n> -\tint buck = dev->driver_data;\n> +\tint buck = dev->driver_data - 1;\n>\n> \treturn _buck_set_enable(dev->parent, buck, enable);\n> }\n> @@ -180,16 +334,8 @@ static int buck_set_enable(struct udevice *dev, bool enable)\n> static int buck_get_enable(struct udevice *dev)\n> {\n> \tint buck = dev->driver_data - 1;\n> -\tint ret;\n> -\tuint mask;\n> -\n> -\tmask = 1 << buck;\n>\n> -\tret = pmic_reg_read(dev->parent, REG_DCDC_EN);\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> -\n> -\treturn ret & mask ? true : false;\n> +\treturn _buck_get_enable(dev->parent, buck);\n> }\n>\n> static int ldo_get_value(struct udevice *dev)\n> @@ -228,27 +374,15 @@ static int ldo_set_value(struct udevice *dev, int uvolt)\n> static int ldo_set_enable(struct udevice *dev, bool enable)\n> {\n> \tint ldo = dev->driver_data - 1;\n> -\tuint mask;\n> -\n> -\tmask = 1 << ldo;\n>\n> -\treturn pmic_clrsetbits(dev->parent, REG_LDO_EN, mask,\n> -\t\t\t       enable ? mask : 0);\n> +\treturn _ldo_set_enable(dev->parent, ldo, enable);\n> }\n>\n> static int ldo_get_enable(struct udevice *dev)\n> {\n> \tint ldo = dev->driver_data - 1;\n> -\tint ret;\n> -\tuint mask;\n> -\n> -\tmask = 1 << ldo;\n>\n> -\tret = pmic_reg_read(dev->parent, REG_LDO_EN);\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> -\n> -\treturn ret & mask ? true : false;\n> +\treturn _ldo_get_enable(dev->parent, ldo);\n> }\n>\n> static int switch_set_enable(struct udevice *dev, bool enable)\n> diff --git a/include/power/rk8xx_pmic.h b/include/power/rk8xx_pmic.h\n> index 47a6b36..8e821c3 100644\n> --- a/include/power/rk8xx_pmic.h\n> +++ b/include/power/rk8xx_pmic.h\n> @@ -171,8 +171,15 @@ enum {\n> };\n>\n> enum {\n> -\tRK805_ID = 0x8050,\n> +\tRK816_REG_DCDC_EN1 = 0x23,\n> +\tRK816_REG_DCDC_EN2,\n> +\tRK816_REG_LDO_EN1 = 0x27,\n> +\tRK816_REG_LDO_EN2,\n> +};\n> +\n> +enum {\n> \tRK808_ID = 0x0000,\n> +\tRK816_ID = 0x8160,\n> \tRK818_ID = 0x8180,\n> };\n>\n>","headers":{"Return-Path":"<u-boot-bounces@lists.denx.de>","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=<UNKNOWN>)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xstPt6L2bz9s78\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 14 Sep 2017 06:22:42 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 901E4C22300; Wed, 13 Sep 2017 20:22:39 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 15777C21F61;\n\tWed, 13 Sep 2017 20:22:35 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 11EE1C21F61; Wed, 13 Sep 2017 20:22:34 +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 C3AFEC21C34\n\tfor <u-boot@lists.denx.de>; Wed, 13 Sep 2017 20:22:33 +0000 (UTC)","from [86.59.122.178] (port=60360 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 <philipp.tomsich@theobroma-systems.com>)\n\tid 1dsEB5-0004wh-0i; Wed, 13 Sep 2017 22:22:31 +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 <philipp.tomsich@theobroma-systems.com>)\n\tid 1dsEB4-0007hF-HA; Wed, 13 Sep 2017 22:22:30 +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:22:29 +0200 (CEST)","From":"Philipp Tomsich <philipp.tomsich@theobroma-systems.com>","X-X-Sender":"ptomsich@vpn-10-11-0-14.lan","To":"Kever Yang <kever.yang@rock-chips.com>","In-Reply-To":"<1505297094-5273-2-git-send-email-kever.yang@rock-chips.com>","Message-ID":"<alpine.OSX.2.21.1709132209360.52090@vpn-10-11-0-14.lan>","References":"<1505297094-5273-2-git-send-email-kever.yang@rock-chips.com>","User-Agent":"Alpine 2.21 (OSX 202 2017-01-01)","MIME-Version":"1.0","Cc":"u-boot@lists.denx.de, Elaine Zhang <zhangqing@rock-chips.com>,\n\tJacob Chen <jacob-chen@iotwrt.com>","Subject":"Re: [U-Boot] [U-Boot,2/5] power: pmic: rk816: support rk816 pmic","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion <u-boot.lists.denx.de>","List-Unsubscribe":"<https://lists.denx.de/options/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=unsubscribe>","List-Archive":"<http://lists.denx.de/pipermail/u-boot/>","List-Post":"<mailto:u-boot@lists.denx.de>","List-Help":"<mailto:u-boot-request@lists.denx.de?subject=help>","List-Subscribe":"<https://lists.denx.de/listinfo/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=subscribe>","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" <u-boot-bounces@lists.denx.de>"}},{"id":1769758,"web_url":"http://patchwork.ozlabs.org/comment/1769758/","msgid":"<CAPnjgZ0qgSz7zAFk-ybixuVTC80X2PyK5wWdrozZmaxcDZncpQ@mail.gmail.com>","list_archive_url":null,"date":"2017-09-17T17:52:46","subject":"Re: [U-Boot] [U-Boot,2/5] power: pmic: rk816: support rk816 pmic","submitter":{"id":6170,"url":"http://patchwork.ozlabs.org/api/people/6170/","name":"Simon Glass","email":"sjg@chromium.org"},"content":"Hi,\n\nOn 13 September 2017 at 14:22, Philipp Tomsich\n<philipp.tomsich@theobroma-systems.com> wrote:\n>\n>\n> On Wed, 13 Sep 2017, Kever Yang wrote:\n>\n>> From: Elaine Zhang <zhangqing@rock-chips.com>\n>>\n>> Add Rockchip pmic rk816 support.\n>\n>\n> Could you summarise some of the key-features of the RK816 here? Just a few\n> words, so we know how it's different from the others and what it's used for\n> (e.g. \"PMIC matching the RK3xxx\").\n>\n>\n>>\n>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>\n>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>\n>> ---\n>>\n>> drivers/power/pmic/rk8xx.c      |   1 +\n>> drivers/power/regulator/rk8xx.c | 212\n>> ++++++++++++++++++++++++++++++++--------\n>> include/power/rk8xx_pmic.h      |   9 +-\n>> 3 files changed, 182 insertions(+), 40 deletions(-)\n>>\n>> diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c\n>> index eb3ec0f..0fdea95 100644\n>> --- a/drivers/power/pmic/rk8xx.c\n>> +++ b/drivers/power/pmic/rk8xx.c\n>> @@ -100,6 +100,7 @@ static struct dm_pmic_ops rk8xx_ops = {\n>>\n>> static const struct udevice_id rk8xx_ids[] = {\n>>         { .compatible = \"rockchip,rk808\" },\n>> +       { .compatible = \"rockchip,rk816\" },\n>>         { .compatible = \"rockchip,rk818\" },\n>>         { }\n>> };\n>> diff --git a/drivers/power/regulator/rk8xx.c\n>> b/drivers/power/regulator/rk8xx.c\n>> index 76fc2ef..cf3566e 100644\n>> --- a/drivers/power/regulator/rk8xx.c\n>> +++ b/drivers/power/regulator/rk8xx.c\n>> @@ -48,6 +48,21 @@ static const struct rk8xx_reg_info rk808_buck[] = {\n>>         { 1800000, 100000, REG_BUCK4_ON_VSEL, RK808_BUCK4_VSEL_MASK, },\n>> };\n>>\n>> +static const struct rk8xx_reg_info rk816_buck[] = {\n>> +       /* buck 1 */\n>> +       { 712500, 12500, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>> +       { 1800000, 200000, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>> +       { 2300000, 0, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>> +       /* buck 2 */\n>> +       { 712500, 12500, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>> +       { 1800000, 200000, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>> +       { 2300000, 0, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>> +       /* buck 3 */\n>> +       { 712500, 12500, -1, RK818_BUCK_VSEL_MASK, },\n>> +       /* buck 4 */\n>> +       { 800000, 100000, REG_BUCK4_ON_VSEL, RK818_BUCK4_VSEL_MASK, },\n>> +};\n>> +\n>> static const struct rk8xx_reg_info rk818_buck[] = {\n>>         { 712500, 12500, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>>         { 712500, 12500, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>> @@ -67,6 +82,15 @@ static const struct rk8xx_reg_info rk808_ldo[] = {\n>>         { 1800000, 100000, REG_LDO8_ON_VSEL, RK808_LDO_VSEL_MASK, },\n>> };\n>>\n>> +static const struct rk8xx_reg_info rk816_ldo[] = {\n>> +       { 800000, 100000, REG_LDO1_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>> +       { 800000, 100000, REG_LDO2_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>> +       { 800000, 100000, REG_LDO3_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>> +       { 800000, 100000, REG_LDO4_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>> +       { 800000, 100000, REG_LDO5_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>> +       { 800000, 100000, REG_LDO6_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>> +};\n>> +\n>> static const struct rk8xx_reg_info rk818_ldo[] = {\n>>         { 1800000, 100000, REG_LDO1_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>>         { 1800000, 100000, REG_LDO2_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>> @@ -88,10 +112,24 @@ static const uint rk818_chrg_shutdown_vsel_array[] =\n>> {\n>> };\n>>\n>> static const struct rk8xx_reg_info *get_buck_reg(struct udevice *pmic,\n>> -                                            int num)\n>> +                                                int num, int uvolt)\n>> {\n>>         struct rk8xx_priv *priv = dev_get_priv(pmic);\n>> +\n>>         switch (priv->variant) {\n>> +       case RK816_ID:\n>> +               switch (num) {\n>> +               case 0:\n>> +               case 1:\n>> +                       if (uvolt <= 1450000)\n>> +                               return &rk816_buck[num * 3 + 0];\n>> +                       else if (uvolt <= 2200000)\n>> +                               return &rk816_buck[num * 3 + 1];\n>> +                       else\n>> +                               return &rk816_buck[num * 3 + 2];\n>> +               default:\n>> +                       return &rk816_buck[num + 4];\n>> +               }\n>\n>\n> I am very concerned with this driver turning into a large switch statement:\n> this really is not the way driver_data is supposed to be used.\n>\n> Can we have a single path through this function and use driver_data to\n> parameterize the cut-offs? As an alternative, we'll need to start splitting\n> this into separate drivers.\n\nYes it is worth considering that. I don't see a good reason to have\nthis new device in the same driver, given all the changes.\n\nIf there is shared code it could be split out into a new file.\n\nRegards,\nSimon","headers":{"Return-Path":"<u-boot-bounces@lists.denx.de>","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=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"BlvuOxIO\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"ANt7yddn\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xwHWn6Nx8z9sDB\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 18 Sep 2017 04:21:09 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 3B877C21C39; Sun, 17 Sep 2017 18:21:08 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id C108CC21DC0;\n\tSun, 17 Sep 2017 17:54:24 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 038DAC21DBA; Sun, 17 Sep 2017 17:54:22 +0000 (UTC)","from mail-qt0-f180.google.com (mail-qt0-f180.google.com\n\t[209.85.216.180])\n\tby lists.denx.de (Postfix) with ESMTPS id 5C5DBC21D5D\n\tfor <u-boot@lists.denx.de>; Sun, 17 Sep 2017 17:53:08 +0000 (UTC)","by mail-qt0-f180.google.com with SMTP id t46so5794448qtj.2\n\tfor <u-boot@lists.denx.de>; Sun, 17 Sep 2017 10:53:08 -0700 (PDT)","by 10.200.37.200 with HTTP; Sun, 17 Sep 2017 10:52:46 -0700 (PDT)"],"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=RCVD_IN_DNSWL_NONE,\n\tRCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID autolearn=unavailable\n\tautolearn_force=no version=3.4.0","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n\ts=20161025; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=d3KhfMm/eR6wljDoXii9Zdk6e5c7KuR17e0WrAbc7g0=;\n\tb=BlvuOxIOcqn6rDAa88TUCV6x3BXwRQyMoPKR+XUbLxD5v4fzed93gKqIwslP8kxARn\n\tWwtUm76xsfCjsrFwIEp0D5nX4fXSXWEBdAMWefuVVvsoJydxMka6tPakJQRwpnNsJ15D\n\tw5xPfQ8b1cU+AORyJIPFUrtc79XzjdbIi/FWiWcO6AS5la4umy83ZoajLA1Jnl1smqBa\n\td3fwxWP+hSDAzOR8RmnfWrtBBbn6Szvd+M8WbVMMOoTtDRJc99mQFt5KjJEIKbPv4cl4\n\taNtw8uODYQzFo5C8NslEJEL6LBuTBCt8EgRGl0Io3u+IGANhFy6mtdW8GhfDzlWr+lEo\n\tuDKw==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=d3KhfMm/eR6wljDoXii9Zdk6e5c7KuR17e0WrAbc7g0=;\n\tb=ANt7yddnzQQz5/K3ObbD0VxMacPWW8Mp/gHie6PE5aNeW83bM04vP9XGmxIepeze/e\n\tBLkxtlCv8GXQ5yv6ABUhooa4RUXMcUClipO4n7JkiGASCKMOurwG8inW9JHXQ8aiVhkG\n\tnrzcuEaCQ0HXBSKepE00EW3irP7S5eOJ1zQqE="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=d3KhfMm/eR6wljDoXii9Zdk6e5c7KuR17e0WrAbc7g0=;\n\tb=rNGHUCqPgD86AdcXujwrgS68+SbgYE81X3i+XkTMAYYiNaIkiLYGPsSAJSpLb8Tagw\n\tsSufEebOJ+MKizZf79lYqCKL6XtGcAUFaIqgN4LxgudiQyamg9xZPzzJHKqwb03ApYjJ\n\tZwrhyRK0ndU001tTXzpKr9cweJ5taICvF1fEvBHNscoy3JslulaMIvLHqIj0Em7Ie41t\n\tss3DEkukOA/k9pbsl7Y9BfUiqTzPdW5oQNuM39DKDBVNFRIsDYvXBi/pJ0PQjJ+o+fJx\n\tJYe7NH7wfvvJfcxqBOw2mbhb+OHZ2GhGNtxO3WMXqgw59RWTUNugchzd6JaBmWzC84Yx\n\t5FCQ==","X-Gm-Message-State":"AHPjjUhYiPj7yQ9bC7L+icKaWEkFh5O//627Rp2+0UUGagyUAB6iFKp2\n\tzY/2SZ0EWy/wsInzQ/SedTWFxU+teLP4TvMuV7HT0A==","X-Google-Smtp-Source":"AOwi7QB8qZD/9na85AO61QR3LO1wM5mHBBsGklWETpgVsu83Z2QXk7H07rw7FHdYVLxRRwMzevq/KDfJOYvobeCTUYI=","X-Received":"by 10.200.52.241 with SMTP id x46mr38438568qtb.38.1505670787053; \n\tSun, 17 Sep 2017 10:53:07 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<alpine.OSX.2.21.1709132209360.52090@vpn-10-11-0-14.lan>","References":"<1505297094-5273-2-git-send-email-kever.yang@rock-chips.com>\n\t<alpine.OSX.2.21.1709132209360.52090@vpn-10-11-0-14.lan>","From":"Simon Glass <sjg@chromium.org>","Date":"Sun, 17 Sep 2017 11:52:46 -0600","X-Google-Sender-Auth":"cELcgOHUOQsbjuOUEqTe1JS6TvY","Message-ID":"<CAPnjgZ0qgSz7zAFk-ybixuVTC80X2PyK5wWdrozZmaxcDZncpQ@mail.gmail.com>","To":"Philipp Tomsich <philipp.tomsich@theobroma-systems.com>","Cc":"U-Boot Mailing List <u-boot@lists.denx.de>,\n\tElaine Zhang <zhangqing@rock-chips.com>,\n\tJacob Chen <jacob-chen@iotwrt.com>","Subject":"Re: [U-Boot] [U-Boot,2/5] power: pmic: rk816: support rk816 pmic","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion <u-boot.lists.denx.de>","List-Unsubscribe":"<https://lists.denx.de/options/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=unsubscribe>","List-Archive":"<http://lists.denx.de/pipermail/u-boot/>","List-Post":"<mailto:u-boot@lists.denx.de>","List-Help":"<mailto:u-boot-request@lists.denx.de?subject=help>","List-Subscribe":"<https://lists.denx.de/listinfo/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=subscribe>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" <u-boot-bounces@lists.denx.de>"}},{"id":1776068,"web_url":"http://patchwork.ozlabs.org/comment/1776068/","msgid":"<339040cd-47fe-c3d5-8193-87985d1491ac@rock-chips.com>","list_archive_url":null,"date":"2017-09-27T07:00:54","subject":"Re: [U-Boot] [U-Boot,2/5] power: pmic: rk816: support rk816 pmic","submitter":{"id":64532,"url":"http://patchwork.ozlabs.org/api/people/64532/","name":"Kever Yang","email":"kever.yang@rock-chips.com"},"content":"Hi Simon, Philipp,\n\n\nOn 09/18/2017 01:52 AM, Simon Glass wrote:\n> Hi,\n>\n> On 13 September 2017 at 14:22, Philipp Tomsich\n> <philipp.tomsich@theobroma-systems.com> wrote:\n>>\n>> On Wed, 13 Sep 2017, Kever Yang wrote:\n>>\n>>> From: Elaine Zhang <zhangqing@rock-chips.com>\n>>>\n>>> Add Rockchip pmic rk816 support.\n>>\n>> Could you summarise some of the key-features of the RK816 here? Just a few\n>> words, so we know how it's different from the others and what it's used for\n>> (e.g. \"PMIC matching the RK3xxx\").\n\nWill add detail difference between these chips, they are very similar as \nregulator and\nit's reasonable to make then in one driver instead of use different \ndrivers for them:\n- different DCDC/LDO numbers;\n- different voltage output range and current loading;\n- RK816 and RK805 add mask bit for enable/disable to make read/write \nmore safe.\n\n>>\n>>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>\n>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>\n>>> ---\n>>>\n>>> drivers/power/pmic/rk8xx.c      |   1 +\n>>> drivers/power/regulator/rk8xx.c | 212\n>>> ++++++++++++++++++++++++++++++++--------\n>>> include/power/rk8xx_pmic.h      |   9 +-\n>>> 3 files changed, 182 insertions(+), 40 deletions(-)\n>>>\n>>> diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c\n>>> index eb3ec0f..0fdea95 100644\n>>> --- a/drivers/power/pmic/rk8xx.c\n>>> +++ b/drivers/power/pmic/rk8xx.c\n>>> @@ -100,6 +100,7 @@ static struct dm_pmic_ops rk8xx_ops = {\n>>>\n>>> static const struct udevice_id rk8xx_ids[] = {\n>>>          { .compatible = \"rockchip,rk808\" },\n>>> +       { .compatible = \"rockchip,rk816\" },\n>>>          { .compatible = \"rockchip,rk818\" },\n>>>          { }\n>>> };\n>>> diff --git a/drivers/power/regulator/rk8xx.c\n>>> b/drivers/power/regulator/rk8xx.c\n>>> index 76fc2ef..cf3566e 100644\n>>> --- a/drivers/power/regulator/rk8xx.c\n>>> +++ b/drivers/power/regulator/rk8xx.c\n>>> @@ -48,6 +48,21 @@ static const struct rk8xx_reg_info rk808_buck[] = {\n>>>          { 1800000, 100000, REG_BUCK4_ON_VSEL, RK808_BUCK4_VSEL_MASK, },\n>>> };\n>>>\n>>> +static const struct rk8xx_reg_info rk816_buck[] = {\n>>> +       /* buck 1 */\n>>> +       { 712500, 12500, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>>> +       { 1800000, 200000, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>>> +       { 2300000, 0, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>>> +       /* buck 2 */\n>>> +       { 712500, 12500, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>>> +       { 1800000, 200000, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>>> +       { 2300000, 0, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>>> +       /* buck 3 */\n>>> +       { 712500, 12500, -1, RK818_BUCK_VSEL_MASK, },\n>>> +       /* buck 4 */\n>>> +       { 800000, 100000, REG_BUCK4_ON_VSEL, RK818_BUCK4_VSEL_MASK, },\n>>> +};\n>>> +\n>>> static const struct rk8xx_reg_info rk818_buck[] = {\n>>>          { 712500, 12500, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>>>          { 712500, 12500, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, },\n>>> @@ -67,6 +82,15 @@ static const struct rk8xx_reg_info rk808_ldo[] = {\n>>>          { 1800000, 100000, REG_LDO8_ON_VSEL, RK808_LDO_VSEL_MASK, },\n>>> };\n>>>\n>>> +static const struct rk8xx_reg_info rk816_ldo[] = {\n>>> +       { 800000, 100000, REG_LDO1_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>>> +       { 800000, 100000, REG_LDO2_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>>> +       { 800000, 100000, REG_LDO3_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>>> +       { 800000, 100000, REG_LDO4_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>>> +       { 800000, 100000, REG_LDO5_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>>> +       { 800000, 100000, REG_LDO6_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>>> +};\n>>> +\n>>> static const struct rk8xx_reg_info rk818_ldo[] = {\n>>>          { 1800000, 100000, REG_LDO1_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>>>          { 1800000, 100000, REG_LDO2_ON_VSEL, RK818_LDO_VSEL_MASK, },\n>>> @@ -88,10 +112,24 @@ static const uint rk818_chrg_shutdown_vsel_array[] =\n>>> {\n>>> };\n>>>\n>>> static const struct rk8xx_reg_info *get_buck_reg(struct udevice *pmic,\n>>> -                                            int num)\n>>> +                                                int num, int uvolt)\n>>> {\n>>>          struct rk8xx_priv *priv = dev_get_priv(pmic);\n>>> +\n>>>          switch (priv->variant) {\n>>> +       case RK816_ID:\n>>> +               switch (num) {\n>>> +               case 0:\n>>> +               case 1:\n>>> +                       if (uvolt <= 1450000)\n>>> +                               return &rk816_buck[num * 3 + 0];\n>>> +                       else if (uvolt <= 2200000)\n>>> +                               return &rk816_buck[num * 3 + 1];\n>>> +                       else\n>>> +                               return &rk816_buck[num * 3 + 2];\n>>> +               default:\n>>> +                       return &rk816_buck[num + 4];\n>>> +               }\n>>\n>> I am very concerned with this driver turning into a large switch statement:\n>> this really is not the way driver_data is supposed to be used.\n>>\n>> Can we have a single path through this function and use driver_data to\n>> parameterize the cut-offs? As an alternative, we'll need to start splitting\n>> this into separate drivers.\n> Yes it is worth considering that. I don't see a good reason to have\n> this new device in the same driver, given all the changes.\n>\n> If there is shared code it could be split out into a new file.\n\nThe framework and the setting for all these chips are the same, except \nthe BUCK/LDO\nnumbers are different, and rk805/rk816 need extra mask bit for \nenable/disable.\n\nAbstract the difference into array structure is a good way to share the \ncode, I don't\nthink split this into rk805.c rk808.c rk816.c and rk818.c will have a \nbetter shape.\n\nBTW, these are all the PMICs we have since early 2013, so don't worry, \nit won't grow into\nlarge switch statement.\n\nThanks,\n- Kever\n>\n> Regards,\n> Simon\n>","headers":{"Return-Path":"<u-boot-bounces@lists.denx.de>","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=<UNKNOWN>)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3y27yn3JKgz9t4Z\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 27 Sep 2017 17:01:21 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 33C13C21DC3; Wed, 27 Sep 2017 07:01:18 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 5F785C21C46;\n\tWed, 27 Sep 2017 07:01:15 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 635E3C21C46; Wed, 27 Sep 2017 07:01:13 +0000 (UTC)","from regular1.263xmail.com (regular1.263xmail.com [211.150.99.140])\n\tby lists.denx.de (Postfix) with ESMTPS id 38E24C21C40\n\tfor <u-boot@lists.denx.de>; Wed, 27 Sep 2017 07:01:09 +0000 (UTC)","from kever.yang?rock-chips.com (unknown [192.168.165.103])\n\tby regular1.263xmail.com (Postfix) with ESMTP id 1901F4CE9;\n\tWed, 27 Sep 2017 15:01:02 +0800 (CST)","from [192.168.60.65] (localhost [127.0.0.1])\n\tby smtp.263.net (Postfix) with ESMTPA id 90B0A431;\n\tWed, 27 Sep 2017 15:00:57 +0800 (CST)","from [192.168.60.65] (unknown [103.29.142.67])\n\tby smtp.263.net (Postfix) whith ESMTP id 1631591CMY8;\n\tWed, 27 Sep 2017 15:01:02 +0800 (CST)"],"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=RCVD_IN_DNSWL_NONE,\n\tRCVD_IN_MSPIKE_H2 autolearn=unavailable autolearn_force=no\n\tversion=3.4.0","X-263anti-spam":"KSV:0;","X-MAIL-GRAY":"0","X-MAIL-DELIVERY":"1","X-KSVirus-check":"0","X-ABS-CHECKED":"4","X-RL-SENDER":"kever.yang@rock-chips.com","X-FST-TO":"jacob-chen@iotwrt.com","X-SENDER-IP":"103.29.142.67","X-LOGIN-NAME":"kever.yang@rock-chips.com","X-UNIQUE-TAG":"<355c11929cedd6ab7e08102e06142644>","X-ATTACHMENT-NUM":"0","X-SENDER":"yk@rock-chips.com","X-DNS-TYPE":"0","To":"Simon Glass <sjg@chromium.org>,\n\tPhilipp Tomsich <philipp.tomsich@theobroma-systems.com>","References":"<1505297094-5273-2-git-send-email-kever.yang@rock-chips.com>\n\t<alpine.OSX.2.21.1709132209360.52090@vpn-10-11-0-14.lan>\n\t<CAPnjgZ0qgSz7zAFk-ybixuVTC80X2PyK5wWdrozZmaxcDZncpQ@mail.gmail.com>","From":"Kever Yang <kever.yang@rock-chips.com>","Message-ID":"<339040cd-47fe-c3d5-8193-87985d1491ac@rock-chips.com>","Date":"Wed, 27 Sep 2017 15:00:54 +0800","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101\n\tThunderbird/45.8.0","MIME-Version":"1.0","In-Reply-To":"<CAPnjgZ0qgSz7zAFk-ybixuVTC80X2PyK5wWdrozZmaxcDZncpQ@mail.gmail.com>","Cc":"U-Boot Mailing List <u-boot@lists.denx.de>,\n\tElaine Zhang <zhangqing@rock-chips.com>,\n\tJacob Chen <jacob-chen@iotwrt.com>","Subject":"Re: [U-Boot] [U-Boot,2/5] power: pmic: rk816: support rk816 pmic","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion <u-boot.lists.denx.de>","List-Unsubscribe":"<https://lists.denx.de/options/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=unsubscribe>","List-Archive":"<http://lists.denx.de/pipermail/u-boot/>","List-Post":"<mailto:u-boot@lists.denx.de>","List-Help":"<mailto:u-boot-request@lists.denx.de?subject=help>","List-Subscribe":"<https://lists.denx.de/listinfo/u-boot>,\n\t<mailto:u-boot-request@lists.denx.de?subject=subscribe>","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" <u-boot-bounces@lists.denx.de>"}}]