mbox series

[v3,0/3] common SVDM version and VDO from dt

Message ID 20210201133421.408508-1-kyletso@google.com
Headers show
Series common SVDM version and VDO from dt | expand

Message

Kyle Tso Feb. 1, 2021, 1:34 p.m. UTC
patch v2:
https://lore.kernel.org/linux-devicetree/20210131151832.215931-1-kyletso@google.com/

Changes since v2:
=================
usb: typec: Determine common SVDM Versions
- rename the variable and the functions (remove the text "common")
- remove the macro

dt-bindings: connector: Add SVDM VDO properties
- no change

usb: typec: tcpm: Get Sink VDO from fwnode
- use fwnode_property_count_u32 instead to get the count
- revise the error handling

Kyle Tso (3):
  usb: typec: Determine common SVDM Versions
  dt-bindings: connector: Add SVDM VDO properties
  usb: typec: tcpm: Get Sink VDO from fwnode

 .../bindings/connector/usb-connector.yaml     |  11 +
 drivers/usb/typec/altmodes/displayport.c      |   8 +-
 drivers/usb/typec/class.c                     |  21 +-
 drivers/usb/typec/tcpm/tcpm.c                 |  61 +++-
 drivers/usb/typec/ucsi/displayport.c          |  12 +-
 drivers/usb/typec/ucsi/ucsi.c                 |   2 +
 include/dt-bindings/usb/pd.h                  | 311 ++++++++++++++++-
 include/linux/usb/pd_vdo.h                    | 315 ++++++++++++++----
 include/linux/usb/typec.h                     |  10 +
 9 files changed, 661 insertions(+), 90 deletions(-)

Comments

Guenter Roeck Feb. 1, 2021, 3:04 p.m. UTC | #1
On 2/1/21 5:34 AM, Kyle Tso wrote:
> Commit a079973f462a ("usb: typec: tcpm: Remove tcpc_config
> configuration mechanism") removed the tcpc_config which includes the
> Sink VDO and it is not yet added back with fwnode. Add it now.
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes since v2:
> - use fwnode_property_count_u32 instead to get the count
> - revise the error handling
> 
>  drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 979b7ee6473c..9b13e19118f0 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -5677,6 +5677,20 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
>  			port->new_source_frs_current = frs_current;
>  	}
>  
> +	/* sink-vdos is optional */
> +	ret = fwnode_property_count_u32(fwnode, "sink-vdos");
> +	if (ret < 0)
> +		ret = 0;
> +
> +	port->nr_snk_vdo = min(ret, VDO_MAX_OBJECTS);
> +	if (port->nr_snk_vdo) {
> +		ret = fwnode_property_read_u32_array(fwnode, "sink-vdos",
> +						     port->snk_vdo,
> +						     port->nr_snk_vdo);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
>  
>
Hans de Goede Feb. 1, 2021, 3:05 p.m. UTC | #2
Hi,

On 2/1/21 2:34 PM, Kyle Tso wrote:
> patch v2:
> https://lore.kernel.org/linux-devicetree/20210131151832.215931-1-kyletso@google.com/
> 
> Changes since v2:
> =================
> usb: typec: Determine common SVDM Versions
> - rename the variable and the functions (remove the text "common")
> - remove the macro
> 
> dt-bindings: connector: Add SVDM VDO properties
> - no change
> 
> usb: typec: tcpm: Get Sink VDO from fwnode
> - use fwnode_property_count_u32 instead to get the count
> - revise the error handling
> 
> Kyle Tso (3):
>   usb: typec: Determine common SVDM Versions
>   dt-bindings: connector: Add SVDM VDO properties
>   usb: typec: tcpm: Get Sink VDO from fwnode

I wanted to point out that I have a somewhat related series "pending".

I put quotes around pending because it has been reviewed quite a while
ago and have not managed to make the time to post a new version since then.

My series is somewhat/mostly orthogonal, but I think it is good
to keep it in mind since it also is about specifying VDOs, but then
for alternate-modes, see:

https://lore.kernel.org/linux-usb/20200714113617.10470-1-hdegoede@redhat.com/

And I think there might be some overlap with the last patch in this series,
although that does not call typec_port_register_altmode(). 

Regards,

Hans




p.s.

I was actually planning on replying to an earlier version of
this series, *after* I posted a new version of my own series,
but I'm swamped so it looks like I will not get around to posting
a new version of my own series anytime soon.
Heikki Krogerus Feb. 2, 2021, 8:22 a.m. UTC | #3
On Mon, Feb 01, 2021 at 09:34:21PM +0800, Kyle Tso wrote:
> Commit a079973f462a ("usb: typec: tcpm: Remove tcpc_config

> configuration mechanism") removed the tcpc_config which includes the

> Sink VDO and it is not yet added back with fwnode. Add it now.

> 

> Signed-off-by: Kyle Tso <kyletso@google.com>


Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>


> ---

> Changes since v2:

> - use fwnode_property_count_u32 instead to get the count

> - revise the error handling

> 

>  drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++++

>  1 file changed, 14 insertions(+)

> 

> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c

> index 979b7ee6473c..9b13e19118f0 100644

> --- a/drivers/usb/typec/tcpm/tcpm.c

> +++ b/drivers/usb/typec/tcpm/tcpm.c

> @@ -5677,6 +5677,20 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,

>  			port->new_source_frs_current = frs_current;

>  	}

>  

> +	/* sink-vdos is optional */

> +	ret = fwnode_property_count_u32(fwnode, "sink-vdos");

> +	if (ret < 0)

> +		ret = 0;

> +

> +	port->nr_snk_vdo = min(ret, VDO_MAX_OBJECTS);

> +	if (port->nr_snk_vdo) {

> +		ret = fwnode_property_read_u32_array(fwnode, "sink-vdos",

> +						     port->snk_vdo,

> +						     port->nr_snk_vdo);

> +		if (ret < 0)

> +			return ret;

> +	}

> +

>  	return 0;

>  }

