mbox series

[v2,00/20] Initial support for Marvell MMP3 SoC

Message ID 20190822092643.593488-1-lkundrak@v3.sk
Headers show
Series Initial support for Marvell MMP3 SoC | expand

Message

Lubomir Rintel Aug. 22, 2019, 9:26 a.m. UTC
Hi, 

this is a second spin of a patch set that adds support for the Marvell
MMP3 processor. MMP3 is used in OLPC XO-4 laptops, Panasonic Toughpad
FZ-A1 tablet and Dell Wyse 3020 Tx0D thin clients. 

Compared to v1, there's a handful of fixes in response to reviews. Patch
02/20 is new. Details in individual patches.
 
Apart from the adjustments in mach-mmp/, the patch makes necessary 
changes to the irqchip driver and adds an USB2 PHY driver. The latter 
has a dependency on the mach-mmp/ changes, so it can't be submitted 
separately.
 
The patch set has been tested to work on Wyse Tx0D and not ruin MMP2 
support on XO-1.75. 

Thanks
Lubo

Comments

Marc Zyngier Aug. 22, 2019, 10:31 a.m. UTC | #1
On 22/08/2019 10:26, Lubomir Rintel wrote:
> Hi, 
> 
> this is a second spin of a patch set that adds support for the Marvell
> MMP3 processor. MMP3 is used in OLPC XO-4 laptops, Panasonic Toughpad
> FZ-A1 tablet and Dell Wyse 3020 Tx0D thin clients. 
> 
> Compared to v1, there's a handful of fixes in response to reviews. Patch
> 02/20 is new. Details in individual patches.
>  
> Apart from the adjustments in mach-mmp/, the patch makes necessary 
> changes to the irqchip driver and adds an USB2 PHY driver. The latter 
> has a dependency on the mach-mmp/ changes, so it can't be submitted 
> separately.
>  
> The patch set has been tested to work on Wyse Tx0D and not ruin MMP2 
> support on XO-1.75. 

How do you want this series to be merged? I'm happy to take the irqchip
related patches as well as the corresponding DT change (once reviewed)
through my tree.

Thanks,

	M.
Olof Johansson Aug. 22, 2019, 4:23 p.m. UTC | #2
On Thu, Aug 22, 2019 at 3:32 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On 22/08/2019 10:26, Lubomir Rintel wrote:
> > Hi,
> >
> > this is a second spin of a patch set that adds support for the Marvell
> > MMP3 processor. MMP3 is used in OLPC XO-4 laptops, Panasonic Toughpad
> > FZ-A1 tablet and Dell Wyse 3020 Tx0D thin clients.
> >
> > Compared to v1, there's a handful of fixes in response to reviews. Patch
> > 02/20 is new. Details in individual patches.
> >
> > Apart from the adjustments in mach-mmp/, the patch makes necessary
> > changes to the irqchip driver and adds an USB2 PHY driver. The latter
> > has a dependency on the mach-mmp/ changes, so it can't be submitted
> > separately.
> >
> > The patch set has been tested to work on Wyse Tx0D and not ruin MMP2
> > support on XO-1.75.
>
> How do you want this series to be merged? I'm happy to take the irqchip
> related patches as well as the corresponding DT change (once reviewed)
> through my tree.

DT changes, unless there's lack of backwards compatibility, are best
merged through the platform trees. Especially for new platforms like
these where there's likely going to be nearby changes (and thus
conflicts).

I.e. driver changes I'm all for bringing through driver trees
(including binding patches), but please leave dts/dtsi changes to the
platform.


