* [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
@ 2019-12-13 13:29 ` Guillaume La Roque
0 siblings, 0 replies; 30+ messages in thread
From: Guillaume La Roque @ 2019-12-13 13:29 UTC (permalink / raw)
To: narmstrong, mchehab, hverkuil-cisco, khilman, devicetree
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-media
add register configuration to activate wakeup feature in bl301
Tested-by: Kevin Hilman <khilman@baylibre.com>
Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
---
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 891533060d49..85850b974126 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,18 @@
#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)
+
struct meson_ao_cec_g12a_data {
/* Setup the internal CECB_CTRL2 register */
bool ctrl2_setup;
@@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
+ FIELD_PREP(CECB_LOG_ADDR_MASK,
+ logical_addr));
+
/* 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;
@@ -685,6 +713,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
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
@ 2019-12-13 13:29 ` Guillaume La Roque
0 siblings, 0 replies; 30+ messages in thread
From: Guillaume La Roque @ 2019-12-13 13:29 UTC (permalink / raw)
To: narmstrong, mchehab, hverkuil-cisco, khilman, devicetree
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-media
add register configuration to activate wakeup feature in bl301
Tested-by: Kevin Hilman <khilman@baylibre.com>
Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
---
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 891533060d49..85850b974126 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,18 @@
#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)
+
struct meson_ao_cec_g12a_data {
/* Setup the internal CECB_CTRL2 register */
bool ctrl2_setup;
@@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
+ FIELD_PREP(CECB_LOG_ADDR_MASK,
+ logical_addr));
+
/* 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;
@@ -685,6 +713,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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
2019-12-13 13:29 ` Guillaume La Roque
(?)
@ 2020-01-06 13:18 ` Neil Armstrong
-1 siblings, 0 replies; 30+ messages in thread
From: Neil Armstrong @ 2020-01-06 13:18 UTC (permalink / raw)
To: Guillaume La Roque, mchehab, hverkuil-cisco, khilman, devicetree
Cc: linux-media, linux-amlogic, linux-arm-kernel, linux-kernel
On 13/12/2019 14:29, Guillaume La Roque wrote:
> add register configuration to activate wakeup feature in bl301
>
> Tested-by: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
> ---
> 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 891533060d49..85850b974126 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,18 @@
>
> #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)
> +
> struct meson_ao_cec_g12a_data {
> /* Setup the internal CECB_CTRL2 register */
> bool ctrl2_setup;
> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
> + FIELD_PREP(CECB_LOG_ADDR_MASK,
> + logical_addr));
> +
> /* 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;
> @@ -685,6 +713,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,
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
@ 2020-01-06 13:18 ` Neil Armstrong
0 siblings, 0 replies; 30+ messages in thread
From: Neil Armstrong @ 2020-01-06 13:18 UTC (permalink / raw)
To: Guillaume La Roque, mchehab, hverkuil-cisco, khilman, devicetree
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-media
On 13/12/2019 14:29, Guillaume La Roque wrote:
> add register configuration to activate wakeup feature in bl301
>
> Tested-by: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
> ---
> 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 891533060d49..85850b974126 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,18 @@
>
> #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)
> +
> struct meson_ao_cec_g12a_data {
> /* Setup the internal CECB_CTRL2 register */
> bool ctrl2_setup;
> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
> + FIELD_PREP(CECB_LOG_ADDR_MASK,
> + logical_addr));
> +
> /* 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;
> @@ -685,6 +713,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,
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
@ 2020-01-06 13:18 ` Neil Armstrong
0 siblings, 0 replies; 30+ messages in thread
From: Neil Armstrong @ 2020-01-06 13:18 UTC (permalink / raw)
To: Guillaume La Roque, mchehab, hverkuil-cisco, khilman, devicetree
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-media
On 13/12/2019 14:29, Guillaume La Roque wrote:
> add register configuration to activate wakeup feature in bl301
>
> Tested-by: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
> ---
> 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 891533060d49..85850b974126 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,18 @@
>
> #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)
> +
> struct meson_ao_cec_g12a_data {
> /* Setup the internal CECB_CTRL2 register */
> bool ctrl2_setup;
> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
> + FIELD_PREP(CECB_LOG_ADDR_MASK,
> + logical_addr));
> +
> /* 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;
> @@ -685,6 +713,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,
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
2019-12-13 13:29 ` Guillaume La Roque
(?)
@ 2020-01-07 14:36 ` Hans Verkuil
-1 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2020-01-07 14:36 UTC (permalink / raw)
To: Guillaume La Roque, narmstrong, mchehab, khilman, devicetree
Cc: linux-media, linux-amlogic, linux-arm-kernel, linux-kernel
Hi Guillaume,
On 12/13/19 2:29 PM, Guillaume La Roque wrote:
> add register configuration to activate wakeup feature in bl301
>
> Tested-by: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
> ---
> 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 891533060d49..85850b974126 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,18 @@
>
> #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)
> +
> struct meson_ao_cec_g12a_data {
> /* Setup the internal CECB_CTRL2 register */
> bool ctrl2_setup;
> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
> + FIELD_PREP(CECB_LOG_ADDR_MASK,
> + logical_addr));
> +
> /* 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);
What exactly is enabled here? Looking at CECB_FUNC_CFG_ALL it seems to
enable automatic standby (I presume when the STANDBY message is received?)
and power on (I presume when SET_STREAM_PATH is received?).
Do you really want to automatically handle STANDBY that way? What does this
do on the hardware level anyway? Isn't this something that should be
controlled in userspace?
Similar questions for power on: you may not always want to enable this feature
since it depends very much on the precise use-case.
And which messages it reacts to in order to do a power-on needs to be
documented since this differs depending on whether the CEC adapter is
used for a TV or for a playback device. This feature may be hardwired for
a playback device only, in which case it should probably be disabled if
the CEC adapter is configured as a TV.
In any case I would like to see some more details about how this works,
especially since this is the first implementation of such a feature.
I suspect that some userspace API might be needed to get the right level
of control of such a feature.
Regards,
Hans
> + }
> +
> meson_ao_cec_g12a_irq_setup(ao_cec, true);
>
> return 0;
> @@ -685,6 +713,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,
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
@ 2020-01-07 14:36 ` Hans Verkuil
0 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2020-01-07 14:36 UTC (permalink / raw)
To: Guillaume La Roque, narmstrong, mchehab, khilman, devicetree
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-media
Hi Guillaume,
On 12/13/19 2:29 PM, Guillaume La Roque wrote:
> add register configuration to activate wakeup feature in bl301
>
> Tested-by: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
> ---
> 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 891533060d49..85850b974126 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,18 @@
>
> #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)
> +
> struct meson_ao_cec_g12a_data {
> /* Setup the internal CECB_CTRL2 register */
> bool ctrl2_setup;
> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
> + FIELD_PREP(CECB_LOG_ADDR_MASK,
> + logical_addr));
> +
> /* 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);
What exactly is enabled here? Looking at CECB_FUNC_CFG_ALL it seems to
enable automatic standby (I presume when the STANDBY message is received?)
and power on (I presume when SET_STREAM_PATH is received?).
Do you really want to automatically handle STANDBY that way? What does this
do on the hardware level anyway? Isn't this something that should be
controlled in userspace?
Similar questions for power on: you may not always want to enable this feature
since it depends very much on the precise use-case.
And which messages it reacts to in order to do a power-on needs to be
documented since this differs depending on whether the CEC adapter is
used for a TV or for a playback device. This feature may be hardwired for
a playback device only, in which case it should probably be disabled if
the CEC adapter is configured as a TV.
In any case I would like to see some more details about how this works,
especially since this is the first implementation of such a feature.
I suspect that some userspace API might be needed to get the right level
of control of such a feature.
Regards,
Hans
> + }
> +
> meson_ao_cec_g12a_irq_setup(ao_cec, true);
>
> return 0;
> @@ -685,6 +713,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,
>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
@ 2020-01-07 14:36 ` Hans Verkuil
0 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2020-01-07 14:36 UTC (permalink / raw)
To: Guillaume La Roque, narmstrong, mchehab, khilman, devicetree
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-media
Hi Guillaume,
On 12/13/19 2:29 PM, Guillaume La Roque wrote:
> add register configuration to activate wakeup feature in bl301
>
> Tested-by: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
> ---
> 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 891533060d49..85850b974126 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,18 @@
>
> #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)
> +
> struct meson_ao_cec_g12a_data {
> /* Setup the internal CECB_CTRL2 register */
> bool ctrl2_setup;
> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
> + FIELD_PREP(CECB_LOG_ADDR_MASK,
> + logical_addr));
> +
> /* 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);
What exactly is enabled here? Looking at CECB_FUNC_CFG_ALL it seems to
enable automatic standby (I presume when the STANDBY message is received?)
and power on (I presume when SET_STREAM_PATH is received?).
Do you really want to automatically handle STANDBY that way? What does this
do on the hardware level anyway? Isn't this something that should be
controlled in userspace?
Similar questions for power on: you may not always want to enable this feature
since it depends very much on the precise use-case.
And which messages it reacts to in order to do a power-on needs to be
documented since this differs depending on whether the CEC adapter is
used for a TV or for a playback device. This feature may be hardwired for
a playback device only, in which case it should probably be disabled if
the CEC adapter is configured as a TV.
In any case I would like to see some more details about how this works,
especially since this is the first implementation of such a feature.
I suspect that some userspace API might be needed to get the right level
of control of such a feature.
Regards,
Hans
> + }
> +
> meson_ao_cec_g12a_irq_setup(ao_cec, true);
>
> return 0;
> @@ -685,6 +713,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,
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
2020-01-07 14:36 ` Hans Verkuil
(?)
@ 2020-01-10 15:06 ` guillaume La Roque
-1 siblings, 0 replies; 30+ messages in thread
From: guillaume La Roque @ 2020-01-10 15:06 UTC (permalink / raw)
To: Hans Verkuil, narmstrong, mchehab, khilman, devicetree
Cc: linux-media, linux-amlogic, linux-arm-kernel, linux-kernel
Hi Hans,
On 1/7/20 3:36 PM, Hans Verkuil wrote:
> Hi Guillaume,
>
> On 12/13/19 2:29 PM, Guillaume La Roque wrote:
>> add register configuration to activate wakeup feature in bl301
>>
>> Tested-by: Kevin Hilman <khilman@baylibre.com>
>> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
>> ---
>> 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 891533060d49..85850b974126 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,18 @@
>>
>> #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)
>> +
>> struct meson_ao_cec_g12a_data {
>> /* Setup the internal CECB_CTRL2 register */
>> bool ctrl2_setup;
>> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
>> + FIELD_PREP(CECB_LOG_ADDR_MASK,
>> + logical_addr));
>> +
>> /* 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);
> What exactly is enabled here? Looking at CECB_FUNC_CFG_ALL it seems to
> enable automatic standby (I presume when the STANDBY message is received?)
> and power on (I presume when SET_STREAM_PATH is received?).
this register and flags are used by bl301 part.
amlogic implemented a task to check cec event/message.
for power on in bl301 it's not only on SET_STREAM_PATH but also on :
USER_CONTROL_PRESSED
TEXT_VIEW_ON
ACTIVE_SOURCE
ROUTING_CHANGE
when in CECB_FUNC_CFG_REG register we put CECB_FUNC_CFG_CEC_ON and CECB_FUNC_CFG_AUTO_POWER_ON
it's not possible to change this
>
> Do you really want to automatically handle STANDBY that way? What does this
> do on the hardware level anyway? Isn't this something that should be
> controlled in userspace?
in fact i do a new check in bl301 code amlogic do nothing on STANDBY so i will clean code
and activate real option supported by bl301
>
> Similar questions for power on: you may not always want to enable this feature
> since it depends very much on the precise use-case.
>
> And which messages it reacts to in order to do a power-on needs to be
> documented since this differs depending on whether the CEC adapter is
> used for a TV or for a playback device. This feature may be hardwired for
> a playback device only, in which case it should probably be disabled if
> the CEC adapter is configured as a TV.
>
> In any case I would like to see some more details about how this works,
> especially since this is the first implementation of such a feature.
>
> I suspect that some userspace API might be needed to get the right level
> of control of such a feature.
i will send v3 next week with some comments and fix ( disable are missing for example)
actual usercase is for android TV.
when cec was enable android TV want to be wakeup by cec event.
> Regards,
>
> Hans
>
thanks for your review
Regards
Guillaume
>> + }
>> +
>> meson_ao_cec_g12a_irq_setup(ao_cec, true);
>>
>> return 0;
>> @@ -685,6 +713,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,
>>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
@ 2020-01-10 15:06 ` guillaume La Roque
0 siblings, 0 replies; 30+ messages in thread
From: guillaume La Roque @ 2020-01-10 15:06 UTC (permalink / raw)
To: Hans Verkuil, narmstrong, mchehab, khilman, devicetree
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-media
Hi Hans,
On 1/7/20 3:36 PM, Hans Verkuil wrote:
> Hi Guillaume,
>
> On 12/13/19 2:29 PM, Guillaume La Roque wrote:
>> add register configuration to activate wakeup feature in bl301
>>
>> Tested-by: Kevin Hilman <khilman@baylibre.com>
>> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
>> ---
>> 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 891533060d49..85850b974126 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,18 @@
>>
>> #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)
>> +
>> struct meson_ao_cec_g12a_data {
>> /* Setup the internal CECB_CTRL2 register */
>> bool ctrl2_setup;
>> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
>> + FIELD_PREP(CECB_LOG_ADDR_MASK,
>> + logical_addr));
>> +
>> /* 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);
> What exactly is enabled here? Looking at CECB_FUNC_CFG_ALL it seems to
> enable automatic standby (I presume when the STANDBY message is received?)
> and power on (I presume when SET_STREAM_PATH is received?).
this register and flags are used by bl301 part.
amlogic implemented a task to check cec event/message.
for power on in bl301 it's not only on SET_STREAM_PATH but also on :
USER_CONTROL_PRESSED
TEXT_VIEW_ON
ACTIVE_SOURCE
ROUTING_CHANGE
when in CECB_FUNC_CFG_REG register we put CECB_FUNC_CFG_CEC_ON and CECB_FUNC_CFG_AUTO_POWER_ON
it's not possible to change this
>
> Do you really want to automatically handle STANDBY that way? What does this
> do on the hardware level anyway? Isn't this something that should be
> controlled in userspace?
in fact i do a new check in bl301 code amlogic do nothing on STANDBY so i will clean code
and activate real option supported by bl301
>
> Similar questions for power on: you may not always want to enable this feature
> since it depends very much on the precise use-case.
>
> And which messages it reacts to in order to do a power-on needs to be
> documented since this differs depending on whether the CEC adapter is
> used for a TV or for a playback device. This feature may be hardwired for
> a playback device only, in which case it should probably be disabled if
> the CEC adapter is configured as a TV.
>
> In any case I would like to see some more details about how this works,
> especially since this is the first implementation of such a feature.
>
> I suspect that some userspace API might be needed to get the right level
> of control of such a feature.
i will send v3 next week with some comments and fix ( disable are missing for example)
actual usercase is for android TV.
when cec was enable android TV want to be wakeup by cec event.
> Regards,
>
> Hans
>
thanks for your review
Regards
Guillaume
>> + }
>> +
>> meson_ao_cec_g12a_irq_setup(ao_cec, true);
>>
>> return 0;
>> @@ -685,6 +713,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,
>>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
@ 2020-01-10 15:06 ` guillaume La Roque
0 siblings, 0 replies; 30+ messages in thread
From: guillaume La Roque @ 2020-01-10 15:06 UTC (permalink / raw)
To: Hans Verkuil, narmstrong, mchehab, khilman, devicetree
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-media
Hi Hans,
On 1/7/20 3:36 PM, Hans Verkuil wrote:
> Hi Guillaume,
>
> On 12/13/19 2:29 PM, Guillaume La Roque wrote:
>> add register configuration to activate wakeup feature in bl301
>>
>> Tested-by: Kevin Hilman <khilman@baylibre.com>
>> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
>> ---
>> 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 891533060d49..85850b974126 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,18 @@
>>
>> #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)
>> +
>> struct meson_ao_cec_g12a_data {
>> /* Setup the internal CECB_CTRL2 register */
>> bool ctrl2_setup;
>> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
>> + FIELD_PREP(CECB_LOG_ADDR_MASK,
>> + logical_addr));
>> +
>> /* 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);
> What exactly is enabled here? Looking at CECB_FUNC_CFG_ALL it seems to
> enable automatic standby (I presume when the STANDBY message is received?)
> and power on (I presume when SET_STREAM_PATH is received?).
this register and flags are used by bl301 part.
amlogic implemented a task to check cec event/message.
for power on in bl301 it's not only on SET_STREAM_PATH but also on :
USER_CONTROL_PRESSED
TEXT_VIEW_ON
ACTIVE_SOURCE
ROUTING_CHANGE
when in CECB_FUNC_CFG_REG register we put CECB_FUNC_CFG_CEC_ON and CECB_FUNC_CFG_AUTO_POWER_ON
it's not possible to change this
>
> Do you really want to automatically handle STANDBY that way? What does this
> do on the hardware level anyway? Isn't this something that should be
> controlled in userspace?
in fact i do a new check in bl301 code amlogic do nothing on STANDBY so i will clean code
and activate real option supported by bl301
>
> Similar questions for power on: you may not always want to enable this feature
> since it depends very much on the precise use-case.
>
> And which messages it reacts to in order to do a power-on needs to be
> documented since this differs depending on whether the CEC adapter is
> used for a TV or for a playback device. This feature may be hardwired for
> a playback device only, in which case it should probably be disabled if
> the CEC adapter is configured as a TV.
>
> In any case I would like to see some more details about how this works,
> especially since this is the first implementation of such a feature.
>
> I suspect that some userspace API might be needed to get the right level
> of control of such a feature.
i will send v3 next week with some comments and fix ( disable are missing for example)
actual usercase is for android TV.
when cec was enable android TV want to be wakeup by cec event.
> Regards,
>
> Hans
>
thanks for your review
Regards
Guillaume
>> + }
>> +
>> meson_ao_cec_g12a_irq_setup(ao_cec, true);
>>
>> return 0;
>> @@ -685,6 +713,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,
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
2020-01-10 15:06 ` guillaume La Roque
(?)
@ 2020-01-13 13:30 ` Hans Verkuil
-1 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2020-01-13 13:30 UTC (permalink / raw)
To: guillaume La Roque, narmstrong, mchehab, khilman, devicetree
Cc: linux-media, linux-amlogic, linux-arm-kernel, linux-kernel
On 1/10/20 4:06 PM, guillaume La Roque wrote:
> Hi Hans,
>
> On 1/7/20 3:36 PM, Hans Verkuil wrote:
>> Hi Guillaume,
>>
>> On 12/13/19 2:29 PM, Guillaume La Roque wrote:
>>> add register configuration to activate wakeup feature in bl301
>>>
>>> Tested-by: Kevin Hilman <khilman@baylibre.com>
>>> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
>>> ---
>>> 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 891533060d49..85850b974126 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,18 @@
>>>
>>> #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)
>>> +
>>> struct meson_ao_cec_g12a_data {
>>> /* Setup the internal CECB_CTRL2 register */
>>> bool ctrl2_setup;
>>> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
>>> + FIELD_PREP(CECB_LOG_ADDR_MASK,
>>> + logical_addr));
>>> +
>>> /* 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);
>> What exactly is enabled here? Looking at CECB_FUNC_CFG_ALL it seems to
>> enable automatic standby (I presume when the STANDBY message is received?)
>> and power on (I presume when SET_STREAM_PATH is received?).
> this register and flags are used by bl301 part.
>
> amlogic implemented a task to check cec event/message.
>
> for power on in bl301 it's not only on SET_STREAM_PATH but also on :
>
> USER_CONTROL_PRESSED
> TEXT_VIEW_ON
Not IMAGE_VIEW_ON?
> ACTIVE_SOURCE
> ROUTING_CHANGE
>
> when in CECB_FUNC_CFG_REG register we put CECB_FUNC_CFG_CEC_ON and CECB_FUNC_CFG_AUTO_POWER_ON
>
> it's not possible to change this
Too bad since ACTIVE_SOURCE and ROUTING_CHANGE should not power on a CEC device.
See section 11.5.2 in the HDMI 2.0 Specification for more details, if you have
that spec.
>
>>
>> Do you really want to automatically handle STANDBY that way? What does this
>> do on the hardware level anyway? Isn't this something that should be
>> controlled in userspace?
>
> in fact i do a new check in bl301 code amlogic do nothing on STANDBY so i will clean code
>
> and activate real option supported by bl301
>
>>
>> Similar questions for power on: you may not always want to enable this feature
>> since it depends very much on the precise use-case.
>>
>> And which messages it reacts to in order to do a power-on needs to be
>> documented since this differs depending on whether the CEC adapter is
>> used for a TV or for a playback device. This feature may be hardwired for
>> a playback device only, in which case it should probably be disabled if
>> the CEC adapter is configured as a TV.
>>
>> In any case I would like to see some more details about how this works,
>> especially since this is the first implementation of such a feature.
>>
>> I suspect that some userspace API might be needed to get the right level
>> of control of such a feature.
>
> i will send v3 next week with some comments and fix ( disable are missing for example)
>
> actual usercase is for android TV.
>
> when cec was enable android TV want to be wakeup by cec event.
>
>
>> Regards,
>>
>> Hans
>>
>
> thanks for your review
No problem!
Hans
>
> Regards
>
> Guillaume
>
>>> + }
>>> +
>>> meson_ao_cec_g12a_irq_setup(ao_cec, true);
>>>
>>> return 0;
>>> @@ -685,6 +713,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,
>>>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
@ 2020-01-13 13:30 ` Hans Verkuil
0 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2020-01-13 13:30 UTC (permalink / raw)
To: guillaume La Roque, narmstrong, mchehab, khilman, devicetree
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-media
On 1/10/20 4:06 PM, guillaume La Roque wrote:
> Hi Hans,
>
> On 1/7/20 3:36 PM, Hans Verkuil wrote:
>> Hi Guillaume,
>>
>> On 12/13/19 2:29 PM, Guillaume La Roque wrote:
>>> add register configuration to activate wakeup feature in bl301
>>>
>>> Tested-by: Kevin Hilman <khilman@baylibre.com>
>>> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
>>> ---
>>> 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 891533060d49..85850b974126 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,18 @@
>>>
>>> #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)
>>> +
>>> struct meson_ao_cec_g12a_data {
>>> /* Setup the internal CECB_CTRL2 register */
>>> bool ctrl2_setup;
>>> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
>>> + FIELD_PREP(CECB_LOG_ADDR_MASK,
>>> + logical_addr));
>>> +
>>> /* 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);
>> What exactly is enabled here? Looking at CECB_FUNC_CFG_ALL it seems to
>> enable automatic standby (I presume when the STANDBY message is received?)
>> and power on (I presume when SET_STREAM_PATH is received?).
> this register and flags are used by bl301 part.
>
> amlogic implemented a task to check cec event/message.
>
> for power on in bl301 it's not only on SET_STREAM_PATH but also on :
>
> USER_CONTROL_PRESSED
> TEXT_VIEW_ON
Not IMAGE_VIEW_ON?
> ACTIVE_SOURCE
> ROUTING_CHANGE
>
> when in CECB_FUNC_CFG_REG register we put CECB_FUNC_CFG_CEC_ON and CECB_FUNC_CFG_AUTO_POWER_ON
>
> it's not possible to change this
Too bad since ACTIVE_SOURCE and ROUTING_CHANGE should not power on a CEC device.
See section 11.5.2 in the HDMI 2.0 Specification for more details, if you have
that spec.
>
>>
>> Do you really want to automatically handle STANDBY that way? What does this
>> do on the hardware level anyway? Isn't this something that should be
>> controlled in userspace?
>
> in fact i do a new check in bl301 code amlogic do nothing on STANDBY so i will clean code
>
> and activate real option supported by bl301
>
>>
>> Similar questions for power on: you may not always want to enable this feature
>> since it depends very much on the precise use-case.
>>
>> And which messages it reacts to in order to do a power-on needs to be
>> documented since this differs depending on whether the CEC adapter is
>> used for a TV or for a playback device. This feature may be hardwired for
>> a playback device only, in which case it should probably be disabled if
>> the CEC adapter is configured as a TV.
>>
>> In any case I would like to see some more details about how this works,
>> especially since this is the first implementation of such a feature.
>>
>> I suspect that some userspace API might be needed to get the right level
>> of control of such a feature.
>
> i will send v3 next week with some comments and fix ( disable are missing for example)
>
> actual usercase is for android TV.
>
> when cec was enable android TV want to be wakeup by cec event.
>
>
>> Regards,
>>
>> Hans
>>
>
> thanks for your review
No problem!
Hans
>
> Regards
>
> Guillaume
>
>>> + }
>>> +
>>> meson_ao_cec_g12a_irq_setup(ao_cec, true);
>>>
>>> return 0;
>>> @@ -685,6 +713,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,
>>>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
@ 2020-01-13 13:30 ` Hans Verkuil
0 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2020-01-13 13:30 UTC (permalink / raw)
To: guillaume La Roque, narmstrong, mchehab, khilman, devicetree
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-media
On 1/10/20 4:06 PM, guillaume La Roque wrote:
> Hi Hans,
>
> On 1/7/20 3:36 PM, Hans Verkuil wrote:
>> Hi Guillaume,
>>
>> On 12/13/19 2:29 PM, Guillaume La Roque wrote:
>>> add register configuration to activate wakeup feature in bl301
>>>
>>> Tested-by: Kevin Hilman <khilman@baylibre.com>
>>> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
>>> ---
>>> 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 891533060d49..85850b974126 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,18 @@
>>>
>>> #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)
>>> +
>>> struct meson_ao_cec_g12a_data {
>>> /* Setup the internal CECB_CTRL2 register */
>>> bool ctrl2_setup;
>>> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
>>> + FIELD_PREP(CECB_LOG_ADDR_MASK,
>>> + logical_addr));
>>> +
>>> /* 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);
>> What exactly is enabled here? Looking at CECB_FUNC_CFG_ALL it seems to
>> enable automatic standby (I presume when the STANDBY message is received?)
>> and power on (I presume when SET_STREAM_PATH is received?).
> this register and flags are used by bl301 part.
>
> amlogic implemented a task to check cec event/message.
>
> for power on in bl301 it's not only on SET_STREAM_PATH but also on :
>
> USER_CONTROL_PRESSED
> TEXT_VIEW_ON
Not IMAGE_VIEW_ON?
> ACTIVE_SOURCE
> ROUTING_CHANGE
>
> when in CECB_FUNC_CFG_REG register we put CECB_FUNC_CFG_CEC_ON and CECB_FUNC_CFG_AUTO_POWER_ON
>
> it's not possible to change this
Too bad since ACTIVE_SOURCE and ROUTING_CHANGE should not power on a CEC device.
See section 11.5.2 in the HDMI 2.0 Specification for more details, if you have
that spec.
>
>>
>> Do you really want to automatically handle STANDBY that way? What does this
>> do on the hardware level anyway? Isn't this something that should be
>> controlled in userspace?
>
> in fact i do a new check in bl301 code amlogic do nothing on STANDBY so i will clean code
>
> and activate real option supported by bl301
>
>>
>> Similar questions for power on: you may not always want to enable this feature
>> since it depends very much on the precise use-case.
>>
>> And which messages it reacts to in order to do a power-on needs to be
>> documented since this differs depending on whether the CEC adapter is
>> used for a TV or for a playback device. This feature may be hardwired for
>> a playback device only, in which case it should probably be disabled if
>> the CEC adapter is configured as a TV.
>>
>> In any case I would like to see some more details about how this works,
>> especially since this is the first implementation of such a feature.
>>
>> I suspect that some userspace API might be needed to get the right level
>> of control of such a feature.
>
> i will send v3 next week with some comments and fix ( disable are missing for example)
>
> actual usercase is for android TV.
>
> when cec was enable android TV want to be wakeup by cec event.
>
>
>> Regards,
>>
>> Hans
>>
>
> thanks for your review
No problem!
Hans
>
> Regards
>
> Guillaume
>
>>> + }
>>> +
>>> meson_ao_cec_g12a_irq_setup(ao_cec, true);
>>>
>>> return 0;
>>> @@ -685,6 +713,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,
>>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
2020-01-13 13:30 ` Hans Verkuil
(?)
@ 2020-01-13 15:19 ` guillaume La Roque
-1 siblings, 0 replies; 30+ messages in thread
From: guillaume La Roque @ 2020-01-13 15:19 UTC (permalink / raw)
To: Hans Verkuil, narmstrong, mchehab, khilman, devicetree
Cc: linux-media, linux-amlogic, linux-arm-kernel, linux-kernel
On 1/13/20 2:30 PM, Hans Verkuil wrote:
> On 1/10/20 4:06 PM, guillaume La Roque wrote:
>> Hi Hans,
>>
>> On 1/7/20 3:36 PM, Hans Verkuil wrote:
>>> Hi Guillaume,
>>>
>>> On 12/13/19 2:29 PM, Guillaume La Roque wrote:
>>>> add register configuration to activate wakeup feature in bl301
>>>>
>>>> Tested-by: Kevin Hilman <khilman@baylibre.com>
>>>> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
>>>> ---
>>>> 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 891533060d49..85850b974126 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,18 @@
>>>>
>>>> #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)
>>>> +
>>>> struct meson_ao_cec_g12a_data {
>>>> /* Setup the internal CECB_CTRL2 register */
>>>> bool ctrl2_setup;
>>>> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
>>>> + FIELD_PREP(CECB_LOG_ADDR_MASK,
>>>> + logical_addr));
>>>> +
>>>> /* 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);
>>> What exactly is enabled here? Looking at CECB_FUNC_CFG_ALL it seems to
>>> enable automatic standby (I presume when the STANDBY message is received?)
>>> and power on (I presume when SET_STREAM_PATH is received?).
>> this register and flags are used by bl301 part.
>>
>> amlogic implemented a task to check cec event/message.
>>
>> for power on in bl301 it's not only on SET_STREAM_PATH but also on :
>>
>> USER_CONTROL_PRESSED
>> TEXT_VIEW_ON
> Not IMAGE_VIEW_ON?
sorry i forgot it , yes on IMAGE_VIEW_ON too
>
>> ACTIVE_SOURCE
>> ROUTING_CHANGE
>>
>> when in CECB_FUNC_CFG_REG register we put CECB_FUNC_CFG_CEC_ON and CECB_FUNC_CFG_AUTO_POWER_ON
>>
>> it's not possible to change this
> Too bad since ACTIVE_SOURCE and ROUTING_CHANGE should not power on a CEC device.
>
> See section 11.5.2 in the HDMI 2.0 Specification for more details, if you have
> that spec.
i will return your comment to amlogic but not sure they do change in bl301.
>
>>> Do you really want to automatically handle STANDBY that way? What does this
>>> do on the hardware level anyway? Isn't this something that should be
>>> controlled in userspace?
>> in fact i do a new check in bl301 code amlogic do nothing on STANDBY so i will clean code
>>
>> and activate real option supported by bl301
>>
>>> Similar questions for power on: you may not always want to enable this feature
>>> since it depends very much on the precise use-case.
>>>
>>> And which messages it reacts to in order to do a power-on needs to be
>>> documented since this differs depending on whether the CEC adapter is
>>> used for a TV or for a playback device. This feature may be hardwired for
>>> a playback device only, in which case it should probably be disabled if
>>> the CEC adapter is configured as a TV.
>>>
>>> In any case I would like to see some more details about how this works,
>>> especially since this is the first implementation of such a feature.
>>>
>>> I suspect that some userspace API might be needed to get the right level
>>> of control of such a feature.
>> i will send v3 next week with some comments and fix ( disable are missing for example)
>>
>> actual usercase is for android TV.
>>
>> when cec was enable android TV want to be wakeup by cec event.
>>
>>
>>> Regards,
>>>
>>> Hans
>>>
>> thanks for your review
> No problem!
>
> Hans
>
>> Regards
>>
>> Guillaume
>>
>>>> + }
>>>> +
>>>> meson_ao_cec_g12a_irq_setup(ao_cec, true);
>>>>
>>>> return 0;
>>>> @@ -685,6 +713,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,
>>>>
Regards
Guillaume
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
@ 2020-01-13 15:19 ` guillaume La Roque
0 siblings, 0 replies; 30+ messages in thread
From: guillaume La Roque @ 2020-01-13 15:19 UTC (permalink / raw)
To: Hans Verkuil, narmstrong, mchehab, khilman, devicetree
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-media
On 1/13/20 2:30 PM, Hans Verkuil wrote:
> On 1/10/20 4:06 PM, guillaume La Roque wrote:
>> Hi Hans,
>>
>> On 1/7/20 3:36 PM, Hans Verkuil wrote:
>>> Hi Guillaume,
>>>
>>> On 12/13/19 2:29 PM, Guillaume La Roque wrote:
>>>> add register configuration to activate wakeup feature in bl301
>>>>
>>>> Tested-by: Kevin Hilman <khilman@baylibre.com>
>>>> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
>>>> ---
>>>> 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 891533060d49..85850b974126 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,18 @@
>>>>
>>>> #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)
>>>> +
>>>> struct meson_ao_cec_g12a_data {
>>>> /* Setup the internal CECB_CTRL2 register */
>>>> bool ctrl2_setup;
>>>> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
>>>> + FIELD_PREP(CECB_LOG_ADDR_MASK,
>>>> + logical_addr));
>>>> +
>>>> /* 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);
>>> What exactly is enabled here? Looking at CECB_FUNC_CFG_ALL it seems to
>>> enable automatic standby (I presume when the STANDBY message is received?)
>>> and power on (I presume when SET_STREAM_PATH is received?).
>> this register and flags are used by bl301 part.
>>
>> amlogic implemented a task to check cec event/message.
>>
>> for power on in bl301 it's not only on SET_STREAM_PATH but also on :
>>
>> USER_CONTROL_PRESSED
>> TEXT_VIEW_ON
> Not IMAGE_VIEW_ON?
sorry i forgot it , yes on IMAGE_VIEW_ON too
>
>> ACTIVE_SOURCE
>> ROUTING_CHANGE
>>
>> when in CECB_FUNC_CFG_REG register we put CECB_FUNC_CFG_CEC_ON and CECB_FUNC_CFG_AUTO_POWER_ON
>>
>> it's not possible to change this
> Too bad since ACTIVE_SOURCE and ROUTING_CHANGE should not power on a CEC device.
>
> See section 11.5.2 in the HDMI 2.0 Specification for more details, if you have
> that spec.
i will return your comment to amlogic but not sure they do change in bl301.
>
>>> Do you really want to automatically handle STANDBY that way? What does this
>>> do on the hardware level anyway? Isn't this something that should be
>>> controlled in userspace?
>> in fact i do a new check in bl301 code amlogic do nothing on STANDBY so i will clean code
>>
>> and activate real option supported by bl301
>>
>>> Similar questions for power on: you may not always want to enable this feature
>>> since it depends very much on the precise use-case.
>>>
>>> And which messages it reacts to in order to do a power-on needs to be
>>> documented since this differs depending on whether the CEC adapter is
>>> used for a TV or for a playback device. This feature may be hardwired for
>>> a playback device only, in which case it should probably be disabled if
>>> the CEC adapter is configured as a TV.
>>>
>>> In any case I would like to see some more details about how this works,
>>> especially since this is the first implementation of such a feature.
>>>
>>> I suspect that some userspace API might be needed to get the right level
>>> of control of such a feature.
>> i will send v3 next week with some comments and fix ( disable are missing for example)
>>
>> actual usercase is for android TV.
>>
>> when cec was enable android TV want to be wakeup by cec event.
>>
>>
>>> Regards,
>>>
>>> Hans
>>>
>> thanks for your review
> No problem!
>
> Hans
>
>> Regards
>>
>> Guillaume
>>
>>>> + }
>>>> +
>>>> meson_ao_cec_g12a_irq_setup(ao_cec, true);
>>>>
>>>> return 0;
>>>> @@ -685,6 +713,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,
>>>>
Regards
Guillaume
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 3/3] media: platform: meson-ao-cec-g12a: add wakeup support
@ 2020-01-13 15:19 ` guillaume La Roque
0 siblings, 0 replies; 30+ messages in thread
From: guillaume La Roque @ 2020-01-13 15:19 UTC (permalink / raw)
To: Hans Verkuil, narmstrong, mchehab, khilman, devicetree
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-media
On 1/13/20 2:30 PM, Hans Verkuil wrote:
> On 1/10/20 4:06 PM, guillaume La Roque wrote:
>> Hi Hans,
>>
>> On 1/7/20 3:36 PM, Hans Verkuil wrote:
>>> Hi Guillaume,
>>>
>>> On 12/13/19 2:29 PM, Guillaume La Roque wrote:
>>>> add register configuration to activate wakeup feature in bl301
>>>>
>>>> Tested-by: Kevin Hilman <khilman@baylibre.com>
>>>> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
>>>> ---
>>>> 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 891533060d49..85850b974126 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,18 @@
>>>>
>>>> #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)
>>>> +
>>>> struct meson_ao_cec_g12a_data {
>>>> /* Setup the internal CECB_CTRL2 register */
>>>> bool ctrl2_setup;
>>>> @@ -177,6 +190,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 +532,13 @@ 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_LOG_ADDR_MASK,
>>>> + FIELD_PREP(CECB_LOG_ADDR_MASK,
>>>> + logical_addr));
>>>> +
>>>> /* 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);
>>> What exactly is enabled here? Looking at CECB_FUNC_CFG_ALL it seems to
>>> enable automatic standby (I presume when the STANDBY message is received?)
>>> and power on (I presume when SET_STREAM_PATH is received?).
>> this register and flags are used by bl301 part.
>>
>> amlogic implemented a task to check cec event/message.
>>
>> for power on in bl301 it's not only on SET_STREAM_PATH but also on :
>>
>> USER_CONTROL_PRESSED
>> TEXT_VIEW_ON
> Not IMAGE_VIEW_ON?
sorry i forgot it , yes on IMAGE_VIEW_ON too
>
>> ACTIVE_SOURCE
>> ROUTING_CHANGE
>>
>> when in CECB_FUNC_CFG_REG register we put CECB_FUNC_CFG_CEC_ON and CECB_FUNC_CFG_AUTO_POWER_ON
>>
>> it's not possible to change this
> Too bad since ACTIVE_SOURCE and ROUTING_CHANGE should not power on a CEC device.
>
> See section 11.5.2 in the HDMI 2.0 Specification for more details, if you have
> that spec.
i will return your comment to amlogic but not sure they do change in bl301.
>
>>> Do you really want to automatically handle STANDBY that way? What does this
>>> do on the hardware level anyway? Isn't this something that should be
>>> controlled in userspace?
>> in fact i do a new check in bl301 code amlogic do nothing on STANDBY so i will clean code
>>
>> and activate real option supported by bl301
>>
>>> Similar questions for power on: you may not always want to enable this feature
>>> since it depends very much on the precise use-case.
>>>
>>> And which messages it reacts to in order to do a power-on needs to be
>>> documented since this differs depending on whether the CEC adapter is
>>> used for a TV or for a playback device. This feature may be hardwired for
>>> a playback device only, in which case it should probably be disabled if
>>> the CEC adapter is configured as a TV.
>>>
>>> In any case I would like to see some more details about how this works,
>>> especially since this is the first implementation of such a feature.
>>>
>>> I suspect that some userspace API might be needed to get the right level
>>> of control of such a feature.
>> i will send v3 next week with some comments and fix ( disable are missing for example)
>>
>> actual usercase is for android TV.
>>
>> when cec was enable android TV want to be wakeup by cec event.
>>
>>
>>> Regards,
>>>
>>> Hans
>>>
>> thanks for your review
> No problem!
>
> Hans
>
>> Regards
>>
>> Guillaume
>>
>>>> + }
>>>> +
>>>> meson_ao_cec_g12a_irq_setup(ao_cec, true);
>>>>
>>>> return 0;
>>>> @@ -685,6 +713,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,
>>>>
Regards
Guillaume
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread