From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751547AbdFGQIs (ORCPT ); Wed, 7 Jun 2017 12:08:48 -0400 Received: from fllnx209.ext.ti.com ([198.47.19.16]:55464 "EHLO fllnx209.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750831AbdFGQIq (ORCPT ); Wed, 7 Jun 2017 12:08:46 -0400 Subject: Re: [PATCH 4/4] mfd: tps65217: Instantiate sub-devices from device tree To: Enric Balletbo i Serra , , , , , , , Mark Brown CC: Dmitry Torokhov , Lee Jones , Daniel Thompson , Jingoo Han , Richard Purdie , Jacek Anaszewski , Pavel Machek , Rob Herring , Mark Rutland , Russell King , Tony Lindgren , References: <20170607103242.16008-1-enric.balletbo@collabora.com> <20170607103242.16008-4-enric.balletbo@collabora.com> From: Grygorii Strashko Message-ID: <6359abe2-9a21-78a2-f5ea-87e2e3335af3@ti.com> Date: Wed, 7 Jun 2017 11:05:32 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <20170607103242.16008-4-enric.balletbo@collabora.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [128.247.59.147] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/07/2017 05:32 AM, Enric Balletbo i Serra wrote: > The driver boots only via device tree but currently all the MFD sub-devices > are instatiated independently of the device tree configuration, i.e > although tps65217-charger is disabled by default it's instantiated by the > MFD driver. > > Instead of register all sub-devices, if the TPS65217 device tree node has a > sub-node enabled, try to instatiate them as MFD sub-devices but not > instantiate sub-nodes that are not enabled. > > Signed-off-by: Enric Balletbo i Serra > --- > drivers/mfd/tps65217.c | 56 +++++++------------------------------------- > include/linux/mfd/tps65217.h | 6 ----- > 2 files changed, 9 insertions(+), 53 deletions(-) > > diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c > index f769c7d..9effdda 100644 > --- a/drivers/mfd/tps65217.c > +++ b/drivers/mfd/tps65217.c > @@ -33,14 +33,7 @@ > #include > #include > > -static struct resource charger_resources[] = { > - DEFINE_RES_IRQ_NAMED(TPS65217_IRQ_AC, "AC"), > - DEFINE_RES_IRQ_NAMED(TPS65217_IRQ_USB, "USB"), > -}; > - > -static struct resource pb_resources[] = { > - DEFINE_RES_IRQ_NAMED(TPS65217_IRQ_PB, "PB"), > -}; May be I messed smth, but how about interrupts for charger and pwrbutton? > +#define TPS65217_NUM_IRQ 3 > > static void tps65217_irq_lock(struct irq_data *data) > { > @@ -86,29 +79,6 @@ static struct irq_chip tps65217_irq_chip = { > .irq_disable = tps65217_irq_disable, > }; > > -static struct mfd_cell tps65217s[] = { > - { > - .name = "tps65217-pmic", > - .of_compatible = "ti,tps65217-pmic", > - }, > - { > - .name = "tps65217-bl", > - .of_compatible = "ti,tps65217-bl", > - }, > - { > - .name = "tps65217-charger", > - .num_resources = ARRAY_SIZE(charger_resources), > - .resources = charger_resources, > - .of_compatible = "ti,tps65217-charger", > - }, > - { > - .name = "tps65217-pwrbutton", > - .num_resources = ARRAY_SIZE(pb_resources), > - .resources = pb_resources, > - .of_compatible = "ti,tps65217-pwrbutton", > - }, > -}; > - > static irqreturn_t tps65217_irq_thread(int irq, void *data) > { > struct tps65217 *tps = data; > @@ -359,23 +329,8 @@ static int tps65217_probe(struct i2c_client *client, > return ret; > } > > - if (client->irq) { > + if (client->irq) > tps65217_irq_init(tps, client->irq); > - } else { > - int i; > - > - /* Don't tell children about IRQ resources which won't fire */ > - for (i = 0; i < ARRAY_SIZE(tps65217s); i++) > - tps65217s[i].num_resources = 0; > - } > - > - ret = devm_mfd_add_devices(tps->dev, -1, tps65217s, > - ARRAY_SIZE(tps65217s), NULL, 0, > - tps->irq_domain); > - if (ret < 0) { > - dev_err(tps->dev, "mfd_add_devices failed: %d\n", ret); > - return ret; > - } And as I remember there was a request to use mfd_add_devices() and not of_platform_populate() for mfd sub-devices instantiation. > > ret = tps65217_reg_read(tps, TPS65217_REG_CHIPID, &version); > if (ret < 0) { > @@ -384,6 +339,13 @@ static int tps65217_probe(struct i2c_client *client, > return ret; > } > > + ret = of_platform_populate(client->dev.of_node, NULL, NULL, > + &client->dev); > + if (ret) { > + dev_err(&client->dev, "Failed to register sub-devices\n"); > + return ret; > + } > + > /* Set the PMIC to shutdown on PWR_EN toggle */ > if (status_off) { > ret = tps65217_set_bits(tps, TPS65217_REG_STATUS, > diff --git a/include/linux/mfd/tps65217.h b/include/linux/mfd/tps65217.h > index eac2857..dfc51f5 100644 > --- a/include/linux/mfd/tps65217.h > +++ b/include/linux/mfd/tps65217.h > @@ -236,12 +236,6 @@ struct tps65217_bl_pdata { > int dft_brightness; > }; > > -/* Interrupt numbers */ > -#define TPS65217_IRQ_USB 0 > -#define TPS65217_IRQ_AC 1 > -#define TPS65217_IRQ_PB 2 > -#define TPS65217_NUM_IRQ 3 > - > /** > * struct tps65217_board - packages regulator init data > * @tps65217_regulator_data: regulator initialization values > -- regards, -grygorii