| 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
| ||
|
Message-ID: <CADBw62oCNrvTgu9k-iykaX4bbpVWjbg3GN+v5yPBCc9R2bR3Pg@mail.gmail.com>
Date: Mon, 15 Jun 2020 22:11:42 +0800
From: Baolin Wang <baolin.wang7@...il.com>
To: Chunyan Zhang <zhang.lyra@...il.com>
Cc: Lee Jones <lee.jones@...aro.org>,
LKML <linux-kernel@...r.kernel.org>,
Orson Zhai <orsonzhai@...il.com>,
Chunyan Zhang <chunyan.zhang@...soc.com>
Subject: Re: [PATCH] mfd: sprd: get subdevices from DT instead for SC27XX SPI
On Mon, Jun 15, 2020 at 11:47 AM Chunyan Zhang <zhang.lyra@...il.com> wrote:
>
> From: Chunyan Zhang <chunyan.zhang@...soc.com>
>
> SC27XX-SPI added subdevices according to a pre-defined mfd_cell array,
> no matter these devices were really included on board. So with this
> patch we switch to a new way of detecting subdevices which are
> defined in the devicetree.
>
> Signed-off-by: Chunyan Zhang <chunyan.zhang@...soc.com>
> ---
> drivers/mfd/sprd-sc27xx-spi.c | 102 +++++++++++-----------------------
> 1 file changed, 31 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
> index 33336cde4724..aa3daa0cfcf5 100644
> --- a/drivers/mfd/sprd-sc27xx-spi.c
> +++ b/drivers/mfd/sprd-sc27xx-spi.c
> @@ -93,73 +93,6 @@ enum usb_charger_type sprd_pmic_detect_charger_type(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(sprd_pmic_detect_charger_type);
>
> -static const struct mfd_cell sprd_pmic_devs[] = {
> - {
> - .name = "sc27xx-wdt",
> - .of_compatible = "sprd,sc2731-wdt",
> - }, {
> - .name = "sc27xx-rtc",
> - .of_compatible = "sprd,sc2731-rtc",
> - }, {
> - .name = "sc27xx-charger",
> - .of_compatible = "sprd,sc2731-charger",
> - }, {
> - .name = "sc27xx-chg-timer",
> - .of_compatible = "sprd,sc2731-chg-timer",
> - }, {
> - .name = "sc27xx-fast-chg",
> - .of_compatible = "sprd,sc2731-fast-chg",
> - }, {
> - .name = "sc27xx-chg-wdt",
> - .of_compatible = "sprd,sc2731-chg-wdt",
> - }, {
> - .name = "sc27xx-typec",
> - .of_compatible = "sprd,sc2731-typec",
> - }, {
> - .name = "sc27xx-flash",
> - .of_compatible = "sprd,sc2731-flash",
> - }, {
> - .name = "sc27xx-eic",
> - .of_compatible = "sprd,sc2731-eic",
> - }, {
> - .name = "sc27xx-efuse",
> - .of_compatible = "sprd,sc2731-efuse",
> - }, {
> - .name = "sc27xx-thermal",
> - .of_compatible = "sprd,sc2731-thermal",
> - }, {
> - .name = "sc27xx-adc",
> - .of_compatible = "sprd,sc2731-adc",
> - }, {
> - .name = "sc27xx-audio-codec",
> - .of_compatible = "sprd,sc2731-audio-codec",
> - }, {
> - .name = "sc27xx-regulator",
> - .of_compatible = "sprd,sc2731-regulator",
> - }, {
> - .name = "sc27xx-vibrator",
> - .of_compatible = "sprd,sc2731-vibrator",
> - }, {
> - .name = "sc27xx-keypad-led",
> - .of_compatible = "sprd,sc2731-keypad-led",
> - }, {
> - .name = "sc27xx-bltc",
> - .of_compatible = "sprd,sc2731-bltc",
> - }, {
> - .name = "sc27xx-fgu",
> - .of_compatible = "sprd,sc2731-fgu",
> - }, {
> - .name = "sc27xx-7sreset",
> - .of_compatible = "sprd,sc2731-7sreset",
> - }, {
> - .name = "sc27xx-poweroff",
> - .of_compatible = "sprd,sc2731-poweroff",
> - }, {
> - .name = "sc27xx-syscon",
> - .of_compatible = "sprd,sc2731-syscon",
> - },
> -};
> -
> static int sprd_pmic_spi_write(void *context, const void *data, size_t count)
> {
> struct device *dev = context;
> @@ -205,6 +138,35 @@ static const struct regmap_config sprd_pmic_config = {
> .max_register = 0xffff,
> };
>
> +static int sprd_pmic_add_subdevices(struct device *dev, int id,
> + struct irq_domain *domain)
> +{
> + int ret = 0;
> + struct device_node *child, *parent = dev->of_node;
> + struct mfd_cell cell = {0};
> + const char *comp;
> + unsigned int prefix_len = strlen("sprd,");
> + char buf[30];
> +
> + for_each_child_of_node(parent, child) {
> + comp = of_get_property(child, "compatible", NULL);
> + if (!comp || strncmp("sprd,", comp, prefix_len))
> + return -EINVAL;
> +
> + memcpy(buf, comp, strlen(comp) + 1);
> + cell.of_compatible = buf;
> + cell.name = buf + prefix_len;
I feel it is a little hackish, you can create any device nodes from DT
even the PMIC does not support.
I am not sure the problem caused by the original method, you can use
the compatible string to be compatible with different PMICs, such as
SC2730 ADC:
compatible = "sprd,sc2731-adc", "sprd,sc2730-adc";
Moreover I think you can add the "sprd,sc2730-xxx" compatible string
for the mfd_cell in this driver if you are sure we want to support
SC2730 PMIC.
> +
> + ret = devm_mfd_add_devices(dev, id, &cell, 1, NULL, 0, domain);
> + if (ret) {
> + pr_err("devm_mfd_add_devices return fail ret=%d\n", ret);
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> static int sprd_pmic_probe(struct spi_device *spi)
> {
> struct sprd_pmic *ddata;
> @@ -263,10 +225,8 @@ static int sprd_pmic_probe(struct spi_device *spi)
> return ret;
> }
>
> - ret = devm_mfd_add_devices(&spi->dev, PLATFORM_DEVID_AUTO,
> - sprd_pmic_devs, ARRAY_SIZE(sprd_pmic_devs),
> - NULL, 0,
> - regmap_irq_get_domain(ddata->irq_data));
> + ret = sprd_pmic_add_subdevices(&spi->dev, PLATFORM_DEVID_AUTO,
> + regmap_irq_get_domain(ddata->irq_data));
> if (ret) {
> dev_err(&spi->dev, "Failed to register device %d\n", ret);
> return ret;
> --
> 2.20.1
>
--
Baolin Wang
Powered by blists - more mailing lists