Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1395691 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for  Tegra210 AGIC Jon Hunter <jonathanh@nvidia.com> - 2016-05-06 10:40 +0200
    Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for  Tegra210 AGIC Geert Uytterhoeven <geert@linux-m68k.org> - 2016-05-07 16:20 +0200
      Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for  Tegra210 AGIC Jon Hunter <jonathanh@nvidia.com> - 2016-05-08 14:30 +0200
        Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for  Tegra210 AGIC Marc Zyngier <marc.zyngier@arm.com> - 2016-05-09 11:40 +0200
      Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for  Tegra210 AGIC Rob Herring <robh+dt@kernel.org> - 2016-05-11 18:00 +0200
        Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for  Tegra210 AGIC Jon Hunter <jonathanh@nvidia.com> - 2016-05-11 18:10 +0200
          Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for  Tegra210 AGIC Jon Hunter <jonathanh@nvidia.com> - 2016-05-11 18:20 +0200
            Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for  Tegra210 AGIC Rob Herring <robh+dt@kernel.org> - 2016-05-11 18:40 +0200
              Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for  Tegra210 AGIC Jon Hunter <jonathanh@nvidia.com> - 2016-05-11 19:00 +0200
                Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for  Tegra210 AGIC Mark Rutland <mark.rutland@arm.com> - 2016-05-11 19:30 +0200
                  Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for  Tegra210 AGIC Jon Hunter <jonathanh@nvidia.com> - 2016-05-11 21:50 +0200
          Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for  Tegra210 AGIC Jon Hunter <jonathanh@nvidia.com> - 2016-05-11 18:20 +0200

#1395691 — Re: [PATCH V2 13/14] dt-bindings: arm-gic: Add documentation for Tegra210 AGIC

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 ...

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
"tegra210-agic" for the compatibility flag.

Cheers
Jon

[toc] | [next] | [standalone]

#1396310

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

[toc] | [prev] | [next] | [standalone]

#1396422

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

[toc] | [prev] | [next] | [standalone]

#1396879

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.
-- 
Jazz is not dead. It just smells funny...

[toc] | [prev] | [next] | [standalone]

#1399294

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

[toc] | [prev] | [next] | [standalone]

#1399305

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

[toc] | [prev] | [next] | [standalone]

#1399314

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

[toc] | [prev] | [next] | [standalone]

#1399341

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

[toc] | [prev] | [next] | [standalone]

#1399366

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

[toc] | [prev] | [next] | [standalone]

#1399378

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.

[toc] | [prev] | [next] | [standalone]

#1399456

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

[toc] | [prev] | [next] | [standalone]

#1399318

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web