Manu Abraham wrote:
> Hello,
>
> Someone with a Twinhan card having diseqc equipment test this patch ?
> One needs to apply the cleanup patch and then the diseqc patch.
>
> Note:
> In the patch,
>
> + if (cmd->msg_len == 0 || cmd->msg_len > 5)
> return -EINVAL;
>
> please do change
>
> cmd->msg_len > 5 to cmd->msg_len > 6
>
> Thanks,
> Manu
>
>[snip]
>
>
>--- dst.c.orig 2005-10-24 15:50:24.000000000 +0400
>+++ dst.c 2005-10-24 15:47:37.000000000 +0400
>@@ -630,7 +630,7 @@ struct dst_types dst_tlist[] = {
> .offset = 0,
> .dst_type = DST_TYPE_IS_SAT,
> .type_flags = DST_TYPE_HAS_SYMDIV | DST_TYPE_HAS_FW_1,
>- .dst_feature = 0
>+ .dst_feature = DST_TYPE_HAS_DISEQC3
> }, /* obsolete */
>
> {
>@@ -639,7 +639,8 @@ struct dst_types dst_tlist[] = {
> .dst_type = DST_TYPE_IS_SAT,
> .type_flags = DST_TYPE_HAS_TS204 | DST_TYPE_HAS_NEWTUNE
> | DST_TYPE_HAS_FW_1,
>- .dst_feature = 0
>+
>+ .dst_feature = DST_TYPE_HAS_DISEQC3 | DST_TYPE_HAS_DISEQC4
> }, /* obsolete */
>
> {
>@@ -659,7 +660,8 @@ struct dst_types dst_tlist[] = {
> .offset = 0,
> .dst_type = DST_TYPE_IS_SAT,
> .type_flags = DST_TYPE_HAS_SYMDIV | DST_TYPE_HAS_FW_1,
>- .dst_feature = 0
>+ .dst_feature = DST_TYPE_HAS_DISEQC3 | DST_TYPE_HAS_DISEQC4 |
>+ DST_TYPE_HAS_DISEQC5
> }, /* obsolete */
>
> {
>@@ -669,7 +671,8 @@ struct dst_types dst_tlist[] = {
> .type_flags = DST_TYPE_HAS_TS204 | DST_TYPE_HAS_NEWTUNE |
> DST_TYPE_HAS_FW_1,
>
>- .dst_feature = DST_TYPE_HAS_CA
>+ .dst_feature = DST_TYPE_HAS_CA | DST_TYPE_HAS_DISEQC3 |
>+ DST_TYPE_HAS_DISEQC4 | DST_TYPE_HAS_DISEQC5
> }, /* An OEM board */
>
> {
>@@ -680,7 +683,7 @@ struct dst_types dst_tlist[] = {
> DST_TYPE_HAS_FW_BUILD | DST_TYPE_HAS_INC_COUNT,
>
> .dst_feature = DST_TYPE_HAS_CA | DST_TYPE_HAS_DISEQC3 |
>- DST_TYPE_HAS_DISEQC4 | DST_TYPE_HAS_MOTO |
>+ DST_TYPE_HAS_DISEQC4 | DST_TYPE_HAS_DISEQC5 |
> DST_TYPE_HAS_MAC
> },
>
>@@ -1260,9 +1263,20 @@ static int dst_set_diseqc(struct dvb_fro
>
> if (state->dst_type != DST_TYPE_IS_SAT)
> return 0;
>- if (cmd->msg_len == 0 || cmd->msg_len > 4)
>+// if (cmd->msg_len == 0 || cmd->msg_len > 4)
>+// return -EINVAL;
>+ if (cmd->msg_len == 0 || cmd->msg_len > 5)
>
>
Ok, this should be "6", not "5" as you said...
> return -EINVAL;
>- memcpy(&paket[3], cmd->msg, cmd->msg_len);
>
>
Why not keep this line, but put it after the checks?
>+ if ((cmd->msg_len == 3) && (state->dst_hw_cap & DST_TYPE_HAS_DISEQC3)) {
>+ memcpy(&paket[3], cmd->msg, cmd->msg_len);
>+ paket[2] = 0x03;
>+ } else if ((cmd->msg_len == 4) && (state->dst_hw_cap & DST_TYPE_HAS_DISEQC4)) {
>+ memcpy(&paket[3], cmd->msg, cmd->msg_len);
>+ paket[2] = 0x04;
>+ } else if ((cmd->msg_len == 5) && (state->dst_hw_cap & DST_TYPE_HAS_DISEQC5)) {
>+ memcpy(&paket[2], cmd->msg, cmd->msg_len);
>+ }
>
>
Need:
} else
return -EINVAL;
here... And then ... why not set:
paket[2] = cmd->msg_len;
here instead? And then following it with the above memcpy():
memcpy(&paket[2], cmd->msg, cmd->msg_len);
The case of "msg_len == 6" never gets handled properly, however.
I couldn't find a case of DST_TYPE_HAS_MOTO being tested
anywhere...
>+
> paket[7] = dst_check_sum(&paket[0], 7);
> dst_command(state, paket, 8);
> return 0;
>
>
>
In short, write the code as:
if (state->dst_type != DST_TYPE_IS_SAT)
return 0;
if (cmd->msg_len == 0 || cmd->msg_len > 6)
return -EINVAL;
if ((cmd->msg_len == 3) && (state->dst_hw_cap & DST_TYPE_HAS_DISEQC3)) {
;
} else if ((cmd->msg_len == 4) && (state->dst_hw_cap & DST_TYPE_HAS_DISEQC4)) {
;
} else if ((cmd->msg_len == 5) && (state->dst_hw_cap & DST_TYPE_HAS_DISEQC5)) {
;
} else
return -EINVAL;
memcpy(&paket[3], cmd->msg, cmd->msg_len);
paket[2] = cmd->msg_len;
paket[7] = dst_check_sum(&paket[0], 7);
dst_command(state, paket, 8);
return 0;
Note that "msg_len == 6" will be handled by the "return -EINVAL".
