RE: [PATCH v2 03/12] staging: typec: tcpci: support port config passed via dt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Heikki Krogerus [mailto:heikki.krogerus@xxxxxxxxxxxxxxx]
> Sent: 2018年3月5日 17:54
> To: Jun Li <jun.li@xxxxxxx>
> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx; linux@xxxxxxxxxxxx;
> a.hajda@xxxxxxxxxxx; mark.rutland@xxxxxxx; yueyao@xxxxxxxxxx;
> Peter Chen <peter.chen@xxxxxxx>; garsilva@xxxxxxxxxxxxxx;
> o_leveque@xxxxxxxxx; shufan_lee@xxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>
> Subject: Re: [PATCH v2 03/12] staging: typec: tcpci: support port config
> passed via dt
> 
> On Mon, Mar 05, 2018 at 08:53:00AM +0000, Jun Li wrote:
> > > On Mon, Feb 26, 2018 at 02:30:53PM +0000, Jun Li wrote:
> > > > > > +	child = of_get_child_by_name(tcpci->dev->of_node,
> "connector");
> > > > > > +	if (!child) {
> > > > > > +		dev_err(tcpci->dev, "failed to get connector node.\n");
> > > > > > +		return -EINVAL;
> > > > > > +	}
> > > > >
> > > > > Why do you need separate child node for the connector? You will
> > > > > always have only one connector per tcpc, i.e. the tcpci already
> > > > > represents the connector and all its capabilities.
> > > > >
> > > > This is my original idea, my understanding is Rob expects those
> > > > properties should apply for a common usb connector node[1], that
> > > > way I need add a child node for it, sorry I didn't make the
> > > > dt-binding patches come first in this series, please see patch 10,11.
> > > >
> > > > [1]
> > > >
> > >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> > > at
> > > >
> > >
> chwork.kernel.org%2Fpatch%2F10231447%2F&data=02%7C01%7Cjun.li%40
> > > nxp.co
> > > >
> > >
> m%7Ce37ed8b084374e241d2e08d57dd1b02a%7C686ea1d3bc2b4c6fa92cd9
> > > 9c5c30163
> > > >
> > >
> 5%7C0%7C0%7C636553261972212376&sdata=hSNiAfXoTTzK3TjjkjWo7OJL7
> > > %2B3gDHT
> > > > I8NO0FQviDd4%3D&reserved=0
> > >
> > > But was the idea really to put properties like the TCPC capabilities
> > > under the usb connector child node? That will force us to extract
> > > the same properties in two different methods in every USB Type-C
> > > driver. One extracting them from DT, and another from other FW
> interfaces and build-in properties.
> > >
> > > To avoid that, let's just expect to get these properties in the node
> > > for tcpc, not the usb connector child.
> >
> > I misunderstood it's better to put typec props under connector node in
> > all cases so we can have a unified typec description. As Rob comments
> > that's only required for multiple connectors for one controller, not
> > for simple connector like my case, I will put those props under tcpc node.
> 
> Hold on! I was not considering multi-port PD/Type-C controllers.
> 
> I'm wrong about the child node forcing us to have two methods for
> extracting the properties in the drivers. It does mean we can not use
> device_property* functions as the child node is not (yet) bind to any struct
> device, but we can still use fwnode_property* functions, which is fine.
> 

Yes, fwnode_property* function can be used in this case.

> So it actually does make sense to define those properties for the
> "connector" node instead of TCPC parent. They are generic "Type-C"
> properties (right?), so we may want to use them with multiport devices as
> well.
> 

Yes, that's the idea of my v2, I will keep this but via fwnode_property*.

> 
> Br,
> 
> --
> heikki
?韬{.n?????%??檩??w?{.n????z谵{???塄}?财??j:+v??????2??璀??摺?囤??z夸z罐?+?????w棹f




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux