Re: [PATCH v2 3/7] mfd: pm8921: Migrate to irqdomains
On Tue, 07 Jan 2014, Stephen Boyd wrote: > On 01/07, Lee Jones wrote: > > > >> + return of_platform_populate(pdev->dev.of_node, NULL, NULL, > > > >> >dev); > > > >> +} > > > > Can't you use the MFD core instead? > > > > > > > > > > Are you suggesting using mfd_add_devices()? At first glance it looks > > > like that would require an array of mfd_cell structures that do nothing > > > besides match compatible strings in the DT. Using of_platform_populate() > > > achieves the same goal and doesn't require an array of mfd_cell > > > structures for each different pm8xxx chip that comes along, meaning > > > simpler code. > > > > I'm inclined to agree, but playing Devil's advocate here, as a device > > using the MFD subsystem it's often clearer to readers and other people > > looking for examples if the MFD core functionality is used. For > > instance, I now have no idea what devices the PM8xxx encompasses > > without looking at the DTS file. A small cell structure is a small > > price to pay for code clarity IMHO. > > > > Why not just put that information in the binding document? And > how is this different from adding a bunch of C files to match a > set of compatible strings that a dts file has just so that we can > add all the devices on a board (think board files for an SoC). > Sure it documents the devices on a board, but we've been moving > away from that by using of_platform_populate(). > > IMHO the code is clear. I want to add all subnodes of this > device's node as children struct devices. Using > of_platform_populate() says that, whereas mfd_add_devices() says > I want to add these specific subnodes of this device's node. > > Also, as more drivers are written and more bindings are ratified > this platform driver will need to be updated with more cells and > more compatible strings, causing more inter-tree dependencies and > more patches. Please, let's avoid this if we can. Okay, that's fine. There are a few discussions floating around about this. If I find some time, I'll have to have a think about the pros and cons of either approach. This is okay for now. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/7] mfd: pm8921: Migrate to irqdomains
On Tue, 07 Jan 2014, Stephen Boyd wrote: On 01/07, Lee Jones wrote: + return of_platform_populate(pdev-dev.of_node, NULL, NULL, pdev-dev); +} Can't you use the MFD core instead? Are you suggesting using mfd_add_devices()? At first glance it looks like that would require an array of mfd_cell structures that do nothing besides match compatible strings in the DT. Using of_platform_populate() achieves the same goal and doesn't require an array of mfd_cell structures for each different pm8xxx chip that comes along, meaning simpler code. I'm inclined to agree, but playing Devil's advocate here, as a device using the MFD subsystem it's often clearer to readers and other people looking for examples if the MFD core functionality is used. For instance, I now have no idea what devices the PM8xxx encompasses without looking at the DTS file. A small cell structure is a small price to pay for code clarity IMHO. Why not just put that information in the binding document? And how is this different from adding a bunch of C files to match a set of compatible strings that a dts file has just so that we can add all the devices on a board (think board files for an SoC). Sure it documents the devices on a board, but we've been moving away from that by using of_platform_populate(). IMHO the code is clear. I want to add all subnodes of this device's node as children struct devices. Using of_platform_populate() says that, whereas mfd_add_devices() says I want to add these specific subnodes of this device's node. Also, as more drivers are written and more bindings are ratified this platform driver will need to be updated with more cells and more compatible strings, causing more inter-tree dependencies and more patches. Please, let's avoid this if we can. Okay, that's fine. There are a few discussions floating around about this. If I find some time, I'll have to have a think about the pros and cons of either approach. This is okay for now. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/7] mfd: pm8921: Migrate to irqdomains
On 01/07, Lee Jones wrote: > > >> +return of_platform_populate(pdev->dev.of_node, NULL, NULL, > > >> >dev); > > >> +} > > > Can't you use the MFD core instead? > > > > > > > Are you suggesting using mfd_add_devices()? At first glance it looks > > like that would require an array of mfd_cell structures that do nothing > > besides match compatible strings in the DT. Using of_platform_populate() > > achieves the same goal and doesn't require an array of mfd_cell > > structures for each different pm8xxx chip that comes along, meaning > > simpler code. > > I'm inclined to agree, but playing Devil's advocate here, as a device > using the MFD subsystem it's often clearer to readers and other people > looking for examples if the MFD core functionality is used. For > instance, I now have no idea what devices the PM8xxx encompasses > without looking at the DTS file. A small cell structure is a small > price to pay for code clarity IMHO. > Why not just put that information in the binding document? And how is this different from adding a bunch of C files to match a set of compatible strings that a dts file has just so that we can add all the devices on a board (think board files for an SoC). Sure it documents the devices on a board, but we've been moving away from that by using of_platform_populate(). IMHO the code is clear. I want to add all subnodes of this device's node as children struct devices. Using of_platform_populate() says that, whereas mfd_add_devices() says I want to add these specific subnodes of this device's node. Also, as more drivers are written and more bindings are ratified this platform driver will need to be updated with more cells and more compatible strings, causing more inter-tree dependencies and more patches. Please, let's avoid this if we can. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/7] mfd: pm8921: Migrate to irqdomains
> >>unsigned intnum_irqs;
> >>unsigned intnum_blocks;
> >>unsigned intnum_masters;
> >> @@ -138,7 +137,7 @@ static int pm8xxx_irq_block_handler(struct pm_irq_chip
> >> *chip, int block)
> >>for (i = 0; i < 8; i++) {
> >>if (bits & (1 << i)) {
> >>pmirq = block * 8 + i;
> >> - irq = pmirq + chip->irq_base;
> >> + irq = irq_find_mapping(chip->domain, pmirq);
> > Going by this patch only, it appears you're calling irq_find_mapping()
> > before you've called irq_create_mapping(). This won't work, so unless
> > you've called the latter in a previous patch, you should ensure that
> > you do so.
> >
>
> Interrupts seem to work. I think that's because the mapping is created
> when the consumer drivers call request_irq().
>
> From what I can tell, if we call irq_find_mapping() and there is no
> mapping associated with it then we have a spurious irq. If that happens
> we'll call handle_generic_irq() with 0 and that will cause
> handle_bad_irq() to be called and a debug message to be logged. That
> seems like a good outcome.
I would try to adhere to the documentation in case we are missing
something or some of the semantics change. Please read:
Documentation/IRQ-domain.txt. Specifically, "=== irq_domain usage ==="
from line 39, which says to call irq_create_mapping() to indeed, create
the mapping.
> > What does the sizeof(u8) add here?
> >
>
> This was just keeping the same code that was already there. I will do
> sizeof(chip->config[0]) instead which is more future proof if that array
> changes type later on.
Ah, now I see what it's doing. Perhaps brackets would be of use to
ensure readers aren't confused. I also think the sizeof() would be
helpful too, so:
chip = devm_kzalloc(>dev, sizeof(*chip) +
(sizeof(chip->config[0]) * nirqs), GFP_KERNEL);
> >> + return of_platform_populate(pdev->dev.of_node, NULL, NULL, >dev);
> >> +}
> > Can't you use the MFD core instead?
> >
>
> Are you suggesting using mfd_add_devices()? At first glance it looks
> like that would require an array of mfd_cell structures that do nothing
> besides match compatible strings in the DT. Using of_platform_populate()
> achieves the same goal and doesn't require an array of mfd_cell
> structures for each different pm8xxx chip that comes along, meaning
> simpler code.
I'm inclined to agree, but playing Devil's advocate here, as a device
using the MFD subsystem it's often clearer to readers and other people
looking for examples if the MFD core functionality is used. For
instance, I now have no idea what devices the PM8xxx encompasses
without looking at the DTS file. A small cell structure is a small
price to pay for code clarity IMHO.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/7] mfd: pm8921: Migrate to irqdomains
unsigned intnum_irqs;
unsigned intnum_blocks;
unsigned intnum_masters;
@@ -138,7 +137,7 @@ static int pm8xxx_irq_block_handler(struct pm_irq_chip
*chip, int block)
for (i = 0; i 8; i++) {
if (bits (1 i)) {
pmirq = block * 8 + i;
- irq = pmirq + chip-irq_base;
+ irq = irq_find_mapping(chip-domain, pmirq);
Going by this patch only, it appears you're calling irq_find_mapping()
before you've called irq_create_mapping(). This won't work, so unless
you've called the latter in a previous patch, you should ensure that
you do so.
Interrupts seem to work. I think that's because the mapping is created
when the consumer drivers call request_irq().
From what I can tell, if we call irq_find_mapping() and there is no
mapping associated with it then we have a spurious irq. If that happens
we'll call handle_generic_irq() with 0 and that will cause
handle_bad_irq() to be called and a debug message to be logged. That
seems like a good outcome.
I would try to adhere to the documentation in case we are missing
something or some of the semantics change. Please read:
Documentation/IRQ-domain.txt. Specifically, === irq_domain usage ===
from line 39, which says to call irq_create_mapping() to indeed, create
the mapping.
What does the sizeof(u8) add here?
This was just keeping the same code that was already there. I will do
sizeof(chip-config[0]) instead which is more future proof if that array
changes type later on.
Ah, now I see what it's doing. Perhaps brackets would be of use to
ensure readers aren't confused. I also think the sizeof() would be
helpful too, so:
chip = devm_kzalloc(pdev-dev, sizeof(*chip) +
(sizeof(chip-config[0]) * nirqs), GFP_KERNEL);
+ return of_platform_populate(pdev-dev.of_node, NULL, NULL, pdev-dev);
+}
Can't you use the MFD core instead?
Are you suggesting using mfd_add_devices()? At first glance it looks
like that would require an array of mfd_cell structures that do nothing
besides match compatible strings in the DT. Using of_platform_populate()
achieves the same goal and doesn't require an array of mfd_cell
structures for each different pm8xxx chip that comes along, meaning
simpler code.
I'm inclined to agree, but playing Devil's advocate here, as a device
using the MFD subsystem it's often clearer to readers and other people
looking for examples if the MFD core functionality is used. For
instance, I now have no idea what devices the PM8xxx encompasses
without looking at the DTS file. A small cell structure is a small
price to pay for code clarity IMHO.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/7] mfd: pm8921: Migrate to irqdomains
On 01/07, Lee Jones wrote: +return of_platform_populate(pdev-dev.of_node, NULL, NULL, pdev-dev); +} Can't you use the MFD core instead? Are you suggesting using mfd_add_devices()? At first glance it looks like that would require an array of mfd_cell structures that do nothing besides match compatible strings in the DT. Using of_platform_populate() achieves the same goal and doesn't require an array of mfd_cell structures for each different pm8xxx chip that comes along, meaning simpler code. I'm inclined to agree, but playing Devil's advocate here, as a device using the MFD subsystem it's often clearer to readers and other people looking for examples if the MFD core functionality is used. For instance, I now have no idea what devices the PM8xxx encompasses without looking at the DTS file. A small cell structure is a small price to pay for code clarity IMHO. Why not just put that information in the binding document? And how is this different from adding a bunch of C files to match a set of compatible strings that a dts file has just so that we can add all the devices on a board (think board files for an SoC). Sure it documents the devices on a board, but we've been moving away from that by using of_platform_populate(). IMHO the code is clear. I want to add all subnodes of this device's node as children struct devices. Using of_platform_populate() says that, whereas mfd_add_devices() says I want to add these specific subnodes of this device's node. Also, as more drivers are written and more bindings are ratified this platform driver will need to be updated with more cells and more compatible strings, causing more inter-tree dependencies and more patches. Please, let's avoid this if we can. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/7] mfd: pm8921: Migrate to irqdomains
On 01/06/14 07:53, Lee Jones wrote:
>
>> @@ -56,8 +56,7 @@
>> struct pm_irq_chip {
>> struct device *dev;
>> spinlock_t pm_irq_lock;
>> -unsigned intdevirq;
>> -unsigned intirq_base;
>> +struct irq_domain *domain;
> It's probably best to explicitly specify 'irqdomain' here in order to
> eliminate any possibility of ambiguity.
Ok. Renamed.
>
>> unsigned intnum_irqs;
>> unsigned intnum_blocks;
>> unsigned intnum_masters;
>> @@ -138,7 +137,7 @@ static int pm8xxx_irq_block_handler(struct pm_irq_chip
>> *chip, int block)
>> for (i = 0; i < 8; i++) {
>> if (bits & (1 << i)) {
>> pmirq = block * 8 + i;
>> -irq = pmirq + chip->irq_base;
>> +irq = irq_find_mapping(chip->domain, pmirq);
> Going by this patch only, it appears you're calling irq_find_mapping()
> before you've called irq_create_mapping(). This won't work, so unless
> you've called the latter in a previous patch, you should ensure that
> you do so.
>
Interrupts seem to work. I think that's because the mapping is created
when the consumer drivers call request_irq().
>From what I can tell, if we call irq_find_mapping() and there is no
mapping associated with it then we have a spurious irq. If that happens
we'll call handle_generic_irq() with 0 and that will cause
handle_bad_irq() to be called and a debug message to be logged. That
seems like a good outcome.
>> -master = block / 8;
> What was the point in this anyway? Was it completely superfluous?
I think it was just copy/paste without thinking if the variables are
actually used.
>
>> +static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq)
>> +{
>> +struct pm_irq_chip *chip;
>> +unsigned int nirqs = 256;
> No magic numbers please.
Done.
>
>> +chip = devm_kzalloc(>dev, sizeof(*chip) + sizeof(u8) * nirqs,
>> +GFP_KERNEL);
> What does the sizeof(u8) add here?
>
This was just keeping the same code that was already there. I will do
sizeof(chip->config[0]) instead which is more future proof if that array
changes type later on.
>
>> +return of_platform_populate(pdev->dev.of_node, NULL, NULL, >dev);
>> +}
> Can't you use the MFD core instead?
>
Are you suggesting using mfd_add_devices()? At first glance it looks
like that would require an array of mfd_cell structures that do nothing
besides match compatible strings in the DT. Using of_platform_populate()
achieves the same goal and doesn't require an array of mfd_cell
structures for each different pm8xxx chip that comes along, meaning
simpler code.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/7] mfd: pm8921: Migrate to irqdomains
> Convert this driver to use irqdomains so that the PMIC's child
> devices can be converted to devicetree.
>
> Signed-off-by: Stephen Boyd
> ---
> drivers/mfd/pm8921-core.c | 184
> ++
> include/linux/mfd/pm8xxx/irq.h| 36
> include/linux/mfd/pm8xxx/pm8921.h | 30 ---
> 3 files changed, 65 insertions(+), 185 deletions(-)
> delete mode 100644 include/linux/mfd/pm8xxx/irq.h
> delete mode 100644 include/linux/mfd/pm8xxx/pm8921.h
>
> @@ -56,8 +56,7 @@
> struct pm_irq_chip {
> struct device *dev;
> spinlock_t pm_irq_lock;
> - unsigned intdevirq;
> - unsigned intirq_base;
> + struct irq_domain *domain;
It's probably best to explicitly specify 'irqdomain' here in order to
eliminate any possibility of ambiguity.
> unsigned intnum_irqs;
> unsigned intnum_blocks;
> unsigned intnum_masters;
> @@ -138,7 +137,7 @@ static int pm8xxx_irq_block_handler(struct pm_irq_chip
> *chip, int block)
> for (i = 0; i < 8; i++) {
> if (bits & (1 << i)) {
> pmirq = block * 8 + i;
> - irq = pmirq + chip->irq_base;
> + irq = irq_find_mapping(chip->domain, pmirq);
Going by this patch only, it appears you're calling irq_find_mapping()
before you've called irq_create_mapping(). This won't work, so unless
you've called the latter in a previous patch, you should ensure that
you do so.
> generic_handle_irq(irq);
> }
> }
> @@ -197,12 +196,11 @@ static void pm8xxx_irq_handler(unsigned int irq, struct
> irq_desc *desc)
> static void pm8xxx_irq_mask_ack(struct irq_data *d)
> {
> struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
> - unsigned int pmirq = d->irq - chip->irq_base;
> - int master, irq_bit;
> + unsigned int pmirq = irqd_to_hwirq(d);
Why don't you call it hwirq instead of pmirq?
> + int irq_bit;
> u8 block, config;
>
> block = pmirq / 8;
Ah, I guess you're keeping the original naming convention.
> - master = block / 8;
What was the point in this anyway? Was it completely superfluous?
> +static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq)
> +{
> + struct pm_irq_chip *chip;
> + unsigned int nirqs = 256;
No magic numbers please.
> + chip = devm_kzalloc(>dev, sizeof(*chip) + sizeof(u8) * nirqs,
> + GFP_KERNEL);
What does the sizeof(u8) add here?
> + return of_platform_populate(pdev->dev.of_node, NULL, NULL, >dev);
> +}
Can't you use the MFD core instead?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/7] mfd: pm8921: Migrate to irqdomains
Convert this driver to use irqdomains so that the PMIC's child
devices can be converted to devicetree.
Signed-off-by: Stephen Boyd sb...@codeaurora.org
---
drivers/mfd/pm8921-core.c | 184
++
include/linux/mfd/pm8xxx/irq.h| 36
include/linux/mfd/pm8xxx/pm8921.h | 30 ---
3 files changed, 65 insertions(+), 185 deletions(-)
delete mode 100644 include/linux/mfd/pm8xxx/irq.h
delete mode 100644 include/linux/mfd/pm8xxx/pm8921.h
snip
@@ -56,8 +56,7 @@
struct pm_irq_chip {
struct device *dev;
spinlock_t pm_irq_lock;
- unsigned intdevirq;
- unsigned intirq_base;
+ struct irq_domain *domain;
It's probably best to explicitly specify 'irqdomain' here in order to
eliminate any possibility of ambiguity.
unsigned intnum_irqs;
unsigned intnum_blocks;
unsigned intnum_masters;
@@ -138,7 +137,7 @@ static int pm8xxx_irq_block_handler(struct pm_irq_chip
*chip, int block)
for (i = 0; i 8; i++) {
if (bits (1 i)) {
pmirq = block * 8 + i;
- irq = pmirq + chip-irq_base;
+ irq = irq_find_mapping(chip-domain, pmirq);
Going by this patch only, it appears you're calling irq_find_mapping()
before you've called irq_create_mapping(). This won't work, so unless
you've called the latter in a previous patch, you should ensure that
you do so.
generic_handle_irq(irq);
}
}
@@ -197,12 +196,11 @@ static void pm8xxx_irq_handler(unsigned int irq, struct
irq_desc *desc)
static void pm8xxx_irq_mask_ack(struct irq_data *d)
{
struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
- unsigned int pmirq = d-irq - chip-irq_base;
- int master, irq_bit;
+ unsigned int pmirq = irqd_to_hwirq(d);
Why don't you call it hwirq instead of pmirq?
+ int irq_bit;
u8 block, config;
block = pmirq / 8;
Ah, I guess you're keeping the original naming convention.
- master = block / 8;
What was the point in this anyway? Was it completely superfluous?
snip
+static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq)
+{
+ struct pm_irq_chip *chip;
+ unsigned int nirqs = 256;
No magic numbers please.
+ chip = devm_kzalloc(pdev-dev, sizeof(*chip) + sizeof(u8) * nirqs,
+ GFP_KERNEL);
What does the sizeof(u8) add here?
snip
+ return of_platform_populate(pdev-dev.of_node, NULL, NULL, pdev-dev);
+}
Can't you use the MFD core instead?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/7] mfd: pm8921: Migrate to irqdomains
On 01/06/14 07:53, Lee Jones wrote:
@@ -56,8 +56,7 @@
struct pm_irq_chip {
struct device *dev;
spinlock_t pm_irq_lock;
-unsigned intdevirq;
-unsigned intirq_base;
+struct irq_domain *domain;
It's probably best to explicitly specify 'irqdomain' here in order to
eliminate any possibility of ambiguity.
Ok. Renamed.
unsigned intnum_irqs;
unsigned intnum_blocks;
unsigned intnum_masters;
@@ -138,7 +137,7 @@ static int pm8xxx_irq_block_handler(struct pm_irq_chip
*chip, int block)
for (i = 0; i 8; i++) {
if (bits (1 i)) {
pmirq = block * 8 + i;
-irq = pmirq + chip-irq_base;
+irq = irq_find_mapping(chip-domain, pmirq);
Going by this patch only, it appears you're calling irq_find_mapping()
before you've called irq_create_mapping(). This won't work, so unless
you've called the latter in a previous patch, you should ensure that
you do so.
Interrupts seem to work. I think that's because the mapping is created
when the consumer drivers call request_irq().
From what I can tell, if we call irq_find_mapping() and there is no
mapping associated with it then we have a spurious irq. If that happens
we'll call handle_generic_irq() with 0 and that will cause
handle_bad_irq() to be called and a debug message to be logged. That
seems like a good outcome.
-master = block / 8;
What was the point in this anyway? Was it completely superfluous?
I think it was just copy/paste without thinking if the variables are
actually used.
+static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq)
+{
+struct pm_irq_chip *chip;
+unsigned int nirqs = 256;
No magic numbers please.
Done.
+chip = devm_kzalloc(pdev-dev, sizeof(*chip) + sizeof(u8) * nirqs,
+GFP_KERNEL);
What does the sizeof(u8) add here?
This was just keeping the same code that was already there. I will do
sizeof(chip-config[0]) instead which is more future proof if that array
changes type later on.
+return of_platform_populate(pdev-dev.of_node, NULL, NULL, pdev-dev);
+}
Can't you use the MFD core instead?
Are you suggesting using mfd_add_devices()? At first glance it looks
like that would require an array of mfd_cell structures that do nothing
besides match compatible strings in the DT. Using of_platform_populate()
achieves the same goal and doesn't require an array of mfd_cell
structures for each different pm8xxx chip that comes along, meaning
simpler code.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
