| 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: <50B46F55.4000107@marvell.com>
Date: Tue, 27 Nov 2012 15:44:21 +0800
From: Qing Xu <qingx@...vell.com>
To: Haojian Zhuang <haojian.zhuang@...il.com>
Cc: Samuel Ortiz <sameo@...ux.intel.com>,
Grant Likely <grant.likely@...retlab.ca>,
Rob Herring <rob.herring@...xeda.com>,
Chao Xie <cxie4@...vell.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree-discuss@...ts.ozlabs.org"
<devicetree-discuss@...ts.ozlabs.org>
Subject: Re: [PATCH 1/7] mfd: max8925: add irqdomain for dt
On 11/23/2012 05:05 PM, Haojian Zhuang wrote:
> On Tue, Nov 6, 2012 at 3:37 PM, Qing Xu <qingx@...vell.com> wrote:
>> From: Qing Xu <qingx@...vell.com>
>>
>> Add irqdomains for max8925's main irq, and touch irq.
>> Wrap irq register operations into irqdomain's map func.
>> it is necessary for dt support.
>> Also, add dt support for max8925 driver.
>>
>> Signed-off-by: Qing Xu <qingx@...vell.com>
>> ---
>> drivers/mfd/max8925-core.c | 87 ++++++++++++++++++++++++++++---------------
>> drivers/mfd/max8925-i2c.c | 32 +++++++++++++++-
>> include/linux/mfd/max8925.h | 12 ++++-
>> 3 files changed, 96 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/mfd/max8925-core.c b/drivers/mfd/max8925-core.c
>> index 1e0ab0a..dcc218a 100644
>> --- a/drivers/mfd/max8925-core.c
>> +++ b/drivers/mfd/max8925-core.c
>> @@ -14,10 +14,14 @@
>> #include <linux/i2c.h>
>> #include <linux/irq.h>
>> #include <linux/interrupt.h>
>> +#include <linux/irqdomain.h>
>> #include <linux/platform_device.h>
>> #include <linux/regulator/machine.h>
>> #include <linux/mfd/core.h>
>> #include <linux/mfd/max8925.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_irq.h>
>>
>> static struct resource bk_resources[] __devinitdata = {
>> { 0x84, 0x84, "mode control", IORESOURCE_REG, },
>> @@ -639,17 +643,34 @@ static struct irq_chip max8925_irq_chip = {
>> .irq_disable = max8925_irq_disable,
>> };
>>
>> +static int max8925_irq_domain_map(struct irq_domain *d, unsigned int virq,
>> + irq_hw_number_t hw)
>> +{
>> + irq_set_chip_data(virq, d->host_data);
>> + irq_set_chip_and_handler(virq, &max8925_irq_chip, handle_edge_irq);
>> + irq_set_nested_thread(virq, 1);
>> +#ifdef CONFIG_ARM
>> + set_irq_flags(virq, IRQF_VALID);
>> +#else
>> + irq_set_noprobe(virq);
>> +#endif
>> + return 0;
>> +}
>> +
>> +static struct irq_domain_ops max8925_irq_domain_ops = {
>> + .map = max8925_irq_domain_map,
>> + .xlate = irq_domain_xlate_onetwocell,
>> +};
>> +
>> +
>> static int max8925_irq_init(struct max8925_chip *chip, int irq,
>> struct max8925_platform_data *pdata)
>> {
>> unsigned long flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
>> - int i, ret;
>> - int __irq;
>> + int ret;
>> + int tsc_irq;
>> + struct device_node *node = chip->dev->of_node;
>>
>> - if (!pdata || !pdata->irq_base) {
>> - dev_warn(chip->dev, "No interrupt support on IRQ base\n");
>> - return -EINVAL;
>> - }
>> /* clear all interrupts */
>> max8925_reg_read(chip->i2c, MAX8925_CHG_IRQ1);
>> max8925_reg_read(chip->i2c, MAX8925_CHG_IRQ2);
>> @@ -667,45 +688,51 @@ static int max8925_irq_init(struct max8925_chip *chip, int irq,
>> max8925_reg_write(chip->rtc, MAX8925_RTC_IRQ_MASK, 0xff);
>>
>> mutex_init(&chip->irq_lock);
>> - chip->core_irq = irq;
>> - chip->irq_base = pdata->irq_base;
>>
>> - /* register with genirq */
>> - for (i = 0; i < ARRAY_SIZE(max8925_irqs); i++) {
>> - __irq = i + chip->irq_base;
>> - irq_set_chip_data(__irq, chip);
>> - irq_set_chip_and_handler(__irq, &max8925_irq_chip,
>> - handle_edge_irq);
>> - irq_set_nested_thread(__irq, 1);
>> -#ifdef CONFIG_ARM
>> - set_irq_flags(__irq, IRQF_VALID);
>> -#else
>> - irq_set_noprobe(__irq);
>> -#endif
>> - }
>> - if (!irq) {
>> - dev_warn(chip->dev, "No interrupt support on core IRQ\n");
>> - goto tsc_irq;
>> + /* domain1: init charger/rtc/onkey irq domain*/
>> + chip->irq_base = irq_alloc_descs(-1, 0, MAX8925_NR_IRQS, 0);
>> + if (chip->irq_base < 0) {
>> + dev_err(chip->dev, "Failed to allocate interrupts, ret:%d\n",
>> + chip->irq_base);
>> + return -EBUSY;
>> }
>>
>> + irq_domain_add_legacy(node, MAX8925_NR_IRQS, chip->irq_base, 0,
>> + &max8925_irq_domain_ops, chip);
>> + chip->core_irq = irq;
>> + if (!chip->core_irq)
>> + return -EBUSY;
>> +
>> ret = request_threaded_irq(irq, NULL, max8925_irq, flags,
>> "max8925", chip);
>> if (ret) {
>> dev_err(chip->dev, "Failed to request core IRQ: %d\n", ret);
>> chip->core_irq = 0;
>> + return -EBUSY;
>> }
>>
>> -tsc_irq:
>> + /* domain2: init touch irq domain*/
>> /* mask TSC interrupt */
>> max8925_reg_write(chip->adc, MAX8925_TSC_IRQ_MASK, 0x0f);
>>
>> - if (!pdata->tsc_irq) {
>> + chip->tsc_irq_base = irq_alloc_descs(-1, 0, MAX8925_NR_TSC_IRQS, 0);
>> + if (chip->tsc_irq < 0) {
>> + dev_err(chip->dev, "Failed to allocate interrupts, ret:%d\n",
>> + chip->tsc_irq_base);
>> + return -EBUSY;
>> + }
>> +
>> + irq_domain_add_legacy(node, MAX8925_NR_TSC_IRQS, chip->tsc_irq_base, 0,
>> + &max8925_irq_domain_ops, chip);
>> +
>> + tsc_irq = irq_of_parse_and_map(node, 1);
>> +
> I'm confused on this. Let's look at your definition in DTS.
>
> + pmic: max8925@3c {
> + compatible = "marvell,max8925";
> + reg = <0x3c>;
> + interrupts = <1 0>;
> + interrupt-parent = <&intcmux4>;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> pmic is defined as interrupt controller. So it could be referenced as
> interrupt-parent
> by other drivers.
>
> Now you split TSC_IRQS from original MAX8925_NR_IRQS. So TSC_IRQS are calculated
> from 0. How do you distinguish TSC_IRQS from normal MAX8925_IRQS? Only
> one phandle
> is defined in your DTS file.
>
> So either you keep TSC_IRQS in MAX8925_NR_IRQS, or you add a child
> node to define
> a new interrupt controller for tsc irqs.
>
>> + if (!tsc_irq) {
>> dev_warn(chip->dev, "No interrupt support on TSC IRQ\n");
>> return 0;
>> }
>> - chip->tsc_irq = pdata->tsc_irq;
>> -
>> - ret = request_threaded_irq(chip->tsc_irq, NULL, max8925_tsc_irq,
>> + chip->tsc_irq = tsc_irq;
>> + ret = request_threaded_irq(tsc_irq, NULL, max8925_tsc_irq,
>> flags, "max8925-tsc", chip);
>> if (ret) {
>> dev_err(chip->dev, "Failed to request TSC IRQ: %d\n", ret);
>> @@ -876,7 +903,7 @@ int __devinit max8925_device_init(struct max8925_chip *chip,
>> if (pdata && pdata->power) {
>> ret = mfd_add_devices(chip->dev, 0, &power_devs[0],
>> ARRAY_SIZE(power_devs),
>> - &power_supply_resources[0], 0, NULL);
>> + &power_supply_resources[0], 0, NULL);
>> if (ret < 0) {
>> dev_err(chip->dev, "Failed to add power supply "
>> "subdev\n");
>> diff --git a/drivers/mfd/max8925-i2c.c b/drivers/mfd/max8925-i2c.c
>> index d9e4b36..b713be6 100644
>> --- a/drivers/mfd/max8925-i2c.c
>> +++ b/drivers/mfd/max8925-i2c.c
>> @@ -135,13 +135,35 @@ static const struct i2c_device_id max8925_id_table[] = {
>> };
>> MODULE_DEVICE_TABLE(i2c, max8925_id_table);
>>
>> +
>> +static int __devinit max8925_dt_init(struct device_node *np,
>> + struct device *dev,
>> + struct max8925_platform_data *pdata)
>> +{
>> + return 0;
>> +}
>> +
> If you don't need this interface, you need define it.
>
>> static int __devinit max8925_probe(struct i2c_client *client,
>> const struct i2c_device_id *id)
>> {
>> struct max8925_platform_data *pdata = client->dev.platform_data;
>> static struct max8925_chip *chip;
>> + struct device_node *node = client->dev.of_node;
>> + int ret;
>>
>> - if (!pdata) {
>> + if (node && !pdata) {
>> + /* parse DT to get platform data */
>> + pdata = devm_kzalloc(&client->dev,
>> + sizeof(struct max8925_platform_data),
>> + GFP_KERNEL);
>> + if (!pdata)
>> + return -ENOMEM;
>> + ret = max8925_dt_init(node, &client->dev, pdata);
>> + if (ret) {
>> + pr_info("%s: failed to parse dt info\n", __func__);
>> + return -EINVAL;
>> + }
>> + } else if (!pdata) {
>> pr_info("%s: platform data is missing\n", __func__);
>> return -EINVAL;
>> }
>> @@ -203,11 +225,18 @@ static int max8925_resume(struct device *dev)
>>
>> static SIMPLE_DEV_PM_OPS(max8925_pm_ops, max8925_suspend, max8925_resume);
>>
>> +static const struct of_device_id max8925_dt_ids[] = {
>> + { .compatible = "marvell,max8925", },
> max8925 isn't the product of Marvell. The vendor is maxium.
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, max8925_dt_ids);
>> +
>> static struct i2c_driver max8925_driver = {
>> .driver = {
>> .name = "max8925",
>> .owner = THIS_MODULE,
>> .pm = &max8925_pm_ops,
>> + .of_match_table = of_match_ptr(max8925_dt_ids),
>> },
>> .probe = max8925_probe,
>> .remove = __devexit_p(max8925_remove),
>> @@ -217,7 +246,6 @@ static struct i2c_driver max8925_driver = {
>> static int __init max8925_i2c_init(void)
>> {
>> int ret;
>> -
>> ret = i2c_add_driver(&max8925_driver);
>> if (ret != 0)
>> pr_err("Failed to register MAX8925 I2C driver: %d\n", ret);
>> diff --git a/include/linux/mfd/max8925.h b/include/linux/mfd/max8925.h
>> index 74d8e29..67bc540 100644
>> --- a/include/linux/mfd/max8925.h
>> +++ b/include/linux/mfd/max8925.h
>> @@ -185,11 +185,18 @@ enum {
>> MAX8925_IRQ_GPM_SYSCKEN_R,
>> MAX8925_IRQ_RTC_ALARM1,
>> MAX8925_IRQ_RTC_ALARM0,
>> + MAX8925_NR_IRQS,
>> +};
>> +
>> +/**/
>> +enum {
>> MAX8925_IRQ_TSC_STICK,
>> MAX8925_IRQ_TSC_NSTICK,
>> - MAX8925_NR_IRQS,
>> + MAX8925_NR_TSC_IRQS,
>> };
>>
>> +
>> +
>> struct max8925_chip {
>> struct device *dev;
>> struct i2c_client *i2c;
>> @@ -201,7 +208,7 @@ struct max8925_chip {
>> int irq_base;
>> int core_irq;
>> int tsc_irq;
>> -
>> + int tsc_irq_base;
>> unsigned int wakeup_flag;
>> };
>>
>> @@ -259,7 +266,6 @@ struct max8925_platform_data {
>> struct regulator_init_data *ldo20;
>>
>> int irq_base;
>> - int tsc_irq;
>> };
>>
>> extern int max8925_reg_read(struct i2c_client *, int);
>> --
>> 1.7.0.4
>>
update both dts and this patch in v2, please help me review again,
thank a lot!!
-Qing
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists