[Linux-kernel-mentees] [PATCH] cec-follower: add tuner analog service emulation

[Linux-kernel-mentees] [PATCH] cec-follower: add tuner analog service emulation

[c0d1n61at3]

Implement the following tuner control features:
 - <Select Analogue Service>
 - <Give Tuner Device Status> and reply <Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---
 utils/cec-follower/cec-tuner.cpp | 51 ++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index 912adcb9..ed198ac8 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -87,6 +87,28 @@ static unsigned int analog_freqs_khz[3][9][3] =
 	}
 };
 
+static struct cec_op_tuner_device_info tuner_dev_info = {};
+
+static void set_analog_channel_freq(const unsigned int ana_freq_khz) {
+	tuner_dev_info.analog.ana_freq = (ana_freq_khz * 10) / 625;
+}
+
+static bool set_analog_tuner_dev_info(const struct cec_msg *msg)
+{
+	tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	tuner_dev_info.is_analog = true;
+	cec_ops_select_analogue_service(msg,
+					&tuner_dev_info.analog.ana_bcast_type,
+					&tuner_dev_info.analog.ana_freq,
+					&tuner_dev_info.analog.bcast_system);
+	if (tuner_dev_info.analog.ana_bcast_type > 2 ||
+	    tuner_dev_info.analog.bcast_system > 8)
+		return false;
+	set_analog_channel_freq(analog_freqs_khz[1][tuner_dev_info.analog.bcast_system][tuner_dev_info.analog.ana_bcast_type]);
+	return true;
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	switch (msg.msg[1]) {
@@ -101,29 +123,34 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
 		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
-
 	case CEC_MSG_TUNER_DEVICE_STATUS:
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			msg.msg[1] = CEC_MSG_FEATURE_ABORT;
+			msg.msg[2] = CEC_OP_ABORT_REFUSED;
+			transmit(node, &msg);
+			return;
+		}
+		if (!set_analog_tuner_dev_info(&msg)) {
+			msg.msg[1] = CEC_MSG_FEATURE_ABORT;
+			msg.msg[2] = CEC_OP_ABORT_INVALID_OP;
+			transmit(node, &msg);
+			return;
+		}
+		return;
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH] cec-follower: add tuner analog service emulation

[Jiunn Chang]

Implement the following tuner control features:
 - <Select Analogue Service>
 - <Give Tuner Device Status> and reply <Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---
 utils/cec-follower/cec-tuner.cpp | 51 ++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index 912adcb9..ed198ac8 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -87,6 +87,28 @@ static unsigned int analog_freqs_khz[3][9][3] =
 	}
 };
 
+static struct cec_op_tuner_device_info tuner_dev_info = {};
+
+static void set_analog_channel_freq(const unsigned int ana_freq_khz) {
+	tuner_dev_info.analog.ana_freq = (ana_freq_khz * 10) / 625;
+}
+
+static bool set_analog_tuner_dev_info(const struct cec_msg *msg)
+{
+	tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	tuner_dev_info.is_analog = true;
+	cec_ops_select_analogue_service(msg,
+					&tuner_dev_info.analog.ana_bcast_type,
+					&tuner_dev_info.analog.ana_freq,
+					&tuner_dev_info.analog.bcast_system);
+	if (tuner_dev_info.analog.ana_bcast_type > 2 ||
+	    tuner_dev_info.analog.bcast_system > 8)
+		return false;
+	set_analog_channel_freq(analog_freqs_khz[1][tuner_dev_info.analog.bcast_system][tuner_dev_info.analog.ana_bcast_type]);
+	return true;
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	switch (msg.msg[1]) {
@@ -101,29 +123,34 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
 		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
-
 	case CEC_MSG_TUNER_DEVICE_STATUS:
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			msg.msg[1] = CEC_MSG_FEATURE_ABORT;
+			msg.msg[2] = CEC_OP_ABORT_REFUSED;
+			transmit(node, &msg);
+			return;
+		}
+		if (!set_analog_tuner_dev_info(&msg)) {
+			msg.msg[1] = CEC_MSG_FEATURE_ABORT;
+			msg.msg[2] = CEC_OP_ABORT_INVALID_OP;
+			transmit(node, &msg);
+			return;
+		}
+		return;
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH] cec-follower: add tuner analog service emulation

[hverkuil]

On 9/17/19 11:43 AM, Jiunn Chang wrote:
> Implement the following tuner control features:
>  - <Select Analogue Service>
>  - <Give Tuner Device Status> and reply <Tuner Device Status>
> 
> Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
> ---
>  utils/cec-follower/cec-tuner.cpp | 51 ++++++++++++++++++++++++--------
>  1 file changed, 39 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index 912adcb9..ed198ac8 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -87,6 +87,28 @@ static unsigned int analog_freqs_khz[3][9][3] =
>  	}
>  };
>  
> +static struct cec_op_tuner_device_info tuner_dev_info = {};

This shouldn't be a global, instead add it to struct state (cec-follower.h).
That way the complete state of the follower is in a single place.

> +
> +static void set_analog_channel_freq(const unsigned int ana_freq_khz) {

It is never needed to add const to an int argument.

And { should be on the next line.

> +	tuner_dev_info.analog.ana_freq = (ana_freq_khz * 10) / 625;
> +}
> +
> +static bool set_analog_tuner_dev_info(const struct cec_msg *msg)
> +{
> +	tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> +	tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
> +	tuner_dev_info.is_analog = true;
> +	cec_ops_select_analogue_service(msg,
> +					&tuner_dev_info.analog.ana_bcast_type,
> +					&tuner_dev_info.analog.ana_freq,
> +					&tuner_dev_info.analog.bcast_system);
> +	if (tuner_dev_info.analog.ana_bcast_type > 2 ||
> +	    tuner_dev_info.analog.bcast_system > 8)
> +		return false;
> +	set_analog_channel_freq(analog_freqs_khz[1][tuner_dev_info.analog.bcast_system][tuner_dev_info.analog.ana_bcast_type]);

Shouldn't this be

set_analog_channel_freq(analog_freqs_khz[tuner_dev_info.analog.bcast_type][tuner_dev_info.analog.ana_bcast_type][1]);

Rather than using the middle frequency, find the frequency that's closest to
the requested frequency.

Update tuner_dev_info.analog.ana_freq with that new frequency so that this is stored
in the state.

> +	return true;
> +}
> +
>  void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
>  {
>  	switch (msg.msg[1]) {
> @@ -101,29 +123,34 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		*/
>  
>  	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
> -		if (!cec_has_tuner(1 << me))
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
>  			break;
>  
> -		struct cec_op_tuner_device_info tuner_dev_info = {};
> -
>  		cec_msg_set_reply_to(&msg, &msg);
> -		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> -		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
> -		tuner_dev_info.is_analog = false;
> -		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
> -		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
> -		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
> -		tuner_dev_info.digital.channel.minor = 1;
> -
>  		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
>  		transmit(node, &msg);
>  		return;
>  	}
> -

Spurious line deletion.

>  	case CEC_MSG_TUNER_DEVICE_STATUS:
>  		return;
>  
>  	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
> +			break;
> +
> +		if (tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
> +			msg.msg[1] = CEC_MSG_FEATURE_ABORT;
> +			msg.msg[2] = CEC_OP_ABORT_REFUSED;
> +			transmit(node, &msg);

Use the reply_feature_abort() function from cec-processing.cpp for this.

> +			return;
> +		}
> +		if (!set_analog_tuner_dev_info(&msg)) {
> +			msg.msg[1] = CEC_MSG_FEATURE_ABORT;
> +			msg.msg[2] = CEC_OP_ABORT_INVALID_OP;
> +			transmit(node, &msg);
> +			return;
> +		}
> +		return;

Add a newline here.

>  	case CEC_MSG_SELECT_DIGITAL_SERVICE:
>  	case CEC_MSG_TUNER_STEP_DECREMENT:
>  	case CEC_MSG_TUNER_STEP_INCREMENT:
> 

Regards,

	Hans

[Linux-kernel-mentees] [PATCH] cec-follower: add tuner analog service emulation

[Hans Verkuil]

On 9/17/19 11:43 AM, Jiunn Chang wrote:
> Implement the following tuner control features:
>  - <Select Analogue Service>
>  - <Give Tuner Device Status> and reply <Tuner Device Status>
> 
> Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
> ---
>  utils/cec-follower/cec-tuner.cpp | 51 ++++++++++++++++++++++++--------
>  1 file changed, 39 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index 912adcb9..ed198ac8 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -87,6 +87,28 @@ static unsigned int analog_freqs_khz[3][9][3] =
>  	}
>  };
>  
> +static struct cec_op_tuner_device_info tuner_dev_info = {};

This shouldn't be a global, instead add it to struct state (cec-follower.h).
That way the complete state of the follower is in a single place.

> +
> +static void set_analog_channel_freq(const unsigned int ana_freq_khz) {

It is never needed to add const to an int argument.

And { should be on the next line.

> +	tuner_dev_info.analog.ana_freq = (ana_freq_khz * 10) / 625;
> +}
> +
> +static bool set_analog_tuner_dev_info(const struct cec_msg *msg)
> +{
> +	tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> +	tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
> +	tuner_dev_info.is_analog = true;
> +	cec_ops_select_analogue_service(msg,
> +					&tuner_dev_info.analog.ana_bcast_type,
> +					&tuner_dev_info.analog.ana_freq,
> +					&tuner_dev_info.analog.bcast_system);
> +	if (tuner_dev_info.analog.ana_bcast_type > 2 ||
> +	    tuner_dev_info.analog.bcast_system > 8)
> +		return false;
> +	set_analog_channel_freq(analog_freqs_khz[1][tuner_dev_info.analog.bcast_system][tuner_dev_info.analog.ana_bcast_type]);

Shouldn't this be

set_analog_channel_freq(analog_freqs_khz[tuner_dev_info.analog.bcast_type][tuner_dev_info.analog.ana_bcast_type][1]);

Rather than using the middle frequency, find the frequency that's closest to
the requested frequency.

Update tuner_dev_info.analog.ana_freq with that new frequency so that this is stored
in the state.

> +	return true;
> +}
> +
>  void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
>  {
>  	switch (msg.msg[1]) {
> @@ -101,29 +123,34 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		*/
>  
>  	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
> -		if (!cec_has_tuner(1 << me))
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
>  			break;
>  
> -		struct cec_op_tuner_device_info tuner_dev_info = {};
> -
>  		cec_msg_set_reply_to(&msg, &msg);
> -		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> -		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
> -		tuner_dev_info.is_analog = false;
> -		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
> -		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
> -		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
> -		tuner_dev_info.digital.channel.minor = 1;
> -
>  		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
>  		transmit(node, &msg);
>  		return;
>  	}
> -

Spurious line deletion.

>  	case CEC_MSG_TUNER_DEVICE_STATUS:
>  		return;
>  
>  	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
> +			break;
> +
> +		if (tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
> +			msg.msg[1] = CEC_MSG_FEATURE_ABORT;
> +			msg.msg[2] = CEC_OP_ABORT_REFUSED;
> +			transmit(node, &msg);

Use the reply_feature_abort() function from cec-processing.cpp for this.

> +			return;
> +		}
> +		if (!set_analog_tuner_dev_info(&msg)) {
> +			msg.msg[1] = CEC_MSG_FEATURE_ABORT;
> +			msg.msg[2] = CEC_OP_ABORT_INVALID_OP;
> +			transmit(node, &msg);
> +			return;
> +		}
> +		return;

Add a newline here.

>  	case CEC_MSG_SELECT_DIGITAL_SERVICE:
>  	case CEC_MSG_TUNER_STEP_DECREMENT:
>  	case CEC_MSG_TUNER_STEP_INCREMENT:
> 

Regards,

	Hans

[Linux-kernel-mentees] [PATCH v2] cec-follower: add tuner analog service emulation

[c0d1n61at3]

Implement the following tuner control features:
 - <Select Analogue Service>
 - <Give Tuner Device Status> and reply <Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---
 utils/cec-follower/cec-follower.h |  1 +
 utils/cec-follower/cec-tuner.cpp  | 89 ++++++++++++++++++++++++++-----
 2 files changed, 78 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 9f5f1be4..9c146be1 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index 912adcb9..ffd1776f 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -87,6 +88,67 @@ static unsigned int analog_freqs_khz[3][9][3] =
 	}
 };
 
+static struct state state;
+
+static void set_analog_channel_freq(__u16 *ana_freq)
+{
+	unsigned int ana_freq_khz = (*ana_freq * 625) / 10;
+	unsigned int current = analog_freqs_khz[state.tuner_dev_info.analog.bcast_system][state.tuner_dev_info.analog.ana_bcast_type][0];
+	for (int i = 0; i < 3; i++) {
+		if (abs(int(ana_freq_khz - analog_freqs_khz[state.tuner_dev_info.analog.bcast_system][state.tuner_dev_info.analog.ana_bcast_type][i])) <
+		    abs(int(ana_freq_khz - current))) {
+			current = analog_freqs_khz[state.tuner_dev_info.analog.bcast_system][state.tuner_dev_info.analog.ana_bcast_type][i];
+		}
+	}
+	state.tuner_dev_info.analog.ana_freq = (current * 10) / 625;
+}
+
+static bool set_analog_tuner_dev_info(const struct cec_msg *msg)
+{
+	state.tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	state.tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	state.tuner_dev_info.is_analog = true;
+	cec_ops_select_analogue_service(msg,
+					&state.tuner_dev_info.analog.ana_bcast_type,
+					&state.tuner_dev_info.analog.ana_freq,
+					&state.tuner_dev_info.analog.bcast_system);
+	if (state.tuner_dev_info.analog.ana_bcast_type > 2 ||
+	    state.tuner_dev_info.analog.bcast_system > 8)
+		return false;
+	set_analog_channel_freq(&state.tuner_dev_info.analog.ana_freq);
+	return true;
+}
+
+static void reply_feature_abort(struct node *node, struct cec_msg *msg, __u8 reason = CEC_OP_ABORT_UNRECOGNIZED_OP)
+{
+	unsigned la = cec_msg_initiator(msg);
+	__u8 opcode = cec_msg_opcode(msg);
+	__u64 ts_now = get_ts();
+
+	if (cec_msg_is_broadcast(msg) || cec_msg_initiator(msg) == CEC_LOG_ADDR_UNREGISTERED)
+		return;
+	if (reason == CEC_OP_ABORT_UNRECOGNIZED_OP) {
+		la_info[la].feature_aborted[opcode].count++;
+		if (la_info[la].feature_aborted[opcode].count == 2) {
+			/* If the Abort Reason was "Unrecognized opcode", the Initiator should not send
+			   the same message to the same Follower again at that time to avoid saturating
+			   the bus. */
+			warn("Received message %s from LA %d (%s) shortly after\n",
+				opcode2s(msg).c_str(), la, la2s(la));
+			warn("replying Feature Abort [Unrecognized Opcode] to the same message.\n");
+		}
+	}
+	else if (la_info[la].feature_aborted[opcode].count) {
+		warn("Replying Feature Abort with abort reason different than [Unrecognized Opcode]\n");
+		warn("to message that has previously been replied Feature Abort to with [Unrecognized Opcode].\n");
+	}
+	else
+		la_info[la].feature_aborted[opcode].ts = ts_now;
+
+	cec_msg_reply_feature_abort(msg, reason);
+	transmit(node, msg);
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	switch (msg.msg[1]) {
@@ -101,21 +163,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -124,6 +176,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!set_analog_tuner_dev_info(&msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v2] cec-follower: add tuner analog service emulation

[Jiunn Chang]

Implement the following tuner control features:
 - <Select Analogue Service>
 - <Give Tuner Device Status> and reply <Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---
 utils/cec-follower/cec-follower.h |  1 +
 utils/cec-follower/cec-tuner.cpp  | 89 ++++++++++++++++++++++++++-----
 2 files changed, 78 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 9f5f1be4..9c146be1 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index 912adcb9..ffd1776f 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -87,6 +88,67 @@ static unsigned int analog_freqs_khz[3][9][3] =
 	}
 };
 
