diff mbox

[V2,13/14] dt-bindings: arm-gic: Add documentation for Tegra210 AGIC

Message ID 572C56A6.7020408@nvidia.com
State Superseded, archived
Headers show

Commit Message

Jon Hunter May 6, 2016, 8:32 a.m. UTC
Hi Mark,

On 28/04/16 10:55, Mark Rutland wrote:

[...]

> The "nvidia,tegra210-agic" string can be taken as describing any
> Tegra-210 specific integration quirks, though I agree that's also not
> fantastic for extending PM support beyond Tegra 210 and variants
> thereof.
> 
> So maybe the best approach is bailing out in the presence of clocks
> and/or power domains after all, on the assumption that nothing today has
> those properties, though I fear we may have problems with that later
> down the line if/when people describe those for the root GIC to describe
> those must be hogged, even if not explicitly managed.

On further testing, by bailing out in the presence of clocks and/or
power-domains, the problem I now see is that although the primary gic-400
has been registered, we still try to probe it again later as it matches
the platform driver. One way to avoid this would be ...

"tegra210-agic" for the compatibility flag.

Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Geert Uytterhoeven May 7, 2016, 2:10 p.m. UTC | #1
Hi Jon,

On Fri, May 6, 2016 at 10:32 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>> The "nvidia,tegra210-agic" string can be taken as describing any
>> Tegra-210 specific integration quirks, though I agree that's also not
>> fantastic for extending PM support beyond Tegra 210 and variants
>> thereof.
>>
>> So maybe the best approach is bailing out in the presence of clocks
>> and/or power domains after all, on the assumption that nothing today has
>> those properties, though I fear we may have problems with that later
>> down the line if/when people describe those for the root GIC to describe
>> those must be hogged, even if not explicitly managed.
>
> On further testing, by bailing out in the presence of clocks and/or
> power-domains, the problem I now see is that although the primary gic-400
> has been registered, we still try to probe it again later as it matches
> the platform driver. One way to avoid this would be ...
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index e7bfc175b8e1..631da7ad0dbf 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -556,6 +556,8 @@ void __init of_irq_init(const struct of_device_id *matches)
>                          * its children can get processed in a subsequent pass.
>                          */
>                         list_add_tail(&desc->list, &intc_parent_list);
> +
> +                       of_node_set_flag(desc->dev, OF_POPULATED);
>                 }

That sounds like the right thing to do to me...

> If this is not appropriate then I guess I will just need to use
> "tegra210-agic" for the compatibility flag.

As I want this for plain gic-400, I'd be unhappy ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter May 8, 2016, 12:25 p.m. UTC | #2
Hi Geert,

On 07/05/16 15:10, Geert Uytterhoeven wrote:
> Hi Jon,
> 
> On Fri, May 6, 2016 at 10:32 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>> The "nvidia,tegra210-agic" string can be taken as describing any
>>> Tegra-210 specific integration quirks, though I agree that's also not
>>> fantastic for extending PM support beyond Tegra 210 and variants
>>> thereof.
>>>
>>> So maybe the best approach is bailing out in the presence of clocks
>>> and/or power domains after all, on the assumption that nothing today has
>>> those properties, though I fear we may have problems with that later
>>> down the line if/when people describe those for the root GIC to describe
>>> those must be hogged, even if not explicitly managed.
>>
>> On further testing, by bailing out in the presence of clocks and/or
>> power-domains, the problem I now see is that although the primary gic-400
>> has been registered, we still try to probe it again later as it matches
>> the platform driver. One way to avoid this would be ...
>>
>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>> index e7bfc175b8e1..631da7ad0dbf 100644
>> --- a/drivers/of/irq.c
>> +++ b/drivers/of/irq.c
>> @@ -556,6 +556,8 @@ void __init of_irq_init(const struct of_device_id *matches)
>>                          * its children can get processed in a subsequent pass.
>>                          */
>>                         list_add_tail(&desc->list, &intc_parent_list);
>> +
>> +                       of_node_set_flag(desc->dev, OF_POPULATED);
>>                 }
> 
> That sounds like the right thing to do to me...