-Olof
Florian Fainelli Aug. 22, 2019, 4:36 p.m. UTC | #3
On 8/22/19 2:26 AM, Lubomir Rintel wrote:
> Used to bring up the second core on MMP3.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> 
> ---
> Changes since v1:
> - Wrap SW_BRANCH_VIRT_ADDR with __pa_symbol()
> 
>  arch/arm/mach-mmp/Makefile  |  3 +++
>  arch/arm/mach-mmp/platsmp.c | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
>  create mode 100644 arch/arm/mach-mmp/platsmp.c
> 
> diff --git a/arch/arm/mach-mmp/Makefile b/arch/arm/mach-mmp/Makefile
> index 322c1c97dc900..7b3a7f979eece 100644
> --- a/arch/arm/mach-mmp/Makefile
> +++ b/arch/arm/mach-mmp/Makefile
> @@ -22,6 +22,9 @@ ifeq ($(CONFIG_PM),y)
>  obj-$(CONFIG_CPU_PXA910)	+= pm-pxa910.o
>  obj-$(CONFIG_CPU_MMP2)		+= pm-mmp2.o
>  endif
> +ifeq ($(CONFIG_SMP),y)
> +obj-$(CONFIG_MACH_MMP3_DT)	+= platsmp.o
> +endif
>  
>  # board support
>  obj-$(CONFIG_MACH_ASPENITE)	+= aspenite.o
> diff --git a/arch/arm/mach-mmp/platsmp.c b/arch/arm/mach-mmp/platsmp.c
> new file mode 100644
> index 0000000000000..98d5ef23623cb
> --- /dev/null
> +++ b/arch/arm/mach-mmp/platsmp.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2019 Lubomir Rintel <lkundrak@v3.sk>
> + */
> +#include <linux/io.h>
> +#include <asm/smp_scu.h>
> +#include <asm/smp.h>
> +#include "addr-map.h"
> +
> +#define SW_BRANCH_VIRT_ADDR	CIU_REG(0x24)
> +
> +static int mmp3_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> +	/*
> +	 * Apparently, the boot ROM on the second core spins on this
> +	 * register becoming non-zero and then jumps to the address written
> +	 * there. No IPIs involved.
> +	 */
> +	__raw_writel(virt_to_phys(secondary_startup),
> +			__pa_symbol(SW_BRANCH_VIRT_ADDR));


That looks wrong, the __pa_symbol() is applicable to secondary_startup,
while SW_BRANCH_VIRT_ADDR does not need that.

> +	return 0;
> +}
> +
> +static void mmp3_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +	scu_enable(SCU_VIRT_BASE);
> +}
> +
> +static const struct smp_operations mmp3_smp_ops __initconst = {
> +	.smp_prepare_cpus	= mmp3_smp_prepare_cpus,
> +	.smp_boot_secondary	= mmp3_boot_secondary,
> +};
> +CPU_METHOD_OF_DECLARE(mmp3_smp, "marvell,mmp3-smp", &mmp3_smp_ops);
>
Lubomir Rintel Aug. 23, 2019, 7:13 a.m. UTC | #4
On Thu, 2019-08-22 at 09:36 -0700, Florian Fainelli wrote:
> On 8/22/19 2:26 AM, Lubomir Rintel wrote:
> > Used to bring up the second core on MMP3.
> > 
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > 
> > ---
> > Changes since v1:
> > - Wrap SW_BRANCH_VIRT_ADDR with __pa_symbol()
> > 
> >  arch/arm/mach-mmp/Makefile  |  3 +++
> >  arch/arm/mach-mmp/platsmp.c | 33 +++++++++++++++++++++++++++++++++
> >  2 files changed, 36 insertions(+)
> >  create mode 100644 arch/arm/mach-mmp/platsmp.c
> > 
> > diff --git a/arch/arm/mach-mmp/Makefile b/arch/arm/mach-mmp/Makefile
> > index 322c1c97dc900..7b3a7f979eece 100644
> > --- a/arch/arm/mach-mmp/Makefile
> > +++ b/arch/arm/mach-mmp/Makefile
> > @@ -22,6 +22,9 @@ ifeq ($(CONFIG_PM),y)
> >  obj-$(CONFIG_CPU_PXA910)	+= pm-pxa910.o
> >  obj-$(CONFIG_CPU_MMP2)		+= pm-mmp2.o
> >  endif
> > +ifeq ($(CONFIG_SMP),y)
> > +obj-$(CONFIG_MACH_MMP3_DT)	+= platsmp.o
> > +endif
> >  
> >  # board support
> >  obj-$(CONFIG_MACH_ASPENITE)	+= aspenite.o
> > diff --git a/arch/arm/mach-mmp/platsmp.c b/arch/arm/mach-mmp/platsmp.c
> > new file mode 100644
> > index 0000000000000..98d5ef23623cb
> > --- /dev/null
> > +++ b/arch/arm/mach-mmp/platsmp.c
> > @@ -0,0 +1,33 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) 2019 Lubomir Rintel <lkundrak@v3.sk>
> > + */
> > +#include <linux/io.h>
> > +#include <asm/smp_scu.h>
> > +#include <asm/smp.h>
> > +#include "addr-map.h"
> > +
> > +#define SW_BRANCH_VIRT_ADDR	CIU_REG(0x24)
> > +
> > +static int mmp3_boot_secondary(unsigned int cpu, struct task_struct *idle)
> > +{
> > +	/*
> > +	 * Apparently, the boot ROM on the second core spins on this
> > +	 * register becoming non-zero and then jumps to the address written
> > +	 * there. No IPIs involved.
> > +	 */
> > +	__raw_writel(virt_to_phys(secondary_startup),
> > +			__pa_symbol(SW_BRANCH_VIRT_ADDR));
> 
> That looks wrong, the __pa_symbol() is applicable to secondary_startup,
> while SW_BRANCH_VIRT_ADDR does not need that.

Whoops, sorry for that. Will fix in the next patch version in a few
days.

Thanks
Lubo
Lubomir Rintel Aug. 23, 2019, 7:21 a.m. UTC | #5
On Thu, 2019-08-22 at 11:31 +0100, Marc Zyngier wrote:
> On 22/08/2019 10:26, Lubomir Rintel wrote:
> > Hi, 
> > 
> > this is a second spin of a patch set that adds support for the Marvell
> > MMP3 processor. MMP3 is used in OLPC XO-4 laptops, Panasonic Toughpad
> > FZ-A1 tablet and Dell Wyse 3020 Tx0D thin clients. 
> > 
> > Compared to v1, there's a handful of fixes in response to reviews. Patch
> > 02/20 is new. Details in individual patches.
> >  
> > Apart from the adjustments in mach-mmp/, the patch makes necessary 
> > changes to the irqchip driver and adds an USB2 PHY driver. The latter 
> > has a dependency on the mach-mmp/ changes, so it can't be submitted 
> > separately.
> >  
> > The patch set has been tested to work on Wyse Tx0D and not ruin MMP2 
> > support on XO-1.75. 
> 
> How do you want this series to be merged? I'm happy to take the irqchip
> related patches as well as the corresponding DT change (once reviewed)
> through my tree.

I was hoping for the Arm SoC tree, because there are some dependencies
(MMP3 USB PHY depends on MMP3 SoC).

That said, the irqchip patches are rather independent and the only
downside of them going in via a different tree will be that the other
tree that will lack them won't boot on MMP3 (things will compile
though). I don't know if that's okay. What's typically done in cases
like these?


