Re: [RFC PATCH 3/4] usb: typec: USB Type-C Port Manager (tcpm)

2016-08-03 Thread Guenter Roeck 
On Wed, Aug 3, 2016 at 2:28 AM, Oliver Neukum  wrote:
> On Tue, 2016-08-02 at 13:32 -0700, Guenter Roeck wrote:
>> +static bool svdm_consume_svids(struct tcpm_port *port, const u32
>> *payload,
>> +  int cnt)
>> +{
>> +   struct pd_mode_data *pmdata = >mode_data;
>> +   int i;
>> +
>> +   for (i = 1; i < cnt; i++) {
>> +   u16 svid;
>> +
>> +   svid = (payload[i] >> 16) & 0x;
>> +   if (!svid)
>> +   return false;
>
> Hi,
>
> this looks like an endianness bug.
>

Yes, you are right, and there are lots of those in my code. Thanks for
bringing it up.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/4] usb: typec: USB Type-C Port Manager (tcpm)

2016-08-03 Thread Guenter Roeck 
On Wed, Aug 3, 2016 at 2:18 AM, Oliver Neukum  wrote:
> On Tue, 2016-08-02 at 13:32 -0700, Guenter Roeck wrote:
>> +static int tcpm_set_polarity(struct tcpm_port *port,
>> +enum typec_cc_polarity polarity)
>> +{
>> +   tcpm_log(port, "polarity %d", polarity);
>> +
>> +   port->polarity = polarity;
>> +
>> +   return port->tcpc->set_polarity(port->tcpc, port->polarity);
>> +}
>
> Here you don't care about the result.

Changed to only set port->polarity if the set_polarity callback was successful.

Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/4] usb: typec: USB Type-C Port Manager (tcpm)

2016-08-03 Thread Oliver Neukum 
On Tue, 2016-08-02 at 13:32 -0700, Guenter Roeck wrote:
> +static bool svdm_consume_svids(struct tcpm_port *port, const u32
> *payload,
> +  int cnt)
> +{
> +   struct pd_mode_data *pmdata = >mode_data;
> +   int i;
> +
> +   for (i = 1; i < cnt; i++) {
> +   u16 svid;
> +
> +   svid = (payload[i] >> 16) & 0x;
> +   if (!svid)
> +   return false;

Hi,

this looks like an endianness bug.

Regards
Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/4] usb: typec: USB Type-C Port Manager (tcpm)

2016-08-03 Thread Oliver Neukum 
On Tue, 2016-08-02 at 13:32 -0700, Guenter Roeck wrote:
> +static int tcpm_set_polarity(struct tcpm_port *port,
> +enum typec_cc_polarity polarity)
> +{
> +   tcpm_log(port, "polarity %d", polarity);
> +
> +   port->polarity = polarity;
> +
> +   return port->tcpc->set_polarity(port->tcpc, port->polarity);
> +}

Here you don't care about the result.

> +
> +static int tcpm_set_vconn(struct tcpm_port *port, bool enable)
> +{
> +   int ret = 0;
> +
> +   tcpm_log(port, "vconn:=%d", enable);
> +
> +   ret = port->tcpc->set_vconn(port->tcpc, enable);
> +   if (!ret) {
> +   port->con.vconn_role = enable ? TYPEC_SOURCE
> + : TYPEC_SINK;
> +   typec_set_vconn_role(port->typec_port,
> port->con.vconn_role);
> +   }
> +
> +   return ret;
> +}

But here you do. Which is right?

Regards
Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html