+static struct state state;
+
+static void set_analog_channel_freq(__u16 *ana_freq)
+{
+	unsigned int ana_freq_khz = (*ana_freq * 625) / 10;
+	unsigned int current = analog_freqs_khz[state.tuner_dev_info.analog.bcast_system][state.tuner_dev_info.analog.ana_bcast_type][0];
+	for (int i = 0; i < 3; i++) {
+		if (abs(int(ana_freq_khz - analog_freqs_khz[state.tuner_dev_info.analog.bcast_system][state.tuner_dev_info.analog.ana_bcast_type][i])) <
+		    abs(int(ana_freq_khz - current))) {
+			current = analog_freqs_khz[state.tuner_dev_info.analog.bcast_system][state.tuner_dev_info.analog.ana_bcast_type][i];
+		}
+	}
+	state.tuner_dev_info.analog.ana_freq = (current * 10) / 625;
+}
+
+static bool set_analog_tuner_dev_info(const struct cec_msg *msg)
+{
+	state.tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	state.tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	state.tuner_dev_info.is_analog = true;
+	cec_ops_select_analogue_service(msg,
+					&state.tuner_dev_info.analog.ana_bcast_type,
+					&state.tuner_dev_info.analog.ana_freq,
+					&state.tuner_dev_info.analog.bcast_system);
+	if (state.tuner_dev_info.analog.ana_bcast_type > 2 ||
+	    state.tuner_dev_info.analog.bcast_system > 8)
+		return false;
+	set_analog_channel_freq(&state.tuner_dev_info.analog.ana_freq);
+	return true;
+}
+
+static void reply_feature_abort(struct node *node, struct cec_msg *msg, __u8 reason = CEC_OP_ABORT_UNRECOGNIZED_OP)
+{
+	unsigned la = cec_msg_initiator(msg);
+	__u8 opcode = cec_msg_opcode(msg);
+	__u64 ts_now = get_ts();
+
+	if (cec_msg_is_broadcast(msg) || cec_msg_initiator(msg) == CEC_LOG_ADDR_UNREGISTERED)
+		return;
+	if (reason == CEC_OP_ABORT_UNRECOGNIZED_OP) {
+		la_info[la].feature_aborted[opcode].count++;
+		if (la_info[la].feature_aborted[opcode].count == 2) {
+			/* If the Abort Reason was "Unrecognized opcode", the Initiator should not send
+			   the same message to the same Follower again at that time to avoid saturating
+			   the bus. */
+			warn("Received message %s from LA %d (%s) shortly after\n",
+				opcode2s(msg).c_str(), la, la2s(la));
+			warn("replying Feature Abort [Unrecognized Opcode] to the same message.\n");
+		}
+	}
+	else if (la_info[la].feature_aborted[opcode].count) {
+		warn("Replying Feature Abort with abort reason different than [Unrecognized Opcode]\n");
+		warn("to message that has previously been replied Feature Abort to with [Unrecognized Opcode].\n");
+	}
+	else
+		la_info[la].feature_aborted[opcode].ts = ts_now;
+
+	cec_msg_reply_feature_abort(msg, reason);
+	transmit(node, msg);
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	switch (msg.msg[1]) {
@@ -101,21 +163,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -124,6 +176,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!set_analog_tuner_dev_info(&msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v3] cec-follower: add tuner analog service emulation

[c0d1n61at3]

Implement the following tuner control features:
  - <Select Analogue Service>
  - <Give Tuner Device Status> and reply <Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---

Changes made since v1:
  - Fix typos/bugs
  - Import reply_feature_abort() from cec-processing.cpp
  - Add functionality to choose nearest frequency

Changes made since v2:
  - Fix typos/bugs
  - Use state from node in cec-follower.h
  - Rename functions to analog_ prefix

---
 utils/cec-follower/cec-follower.h |  1 +
 utils/cec-follower/cec-tuner.cpp  | 87 ++++++++++++++++++++++++++-----
 2 files changed, 76 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 9f5f1be4..9c146be1 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index 912adcb9..1b2c0608 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -87,6 +88,65 @@ static unsigned int analog_freqs_khz[3][9][3] =
 	}
 };
 
+static void analog_set_tuner_chan_freq(struct node *node)
+{
+	unsigned int ana_freq_khz = (node->state.tuner_dev_info.analog.ana_freq * 625) / 10;
+	unsigned int nearest = analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][0];
+	for (int i = 0; i < 3; i++) {
+		if (abs(int(ana_freq_khz - analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][i])) <
+		    abs(int(ana_freq_khz - nearest))) {
+			nearest = analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][i];
+		}
+	}
+	node->state.tuner_dev_info.analog.ana_freq = (nearest * 10) / 625;
+}
+
+static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
+{
+	node->state.tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	node->state.tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	node->state.tuner_dev_info.is_analog = true;
+	cec_ops_select_analogue_service(msg,
+					&node->state.tuner_dev_info.analog.ana_bcast_type,
+					&node->state.tuner_dev_info.analog.ana_freq,
+					&node->state.tuner_dev_info.analog.bcast_system);
+	if (node->state.tuner_dev_info.analog.ana_bcast_type > 2 ||
+	    node->state.tuner_dev_info.analog.bcast_system > 8)
+		return false;
+	analog_set_tuner_chan_freq(node);
+	return true;
+}
+
+static void reply_feature_abort(struct node *node, struct cec_msg *msg, __u8 reason = CEC_OP_ABORT_UNRECOGNIZED_OP)
+{
+	unsigned la = cec_msg_initiator(msg);
+	__u8 opcode = cec_msg_opcode(msg);
+	__u64 ts_now = get_ts();
+
+	if (cec_msg_is_broadcast(msg) || cec_msg_initiator(msg) == CEC_LOG_ADDR_UNREGISTERED)
+		return;
+	if (reason == CEC_OP_ABORT_UNRECOGNIZED_OP) {
+		la_info[la].feature_aborted[opcode].count++;
+		if (la_info[la].feature_aborted[opcode].count == 2) {
+			/* If the Abort Reason was "Unrecognized opcode", the Initiator should not send
+			   the same message to the same Follower again at that time to avoid saturating
+			   the bus. */
+			warn("Received message %s from LA %d (%s) shortly after\n",
+				opcode2s(msg).c_str(), la, la2s(la));
+			warn("replying Feature Abort [Unrecognized Opcode] to the same message.\n");
+		}
+	}
+	else if (la_info[la].feature_aborted[opcode].count) {
+		warn("Replying Feature Abort with abort reason different than [Unrecognized Opcode]\n");
+		warn("to message that has previously been replied Feature Abort to with [Unrecognized Opcode].\n");
+	}
+	else
+		la_info[la].feature_aborted[opcode].ts = ts_now;
+
+	cec_msg_reply_feature_abort(msg, reason);
+	transmit(node, msg);
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	switch (msg.msg[1]) {
@@ -101,21 +161,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -124,6 +174,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!analog_set_tuner_dev_info(node, &msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v3] cec-follower: add tuner analog service emulation

[Jiunn Chang]

Implement the following tuner control features:
  - <Select Analogue Service>
  - <Give Tuner Device Status> and reply <Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---

Changes made since v1:
  - Fix typos/bugs
  - Import reply_feature_abort() from cec-processing.cpp
  - Add functionality to choose nearest frequency

Changes made since v2:
  - Fix typos/bugs
  - Use state from node in cec-follower.h
  - Rename functions to analog_ prefix

---
 utils/cec-follower/cec-follower.h |  1 +
 utils/cec-follower/cec-tuner.cpp  | 87 ++++++++++++++++++++++++++-----
 2 files changed, 76 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 9f5f1be4..9c146be1 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index 912adcb9..1b2c0608 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -87,6 +88,65 @@ static unsigned int analog_freqs_khz[3][9][3] =
 	}
 };
 