OK. The more I think about this, it does seem silly to create a device
and pdata for a device that has already been instantiated.

>> If this is not appropriate then I guess I will just need to use
>> "tegra210-agic" for the compatibility flag.
> 
> As I want this for plain gic-400, I'd be unhappy ;-)

No problem. However, there is more work that would be needed to get this
to work for root controllers which I think that you want.

Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier May 9, 2016, 9:32 a.m. UTC | #3
On 08/05/16 13:25, Jon Hunter wrote:
> Hi Geert,
> 
> On 07/05/16 15:10, Geert Uytterhoeven wrote:
>> Hi Jon,
>>
>> On Fri, May 6, 2016 at 10:32 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>>> The "nvidia,tegra210-agic" string can be taken as describing any
>>>> Tegra-210 specific integration quirks, though I agree that's also not
>>>> fantastic for extending PM support beyond Tegra 210 and variants
>>>> thereof.
>>>>
>>>> So maybe the best approach is bailing out in the presence of clocks
>>>> and/or power domains after all, on the assumption that nothing today has
>>>> those properties, though I fear we may have problems with that later
>>>> down the line if/when people describe those for the root GIC to describe
>>>> those must be hogged, even if not explicitly managed.
>>>
>>> On further testing, by bailing out in the presence of clocks and/or
>>> power-domains, the problem I now see is that although the primary gic-400
>>> has been registered, we still try to probe it again later as it matches
>>> the platform driver. One way to avoid this would be ...
>>>
>>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>>> index e7bfc175b8e1..631da7ad0dbf 100644
>>> --- a/drivers/of/irq.c
>>> +++ b/drivers/of/irq.c
>>> @@ -556,6 +556,8 @@ void __init of_irq_init(const struct of_device_id *matches)
>>>                          * its children can get processed in a subsequent pass.
>>>                          */
>>>                         list_add_tail(&desc->list, &intc_parent_list);
>>> +
>>> +                       of_node_set_flag(desc->dev, OF_POPULATED);
>>>                 }
>>
>> That sounds like the right thing to do to me...
> 
> OK. The more I think about this, it does seem silly to create a device
> and pdata for a device that has already been instantiated.
> 
>>> If this is not appropriate then I guess I will just need to use
>>> "tegra210-agic" for the compatibility flag.
>>
>> As I want this for plain gic-400, I'd be unhappy ;-)
> 
> No problem. However, there is more work that would be needed to get this
> to work for root controllers which I think that you want.

All this brings the discussion back to the root of the problem: irqchips
(and timers) are not first class devices, because we need them too early
for that.

I'd really like to solve this, but the kernel init is incredibly
complicated, and the subsystem dependencies completely undocumented. It
looks like we need the timer early because the we fork a thread for
PID-1, and the scheduler is going to need some form of tick.

So ideally, we'd be able to move the irq/timer stuff *after* the device
framework (which itself requires devtmpfs to be up and running, hence
dragging the whole VM and VFS), but before the scheduler is initialized.
I'm sure there is plenty of other dependencies I haven't worked out yet.

If anyone has some spare time and willing to help, please speak now! ;-)

Thanks,

	M.
Rob Herring May 11, 2016, 3:51 p.m. UTC | #4
On Sat, May 7, 2016 at 9:10 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Jon,
>
> On Fri, May 6, 2016 at 10:32 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>> The "nvidia,tegra210-agic" string can be taken as describing any
>>> Tegra-210 specific integration quirks, though I agree that's also not
>>> fantastic for extending PM support beyond Tegra 210 and variants
>>> thereof.
>>>
>>> So maybe the best approach is bailing out in the presence of clocks
>>> and/or power domains after all, on the assumption that nothing today has
>>> those properties, though I fear we may have problems with that later
>>> down the line if/when people describe those for the root GIC to describe
>>> those must be hogged, even if not explicitly managed.
>>
>> On further testing, by bailing out in the presence of clocks and/or
>> power-domains, the problem I now see is that although the primary gic-400
>> has been registered, we still try to probe it again later as it matches
>> the platform driver. One way to avoid this would be ...
>>
>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>> index e7bfc175b8e1..631da7ad0dbf 100644
>> --- a/drivers/of/irq.c
>> +++ b/drivers/of/irq.c
>> @@ -556,6 +556,8 @@ void __init of_irq_init(const struct of_device_id *matches)
>>                          * its children can get processed in a subsequent pass.
>>                          */
>>                         list_add_tail(&desc->list, &intc_parent_list);
>> +
>> +                       of_node_set_flag(desc->dev, OF_POPULATED);
>>                 }
>
> That sounds like the right thing to do to me...

Seems fine to me, but it would be a problem since this is a global
decision if you wanted to have some hand-off from an "early driver" to
a platform driver. I guess setting the flag could move to drivers that
need it although I don't think drivers should be touching the flags.

>> If this is not appropriate then I guess I will just need to use
>> "tegra210-agic" for the compatibility flag.
>
> As I want this for plain gic-400, I'd be unhappy ;-)

IMO, the plain gic-400 should not have these dependencies and you
should use SoC specific compatible strings should you need to deal
with this problem.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter May 11, 2016, 4:08 p.m. UTC | #5
Hi Rob,

On 11/05/16 16:51, Rob Herring wrote:
> On Sat, May 7, 2016 at 9:10 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> Hi Jon,
>>
>> On Fri, May 6, 2016 at 10:32 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>>> The "nvidia,tegra210-agic" string can be taken as describing any
>>>> Tegra-210 specific integration quirks, though I agree that's also not
>>>> fantastic for extending PM support beyond Tegra 210 and variants
>>>> thereof.
>>>>
>>>> So maybe the best approach is bailing out in the presence of clocks
>>>> and/or power domains after all, on the assumption that nothing today has
>>>> those properties, though I fear we may have problems with that later
>>>> down the line if/when people describe those for the root GIC to describe
>>>> those must be hogged, even if not explicitly managed.
>>>
>>> On further testing, by bailing out in the presence of clocks and/or
>>> power-domains, the problem I now see is that although the primary gic-400
>>> has been registered, we still try to probe it again later as it matches
>>> the platform driver. One way to avoid this would be ...
>>>
>>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>>> index e7bfc175b8e1..631da7ad0dbf 100644
>>> --- a/drivers/of/irq.c
>>> +++ b/drivers/of/irq.c
>>> @@ -556,6 +556,8 @@ void __init of_irq_init(const struct of_device_id *matches)
>>>                          * its children can get processed in a subsequent pass.
>>>                          */
>>>                         list_add_tail(&desc->list, &intc_parent_list);
>>> +
>>> +                       of_node_set_flag(desc->dev, OF_POPULATED);
>>>                 }
>>
>> That sounds like the right thing to do to me...
> 
> Seems fine to me, but it would be a problem since this is a global
> decision if you wanted to have some hand-off from an "early driver" to
> a platform driver. I guess setting the flag could move to drivers that
> need it although I don't think drivers should be touching the flags.

Isn't this the other way around? Setting this flag means that I have
been populated and so don't bother creating a platform device for this
device as it isn't needed. A by-product if this, is that if we did
happen to have a platform driver for the irqchip that also has an early
driver, then the hand-off would never happen if the early init was
successful.

The driver would still have to decide whether to hand-off and to do that
it would need to return an error from the early driver [0].

>>> If this is not appropriate then I guess I will just need to use
>>> "tegra210-agic" for the compatibility flag.
>>
>> As I want this for plain gic-400, I'd be unhappy ;-)
> 
> IMO, the plain gic-400 should not have these dependencies and you
> should use SoC specific compatible strings should you need to deal
> with this problem.

It is fine for my case, but it does mean I cannot say ...

	compatible = "tegra210-agic", "gic-400";

... because this will always match the early driver (unless we do
something like I have suggested above). So I would have ...

	compatible = "tegra210-agic";

Cheers
Jon

[0] http://marc.info/?l=devicetree&m=146237938725709&w=2
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter May 11, 2016, 4:10 p.m. UTC | #6
On 11/05/16 17:08, Jon Hunter wrote:
> Hi Rob,
> 
> On 11/05/16 16:51, Rob Herring wrote:
>> On Sat, May 7, 2016 at 9:10 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> Hi Jon,
>>>
>>> On Fri, May 6, 2016 at 10:32 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>>>> The "nvidia,tegra210-agic" string can be taken as describing any
>>>>> Tegra-210 specific integration quirks, though I agree that's also not
>>>>> fantastic for extending PM support beyond Tegra 210 and variants
>>>>> thereof.
>>>>>
>>>>> So maybe the best approach is bailing out in the presence of clocks
>>>>> and/or power domains after all, on the assumption that nothing today has
>>>>> those properties, though I fear we may have problems with that later
>>>>> down the line if/when people describe those for the root GIC to describe
>>>>> those must be hogged, even if not explicitly managed.
>>>>
>>>> On further testing, by bailing out in the presence of clocks and/or
>>>> power-domains, the problem I now see is that although the primary gic-400
>>>> has been registered, we still try to probe it again later as it matches
>>>> the platform driver. One way to avoid this would be ...
>>>>
>>>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>>>> index e7bfc175b8e1..631da7ad0dbf 100644
>>>> --- a/drivers/of/irq.c
>>>> +++ b/drivers/of/irq.c
>>>> @@ -556,6 +556,8 @@ void __init of_irq_init(const struct of_device_id *matches)
>>>>                          * its children can get processed in a subsequent pass.
>>>>                          */
>>>>                         list_add_tail(&desc->list, &intc_parent_list);
>>>> +
>>>> +                       of_node_set_flag(desc->dev, OF_POPULATED);
>>>>                 }
>>>
>>> That sounds like the right thing to do to me...
>>
>> Seems fine to me, but it would be a problem since this is a global
>> decision if you wanted to have some hand-off from an "early driver" to
>> a platform driver. I guess setting the flag could move to drivers that
>> need it although I don't think drivers should be touching the flags.
> 
> Isn't this the other way around? Setting this flag means that I have
> been populated and so don't bother creating a platform device for this
> device as it isn't needed. A by-product if this, is that if we did
> happen to have a platform driver for the irqchip that also has an early
> driver, then the hand-off would never happen if the early init was
> successful.
> 
> The driver would still have to decide whether to hand-off and to do that
> it would need to return an error from the early driver [0].

I mean, the "early driver would still have to decide whether to hand-off
..."

Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter May 11, 2016, 4:16 p.m. UTC | #7
On 11/05/16 17:08, Jon Hunter wrote:
> Hi Rob,
> 
> On 11/05/16 16:51, Rob Herring wrote:
>> On Sat, May 7, 2016 at 9:10 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> Hi Jon,
>>>
>>> On Fri, May 6, 2016 at 10:32 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>>>> The "nvidia,tegra210-agic" string can be taken as describing any
>>>>> Tegra-210 specific integration quirks, though I agree that's also not
>>>>> fantastic for extending PM support beyond Tegra 210 and variants
>>>>> thereof.
>>>>>
>>>>> So maybe the best approach is bailing out in the presence of clocks
>>>>> and/or power domains after all, on the assumption that nothing today has
>>>>> those properties, though I fear we may have problems with that later
>>>>> down the line if/when people describe those for the root GIC to describe
>>>>> those must be hogged, even if not explicitly managed.
>>>>
>>>> On further testing, by bailing out in the presence of clocks and/or
>>>> power-domains, the problem I now see is that although the primary gic-400
>>>> has been registered, we still try to probe it again later as it matches
>>>> the platform driver. One way to avoid this would be ...
>>>>
>>>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>>>> index e7bfc175b8e1..631da7ad0dbf 100644
>>>> --- a/drivers/of/irq.c
>>>> +++ b/drivers/of/irq.c
>>>> @@ -556,6 +556,8 @@ void __init of_irq_init(const struct of_device_id *matches)
>>>>                          * its children can get processed in a subsequent pass.
>>>>                          */
>>>>                         list_add_tail(&desc->list, &intc_parent_list);
>>>> +
>>>> +                       of_node_set_flag(desc->dev, OF_POPULATED);
>>>>                 }
>>>
>>> That sounds like the right thing to do to me...
>>
>> Seems fine to me, but it would be a problem since this is a global
>> decision if you wanted to have some hand-off from an "early driver" to
>> a platform driver. I guess setting the flag could move to drivers that
>> need it although I don't think drivers should be touching the flags.
> 
> Isn't this the other way around? Setting this flag means that I have
> been populated and so don't bother creating a platform device for this
> device as it isn't needed. A by-product if this, is that if we did
> happen to have a platform driver for the irqchip that also has an early
> driver, then the hand-off would never happen if the early init was
> successful.
> 
> The driver would still have to decide whether to hand-off and to do that
> it would need to return an error from the early driver [0].
> 
>>>> If this is not appropriate then I guess I will just need to use
>>>> "tegra210-agic" for the compatibility flag.
>>>
>>> As I want this for plain gic-400, I'd be unhappy ;-)
>>
>> IMO, the plain gic-400 should not have these dependencies and you
>> should use SoC specific compatible strings should you need to deal
>> with this problem.
> 
> It is fine for my case, but it does mean I cannot say ...
> 
> 	compatible = "tegra210-agic", "gic-400";
> 
> ... because this will always match the early driver (unless we do
> something like I have suggested above). So I would have ...

Sorry this is wrong. The above will always match the early driver.

The problem with the above compatibility string is that, if the platform
driver matches "gic-400" then it will try to probe all gic-400s even if
they have been initialised early and this is definitely not what we
want. This could be solved by setting the OF_POPULATED flag.

Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring May 11, 2016, 4:30 p.m. UTC | #8
On Wed, May 11, 2016 at 11:16 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>
> On 11/05/16 17:08, Jon Hunter wrote:
>> Hi Rob,
>>
>> On 11/05/16 16:51, Rob Herring wrote:
>>> On Sat, May 7, 2016 at 9:10 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>> Hi Jon,
>>>>
>>>> On Fri, May 6, 2016 at 10:32 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>>>>> The "nvidia,tegra210-agic" string can be taken as describing any
>>>>>> Tegra-210 specific integration quirks, though I agree that's also not
>>>>>> fantastic for extending PM support beyond Tegra 210 and variants
>>>>>> thereof.
>>>>>>
>>>>>> So maybe the best approach is bailing out in the presence of clocks
>>>>>> and/or power domains after all, on the assumption that nothing today has
>>>>>> those properties, though I fear we may have problems with that later
>>>>>> down the line if/when people describe those for the root GIC to describe
>>>>>> those must be hogged, even if not explicitly managed.
>>>>>
>>>>> On further testing, by bailing out in the presence of clocks and/or
>>>>> power-domains, the problem I now see is that although the primary gic-400
>>>>> has been registered, we still try to probe it again later as it matches
>>>>> the platform driver. One way to avoid this would be ...
>>>>>
>>>>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>>>>> index e7bfc175b8e1..631da7ad0dbf 100644
>>>>> --- a/drivers/of/irq.c
>>>>> +++ b/drivers/of/irq.c
>>>>> @@ -556,6 +556,8 @@ void __init of_irq_init(const struct of_device_id *matches)
>>>>>                          * its children can get processed in a subsequent pass.
>>>>>                          */
>>>>>                         list_add_tail(&desc->list, &intc_parent_list);
>>>>> +
>>>>> +                       of_node_set_flag(desc->dev, OF_POPULATED);
>>>>>                 }
>>>>
>>>> That sounds like the right thing to do to me...
>>>
>>> Seems fine to me, but it would be a problem since this is a global
>>> decision if you wanted to have some hand-off from an "early driver" to
>>> a platform driver. I guess setting the flag could move to drivers that
>>> need it although I don't think drivers should be touching the flags.
>>
>> Isn't this the other way around? Setting this flag means that I have
>> been populated and so don't bother creating a platform device for this
>> device as it isn't needed. A by-product if this, is that if we did
>> happen to have a platform driver for the irqchip that also has an early
>> driver, then the hand-off would never happen if the early init was
>> successful.
>>
>> The driver would still have to decide whether to hand-off and to do that
>> it would need to return an error from the early driver [0].
>>
>>>>> If this is not appropriate then I guess I will just need to use
>>>>> "tegra210-agic" for the compatibility flag.
>>>>
>>>> As I want this for plain gic-400, I'd be unhappy ;-)
>>>
>>> IMO, the plain gic-400 should not have these dependencies and you
>>> should use SoC specific compatible strings should you need to deal
>>> with this problem.
>>
>> It is fine for my case, but it does mean I cannot say ...
>>
>>       compatible = "tegra210-agic", "gic-400";
>>
>> ... because this will always match the early driver (unless we do
>> something like I have suggested above). So I would have ...
>
> Sorry this is wrong. The above will always match the early driver.
>
> The problem with the above compatibility string is that, if the platform
> driver matches "gic-400" then it will try to probe all gic-400s even if
> they have been initialised early and this is definitely not what we
> want. This could be solved by setting the OF_POPULATED flag.

