* [PATCH v2 1/2] media: rc: meson-ir: fix enabling raw/soft-decoding mode
2016-06-26 21:06 [v2] media: rc: fix Meson IR decoder Martin Blumenstingl
@ 2016-06-26 21:06 ` Martin Blumenstingl
2016-06-26 21:06 ` [PATCH v2 2/2] ARM: dts: meson: fixed size of the meson-ir registers Martin Blumenstingl
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Martin Blumenstingl @ 2016-06-26 21:06 UTC (permalink / raw)
To: linux-arm-kernel
According to the datasheet of Meson8b (S805) and GXBB (S905) the
decoding mode is configured in AO_MF_IR_DEC_REG2 (offset 0x20) using
bits 0-3.
The "duration" field may not be set correctly when any of the hardware
decoding modes. This can happen while a "default" decoding mode
(either set by the bootloader or the chip's default, which uses NEC as
it's default) is used.
While here, I also added defines for the protocols which can be decoded
by the hardware (more work is needed to be actually able to use them
though).
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
changes in v1 -> v2:
- fixed subject of the patch to include meson-ir
- fixed double-shifting of the decoder mode values
drivers/media/rc/meson-ir.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index fcc3b82..622a4160 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -32,13 +32,10 @@
#define IR_DEC_FRAME 0x14
#define IR_DEC_STATUS 0x18
#define IR_DEC_REG1 0x1c
+#define IR_DEC_REG2 0x20
#define REG0_RATE_MASK (BIT(11) - 1)
-#define REG1_MODE_MASK (BIT(7) | BIT(8))
-#define REG1_MODE_NEC (0 << 7)
-#define REG1_MODE_GENERAL (2 << 7)
-
#define REG1_TIME_IV_SHIFT 16
#define REG1_TIME_IV_MASK ((BIT(13) - 1) << REG1_TIME_IV_SHIFT)
@@ -51,6 +48,20 @@
#define REG1_RESET BIT(0)
#define REG1_ENABLE BIT(15)
+#define REG2_DEC_MODE_SHIFT 0
+#define REG2_DEC_MODE_MASK GENMASK(3, REG2_DEC_MODE_SHIFT)
+#define REG2_DEC_MODE_NEC 0x0
+#define REG2_DEC_MODE_RAW 0x2
+#define REG2_DEC_MODE_THOMSON 0x4
+#define REG2_DEC_MODE_TOSHIBA 0x5
+#define REG2_DEC_MODE_SONY 0x6
+#define REG2_DEC_MODE_RC5 0x7
+#define REG2_DEC_MODE_RC6 0x9
+#define REG2_DEC_MODE_RCMM 0xa
+#define REG2_DEC_MODE_DUOKAN 0xb
+#define REG2_DEC_MODE_COMCAST 0xe
+#define REG2_DEC_MODE_SANYO 0xf
+
#define STATUS_IR_DEC_IN BIT(8)
#define MESON_TRATE 10 /* us */
@@ -158,8 +169,9 @@ static int meson_ir_probe(struct platform_device *pdev)
/* Reset the decoder */
meson_ir_set_mask(ir, IR_DEC_REG1, REG1_RESET, REG1_RESET);
meson_ir_set_mask(ir, IR_DEC_REG1, REG1_RESET, 0);
- /* Set general operation mode */
- meson_ir_set_mask(ir, IR_DEC_REG1, REG1_MODE_MASK, REG1_MODE_GENERAL);
+ /* Enable raw/soft-decode mode */
+ meson_ir_set_mask(ir, IR_DEC_REG2, REG2_DEC_MODE_MASK,
+ REG2_DEC_MODE_RAW << REG2_DEC_MODE_SHIFT);
/* Set rate */
meson_ir_set_mask(ir, IR_DEC_REG0, REG0_RATE_MASK, MESON_TRATE - 1);
/* IRQ on rising and falling edges */
--
2.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/2] ARM: dts: meson: fixed size of the meson-ir registers
2016-06-26 21:06 [v2] media: rc: fix Meson IR decoder Martin Blumenstingl
2016-06-26 21:06 ` [PATCH v2 1/2] media: rc: meson-ir: fix enabling raw/soft-decoding mode Martin Blumenstingl
@ 2016-06-26 21:06 ` Martin Blumenstingl
2016-06-27 8:43 ` Carlo Caione
2016-06-27 6:28 ` [v2] media: rc: fix Meson IR decoder Neil Armstrong
2016-06-27 12:57 ` Neil Armstrong
3 siblings, 1 reply; 6+ messages in thread
From: Martin Blumenstingl @ 2016-06-26 21:06 UTC (permalink / raw)
To: linux-arm-kernel
According to the reference driver (and the datasheet of the newer
Meson8b/S805 and GXBB/S905 SoCs) there are 14 registers, each 32 bit
wide.
Adjust the register size to reflect that, as register offset 0x20 is
now also needed by the meson-ir driver.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
changes in v1 -> v2:
- new patch, this is needed because we are now trying to read/write
offset 0x20 which is beyond the space which was reserved previously
arch/arm/boot/dts/meson.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
index 8c77c87..0f5722a 100644
--- a/arch/arm/boot/dts/meson.dtsi
+++ b/arch/arm/boot/dts/meson.dtsi
@@ -147,7 +147,7 @@
ir_receiver: ir-receiver at c8100480 {
compatible= "amlogic,meson6-ir";
- reg = <0xc8100480 0x20>;
+ reg = <0xc8100480 0x34>;
interrupts = <0 15 1>;
status = "disabled";
};
--
2.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/2] ARM: dts: meson: fixed size of the meson-ir registers
2016-06-26 21:06 ` [PATCH v2 2/2] ARM: dts: meson: fixed size of the meson-ir registers Martin Blumenstingl
@ 2016-06-27 8:43 ` Carlo Caione
0 siblings, 0 replies; 6+ messages in thread
From: Carlo Caione @ 2016-06-27 8:43 UTC (permalink / raw)
To: linux-arm-kernel
On 26/06/16 23:06, Martin Blumenstingl wrote:
> According to the reference driver (and the datasheet of the newer
> Meson8b/S805 and GXBB/S905 SoCs) there are 14 registers, each 32 bit
> wide.
Then why are you modifying the DTS for the Meson6? As Neil already
suggested, it seems that the hardware has been slightly modified for the
latest SoCs, so this approach is clearly wrong.
Add a new compatible and use of_device_get_match_data() to get the SoC
specific data.
> Adjust the register size to reflect that, as register offset 0x20 is
> now also needed by the meson-ir driver.
According to the AML8726-MX (meson6) datasheet the value of 0x20 is
correct, at least for that hardware.
Cheers,
--
Carlo Caione
^ permalink raw reply [flat|nested] 6+ messages in thread
* [v2] media: rc: fix Meson IR decoder
2016-06-26 21:06 [v2] media: rc: fix Meson IR decoder Martin Blumenstingl
2016-06-26 21:06 ` [PATCH v2 1/2] media: rc: meson-ir: fix enabling raw/soft-decoding mode Martin Blumenstingl
2016-06-26 21:06 ` [PATCH v2 2/2] ARM: dts: meson: fixed size of the meson-ir registers Martin Blumenstingl
@ 2016-06-27 6:28 ` Neil Armstrong
2016-06-27 12:57 ` Neil Armstrong
3 siblings, 0 replies; 6+ messages in thread
From: Neil Armstrong @ 2016-06-27 6:28 UTC (permalink / raw)
To: linux-arm-kernel
On 06/26/2016 11:06 PM, Martin Blumenstingl wrote:
> The meson-ir driver uses the wrong offset (at least according to
> Amlogic's reference driver as well as the datasheets of the
> Meson8b/S805 and GXBB/S905).
> This means that we are getting incorrect durations (REG1_TIME_IV)
> reported from the hardware.
Hi,
I'm quite sure the registers are good for meson6 actually, and
it seems reasonable Amlogic made the HW evolve for the Meson8 and GXBB platforms.
>
> This problem was also noticed by some people trying to use this on an
> ODROID-C1 and ODROID-C2 - the workaround there (probably because the
> datasheets were not publicy available yet at that time) was to switch
> to ir_raw_event_store_edge (which leaves it up to the kernel to measure
> the duration of a pulse). See [0] and [1] for the corresponding
> patches.
Since we are using devicetree, the correct way to achieve this fix is not
to drop support for meson6 (what you do) but add a logic to select the correct
register for meson8 and gxbb if their compatible string are encountered.
> Please note that I was only able to test this on an GXBB/S905 based
> device (due to lack of other hardware).
I made this fix already but lacked time to actually test it on HW :
https://github.com/torvalds/linux/compare/master...superna9999:amlogic/v4.7/ir
My patch is missing the meson8b support, and may need a supplementary compatible check or
a separate dt match table.
Neil
>
> [0] https://github.com/erdoukki/linux-amlogic-1/commit/969b2e2242fb14a13cb651f9a1cf771b599c958b
> [1] http://forum.odroid.com/viewtopic.php?f=135&t=20504
>
PS: BTW could you format the cover letter using the git format-patch --cover-letter instead and
add the v2 using the -subject-prefix like :
# git format-patch --cover-letter --signoff --subject-prefix "PATCH v2" -2
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [v2] media: rc: fix Meson IR decoder
2016-06-26 21:06 [v2] media: rc: fix Meson IR decoder Martin Blumenstingl
` (2 preceding siblings ...)
2016-06-27 6:28 ` [v2] media: rc: fix Meson IR decoder Neil Armstrong
@ 2016-06-27 12:57 ` Neil Armstrong
3 siblings, 0 replies; 6+ messages in thread
From: Neil Armstrong @ 2016-06-27 12:57 UTC (permalink / raw)
To: linux-arm-kernel
On 06/27/2016 12:53 PM, Martin Blumenstingl wrote:
> On Mon, Jun 27, 2016 at 8:27 AM, Neil Armstrong <narmstrong@baylibre.com> wrote:
>> I'm quite sure the registers are good for meson6 actually, and
>> it seems reasonable Amlogic made the HW evolve for the Meson8 and GXBB platforms.
> OK, then from now on I will NOT assume anymore that the reference code
> also works on Meson6 platforms. Thanks for clarifying this.
Yes it's quite safer to assume this !
>> Since we are using devicetree, the correct way to achieve this fix is not
>> to drop support for meson6 (what you do) but add a logic to select the correct
>> register for meson8 and gxbb if their compatible string are encountered.
>
>> I made this fix already but lacked time to actually test it on HW :
>> https://github.com/torvalds/linux/compare/master...superna9999:amlogic/v4.7/ir
>>
>> My patch is missing the meson8b support, and may need a supplementary compatible check or
>> a separate dt match table.
> I can test it on GXBB (only, I do not have Meson8b hardware, but
> according to the datasheet the registers are the same).
> If you want I can start based on your patch series. Should we add only
> a binding for amlogic,meson8b and re-use that in meson-gxbb.dtsi or
> should we add both (8b and gxbb) bindings instead?
Yes, no problem !
Add the two bindings, it's a better practice and we can track more easily which
hardware is really supported from the driver point of view.
>> PS: BTW could you format the cover letter using the git format-patch --cover-letter instead and
>> add the v2 using the -subject-prefix like :
>> # git format-patch --cover-letter --signoff --subject-prefix "PATCH v2" -2
> sounds like this is what other devs are using as well - thanks for
> letting me know
>
Neil
^ permalink raw reply [flat|nested] 6+ messages in thread