| Message ID | 20230922080546.26442-1-zelong.dong@amlogic.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: amlogic: Fix IR Controller register area for Meson-GX/AXG/G12 series SoCs | expand |
Hi, Le 22/09/2023 à 10:05, zelong dong a écrit : > From: Zelong Dong <zelong.dong@amlogic.com> > > Amloic Meson-6 SoC only has one IR Controller, since then, there are 2 IR > Controllers in every SoCs, one is Legacy IR Controller (same as Meson-6's one), > the new one is Multi-Format IR Controller (abbreviated "MF-IR", > which supports more than one IR Protocol). MF-IR was updated several times, > so different SoCs could be got different register sizes. > > There are all IR Controller register areas from upstream's SoCs: > Legacy IR MF-IR > Meson-6 | <0xc8100480 0x20> | NULL | > Meson-8/8B/8M2 | <0xc8100480 0x20> | <0xc8100580 0x30> | > Meson-GXBB | <0xc8100480 0x20> | <0xc8100580 0x44> | > Meson-GXM/GXL | <0xc8100480 0x20> | <0xc8100580 0x54> | > Meson-AXG/G12A/G12B/SM1 | <0xff808000 0x20> | <0xff808040 0x58> | > > About Meson-IR driver, MF-IR was supported from Meson-8, which was distinguished > by compatible string (of_device_is_compatible(node, "amlogic,meson6-ir")), > and only one register (macro 'IR_DEC_REG2') was added because controller worked > in raw decoder mode, later registers are unused, so we recommend that register > size should be 0x24 if MF-IR is in use. > > There are 2 ways to fix. > > MF-IR is in use: > Meson-8/8B/8M2/GXBB/GXM/GXL: <0xc8100580 0x24> > Meson-AXG/G12A/G12B/SM1: <0xff808040 0x24> > > Legacy IR is in use: > Meson-8/8B/8M2/GXBB/GXM/GXL: <0xc8100480 0x20> > Meson-AXG/G12A/G12B/SM1: <0xff808000 0x20> This is slighly confusing, so you mean since Meson-8 there's 2 IR controllers which have the same register mapping, and both works with the "amlogic,meson6-ir" compatible ? So why should we switch to the MF-IR address ? what does it exactly change ? If we want to be clean: - both should be added, legacy IR with current address + reg size change - a new compatible should be added for the MF IR, with "amlogic,meson6-ir" as fallback to take in account they are compatible - a new node should be added for the MR IR - DTs should be switch to the MF IR label instead of the legacy IR With this we would be able to take advantage of the MR IF functionalities while keeping driver functionnal with old DTs and new DTs with old kernels. Thanks, Neil > > Fixes: 2bfe8412c5388a ("arm64: dts: meson-g12a: Add IR nodes") > Fixes: 7bd46a79aad549 ("ARM64: dts: meson-axg: enable IR controller") > Fixes: c58d77855f0069 ("ARM64: dts: meson-gxbb: Add Infrared Remote Controller decoder") > Link: https://lore.kernel.org/all/20160820095424.636-5-martin.blumenstingl@googlemail.com/ > Signed-off-by: Zelong Dong <zelong.dong@amlogic.com> > --- > arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 4 ++-- > arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 4 ++-- > arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi > index 768d0ed78dbe..dd8c58e74322 100644 > --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi > +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi > @@ -1705,9 +1705,9 @@ pwm_AO_ab: pwm@7000 { > status = "disabled"; > }; > > - ir: ir@8000 { > + ir: ir@8040 { > compatible = "amlogic,meson-gxbb-ir"; > - reg = <0x0 0x8000 0x0 0x20>; > + reg = <0x0 0x8040 0x0 0x24>; > interrupts = <GIC_SPI 196 IRQ_TYPE_EDGE_RISING>; > status = "disabled"; > }; > diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi > index ff68b911b729..e12cea5fa889 100644 > --- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi > +++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi > @@ -2084,9 +2084,9 @@ pwm_AO_ab: pwm@7000 { > status = "disabled"; > }; > > - ir: ir@8000 { > + ir: ir@8040 { > compatible = "amlogic,meson-gxbb-ir"; > - reg = <0x0 0x8000 0x0 0x20>; > + reg = <0x0 0x8040 0x0 0x24>; > interrupts = <GIC_SPI 196 IRQ_TYPE_EDGE_RISING>; > status = "disabled"; > }; > diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi > index 2673f0dbafe7..0c04e34a0923 100644 > --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi > +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi > @@ -506,7 +506,7 @@ pwm_AO_ab: pwm@550 { > > ir: ir@580 { > compatible = "amlogic,meson-gx-ir", "amlogic,meson-gxbb-ir"; > - reg = <0x0 0x00580 0x0 0x40>; > + reg = <0x0 0x00580 0x0 0x24>; > interrupts = <GIC_SPI 196 IRQ_TYPE_EDGE_RISING>; > status = "disabled"; > };
在 2023/9/29 12:00, Neil Armstrong 写道: > Hi, > > Le 22/09/2023 à 10:05, zelong dong a écrit : >> From: Zelong Dong <zelong.dong@amlogic.com> >> >> Amloic Meson-6 SoC only has one IR Controller, since then, there are 2 IR >> Controllers in every SoCs, one is Legacy IR Controller (same as >> Meson-6's one), >> the new one is Multi-Format IR Controller (abbreviated "MF-IR", >> which supports more than one IR Protocol). MF-IR was updated several >> times, >> so different SoCs could be got different register sizes. >> >> There are all IR Controller register areas from upstream's SoCs: >> Legacy IR MF-IR >> Meson-6 | <0xc8100480 0x20> | NULL | >> Meson-8/8B/8M2 | <0xc8100480 0x20> | <0xc8100580 0x30> | >> Meson-GXBB | <0xc8100480 0x20> | <0xc8100580 0x44> | >> Meson-GXM/GXL | <0xc8100480 0x20> | <0xc8100580 0x54> | >> Meson-AXG/G12A/G12B/SM1 | <0xff808000 0x20> | <0xff808040 0x58> | >> >> About Meson-IR driver, MF-IR was supported from Meson-8, which was >> distinguished >> by compatible string (of_device_is_compatible(node, >> "amlogic,meson6-ir")), >> and only one register (macro 'IR_DEC_REG2') was added because >> controller worked >> in raw decoder mode, later registers are unused, so we recommend that >> register >> size should be 0x24 if MF-IR is in use. >> >> There are 2 ways to fix. >> >> MF-IR is in use: >> Meson-8/8B/8M2/GXBB/GXM/GXL: <0xc8100580 0x24> >> Meson-AXG/G12A/G12B/SM1: <0xff808040 0x24> >> >> Legacy IR is in use: >> Meson-8/8B/8M2/GXBB/GXM/GXL: <0xc8100480 0x20> >> Meson-AXG/G12A/G12B/SM1: <0xff808000 0x20> > > This is slighly confusing, so you mean since Meson-8 there's 2 IR > controllers > which have the same register mapping, and both works with the > "amlogic,meson6-ir" compatible ? They are not the same register mapping but similar. Compatible "amlogic,meson6-ir" only works with legacy IR, other compatibles work with MF-IR. Please refer to https://lore.kernel.org/all/20160820095424.636-5-martin.blumenstingl@googlemail.com/ > > So why should we switch to the MF-IR address ? what does it exactly > change ? In raw decoder mode, it's not necessarily to switch to MF-IR, you could use "amlogic,meson6-ir" with legacy IR register area. This patch is for fix the wrong relation between compatible and register area. If compatible is "amlogic,meson-gxbb-ir", the register should be from MF-IR. > > If we want to be clean: > - both should be added, legacy IR with current address + reg size change > - a new compatible should be added for the MF IR, with > "amlogic,meson6-ir" as fallback to take in account they are compatible > - a new node should be added for the MR IR > - DTs should be switch to the MF IR label instead of the legacy IR > > With this we would be able to take advantage of the MR IF > functionalities while keeping driver functionnal > with old DTs and new DTs with old kernels. Legacy IR and MF-IR share the same IRQ number, so I recommend that chose one to use. Do they both need to be added? In a word, this patch is not for new function, just fix the wrong register addr. > > Thanks, > Neil > >> >> Fixes: 2bfe8412c5388a ("arm64: dts: meson-g12a: Add IR nodes") >> Fixes: 7bd46a79aad549 ("ARM64: dts: meson-axg: enable IR controller") >> Fixes: c58d77855f0069 ("ARM64: dts: meson-gxbb: Add Infrared Remote >> Controller decoder") >> Link: >> https://lore.kernel.org/all/20160820095424.636-5-martin.blumenstingl@googlemail.com/ >> Signed-off-by: Zelong Dong <zelong.dong@amlogic.com> >> --- >> arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 4 ++-- >> arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 4 ++-- >> arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 2 +- >> 3 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi >> b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi >> index 768d0ed78dbe..dd8c58e74322 100644 >> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi >> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi >> @@ -1705,9 +1705,9 @@ pwm_AO_ab: pwm@7000 { >> status = "disabled"; >> }; >> >> - ir: ir@8000 { >> + ir: ir@8040 { >> compatible = "amlogic,meson-gxbb-ir"; >> - reg = <0x0 0x8000 0x0 0x20>; >> + reg = <0x0 0x8040 0x0 0x24>; >> interrupts = <GIC_SPI 196 >> IRQ_TYPE_EDGE_RISING>; >> status = "disabled"; >> }; >> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi >> b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi >> index ff68b911b729..e12cea5fa889 100644 >> --- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi >> +++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi >> @@ -2084,9 +2084,9 @@ pwm_AO_ab: pwm@7000 { >> status = "disabled"; >> }; >> >> - ir: ir@8000 { >> + ir: ir@8040 { >> compatible = "amlogic,meson-gxbb-ir"; >> - reg = <0x0 0x8000 0x0 0x20>; >> + reg = <0x0 0x8040 0x0 0x24>; >> interrupts = <GIC_SPI 196 >> IRQ_TYPE_EDGE_RISING>; >> status = "disabled"; >> }; >> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi >> b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi >> index 2673f0dbafe7..0c04e34a0923 100644 >> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi >> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi >> @@ -506,7 +506,7 @@ pwm_AO_ab: pwm@550 { >> >> ir: ir@580 { >> compatible = "amlogic,meson-gx-ir", >> "amlogic,meson-gxbb-ir"; >> - reg = <0x0 0x00580 0x0 0x40>; >> + reg = <0x0 0x00580 0x0 0x24>; >> interrupts = <GIC_SPI 196 >> IRQ_TYPE_EDGE_RISING>; >> status = "disabled"; >> }; >
diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi index 768d0ed78dbe..dd8c58e74322 100644 --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi @@ -1705,9 +1705,9 @@ pwm_AO_ab: pwm@7000 { status = "disabled"; }; - ir: ir@8000 { + ir: ir@8040 { compatible = "amlogic,meson-gxbb-ir"; - reg = <0x0 0x8000 0x0 0x20>; + reg = <0x0 0x8040 0x0 0x24>; interrupts = <GIC_SPI 196 IRQ_TYPE_EDGE_RISING>; status = "disabled"; }; diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi index ff68b911b729..e12cea5fa889 100644 --- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi @@ -2084,9 +2084,9 @@ pwm_AO_ab: pwm@7000 { status = "disabled"; }; - ir: ir@8000 { + ir: ir@8040 { compatible = "amlogic,meson-gxbb-ir"; - reg = <0x0 0x8000 0x0 0x20>; + reg = <0x0 0x8040 0x0 0x24>; interrupts = <GIC_SPI 196 IRQ_TYPE_EDGE_RISING>; status = "disabled"; }; diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi index 2673f0dbafe7..0c04e34a0923 100644 --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi @@ -506,7 +506,7 @@ pwm_AO_ab: pwm@550 { ir: ir@580 { compatible = "amlogic,meson-gx-ir", "amlogic,meson-gxbb-ir"; - reg = <0x0 0x00580 0x0 0x40>; + reg = <0x0 0x00580 0x0 0x24>; interrupts = <GIC_SPI 196 IRQ_TYPE_EDGE_RISING>; status = "disabled"; };