2019-12-12 15:00:28

by Guillaume LA ROQUE

[permalink] [raw]
Subject: [PATCH 3/3] media: platform: meson-ao-cec-g12a: add wakeup support

add register configuration to activate wakeup feature in bl301

Signed-off-by: Guillaume La Roque <[email protected]>
---
drivers/media/platform/meson/ao-cec-g12a.c | 33 ++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/drivers/media/platform/meson/ao-cec-g12a.c b/drivers/media/platform/meson/ao-cec-g12a.c
index 3b39e875292e..d441b5a62b0c 100644
--- a/drivers/media/platform/meson/ao-cec-g12a.c
+++ b/drivers/media/platform/meson/ao-cec-g12a.c
@@ -25,6 +25,7 @@
#include <media/cec.h>
#include <media/cec-notifier.h>
#include <linux/clk-provider.h>
+#include <linux/mfd/syscon.h>

/* CEC Registers */

@@ -168,6 +169,19 @@

#define CECB_WAKEUPCTRL 0x31

+#define CECB_FUNC_CFG_REG 0xA0
+#define CECB_FUNC_CFG_MASK GENMASK(6, 0)
+#define CECB_FUNC_CFG_CEC_ON 0x01
+#define CECB_FUNC_CFG_OTP_ON 0x02
+#define CECB_FUNC_CFG_AUTO_STANDBY 0x04
+#define CECB_FUNC_CFG_AUTO_POWER_ON 0x08
+#define CECB_FUNC_CFG_ALL 0x2f
+#define CECB_FUNC_CFG_NONE 0x0
+
+#define CECB_LOG_ADDR_REG 0xA4
+#define CECB_LOG_ADDR_MASK GENMASK(22, 16)
+#define CECB_LOG_ADDR_SHIFT 16
+
struct meson_ao_cec_g12a_data {
/* Setup the internal CECB_CTRL2 register */
bool ctrl2_setup;
@@ -177,6 +191,7 @@ struct meson_ao_cec_g12a_device {
struct platform_device *pdev;
struct regmap *regmap;
struct regmap *regmap_cec;
+ struct regmap *regmap_ao_sysctrl;
spinlock_t cec_reg_lock;
struct cec_notifier *notify;
struct cec_adapter *adap;
@@ -518,6 +533,12 @@ meson_ao_cec_g12a_set_log_addr(struct cec_adapter *adap, u8 logical_addr)
BIT(logical_addr - 8));
}

+ if (ao_cec->regmap_ao_sysctrl)
+ ret |= regmap_update_bits(ao_cec->regmap_ao_sysctrl,
+ CECB_LOG_ADDR_REG,
+ CECB_FUNC_CFG_MASK,
+ logical_addr << CECB_LOG_ADDR_SHIFT);
+
/* Always set Broadcast/Unregistered 15 address */
ret |= regmap_update_bits(ao_cec->regmap_cec, CECB_LADD_HIGH,
BIT(CEC_LOG_ADDR_UNREGISTERED - 8),
@@ -618,6 +639,13 @@ static int meson_ao_cec_g12a_adap_enable(struct cec_adapter *adap, bool enable)
regmap_write(ao_cec->regmap_cec, CECB_CTRL2,
FIELD_PREP(CECB_CTRL2_RISE_DEL_MAX, 2));

+ if (ao_cec->regmap_ao_sysctrl) {
+ regmap_update_bits(ao_cec->regmap_ao_sysctrl,
+ CECB_FUNC_CFG_REG,
+ CECB_FUNC_CFG_MASK,
+ CECB_FUNC_CFG_ALL);
+ }
+
meson_ao_cec_g12a_irq_setup(ao_cec, true);

return 0;
@@ -692,6 +720,11 @@ static int meson_ao_cec_g12a_probe(struct platform_device *pdev)
goto out_probe_adapter;
}

+ ao_cec->regmap_ao_sysctrl = syscon_regmap_lookup_by_phandle
+ (pdev->dev.of_node, "amlogic,ao-sysctrl");
+ if (IS_ERR(ao_cec->regmap_ao_sysctrl))
+ dev_warn(&pdev->dev, "ao-sysctrl syscon regmap lookup failed.\n");
+
irq = platform_get_irq(pdev, 0);
ret = devm_request_threaded_irq(&pdev->dev, irq,
meson_ao_cec_g12a_irq,
--
2.17.1


2019-12-12 19:59:52

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH 3/3] media: platform: meson-ao-cec-g12a: add wakeup support

