Now that all the DT drivers have switched to platform_get_irq() we can now
safely drop the static setup of IRQ resource from DT core code.
With the above change hierarchical setup of irq domains is no longer
bypassed and thus allowing hierarchical interrupt domains to describe
interrupts using "interrupts" DT property.
Signed-off-by: Lad Prabhakar <[email protected]>
---
Hi All,
Sending this as RFC as couple of more drivers need to hit -rc yet with
the platform_get_irq() change while that is in progress I wanted to get
some feedback on this patch.
Cheers,
Prabhakar
---
drivers/of/platform.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 793350028906..6890f7fe556f 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -114,35 +114,31 @@ struct platform_device *of_device_alloc(struct device_node *np,
struct device *parent)
{
struct platform_device *dev;
- int rc, i, num_reg = 0, num_irq;
+ int rc, i, num_reg = 0;
struct resource *res, temp_res;
dev = platform_device_alloc("", PLATFORM_DEVID_NONE);
if (!dev)
return NULL;
- /* count the io and irq resources */
+ /* count the io resources */
while (of_address_to_resource(np, num_reg, &temp_res) == 0)
num_reg++;
- num_irq = of_irq_count(np);
/* Populate the resource table */
- if (num_irq || num_reg) {
- res = kcalloc(num_irq + num_reg, sizeof(*res), GFP_KERNEL);
+ if (num_reg) {
+ res = kcalloc(num_reg, sizeof(*res), GFP_KERNEL);
if (!res) {
platform_device_put(dev);
return NULL;
}
- dev->num_resources = num_reg + num_irq;
+ dev->num_resources = num_reg;
dev->resource = res;
for (i = 0; i < num_reg; i++, res++) {
rc = of_address_to_resource(np, i, res);
WARN_ON(rc);
}
- if (of_irq_to_resource_table(np, res, num_irq) != num_irq)
- pr_debug("not all legacy IRQ resources mapped for %pOFn\n",
- np);
}
dev->dev.of_node = of_node_get(np);
--
2.17.1
On Wed, 16 Mar 2022 20:06:33 +0000,
Lad Prabhakar <[email protected]> wrote:
>
> Now that all the DT drivers have switched to platform_get_irq() we can now
> safely drop the static setup of IRQ resource from DT core code.
>
> With the above change hierarchical setup of irq domains is no longer
> bypassed and thus allowing hierarchical interrupt domains to describe
> interrupts using "interrupts" DT property.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> Hi All,
>
> Sending this as RFC as couple of more drivers need to hit -rc yet with
> the platform_get_irq() change while that is in progress I wanted to get
> some feedback on this patch.
>
> Cheers,
> Prabhakar
> ---
> drivers/of/platform.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 793350028906..6890f7fe556f 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -114,35 +114,31 @@ struct platform_device *of_device_alloc(struct device_node *np,
> struct device *parent)
> {
> struct platform_device *dev;
> - int rc, i, num_reg = 0, num_irq;
> + int rc, i, num_reg = 0;
> struct resource *res, temp_res;
>
> dev = platform_device_alloc("", PLATFORM_DEVID_NONE);
> if (!dev)
> return NULL;
>
> - /* count the io and irq resources */
> + /* count the io resources */
> while (of_address_to_resource(np, num_reg, &temp_res) == 0)
> num_reg++;
> - num_irq = of_irq_count(np);
>
> /* Populate the resource table */
> - if (num_irq || num_reg) {
> - res = kcalloc(num_irq + num_reg, sizeof(*res), GFP_KERNEL);
> + if (num_reg) {
> + res = kcalloc(num_reg, sizeof(*res), GFP_KERNEL);
> if (!res) {
> platform_device_put(dev);
> return NULL;
> }
>
> - dev->num_resources = num_reg + num_irq;
> + dev->num_resources = num_reg;
> dev->resource = res;
> for (i = 0; i < num_reg; i++, res++) {
> rc = of_address_to_resource(np, i, res);
> WARN_ON(rc);
> }
> - if (of_irq_to_resource_table(np, res, num_irq) != num_irq)
> - pr_debug("not all legacy IRQ resources mapped for %pOFn\n",
> - np);
> }
>
> dev->dev.of_node = of_node_get(np);
I think this definitely goes in the right direction by not eagerly
populating resources without a driver actually needing it. If anything
breaks, that should be seen as an opportunity to fix the users of this
misfeature. I booted a couple of boxes with this patch, and nothing
caught fire, so:
Acked-by: Marc Zyngier <[email protected]>
Tested-by: Marc Zyngier <[email protected]>
M.
--
Without deviation from the norm, progress is not possible.
On Wed, Mar 16, 2022 at 08:06:33PM +0000, Lad Prabhakar wrote:
> Now that all the DT drivers have switched to platform_get_irq() we can now
> safely drop the static setup of IRQ resource from DT core code.
>
> With the above change hierarchical setup of irq domains is no longer
> bypassed and thus allowing hierarchical interrupt domains to describe
> interrupts using "interrupts" DT property.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> Hi All,
>
> Sending this as RFC as couple of more drivers need to hit -rc yet with
> the platform_get_irq() change while that is in progress I wanted to get
> some feedback on this patch.
I just applied this on top of current master and pushed to my
for-kernelci branch. It should show up in kernelCI in a bit. I did this
before all the fixes too and there were definitely a couple of test
regressions.
Rob
Hi Rob,
On Thu, Mar 31, 2022 at 10:02 PM Rob Herring <[email protected]> wrote:
>
> On Wed, Mar 16, 2022 at 08:06:33PM +0000, Lad Prabhakar wrote:
> > Now that all the DT drivers have switched to platform_get_irq() we can now
> > safely drop the static setup of IRQ resource from DT core code.
> >
> > With the above change hierarchical setup of irq domains is no longer
> > bypassed and thus allowing hierarchical interrupt domains to describe
> > interrupts using "interrupts" DT property.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > Hi All,
> >
> > Sending this as RFC as couple of more drivers need to hit -rc yet with
> > the platform_get_irq() change while that is in progress I wanted to get
> > some feedback on this patch.
>
> I just applied this on top of current master and pushed to my
> for-kernelci branch. It should show up in kernelCI in a bit. I did this
> before all the fixes too and there were definitely a couple of test
> regressions.
>
Any chance you can share the regressions or maybe the CI script so
that I can reproduce it locally.
Cheers,
Prabhakar
On Fri, Apr 1, 2022 at 2:42 AM Lad, Prabhakar
<[email protected]> wrote:
>
> Hi Rob,
>
> On Thu, Mar 31, 2022 at 10:02 PM Rob Herring <[email protected]> wrote:
> >
> > On Wed, Mar 16, 2022 at 08:06:33PM +0000, Lad Prabhakar wrote:
> > > Now that all the DT drivers have switched to platform_get_irq() we can now
> > > safely drop the static setup of IRQ resource from DT core code.
> > >
> > > With the above change hierarchical setup of irq domains is no longer
> > > bypassed and thus allowing hierarchical interrupt domains to describe
> > > interrupts using "interrupts" DT property.
> > >
> > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > ---
> > > Hi All,
> > >
> > > Sending this as RFC as couple of more drivers need to hit -rc yet with
> > > the platform_get_irq() change while that is in progress I wanted to get
> > > some feedback on this patch.
> >
> > I just applied this on top of current master and pushed to my
> > for-kernelci branch. It should show up in kernelCI in a bit. I did this
> > before all the fixes too and there were definitely a couple of test
> > regressions.
> >
> Any chance you can share the regressions or maybe the CI script so
> that I can reproduce it locally.
It will show up here[1] as 'robh', but it still hasn't built yet which
is strange.
Rob
[1] https://linux.kernelci.org/job/
On Wed, Mar 16, 2022 at 08:06:33PM +0000, Lad Prabhakar wrote:
> Now that all the DT drivers have switched to platform_get_irq() we can now
> safely drop the static setup of IRQ resource from DT core code.
>
> With the above change hierarchical setup of irq domains is no longer
> bypassed and thus allowing hierarchical interrupt domains to describe
> interrupts using "interrupts" DT property.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> Hi All,
>
> Sending this as RFC as couple of more drivers need to hit -rc yet with
> the platform_get_irq() change while that is in progress I wanted to get
> some feedback on this patch.
I've applied this now and it is in today's linux-next. Keep an eye out
for any regression reports. There's one for i.MX8 in kernel-ci, but I
don't think it is related.
Rob
On Wed, Apr 6, 2022 at 2:57 PM Rob Herring <[email protected]> wrote:
>
> On Wed, Mar 16, 2022 at 08:06:33PM +0000, Lad Prabhakar wrote:
> > Now that all the DT drivers have switched to platform_get_irq() we can now
> > safely drop the static setup of IRQ resource from DT core code.
> >
> > With the above change hierarchical setup of irq domains is no longer
> > bypassed and thus allowing hierarchical interrupt domains to describe
> > interrupts using "interrupts" DT property.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > Hi All,
> >
> > Sending this as RFC as couple of more drivers need to hit -rc yet with
> > the platform_get_irq() change while that is in progress I wanted to get
> > some feedback on this patch.
>
> I've applied this now and it is in today's linux-next. Keep an eye out
> for any regression reports. There's one for i.MX8 in kernel-ci, but I
> don't think it is related.
>
Thanks for the heads up, sure I will keep an eye on it.
Cheers,
Prabhakar
Hi, Lad
# sorry for the confusion if you have received it before with the
non-plain-text mode
In this change you said "all the DT drivers have switched to
platform_get_irq()",
could you please help share with me one example about the above change
as reference?
We have one hikey960 android build with some out of tree changes,
which could not boot
successfully with some errors on surfaceflinger(I am not sure it's a
problem with the gpu or display),
but could boot if I have this change reverted.
I guess it needs some changes on the gpu/display dts or driver side to
have it work
with this change, not sure if you could give some suggestions on the fix.
And here are two out of tree changes might be related listed here just
for reference in case:
https://android-review.linaro.org/c/kernel/common/+/21680
https://android-review.linaro.org/c/kernel/common/+/21682
Thanks in advance!
On Thu, 17 Mar 2022 at 04:07, Lad Prabhakar
<[email protected]> wrote:
>
> Now that all the DT drivers have switched to platform_get_irq() we can now
> safely drop the static setup of IRQ resource from DT core code.
>
> With the above change hierarchical setup of irq domains is no longer
> bypassed and thus allowing hierarchical interrupt domains to describe
> interrupts using "interrupts" DT property.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> Hi All,
>
> Sending this as RFC as couple of more drivers need to hit -rc yet with
> the platform_get_irq() change while that is in progress I wanted to get
> some feedback on this patch.
>
> Cheers,
> Prabhakar
> ---
> drivers/of/platform.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 793350028906..6890f7fe556f 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -114,35 +114,31 @@ struct platform_device *of_device_alloc(struct device_node *np,
> struct device *parent)
> {
> struct platform_device *dev;
> - int rc, i, num_reg = 0, num_irq;
> + int rc, i, num_reg = 0;
> struct resource *res, temp_res;
>
> dev = platform_device_alloc("", PLATFORM_DEVID_NONE);
> if (!dev)
> return NULL;
>
> - /* count the io and irq resources */
> + /* count the io resources */
> while (of_address_to_resource(np, num_reg, &temp_res) == 0)
> num_reg++;
> - num_irq = of_irq_count(np);
>
> /* Populate the resource table */
> - if (num_irq || num_reg) {
> - res = kcalloc(num_irq + num_reg, sizeof(*res), GFP_KERNEL);
> + if (num_reg) {
> + res = kcalloc(num_reg, sizeof(*res), GFP_KERNEL);
> if (!res) {
> platform_device_put(dev);
> return NULL;
> }
>
> - dev->num_resources = num_reg + num_irq;
> + dev->num_resources = num_reg;
> dev->resource = res;
> for (i = 0; i < num_reg; i++, res++) {
> rc = of_address_to_resource(np, i, res);
> WARN_ON(rc);
> }
> - if (of_irq_to_resource_table(np, res, num_irq) != num_irq)
> - pr_debug("not all legacy IRQ resources mapped for %pOFn\n",
> - np);
> }
>
> dev->dev.of_node = of_node_get(np);
> --
> 2.17.1
>
--
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/linaro-android
Hi Yongqin,
On Sat, Jun 11, 2022 at 6:28 AM Yongqin Liu <[email protected]> wrote:
>
> Hi, Lad
>
> # sorry for the confusion if you have received it before with the
> non-plain-text mode
>
> In this change you said "all the DT drivers have switched to
> platform_get_irq()",
> could you please help share with me one example about the above change
> as reference?
The change is we just switch to using platform_get_irq() [0] for
fetching IRQ numbers.
> We have one hikey960 android build with some out of tree changes,
> which could not boot
> successfully with some errors on surfaceflinger(I am not sure it's a
> problem with the gpu or display),
> but could boot if I have this change reverted.
>
> I guess it needs some changes on the gpu/display dts or driver side to
> have it work
Just the changes to the driver is needed.
> with this change, not sure if you could give some suggestions on the fix.
>
> And here are two out of tree changes might be related listed here just
> for reference in case:
> https://android-review.linaro.org/c/kernel/common/+/21680
> https://android-review.linaro.org/c/kernel/common/+/21682
>
[0] https://lore.kernel.org/lkml/[email protected]/
Cheers,
Prabhakar
Hi, Lad
Thanks a lot for the links and suggestions!
Finally I resolved the problem with the call of
platform_get_irq_byname and irq_get_trigger_type.
Btw, I just have a question about the of_irq_to_resource function.
At the beginning I tried to use platform_get_irq and of_irq_to_resource
to get the irq name and flags information, but it seems of_irq_to_resource
does not work as expected, maybe I called incorrectly somewhere,
here I just want to ask, do you think that if of_irq_to_resource still
could be used to
get the resource with the irq returned from platform_get_irq?
Thanks,
Yongqin Liu
On Sat, 11 Jun 2022 at 16:01, Lad, Prabhakar <[email protected]> wrote:
>
> Hi Yongqin,
>
> On Sat, Jun 11, 2022 at 6:28 AM Yongqin Liu <[email protected]> wrote:
> >
> > Hi, Lad
> >
> > # sorry for the confusion if you have received it before with the
> > non-plain-text mode
> >
> > In this change you said "all the DT drivers have switched to
> > platform_get_irq()",
> > could you please help share with me one example about the above change
> > as reference?
> The change is we just switch to using platform_get_irq() [0] for
> fetching IRQ numbers.
>
> > We have one hikey960 android build with some out of tree changes,
> > which could not boot
> > successfully with some errors on surfaceflinger(I am not sure it's a
> > problem with the gpu or display),
> > but could boot if I have this change reverted.
> >
> > I guess it needs some changes on the gpu/display dts or driver side to
> > have it work
> Just the changes to the driver is needed.
>
> > with this change, not sure if you could give some suggestions on the fix.
> >
> > And here are two out of tree changes might be related listed here just
> > for reference in case:
> > https://android-review.linaro.org/c/kernel/common/+/21680
> > https://android-review.linaro.org/c/kernel/common/+/21682
> >
>
> [0] https://lore.kernel.org/lkml/[email protected]/
>
> Cheers,
> Prabhakar
--
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/linaro-android
On Mon, Jun 13, 2022 at 1:41 PM Yongqin Liu <[email protected]> wrote:
>
> Hi, Lad
>
> Thanks a lot for the links and suggestions!
> Finally I resolved the problem with the call of
> platform_get_irq_byname and irq_get_trigger_type.
>
> Btw, I just have a question about the of_irq_to_resource function.
> At the beginning I tried to use platform_get_irq and of_irq_to_resource
> to get the irq name and flags information, but it seems of_irq_to_resource
> does not work as expected, maybe I called incorrectly somewhere,
> here I just want to ask, do you think that if of_irq_to_resource still
> could be used to
> get the resource with the irq returned from platform_get_irq?
>
>
Yes of_irq_to_resource() can be used to get the irq number.
Cheers,
Prabhakar