From: Wolfram Sang <wsa@the-dreams.de>
To: Lee Jones <lee.jones@linaro.org>
Cc: gene_chen@richtek.com, linux-kernel@vger.kernel.org,
cy_huang@richtek.com, linux-mediatek@lists.infradead.org,
matthias.bgg@gmail.com, Gene Chen <gene.chen.richtek@gmail.com>,
Wilma.Wu@mediatek.com, linux-arm-kernel@lists.infradead.org,
shufan_lee@richtek.com
Subject: Re: [PATCH v4] mfd: mt6360: add pmic mt6360 driver
Date: Thu, 24 Oct 2019 10:52:13 +0200 [thread overview]
Message-ID: <20191024085212.GB2850@kunai> (raw)
In-Reply-To: <20191024082623.GK15843@dell>
[-- Attachment #1.1: Type: text/plain, Size: 1681 bytes --]
> > + for (i = 0; i < MT6360_SLAVE_MAX; i++) {
> > + if (mt6360_slave_addr[i] == client->addr) {
> > + mpd->i2c[i] = client;
> > + continue;
> > + }
Not knowing the DT bindings, I wonder if we can let the for-loop start
at 1 and do beforehand:
mpd->i2c[0] = client;
That would save the above if block. However, this is a minor nit.
> > + mpd->i2c[i] = i2c_new_dummy(client->adapter,
> > + mt6360_slave_addr[i]);
Please use the new API i2c_new_dummy_device here...
> > + if (!mpd->i2c[i]) {
... and IS_ERR() here.
> > + dev_err(&client->dev, "new i2c dev [%d] fail\n", i);
> > + ret = -ENODEV;
Then you can also return a proper errno value.
You can probably also use devm_i2c_new_dummy_device()...
> > + goto out;
> > + }
> > + i2c_set_clientdata(mpd->i2c[i], mpd);
> > + }
> > +
> > + ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO,
> > + mt6360_devs, ARRAY_SIZE(mt6360_devs), NULL,
> > + 0, regmap_irq_get_domain(mpd->irq_data));
> > + if (ret < 0) {
> > + dev_err(&client->dev, "mfd add cells fail\n");
> > + goto out;
> > + }
> > +
> > + return 0;
> > +out:
> > + while (--i >= 0) {
> > + if (mpd->i2c[i]->addr == client->addr)
> > + continue;
> > + i2c_unregister_device(mpd->i2c[i]);
... to save this ...
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int mt6360_pmu_remove(struct i2c_client *client)
> > +{
> > + struct mt6360_pmu_data *mpd = i2c_get_clientdata(client);
> > + int i;
> > +
> > + for (i = 0; i < MT6360_SLAVE_MAX; i++) {
> > + if (mpd->i2c[i]->addr == client->addr)
> > + continue;
> > + i2c_unregister_device(mpd->i2c[i]);
> > + }
... and this. But please double check.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
WARNING: multiple messages have this Message-ID (diff)
From: Wolfram Sang <wsa@the-dreams.de>
To: Lee Jones <lee.jones@linaro.org>
Cc: gene_chen@richtek.com, linux-kernel@vger.kernel.org,
cy_huang@richtek.com, linux-mediatek@lists.infradead.org,
matthias.bgg@gmail.com, Gene Chen <gene.chen.richtek@gmail.com>,
Wilma.Wu@mediatek.com, linux-arm-kernel@lists.infradead.org,
shufan_lee@richtek.com
Subject: Re: [PATCH v4] mfd: mt6360: add pmic mt6360 driver
Date: Thu, 24 Oct 2019 10:52:13 +0200 [thread overview]
Message-ID: <20191024085212.GB2850@kunai> (raw)
In-Reply-To: <20191024082623.GK15843@dell>
[-- Attachment #1.1: Type: text/plain, Size: 1681 bytes --]
> > + for (i = 0; i < MT6360_SLAVE_MAX; i++) {
> > + if (mt6360_slave_addr[i] == client->addr) {
> > + mpd->i2c[i] = client;
> > + continue;
> > + }
Not knowing the DT bindings, I wonder if we can let the for-loop start
at 1 and do beforehand:
mpd->i2c[0] = client;
That would save the above if block. However, this is a minor nit.
> > + mpd->i2c[i] = i2c_new_dummy(client->adapter,
> > + mt6360_slave_addr[i]);
Please use the new API i2c_new_dummy_device here...
> > + if (!mpd->i2c[i]) {
... and IS_ERR() here.
> > + dev_err(&client->dev, "new i2c dev [%d] fail\n", i);
> > + ret = -ENODEV;
Then you can also return a proper errno value.
You can probably also use devm_i2c_new_dummy_device()...
> > + goto out;
> > + }
> > + i2c_set_clientdata(mpd->i2c[i], mpd);
> > + }
> > +
> > + ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO,
> > + mt6360_devs, ARRAY_SIZE(mt6360_devs), NULL,
> > + 0, regmap_irq_get_domain(mpd->irq_data));
> > + if (ret < 0) {
> > + dev_err(&client->dev, "mfd add cells fail\n");
> > + goto out;
> > + }
> > +
> > + return 0;
> > +out:
> > + while (--i >= 0) {
> > + if (mpd->i2c[i]->addr == client->addr)
> > + continue;
> > + i2c_unregister_device(mpd->i2c[i]);
... to save this ...
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int mt6360_pmu_remove(struct i2c_client *client)
> > +{
> > + struct mt6360_pmu_data *mpd = i2c_get_clientdata(client);
> > + int i;
> > +
> > + for (i = 0; i < MT6360_SLAVE_MAX; i++) {
> > + if (mpd->i2c[i]->addr == client->addr)
> > + continue;
> > + i2c_unregister_device(mpd->i2c[i]);
> > + }
... and this. But please double check.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Wolfram Sang <wsa@the-dreams.de>
To: Lee Jones <lee.jones@linaro.org>
Cc: Gene Chen <gene.chen.richtek@gmail.com>,
matthias.bgg@gmail.com, linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
gene_chen@richtek.com, Wilma.Wu@mediatek.com,
shufan_lee@richtek.com, cy_huang@richtek.com
Subject: Re: [PATCH v4] mfd: mt6360: add pmic mt6360 driver
Date: Thu, 24 Oct 2019 10:52:13 +0200 [thread overview]
Message-ID: <20191024085212.GB2850@kunai> (raw)
In-Reply-To: <20191024082623.GK15843@dell>
[-- Attachment #1: Type: text/plain, Size: 1681 bytes --]
> > + for (i = 0; i < MT6360_SLAVE_MAX; i++) {
> > + if (mt6360_slave_addr[i] == client->addr) {
> > + mpd->i2c[i] = client;
> > + continue;
> > + }
Not knowing the DT bindings, I wonder if we can let the for-loop start
at 1 and do beforehand:
mpd->i2c[0] = client;
That would save the above if block. However, this is a minor nit.
> > + mpd->i2c[i] = i2c_new_dummy(client->adapter,
> > + mt6360_slave_addr[i]);
Please use the new API i2c_new_dummy_device here...
> > + if (!mpd->i2c[i]) {
... and IS_ERR() here.
> > + dev_err(&client->dev, "new i2c dev [%d] fail\n", i);
> > + ret = -ENODEV;
Then you can also return a proper errno value.
You can probably also use devm_i2c_new_dummy_device()...
> > + goto out;
> > + }
> > + i2c_set_clientdata(mpd->i2c[i], mpd);
> > + }
> > +
> > + ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO,
> > + mt6360_devs, ARRAY_SIZE(mt6360_devs), NULL,
> > + 0, regmap_irq_get_domain(mpd->irq_data));
> > + if (ret < 0) {
> > + dev_err(&client->dev, "mfd add cells fail\n");
> > + goto out;
> > + }
> > +
> > + return 0;
> > +out:
> > + while (--i >= 0) {
> > + if (mpd->i2c[i]->addr == client->addr)
> > + continue;
> > + i2c_unregister_device(mpd->i2c[i]);
... to save this ...
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int mt6360_pmu_remove(struct i2c_client *client)
> > +{
> > + struct mt6360_pmu_data *mpd = i2c_get_clientdata(client);
> > + int i;
> > +
> > + for (i = 0; i < MT6360_SLAVE_MAX; i++) {
> > + if (mpd->i2c[i]->addr == client->addr)
> > + continue;
> > + i2c_unregister_device(mpd->i2c[i]);
> > + }
... and this. But please double check.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2019-10-24 8:53 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-22 13:02 [PATCH v4] mfd: mt6360: add pmic mt6360 driver Gene Chen
2019-10-22 13:02 ` Gene Chen
2019-10-22 13:02 ` Gene Chen
2019-10-24 8:26 ` Lee Jones
2019-10-24 8:26 ` Lee Jones
2019-10-24 8:26 ` Lee Jones
2019-10-24 8:52 ` Wolfram Sang [this message]
2019-10-24 8:52 ` Wolfram Sang
2019-10-24 8:52 ` Wolfram Sang
2019-10-24 8:29 ` Lee Jones
2019-10-24 8:29 ` Lee Jones
2019-10-24 8:29 ` Lee Jones
2019-10-24 14:42 ` kbuild test robot
2019-10-24 14:42 ` kbuild test robot
2019-10-24 14:42 ` kbuild test robot
2019-10-24 14:42 ` kbuild test robot
2019-10-24 15:07 ` kbuild test robot
2019-10-24 15:07 ` kbuild test robot
2019-10-24 15:07 ` kbuild test robot
2019-10-24 15:07 ` kbuild test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191024085212.GB2850@kunai \
--to=wsa@the-dreams.de \
--cc=Wilma.Wu@mediatek.com \
--cc=cy_huang@richtek.com \
--cc=gene.chen.richtek@gmail.com \
--cc=gene_chen@richtek.com \
--cc=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=shufan_lee@richtek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.