Re: [EXT] [PATCH 2/2] arm64: dts: ls1028a: add "fsl,vf610-edma" compatible

[Michael Walle]

Hi Peng,

Am 2020-03-07 11:32, schrieb Peng Ma:
> > -----Original Message-----
> > From: Michael Walle <michael@xxxxxxxx>
> > Sent: 2020å3æ7æ 17:26
> > To: Peng Ma <peng.ma@xxxxxxx>
> > Cc: dmaengine@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> > linux-kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; 
> > Vinod Koul
> > <vkoul@xxxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; Mark Rutland
> > <mark.rutland@xxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; Leo Li
> > <leoyang.li@xxxxxxx>
> > Subject: Re: [EXT] [PATCH 2/2] arm64: dts: ls1028a: add 
> > "fsl,vf610-edma"
> > compatible
> >
> > Caution: EXT Email
> >
> > Hi Peng,
> >
> > Am 2020-03-07 03:09, schrieb Peng Ma:
> > > > -----Original Message-----
> > > > From: Michael Walle <michael@xxxxxxxx>
> > > > Sent: 2020å3æ7æ 4:54
> > > > To: dmaengine@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> > > > linux-kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> > > > Cc: Vinod Koul <vkoul@xxxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>;
> > > > Mark Rutland <mark.rutland@xxxxxxx>; Shawn Guo
> > <shawnguo@xxxxxxxxxx>;
> > > > Leo Li <leoyang.li@xxxxxxx>; Peng Ma <peng.ma@xxxxxxx>; Michael 
> > > > Walle
> > > > <michael@xxxxxxxx>
> > > > Subject: [EXT] [PATCH 2/2] arm64: dts: ls1028a: add "fsl,vf610-edma"
> > > > compatible
> > > >
> > > > Caution: EXT Email
> > > >
> > > > The bootloader does the IOMMU fixup and dynamically adds the 
> > > > "iommus"
> > > > property to devices according to its compatible string. In case of
> > > > the eDMA controller this property is missing. Add it. After that the
> > > > IOMMU will work with the eDMA core.
> > > >
> > > > Signed-off-by: Michael Walle <michael@xxxxxxxx>
> > > > ---
> > > > arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> > > > b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> > > > index b152fa90cf5c..aa467bff2209 100644
> > > > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> > > > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> > > > @@ -447,7 +447,7 @@
> > > >
> > > >                edma0: dma-controller@22c0000 {
> > > >                        #dma-cells = <2>;
> > > > -                       compatible = "fsl,ls1028a-edma";
> > > > +                       compatible = "fsl,ls1028a-edma",
> > > > + "fsl,vf610-edma";
> > > Hi Michael,
> > >
> > > You should change it on bootloader instead of kernel, Some Reg of
> > > LS1028a is different from others, So we used compatible
> > > "fsl,ls1028a-edm" to distinguish "
> > > fsl,vf610-edma".
> >
> > Yes this might be the right thing to do. So since it is NXPs 
> > bootloader feel free to
> > fix that ;) Looking at the u-boot code right now, I don't even know it 
> > that is the
> > right fix at all. The fixup code in u-boot is SoC independent (its in 
> > fsl_icid.h and is
> > enabled with CONFIG_LSCH3, ie your chassis version). For example, the 
> > sdhc
> > fixup will scan the nodes for "compatible = fsl,esdhc", which is also 
> > the
> > secondary compatible for the "ls1028a-esdhc" compatible.
> >
> > And here is another reason to have it this way: we need backwards 
> > compatibility,
> > the are already boards out there whose bootloader will fix-up the 
> > "old" node.
> > Thus I don't see any other possibilty.
> [Peng Ma] OK, There is non fixed on uboot.
> I will fix it on uboot, if you want to use now, please change the
> uboot as below:

As I told you, I cannot be changed for shipped bootloaders. While it can 
be
changed for newer ones, I would really like to retain backwards 
compatibility.
And so should you.

That being said, I don't see a problem in having both compatibles. Linux
will use the ls1028a-emda one and u-boot will fix up the "older"
vf610-edma one.

Unfortunately, this patch will not only affect eDMA but all other 
drivers
which uses eDMA, eg. sound, lpuart, i2c and maybe DSPI.

-michael

> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ls1028_ids.c
> b/arch/arm/cpu/armv8/fsl-layerscape/ls1028_ids.c
> index d9d125e8ba..db9dd69548 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/ls1028_ids.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/ls1028_ids.c
> @@ -14,7 +14,7 @@ struct icid_id_table icid_tbl[] = {
>         SET_SDHC_ICID(1, FSL_SDMMC_STREAM_ID),
>         SET_SDHC_ICID(2, FSL_SDMMC2_STREAM_ID),
>         SET_SATA_ICID(1, "fsl,ls1028a-ahci", FSL_SATA1_STREAM_ID),
> -       SET_EDMA_ICID(FSL_EDMA_STREAM_ID),
> +       SET_EDMA_ICID_LS1028(FSL_EDMA_STREAM_ID),
>         SET_QDMA_ICID("fsl,ls1028a-qdma", FSL_DMA_STREAM_ID),
>         SET_GPU_ICID("fsl,ls1028a-gpu", FSL_GPU_STREAM_ID),
>         SET_DISPLAY_ICID(FSL_DISPLAY_STREAM_ID),
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
> b/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
> index 37e2fe4e66..15d0b60dbe 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
> @@ -144,6 +144,10 @@ extern int fman_icid_tbl_sz;
>         SET_GUR_ICID("fsl,vf610-edma", streamid, spare3_amqr,\
>                 EDMA_BASE_ADDR)
>
> +#define SET_EDMA_ICID_LS1028(streamid) \
> +       SET_GUR_ICID("fsl,ls1028a-edma", streamid, spare3_amqr,\
> +               EDMA_BASE_ADDR)
> +
>  #define SET_GPU_ICID(compat, streamid) \
>         SET_GUR_ICID(compat, streamid, misc1_amqr,\
>                 GPU_BASE_ADDR)
>
> BR,
> Peng
> > -michael
> >
> > > Thanks,
> > > Peng
> > > >                        reg = <0x0 0x22c0000 0x0 0x10000>,
> > > >                              <0x0 0x22d0000 0x0 0x10000>,
> > > >                              <0x0 0x22e0000 0x0 0x10000>;
> > > > --
> > > > 2.20.1