> Thanks,
> 
> 	M.

Thank you
Lubo
Marc Zyngier Aug. 23, 2019, 9:42 a.m. UTC | #6
On 23/08/2019 08:21, Lubomir Rintel wrote:
> On Thu, 2019-08-22 at 11:31 +0100, Marc Zyngier wrote:
>> On 22/08/2019 10:26, Lubomir Rintel wrote:
>>> Hi, 
>>>
>>> this is a second spin of a patch set that adds support for the Marvell
>>> MMP3 processor. MMP3 is used in OLPC XO-4 laptops, Panasonic Toughpad
>>> FZ-A1 tablet and Dell Wyse 3020 Tx0D thin clients. 
>>>
>>> Compared to v1, there's a handful of fixes in response to reviews. Patch
>>> 02/20 is new. Details in individual patches.
>>>  
>>> Apart from the adjustments in mach-mmp/, the patch makes necessary 
>>> changes to the irqchip driver and adds an USB2 PHY driver. The latter 
>>> has a dependency on the mach-mmp/ changes, so it can't be submitted 
>>> separately.
>>>  
>>> The patch set has been tested to work on Wyse Tx0D and not ruin MMP2 
>>> support on XO-1.75. 
>>
>> How do you want this series to be merged? I'm happy to take the irqchip
>> related patches as well as the corresponding DT change (once reviewed)
>> through my tree.
> 
> I was hoping for the Arm SoC tree, because there are some dependencies
> (MMP3 USB PHY depends on MMP3 SoC).
> 
> That said, the irqchip patches are rather independent and the only
> downside of them going in via a different tree will be that the other
> tree that will lack them won't boot on MMP3 (things will compile
> though). I don't know if that's okay. What's typically done in cases
> like these?

I usually take the irqchip patches that can be built standalone (without
dependency on header files, for example). If you want them to go via
another tree, stick my

	Acked-by: Marc Zyngier <maz@kernel.org>

