From: Lee Jones <lee.jones@linaro.org>
To: Baolin Wang <baolin.wang7@gmail.com>
Cc: Chunyan Zhang <zhang.lyra@gmail.com>,
LKML <linux-kernel@vger.kernel.org>,
Orson Zhai <orsonzhai@gmail.com>,
Chunyan Zhang <chunyan.zhang@unisoc.com>
Subject: Re: [PATCH] mfd: sprd: get subdevices from DT instead for SC27XX SPI
Date: Tue, 16 Jun 2020 08:12:17 +0100 [thread overview]
Message-ID: <20200616071217.GE2608702@dell> (raw)
In-Reply-To: <CADBw62oCNrvTgu9k-iykaX4bbpVWjbg3GN+v5yPBCc9R2bR3Pg@mail.gmail.com>
On Mon, 15 Jun 2020, Baolin Wang wrote:
> On Mon, Jun 15, 2020 at 11:47 AM Chunyan Zhang <zhang.lyra@gmail.com> wrote:
> >
> > From: Chunyan Zhang <chunyan.zhang@unisoc.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@unisoc.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.
Why not just use of_platform_populate()?
> > +
> > + 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;
> >
>
>
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
prev parent reply other threads:[~2020-06-16 7:12 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-15 3:47 [PATCH] mfd: sprd: get subdevices from DT instead for SC27XX SPI Chunyan Zhang
2020-06-15 14:11 ` Baolin Wang
2020-06-15 15:08 ` Orson Zhai
2020-06-15 23:30 ` Baolin Wang
2020-06-16 8:12 ` Lee Jones
2020-06-16 7:12 ` Lee Jones [this message]
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=20200616071217.GE2608702@dell \
--to=lee.jones@linaro.org \
--cc=baolin.wang7@gmail.com \
--cc=chunyan.zhang@unisoc.com \
--cc=linux-kernel@vger.kernel.org \
--cc=orsonzhai@gmail.com \
--cc=zhang.lyra@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).