[PATCH v2 0/3] Add support of CEC wakeup on Amlogic G12 and SM1 SoCs

[PATCH v2 0/3] Add support of CEC wakeup on Amlogic G12 and SM1 SoCs

[Guillaume La Roque]

this patchset add support of CEC wakeup.
We need to set logical address and activate some options in registers before going in suspend.
Registers address and options values come from amlogic driver.

Changes since v1:
- fix bad mask variable use to set logical address.
- use FIELD_PREP instead of shift value.

Guillaume La Roque (3):
  media: dt-bindings: media: meson-ao-cec: Add support of ao-sysctrl
    syscon
  arm64: dts: meson-g12g12: add syscon phandle in cec node
  media: platform: meson-ao-cec-g12a: add wakeup support

 .../media/amlogic,meson-gx-ao-cec.yaml        |  4 +++
 .../boot/dts/amlogic/meson-g12-common.dtsi    |  1 +
 drivers/media/platform/meson/ao-cec-g12a.c    | 33 +++++++++++++++++++
 3 files changed, 38 insertions(+)

-- 
2.17.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v2 1/3] media: dt-bindings: media: meson-ao-cec: Add support of ao-sysctrl syscon

[Guillaume La Roque]

ao-sysctrl syscon phandle property is needed for CEC wakeup support.
This property is optional.

Tested-by: Kevin Hilman <khilman@baylibre.com>
Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
---
 .../devicetree/bindings/media/amlogic,meson-gx-ao-cec.yaml    | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/amlogic,meson-gx-ao-cec.yaml b/Documentation/devicetree/bindings/media/amlogic,meson-gx-ao-cec.yaml
index 41197578f19a..1f721498d249 100644
--- a/Documentation/devicetree/bindings/media/amlogic,meson-gx-ao-cec.yaml
+++ b/Documentation/devicetree/bindings/media/amlogic,meson-gx-ao-cec.yaml
@@ -32,6 +32,10 @@ properties:
     allOf:
       - $ref: /schemas/types.yaml#/definitions/phandle
 
+  amlogic,ao-sysctrl:
+    description: phandle to the ao-sysctrl syscon
+    $ref: '/schemas/types.yaml#/definitions/phandle'
+
 allOf:
   - if:
       properties:
-- 
2.17.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2 1/3] media: dt-bindings: media: meson-ao-cec: Add support of ao-sysctrl syscon

[Rob Herring]

On Fri, 13 Dec 2019 14:29:54 +0100, Guillaume La Roque wrote:
> ao-sysctrl syscon phandle property is needed for CEC wakeup support.
> This property is optional.
> 
> Tested-by: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
> ---
>  .../devicetree/bindings/media/amlogic,meson-gx-ao-cec.yaml    | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v2 2/3] arm64: dts: meson-g12g12: add syscon phandle in cec node

[Guillaume La Roque]

add syscon phandle in cec node to activate wakeup support

Tested-by: Kevin Hilman <khilman@baylibre.com>
Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index 7fabc8d9654a..98c6a1d1d035 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -1899,6 +1899,7 @@
 				interrupts = <GIC_SPI 203 IRQ_TYPE_EDGE_RISING>;
 				clocks = <&clkc_AO CLKID_AO_CTS_OSCIN>;
 				clock-names = "oscin";
+				amlogic,ao-sysctrl = <&rti>;
 				status = "disabled";
 			};
 
-- 
2.17.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Guillaume La Roque]

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

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

[Neil Armstrong]

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

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

[Hans Verkuil]

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

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

[guillaume La Roque]

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

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

[Hans Verkuil]

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

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

[guillaume La Roque]

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