>  

> -- 

> 2.30.0.365.g02bc693789-goog


thanks,

-- 
heikki
Kyle Tso Feb. 2, 2021, 8:37 a.m. UTC | #4
On Mon, Feb 1, 2021 at 11:35 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Mon, Feb 01, 2021 at 09:34:19PM +0800, Kyle Tso wrote:
> > PD Spec Revision 3.0 Version 2.0 + ECNs 2020-12-10
> >   6.4.4.2.3 Structured VDM Version
> >   "The Structured VDM Version field of the Discover Identity Command
> >   sent and received during VDM discovery Shall be used to determine the
> >   lowest common Structured VDM Version supported by the Port Partners or
> >   Cable Plug and Shall continue to operate using this Specification
> >   Revision until they are Detached."
> >
> > Also clear the fields newly defined in SVDM version 2.0 for
> > compatibilities. And fix some VDO definitions changed in the Spec.
> >
> > Signed-off-by: Kyle Tso <kyletso@google.com>
> > ---
> > Changes since v2:
> > - rename the variable and the functions (remove the text "common")
> > - remove the macro
> >
> >  drivers/usb/typec/altmodes/displayport.c |   8 +-
> >  drivers/usb/typec/class.c                |  21 +-
> >  drivers/usb/typec/tcpm/tcpm.c            |  47 +++-
> >  drivers/usb/typec/ucsi/displayport.c     |  12 +-
> >  drivers/usb/typec/ucsi/ucsi.c            |   2 +
> >  include/linux/usb/pd_vdo.h               | 315 +++++++++++++++++------
> >  include/linux/usb/typec.h                |  10 +
> >  7 files changed, 326 insertions(+), 89 deletions(-)
>
> I think there is some room to split this one at least a little. The
> changes to the class (so drivers/usb/typec/class.c and
> include/linux/usb/typec.h) could be introduced separately at least.
>
> So I see there are two changes in this patch. You are modifying the
> class, and then there are the updated PD definitions. Both should be
> introduced in separate patches IMO. I think also each driver (so
> ucsi.c and tcpm.c) can handle the changes to the class in its own
> patch. The modifications to the PD definitions and updated VDO() and
> so on can be handled in a single patch I guess.
>
I will separate this patch to different ones.

thanks,
Kyle

>
> thanks,
>
> --
> heikki