| Message ID | 20240205164316.805408-2-m.felsch@pengutronix.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | USB-C TCPM Orientation Support | expand |
On 05/02/2024 17:43, Marco Felsch wrote: > This binding descripes the generic TCPCI specification [1]. So add the Typo: describes. No, this binding describes PTN5110, not generic TCPCI. This is not accurate commit description. > generic binding support since which can be used if an different TCPC is > used compatible which is compatible to [1]. Sorry, cannot parse it. Please run it through native speaker, Google grammar check, ChatGPT or some other way. > > [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > --- > Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml > index eaedb4cc6b6c..7bd7bbbac9e0 100644 > --- a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml > +++ b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml > @@ -11,7 +11,9 @@ maintainers: > > properties: > compatible: > - const: nxp,ptn5110 > + enum: > + - nxp,ptn5110 > + - tcpci I don't think this is correct. First, this is binding for NXP chip, so why generic implementation should be here? I would expect it in its own dedicated binding. Second, we rarely want generic compatibles. Care to share more details? Are all details expected to follow spec, without need of quirks? Best regards, Krzysztof
On 24-02-06, Krzysztof Kozlowski wrote: > On 05/02/2024 17:43, Marco Felsch wrote: > > This binding descripes the generic TCPCI specification [1]. So add the > > Typo: describes. Argh. > No, this binding describes PTN5110, not generic TCPCI. This is not > accurate commit description. This binding is currently missued if another TCPCI conform chip is used which requires no special handling. I could have dropped this commit since the 'tcpci' is already present at i2c-device-id level. > > > generic binding support since which can be used if an different TCPC is > > used compatible which is compatible to [1]. > > Sorry, cannot parse it. Please run it through native speaker, Google > grammar check, ChatGPT or some other way. Argh.. you're right, sorry. I will rephrase it. > > [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > --- > > Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml > > index eaedb4cc6b6c..7bd7bbbac9e0 100644 > > --- a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml > > +++ b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml > > @@ -11,7 +11,9 @@ maintainers: > > > > properties: > > compatible: > > - const: nxp,ptn5110 > > + enum: > > + - nxp,ptn5110 > > + - tcpci > > I don't think this is correct. First, this is binding for NXP chip, so > why generic implementation should be here? I would expect it in its own > dedicated binding. The nxp,ptn5110 device was the first driver which implements an TCPCI conform driver. The driver already support the tcpci binding for i2c-id devices as I mentioned above. IMHO this whole binding (file) should be converted and the nxp,ptn5110 compatible should be marked as deprecated. > Second, we rarely want generic compatibles. Care to share more details? As said above this particular NXP chip is an TCPCI conform chip. There is nothing special about it. There are other vendors like OnSemi (in my case) which implement also an TCPCI conform chip. The (Linux) driver already binds to the generic tcpci compatible if the i2c-core falls back to the i2c-device id. It's even more confusing that the i2c-id supports only the generic binding the of-compatible support only the specifc one. > Are all details expected to follow spec, without need of quirks? Please see above, I hope this helps. Regards, Marco > > Best regards, > Krzysztof > >
On 24-02-06, Marco Felsch wrote: > On 24-02-06, Krzysztof Kozlowski wrote: > > On 05/02/2024 17:43, Marco Felsch wrote: > > > This binding descripes the generic TCPCI specification [1]. So add the > > > > Typo: describes. > > Argh. > > > No, this binding describes PTN5110, not generic TCPCI. This is not > > accurate commit description. > > This binding is currently missued if another TCPCI conform chip is used /missused/misused/ > which requires no special handling. I could have dropped this commit > since the 'tcpci' is already present at i2c-device-id level. > > > > > > generic binding support since which can be used if an different TCPC is > > > used compatible which is compatible to [1]. > > > > Sorry, cannot parse it. Please run it through native speaker, Google > > grammar check, ChatGPT or some other way. > > Argh.. you're right, sorry. I will rephrase it. > > > > [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf > > > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > --- > > > Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml > > > index eaedb4cc6b6c..7bd7bbbac9e0 100644 > > > --- a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml > > > +++ b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml > > > @@ -11,7 +11,9 @@ maintainers: > > > > > > properties: > > > compatible: > > > - const: nxp,ptn5110 > > > + enum: > > > + - nxp,ptn5110 > > > + - tcpci > > > > I don't think this is correct. First, this is binding for NXP chip, so > > why generic implementation should be here? I would expect it in its own > > dedicated binding. > > The nxp,ptn5110 device was the first driver which implements an TCPCI > conform driver. The driver already support the tcpci binding for i2c-id > devices as I mentioned above. IMHO this whole binding (file) should be > converted and the nxp,ptn5110 compatible should be marked as deprecated. > > > Second, we rarely want generic compatibles. Care to share more details? > > As said above this particular NXP chip is an TCPCI conform chip. There > is nothing special about it. There are other vendors like OnSemi (in my > case) which implement also an TCPCI conform chip. The (Linux) driver > already binds to the generic tcpci compatible if the i2c-core falls back > to the i2c-device id. It's even more confusing that the i2c-id supports > only the generic binding the of-compatible support only the specifc one. > > > Are all details expected to follow spec, without need of quirks? > > Please see above, I hope this helps. > > Regards, > Marco > > > > > Best regards, > > Krzysztof > > > > > >
On 06/02/2024 15:52, Marco Felsch wrote: > On 24-02-06, Krzysztof Kozlowski wrote: >> On 05/02/2024 17:43, Marco Felsch wrote: >>> This binding descripes the generic TCPCI specification [1]. So add the >> >> Typo: describes. > > Argh. > >> No, this binding describes PTN5110, not generic TCPCI. This is not >> accurate commit description. > > This binding is currently missued if another TCPCI conform chip is used Why would people misuse binding instead of doing things properly? :) > which requires no special handling. I could have dropped this commit > since the 'tcpci' is already present at i2c-device-id level. > >> >>> generic binding support since which can be used if an different TCPC is >>> used compatible which is compatible to [1]. >> >> Sorry, cannot parse it. Please run it through native speaker, Google >> grammar check, ChatGPT or some other way. > > Argh.. you're right, sorry. I will rephrase it. > >>> [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf >>> >>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> >>> --- >>> Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml >>> index eaedb4cc6b6c..7bd7bbbac9e0 100644 >>> --- a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml >>> +++ b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml >>> @@ -11,7 +11,9 @@ maintainers: >>> >>> properties: >>> compatible: >>> - const: nxp,ptn5110 >>> + enum: >>> + - nxp,ptn5110 >>> + - tcpci >> >> I don't think this is correct. First, this is binding for NXP chip, so >> why generic implementation should be here? I would expect it in its own >> dedicated binding. > > The nxp,ptn5110 device was the first driver which implements an TCPCI > conform driver. The driver already support the tcpci binding for i2c-id > devices as I mentioned above. IMHO this whole binding (file) should be > converted and the nxp,ptn5110 compatible should be marked as deprecated. You speak about driver, but I was speaking about binding. > >> Second, we rarely want generic compatibles. Care to share more details? > > As said above this particular NXP chip is an TCPCI conform chip. There > is nothing special about it. There are other vendors like OnSemi (in my > case) which implement also an TCPCI conform chip. The (Linux) driver > already binds to the generic tcpci compatible if the i2c-core falls back > to the i2c-device id. It's even more confusing that the i2c-id supports > only the generic binding the of-compatible support only the specifc one. I don't know much about TCPCI, so maybe questions are obvious: you are claiming that there will be no differentiating hardware element, like reset-gpios or power supply for none of TCPCI-conforming chips? All of them will never need any different hardware configuration? Is this what you claim? Just to remind: there was such claim for USB and PCI till we figured out it was simply wrong and we are living now with on-board hubs and PCI power-sequencing stuff. > >> Are all details expected to follow spec, without need of quirks? > > Please see above, I hope this helps. Sorry, doesn't. You still speak about driver and how it can bind to something. I did not ask about this at all. To be clear: WE ARE NOT TALKING ABOUT LINUX DRIVER. We talk about hardware and how it is represented in Devicetree, including its supplies, pins, GPIOs and any ideas hardware engineers like to bring. Best regards, Krzysztof
On 24-02-06, Krzysztof Kozlowski wrote: > On 06/02/2024 15:52, Marco Felsch wrote: > > On 24-02-06, Krzysztof Kozlowski wrote: > >> On 05/02/2024 17:43, Marco Felsch wrote: > >>> This binding descripes the generic TCPCI specification [1]. So add the > >> > >> Typo: describes. > > > > Argh. > > > >> No, this binding describes PTN5110, not generic TCPCI. This is not > >> accurate commit description. > > > > This binding is currently missued if another TCPCI conform chip is used > > Why would people misuse binding instead of doing things properly? :) You know people... ;) ... > >>> properties: > >>> compatible: > >>> - const: nxp,ptn5110 > >>> + enum: > >>> + - nxp,ptn5110 > >>> + - tcpci > >> > >> I don't think this is correct. First, this is binding for NXP chip, so > >> why generic implementation should be here? I would expect it in its own > >> dedicated binding. > > > > The nxp,ptn5110 device was the first driver which implements an TCPCI > > conform driver. The driver already support the tcpci binding for i2c-id > > devices as I mentioned above. IMHO this whole binding (file) should be > > converted and the nxp,ptn5110 compatible should be marked as deprecated. > > You speak about driver, but I was speaking about binding. I know and I was afraid of mention the driver within this conversation since this is all about bindings and devices :) Nevertheless this particular NXP device does support the generic "tcpci" compatible already. The support is pulled indirectly via the i2c_device_id.name which is in the end used for of/acpi/legacy devices. > >> Second, we rarely want generic compatibles. Care to share more details? > > > > As said above this particular NXP chip is an TCPCI conform chip. There > > is nothing special about it. There are other vendors like OnSemi (in my > > case) which implement also an TCPCI conform chip. The (Linux) driver > > already binds to the generic tcpci compatible if the i2c-core falls back > > to the i2c-device id. It's even more confusing that the i2c-id supports > > only the generic binding the of-compatible support only the specifc one. > > I don't know much about TCPCI, so maybe questions are obvious: you are > claiming that there will be no differentiating hardware element, like > reset-gpios or power supply for none of TCPCI-conforming chips? All of > them will never need any different hardware configuration? Of course TCPCI doesn't mention reset gpios or power supplies but if you use this argumentation the already supported NXP device shouldn't be available too since the binding is missing the VDD supply ;) Since we never break compatibility, the vdd-supply have to be optional and the same can be done for reset-gpios. > Is this what you claim? Please see above. > Just to remind: there was such claim for USB and PCI till we figured out > it was simply wrong and we are living now with on-board hubs and PCI > power-sequencing stuff. Don't get me wrong, I get your point. In the end I don't care and can copy'n'paste the whole file and change the compatible to the OnSemi device or I can add the dedicated OnSemi compatible to this file. But I don't wanted to add an 2nd specific compatible while the device already supports the generic one but via i2c_device_id.name. Therefore I aligned the i2c_device_id with the of_device_id. > >> Are all details expected to follow spec, without need of quirks? > > > > Please see above, I hope this helps. > > Sorry, doesn't. You still speak about driver and how it can bind to > something. I did not ask about this at all. > > To be clear: > WE ARE NOT TALKING ABOUT LINUX DRIVER. I KNOW > We talk about hardware and how it is represented in Devicetree, > including its supplies, pins, GPIOs and any ideas hardware engineers > like to bring. I Know that too. Regards, Marco
On 07/02/2024 10:05, Marco Felsch wrote: > On 24-02-06, Krzysztof Kozlowski wrote: >> On 06/02/2024 15:52, Marco Felsch wrote: >>> On 24-02-06, Krzysztof Kozlowski wrote: >>>> On 05/02/2024 17:43, Marco Felsch wrote: >>>>> This binding descripes the generic TCPCI specification [1]. So add the >>>> >>>> Typo: describes. >>> >>> Argh. >>> >>>> No, this binding describes PTN5110, not generic TCPCI. This is not >>>> accurate commit description. >>> >>> This binding is currently missued if another TCPCI conform chip is used >> >> Why would people misuse binding instead of doing things properly? :) > > You know people... ;) > > ... > >>>>> properties: >>>>> compatible: >>>>> - const: nxp,ptn5110 >>>>> + enum: >>>>> + - nxp,ptn5110 >>>>> + - tcpci >>>> >>>> I don't think this is correct. First, this is binding for NXP chip, so >>>> why generic implementation should be here? I would expect it in its own >>>> dedicated binding. >>> >>> The nxp,ptn5110 device was the first driver which implements an TCPCI >>> conform driver. The driver already support the tcpci binding for i2c-id >>> devices as I mentioned above. IMHO this whole binding (file) should be >>> converted and the nxp,ptn5110 compatible should be marked as deprecated. >> >> You speak about driver, but I was speaking about binding. > > I know and I was afraid of mention the driver within this conversation > since this is all about bindings and devices :) > > Nevertheless this particular NXP device does support the generic "tcpci" > compatible already. The support is pulled indirectly via the > i2c_device_id.name which is in the end used for of/acpi/legacy devices. > >>>> Second, we rarely want generic compatibles. Care to share more details? >>> >>> As said above this particular NXP chip is an TCPCI conform chip. There >>> is nothing special about it. There are other vendors like OnSemi (in my >>> case) which implement also an TCPCI conform chip. The (Linux) driver >>> already binds to the generic tcpci compatible if the i2c-core falls back >>> to the i2c-device id. It's even more confusing that the i2c-id supports >>> only the generic binding the of-compatible support only the specifc one. >> >> I don't know much about TCPCI, so maybe questions are obvious: you are >> claiming that there will be no differentiating hardware element, like >> reset-gpios or power supply for none of TCPCI-conforming chips? All of >> them will never need any different hardware configuration? > > Of course TCPCI doesn't mention reset gpios or power supplies but if you > use this argumentation the already supported NXP device shouldn't be > available too since the binding is missing the VDD supply ;) Since we The existing binding is incomplete and maybe, as you suggested, misused, but this is not a reason to make it worse. > never break compatibility, the vdd-supply have to be optional and the > same can be done for reset-gpios. So the answer to my questions is: They will not be 100% identical and they will need customization? > >> Is this what you claim? > > Please see above. > >> Just to remind: there was such claim for USB and PCI till we figured out >> it was simply wrong and we are living now with on-board hubs and PCI >> power-sequencing stuff. > > Don't get me wrong, I get your point. In the end I don't care and can > copy'n'paste the whole file and change the compatible to the OnSemi > device or I can add the dedicated OnSemi compatible to this file. But I > don't wanted to add an 2nd specific compatible while the device already > supports the generic one but via i2c_device_id.name. Therefore I aligned > the i2c_device_id with the of_device_id. You can add generic compatible used as fallback. That's pretty common practice. > >>>> Are all details expected to follow spec, without need of quirks? >>> >>> Please see above, I hope this helps. >> >> Sorry, doesn't. You still speak about driver and how it can bind to >> something. I did not ask about this at all. >> >> To be clear: >> WE ARE NOT TALKING ABOUT LINUX DRIVER. > > I KNOW > >> We talk about hardware and how it is represented in Devicetree, >> including its supplies, pins, GPIOs and any ideas hardware engineers >> like to bring. Then terms "driver" and "binding" (or matching) do not fit here as arguments whether specific compatible should be there or not. There is guideline for that: writing bindings, which exactly, 100% covers this thing here. Best regards, Krzysztof
On 24-02-07, Krzysztof Kozlowski wrote: > On 07/02/2024 10:05, Marco Felsch wrote: > > On 24-02-06, Krzysztof Kozlowski wrote: > >> On 06/02/2024 15:52, Marco Felsch wrote: > >>> On 24-02-06, Krzysztof Kozlowski wrote: > >>>> On 05/02/2024 17:43, Marco Felsch wrote: > >>>>> This binding descripes the generic TCPCI specification [1]. So add the ... > > Don't get me wrong, I get your point. In the end I don't care and can > > copy'n'paste the whole file and change the compatible to the OnSemi > > device or I can add the dedicated OnSemi compatible to this file. But I > > don't wanted to add an 2nd specific compatible while the device already > > supports the generic one but via i2c_device_id.name. Therefore I aligned > > the i2c_device_id with the of_device_id. > > You can add generic compatible used as fallback. That's pretty common > practice. Okay. To bring this discussion to an end, I will add the generic compatible as fallback :) Thanks, Marco > > > > >>>> Are all details expected to follow spec, without need of quirks? > >>> > >>> Please see above, I hope this helps. > >> > >> Sorry, doesn't. You still speak about driver and how it can bind to > >> something. I did not ask about this at all. > >> > >> To be clear: > >> WE ARE NOT TALKING ABOUT LINUX DRIVER. > > > > I KNOW > > > >> We talk about hardware and how it is represented in Devicetree, > >> including its supplies, pins, GPIOs and any ideas hardware engineers > >> like to bring. > > Then terms "driver" and "binding" (or matching) do not fit here as > arguments whether specific compatible should be there or not. There is > guideline for that: writing bindings, which exactly, 100% covers this > thing here. > > Best regards, > Krzysztof > >
diff --git a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml index eaedb4cc6b6c..7bd7bbbac9e0 100644 --- a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml +++ b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml @@ -11,7 +11,9 @@ maintainers: properties: compatible: - const: nxp,ptn5110 + enum: + - nxp,ptn5110 + - tcpci reg: maxItems: 1
This binding descripes the generic TCPCI specification [1]. So add the generic binding support since which can be used if an different TCPC is used compatible which is compatible to [1]. [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> --- Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)