A platform driver for just gic-400 is wrong IMO until we have platform
drivers for all interrupt controllers.

Another reason to set OF_POPULATED flag is we are needlessly creating
platform devices for irq controllers that will never have platform
drivers. So I'd go with that approach.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter May 11, 2016, 4:53 p.m. UTC | #9
Hi Rob, Mark,

On 11/05/16 17:30, Rob Herring wrote:
> A platform driver for just gic-400 is wrong IMO until we have platform
> drivers for all interrupt controllers.

Yes, that is fine with me, but can we decide on whether the platform
driver should match "tegra210-agic" or the early driver should bail-out
if clocks/power-domains are present?

I am fine with either, but I think that Rob prefers the tegra210-agic
compat string and Mark prefers to bail-out of the early driver if
clocks/power-domains are present.

> Another reason to set OF_POPULATED flag is we are needlessly creating
> platform devices for irq controllers that will never have platform
> drivers. So I'd go with that approach.

Yes exactly, that was the point I was trying to make ;-)

Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland May 11, 2016, 5:28 p.m. UTC | #10
On Wed, May 11, 2016 at 05:53:43PM +0100, Jon Hunter wrote:
> Hi Rob, Mark,
> 
> On 11/05/16 17:30, Rob Herring wrote:
> > A platform driver for just gic-400 is wrong IMO until we have platform
> > drivers for all interrupt controllers.
> 
> Yes, that is fine with me, but can we decide on whether the platform
> driver should match "tegra210-agic" or the early driver should bail-out
> if clocks/power-domains are present?
> 
> I am fine with either, but I think that Rob prefers the tegra210-agic
> compat string and Mark prefers to bail-out of the early driver if
> clocks/power-domains are present.

If anything, I wasn't too keen on bailing out becuase of those
properties, as I mentioned for the case of a root interrupt controller.

I am happy to match the "tegra210-agic" string specifically.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter May 11, 2016, 7:49 p.m. UTC | #11
On 11/05/16 18:28, Mark Rutland wrote:
> On Wed, May 11, 2016 at 05:53:43PM +0100, Jon Hunter wrote:
>> Hi Rob, Mark,
>>
>> On 11/05/16 17:30, Rob Herring wrote:
>>> A platform driver for just gic-400 is wrong IMO until we have platform
>>> drivers for all interrupt controllers.
>>
>> Yes, that is fine with me, but can we decide on whether the platform
>> driver should match "tegra210-agic" or the early driver should bail-out
>> if clocks/power-domains are present?
>>
>> I am fine with either, but I think that Rob prefers the tegra210-agic
>> compat string and Mark prefers to bail-out of the early driver if
>> clocks/power-domains are present.
> 
> If anything, I wasn't too keen on bailing out becuase of those
> properties, as I mentioned for the case of a root interrupt controller.
> 
> I am happy to match the "tegra210-agic" string specifically.

Thanks guys. I will stick with tegra210-agic for now.

Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index e7bfc175b8e1..631da7ad0dbf 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -556,6 +556,8 @@  void __init of_irq_init(const struct of_device_id *matches)
                         * its children can get processed in a subsequent pass.
                         */
                        list_add_tail(&desc->list, &intc_parent_list);
+
+                       of_node_set_flag(desc->dev, OF_POPULATED);
                }

If this is not appropriate then I guess I will just need to use