Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1493915 > unrolled thread

RE: [RFC PATCH v3 1/2] usb: typec: USB Type-C Port Manager (tcpm)

Started byJun Li <jun.li@nxp.com>
First post2016-09-30 09:00 +0200
Last post2016-09-30 23:10 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  RE: [RFC PATCH v3 1/2] usb: typec: USB Type-C Port Manager (tcpm) Jun Li <jun.li@nxp.com> - 2016-09-30 09:00 +0200
    Re: [RFC PATCH v3 1/2] usb: typec: USB Type-C Port Manager (tcpm) Guenter Roeck <groeck@google.com> - 2016-09-30 21:10 +0200
      Re: [RFC PATCH v3 1/2] usb: typec: USB Type-C Port Manager (tcpm) Joe Perches <joe@perches.com> - 2016-09-30 21:50 +0200
        Re: [RFC PATCH v3 1/2] usb: typec: USB Type-C Port Manager (tcpm) Guenter Roeck <groeck@google.com> - 2016-09-30 23:00 +0200
          Re: [RFC PATCH v3 1/2] usb: typec: USB Type-C Port Manager (tcpm) Joe Perches <joe@perches.com> - 2016-09-30 23:10 +0200

#1493915 — RE: [RFC PATCH v3 1/2] usb: typec: USB Type-C Port Manager (tcpm)

FromJun Li <jun.li@nxp.com>
Date2016-09-30 09:00 +0200
SubjectRE: [RFC PATCH v3 1/2] usb: typec: USB Type-C Port Manager (tcpm)
Message-ID<smZXA-6Lh-9@gated-at.bofh.it>
Hi Guenter,
> -----Original Message-----
> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> owner@vger.kernel.org] On Behalf Of Guenter Roeck
> Sent: Wednesday, August 24, 2016 5:11 AM
> To: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: Chandra Sekhar Anagani <chandra.sekhar.anagani@intel.com>; Bruce
> Ashfield <bruce.ashfield@windriver.com>; Bin Gao <bin.gao@intel.com>;
> Pranav Tipnis <pranav.tipnis@intel.com>; Heikki Krogerus
> <heikki.krogerus@linux.intel.com>; linux-kernel@vger.kernel.org; linux-
> usb@vger.kernel.org; Guenter Roeck <groeck@chromium.org>
> Subject: [RFC PATCH v3 1/2] usb: typec: USB Type-C Port Manager (tcpm)
> 
...
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> new file mode 100644
> index 000000000000..6b1679af7a25
> --- /dev/null
> +++ b/include/linux/usb/pd.h

...