+static void analog_set_tuner_chan_freq(struct node *node)
+{
+	unsigned int ana_freq_khz = (node->state.tuner_dev_info.analog.ana_freq * 625) / 10;
+	unsigned int nearest = analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][0];
+	for (int i = 0; i < 3; i++) {
+		if (abs(int(ana_freq_khz - analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][i])) <
+		    abs(int(ana_freq_khz - nearest))) {
+			nearest = analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][i];
+		}
+	}
+	node->state.tuner_dev_info.analog.ana_freq = (nearest * 10) / 625;
+}
+
+static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
+{
+	node->state.tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	node->state.tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	node->state.tuner_dev_info.is_analog = true;
+	cec_ops_select_analogue_service(msg,
+					&node->state.tuner_dev_info.analog.ana_bcast_type,
+					&node->state.tuner_dev_info.analog.ana_freq,
+					&node->state.tuner_dev_info.analog.bcast_system);
+	if (node->state.tuner_dev_info.analog.ana_bcast_type > 2 ||
+	    node->state.tuner_dev_info.analog.bcast_system > 8)
+		return false;
+	analog_set_tuner_chan_freq(node);
+	return true;
+}
+
+static void reply_feature_abort(struct node *node, struct cec_msg *msg, __u8 reason = CEC_OP_ABORT_UNRECOGNIZED_OP)
+{
+	unsigned la = cec_msg_initiator(msg);
+	__u8 opcode = cec_msg_opcode(msg);
+	__u64 ts_now = get_ts();
+
+	if (cec_msg_is_broadcast(msg) || cec_msg_initiator(msg) == CEC_LOG_ADDR_UNREGISTERED)
+		return;
+	if (reason == CEC_OP_ABORT_UNRECOGNIZED_OP) {
+		la_info[la].feature_aborted[opcode].count++;
+		if (la_info[la].feature_aborted[opcode].count == 2) {
+			/* If the Abort Reason was "Unrecognized opcode", the Initiator should not send
+			   the same message to the same Follower again at that time to avoid saturating
+			   the bus. */
+			warn("Received message %s from LA %d (%s) shortly after\n",
+				opcode2s(msg).c_str(), la, la2s(la));
+			warn("replying Feature Abort [Unrecognized Opcode] to the same message.\n");
+		}
+	}
+	else if (la_info[la].feature_aborted[opcode].count) {
+		warn("Replying Feature Abort with abort reason different than [Unrecognized Opcode]\n");
+		warn("to message that has previously been replied Feature Abort to with [Unrecognized Opcode].\n");
+	}
+	else
+		la_info[la].feature_aborted[opcode].ts = ts_now;
+
+	cec_msg_reply_feature_abort(msg, reason);
+	transmit(node, msg);
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	switch (msg.msg[1]) {
@@ -101,21 +161,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -124,6 +174,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!analog_set_tuner_dev_info(node, &msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v4] cec-follower: add tuner analog service emulation

[c0d1n61at3]

Implement the following tuner control features:
  - <Select Analogue Service>
  - <Give Tuner Device Status> and reply <Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---

Changes made since v1:
  - Fix typos/bugs
  - Import reply_feature_abort() from cec-processing.cpp
  - Add functionality to choose nearest frequency

Changes made since v2:
  - Fix typos/bugs
  - Use state from node in cec-follower.h
  - Rename functions to analog_ prefix

Changes made since v3:
  - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp

---
 utils/cec-follower/cec-follower.h |  1 +
 utils/cec-follower/cec-tuner.cpp  | 57 ++++++++++++++++++++++++-------
 2 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 9f5f1be4..9c146be1 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index 2303e6bb..87c631e4 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -117,6 +118,35 @@ static void reply_feature_abort(struct node *node, struct cec_msg *msg, __u8 rea
 	transmit(node, msg);
 }
 
+static void analog_set_tuner_chan_freq(struct node *node)
+{
+	unsigned int ana_freq_khz = (node->state.tuner_dev_info.analog.ana_freq * 625) / 10;
+	unsigned int nearest = analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][0];
+	for (int i = 0; i < 3; i++) {
+		if (abs(int(ana_freq_khz - analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][i])) <
+		    abs(int(ana_freq_khz - nearest))) {
+			nearest = analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][i];
+		}
+	}
+	node->state.tuner_dev_info.analog.ana_freq = (nearest * 10) / 625;
+}
+
+static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
+{
+	node->state.tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	node->state.tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	node->state.tuner_dev_info.is_analog = true;
+	cec_ops_select_analogue_service(msg,
+					&node->state.tuner_dev_info.analog.ana_bcast_type,
+					&node->state.tuner_dev_info.analog.ana_freq,
+					&node->state.tuner_dev_info.analog.bcast_system);
+	if (node->state.tuner_dev_info.analog.ana_bcast_type > 2 ||
+	    node->state.tuner_dev_info.analog.bcast_system > 8)
+		return false;
+	analog_set_tuner_chan_freq(node);
+	return true;
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	bool is_bcast = cec_msg_is_broadcast(&msg);
@@ -133,21 +163,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -156,6 +176,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!analog_set_tuner_dev_info(node, &msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v4] cec-follower: add tuner analog service emulation

[Jiunn Chang]

Implement the following tuner control features:
  - <Select Analogue Service>
  - <Give Tuner Device Status> and reply <Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---

Changes made since v1:
  - Fix typos/bugs
  - Import reply_feature_abort() from cec-processing.cpp
  - Add functionality to choose nearest frequency

Changes made since v2:
  - Fix typos/bugs
  - Use state from node in cec-follower.h
  - Rename functions to analog_ prefix

Changes made since v3:
  - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp

---
 utils/cec-follower/cec-follower.h |  1 +
 utils/cec-follower/cec-tuner.cpp  | 57 ++++++++++++++++++++++++-------
 2 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 9f5f1be4..9c146be1 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index 2303e6bb..87c631e4 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -117,6 +118,35 @@ static void reply_feature_abort(struct node *node, struct cec_msg *msg, __u8 rea
 	transmit(node, msg);
 }
 
+static void analog_set_tuner_chan_freq(struct node *node)
+{
+	unsigned int ana_freq_khz = (node->state.tuner_dev_info.analog.ana_freq * 625) / 10;
+	unsigned int nearest = analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][0];
+	for (int i = 0; i < 3; i++) {
+		if (abs(int(ana_freq_khz - analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][i])) <
+		    abs(int(ana_freq_khz - nearest))) {
+			nearest = analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][i];
+		}
+	}
+	node->state.tuner_dev_info.analog.ana_freq = (nearest * 10) / 625;
+}
+
+static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
+{
+	node->state.tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	node->state.tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	node->state.tuner_dev_info.is_analog = true;
+	cec_ops_select_analogue_service(msg,
+					&node->state.tuner_dev_info.analog.ana_bcast_type,
+					&node->state.tuner_dev_info.analog.ana_freq,
+					&node->state.tuner_dev_info.analog.bcast_system);
+	if (node->state.tuner_dev_info.analog.ana_bcast_type > 2 ||
+	    node->state.tuner_dev_info.analog.bcast_system > 8)
+		return false;
+	analog_set_tuner_chan_freq(node);
+	return true;
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	bool is_bcast = cec_msg_is_broadcast(&msg);
@@ -133,21 +163,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -156,6 +176,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!analog_set_tuner_dev_info(node, &msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v4] cec-follower: add tuner analog service emulation

[hverkuil]

On 9/19/19 6:18 AM, Jiunn Chang wrote:
> Implement the following tuner control features:
>   - <Select Analogue Service>
>   - <Give Tuner Device Status> and reply <Tuner Device Status>
> 
> Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
> ---
> 
> Changes made since v1:
>   - Fix typos/bugs
>   - Import reply_feature_abort() from cec-processing.cpp
>   - Add functionality to choose nearest frequency
> 
> Changes made since v2:
>   - Fix typos/bugs
>   - Use state from node in cec-follower.h
>   - Rename functions to analog_ prefix
> 
> Changes made since v3:
>   - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp
> 
> ---
>  utils/cec-follower/cec-follower.h |  1 +
>  utils/cec-follower/cec-tuner.cpp  | 57 ++++++++++++++++++++++++-------
>  2 files changed, 46 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
> index 9f5f1be4..9c146be1 100644
> --- a/utils/cec-follower/cec-follower.h
> +++ b/utils/cec-follower/cec-follower.h
> @@ -51,6 +51,7 @@ struct state {
>  	__u64 rc_press_rx_ts;
>  	unsigned rc_press_hold_count;
>  	unsigned rc_duration_sum;
> +	struct cec_op_tuner_device_info tuner_dev_info;
>  };
>  
>  struct node {
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index 2303e6bb..87c631e4 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <sys/ioctl.h>
> +#include <stdlib.h>
>  
>  #include "cec-follower.h"
>  
> @@ -117,6 +118,35 @@ static void reply_feature_abort(struct node *node, struct cec_msg *msg, __u8 rea
>  	transmit(node, msg);
>  }
>  
> +static void analog_set_tuner_chan_freq(struct node *node)
> +{
> +	unsigned int ana_freq_khz = (node->state.tuner_dev_info.analog.ana_freq * 625) / 10;
> +	unsigned int nearest = analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][0];

I'd make both variable 'int', since that will avoid the int cast in the abs function.

Add newline after variable declarations.

Also make a temp variable like this:

	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;

and use that instead instead of repeating 'node->state.tuner_dev_info.' everywhere.
That's way too long.

> +	for (int i = 0; i < 3; i++) {

Rather than hardcoding '3' here, add a NUM_ANALOG_FREQS define and use that.

Of course, the analog_freqs_khz array declaration should use this new define as well.
This means a new version of 'cec-follower: create analog channel frequencies'.

I'd add a:
		int freq = analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][i];

and use that below.

> +		if (abs(int(ana_freq_khz - analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][i])) <
> +		    abs(int(ana_freq_khz - nearest))) {
> +			nearest = analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][i];
> +		}
> +	}
> +	node->state.tuner_dev_info.analog.ana_freq = (nearest * 10) / 625;
> +}
> +
> +static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
> +{
> +	node->state.tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> +	node->state.tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
> +	node->state.tuner_dev_info.is_analog = true;

Same here, use info pointer.

> +	cec_ops_select_analogue_service(msg,
> +					&node->state.tuner_dev_info.analog.ana_bcast_type,
> +					&node->state.tuner_dev_info.analog.ana_freq,
> +					&node->state.tuner_dev_info.analog.bcast_system);
> +	if (node->state.tuner_dev_info.analog.ana_bcast_type > 2 ||
> +	    node->state.tuner_dev_info.analog.bcast_system > 8)
> +		return false;
> +	analog_set_tuner_chan_freq(node);
> +	return true;
> +}
> +
>  void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
>  {
>  	bool is_bcast = cec_msg_is_broadcast(&msg);
> @@ -133,21 +163,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		*/
>  
>  	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
> -		if (!cec_has_tuner(1 << me))
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
>  			break;
>  
> -		struct cec_op_tuner_device_info tuner_dev_info = {};
> -
>  		cec_msg_set_reply_to(&msg, &msg);
> -		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> -		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
> -		tuner_dev_info.is_analog = false;
> -		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
> -		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
> -		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
> -		tuner_dev_info.digital.channel.minor = 1;
> -
> -		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
> +		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
>  		transmit(node, &msg);
>  		return;
>  	}
> @@ -156,6 +176,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		return;
>  
>  	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
> +			break;
> +
> +		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
> +			return;
> +		}
> +		if (!analog_set_tuner_dev_info(node, &msg)) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
> +			return;
> +		}
> +		return;
> +
>  	case CEC_MSG_SELECT_DIGITAL_SERVICE:
>  	case CEC_MSG_TUNER_STEP_DECREMENT:
>  	case CEC_MSG_TUNER_STEP_INCREMENT:
> 

Can you post a patch series next time? There are now three patches on top of one another,
it's easier to review if I see the whole set.

Regards,

	Hans

[Linux-kernel-mentees] [PATCH v4] cec-follower: add tuner analog service emulation

[Hans Verkuil]

On 9/19/19 6:18 AM, Jiunn Chang wrote:
> Implement the following tuner control features:
>   - <Select Analogue Service>
>   - <Give Tuner Device Status> and reply <Tuner Device Status>
> 
> Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
> ---
> 
> Changes made since v1:
>   - Fix typos/bugs
>   - Import reply_feature_abort() from cec-processing.cpp
>   - Add functionality to choose nearest frequency
> 
> Changes made since v2:
>   - Fix typos/bugs
>   - Use state from node in cec-follower.h
>   - Rename functions to analog_ prefix
> 
> Changes made since v3:
>   - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp
> 
> ---
>  utils/cec-follower/cec-follower.h |  1 +
>  utils/cec-follower/cec-tuner.cpp  | 57 ++++++++++++++++++++++++-------
>  2 files changed, 46 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
> index 9f5f1be4..9c146be1 100644
> --- a/utils/cec-follower/cec-follower.h
> +++ b/utils/cec-follower/cec-follower.h
> @@ -51,6 +51,7 @@ struct state {
>  	__u64 rc_press_rx_ts;
>  	unsigned rc_press_hold_count;
>  	unsigned rc_duration_sum;
> +	struct cec_op_tuner_device_info tuner_dev_info;
>  };
>  
>  struct node {
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index 2303e6bb..87c631e4 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <sys/ioctl.h>
> +#include <stdlib.h>
>  
>  #include "cec-follower.h"
>  
> @@ -117,6 +118,35 @@ static void reply_feature_abort(struct node *node, struct cec_msg *msg, __u8 rea
>  	transmit(node, msg);
>  }
>  
> +static void analog_set_tuner_chan_freq(struct node *node)
> +{
> +	unsigned int ana_freq_khz = (node->state.tuner_dev_info.analog.ana_freq * 625) / 10;
> +	unsigned int nearest = analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][0];

I'd make both variable 'int', since that will avoid the int cast in the abs function.

Add newline after variable declarations.

Also make a temp variable like this:

	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;

and use that instead instead of repeating 'node->state.tuner_dev_info.' everywhere.
That's way too long.

> +	for (int i = 0; i < 3; i++) {

Rather than hardcoding '3' here, add a NUM_ANALOG_FREQS define and use that.

Of course, the analog_freqs_khz array declaration should use this new define as well.
This means a new version of 'cec-follower: create analog channel frequencies'.

I'd add a:
		int freq = analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][i];

and use that below.

> +		if (abs(int(ana_freq_khz - analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][i])) <
> +		    abs(int(ana_freq_khz - nearest))) {
> +			nearest = analog_freqs_khz[node->state.tuner_dev_info.analog.ana_bcast_type][node->state.tuner_dev_info.analog.bcast_system][i];
> +		}
> +	}
> +	node->state.tuner_dev_info.analog.ana_freq = (nearest * 10) / 625;
> +}
> +
> +static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
> +{
> +	node->state.tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> +	node->state.tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
> +	node->state.tuner_dev_info.is_analog = true;

Same here, use info pointer.

> +	cec_ops_select_analogue_service(msg,
> +					&node->state.tuner_dev_info.analog.ana_bcast_type,
> +					&node->state.tuner_dev_info.analog.ana_freq,
> +					&node->state.tuner_dev_info.analog.bcast_system);
> +	if (node->state.tuner_dev_info.analog.ana_bcast_type > 2 ||
> +	    node->state.tuner_dev_info.analog.bcast_system > 8)
> +		return false;
> +	analog_set_tuner_chan_freq(node);
> +	return true;
> +}
> +
>  void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
>  {
>  	bool is_bcast = cec_msg_is_broadcast(&msg);
> @@ -133,21 +163,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		*/
>  
>  	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
> -		if (!cec_has_tuner(1 << me))
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
>  			break;
>  
> -		struct cec_op_tuner_device_info tuner_dev_info = {};
> -
>  		cec_msg_set_reply_to(&msg, &msg);
> -		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> -		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
> -		tuner_dev_info.is_analog = false;
> -		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
> -		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
> -		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
> -		tuner_dev_info.digital.channel.minor = 1;
> -
> -		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
> +		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
>  		transmit(node, &msg);
>  		return;
>  	}
> @@ -156,6 +176,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		return;
>  
>  	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
> +			break;
> +
> +		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
> +			return;
> +		}
> +		if (!analog_set_tuner_dev_info(node, &msg)) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
> +			return;
> +		}
> +		return;
> +
>  	case CEC_MSG_SELECT_DIGITAL_SERVICE:
>  	case CEC_MSG_TUNER_STEP_DECREMENT:
>  	case CEC_MSG_TUNER_STEP_INCREMENT:
> 

Can you post a patch series next time? There are now three patches on top of one another,
it's easier to review if I see the whole set.

Regards,

	Hans

[Linux-kernel-mentees] [PATCH v5] cec-follower: add tuner analog service emulation

[c0d1n61at3]

Implement the following tuner control features:
  - <Select Analogue Service>
  - <Give Tuner Device Status> and reply <Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---

Changes made since v1:
  - Fix typos/bugs
  - Import reply_feature_abort() from cec-processing.cpp
  - Add functionality to choose nearest frequency

Changes made since v2:
  - Fix typos/bugs
  - Use state from node in cec-follower.h
  - Rename functions to analog_ prefix

Changes made since v3:
  - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp

Changes made since v4:
  - Remove int casting in abs()
  - Add temp variables: info, freq to make code easier to read
  - Use NUM_ANALOG_FREQS macro

---
 utils/cec-follower/cec-follower.h |  1 +
 utils/cec-follower/cec-tuner.cpp  | 62 +++++++++++++++++++++++++------
 2 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 03b52217..91b9e037 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index bdeda342..87425083 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -89,6 +90,40 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
 	}
 };
 
+static void analog_set_tuner_chan_freq(struct node *node)
+{
+	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
+	int ana_freq_khz = (info->analog.ana_freq * 625) / 10;
+	int nearest = analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][0];
+
+	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
+		int freq = analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][i];
+
+		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest)) {
+			nearest = freq;
+		}
+	}
+	info->analog.ana_freq = (nearest * 10) / 625;
+}
+
+static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
+{
+	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
+
+	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	info->is_analog = true;
+	cec_ops_select_analogue_service(msg,
+					&info->analog.ana_bcast_type,
+					&info->analog.ana_freq,
+					&info->analog.bcast_system);
+	if (info->analog.ana_bcast_type > 2 ||
+	    info->analog.bcast_system > 8)
+		return false;
+	analog_set_tuner_chan_freq(node);
+	return true;
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	bool is_bcast = cec_msg_is_broadcast(&msg);
@@ -105,21 +140,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -128,6 +153,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!analog_set_tuner_dev_info(node, &msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v5] cec-follower: add tuner analog service emulation

[Jiunn Chang]

Implement the following tuner control features:
  - <Select Analogue Service>
  - <Give Tuner Device Status> and reply <Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---

Changes made since v1:
  - Fix typos/bugs
  - Import reply_feature_abort() from cec-processing.cpp
  - Add functionality to choose nearest frequency

Changes made since v2:
  - Fix typos/bugs
  - Use state from node in cec-follower.h
  - Rename functions to analog_ prefix

Changes made since v3:
  - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp

Changes made since v4:
  - Remove int casting in abs()
  - Add temp variables: info, freq to make code easier to read
  - Use NUM_ANALOG_FREQS macro

---
 utils/cec-follower/cec-follower.h |  1 +
 utils/cec-follower/cec-tuner.cpp  | 62 +++++++++++++++++++++++++------
 2 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 03b52217..91b9e037 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index bdeda342..87425083 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -89,6 +90,40 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
 	}
 };
 
+static void analog_set_tuner_chan_freq(struct node *node)
+{
+	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
+	int ana_freq_khz = (info->analog.ana_freq * 625) / 10;
+	int nearest = analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][0];
+
+	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
+		int freq = analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][i];
+
+		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest)) {
+			nearest = freq;
+		}
+	}
+	info->analog.ana_freq = (nearest * 10) / 625;
+}
+
+static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
+{
+	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
+
+	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	info->is_analog = true;
+	cec_ops_select_analogue_service(msg,
+					&info->analog.ana_bcast_type,
+					&info->analog.ana_freq,
+					&info->analog.bcast_system);
+	if (info->analog.ana_bcast_type > 2 ||
+	    info->analog.bcast_system > 8)
+		return false;
+	analog_set_tuner_chan_freq(node);
+	return true;
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	bool is_bcast = cec_msg_is_broadcast(&msg);
@@ -105,21 +140,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -128,6 +153,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!analog_set_tuner_dev_info(node, &msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v5] cec-follower: add tuner analog service emulation

[hverkuil]

Hi Jiunn,

This is a lot better. Just a few more comments:

On 9/19/19 7:47 PM, Jiunn Chang wrote:
> Implement the following tuner control features:
>   - <Select Analogue Service>
>   - <Give Tuner Device Status> and reply <Tuner Device Status>
> 
> Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
> ---
> 
> Changes made since v1:
>   - Fix typos/bugs
>   - Import reply_feature_abort() from cec-processing.cpp
>   - Add functionality to choose nearest frequency
> 
> Changes made since v2:
>   - Fix typos/bugs
>   - Use state from node in cec-follower.h
>   - Rename functions to analog_ prefix
> 
> Changes made since v3:
>   - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp
> 
> Changes made since v4:
>   - Remove int casting in abs()
>   - Add temp variables: info, freq to make code easier to read
>   - Use NUM_ANALOG_FREQS macro
> 
> ---
>  utils/cec-follower/cec-follower.h |  1 +
>  utils/cec-follower/cec-tuner.cpp  | 62 +++++++++++++++++++++++++------
>  2 files changed, 51 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
> index 03b52217..91b9e037 100644
> --- a/utils/cec-follower/cec-follower.h
> +++ b/utils/cec-follower/cec-follower.h
> @@ -51,6 +51,7 @@ struct state {
>  	__u64 rc_press_rx_ts;
>  	unsigned rc_press_hold_count;
>  	unsigned rc_duration_sum;
> +	struct cec_op_tuner_device_info tuner_dev_info;
>  };
>  
>  struct node {
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index bdeda342..87425083 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <sys/ioctl.h>
> +#include <stdlib.h>
>  
>  #include "cec-follower.h"
>  
> @@ -89,6 +90,40 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
>  	}
>  };
>  
> +static void analog_set_tuner_chan_freq(struct node *node)
> +{
> +	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
> +	int ana_freq_khz = (info->analog.ana_freq * 625) / 10;
> +	int nearest = analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][0];
> +
> +	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
> +		int freq = analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][i];
> +
> +		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest)) {
> +			nearest = freq;
> +		}

You can drop the { } as per the kernel coding style.

> +	}
> +	info->analog.ana_freq = (nearest * 10) / 625;
> +}
> +
> +static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
> +{
> +	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
> +
> +	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> +	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
> +	info->is_analog = true;
> +	cec_ops_select_analogue_service(msg,
> +					&info->analog.ana_bcast_type,
> +					&info->analog.ana_freq,
> +					&info->analog.bcast_system);
> +	if (info->analog.ana_bcast_type > 2 ||
> +	    info->analog.bcast_system > 8)
> +		return false;

cec_ops_select_analogue_service should store the results in temp variables first
and this function should only change node->state.tuner_dev_info if the test above
succeeds. Otherwise you would change the internal state to invalid values.

I also think that analog_set_tuner_chan_freq() shouldn't set info->analog.ana_freq.
That really should happen in this function as that keeps the code that updates
node->state.tuner_dev_info in a single place. Instead that function should just
find the nearest valid frequency, so the prototype becomes something like this:

static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
					    int ana_freq_khz);

This will be needed for e.g. timer/record support as well.


> +	analog_set_tuner_chan_freq(node);
> +	return true;
> +}
> +
>  void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
>  {
>  	bool is_bcast = cec_msg_is_broadcast(&msg);
> @@ -105,21 +140,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		*/
>  
>  	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
> -		if (!cec_has_tuner(1 << me))
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
>  			break;
>  
> -		struct cec_op_tuner_device_info tuner_dev_info = {};
> -
>  		cec_msg_set_reply_to(&msg, &msg);
> -		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> -		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
> -		tuner_dev_info.is_analog = false;
> -		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
> -		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
> -		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
> -		tuner_dev_info.digital.channel.minor = 1;
> -
> -		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
> +		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
>  		transmit(node, &msg);
>  		return;
>  	}
> @@ -128,6 +153,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		return;
>  
>  	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
> +			break;
> +
> +		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
> +			return;
> +		}
> +		if (!analog_set_tuner_dev_info(node, &msg)) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
> +			return;
> +		}
> +		return;
> +
>  	case CEC_MSG_SELECT_DIGITAL_SERVICE:
>  	case CEC_MSG_TUNER_STEP_DECREMENT:
>  	case CEC_MSG_TUNER_STEP_INCREMENT:
> 

I think there is one more thing to do here: state_init() in cec-follower.cpp should
initialize node->state.tuner_dev_info with sane initial values.

It is probably best to have it call a analog_tuner_init() function in cec-tuner.cpp
since it will need to access the analog_freqs_khz table.

Regards,

	Hans

[Linux-kernel-mentees] [PATCH v5] cec-follower: add tuner analog service emulation

[Hans Verkuil]

Hi Jiunn,

This is a lot better. Just a few more comments:

On 9/19/19 7:47 PM, Jiunn Chang wrote:
> Implement the following tuner control features:
>   - <Select Analogue Service>
>   - <Give Tuner Device Status> and reply <Tuner Device Status>
> 
> Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
> ---
> 
> Changes made since v1:
>   - Fix typos/bugs
>   - Import reply_feature_abort() from cec-processing.cpp
>   - Add functionality to choose nearest frequency
> 
> Changes made since v2:
>   - Fix typos/bugs
>   - Use state from node in cec-follower.h
>   - Rename functions to analog_ prefix
> 
> Changes made since v3:
>   - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp
> 
> Changes made since v4:
>   - Remove int casting in abs()
>   - Add temp variables: info, freq to make code easier to read
>   - Use NUM_ANALOG_FREQS macro
> 
> ---
>  utils/cec-follower/cec-follower.h |  1 +
>  utils/cec-follower/cec-tuner.cpp  | 62 +++++++++++++++++++++++++------
>  2 files changed, 51 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
> index 03b52217..91b9e037 100644
> --- a/utils/cec-follower/cec-follower.h
> +++ b/utils/cec-follower/cec-follower.h
> @@ -51,6 +51,7 @@ struct state {
>  	__u64 rc_press_rx_ts;
>  	unsigned rc_press_hold_count;
>  	unsigned rc_duration_sum;
> +	struct cec_op_tuner_device_info tuner_dev_info;
>  };
>  
>  struct node {
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index bdeda342..87425083 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <sys/ioctl.h>
> +#include <stdlib.h>
>  
>  #include "cec-follower.h"
>  
> @@ -89,6 +90,40 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
>  	}
>  };
>  
> +static void analog_set_tuner_chan_freq(struct node *node)
> +{
> +	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
> +	int ana_freq_khz = (info->analog.ana_freq * 625) / 10;
> +	int nearest = analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][0];
> +
> +	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
> +		int freq = analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][i];
> +
> +		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest)) {
> +			nearest = freq;
> +		}

You can drop the { } as per the kernel coding style.

> +	}
> +	info->analog.ana_freq = (nearest * 10) / 625;
> +}
> +
> +static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
> +{
> +	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
> +
> +	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> +	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
> +	info->is_analog = true;
> +	cec_ops_select_analogue_service(msg,
> +					&info->analog.ana_bcast_type,
> +					&info->analog.ana_freq,
> +					&info->analog.bcast_system);
> +	if (info->analog.ana_bcast_type > 2 ||
> +	    info->analog.bcast_system > 8)
> +		return false;

cec_ops_select_analogue_service should store the results in temp variables first
and this function should only change node->state.tuner_dev_info if the test above
succeeds. Otherwise you would change the internal state to invalid values.

I also think that analog_set_tuner_chan_freq() shouldn't set info->analog.ana_freq.
That really should happen in this function as that keeps the code that updates
node->state.tuner_dev_info in a single place. Instead that function should just
find the nearest valid frequency, so the prototype becomes something like this:

static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
					    int ana_freq_khz);

This will be needed for e.g. timer/record support as well.


> +	analog_set_tuner_chan_freq(node);
> +	return true;
> +}
> +
>  void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
>  {
>  	bool is_bcast = cec_msg_is_broadcast(&msg);
> @@ -105,21 +140,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		*/
>  
>  	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
> -		if (!cec_has_tuner(1 << me))
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
>  			break;
>  
> -		struct cec_op_tuner_device_info tuner_dev_info = {};
> -
>  		cec_msg_set_reply_to(&msg, &msg);
> -		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> -		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
> -		tuner_dev_info.is_analog = false;
> -		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
> -		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
> -		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
> -		tuner_dev_info.digital.channel.minor = 1;
> -
> -		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
> +		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
>  		transmit(node, &msg);
>  		return;
>  	}
> @@ -128,6 +153,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		return;
>  
>  	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
> +			break;
> +
> +		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
> +			return;
> +		}
> +		if (!analog_set_tuner_dev_info(node, &msg)) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
> +			return;
> +		}
> +		return;
> +
>  	case CEC_MSG_SELECT_DIGITAL_SERVICE:
>  	case CEC_MSG_TUNER_STEP_DECREMENT:
>  	case CEC_MSG_TUNER_STEP_INCREMENT:
> 

I think there is one more thing to do here: state_init() in cec-follower.cpp should
initialize node->state.tuner_dev_info with sane initial values.

It is probably best to have it call a analog_tuner_init() function in cec-tuner.cpp
since it will need to access the analog_freqs_khz table.

Regards,

	Hans

[Linux-kernel-mentees] [PATCH v6] cec-follower: add tuner analog service emulation

[c0d1n61at3]

Implement the following tuner control features:
  - <Select Analogue Service>
  - <Give Tuner Device Status> and reply <Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---

Changes made since v1:
  - Fix typos/bugs
  - Import reply_feature_abort() from cec-processing.cpp
  - Add functionality to choose nearest frequency

Changes made since v2:
  - Fix typos/bugs
  - Use state from node in cec-follower.h
  - Rename functions to analog_ prefix

Changes made since v3:
  - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp

Changes made since v4:
  - Remove int casting in abs()
  - Add temp variables: info, freq to make code easier to read
  - Use NUM_ANALOG_FREQS macro

Changes made since v5:
  - Add analog_tuner_init() to cec-tuner.cpp
  - Change state_init() in cec-follower.cpp to use analog_tuner_init()
  - Rename function to analog_get_nearest_freq()
  - Change analog_get_nearest_freq() to only return freq in Khz
  - Change analog_set_tuner_dev_info() to set analog freq too

---
 utils/cec-follower/cec-follower.cpp |  1 +
 utils/cec-follower/cec-follower.h   |  2 +
 utils/cec-follower/cec-tuner.cpp    | 73 ++++++++++++++++++++++++-----
 3 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
index dca0f627..4243fdd9 100644
--- a/utils/cec-follower/cec-follower.cpp
+++ b/utils/cec-follower/cec-follower.cpp
@@ -298,6 +298,7 @@ void state_init(struct node &node)
 	node.state.sac_active = false;
 	node.state.volume = 50;
 	node.state.mute = false;
+	analog_tuner_init(&node.state.tuner_dev_info);
 }
 
 int main(int argc, char **argv)
diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 954e001f..a53c16fe 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
@@ -220,6 +221,7 @@ std::string opcode2s(const struct cec_msg *msg);
 void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
 
 // cec-tuner.cpp
+void analog_tuner_init(struct cec_op_tuner_device_info *tuner_dev_info);
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
 
 // CEC processing
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index bdeda342..68d945d5 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -89,6 +90,51 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
 	}
 };
 
+void analog_tuner_init(struct cec_op_tuner_device_info *info)
+{
+	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	info->is_analog = true;
+	info->analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
+	info->analog.ana_freq =
+		analog_freqs_khz[CEC_OP_ANA_BCAST_TYPE_CABLE][CEC_OP_BCAST_SYSTEM_PAL_BG][0];
+	info->analog.bcast_system = CEC_OP_BCAST_SYSTEM_PAL_BG;
+}
+
+static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
+                                            int ana_freq_khz)
+{
+	int nearest = analog_freqs_khz[ana_bcast_type][ana_bcast_system][0];
+
+	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
+		int freq = analog_freqs_khz[ana_bcast_type][ana_bcast_system][i];
+
+		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest))
+			nearest = freq;
+	}
+	return nearest;
+}
+
+static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
+{
+	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
+	__u8 type = 0;
+	__u16 freq = 0;
+	__u8 system = 0;
+
+	cec_ops_select_analogue_service(msg, &type, &freq, &system);
+	if (info->analog.ana_bcast_type < 3 && info->analog.bcast_system < 9) {
+		int freq_khz = (freq * 625) / 10;
+		unsigned int nearest = analog_get_nearest_freq(type, system,
+							       freq_khz);
+		info->analog.ana_bcast_type = type;
+		info->analog.ana_freq = (nearest * 10) / 625;
+		info->analog.bcast_system = system;
+		return true;
+	}
+	return false;
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	bool is_bcast = cec_msg_is_broadcast(&msg);
@@ -105,21 +151,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -128,6 +164,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!analog_set_tuner_dev_info(node, &msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v6] cec-follower: add tuner analog service emulation

[Jiunn Chang]

Implement the following tuner control features:
  - <Select Analogue Service>
  - <Give Tuner Device Status> and reply <Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---

Changes made since v1:
  - Fix typos/bugs
  - Import reply_feature_abort() from cec-processing.cpp
  - Add functionality to choose nearest frequency

Changes made since v2:
  - Fix typos/bugs
  - Use state from node in cec-follower.h
  - Rename functions to analog_ prefix

Changes made since v3:
  - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp

Changes made since v4:
  - Remove int casting in abs()
  - Add temp variables: info, freq to make code easier to read
  - Use NUM_ANALOG_FREQS macro

Changes made since v5:
  - Add analog_tuner_init() to cec-tuner.cpp
  - Change state_init() in cec-follower.cpp to use analog_tuner_init()
  - Rename function to analog_get_nearest_freq()
  - Change analog_get_nearest_freq() to only return freq in Khz
  - Change analog_set_tuner_dev_info() to set analog freq too

---
 utils/cec-follower/cec-follower.cpp |  1 +
 utils/cec-follower/cec-follower.h   |  2 +
 utils/cec-follower/cec-tuner.cpp    | 73 ++++++++++++++++++++++++-----
 3 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
index dca0f627..4243fdd9 100644
--- a/utils/cec-follower/cec-follower.cpp
+++ b/utils/cec-follower/cec-follower.cpp
@@ -298,6 +298,7 @@ void state_init(struct node &node)
 	node.state.sac_active = false;
 	node.state.volume = 50;
 	node.state.mute = false;
+	analog_tuner_init(&node.state.tuner_dev_info);
 }
 
 int main(int argc, char **argv)
diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 954e001f..a53c16fe 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
@@ -220,6 +221,7 @@ std::string opcode2s(const struct cec_msg *msg);
 void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
 
 // cec-tuner.cpp
+void analog_tuner_init(struct cec_op_tuner_device_info *tuner_dev_info);
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
 
 // CEC processing
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index bdeda342..68d945d5 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -89,6 +90,51 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
 	}
 };
 
+void analog_tuner_init(struct cec_op_tuner_device_info *info)
+{
+	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	info->is_analog = true;
+	info->analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
+	info->analog.ana_freq =
+		analog_freqs_khz[CEC_OP_ANA_BCAST_TYPE_CABLE][CEC_OP_BCAST_SYSTEM_PAL_BG][0];
+	info->analog.bcast_system = CEC_OP_BCAST_SYSTEM_PAL_BG;
+}
+
+static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
+                                            int ana_freq_khz)
+{
+	int nearest = analog_freqs_khz[ana_bcast_type][ana_bcast_system][0];
+
+	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
+		int freq = analog_freqs_khz[ana_bcast_type][ana_bcast_system][i];
+
+		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest))
+			nearest = freq;
+	}
+	return nearest;
+}
+
+static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
+{
+	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
+	__u8 type = 0;
+	__u16 freq = 0;
+	__u8 system = 0;
+
+	cec_ops_select_analogue_service(msg, &type, &freq, &system);
+	if (info->analog.ana_bcast_type < 3 && info->analog.bcast_system < 9) {
+		int freq_khz = (freq * 625) / 10;
+		unsigned int nearest = analog_get_nearest_freq(type, system,
+							       freq_khz);
+		info->analog.ana_bcast_type = type;
+		info->analog.ana_freq = (nearest * 10) / 625;
+		info->analog.bcast_system = system;
+		return true;
+	}
+	return false;
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	bool is_bcast = cec_msg_is_broadcast(&msg);
@@ -105,21 +151,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -128,6 +164,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!analog_set_tuner_dev_info(node, &msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v6] cec-follower: add tuner analog service emulation

[skhan]

On 9/20/19 1:32 PM, Jiunn Chang wrote:
> Implement the following tuner control features:
>    - <Select Analogue Service>
>    - <Give Tuner Device Status> and reply <Tuner Device Status>
> 

More information is needed on what this feature does. This
commit log to terse.

> Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
> ---
> 
> Changes made since v1:
>    - Fix typos/bugs
>    - Import reply_feature_abort() from cec-processing.cpp
>    - Add functionality to choose nearest frequency
> 
> Changes made since v2:
>    - Fix typos/bugs
>    - Use state from node in cec-follower.h
>    - Rename functions to analog_ prefix
> 
> Changes made since v3:
>    - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp
> 
> Changes made since v4:
>    - Remove int casting in abs()
>    - Add temp variables: info, freq to make code easier to read
>    - Use NUM_ANALOG_FREQS macro
> 
> Changes made since v5:
>    - Add analog_tuner_init() to cec-tuner.cpp
>    - Change state_init() in cec-follower.cpp to use analog_tuner_init()
>    - Rename function to analog_get_nearest_freq()
>    - Change analog_get_nearest_freq() to only return freq in Khz
>    - Change analog_set_tuner_dev_info() to set analog freq too
> 

I think it is better to reverse this list, so your recent version is at
the top.

> ---
>   utils/cec-follower/cec-follower.cpp |  1 +
>   utils/cec-follower/cec-follower.h   |  2 +
>   utils/cec-follower/cec-tuner.cpp    | 73 ++++++++++++++++++++++++-----
>   3 files changed, 64 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
> index dca0f627..4243fdd9 100644
> --- a/utils/cec-follower/cec-follower.cpp
> +++ b/utils/cec-follower/cec-follower.cpp
> @@ -298,6 +298,7 @@ void state_init(struct node &node)
>   	node.state.sac_active = false;
>   	node.state.volume = 50;
>   	node.state.mute = false;
> +	analog_tuner_init(&node.state.tuner_dev_info);
>   }
>   
>   int main(int argc, char **argv)
> diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
> index 954e001f..a53c16fe 100644
> --- a/utils/cec-follower/cec-follower.h
> +++ b/utils/cec-follower/cec-follower.h
> @@ -51,6 +51,7 @@ struct state {
>   	__u64 rc_press_rx_ts;
>   	unsigned rc_press_hold_count;
>   	unsigned rc_duration_sum;
> +	struct cec_op_tuner_device_info tuner_dev_info;
>   };
>   
>   struct node {
> @@ -220,6 +221,7 @@ std::string opcode2s(const struct cec_msg *msg);
>   void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
>   
>   // cec-tuner.cpp
> +void analog_tuner_init(struct cec_op_tuner_device_info *tuner_dev_info);
>   void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
>   
>   // CEC processing
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index bdeda342..68d945d5 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -4,6 +4,7 @@
>    */
>   
>   #include <sys/ioctl.h>
> +#include <stdlib.h>
>   
>   #include "cec-follower.h"
>   
> @@ -89,6 +90,51 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
>   	}
>   };
>   
> +void analog_tuner_init(struct cec_op_tuner_device_info *info)
> +{
> +	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> +	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
> +	info->is_analog = true;
> +	info->analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
> +	info->analog.ana_freq =
> +		analog_freqs_khz[CEC_OP_ANA_BCAST_TYPE_CABLE][CEC_OP_BCAST_SYSTEM_PAL_BG][0];
> +	info->analog.bcast_system = CEC_OP_BCAST_SYSTEM_PAL_BG;
> +}
> +
> +static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
> +                                            int ana_freq_khz)
> +{
> +	int nearest = analog_freqs_khz[ana_bcast_type][ana_bcast_system][0];
> +
> +	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
> +		int freq = analog_freqs_khz[ana_bcast_type][ana_bcast_system][i];
> +
> +		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest))
> +			nearest = freq;
> +	}
> +	return nearest;
> +}
> +
> +static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
> +{
> +	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
> +	__u8 type = 0;
> +	__u16 freq = 0;
> +	__u8 system = 0;
> +
> +	cec_ops_select_analogue_service(msg, &type, &freq, &system);
> +	if (info->analog.ana_bcast_type < 3 && info->analog.bcast_system < 9) {
> +		int freq_khz = (freq * 625) / 10;
> +		unsigned int nearest = analog_get_nearest_freq(type, system,
> +							       freq_khz);
> +		info->analog.ana_bcast_type = type;
> +		info->analog.ana_freq = (nearest * 10) / 625;
> +		info->analog.bcast_system = system;
> +		return true;
> +	}
> +	return false;
> +}
> +
>   void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
>   {
>   	bool is_bcast = cec_msg_is_broadcast(&msg);
> @@ -105,21 +151,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>   		*/
>   
>   	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
> -		if (!cec_has_tuner(1 << me))
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
>   			break;
>   
> -		struct cec_op_tuner_device_info tuner_dev_info = {};
> -
>   		cec_msg_set_reply_to(&msg, &msg);
> -		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> -		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
> -		tuner_dev_info.is_analog = false;
> -		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
> -		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
> -		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
> -		tuner_dev_info.digital.channel.minor = 1;
> -
> -		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
> +		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
>   		transmit(node, &msg);
>   		return;
>   	}
> @@ -128,6 +164,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>   		return;
>   
>   	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
> +			break;
> +
> +		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
> +			return;
> +		}
> +		if (!analog_set_tuner_dev_info(node, &msg)) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
> +			return;
> +		}
> +		return;
> +
>   	case CEC_MSG_SELECT_DIGITAL_SERVICE:
>   	case CEC_MSG_TUNER_STEP_DECREMENT:
>   	case CEC_MSG_TUNER_STEP_INCREMENT:
> 

[Linux-kernel-mentees] [PATCH v6] cec-follower: add tuner analog service emulation

[Shuah Khan]

On 9/20/19 1:32 PM, Jiunn Chang wrote:
> Implement the following tuner control features:
>    - <Select Analogue Service>
>    - <Give Tuner Device Status> and reply <Tuner Device Status>
> 

More information is needed on what this feature does. This
commit log to terse.

> Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
> ---
> 
> Changes made since v1:
>    - Fix typos/bugs
>    - Import reply_feature_abort() from cec-processing.cpp
>    - Add functionality to choose nearest frequency
> 
> Changes made since v2:
>    - Fix typos/bugs
>    - Use state from node in cec-follower.h
>    - Rename functions to analog_ prefix
> 
> Changes made since v3:
>    - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp
> 
> Changes made since v4:
>    - Remove int casting in abs()
>    - Add temp variables: info, freq to make code easier to read
>    - Use NUM_ANALOG_FREQS macro
> 
> Changes made since v5:
>    - Add analog_tuner_init() to cec-tuner.cpp
>    - Change state_init() in cec-follower.cpp to use analog_tuner_init()
>    - Rename function to analog_get_nearest_freq()
>    - Change analog_get_nearest_freq() to only return freq in Khz
>    - Change analog_set_tuner_dev_info() to set analog freq too
> 

I think it is better to reverse this list, so your recent version is at
the top.

> ---
>   utils/cec-follower/cec-follower.cpp |  1 +
>   utils/cec-follower/cec-follower.h   |  2 +
>   utils/cec-follower/cec-tuner.cpp    | 73 ++++++++++++++++++++++++-----
>   3 files changed, 64 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
> index dca0f627..4243fdd9 100644
> --- a/utils/cec-follower/cec-follower.cpp
> +++ b/utils/cec-follower/cec-follower.cpp
> @@ -298,6 +298,7 @@ void state_init(struct node &node)
>   	node.state.sac_active = false;
>   	node.state.volume = 50;
>   	node.state.mute = false;
> +	analog_tuner_init(&node.state.tuner_dev_info);
>   }
>   
>   int main(int argc, char **argv)
> diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
> index 954e001f..a53c16fe 100644
> --- a/utils/cec-follower/cec-follower.h
> +++ b/utils/cec-follower/cec-follower.h
> @@ -51,6 +51,7 @@ struct state {
>   	__u64 rc_press_rx_ts;
>   	unsigned rc_press_hold_count;
>   	unsigned rc_duration_sum;
> +	struct cec_op_tuner_device_info tuner_dev_info;
>   };
>   
>   struct node {
> @@ -220,6 +221,7 @@ std::string opcode2s(const struct cec_msg *msg);
>   void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
>   
>   // cec-tuner.cpp
> +void analog_tuner_init(struct cec_op_tuner_device_info *tuner_dev_info);
>   void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
>   
>   // CEC processing
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index bdeda342..68d945d5 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -4,6 +4,7 @@
>    */
>   
>   #include <sys/ioctl.h>
> +#include <stdlib.h>
>   
>   #include "cec-follower.h"
>   
> @@ -89,6 +90,51 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
>   	}
>   };
>   
> +void analog_tuner_init(struct cec_op_tuner_device_info *info)
> +{
> +	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> +	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
> +	info->is_analog = true;
> +	info->analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
> +	info->analog.ana_freq =
> +		analog_freqs_khz[CEC_OP_ANA_BCAST_TYPE_CABLE][CEC_OP_BCAST_SYSTEM_PAL_BG][0];
> +	info->analog.bcast_system = CEC_OP_BCAST_SYSTEM_PAL_BG;
> +}
> +
> +static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
> +                                            int ana_freq_khz)
> +{
> +	int nearest = analog_freqs_khz[ana_bcast_type][ana_bcast_system][0];
> +
> +	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
> +		int freq = analog_freqs_khz[ana_bcast_type][ana_bcast_system][i];
> +
> +		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest))
> +			nearest = freq;
> +	}
> +	return nearest;
> +}
> +
> +static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
> +{
> +	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
> +	__u8 type = 0;
> +	__u16 freq = 0;
> +	__u8 system = 0;
> +
> +	cec_ops_select_analogue_service(msg, &type, &freq, &system);
> +	if (info->analog.ana_bcast_type < 3 && info->analog.bcast_system < 9) {
> +		int freq_khz = (freq * 625) / 10;
> +		unsigned int nearest = analog_get_nearest_freq(type, system,
> +							       freq_khz);
> +		info->analog.ana_bcast_type = type;
> +		info->analog.ana_freq = (nearest * 10) / 625;
> +		info->analog.bcast_system = system;
> +		return true;
> +	}
> +	return false;
> +}
> +
>   void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
>   {
>   	bool is_bcast = cec_msg_is_broadcast(&msg);
> @@ -105,21 +151,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>   		*/
>   
>   	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
> -		if (!cec_has_tuner(1 << me))
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
>   			break;
>   
> -		struct cec_op_tuner_device_info tuner_dev_info = {};
> -
>   		cec_msg_set_reply_to(&msg, &msg);
> -		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> -		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
> -		tuner_dev_info.is_analog = false;
> -		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
> -		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
> -		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
> -		tuner_dev_info.digital.channel.minor = 1;
> -
> -		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
> +		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
>   		transmit(node, &msg);
>   		return;
>   	}
> @@ -128,6 +164,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>   		return;
>   
>   	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
> +			break;
> +
> +		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
> +			return;
> +		}
> +		if (!analog_set_tuner_dev_info(node, &msg)) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
> +			return;
> +		}
> +		return;
> +
>   	case CEC_MSG_SELECT_DIGITAL_SERVICE:
>   	case CEC_MSG_TUNER_STEP_DECREMENT:
>   	case CEC_MSG_TUNER_STEP_INCREMENT:
> 

[Linux-kernel-mentees] [PATCH v7] cec-follower: add tuner analog service emulation

[c0d1n61at3]

The cec-follower will now emulate an analog service.  This allows an
initiator device can directly select an analog service by choosing an
analog broadcast type, broadcast system, and frequency. After an analog
service is selected, the cec-follower will also provide the tuner device
status upon request.

Opcodes implemented:
  - <Select Analogue Service>
  - <Give Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---

Changes made since v6:
  - Add more information to commit message

Changes made since v5:
  - Add analog_tuner_init() to cec-tuner.cpp
  - Change state_init() in cec-follower.cpp to use analog_tuner_init()
  - Rename function to analog_get_nearest_freq()
  - Change analog_get_nearest_freq() to only return freq in Khz
  - Change analog_set_tuner_dev_info() to set analog freq too

Changes made since v4:
  - Remove int casting in abs()
  - Add temp variables: info, freq to make code easier to read
  - Use NUM_ANALOG_FREQS macro

Changes made since v3:
  - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp

Changes made since v2:
  - Fix typos/bugs
  - Use state from node in cec-follower.h
  - Rename functions to analog_ prefix

Changes made since v1:
  - Fix typos/bugs
  - Import reply_feature_abort() from cec-processing.cpp
  - Add functionality to choose nearest frequency

---
 utils/cec-follower/cec-follower.cpp |  1 +
 utils/cec-follower/cec-follower.h   |  2 +
 utils/cec-follower/cec-tuner.cpp    | 73 ++++++++++++++++++++++++-----
 3 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
index dca0f627..4243fdd9 100644
--- a/utils/cec-follower/cec-follower.cpp
+++ b/utils/cec-follower/cec-follower.cpp
@@ -298,6 +298,7 @@ void state_init(struct node &node)
 	node.state.sac_active = false;
 	node.state.volume = 50;
 	node.state.mute = false;
+	analog_tuner_init(&node.state.tuner_dev_info);
 }
 
 int main(int argc, char **argv)
diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 954e001f..a53c16fe 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
@@ -220,6 +221,7 @@ std::string opcode2s(const struct cec_msg *msg);
 void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
 
 // cec-tuner.cpp
+void analog_tuner_init(struct cec_op_tuner_device_info *tuner_dev_info);
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
 
 // CEC processing
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index bdeda342..68d945d5 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -89,6 +90,51 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
 	}
 };
 
+void analog_tuner_init(struct cec_op_tuner_device_info *info)
+{
+	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	info->is_analog = true;
+	info->analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
+	info->analog.ana_freq =
+		analog_freqs_khz[CEC_OP_ANA_BCAST_TYPE_CABLE][CEC_OP_BCAST_SYSTEM_PAL_BG][0];
+	info->analog.bcast_system = CEC_OP_BCAST_SYSTEM_PAL_BG;
+}
+
+static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
+                                            int ana_freq_khz)
+{
+	int nearest = analog_freqs_khz[ana_bcast_type][ana_bcast_system][0];
+
+	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
+		int freq = analog_freqs_khz[ana_bcast_type][ana_bcast_system][i];
+
+		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest))
+			nearest = freq;
+	}
+	return nearest;
+}
+
+static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
+{
+	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
+	__u8 type = 0;
+	__u16 freq = 0;
+	__u8 system = 0;
+
+	cec_ops_select_analogue_service(msg, &type, &freq, &system);
+	if (info->analog.ana_bcast_type < 3 && info->analog.bcast_system < 9) {
+		int freq_khz = (freq * 625) / 10;
+		unsigned int nearest = analog_get_nearest_freq(type, system,
+							       freq_khz);
+		info->analog.ana_bcast_type = type;
+		info->analog.ana_freq = (nearest * 10) / 625;
+		info->analog.bcast_system = system;
+		return true;
+	}
+	return false;
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	bool is_bcast = cec_msg_is_broadcast(&msg);
@@ -105,21 +151,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -128,6 +164,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!analog_set_tuner_dev_info(node, &msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v7] cec-follower: add tuner analog service emulation

[Jiunn Chang]

The cec-follower will now emulate an analog service.  This allows an
initiator device can directly select an analog service by choosing an
analog broadcast type, broadcast system, and frequency. After an analog
service is selected, the cec-follower will also provide the tuner device
status upon request.

Opcodes implemented:
  - <Select Analogue Service>
  - <Give Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---

Changes made since v6:
  - Add more information to commit message

Changes made since v5:
  - Add analog_tuner_init() to cec-tuner.cpp
  - Change state_init() in cec-follower.cpp to use analog_tuner_init()
  - Rename function to analog_get_nearest_freq()
  - Change analog_get_nearest_freq() to only return freq in Khz
  - Change analog_set_tuner_dev_info() to set analog freq too

Changes made since v4:
  - Remove int casting in abs()
  - Add temp variables: info, freq to make code easier to read
  - Use NUM_ANALOG_FREQS macro

Changes made since v3:
  - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp

Changes made since v2:
  - Fix typos/bugs
  - Use state from node in cec-follower.h
  - Rename functions to analog_ prefix

Changes made since v1:
  - Fix typos/bugs
  - Import reply_feature_abort() from cec-processing.cpp
  - Add functionality to choose nearest frequency

---
 utils/cec-follower/cec-follower.cpp |  1 +
 utils/cec-follower/cec-follower.h   |  2 +
 utils/cec-follower/cec-tuner.cpp    | 73 ++++++++++++++++++++++++-----
 3 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
index dca0f627..4243fdd9 100644
--- a/utils/cec-follower/cec-follower.cpp
+++ b/utils/cec-follower/cec-follower.cpp
@@ -298,6 +298,7 @@ void state_init(struct node &node)
 	node.state.sac_active = false;
 	node.state.volume = 50;
 	node.state.mute = false;
+	analog_tuner_init(&node.state.tuner_dev_info);
 }
 
 int main(int argc, char **argv)
diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 954e001f..a53c16fe 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
@@ -220,6 +221,7 @@ std::string opcode2s(const struct cec_msg *msg);
 void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
 
 // cec-tuner.cpp
+void analog_tuner_init(struct cec_op_tuner_device_info *tuner_dev_info);
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
 
 // CEC processing
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index bdeda342..68d945d5 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -89,6 +90,51 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
 	}
 };
 
+void analog_tuner_init(struct cec_op_tuner_device_info *info)
+{
+	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	info->is_analog = true;
+	info->analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
+	info->analog.ana_freq =
+		analog_freqs_khz[CEC_OP_ANA_BCAST_TYPE_CABLE][CEC_OP_BCAST_SYSTEM_PAL_BG][0];
+	info->analog.bcast_system = CEC_OP_BCAST_SYSTEM_PAL_BG;
+}
+
+static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
+                                            int ana_freq_khz)
+{
+	int nearest = analog_freqs_khz[ana_bcast_type][ana_bcast_system][0];
+
+	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
+		int freq = analog_freqs_khz[ana_bcast_type][ana_bcast_system][i];
+
+		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest))
+			nearest = freq;
+	}
+	return nearest;
+}
+
+static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
+{
+	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
+	__u8 type = 0;
+	__u16 freq = 0;
+	__u8 system = 0;
+
+	cec_ops_select_analogue_service(msg, &type, &freq, &system);
+	if (info->analog.ana_bcast_type < 3 && info->analog.bcast_system < 9) {
+		int freq_khz = (freq * 625) / 10;
+		unsigned int nearest = analog_get_nearest_freq(type, system,
+							       freq_khz);
+		info->analog.ana_bcast_type = type;
+		info->analog.ana_freq = (nearest * 10) / 625;
+		info->analog.bcast_system = system;
+		return true;
+	}
+	return false;
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	bool is_bcast = cec_msg_is_broadcast(&msg);
@@ -105,21 +151,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -128,6 +164,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!analog_set_tuner_dev_info(node, &msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v7] cec-follower: add tuner analog service emulation

[hverkuil]

On 9/20/19 11:17 PM, Jiunn Chang wrote:
> The cec-follower will now emulate an analog service.  This allows an
> initiator device can directly select an analog service by choosing an
> analog broadcast type, broadcast system, and frequency. After an analog
> service is selected, the cec-follower will also provide the tuner device
> status upon request.
> 
> Opcodes implemented:
>   - <Select Analogue Service>
>   - <Give Tuner Device Status>
> 
> Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
> ---
> 
> Changes made since v6:
>   - Add more information to commit message
> 
> Changes made since v5:
>   - Add analog_tuner_init() to cec-tuner.cpp
>   - Change state_init() in cec-follower.cpp to use analog_tuner_init()
>   - Rename function to analog_get_nearest_freq()
>   - Change analog_get_nearest_freq() to only return freq in Khz
>   - Change analog_set_tuner_dev_info() to set analog freq too
> 
> Changes made since v4:
>   - Remove int casting in abs()
>   - Add temp variables: info, freq to make code easier to read
>   - Use NUM_ANALOG_FREQS macro
> 
> Changes made since v3:
>   - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp
> 
> Changes made since v2:
>   - Fix typos/bugs
>   - Use state from node in cec-follower.h
>   - Rename functions to analog_ prefix
> 
> Changes made since v1:
>   - Fix typos/bugs
>   - Import reply_feature_abort() from cec-processing.cpp
>   - Add functionality to choose nearest frequency
> 
> ---
>  utils/cec-follower/cec-follower.cpp |  1 +
>  utils/cec-follower/cec-follower.h   |  2 +
>  utils/cec-follower/cec-tuner.cpp    | 73 ++++++++++++++++++++++++-----
>  3 files changed, 64 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
> index dca0f627..4243fdd9 100644
> --- a/utils/cec-follower/cec-follower.cpp
> +++ b/utils/cec-follower/cec-follower.cpp
> @@ -298,6 +298,7 @@ void state_init(struct node &node)
>  	node.state.sac_active = false;
>  	node.state.volume = 50;
>  	node.state.mute = false;
> +	analog_tuner_init(&node.state.tuner_dev_info);
>  }
>  
>  int main(int argc, char **argv)
> diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
> index 954e001f..a53c16fe 100644
> --- a/utils/cec-follower/cec-follower.h
> +++ b/utils/cec-follower/cec-follower.h
> @@ -51,6 +51,7 @@ struct state {
>  	__u64 rc_press_rx_ts;
>  	unsigned rc_press_hold_count;
>  	unsigned rc_duration_sum;
> +	struct cec_op_tuner_device_info tuner_dev_info;
>  };
>  
>  struct node {
> @@ -220,6 +221,7 @@ std::string opcode2s(const struct cec_msg *msg);
>  void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
>  
>  // cec-tuner.cpp
> +void analog_tuner_init(struct cec_op_tuner_device_info *tuner_dev_info);
>  void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
>  
>  // CEC processing
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index bdeda342..68d945d5 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <sys/ioctl.h>
> +#include <stdlib.h>
>  
>  #include "cec-follower.h"
>  
> @@ -89,6 +90,51 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
>  	}
>  };
>  
> +void analog_tuner_init(struct cec_op_tuner_device_info *info)
> +{
> +	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> +	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
> +	info->is_analog = true;
> +	info->analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
> +	info->analog.ana_freq =
> +		analog_freqs_khz[CEC_OP_ANA_BCAST_TYPE_CABLE][CEC_OP_BCAST_SYSTEM_PAL_BG][0];
> +	info->analog.bcast_system = CEC_OP_BCAST_SYSTEM_PAL_BG;

Set bcast_system before setting ana_freq, and when you set ana_freq use the
analog.ana_bcast_type and analog.bcast_system as indices.

That way you avoid the case where in the future someone changes e.g. ana_bcast_type
but forgets to update the index in the ana_freq assignment as well.

It's a little thing, but it makes for more robust programs.

> +}
> +
> +static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
> +                                            int ana_freq_khz)
> +{
> +	int nearest = analog_freqs_khz[ana_bcast_type][ana_bcast_system][0];
> +
> +	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
> +		int freq = analog_freqs_khz[ana_bcast_type][ana_bcast_system][i];
> +
> +		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest))
> +			nearest = freq;
> +	}
> +	return nearest;
> +}
> +
> +static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
> +{
> +	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
> +	__u8 type = 0;
> +	__u16 freq = 0;
> +	__u8 system = 0;

No need to zero these three variables. cec_ops_select_analogue_service will always
overwrite them.

> +
> +	cec_ops_select_analogue_service(msg, &type, &freq, &system);
> +	if (info->analog.ana_bcast_type < 3 && info->analog.bcast_system < 9) {
> +		int freq_khz = (freq * 625) / 10;
> +		unsigned int nearest = analog_get_nearest_freq(type, system,
> +							       freq_khz);

Add empty line between variable declarations and the following code.

> +		info->analog.ana_bcast_type = type;
> +		info->analog.ana_freq = (nearest * 10) / 625;
> +		info->analog.bcast_system = system;
> +		return true;
> +	}
> +	return false;
> +}
> +
>  void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
>  {
>  	bool is_bcast = cec_msg_is_broadcast(&msg);
> @@ -105,21 +151,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		*/
>  
>  	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
> -		if (!cec_has_tuner(1 << me))
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
>  			break;
>  
> -		struct cec_op_tuner_device_info tuner_dev_info = {};
> -
>  		cec_msg_set_reply_to(&msg, &msg);
> -		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> -		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
> -		tuner_dev_info.is_analog = false;
> -		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
> -		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
> -		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
> -		tuner_dev_info.digital.channel.minor = 1;
> -
> -		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
> +		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
>  		transmit(node, &msg);
>  		return;
>  	}
> @@ -128,6 +164,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		return;
>  
>  	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
> +			break;
> +
> +		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
> +			return;
> +		}
> +		if (!analog_set_tuner_dev_info(node, &msg)) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
> +			return;
> +		}
> +		return;
> +
>  	case CEC_MSG_SELECT_DIGITAL_SERVICE:
>  	case CEC_MSG_TUNER_STEP_DECREMENT:
>  	case CEC_MSG_TUNER_STEP_INCREMENT:
> 

Regards,

	Hans

[Linux-kernel-mentees] [PATCH v7] cec-follower: add tuner analog service emulation

[Hans Verkuil]

On 9/20/19 11:17 PM, Jiunn Chang wrote:
> The cec-follower will now emulate an analog service.  This allows an
> initiator device can directly select an analog service by choosing an
> analog broadcast type, broadcast system, and frequency. After an analog
> service is selected, the cec-follower will also provide the tuner device
> status upon request.
> 
> Opcodes implemented:
>   - <Select Analogue Service>
>   - <Give Tuner Device Status>
> 
> Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
> ---
> 
> Changes made since v6:
>   - Add more information to commit message
> 
> Changes made since v5:
>   - Add analog_tuner_init() to cec-tuner.cpp
>   - Change state_init() in cec-follower.cpp to use analog_tuner_init()
>   - Rename function to analog_get_nearest_freq()
>   - Change analog_get_nearest_freq() to only return freq in Khz
>   - Change analog_set_tuner_dev_info() to set analog freq too
> 
> Changes made since v4:
>   - Remove int casting in abs()
>   - Add temp variables: info, freq to make code easier to read
>   - Use NUM_ANALOG_FREQS macro
> 
> Changes made since v3:
>   - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp
> 
> Changes made since v2:
>   - Fix typos/bugs
>   - Use state from node in cec-follower.h
>   - Rename functions to analog_ prefix
> 
> Changes made since v1:
>   - Fix typos/bugs
>   - Import reply_feature_abort() from cec-processing.cpp
>   - Add functionality to choose nearest frequency
> 
> ---
>  utils/cec-follower/cec-follower.cpp |  1 +
>  utils/cec-follower/cec-follower.h   |  2 +
>  utils/cec-follower/cec-tuner.cpp    | 73 ++++++++++++++++++++++++-----
>  3 files changed, 64 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
> index dca0f627..4243fdd9 100644
> --- a/utils/cec-follower/cec-follower.cpp
> +++ b/utils/cec-follower/cec-follower.cpp
> @@ -298,6 +298,7 @@ void state_init(struct node &node)
>  	node.state.sac_active = false;
>  	node.state.volume = 50;
>  	node.state.mute = false;
> +	analog_tuner_init(&node.state.tuner_dev_info);
>  }
>  
>  int main(int argc, char **argv)
> diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
> index 954e001f..a53c16fe 100644
> --- a/utils/cec-follower/cec-follower.h
> +++ b/utils/cec-follower/cec-follower.h
> @@ -51,6 +51,7 @@ struct state {
>  	__u64 rc_press_rx_ts;
>  	unsigned rc_press_hold_count;
>  	unsigned rc_duration_sum;
> +	struct cec_op_tuner_device_info tuner_dev_info;
>  };
>  
>  struct node {
> @@ -220,6 +221,7 @@ std::string opcode2s(const struct cec_msg *msg);
>  void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
>  
>  // cec-tuner.cpp
> +void analog_tuner_init(struct cec_op_tuner_device_info *tuner_dev_info);
>  void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
>  
>  // CEC processing
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index bdeda342..68d945d5 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <sys/ioctl.h>
> +#include <stdlib.h>
>  
>  #include "cec-follower.h"
>  
> @@ -89,6 +90,51 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
>  	}
>  };
>  
> +void analog_tuner_init(struct cec_op_tuner_device_info *info)
> +{
> +	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> +	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
> +	info->is_analog = true;
> +	info->analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
> +	info->analog.ana_freq =
> +		analog_freqs_khz[CEC_OP_ANA_BCAST_TYPE_CABLE][CEC_OP_BCAST_SYSTEM_PAL_BG][0];
> +	info->analog.bcast_system = CEC_OP_BCAST_SYSTEM_PAL_BG;

Set bcast_system before setting ana_freq, and when you set ana_freq use the
analog.ana_bcast_type and analog.bcast_system as indices.

That way you avoid the case where in the future someone changes e.g. ana_bcast_type
but forgets to update the index in the ana_freq assignment as well.

It's a little thing, but it makes for more robust programs.

> +}
> +
> +static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
> +                                            int ana_freq_khz)
> +{
> +	int nearest = analog_freqs_khz[ana_bcast_type][ana_bcast_system][0];
> +
> +	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
> +		int freq = analog_freqs_khz[ana_bcast_type][ana_bcast_system][i];
> +
> +		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest))
> +			nearest = freq;
> +	}
> +	return nearest;
> +}
> +
> +static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
> +{
> +	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
> +	__u8 type = 0;
> +	__u16 freq = 0;
> +	__u8 system = 0;

No need to zero these three variables. cec_ops_select_analogue_service will always
overwrite them.

> +
> +	cec_ops_select_analogue_service(msg, &type, &freq, &system);
> +	if (info->analog.ana_bcast_type < 3 && info->analog.bcast_system < 9) {
> +		int freq_khz = (freq * 625) / 10;
> +		unsigned int nearest = analog_get_nearest_freq(type, system,
> +							       freq_khz);

Add empty line between variable declarations and the following code.

> +		info->analog.ana_bcast_type = type;
> +		info->analog.ana_freq = (nearest * 10) / 625;
> +		info->analog.bcast_system = system;
> +		return true;
> +	}
> +	return false;
> +}
> +
>  void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
>  {
>  	bool is_bcast = cec_msg_is_broadcast(&msg);
> @@ -105,21 +151,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		*/
>  
>  	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
> -		if (!cec_has_tuner(1 << me))
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
>  			break;
>  
> -		struct cec_op_tuner_device_info tuner_dev_info = {};
> -
>  		cec_msg_set_reply_to(&msg, &msg);
> -		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> -		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
> -		tuner_dev_info.is_analog = false;
> -		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
> -		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
> -		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
> -		tuner_dev_info.digital.channel.minor = 1;
> -
> -		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
> +		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
>  		transmit(node, &msg);
>  		return;
>  	}
> @@ -128,6 +164,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>  		return;
>  
>  	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
> +			break;
> +
> +		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
> +			return;
> +		}
> +		if (!analog_set_tuner_dev_info(node, &msg)) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
> +			return;
> +		}
> +		return;
> +
>  	case CEC_MSG_SELECT_DIGITAL_SERVICE:
>  	case CEC_MSG_TUNER_STEP_DECREMENT:
>  	case CEC_MSG_TUNER_STEP_INCREMENT:
> 

Regards,

	Hans

[Linux-kernel-mentees] [PATCH v8] cec-follower: add tuner analog service emulation

[c0d1n61at3]

The cec-follower will now emulate an analog service.  This allows an
initiator device can directly select an analog service by choosing an
analog broadcast type, broadcast system, and frequency. After an analog
service is selected, the cec-follower will also provide the tuner device
status upon request.

Opcodes implemented:
  - <Select Analogue Service>
  - <Give Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---

Changes made since v7:
  - Make analog_tuner_init() more robust
  - Remove redundancy from analog_set_tuner_dev_info()

Changes made since v6:
  - Add more information to commit message

Changes made since v5:
  - Add analog_tuner_init() to cec-tuner.cpp
  - Change state_init() in cec-follower.cpp to use analog_tuner_init()
  - Rename function to analog_get_nearest_freq()
  - Change analog_get_nearest_freq() to only return freq in Khz
  - Change analog_set_tuner_dev_info() to set analog freq too

Changes made since v4:
  - Remove int casting in abs()
  - Add temp variables: info, freq to make code easier to read
  - Use NUM_ANALOG_FREQS macro

Changes made since v3:
  - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp

Changes made since v2:
  - Fix typos/bugs
  - Use state from node in cec-follower.h
  - Rename functions to analog_ prefix

Changes made since v1:
  - Fix typos/bugs
  - Import reply_feature_abort() from cec-processing.cpp
  - Add functionality to choose nearest frequency

---
 utils/cec-follower/cec-follower.cpp |  1 +
 utils/cec-follower/cec-follower.h   |  2 +
 utils/cec-follower/cec-tuner.cpp    | 73 ++++++++++++++++++++++++-----
 3 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
index dca0f627..4243fdd9 100644
--- a/utils/cec-follower/cec-follower.cpp
+++ b/utils/cec-follower/cec-follower.cpp
@@ -298,6 +298,7 @@ void state_init(struct node &node)
 	node.state.sac_active = false;
 	node.state.volume = 50;
 	node.state.mute = false;
+	analog_tuner_init(&node.state.tuner_dev_info);
 }
 
 int main(int argc, char **argv)
diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 954e001f..a53c16fe 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
@@ -220,6 +221,7 @@ std::string opcode2s(const struct cec_msg *msg);
 void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
 
 // cec-tuner.cpp
+void analog_tuner_init(struct cec_op_tuner_device_info *tuner_dev_info);
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
 
 // CEC processing
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index bdeda342..acc3fd00 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -89,6 +90,51 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
 	}
 };
 
+void analog_tuner_init(struct cec_op_tuner_device_info *info)
+{
+	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	info->is_analog = true;
+	info->analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
+	info->analog.bcast_system = CEC_OP_BCAST_SYSTEM_PAL_BG;
+	info->analog.ana_freq =
+		analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][0];
+}
+
+static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
+                                            int ana_freq_khz)
+{
+	int nearest = analog_freqs_khz[ana_bcast_type][ana_bcast_system][0];
+
+	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
+		int freq = analog_freqs_khz[ana_bcast_type][ana_bcast_system][i];
+
+		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest))
+			nearest = freq;
+	}
+	return nearest;
+}
+
+static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
+{
+	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
+	__u8 type;
+	__u16 freq;
+	__u8 system;
+
+	cec_ops_select_analogue_service(msg, &type, &freq, &system);
+	if (info->analog.ana_bcast_type < 3 && info->analog.bcast_system < 9) {
+		int freq_khz = (freq * 625) / 10;
+		unsigned int nearest = analog_get_nearest_freq(type, system,
+							       freq_khz);
+		info->analog.ana_bcast_type = type;
+		info->analog.ana_freq = (nearest * 10) / 625;
+		info->analog.bcast_system = system;
+		return true;
+	}
+	return false;
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	bool is_bcast = cec_msg_is_broadcast(&msg);
@@ -105,21 +151,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -128,6 +164,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!analog_set_tuner_dev_info(node, &msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v8] cec-follower: add tuner analog service emulation

[Jiunn Chang]

The cec-follower will now emulate an analog service.  This allows an
initiator device can directly select an analog service by choosing an
analog broadcast type, broadcast system, and frequency. After an analog
service is selected, the cec-follower will also provide the tuner device
status upon request.

Opcodes implemented:
  - <Select Analogue Service>
  - <Give Tuner Device Status>

Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
---

Changes made since v7:
  - Make analog_tuner_init() more robust
  - Remove redundancy from analog_set_tuner_dev_info()

Changes made since v6:
  - Add more information to commit message

Changes made since v5:
  - Add analog_tuner_init() to cec-tuner.cpp
  - Change state_init() in cec-follower.cpp to use analog_tuner_init()
  - Rename function to analog_get_nearest_freq()
  - Change analog_get_nearest_freq() to only return freq in Khz
  - Change analog_set_tuner_dev_info() to set analog freq too

Changes made since v4:
  - Remove int casting in abs()
  - Add temp variables: info, freq to make code easier to read
  - Use NUM_ANALOG_FREQS macro

Changes made since v3:
  - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp

Changes made since v2:
  - Fix typos/bugs
  - Use state from node in cec-follower.h
  - Rename functions to analog_ prefix

Changes made since v1:
  - Fix typos/bugs
  - Import reply_feature_abort() from cec-processing.cpp
  - Add functionality to choose nearest frequency

---
 utils/cec-follower/cec-follower.cpp |  1 +
 utils/cec-follower/cec-follower.h   |  2 +
 utils/cec-follower/cec-tuner.cpp    | 73 ++++++++++++++++++++++++-----
 3 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
index dca0f627..4243fdd9 100644
--- a/utils/cec-follower/cec-follower.cpp
+++ b/utils/cec-follower/cec-follower.cpp
@@ -298,6 +298,7 @@ void state_init(struct node &node)
 	node.state.sac_active = false;
 	node.state.volume = 50;
 	node.state.mute = false;
+	analog_tuner_init(&node.state.tuner_dev_info);
 }
 
 int main(int argc, char **argv)
diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 954e001f..a53c16fe 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	__u64 rc_press_rx_ts;
 	unsigned rc_press_hold_count;
 	unsigned rc_duration_sum;
+	struct cec_op_tuner_device_info tuner_dev_info;
 };
 
 struct node {
@@ -220,6 +221,7 @@ std::string opcode2s(const struct cec_msg *msg);
 void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
 
 // cec-tuner.cpp
+void analog_tuner_init(struct cec_op_tuner_device_info *tuner_dev_info);
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
 
 // CEC processing
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index bdeda342..acc3fd00 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <stdlib.h>
 
 #include "cec-follower.h"
 
@@ -89,6 +90,51 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
 	}
 };
 
+void analog_tuner_init(struct cec_op_tuner_device_info *info)
+{
+	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
+	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
+	info->is_analog = true;
+	info->analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
+	info->analog.bcast_system = CEC_OP_BCAST_SYSTEM_PAL_BG;
+	info->analog.ana_freq =
+		analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][0];
+}
+
+static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
+                                            int ana_freq_khz)
+{
+	int nearest = analog_freqs_khz[ana_bcast_type][ana_bcast_system][0];
+
+	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
+		int freq = analog_freqs_khz[ana_bcast_type][ana_bcast_system][i];
+
+		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest))
+			nearest = freq;
+	}
+	return nearest;
+}
+
+static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
+{
+	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
+	__u8 type;
+	__u16 freq;
+	__u8 system;
+
+	cec_ops_select_analogue_service(msg, &type, &freq, &system);
+	if (info->analog.ana_bcast_type < 3 && info->analog.bcast_system < 9) {
+		int freq_khz = (freq * 625) / 10;
+		unsigned int nearest = analog_get_nearest_freq(type, system,
+							       freq_khz);
+		info->analog.ana_bcast_type = type;
+		info->analog.ana_freq = (nearest * 10) / 625;
+		info->analog.bcast_system = system;
+		return true;
+	}
+	return false;
+}
+
 void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	bool is_bcast = cec_msg_is_broadcast(&msg);
@@ -105,21 +151,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		*/
 
 	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
-		if (!cec_has_tuner(1 << me))
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
 			break;
 