on patches #6 through #9.

Thanks,

	M.
Lubomir Rintel Aug. 26, 2019, 11:59 a.m. UTC | #7
On Fri, 2019-08-23 at 10:42 +0100, Marc Zyngier wrote:
> On 23/08/2019 08:21, Lubomir Rintel wrote:
> > On Thu, 2019-08-22 at 11:31 +0100, Marc Zyngier wrote:
> > > On 22/08/2019 10:26, Lubomir Rintel wrote:
> > > > Hi, 
> > > > 
> > > > this is a second spin of a patch set that adds support for the Marvell
> > > > MMP3 processor. MMP3 is used in OLPC XO-4 laptops, Panasonic Toughpad
> > > > FZ-A1 tablet and Dell Wyse 3020 Tx0D thin clients. 
> > > > 
> > > > Compared to v1, there's a handful of fixes in response to reviews. Patch
> > > > 02/20 is new. Details in individual patches.
> > > >  
> > > > Apart from the adjustments in mach-mmp/, the patch makes necessary 
> > > > changes to the irqchip driver and adds an USB2 PHY driver. The latter 
> > > > has a dependency on the mach-mmp/ changes, so it can't be submitted 
> > > > separately.
> > > >  
> > > > The patch set has been tested to work on Wyse Tx0D and not ruin MMP2 
> > > > support on XO-1.75. 
> > > 
> > > How do you want this series to be merged? I'm happy to take the irqchip
> > > related patches as well as the corresponding DT change (once reviewed)
> > > through my tree.
> > 
> > I was hoping for the Arm SoC tree, because there are some dependencies
> > (MMP3 USB PHY depends on MMP3 SoC).
> > 
> > That said, the irqchip patches are rather independent and the only
> > downside of them going in via a different tree will be that the other
> > tree that will lack them won't boot on MMP3 (things will compile
> > though). I don't know if that's okay. What's typically done in cases
> > like these?
> 
> I usually take the irqchip patches that can be built standalone (without
> dependency on header files, for example). If you want them to go via
> another tree, stick my
> 
> 	Acked-by: Marc Zyngier <maz@kernel.org>
> 
> on patches #6 through #9.

Actually, please go ahead and pick the irqchip patches into your tree.

The rest of the patch set may need a couple more spins, and it will be
nice if it gets shorter.

Thank you
Lubo
Marc Zyngier Aug. 30, 2019, 2:26 p.m. UTC | #8
On 26/08/2019 12:59, Lubomir Rintel wrote:
> On Fri, 2019-08-23 at 10:42 +0100, Marc Zyngier wrote:
>> On 23/08/2019 08:21, Lubomir Rintel wrote:
>>> On Thu, 2019-08-22 at 11:31 +0100, Marc Zyngier wrote:
>>>> On 22/08/2019 10:26, Lubomir Rintel wrote:
>>>>> Hi, 
>>>>>
>>>>> this is a second spin of a patch set that adds support for the Marvell
>>>>> MMP3 processor. MMP3 is used in OLPC XO-4 laptops, Panasonic Toughpad
>>>>> FZ-A1 tablet and Dell Wyse 3020 Tx0D thin clients. 
>>>>>
>>>>> Compared to v1, there's a handful of fixes in response to reviews. Patch
>>>>> 02/20 is new. Details in individual patches.
>>>>>  
>>>>> Apart from the adjustments in mach-mmp/, the patch makes necessary 
>>>>> changes to the irqchip driver and adds an USB2 PHY driver. The latter 
>>>>> has a dependency on the mach-mmp/ changes, so it can't be submitted 
>>>>> separately.
>>>>>  
>>>>> The patch set has been tested to work on Wyse Tx0D and not ruin MMP2 
>>>>> support on XO-1.75. 
>>>>
>>>> How do you want this series to be merged? I'm happy to take the irqchip
>>>> related patches as well as the corresponding DT change (once reviewed)
>>>> through my tree.
>>>
>>> I was hoping for the Arm SoC tree, because there are some dependencies
>>> (MMP3 USB PHY depends on MMP3 SoC).
>>>
>>> That said, the irqchip patches are rather independent and the only
>>> downside of them going in via a different tree will be that the other
>>> tree that will lack them won't boot on MMP3 (things will compile
>>> though). I don't know if that's okay. What's typically done in cases
>>> like these?
>>
>> I usually take the irqchip patches that can be built standalone (without
>> dependency on header files, for example). If you want them to go via
>> another tree, stick my
>>
>> 	Acked-by: Marc Zyngier <maz@kernel.org>
>>
>> on patches #6 through #9.
> 
> Actually, please go ahead and pick the irqchip patches into your tree.
> 
> The rest of the patch set may need a couple more spins, and it will be
> nice if it gets shorter.

