From: Steve Twiss <stwiss.opensource@diasemi.com>
To: Lee Jones <lee.jones@linaro.org>
Cc: LINUX-KERNEL <linux-kernel@vger.kernel.org>,
DEVICETREE <devicetree@vger.kernel.org>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Eduardo Valentin <edubezval@gmail.com>,
Guenter Roeck <linux@roeck-us.net>,
LINUX-INPUT <linux-input@vger.kernel.org>,
LINUX-PM <linux-pm@vger.kernel.org>,
LINUX-WATCHDOG <linux-watchdog@vger.kernel.org>,
Liam Girdwood <lgirdwood@gmail.com>,
Mark Brown <broonie@kernel.org>,
"Mark Rutland" <mark.rutland@arm.com>,
Rob Herring <robh+dt@kernel.org>,
"Support Opensource" <Support.Opensource@diasemi.com>,
Wim Van Sebroeck <wim@iguana.be>, Zhang Rui <rui.zhang@intel.com>
Subject: RE: [PATCH V3 5/9] mfd: da9061: MFD core support
Date: Mon, 7 Nov 2016 15:25:00 +0000 [thread overview]
Message-ID: <6ED8E3B22081A4459DAC7699F3695FB7018CCE7CB6@SW-EX-MBX02.diasemi.com> (raw)
In-Reply-To: <20161102142854.GS13127@dell>
On 02 November 2016 14:29, Lee Jones wrote:
> On Mon, 31 Oct 2016, Steve Twiss wrote:
> > From: Steve Twiss <stwiss.opensource@diasemi.com>
> >
> > @@ -475,7 +855,25 @@ static int da9062_i2c_probe(struct i2c_client *i2c,
> > return -EINVAL;
> > }
> >
> > - chip->regmap = devm_regmap_init_i2c(i2c, &da9062_regmap_config);
> > + switch (chip->chip_type) {
> > + case(COMPAT_TYPE_DA9061):
> > + cell = da9061_devs;
> > + cell_num = ARRAY_SIZE(da9061_devs);
> > + irq_chip = &da9061_irq_chip;
> > + config = &da9061_regmap_config;
> > + break;
> > + case(COMPAT_TYPE_DA9062):
> > + cell = da9062_devs;
> > + cell_num = ARRAY_SIZE(da9062_devs);
> > + irq_chip = &da9062_irq_chip;
> > + config = &da9062_regmap_config;
> > + break;
> > + default:
> > + dev_err(chip->dev, "Unrecognised chip type\n");
> > + return -ENODEV;
> > + }
>
> I very much dislike when MFD and OF functionality is mixed.
>
> In your case you can use da9062_get_device_type() to dynamically
> interrogate the device and register using the correct MFD cells that
> way.
Hi Lee,
It's the device tree that decides what the chip type is. It's not chip
interrogation in this case. The ordering dictates this I think: to access the
hardware ID register, a regmap definition is needed first. But because the
correct I2C register map requires a knowledge of what chip is being used,
it becomes a circular dependency.
To solve this dependency, I define the chip type (DA9061 or DA9062) in the
device tree and assign the correct regmap first before accessing any registers.
> > + chip->regmap = devm_regmap_init_i2c(i2c, config);
> > if (IS_ERR(chip->regmap)) {
> > ret = PTR_ERR(chip->regmap);
> > dev_err(chip->dev, "Failed to allocate register map: %d\n",
> > @@ -493,7 +891,7 @@ static int da9062_i2c_probe(struct i2c_client *i2c,
> >
> > ret = regmap_add_irq_chip(chip->regmap, i2c->irq,
> > IRQF_TRIGGER_LOW | IRQF_ONESHOT |IRQF_SHARED,
> > - -1, &da9062_irq_chip,
> > + -1, irq_chip,
>
> What is -1?
.. it's a request for an irq_base.
http://lxr.free-electrons.com/source/kernel/irq/irqdesc.c#L477
Is there a reason I shouldn't be doing that?
There doesn't seem to be a #define anywhere, and using -1 seems
to be the standard in the kernel at the moment.
Regards,
Steve
next prev parent reply other threads:[~2016-11-07 15:25 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-31 16:02 [PATCH V3 0/9] da9061: DA9061 driver submission Steve Twiss
2016-10-31 16:02 ` [PATCH V3 1/9] Documentation: devicetree: input: additions for da9061 onkey driver Steve Twiss
2016-11-09 0:17 ` Dmitry Torokhov
2016-11-09 18:24 ` Rob Herring
2016-10-31 16:02 ` [PATCH V3 4/9] Documentation: devicetree: mfd: da9062/61 MFD binding Steve Twiss
2016-11-02 14:32 ` Lee Jones
2016-11-07 15:28 ` Steve Twiss
2016-11-09 18:24 ` Rob Herring
2016-10-31 16:02 ` [PATCH V3 3/9] Documentation: devicetree: thermal: da9062/61 TJUNC temperature binding Steve Twiss
2016-11-09 18:24 ` Rob Herring
2016-10-31 16:02 ` [PATCH V3 2/9] Documentation: devicetree: watchdog: da9062/61 watchdog timer binding Steve Twiss
2016-11-09 18:24 ` Rob Herring
2016-10-31 16:02 ` [PATCH V3 6/9] regulator: da9061: BUCK and LDO regulator driver Steve Twiss
2016-10-31 16:02 ` [PATCH V3 5/9] mfd: da9061: MFD core support Steve Twiss
2016-11-02 14:28 ` Lee Jones
2016-11-07 15:25 ` Steve Twiss [this message]
2016-10-31 16:02 ` [PATCH V3 7/9] watchdog: da9062/61: watchdog driver Steve Twiss
2016-10-31 16:28 ` Guenter Roeck
2016-10-31 16:02 ` [PATCH V3 8/9] thermal: da9062/61: Thermal junction temperature monitoring driver Steve Twiss
2016-11-02 13:28 ` Lukasz Luba
2016-11-09 18:20 ` Steve Twiss
2016-11-11 9:57 ` Lukasz Luba
2016-10-31 16:02 ` [PATCH V3 9/9] MAINTAINERS: da9062/61 updates to the Dialog Semiconductor search terms Steve Twiss
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=6ED8E3B22081A4459DAC7699F3695FB7018CCE7CB6@SW-EX-MBX02.diasemi.com \
--to=stwiss.opensource@diasemi.com \
--cc=Support.Opensource@diasemi.com \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.torokhov@gmail.com \
--cc=edubezval@gmail.com \
--cc=lee.jones@linaro.org \
--cc=lgirdwood@gmail.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-watchdog@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=mark.rutland@arm.com \
--cc=robh+dt@kernel.org \
--cc=rui.zhang@intel.com \
--cc=wim@iguana.be \
/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).