-		struct cec_op_tuner_device_info tuner_dev_info = {};
-
 		cec_msg_set_reply_to(&msg, &msg);
-		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
-		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
-		tuner_dev_info.is_analog = false;
-		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
-		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
-		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
-		tuner_dev_info.digital.channel.minor = 1;
-
-		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
+		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
 		transmit(node, &msg);
 		return;
 	}
@@ -128,6 +164,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
 		return;
 
 	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
+		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
+			break;
+
+		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
+			return;
+		}
+		if (!analog_set_tuner_dev_info(node, &msg)) {
+			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+			return;
+		}
+		return;
+
 	case CEC_MSG_SELECT_DIGITAL_SERVICE:
 	case CEC_MSG_TUNER_STEP_DECREMENT:
 	case CEC_MSG_TUNER_STEP_INCREMENT:
-- 
2.23.0

[Linux-kernel-mentees] [PATCH v8] cec-follower: add tuner analog service emulation

[skhan]

On 9/24/19 6:14 AM, Jiunn Chang wrote:
> The cec-follower will now emulate an analog service.  This allows an
> initiator device can directly select an analog service by choosing an

to select? Something doesn't read right in this sentence.

> analog broadcast type, broadcast system, and frequency. After an analog
> service is selected, the cec-follower will also provide the tuner device
> status upon request.
> 
> Opcodes implemented:
>    - <Select Analogue Service>
>    - <Give Tuner Device Status>
> 
> Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
> ---
> 
> Changes made since v7:
>    - Make analog_tuner_init() more robust
>    - Remove redundancy from analog_set_tuner_dev_info()
> 
> Changes made since v6:
>    - Add more information to commit message
> 
> Changes made since v5:
>    - Add analog_tuner_init() to cec-tuner.cpp
>    - Change state_init() in cec-follower.cpp to use analog_tuner_init()
>    - Rename function to analog_get_nearest_freq()
>    - Change analog_get_nearest_freq() to only return freq in Khz
>    - Change analog_set_tuner_dev_info() to set analog freq too
> 
> Changes made since v4:
>    - Remove int casting in abs()
>    - Add temp variables: info, freq to make code easier to read
>    - Use NUM_ANALOG_FREQS macro
> 
> Changes made since v3:
>    - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp
> 
> Changes made since v2:
>    - Fix typos/bugs
>    - Use state from node in cec-follower.h
>    - Rename functions to analog_ prefix
> 
> Changes made since v1:
>    - Fix typos/bugs
>    - Import reply_feature_abort() from cec-processing.cpp
>    - Add functionality to choose nearest frequency
> 
> ---
>   utils/cec-follower/cec-follower.cpp |  1 +
>   utils/cec-follower/cec-follower.h   |  2 +
>   utils/cec-follower/cec-tuner.cpp    | 73 ++++++++++++++++++++++++-----
>   3 files changed, 64 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
> index dca0f627..4243fdd9 100644
> --- a/utils/cec-follower/cec-follower.cpp
> +++ b/utils/cec-follower/cec-follower.cpp
> @@ -298,6 +298,7 @@ void state_init(struct node &node)
>   	node.state.sac_active = false;
>   	node.state.volume = 50;
>   	node.state.mute = false;
> +	analog_tuner_init(&node.state.tuner_dev_info);
>   }
>   
>   int main(int argc, char **argv)
> diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
> index 954e001f..a53c16fe 100644
> --- a/utils/cec-follower/cec-follower.h
> +++ b/utils/cec-follower/cec-follower.h
> @@ -51,6 +51,7 @@ struct state {
>   	__u64 rc_press_rx_ts;
>   	unsigned rc_press_hold_count;
>   	unsigned rc_duration_sum;
> +	struct cec_op_tuner_device_info tuner_dev_info;
>   };
>   
>   struct node {
> @@ -220,6 +221,7 @@ std::string opcode2s(const struct cec_msg *msg);
>   void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
>   
>   // cec-tuner.cpp
> +void analog_tuner_init(struct cec_op_tuner_device_info *tuner_dev_info);
>   void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
>   
>   // CEC processing
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index bdeda342..acc3fd00 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -4,6 +4,7 @@
>    */
>   
>   #include <sys/ioctl.h>
> +#include <stdlib.h>
>   
>   #include "cec-follower.h"
>   
> @@ -89,6 +90,51 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
>   	}
>   };
>   
> +void analog_tuner_init(struct cec_op_tuner_device_info *info)
> +{
> +	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> +	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
> +	info->is_analog = true;
> +	info->analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
> +	info->analog.bcast_system = CEC_OP_BCAST_SYSTEM_PAL_BG;

Does this need to support NTSC as well? Can user select one or the
other?

> +	info->analog.ana_freq =
> +		analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][0];
> +}
> +
> +static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
> +                                            int ana_freq_khz)
> +{
> +	int nearest = analog_freqs_khz[ana_bcast_type][ana_bcast_system][0];
> +
> +	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
> +		int freq = analog_freqs_khz[ana_bcast_type][ana_bcast_system][i];
> +
> +		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest))
> +			nearest = freq;
> +	}
> +	return nearest;
> +}
> +
> +static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
> +{
> +	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
> +	__u8 type;
> +	__u16 freq;
> +	__u8 system;
> +
> +	cec_ops_select_analogue_service(msg, &type, &freq, &system);
> +	if (info->analog.ana_bcast_type < 3 && info->analog.bcast_system < 9) {
> +		int freq_khz = (freq * 625) / 10;
> +		unsigned int nearest = analog_get_nearest_freq(type, system,
> +							       freq_khz);
> +		info->analog.ana_bcast_type = type;
> +		info->analog.ana_freq = (nearest * 10) / 625;
> +		info->analog.bcast_system = system;
> +		return true;
> +	}
> +	return false;
> +}
> +
>   void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
>   {
>   	bool is_bcast = cec_msg_is_broadcast(&msg);
> @@ -105,21 +151,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>   		*/
>   
>   	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
> -		if (!cec_has_tuner(1 << me))
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
>   			break;
>   
> -		struct cec_op_tuner_device_info tuner_dev_info = {};
> -
>   		cec_msg_set_reply_to(&msg, &msg);
> -		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> -		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
> -		tuner_dev_info.is_analog = false;
> -		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
> -		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
> -		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
> -		tuner_dev_info.digital.channel.minor = 1;
> -
> -		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
> +		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
>   		transmit(node, &msg);
>   		return;
>   	}
> @@ -128,6 +164,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>   		return;
>   
>   	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
> +			break;
> +
> +		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
> +			return;
> +		}
> +		if (!analog_set_tuner_dev_info(node, &msg)) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
> +			return;
> +		}
> +		return;
> +
>   	case CEC_MSG_SELECT_DIGITAL_SERVICE:
>   	case CEC_MSG_TUNER_STEP_DECREMENT:
>   	case CEC_MSG_TUNER_STEP_INCREMENT:
> 

thanks,
-- Shuah

[Linux-kernel-mentees] [PATCH v8] cec-follower: add tuner analog service emulation

[Shuah Khan]

On 9/24/19 6:14 AM, Jiunn Chang wrote:
> The cec-follower will now emulate an analog service.  This allows an
> initiator device can directly select an analog service by choosing an

to select? Something doesn't read right in this sentence.

> analog broadcast type, broadcast system, and frequency. After an analog
> service is selected, the cec-follower will also provide the tuner device
> status upon request.
> 
> Opcodes implemented:
>    - <Select Analogue Service>
>    - <Give Tuner Device Status>
> 
> Signed-off-by: Jiunn Chang <c0d1n61at3 at gmail.com>
> ---
> 
> Changes made since v7:
>    - Make analog_tuner_init() more robust
>    - Remove redundancy from analog_set_tuner_dev_info()
> 
> Changes made since v6:
>    - Add more information to commit message
> 
> Changes made since v5:
>    - Add analog_tuner_init() to cec-tuner.cpp
>    - Change state_init() in cec-follower.cpp to use analog_tuner_init()
>    - Rename function to analog_get_nearest_freq()
>    - Change analog_get_nearest_freq() to only return freq in Khz
>    - Change analog_set_tuner_dev_info() to set analog freq too
> 
> Changes made since v4:
>    - Remove int casting in abs()
>    - Add temp variables: info, freq to make code easier to read
>    - Use NUM_ANALOG_FREQS macro
> 
> Changes made since v3:
>    - Refactor lines numbers since a fix patch was needed for cec-tuner.cpp
> 
> Changes made since v2:
>    - Fix typos/bugs
>    - Use state from node in cec-follower.h
>    - Rename functions to analog_ prefix
> 
> Changes made since v1:
>    - Fix typos/bugs
>    - Import reply_feature_abort() from cec-processing.cpp
>    - Add functionality to choose nearest frequency
> 
> ---
>   utils/cec-follower/cec-follower.cpp |  1 +
>   utils/cec-follower/cec-follower.h   |  2 +
>   utils/cec-follower/cec-tuner.cpp    | 73 ++++++++++++++++++++++++-----
>   3 files changed, 64 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
> index dca0f627..4243fdd9 100644
> --- a/utils/cec-follower/cec-follower.cpp
> +++ b/utils/cec-follower/cec-follower.cpp
> @@ -298,6 +298,7 @@ void state_init(struct node &node)
>   	node.state.sac_active = false;
>   	node.state.volume = 50;
>   	node.state.mute = false;
> +	analog_tuner_init(&node.state.tuner_dev_info);
>   }
>   
>   int main(int argc, char **argv)
> diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
> index 954e001f..a53c16fe 100644
> --- a/utils/cec-follower/cec-follower.h
> +++ b/utils/cec-follower/cec-follower.h
> @@ -51,6 +51,7 @@ struct state {
>   	__u64 rc_press_rx_ts;
>   	unsigned rc_press_hold_count;
>   	unsigned rc_duration_sum;
> +	struct cec_op_tuner_device_info tuner_dev_info;
>   };
>   
>   struct node {
> @@ -220,6 +221,7 @@ std::string opcode2s(const struct cec_msg *msg);
>   void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
>   
>   // cec-tuner.cpp
> +void analog_tuner_init(struct cec_op_tuner_device_info *tuner_dev_info);
>   void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
>   
>   // CEC processing
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index bdeda342..acc3fd00 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -4,6 +4,7 @@
>    */
>   
>   #include <sys/ioctl.h>
> +#include <stdlib.h>
>   
>   #include "cec-follower.h"
>   
> @@ -89,6 +90,51 @@ static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] =
>   	}
>   };
>   
> +void analog_tuner_init(struct cec_op_tuner_device_info *info)
> +{
> +	info->rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> +	info->tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_ANALOGUE;
> +	info->is_analog = true;
> +	info->analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
> +	info->analog.bcast_system = CEC_OP_BCAST_SYSTEM_PAL_BG;

Does this need to support NTSC as well? Can user select one or the
other?

> +	info->analog.ana_freq =
> +		analog_freqs_khz[info->analog.ana_bcast_type][info->analog.bcast_system][0];
> +}
> +
> +static unsigned int analog_get_nearest_freq(__u8 ana_bcast_type, __u8 ana_bcast_system,
> +                                            int ana_freq_khz)
> +{
> +	int nearest = analog_freqs_khz[ana_bcast_type][ana_bcast_system][0];
> +
> +	for (int i = 0; i < NUM_ANALOG_FREQS; i++) {
> +		int freq = analog_freqs_khz[ana_bcast_type][ana_bcast_system][i];
> +
> +		if (abs(ana_freq_khz - freq) < abs(ana_freq_khz - nearest))
> +			nearest = freq;
> +	}
> +	return nearest;
> +}
> +
> +static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
> +{
> +	struct cec_op_tuner_device_info *info = &node->state.tuner_dev_info;
> +	__u8 type;
> +	__u16 freq;
> +	__u8 system;
> +
> +	cec_ops_select_analogue_service(msg, &type, &freq, &system);
> +	if (info->analog.ana_bcast_type < 3 && info->analog.bcast_system < 9) {
> +		int freq_khz = (freq * 625) / 10;
> +		unsigned int nearest = analog_get_nearest_freq(type, system,
> +							       freq_khz);
> +		info->analog.ana_bcast_type = type;
> +		info->analog.ana_freq = (nearest * 10) / 625;
> +		info->analog.bcast_system = system;
> +		return true;
> +	}
> +	return false;
> +}
> +
>   void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
>   {
>   	bool is_bcast = cec_msg_is_broadcast(&msg);
> @@ -105,21 +151,11 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>   		*/
>   
>   	case CEC_MSG_GIVE_TUNER_DEVICE_STATUS: {
> -		if (!cec_has_tuner(1 << me))
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
>   			break;
>   
> -		struct cec_op_tuner_device_info tuner_dev_info = {};
> -
>   		cec_msg_set_reply_to(&msg, &msg);
> -		tuner_dev_info.rec_flag = CEC_OP_REC_FLAG_NOT_USED;
> -		tuner_dev_info.tuner_display_info = CEC_OP_TUNER_DISPLAY_INFO_NONE;
> -		tuner_dev_info.is_analog = false;
> -		tuner_dev_info.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_CHANNEL;
> -		tuner_dev_info.digital.dig_bcast_system = CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C;
> -		tuner_dev_info.digital.channel.channel_number_fmt = CEC_OP_CHANNEL_NUMBER_FMT_1_PART;
> -		tuner_dev_info.digital.channel.minor = 1;
> -
> -		cec_msg_tuner_device_status(&msg, &tuner_dev_info);
> +		cec_msg_tuner_device_status(&msg, &node->state.tuner_dev_info);
>   		transmit(node, &msg);
>   		return;
>   	}
> @@ -128,6 +164,19 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
>   		return;
>   
>   	case CEC_MSG_SELECT_ANALOGUE_SERVICE:
> +		if (!cec_has_tuner(1 << me) && !cec_has_tv(1 << me))
> +			break;
> +
> +		if (node->state.tuner_dev_info.rec_flag == CEC_OP_REC_FLAG_USED) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_REFUSED);
> +			return;
> +		}
> +		if (!analog_set_tuner_dev_info(node, &msg)) {
> +			reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
> +			return;
> +		}
> +		return;
> +
>   	case CEC_MSG_SELECT_DIGITAL_SERVICE:
>   	case CEC_MSG_TUNER_STEP_DECREMENT:
>   	case CEC_MSG_TUNER_STEP_INCREMENT:
> 

thanks,
-- Shuah