Applied to irqchip-next.

	M.
Rob Herring March 9, 2020, 4:25 p.m. UTC | #9
On Thu, Aug 22, 2019 at 4:34 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
>
> The "regs" property of the "mrvl,mmp2-mux-intc" devices are silly. They
> are offsets from intc's base, not addresses on the parent bus. At this
> point it probably can't be fixed.

Another option is for platform code to fixup the live DT and just add
'ranges' to make this work.

> On an OLPC XO-1.75 machine, the muxes are children of the intc, not the
> axi bus, and thus of_address_to_resource() won't work. We should treat
> the values as mere integers as opposed to bus addresses.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> Acked-by: Pavel Machek <pavel@ucw.cz>
>
> ---
> Changes since v4 of "MMP platform fixes" set:
> - Add a comment, as suggested by Pavel
>
> Changes since v1:
> - Fix up typoes in the comment
> - Do not allow the regs property be larger than the bindings document.
>
>  drivers/irqchip/irq-mmp.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/irqchip/irq-mmp.c b/drivers/irqchip/irq-mmp.c
> index 14618dc0bd396..e41e47ab71d3b 100644
> --- a/drivers/irqchip/irq-mmp.c
> +++ b/drivers/irqchip/irq-mmp.c
> @@ -424,9 +424,9 @@ IRQCHIP_DECLARE(mmp2_intc, "mrvl,mmp2-intc", mmp2_of_init);
>  static int __init mmp2_mux_of_init(struct device_node *node,
>                                    struct device_node *parent)
>  {
> -       struct resource res;
>         int i, ret, irq, j = 0;
>         u32 nr_irqs, mfp_irq;
> +       u32 reg[4];
>
>         if (!parent)
>                 return -ENODEV;
> @@ -438,18 +438,22 @@ static int __init mmp2_mux_of_init(struct device_node *node,
>                 pr_err("Not found mrvl,intc-nr-irqs property\n");
>                 return -EINVAL;
>         }
> -       ret = of_address_to_resource(node, 0, &res);
> +
> +       /*
> +        * For historical reasons, the "regs" property of the
> +        * mrvl,mmp2-mux-intc is not a regular "regs" property containing
> +        * addresses on the parent bus, but offsets from the intc's base.
> +        * That is why we can't use of_address_to_resource() here.
> +        */
> +       ret = of_property_read_variable_u32_array(node, "reg", reg,
> +                                                 ARRAY_SIZE(reg),
> +                                                 ARRAY_SIZE(reg));
>         if (ret < 0) {
>                 pr_err("Not found reg property\n");
>                 return -EINVAL;
>         }
> -       icu_data[i].reg_status = mmp_icu_base + res.start;

Seems like it was treated as an offset already?

> -       ret = of_address_to_resource(node, 1, &res);
> -       if (ret < 0) {
> -               pr_err("Not found reg property\n");
> -               return -EINVAL;
> -       }
> -       icu_data[i].reg_mask = mmp_icu_base + res.start;
> +       icu_data[i].reg_status = mmp_icu_base + reg[0];
> +       icu_data[i].reg_mask = mmp_icu_base + reg[2];

This is a bit fragile as it's hard coding the cell sizes. Are they the
same for all the platforms? I'd be more comfortable doing that in
platform specific fixup code than here.

Rob
Rob Herring March 9, 2020, 4:28 p.m. UTC | #10
On Mon, Mar 9, 2020 at 11:25 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Thu, Aug 22, 2019 at 4:34 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
> >
> > The "regs" property of the "mrvl,mmp2-mux-intc" devices are silly. They
> > are offsets from intc's base, not addresses on the parent bus. At this
> > point it probably can't be fixed.
>
> Another option is for platform code to fixup the live DT and just add
> 'ranges' to make this work.

Nevermind my reply on this old thread. It caught my attention when
looking for the other thread and I missed the date.

Rob