Re: [PATCH 3/6] ARM: integrator: delete non-devicetree boot path

Re: [PATCH 3/6] ARM: integrator: delete non-devicetree boot path

[Mark Rutland]

[Adding devicetree to Cc]

On Thu, Oct 03, 2013 at 05:03:22PM +0100, Linus Walleij wrote:
> The Device Tree boot path now supports everything the ATAG
> boot can provide, and the two are equivalent. This deletes
> the ATAG boot path from the Integrator/AP and
> Integrator/CP platforms to move them on to the future.
> 
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm/Kconfig                         |   1 +
>  arch/arm/mach-integrator/core.c          |  57 ----------
>  arch/arm/mach-integrator/integrator_ap.c | 135 ------------------------
>  arch/arm/mach-integrator/integrator_cp.c | 173 -------------------------------
>  arch/arm/mach-integrator/pci_v3.c        | 122 +++++-----------------
>  5 files changed, 25 insertions(+), 463 deletions(-)
> 

[...]

> +static int __init pci_v3_probe(struct platform_device *pdev)
>  {
> +       struct device_node *np = pdev->dev.of_node;
>         struct of_pci_range_parser parser;
>         struct of_pci_range range;
>         struct resource *res;
>         int irq, ret;
> 
> +       /* Remap the Integrator system controller */
> +       ap_syscon_base = ioremap(INTEGRATOR_SC_BASE, 0x100);

It seems a shame that this didn't get described in the DT. This hasn't
been fully moved over...

> +       if (!ap_syscon_base) {
> +               dev_err(&pdev->dev, "unable to remap the AP syscon for PCIv3\n");
> +               return -ENODEV;
> +       }
> +
> +       /* Device tree probe path */
> +       if (!np) {
> +               dev_err(&pdev->dev, "no device tree node for PCIv3\n");
> +               return -ENODEV;
> +       }

We didn't unmap the syscon registers, yet we mapped them here. Either
we've got a useless mapping, or we're mapping it from the wrong block of
code...

Cheers,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] ARM: integrator: delete non-devicetree boot path

[Linus Walleij]

On Tue, Oct 8, 2013 at 11:51 AM, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:

>> +       /* Remap the Integrator system controller */
>> +       ap_syscon_base = ioremap(INTEGRATOR_SC_BASE, 0x100);
>
> It seems a shame that this didn't get described in the DT. This hasn't
> been fully moved over...

No. There is still stuff to move over, and not only the system controller,
also the EBI (external bus interface) on the AP.

But we can still work from this point, or do you mean that we cannot
proceed to delete the ATAG boot path before we fix this?

My idea is that deleting this makes the code easier to handle after
the shrink.

>> +       if (!ap_syscon_base) {
>> +               dev_err(&pdev->dev, "unable to remap the AP syscon for PCIv3\n");
>> +               return -ENODEV;
>> +       }
>> +
>> +       /* Device tree probe path */
>> +       if (!np) {
>> +               dev_err(&pdev->dev, "no device tree node for PCIv3\n");
>> +               return -ENODEV;
>> +       }
>
> We didn't unmap the syscon registers, yet we mapped them here. Either
> we've got a useless mapping, or we're mapping it from the wrong block of
> code...

Sorry I'm not following this. The only thing the patch does is remove the
ATAG boot path, moving the relevant part of the DT boot path over
to the DT probe function and renaming it pci_v3_probe() in the process
which might make the patch messy but that is all it does.

If you mean that we should unmap the syscon base you're right
but that would be a separate patch, right?

I'll write a separate patch for that instead using devm_* so we
don't have to unmap it by calling any special function.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html