Hi Guillaume,

(I don't know the specifics of this hardware but I have two general
comments below)

On Thu, Dec 12, 2019 at 4:00 PM Guillaume La Roque
<[email protected]> wrote:
> +#define CECB_FUNC_CFG_REG 0xA0
> +#define CECB_FUNC_CFG_MASK GENMASK(6, 0)
> +#define CECB_FUNC_CFG_CEC_ON 0x01
> +#define CECB_FUNC_CFG_OTP_ON 0x02
> +#define CECB_FUNC_CFG_AUTO_STANDBY 0x04
> +#define CECB_FUNC_CFG_AUTO_POWER_ON 0x08
> +#define CECB_FUNC_CFG_ALL 0x2f
> +#define CECB_FUNC_CFG_NONE 0x0
> +
> +#define CECB_LOG_ADDR_REG 0xA4
> +#define CECB_LOG_ADDR_MASK GENMASK(22, 16)
do these registers have some RTI_* prefix in the datasheet?
that would make it easier to spot that these registers belong to AO /
RTI (while all other registers belong to the CEC controller)

[...]
> + if (ao_cec->regmap_ao_sysctrl)
> + ret |= regmap_update_bits(ao_cec->regmap_ao_sysctrl,
> + CECB_LOG_ADDR_REG,
> + CECB_FUNC_CFG_MASK,
why do we need to mask CECB_FUNC_CFG_MASK (from register 0xa0) in the
CECB_LOG_ADDR_REG register (0xa4)?

> + logical_addr << CECB_LOG_ADDR_SHIFT);
FIELD_PREP(CECB_FUNC_CFG_MASK, logical_addr) would make it consistent
with the rest of the driver
then you can also drop the #define CECB_LOG_ADDR_SHIFT


Martin

2019-12-12 21:33:29

by Guillaume LA ROQUE

[permalink] [raw]
Subject: Re: [PATCH 3/3] media: platform: meson-ao-cec-g12a: add wakeup support

Hi Martin,


thanks for review

On 12/12/19 8:57 PM, Martin Blumenstingl wrote:
> Hi Guillaume,
>
> (I don't know the specifics of this hardware but I have two general
> comments below)
>
> On Thu, Dec 12, 2019 at 4:00 PM Guillaume La Roque
> <[email protected]> wrote:
>> +#define CECB_FUNC_CFG_REG 0xA0
>> +#define CECB_FUNC_CFG_MASK GENMASK(6, 0)
>> +#define CECB_FUNC_CFG_CEC_ON 0x01
>> +#define CECB_FUNC_CFG_OTP_ON 0x02
>> +#define CECB_FUNC_CFG_AUTO_STANDBY 0x04
>> +#define CECB_FUNC_CFG_AUTO_POWER_ON 0x08
>> +#define CECB_FUNC_CFG_ALL 0x2f
>> +#define CECB_FUNC_CFG_NONE 0x0
>> +
>> +#define CECB_LOG_ADDR_REG 0xA4
>> +#define CECB_LOG_ADDR_MASK GENMASK(22, 16)
> do these registers have some RTI_* prefix in the datasheet?
> that would make it easier to spot that these registers belong to AO /
> RTI (while all other registers belong to the CEC controller)

as i say register info come from amlogic BSP.

nothing in datasheet unfortunately. in amlogic code , this register are called AO_DEBUG_REG0 and AO_DEBUG_REG1 in amlogic BSP...

>
> [...]
>> + if (ao_cec->regmap_ao_sysctrl)
>> + ret |= regmap_update_bits(ao_cec->regmap_ao_sysctrl,
>> + CECB_LOG_ADDR_REG,
>> + CECB_FUNC_CFG_MASK,
> why do we need to mask CECB_FUNC_CFG_MASK (from register 0xa0) in the
> CECB_LOG_ADDR_REG register (0xa4)?
good point, it's an error i will fix
>
>> + logical_addr << CECB_LOG_ADDR_SHIFT);
> FIELD_PREP(CECB_FUNC_CFG_MASK, logical_addr) would make it consistent
> with the rest of the driver
> then you can also drop the #define CECB_LOG_ADDR_SHIFT
i will
>
> Martin