> +#define PDO_VAR(min_mv, max_mv, max_ma)					\
> +	((PDO_TYPE_VAR << PDO_TYPE_SHIFT) |				\
> +	 ((((min_mv) / 50) & PDO_VAR_MIN_VOLT_MASK) <<			\
> +	  PDO_VAR_MIN_VOLT_SHIFT) |					\
> +	 ((((max_mv) / 50) & PDO_VAR_MAX_VOLT_MASK) <<			\
> +	  PDO_VAR_MAX_VOLT_SHIFT) |					\
> +	 ((((max_ma) / 50) & PDO_VAR_MAX_CURR_MASK) <<			\

((((max_ma) / 10) & PDO_VAR_MAX_CURR_MASK) <<                  \

Li Jun

[toc] | [next] | [standalone]


#1494239

FromGuenter Roeck <groeck@google.com>
Date2016-09-30 21:10 +0200
Message-ID<snbm2-60q-25@gated-at.bofh.it>
In reply to#1493915
On Thu, Sep 29, 2016 at 11:37 PM, Jun Li <jun.li@nxp.com> wrote:
> Hi Guenter,
>> -----Original Message-----
>> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
>> owner@vger.kernel.org] On Behalf Of Guenter Roeck
>> Sent: Wednesday, August 24, 2016 5:11 AM
>> To: Felipe Balbi <felipe.balbi@linux.intel.com>
>> Cc: Chandra Sekhar Anagani <chandra.sekhar.anagani@intel.com>; Bruce
>> Ashfield <bruce.ashfield@windriver.com>; Bin Gao <bin.gao@intel.com>;
>> Pranav Tipnis <pranav.tipnis@intel.com>; Heikki Krogerus
>> <heikki.krogerus@linux.intel.com>; linux-kernel@vger.kernel.org; linux-
>> usb@vger.kernel.org; Guenter Roeck <groeck@chromium.org>
>> Subject: [RFC PATCH v3 1/2] usb: typec: USB Type-C Port Manager (tcpm)
>>
> ...
>> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
>> new file mode 100644
>> index 000000000000..6b1679af7a25
>> --- /dev/null
>> +++ b/include/linux/usb/pd.h
>
> ...
>
>> +#define PDO_VAR(min_mv, max_mv, max_ma)                                      \
>> +     ((PDO_TYPE_VAR << PDO_TYPE_SHIFT) |                             \
>> +      ((((min_mv) / 50) & PDO_VAR_MIN_VOLT_MASK) <<                  \
>> +       PDO_VAR_MIN_VOLT_SHIFT) |                                     \
>> +      ((((max_mv) / 50) & PDO_VAR_MAX_VOLT_MASK) <<                  \
>> +       PDO_VAR_MAX_VOLT_SHIFT) |                                     \
>> +      ((((max_ma) / 50) & PDO_VAR_MAX_CURR_MASK) <<                  \
>
> ((((max_ma) / 10) & PDO_VAR_MAX_CURR_MASK) <<                  \
>

Thanks, fixed. PDO_BATT has a similar problem, which I noticed while
fixing the above.

Guenter

[toc] | [prev] | [next] | [standalone]


#1494257

FromJoe Perches <joe@perches.com>
Date2016-09-30 21:50 +0200
Message-ID<snbYJ-6eC-1@gated-at.bofh.it>
In reply to#1494239
On Fri, 2016-09-30 at 12:06 -0700, Guenter Roeck wrote:
> On Thu, Sep 29, 2016 at 11:37 PM, Jun Li <jun.li@nxp.com> wrote:
[]
> > diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
[]
> > +#define PDO_VAR(min_mv, max_mv, max_ma)                                      \
> > +     ((PDO_TYPE_VAR << PDO_TYPE_SHIFT) |                             \
> > +      ((((min_mv) / 50) & PDO_VAR_MIN_VOLT_MASK) <<                  \
> > +       PDO_VAR_MIN_VOLT_SHIFT) |                                     \
> > +      ((((max_mv) / 50) & PDO_VAR_MAX_VOLT_MASK) <<                  \
> > +       PDO_VAR_MAX_VOLT_SHIFT) |                                     \
> > +      ((((max_ma) / 50) & PDO_VAR_MAX_CURR_MASK) <<                  \
> 
> 
> ((((max_ma) / 10) & PDO_VAR_MAX_CURR_MASK) <<                  \

This would be easier to read if laid out differently.

#define PDO_VAR(min_mv, max_mv, max_ma)							\
	((PDO_TYPE_VAR << PDO_TYPE_SHIFT) |						\
	 ((((min_mv) / 50) & PDO_VAR_MIN_VOLT_MASK) << PDO_VAR_MIN_VOLT_SHIFT) |	\
	 ((((max_mv) / 50) & PDO_VAR_MAX_VOLT_MASK) << PDO_VAR_MAX_VOLT_SHIFT) |	\
	 ((((max_ma) / 10) & PDO_VAR_MAX_CURR_MASK) << PDO_VAR_MAX_CURR_SHIFT))

[toc] | [prev] | [next] | [standalone]


#1494262

FromGuenter Roeck <groeck@google.com>
Date2016-09-30 23:00 +0200
Message-ID<snd4t-6Ws-1@gated-at.bofh.it>
In reply to#1494257
On Fri, Sep 30, 2016 at 12:41 PM, Joe Perches <joe@perches.com> wrote:
> On Fri, 2016-09-30 at 12:06 -0700, Guenter Roeck wrote:
>> On Thu, Sep 29, 2016 at 11:37 PM, Jun Li <jun.li@nxp.com> wrote:
> []
>> > diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> []
>> > +#define PDO_VAR(min_mv, max_mv, max_ma)                                      \
>> > +     ((PDO_TYPE_VAR << PDO_TYPE_SHIFT) |                             \
>> > +      ((((min_mv) / 50) & PDO_VAR_MIN_VOLT_MASK) <<                  \
>> > +       PDO_VAR_MIN_VOLT_SHIFT) |                                     \
>> > +      ((((max_mv) / 50) & PDO_VAR_MAX_VOLT_MASK) <<                  \
>> > +       PDO_VAR_MAX_VOLT_SHIFT) |                                     \
>> > +      ((((max_ma) / 50) & PDO_VAR_MAX_CURR_MASK) <<                  \
>>
>>
>> ((((max_ma) / 10) & PDO_VAR_MAX_CURR_MASK) <<                  \
>
> This would be easier to read if laid out differently.
>
> #define PDO_VAR(min_mv, max_mv, max_ma)                                                 \
>         ((PDO_TYPE_VAR << PDO_TYPE_SHIFT) |                                             \
>          ((((min_mv) / 50) & PDO_VAR_MIN_VOLT_MASK) << PDO_VAR_MIN_VOLT_SHIFT) |        \
>          ((((max_mv) / 50) & PDO_VAR_MAX_VOLT_MASK) << PDO_VAR_MAX_VOLT_SHIFT) |        \
>          ((((max_ma) / 10) & PDO_VAR_MAX_CURR_MASK) << PDO_VAR_MAX_CURR_SHIFT))
>

Code now looks as follows.

#define PDO_VAR_MIN_VOLT(mv) ((((mv) / 50) & PDO_VAR_MIN_VOLT_MASK) << \
                              PDO_VAR_MIN_VOLT_SHIFT)
#define PDO_VAR_MAX_VOLT(mv) ((((mv) / 50) & PDO_VAR_MAX_VOLT_MASK) << \
                              PDO_VAR_MAX_VOLT_SHIFT)
#define PDO_VAR_MAX_CURR(ma) ((((ma) / 10) & PDO_VAR_MAX_CURR_MASK) << \
                              PDO_VAR_MAX_CURR_SHIFT)

#define PDO_VAR(min_mv, max_mv, max_ma)                         \
        (PDO_TYPE(PDO_TYPE_VAR) | PDO_VAR_MIN_VOLT(min_mv) |    \
         PDO_VAR_MAX_VOLT(max_mv) | PDO_VAR_MAX_CURR(max_ma))

Though maybe I should just ignore line length limits or use shorter defines.

Guenter

[toc] | [prev] | [next] | [standalone]


#1494264

FromJoe Perches <joe@perches.com>
Date2016-09-30 23:10 +0200
Message-ID<sndea-7hu-11@gated-at.bofh.it>
In reply to#1494262
On Fri, 2016-09-30 at 13:57 -0700, Guenter Roeck wrote:
> Code now looks as follows.
> 
> #define PDO_VAR_MIN_VOLT(mv) ((((mv) / 50) & PDO_VAR_MIN_VOLT_MASK) << \
>                               PDO_VAR_MIN_VOLT_SHIFT)
> #define PDO_VAR_MAX_VOLT(mv) ((((mv) / 50) & PDO_VAR_MAX_VOLT_MASK) << \
>                               PDO_VAR_MAX_VOLT_SHIFT)
> #define PDO_VAR_MAX_CURR(ma) ((((ma) / 10) & PDO_VAR_MAX_CURR_MASK) << \
>                               PDO_VAR_MAX_CURR_SHIFT)

When #defines are continued, I generally find it nicer to have the
entire definition on a separate line

#define PDO_VAR_MIN_VOLT(mv)						\
	((((mv) / 50) & PDO_VAR_MIN_VOLT_MASK) << PDO_VAR_MIN_VOLT_SHIFT)

> Though maybe I should just ignore line length limits or use shorter defines.

When it makes sense, yes please.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web