[{"id":1767413,"web_url":"http://patchwork.ozlabs.org/comment/1767413/","msgid":"<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>","list_archive_url":null,"date":"2017-09-12T22:20:27","subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","submitter":{"id":62529,"url":"http://patchwork.ozlabs.org/api/people/62529/","name":"Rob Herring (Arm)","email":"robh@kernel.org"},"content":"On Tue, Sep 05, 2017 at 06:42:20PM +0200, Hans de Goede wrote:\n> Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()\n> to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.\n> \n> Also document the mux-names used by the generic tcpc_mux_dev code in\n> our devicetree bindings.\n> \n> Cc: Rob Herring \n> Cc: Mark Rutland \n> Cc: devicetree@vger.kernel.org\n> Signed-off-by: Hans de Goede \n> ---\n> Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++\n> drivers/staging/typec/fusb302/fusb302.c | 11 ++++++++++-\n> 2 files changed, 13 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n> index 472facfa5a71..63d639eadacd 100644\n> --- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n> @@ -6,6 +6,9 @@ Required properties :\n> - interrupts : Interrupt specifier\n> \n> Optional properties :\n> +- mux-controls : List of mux-ctrl-specifiers containing 1 or 2 muxes\n> +- mux-names : \"type-c-mode-mux\" when using 1 mux, or\n> + \"type-c-mode-mux\", \"usb-role-mux\" when using 2 muxes\n\nI'm not sure this is the right place for this. The mux is outside the \nFUSB302. In a type-C connector node or USB phy node would make more \nsense to me.\n\n> - fcs,max-sink-microvolt : Maximum voltage to negotiate when configured as sink\n> - fcs,max-sink-microamp : Maximum current to negotiate when configured as sink\n> - fcs,max-sink-microwatt : Maximum power to negotiate when configured as sink\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsK4K2m7Gz9t4X\n\tfor ;\n\tWed, 13 Sep 2017 08:20:33 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751105AbdILWUb (ORCPT\n\t);\n\tTue, 12 Sep 2017 18:20:31 -0400","from mail-oi0-f68.google.com ([209.85.218.68]:37327 \"EHLO\n\tmail-oi0-f68.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751003AbdILWU3 (ORCPT\n\t); Tue, 12 Sep 2017 18:20:29 -0400","by mail-oi0-f68.google.com with SMTP id l9so4375421oib.4;\n\tTue, 12 Sep 2017 15:20:29 -0700 (PDT)","from localhost (216-188-254-6.dyn.grandenetworks.net.\n\t[216.188.254.6]) by smtp.gmail.com with ESMTPSA id\n\ts133sm13980069oih.25.2017.09.12.15.20.28\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 12 Sep 2017 15:20:28 -0700 (PDT)"],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=jdcEdd01kNjN+/0rU9mc7iJIEOOK4zpGcNTe1ZHY0JQ=;\n\tb=tHWV3VKHDNO/esJh4yyjdQdrzRtkL8a/+QIN/EIYJYA+xHjwTWtzt3DrnS69Al+jEu\n\tyqHkP5Y9V4Tfl6R9VZonw1dmzMPUKyDuEJjIPpGuHq5p1pVg5Vv8w9EmisEouNArFnPz\n\tAeZMLEQQiJOw7EDc+UvsXmDaIo6PXokrCegeaianofQ7jlwWXEOEf96UDimeYfVS1c5A\n\tOgsKwS3ziWEe4vsVN+ZveO1lt9emYiYhGYsQpqoWNfZqHYHECTQetzs9lK8kndX69K6n\n\tkYcpm8vJycRg4e5C31TV9pwoOQj7fWDSnGlZDS9g3xtT79J+/zMywSRYuC0k/CselEz3\n\tExBQ==","X-Gm-Message-State":"AHPjjUgc7CbgFYv8lYhnJCqmgQBQHKhKsK6v7BYCHEpsoDMLJ/9tukL1\n\ti7H2+TQ1B8uNYg==","X-Google-Smtp-Source":"AOwi7QBLb/MqBi9kYNptCJd+xpC1GNDxNJfK9hM5Y6IuENF8XX1CCyfm+07SaYM/z5QYUJiNY5dbMw==","X-Received":"by 10.202.237.150 with SMTP id\n\tl144mr16982451oih.88.1505254829140; \n\tTue, 12 Sep 2017 15:20:29 -0700 (PDT)","Date":"Tue, 12 Sep 2017 17:20:27 -0500","From":"Rob Herring ","To":"Hans de Goede ","Cc":"MyungJoo Ham ,\n\tChanwoo Choi ,\n\tGuenter Roeck , \n\tHeikki Krogerus ,\n\tDarren Hart ,\n\tAndy Shevchenko , \n\tPeter Rosin , Mathias Nyman ,\n\tlinux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org,\n\tdevel@driverdev.osuosl.org, Kuppuswamy Sathyanarayanan \n\t,\n\tSathyanarayanan Kuppuswamy Natarajan ,\n\tGreg Kroah-Hartman ,\n\tlinux-usb@vger.kernel.org, Mark Rutland ,\n\tdevicetree@vger.kernel.org","Subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","Message-ID":"<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>","References":"<20170905164221.11266-1-hdegoede@redhat.com>\n\t<20170905164221.11266-11-hdegoede@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170905164221.11266-11-hdegoede@redhat.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767686,"web_url":"http://patchwork.ozlabs.org/comment/1767686/","msgid":"","list_archive_url":null,"date":"2017-09-13T08:56:49","subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","submitter":{"id":1893,"url":"http://patchwork.ozlabs.org/api/people/1893/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 13-09-17 00:20, Rob Herring wrote:\n> On Tue, Sep 05, 2017 at 06:42:20PM +0200, Hans de Goede wrote:\n>> Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()\n>> to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.\n>>\n>> Also document the mux-names used by the generic tcpc_mux_dev code in\n>> our devicetree bindings.\n>>\n>> Cc: Rob Herring \n>> Cc: Mark Rutland \n>> Cc: devicetree@vger.kernel.org\n>> Signed-off-by: Hans de Goede \n>> ---\n>> Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++\n>> drivers/staging/typec/fusb302/fusb302.c | 11 ++++++++++-\n>> 2 files changed, 13 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>> index 472facfa5a71..63d639eadacd 100644\n>> --- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>> @@ -6,6 +6,9 @@ Required properties :\n>> - interrupts : Interrupt specifier\n>> \n>> Optional properties :\n>> +- mux-controls : List of mux-ctrl-specifiers containing 1 or 2 muxes\n>> +- mux-names : \"type-c-mode-mux\" when using 1 mux, or\n>> + \"type-c-mode-mux\", \"usb-role-mux\" when using 2 muxes\n> \n> I'm not sure this is the right place for this. The mux is outside the\n> FUSB302. In a type-C connector node or USB phy node would make more\n> sense to me.\n\nThe mux certainly does not belong in the USB phy node, it sits between the USB PHY\nand the Type-C connector and can for example also mux the Type-C pins to a Display\nPort PHY.\n\nAs for putting it in a type-C connector node, currently we do not have such a node,\nthe closest thing we do have to a node describing it is actually the fusb302 node\nitself. E.g. it may also contain a regulator to turn Vbus on / off (already there\nin the code, but I forgot to document this when I added the missing DT bindings\ndoc for the fusb302 with a previous patch).\n\nAlso these properties:\n\n>> - fcs,max-sink-microvolt : Maximum voltage to negotiate when configured as sink\n>> - fcs,max-sink-microamp : Maximum current to negotiate when configured as sink\n>> - fcs,max-sink-microwatt : Maximum power to negotiate when configured as sink\n\nHave more to do with the charger-IC used (which determines the limits) then with\nthe fusb302 itself, but the fusb302 needs to know these as it negotiates the limits.\n\nLikewise the fusb302 negotiates how the data pins will be used and thus to which pins\non the SoC the mux should mux the data pins.\n\nTL;DR: The fusb302 does all the negotiation and ties all the Type-C connected\nICs together, so this seems like the right place for it (it certainly is the\nnatural place to put these from a driver code pov).\n\nRegards,\n\nHans\n\n\n\n\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=hdegoede@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsbBw5FDmz9sPk\n\tfor ;\n\tWed, 13 Sep 2017 18:57:12 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751539AbdIMI45 (ORCPT\n\t);\n\tWed, 13 Sep 2017 04:56:57 -0400","from mx1.redhat.com ([209.132.183.28]:47200 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1750776AbdIMI4z (ORCPT );\n\tWed, 13 Sep 2017 04:56:55 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id E8362C04B94B;\n\tWed, 13 Sep 2017 08:56:54 +0000 (UTC)","from shalem.localdomain (ovpn-116-240.ams2.redhat.com\n\t[10.36.116.240])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 34D705C3FF;\n\tWed, 13 Sep 2017 08:56:51 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com E8362C04B94B","Subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","To":"Rob Herring ","Cc":"MyungJoo Ham ,\n\tChanwoo Choi ,\n\tGuenter Roeck , \n\tHeikki Krogerus ,\n\tDarren Hart ,\n\tAndy Shevchenko , \n\tPeter Rosin , Mathias Nyman ,\n\tlinux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org,\n\tdevel@driverdev.osuosl.org, Kuppuswamy Sathyanarayanan \n\t,\n\tSathyanarayanan Kuppuswamy Natarajan ,\n\tGreg Kroah-Hartman ,\n\tlinux-usb@vger.kernel.org, Mark Rutland ,\n\tdevicetree@vger.kernel.org","References":"<20170905164221.11266-1-hdegoede@redhat.com>\n\t<20170905164221.11266-11-hdegoede@redhat.com>\n\t<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>","From":"Hans de Goede ","Message-ID":"","Date":"Wed, 13 Sep 2017 10:56:49 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]); Wed, 13 Sep 2017 08:56:55 +0000 (UTC)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767894,"web_url":"http://patchwork.ozlabs.org/comment/1767894/","msgid":"","list_archive_url":null,"date":"2017-09-13T13:38:55","subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","submitter":{"id":62529,"url":"http://patchwork.ozlabs.org/api/people/62529/","name":"Rob Herring (Arm)","email":"robh@kernel.org"},"content":"On Wed, Sep 13, 2017 at 3:56 AM, Hans de Goede wrote:\n> Hi,\n>\n>\n> On 13-09-17 00:20, Rob Herring wrote:\n>>\n>> On Tue, Sep 05, 2017 at 06:42:20PM +0200, Hans de Goede wrote:\n>>>\n>>> Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()\n>>> to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.\n>>>\n>>> Also document the mux-names used by the generic tcpc_mux_dev code in\n>>> our devicetree bindings.\n>>>\n>>> Cc: Rob Herring \n>>> Cc: Mark Rutland \n>>> Cc: devicetree@vger.kernel.org\n>>> Signed-off-by: Hans de Goede \n>>> ---\n>>> Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++\n>>> drivers/staging/typec/fusb302/fusb302.c | 11 ++++++++++-\n>>> 2 files changed, 13 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>> b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>> index 472facfa5a71..63d639eadacd 100644\n>>> --- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>> @@ -6,6 +6,9 @@ Required properties :\n>>> - interrupts : Interrupt specifier\n>>> Optional properties :\n>>> +- mux-controls : List of mux-ctrl-specifiers containing 1 or 2\n>>> muxes\n>>> +- mux-names : \"type-c-mode-mux\" when using 1 mux, or\n>>> + \"type-c-mode-mux\", \"usb-role-mux\" when using\n>>> 2 muxes\n>>\n>>\n>> I'm not sure this is the right place for this. The mux is outside the\n>> FUSB302. In a type-C connector node or USB phy node would make more\n>> sense to me.\n>\n>\n> The mux certainly does not belong in the USB phy node, it sits between the\n> USB PHY\n> and the Type-C connector and can for example also mux the Type-C pins to a\n> Display\n> Port PHY.\n\nThinking about this some more, the mux(es) should be its own node(s).\nThen the question is where to put the nodes.\n\n> As for putting it in a type-C connector node, currently we do not have such\n> a node,\n\nWell, you should. Type-C connectors are certainly complicated enough\nthat we'll need one. Plus we already require connector nodes for\ndisplay outputs, so what do we do once you add display muxing?\n\n> the closest thing we do have to a node describing it is actually the fusb302\n> node\n> itself. E.g. it may also contain a regulator to turn Vbus on / off (already\n> there\n> in the code, but I forgot to document this when I added the missing DT\n> bindings\n> doc for the fusb302 with a previous patch).\n\nEither you can have a vbus-supply property in connector node or it can\nbe implied that the controller chip provides that. For example, HDMI\nconnectors have a hpd-gpios property if HPD is connected to GPIO or\nthey have nothing and it's implicit that the HDMI encoder handles HPD.\n\n\n> Also these properties:\n>\n>>> - fcs,max-sink-microvolt : Maximum voltage to negotiate when configured\n>>> as sink\n>>> - fcs,max-sink-microamp : Maximum current to negotiate when configured\n>>> as sink\n>>> - fcs,max-sink-microwatt : Maximum power to negotiate when configured\n>>> as sink\n>\n>\n> Have more to do with the charger-IC used (which determines the limits) then\n> with\n> the fusb302 itself, but the fusb302 needs to know these as it negotiates the\n> limits.\n\nThose should probably be elsewhere and not be fusb302 specific. I did\nack that, but it was a single node for a single component which is\nfine. But once we start adding more external pieces we need to pay\nmore attention to the overall structure.\n\n> Likewise the fusb302 negotiates how the data pins will be used and thus to\n> which pins\n> on the SoC the mux should mux the data pins.\n>\n> TL;DR: The fusb302 does all the negotiation and ties all the Type-C\n> connected\n> ICs together, so this seems like the right place for it (it certainly is the\n> natural place to put these from a driver code pov).\n\nThings in DT should follow what the h/w design looks like which is not\nnecessarily aligned with the driver structure. If the USB PD chip\nneeds information from the charger, then we need a kernel interface\nfor that.\n\nMy concern here is not so much this binding in particular, but rather\nthat we handle Type-C connectors in a common way and not adhoc with\neach platform doing things their own way. Otherwise, we end up with a\nmess of platform specific bindings like charger/battery bindings\n(though there's some work improving those).\n\nRob\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","mail.kernel.org;\n\tdmarc=none (p=none dis=none) header.from=kernel.org","mail.kernel.org;\n\tspf=none smtp.mailfrom=robh@kernel.org"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsjSl2bytz9sNV\n\tfor ;\n\tWed, 13 Sep 2017 23:39:35 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751106AbdIMNjT (ORCPT\n\t);\n\tWed, 13 Sep 2017 09:39:19 -0400","from mail.kernel.org ([198.145.29.99]:45530 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1750993AbdIMNjS (ORCPT );\n\tWed, 13 Sep 2017 09:39:18 -0400","from mail-qk0-f179.google.com (mail-qk0-f179.google.com\n\t[209.85.220.179])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby mail.kernel.org (Postfix) with ESMTPSA id 3EC6821EA8;\n\tWed, 13 Sep 2017 13:39:17 +0000 (UTC)","by mail-qk0-f179.google.com with SMTP id a128so602789qkc.5;\n\tWed, 13 Sep 2017 06:39:17 -0700 (PDT)","by 10.12.209.75 with HTTP; Wed, 13 Sep 2017 06:38:55 -0700 (PDT)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 3EC6821EA8","X-Gm-Message-State":"AHPjjUih1aB6l13RjbXpnQxjZ9t53+tmJVsZPh8JM3DYK0xk1cMlkei3\n\thc2YQV+Z3HsE4cGaDeIlVgOmIL8iLCmqfhH8oQ==","X-Google-Smtp-Source":"AOwi7QCkNg/awK7wLpqOxv2tTQ6gaFh2ACJpdzWWjb0Utmwa0TPnC3O9VSXuLjXeDLaciZ3GBaA5Y49kyGakwOjuDZM=","X-Received":"by 10.55.95.6 with SMTP id t6mr23782721qkb.192.1505309956285;\n\tWed, 13 Sep 2017 06:39:16 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"","References":"<20170905164221.11266-1-hdegoede@redhat.com>\n\t<20170905164221.11266-11-hdegoede@redhat.com>\n\t<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>\n\t","From":"Rob Herring ","Date":"Wed, 13 Sep 2017 08:38:55 -0500","X-Gmail-Original-Message-ID":"","Message-ID":"","Subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","To":"Hans de Goede ","Cc":"MyungJoo Ham ,\n\tChanwoo Choi ,\n\tGuenter Roeck , \n\tHeikki Krogerus ,\n\tDarren Hart ,\n\tAndy Shevchenko , \n\tPeter Rosin , Mathias Nyman ,\n\t\"linux-kernel@vger.kernel.org\" ,\n\tplatform-driver-x86@vger.kernel.org, devel@driverdev.osuosl.org,\n\tKuppuswamy Sathyanarayanan , \n\tSathyanarayanan Kuppuswamy Natarajan ,\n\tGreg Kroah-Hartman ,\n\tLinux USB List ,\n\tMark Rutland ,\n\t\"devicetree@vger.kernel.org\" ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767911,"web_url":"http://patchwork.ozlabs.org/comment/1767911/","msgid":"","list_archive_url":null,"date":"2017-09-13T14:06:31","subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","submitter":{"id":1893,"url":"http://patchwork.ozlabs.org/api/people/1893/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 13-09-17 15:38, Rob Herring wrote:\n> On Wed, Sep 13, 2017 at 3:56 AM, Hans de Goede wrote:\n>> Hi,\n>>\n>>\n>> On 13-09-17 00:20, Rob Herring wrote:\n>>>\n>>> On Tue, Sep 05, 2017 at 06:42:20PM +0200, Hans de Goede wrote:\n>>>>\n>>>> Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()\n>>>> to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.\n>>>>\n>>>> Also document the mux-names used by the generic tcpc_mux_dev code in\n>>>> our devicetree bindings.\n>>>>\n>>>> Cc: Rob Herring \n>>>> Cc: Mark Rutland \n>>>> Cc: devicetree@vger.kernel.org\n>>>> Signed-off-by: Hans de Goede \n>>>> ---\n>>>> Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++\n>>>> drivers/staging/typec/fusb302/fusb302.c | 11 ++++++++++-\n>>>> 2 files changed, 13 insertions(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>> b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>> index 472facfa5a71..63d639eadacd 100644\n>>>> --- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>> @@ -6,6 +6,9 @@ Required properties :\n>>>> - interrupts : Interrupt specifier\n>>>> Optional properties :\n>>>> +- mux-controls : List of mux-ctrl-specifiers containing 1 or 2\n>>>> muxes\n>>>> +- mux-names : \"type-c-mode-mux\" when using 1 mux, or\n>>>> + \"type-c-mode-mux\", \"usb-role-mux\" when using\n>>>> 2 muxes\n>>>\n>>>\n>>> I'm not sure this is the right place for this. The mux is outside the\n>>> FUSB302. In a type-C connector node or USB phy node would make more\n>>> sense to me.\n>>\n>>\n>> The mux certainly does not belong in the USB phy node, it sits between the\n>> USB PHY\n>> and the Type-C connector and can for example also mux the Type-C pins to a\n>> Display\n>> Port PHY.\n> \n> Thinking about this some more, the mux(es) should be its own node(s).\n> Then the question is where to put the nodes.\n\nRight, the mux will be its own node per Documentation/devicetree/bindings/mux/mux-controller.txt\nthe bindings bit this patch is adding is only adding phandles pointing\nto that mux-node as the fusb302 \"consumes\" the mux functionality.\n\nSo as such (the fusb302 is the component which should control the mux)\nI do believe that the bindings this patch adds are correct.\n\n>> As for putting it in a type-C connector node, currently we do not have such\n>> a node,\n> \n> Well, you should. Type-C connectors are certainly complicated enough\n> that we'll need one. Plus we already require connector nodes for\n> display outputs, so what do we do once you add display muxing?\n\nAn interesting question, I'm working on this for x86 + ACPI boards actually,\nnot a board using DT I've been adding DT bindings docs for device-properties\nI use because that seems like the right thing to do where the binding is obvious\n(which I believe it is in this case as the fusb302 is the mux consumer) and\nbecause the device-property code should work the same on x86 + ACPI\n(where some platform-specific drivers attach the device properties) and\non e.g. ARM + DT.\n\nThe rest should probably be left to be figured out when an actual DT\nusing device using the fusb302 or another Type-C controller shows up.\n\n>> the closest thing we do have to a node describing it is actually the fusb302\n>> node\n>> itself. E.g. it may also contain a regulator to turn Vbus on / off (already\n>> there\n>> in the code, but I forgot to document this when I added the missing DT\n>> bindings\n>> doc for the fusb302 with a previous patch).\n> \n> Either you can have a vbus-supply property in connector node or it can\n> be implied that the controller chip provides that. For example, HDMI\n> connectors have a hpd-gpios property if HPD is connected to GPIO or\n> they have nothing and it's implicit that the HDMI encoder handles HPD.\n> \n> \n>> Also these properties:\n>>\n>>>> - fcs,max-sink-microvolt : Maximum voltage to negotiate when configured\n>>>> as sink\n>>>> - fcs,max-sink-microamp : Maximum current to negotiate when configured\n>>>> as sink\n>>>> - fcs,max-sink-microwatt : Maximum power to negotiate when configured\n>>>> as sink\n>>\n>>\n>> Have more to do with the charger-IC used (which determines the limits) then\n>> with\n>> the fusb302 itself, but the fusb302 needs to know these as it negotiates the\n>> limits.\n> \n> Those should probably be elsewhere and not be fusb302 specific. I did\n> ack that, but it was a single node for a single component which is\n> fine. But once we start adding more external pieces we need to pay\n> more attention to the overall structure.\n> \n>> Likewise the fusb302 negotiates how the data pins will be used and thus to\n>> which pins\n>> on the SoC the mux should mux the data pins.\n>>\n>> TL;DR: The fusb302 does all the negotiation and ties all the Type-C\n>> connected\n>> ICs together, so this seems like the right place for it (it certainly is the\n>> natural place to put these from a driver code pov).\n> \n> Things in DT should follow what the h/w design looks like which is not\n> necessarily aligned with the driver structure. If the USB PD chip\n> needs information from the charger, then we need a kernel interface\n> for that.\n\nWell this really is board specific data, the charger IC may be handle\nfor example up to 17V, but if the board is only designed for 12V then\nwe should only negotiate up to 12V because the board may have voltage\noverprotection circuitry which trips at say 14V. This is actually the\ncase with the 2 x86 boards with a Type-C connector and fusb302 Type-C\ncontroller I have, the charger on there can handle upto 17V according\nto its datasheet but Windows never negotiates more then 12V, even when\nattaching a Type-C charger which can do 5V, 9V or 14V, Windows choses 9V.\n\nSo it is the Type-C controller which does the negotiating and\nthe limits may be stricter then the maximum ratings of the\ncharger IC, so I guess my earlier remark of this coming from the\ncharger IC was not accurate and having this info in the Type-C controller\nnode is the right thing to do, sorry about that.\n\n> My concern here is not so much this binding in particular, but rather\n> that we handle Type-C connectors in a common way and not adhoc with\n> each platform doing things their own way. Otherwise, we end up with a\n> mess of platform specific bindings like charger/battery bindings\n> (though there's some work improving those).\n\nI understand, but see my remark about me working on this on\nX86 / ACPI boards. One advantage of this, is that the device-properties\nare being set by platform drivers living under drivers/platform/x86,\nso if the need arises they can be changed without breaking any ABI as\nin my use-case they are 100% kernel internal stuff.\n\nRegards,\n\nHans\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=hdegoede@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsk4h31b1z9sNw\n\tfor ;\n\tThu, 14 Sep 2017 00:07:16 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751735AbdIMOGi (ORCPT\n\t);\n\tWed, 13 Sep 2017 10:06:38 -0400","from mx1.redhat.com ([209.132.183.28]:45622 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751422AbdIMOGg (ORCPT );\n\tWed, 13 Sep 2017 10:06:36 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 565EC7EA85;\n\tWed, 13 Sep 2017 14:06:36 +0000 (UTC)","from shalem.localdomain (ovpn-116-240.ams2.redhat.com\n\t[10.36.116.240])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id B972661B73;\n\tWed, 13 Sep 2017 14:06:32 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 565EC7EA85","Subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","To":"Rob Herring ","Cc":"MyungJoo Ham ,\n\tChanwoo Choi ,\n\tGuenter Roeck , \n\tHeikki Krogerus ,\n\tDarren Hart ,\n\tAndy Shevchenko , \n\tPeter Rosin , Mathias Nyman ,\n\t\"linux-kernel@vger.kernel.org\" ,\n\tplatform-driver-x86@vger.kernel.org, devel@driverdev.osuosl.org,\n\tKuppuswamy Sathyanarayanan , \n\tSathyanarayanan Kuppuswamy Natarajan ,\n\tGreg Kroah-Hartman ,\n\tLinux USB List ,\n\tMark Rutland ,\n\t\"devicetree@vger.kernel.org\" ","References":"<20170905164221.11266-1-hdegoede@redhat.com>\n\t<20170905164221.11266-11-hdegoede@redhat.com>\n\t<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>\n\t\n\t","From":"Hans de Goede ","Message-ID":"","Date":"Wed, 13 Sep 2017 16:06:31 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]); Wed, 13 Sep 2017 14:06:36 +0000 (UTC)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767959,"web_url":"http://patchwork.ozlabs.org/comment/1767959/","msgid":"","list_archive_url":null,"date":"2017-09-13T15:07:59","subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","submitter":{"id":62529,"url":"http://patchwork.ozlabs.org/api/people/62529/","name":"Rob Herring (Arm)","email":"robh@kernel.org"},"content":"On Wed, Sep 13, 2017 at 9:06 AM, Hans de Goede wrote:\n> Hi,\n>\n>\n> On 13-09-17 15:38, Rob Herring wrote:\n>>\n>> On Wed, Sep 13, 2017 at 3:56 AM, Hans de Goede \n>> wrote:\n>>>\n>>> Hi,\n>>>\n>>>\n>>> On 13-09-17 00:20, Rob Herring wrote:\n>>>>\n>>>>\n>>>> On Tue, Sep 05, 2017 at 06:42:20PM +0200, Hans de Goede wrote:\n>>>>>\n>>>>>\n>>>>> Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()\n>>>>> to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.\n>>>>>\n>>>>> Also document the mux-names used by the generic tcpc_mux_dev code in\n>>>>> our devicetree bindings.\n>>>>>\n>>>>> Cc: Rob Herring \n>>>>> Cc: Mark Rutland \n>>>>> Cc: devicetree@vger.kernel.org\n>>>>> Signed-off-by: Hans de Goede \n>>>>> ---\n>>>>> Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++\n>>>>> drivers/staging/typec/fusb302/fusb302.c | 11\n>>>>> ++++++++++-\n>>>>> 2 files changed, 13 insertions(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>> b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>> index 472facfa5a71..63d639eadacd 100644\n>>>>> --- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>> @@ -6,6 +6,9 @@ Required properties :\n>>>>> - interrupts : Interrupt specifier\n>>>>> Optional properties :\n>>>>> +- mux-controls : List of mux-ctrl-specifiers containing 1 or\n>>>>> 2\n>>>>> muxes\n>>>>> +- mux-names : \"type-c-mode-mux\" when using 1 mux, or\n>>>>> + \"type-c-mode-mux\", \"usb-role-mux\" when\n>>>>> using\n>>>>> 2 muxes\n>>>>\n>>>>\n>>>>\n>>>> I'm not sure this is the right place for this. The mux is outside the\n>>>> FUSB302. In a type-C connector node or USB phy node would make more\n>>>> sense to me.\n>>>\n>>>\n>>>\n>>> The mux certainly does not belong in the USB phy node, it sits between\n>>> the\n>>> USB PHY\n>>> and the Type-C connector and can for example also mux the Type-C pins to\n>>> a\n>>> Display\n>>> Port PHY.\n>>\n>>\n>> Thinking about this some more, the mux(es) should be its own node(s).\n>> Then the question is where to put the nodes.\n>\n>\n> Right, the mux will be its own node per\n> Documentation/devicetree/bindings/mux/mux-controller.txt\n> the bindings bit this patch is adding is only adding phandles pointing\n> to that mux-node as the fusb302 \"consumes\" the mux functionality.\n>\n> So as such (the fusb302 is the component which should control the mux)\n> I do believe that the bindings this patch adds are correct.\n\nHumm, that's not how the mux binding works. The mux controller is what\ndrives the mux select lines and is the provider. The consumer is the\nmux device itself. What decides the mux state is determined by what\nyou are muxing, not which node has mux-controls property.\n\nBy putting mux-controls in fusb302 node, you are saying fusb302 is a\nmux (or contains a mux).\n\n\n>>> As for putting it in a type-C connector node, currently we do not have\n>>> such\n>>> a node,\n>>\n>>\n>> Well, you should. Type-C connectors are certainly complicated enough\n>> that we'll need one. Plus we already require connector nodes for\n>> display outputs, so what do we do once you add display muxing?\n>\n>\n> An interesting question, I'm working on this for x86 + ACPI boards actually,\n> not a board using DT I've been adding DT bindings docs for device-properties\n> I use because that seems like the right thing to do where the binding is\n> obvious\n> (which I believe it is in this case as the fusb302 is the mux consumer) and\n> because the device-property code should work the same on x86 + ACPI\n> (where some platform-specific drivers attach the device properties) and\n> on e.g. ARM + DT.\n>\n> The rest should probably be left to be figured out when an actual DT\n> using device using the fusb302 or another Type-C controller shows up.\n\nWell this is a new one (maybe, I suppose others have sneaked by). If\nACPI folks want to use DT bindings, then what do I care. But I have no\ninterest in reviewing ACPI properties. The whole notion of sharing\nbindings between DT and ACPI beyond anything trivial is flawed IMO.\nThe ptifalls have been discussed multiple times before, so I'm not\ngoing to repeat them here.\n\n\n>>> the closest thing we do have to a node describing it is actually the\n>>> fusb302\n>>> node\n>>> itself. E.g. it may also contain a regulator to turn Vbus on / off\n>>> (already\n>>> there\n>>> in the code, but I forgot to document this when I added the missing DT\n>>> bindings\n>>> doc for the fusb302 with a previous patch).\n>>\n>>\n>> Either you can have a vbus-supply property in connector node or it can\n>> be implied that the controller chip provides that. For example, HDMI\n>> connectors have a hpd-gpios property if HPD is connected to GPIO or\n>> they have nothing and it's implicit that the HDMI encoder handles HPD.\n>>\n>>\n>>> Also these properties:\n>>>\n>>>>> - fcs,max-sink-microvolt : Maximum voltage to negotiate when\n>>>>> configured\n>>>>> as sink\n>>>>> - fcs,max-sink-microamp : Maximum current to negotiate when\n>>>>> configured\n>>>>> as sink\n>>>>> - fcs,max-sink-microwatt : Maximum power to negotiate when\n>>>>> configured\n>>>>> as sink\n>>>\n>>>\n>>>\n>>> Have more to do with the charger-IC used (which determines the limits)\n>>> then\n>>> with\n>>> the fusb302 itself, but the fusb302 needs to know these as it negotiates\n>>> the\n>>> limits.\n>>\n>>\n>> Those should probably be elsewhere and not be fusb302 specific. I did\n>> ack that, but it was a single node for a single component which is\n>> fine. But once we start adding more external pieces we need to pay\n>> more attention to the overall structure.\n>>\n>>> Likewise the fusb302 negotiates how the data pins will be used and thus\n>>> to\n>>> which pins\n>>> on the SoC the mux should mux the data pins.\n>>>\n>>> TL;DR: The fusb302 does all the negotiation and ties all the Type-C\n>>> connected\n>>> ICs together, so this seems like the right place for it (it certainly is\n>>> the\n>>> natural place to put these from a driver code pov).\n>>\n>>\n>> Things in DT should follow what the h/w design looks like which is not\n>> necessarily aligned with the driver structure. If the USB PD chip\n>> needs information from the charger, then we need a kernel interface\n>> for that.\n>\n>\n> Well this really is board specific data, the charger IC may be handle\n> for example up to 17V, but if the board is only designed for 12V then\n> we should only negotiate up to 12V because the board may have voltage\n> overprotection circuitry which trips at say 14V. This is actually the\n> case with the 2 x86 boards with a Type-C connector and fusb302 Type-C\n> controller I have, the charger on there can handle upto 17V according\n> to its datasheet but Windows never negotiates more then 12V, even when\n> attaching a Type-C charger which can do 5V, 9V or 14V, Windows choses 9V.\n\nJust as the fusb302 can have board specific properties, so can a charger IC.\n\n> So it is the Type-C controller which does the negotiating and\n> the limits may be stricter then the maximum ratings of the\n> charger IC, so I guess my earlier remark of this coming from the\n> charger IC was not accurate and having this info in the Type-C controller\n> node is the right thing to do, sorry about that.\n>\n>> My concern here is not so much this binding in particular, but rather\n>> that we handle Type-C connectors in a common way and not adhoc with\n>> each platform doing things their own way. Otherwise, we end up with a\n>> mess of platform specific bindings like charger/battery bindings\n>> (though there's some work improving those).\n>\n>\n> I understand, but see my remark about me working on this on\n> X86 / ACPI boards. One advantage of this, is that the device-properties\n> are being set by platform drivers living under drivers/platform/x86,\n> so if the need arises they can be changed without breaking any ABI as\n> in my use-case they are 100% kernel internal stuff.\n\nThen don't document this stuff as DT bindings when it is not really.\n\nRob\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","mail.kernel.org;\n\tdmarc=none (p=none dis=none) header.from=kernel.org","mail.kernel.org;\n\tspf=none smtp.mailfrom=robh@kernel.org"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xslRD4KJxz9sPk\n\tfor ;\n\tThu, 14 Sep 2017 01:08:24 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751734AbdIMPIW (ORCPT\n\t);\n\tWed, 13 Sep 2017 11:08:22 -0400","from mail.kernel.org ([198.145.29.99]:50000 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751460AbdIMPIV (ORCPT );\n\tWed, 13 Sep 2017 11:08:21 -0400","from mail-qk0-f178.google.com (mail-qk0-f178.google.com\n\t[209.85.220.178])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby mail.kernel.org (Postfix) with ESMTPSA id BE41221E91;\n\tWed, 13 Sep 2017 15:08:20 +0000 (UTC)","by mail-qk0-f178.google.com with SMTP id z143so1273676qkb.3;\n\tWed, 13 Sep 2017 08:08:20 -0700 (PDT)","by 10.12.209.75 with HTTP; Wed, 13 Sep 2017 08:07:59 -0700 (PDT)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org BE41221E91","X-Gm-Message-State":"AHPjjUgr8opVv2wVwAN/00nz22mAjtZy2yZHW6T/gCnLibFuvudcSMrk\n\tGNGW0CrQUvbV/6wv5nJxtKpx1j3z6k6i8o4F2g==","X-Google-Smtp-Source":"AOwi7QCBD1X8jbAYvqJd4LMyBwKeo6s88/Vq8YU+90fw4XCW6hTf+DNR+k7N3BVmnk28a0Z8ekmRnAZ845DcB2xWsr0=","X-Received":"by 10.55.95.71 with SMTP id t68mr14871970qkb.233.1505315299850; \n\tWed, 13 Sep 2017 08:08:19 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"","References":"<20170905164221.11266-1-hdegoede@redhat.com>\n\t<20170905164221.11266-11-hdegoede@redhat.com>\n\t<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>\n\t\n\t\n\t","From":"Rob Herring ","Date":"Wed, 13 Sep 2017 10:07:59 -0500","X-Gmail-Original-Message-ID":"","Message-ID":"","Subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","To":"Hans de Goede ","Cc":"MyungJoo Ham ,\n\tChanwoo Choi ,\n\tGuenter Roeck , \n\tHeikki Krogerus ,\n\tDarren Hart ,\n\tAndy Shevchenko , \n\tPeter Rosin , Mathias Nyman ,\n\t\"linux-kernel@vger.kernel.org\" ,\n\tplatform-driver-x86@vger.kernel.org, devel@driverdev.osuosl.org,\n\tKuppuswamy Sathyanarayanan , \n\tSathyanarayanan Kuppuswamy Natarajan ,\n\tGreg Kroah-Hartman ,\n\tLinux USB List ,\n\tMark Rutland ,\n\t\"devicetree@vger.kernel.org\" ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1767979,"web_url":"http://patchwork.ozlabs.org/comment/1767979/","msgid":"","list_archive_url":null,"date":"2017-09-13T15:48:25","subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","submitter":{"id":1893,"url":"http://patchwork.ozlabs.org/api/people/1893/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 13-09-17 17:07, Rob Herring wrote:\n> On Wed, Sep 13, 2017 at 9:06 AM, Hans de Goede wrote:\n>> Hi,\n>>\n>>\n>> On 13-09-17 15:38, Rob Herring wrote:\n>>>\n>>> On Wed, Sep 13, 2017 at 3:56 AM, Hans de Goede \n>>> wrote:\n>>>>\n>>>> Hi,\n>>>>\n>>>>\n>>>> On 13-09-17 00:20, Rob Herring wrote:\n>>>>>\n>>>>>\n>>>>> On Tue, Sep 05, 2017 at 06:42:20PM +0200, Hans de Goede wrote:\n>>>>>>\n>>>>>>\n>>>>>> Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()\n>>>>>> to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.\n>>>>>>\n>>>>>> Also document the mux-names used by the generic tcpc_mux_dev code in\n>>>>>> our devicetree bindings.\n>>>>>>\n>>>>>> Cc: Rob Herring \n>>>>>> Cc: Mark Rutland \n>>>>>> Cc: devicetree@vger.kernel.org\n>>>>>> Signed-off-by: Hans de Goede \n>>>>>> ---\n>>>>>> Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++\n>>>>>> drivers/staging/typec/fusb302/fusb302.c | 11\n>>>>>> ++++++++++-\n>>>>>> 2 files changed, 13 insertions(+), 1 deletion(-)\n>>>>>>\n>>>>>> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>> b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>> index 472facfa5a71..63d639eadacd 100644\n>>>>>> --- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>> @@ -6,6 +6,9 @@ Required properties :\n>>>>>> - interrupts : Interrupt specifier\n>>>>>> Optional properties :\n>>>>>> +- mux-controls : List of mux-ctrl-specifiers containing 1 or\n>>>>>> 2\n>>>>>> muxes\n>>>>>> +- mux-names : \"type-c-mode-mux\" when using 1 mux, or\n>>>>>> + \"type-c-mode-mux\", \"usb-role-mux\" when\n>>>>>> using\n>>>>>> 2 muxes\n>>>>>\n>>>>>\n>>>>>\n>>>>> I'm not sure this is the right place for this. The mux is outside the\n>>>>> FUSB302. In a type-C connector node or USB phy node would make more\n>>>>> sense to me.\n>>>>\n>>>>\n>>>>\n>>>> The mux certainly does not belong in the USB phy node, it sits between\n>>>> the\n>>>> USB PHY\n>>>> and the Type-C connector and can for example also mux the Type-C pins to\n>>>> a\n>>>> Display\n>>>> Port PHY.\n>>>\n>>>\n>>> Thinking about this some more, the mux(es) should be its own node(s).\n>>> Then the question is where to put the nodes.\n>>\n>>\n>> Right, the mux will be its own node per\n>> Documentation/devicetree/bindings/mux/mux-controller.txt\n>> the bindings bit this patch is adding is only adding phandles pointing\n>> to that mux-node as the fusb302 \"consumes\" the mux functionality.\n>>\n>> So as such (the fusb302 is the component which should control the mux)\n>> I do believe that the bindings this patch adds are correct.\n> \n> Humm, that's not how the mux binding works. The mux controller is what\n> drives the mux select lines and is the provider. The consumer is the\n> mux device itself. What decides the mux state is determined by what\n> you are muxing, not which node has mux-controls property.\n> \n> By putting mux-controls in fusb302 node, you are saying fusb302 is a\n> mux (or contains a mux).\n> \n> \n>>>> As for putting it in a type-C connector node, currently we do not have\n>>>> such\n>>>> a node,\n>>>\n>>>\n>>> Well, you should. Type-C connectors are certainly complicated enough\n>>> that we'll need one. Plus we already require connector nodes for\n>>> display outputs, so what do we do once you add display muxing?\n>>\n>>\n>> An interesting question, I'm working on this for x86 + ACPI boards actually,\n>> not a board using DT I've been adding DT bindings docs for device-properties\n>> I use because that seems like the right thing to do where the binding is\n>> obvious\n>> (which I believe it is in this case as the fusb302 is the mux consumer) and\n>> because the device-property code should work the same on x86 + ACPI\n>> (where some platform-specific drivers attach the device properties) and\n>> on e.g. ARM + DT.\n>>\n>> The rest should probably be left to be figured out when an actual DT\n>> using device using the fusb302 or another Type-C controller shows up.\n> \n> Well this is a new one (maybe, I suppose others have sneaked by). If\n> ACPI folks want to use DT bindings, then what do I care. But I have no\n> interest in reviewing ACPI properties. The whole notion of sharing\n> bindings between DT and ACPI beyond anything trivial is flawed IMO.\n> The ptifalls have been discussed multiple times before, so I'm not\n> going to repeat them here.\n\nOk, so shall I just drop the Documentation/devicetree/bindings/usb/fcs,fusb302.txt\npart of this patch then ?\n\nRegards,\n\nHans\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=hdegoede@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsmKZ3qGdz9sPk\n\tfor ;\n\tThu, 14 Sep 2017 01:48:34 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751123AbdIMPsc (ORCPT\n\t);\n\tWed, 13 Sep 2017 11:48:32 -0400","from mx1.redhat.com ([209.132.183.28]:39022 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751145AbdIMPsb (ORCPT );\n\tWed, 13 Sep 2017 11:48:31 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 523015D697;\n\tWed, 13 Sep 2017 15:48:31 +0000 (UTC)","from shalem.localdomain (ovpn-116-240.ams2.redhat.com\n\t[10.36.116.240])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 778D577DC4;\n\tWed, 13 Sep 2017 15:48:27 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 523015D697","Subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","To":"Rob Herring ","Cc":"MyungJoo Ham ,\n\tChanwoo Choi ,\n\tGuenter Roeck , \n\tHeikki Krogerus ,\n\tDarren Hart ,\n\tAndy Shevchenko , \n\tPeter Rosin , Mathias Nyman ,\n\t\"linux-kernel@vger.kernel.org\" ,\n\tplatform-driver-x86@vger.kernel.org, devel@driverdev.osuosl.org,\n\tKuppuswamy Sathyanarayanan , \n\tSathyanarayanan Kuppuswamy Natarajan ,\n\tGreg Kroah-Hartman ,\n\tLinux USB List ,\n\tMark Rutland ,\n\t\"devicetree@vger.kernel.org\" ","References":"<20170905164221.11266-1-hdegoede@redhat.com>\n\t<20170905164221.11266-11-hdegoede@redhat.com>\n\t<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>\n\t\n\t\n\t\n\t","From":"Hans de Goede ","Message-ID":"","Date":"Wed, 13 Sep 2017 17:48:25 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]); Wed, 13 Sep 2017 15:48:31 +0000 (UTC)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1768000,"web_url":"http://patchwork.ozlabs.org/comment/1768000/","msgid":"<20170913161728.GA436@roeck-us.net>","list_archive_url":null,"date":"2017-09-13T16:17:28","subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On Wed, Sep 13, 2017 at 05:48:25PM +0200, Hans de Goede wrote:\n> Hi,\n> \n> On 13-09-17 17:07, Rob Herring wrote:\n> >On Wed, Sep 13, 2017 at 9:06 AM, Hans de Goede wrote:\n> >>Hi,\n> >>\n> >>\n> >>On 13-09-17 15:38, Rob Herring wrote:\n> >>>\n> >>>On Wed, Sep 13, 2017 at 3:56 AM, Hans de Goede \n> >>>wrote:\n> >>>>\n> >>>>Hi,\n> >>>>\n> >>>>\n> >>>>On 13-09-17 00:20, Rob Herring wrote:\n> >>>>>\n> >>>>>\n> >>>>>On Tue, Sep 05, 2017 at 06:42:20PM +0200, Hans de Goede wrote:\n> >>>>>>\n> >>>>>>\n> >>>>>>Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()\n> >>>>>>to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.\n> >>>>>>\n> >>>>>>Also document the mux-names used by the generic tcpc_mux_dev code in\n> >>>>>>our devicetree bindings.\n> >>>>>>\n> >>>>>>Cc: Rob Herring \n> >>>>>>Cc: Mark Rutland \n> >>>>>>Cc: devicetree@vger.kernel.org\n> >>>>>>Signed-off-by: Hans de Goede \n> >>>>>>---\n> >>>>>> Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++\n> >>>>>> drivers/staging/typec/fusb302/fusb302.c | 11\n> >>>>>>++++++++++-\n> >>>>>> 2 files changed, 13 insertions(+), 1 deletion(-)\n> >>>>>>\n> >>>>>>diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n> >>>>>>b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n> >>>>>>index 472facfa5a71..63d639eadacd 100644\n> >>>>>>--- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n> >>>>>>+++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n> >>>>>>@@ -6,6 +6,9 @@ Required properties :\n> >>>>>> - interrupts : Interrupt specifier\n> >>>>>> Optional properties :\n> >>>>>>+- mux-controls : List of mux-ctrl-specifiers containing 1 or\n> >>>>>>2\n> >>>>>>muxes\n> >>>>>>+- mux-names : \"type-c-mode-mux\" when using 1 mux, or\n> >>>>>>+ \"type-c-mode-mux\", \"usb-role-mux\" when\n> >>>>>>using\n> >>>>>>2 muxes\n> >>>>>\n> >>>>>\n> >>>>>\n> >>>>>I'm not sure this is the right place for this. The mux is outside the\n> >>>>>FUSB302. In a type-C connector node or USB phy node would make more\n> >>>>>sense to me.\n> >>>>\n> >>>>\n> >>>>\n> >>>>The mux certainly does not belong in the USB phy node, it sits between\n> >>>>the\n> >>>>USB PHY\n> >>>>and the Type-C connector and can for example also mux the Type-C pins to\n> >>>>a\n> >>>>Display\n> >>>>Port PHY.\n> >>>\n> >>>\n> >>>Thinking about this some more, the mux(es) should be its own node(s).\n> >>>Then the question is where to put the nodes.\n> >>\n> >>\n> >>Right, the mux will be its own node per\n> >>Documentation/devicetree/bindings/mux/mux-controller.txt\n> >>the bindings bit this patch is adding is only adding phandles pointing\n> >>to that mux-node as the fusb302 \"consumes\" the mux functionality.\n> >>\n> >>So as such (the fusb302 is the component which should control the mux)\n> >>I do believe that the bindings this patch adds are correct.\n> >\n> >Humm, that's not how the mux binding works. The mux controller is what\n> >drives the mux select lines and is the provider. The consumer is the\n> >mux device itself. What decides the mux state is determined by what\n> >you are muxing, not which node has mux-controls property.\n> >\n> >By putting mux-controls in fusb302 node, you are saying fusb302 is a\n> >mux (or contains a mux).\n> >\n> >\n> >>>>As for putting it in a type-C connector node, currently we do not have\n> >>>>such\n> >>>>a node,\n> >>>\n> >>>\n> >>>Well, you should. Type-C connectors are certainly complicated enough\n> >>>that we'll need one. Plus we already require connector nodes for\n> >>>display outputs, so what do we do once you add display muxing?\n> >>\n> >>\n> >>An interesting question, I'm working on this for x86 + ACPI boards actually,\n> >>not a board using DT I've been adding DT bindings docs for device-properties\n> >>I use because that seems like the right thing to do where the binding is\n> >>obvious\n> >>(which I believe it is in this case as the fusb302 is the mux consumer) and\n> >>because the device-property code should work the same on x86 + ACPI\n> >>(where some platform-specific drivers attach the device properties) and\n> >>on e.g. ARM + DT.\n> >>\n> >>The rest should probably be left to be figured out when an actual DT\n> >>using device using the fusb302 or another Type-C controller shows up.\n> >\n> >Well this is a new one (maybe, I suppose others have sneaked by). If\n> >ACPI folks want to use DT bindings, then what do I care. But I have no\n> >interest in reviewing ACPI properties. The whole notion of sharing\n> >bindings between DT and ACPI beyond anything trivial is flawed IMO.\n> >The ptifalls have been discussed multiple times before, so I'm not\n> >going to repeat them here.\n> \n> Ok, so shall I just drop the Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n> part of this patch then ?\n> \nFWIW, Android (where the fusb302 driver is coming from) does use dt.\nOn the other side I assume they won't jump on the new mux handling\nimmediately. I won't have time to look into it myself, so whatever\nis done here may not match the \"real\" dt use case. Given that, maybe\nit does make sense to drop the bindings part and revisit once it\nbecomes relevant.\n\nGuenter\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"hIgyyvSA\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xsmz243Dvz9s72\n\tfor ;\n\tThu, 14 Sep 2017 02:17:34 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751662AbdIMQRd (ORCPT\n\t);\n\tWed, 13 Sep 2017 12:17:33 -0400","from mail-pf0-f194.google.com ([209.85.192.194]:33561 \"EHLO\n\tmail-pf0-f194.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751576AbdIMQRb (ORCPT\n\t); Wed, 13 Sep 2017 12:17:31 -0400","by mail-pf0-f194.google.com with SMTP id h4so280642pfk.0;\n\tWed, 13 Sep 2017 09:17:31 -0700 (PDT)","from localhost (108-223-40-66.lightspeed.sntcca.sbcglobal.net.\n\t[108.223.40.66]) by smtp.gmail.com with ESMTPSA id\n\tl12sm23911821pgr.10.2017.09.13.09.17.29\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 13 Sep 2017 09:17:29 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=yTXMw+uW3qMx4aVCiHy/uu4yQEyZAYA6zjA5nNzA9Uo=;\n\tb=hIgyyvSAUx13q0cCT3HzUtYhxHU7UCCA64yQUNPiL0OA5O1ALV+DAk4RKPb18+leGm\n\tjZINXgf5qwOAi9qb6zo0yLKPAddBiBod78hhZhqF//ALg0s2Fddi86MV10twBeDFkA5C\n\txn86awQsf1Oyd/MCc/I+IodAeKRGl8pAswDzM/yC9apbsFR5zjPiM6xGLzXhsaQnNNPT\n\t6Ut8CwMukaZJtQowr8CalixOCBy+lFoWSCeIskIj6ijftph+I2PF45OnJ9bf3sC1BvvF\n\tPSr/IV229sCu7VPswomm70t31VeUa80ONRpwl5ESn/SX2vTDcxzQjiMJcUs1JZ8I5XYr\n\t4Tbg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:date:from:to:cc:subject:message-id\n\t:references:mime-version:content-disposition:in-reply-to:user-agent; \n\tbh=yTXMw+uW3qMx4aVCiHy/uu4yQEyZAYA6zjA5nNzA9Uo=;\n\tb=fscRKibGY/hLOBsWT861CEdsy6sznqI0CUZRlxSC+OZ+O1Ek6hkRiy+P3I2Swif6me\n\thYlQQV8bVG+x1REoqk/1NwNeMK+jjujL/uUc9aqHPYMNHo6HMF9H9ZVFTtxDVEXo9v8/\n\t/MfAQ8b3w4X8G5j1vKk7fGcfIy3Q6noEtjjAjdpVR6Q3/36DyRam68ZvLLD0vIbm+jhy\n\t5D5AaZd3+bL5NblJ24zXTN2wrBMMKraXWZ0HMWuluWiVFI5uWAyl4hcGpfO41EFpWxTB\n\tP1pc6ZnbzcwrZ9ZEZiZpoG3uQbktIZ3fgxyYSTHLmvJ2LrkVa9J+zx44bPftyPLb6J1f\n\tecIg==","X-Gm-Message-State":"AHPjjUgMRcnZF/5RS7JbS+iwWb2pu4al8+gVWj1YUnVY4g5shMqO/Im0\n\tE6D+KUddgKd8xw==","X-Google-Smtp-Source":"ADKCNb7jRqxsaKIXq/r8i83LeWmz9I1CngLnYQEvwR1YJzf7iOL7uAgAj6nM7AK7gqjY+dlG9Zskmw==","X-Received":"by 10.84.210.225 with SMTP id a88mr13941243pli.79.1505319450685; \n\tWed, 13 Sep 2017 09:17:30 -0700 (PDT)","Date":"Wed, 13 Sep 2017 09:17:28 -0700","From":"Guenter Roeck ","To":"Hans de Goede ","Cc":"Rob Herring , MyungJoo Ham , \n\tChanwoo Choi ,\n\tHeikki Krogerus ,\n\tDarren Hart ,\n\tAndy Shevchenko , \n\tPeter Rosin , Mathias Nyman ,\n\t\"linux-kernel@vger.kernel.org\" ,\n\tplatform-driver-x86@vger.kernel.org, devel@driverdev.osuosl.org,\n\tKuppuswamy Sathyanarayanan , \n\tSathyanarayanan Kuppuswamy Natarajan ,\n\tGreg Kroah-Hartman ,\n\tLinux USB List ,\n\tMark Rutland ,\n\t\"devicetree@vger.kernel.org\" ","Subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","Message-ID":"<20170913161728.GA436@roeck-us.net>","References":"<20170905164221.11266-1-hdegoede@redhat.com>\n\t<20170905164221.11266-11-hdegoede@redhat.com>\n\t<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>\n\t\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.5.24 (2015-08-30)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1774637,"web_url":"http://patchwork.ozlabs.org/comment/1774637/","msgid":"<3d207dc4-1dba-ce34-21af-bab001ec68c5@axentia.se>","list_archive_url":null,"date":"2017-09-25T10:34:58","subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","submitter":{"id":66028,"url":"http://patchwork.ozlabs.org/api/people/66028/","name":"Peter Rosin","email":"peda@axentia.se"},"content":"On 2017-09-13 17:48, Hans de Goede wrote:\n> Hi,\n> \n> On 13-09-17 17:07, Rob Herring wrote:\n>> On Wed, Sep 13, 2017 at 9:06 AM, Hans de Goede wrote:\n>>> Hi,\n>>>\n>>>\n>>> On 13-09-17 15:38, Rob Herring wrote:\n>>>>\n>>>> On Wed, Sep 13, 2017 at 3:56 AM, Hans de Goede \n>>>> wrote:\n>>>>>\n>>>>> Hi,\n>>>>>\n>>>>>\n>>>>> On 13-09-17 00:20, Rob Herring wrote:\n>>>>>>\n>>>>>>\n>>>>>> On Tue, Sep 05, 2017 at 06:42:20PM +0200, Hans de Goede wrote:\n>>>>>>>\n>>>>>>>\n>>>>>>> Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()\n>>>>>>> to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.\n>>>>>>>\n>>>>>>> Also document the mux-names used by the generic tcpc_mux_dev code in\n>>>>>>> our devicetree bindings.\n>>>>>>>\n>>>>>>> Cc: Rob Herring \n>>>>>>> Cc: Mark Rutland \n>>>>>>> Cc: devicetree@vger.kernel.org\n>>>>>>> Signed-off-by: Hans de Goede \n>>>>>>> ---\n>>>>>>> Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++\n>>>>>>> drivers/staging/typec/fusb302/fusb302.c | 11\n>>>>>>> ++++++++++-\n>>>>>>> 2 files changed, 13 insertions(+), 1 deletion(-)\n>>>>>>>\n>>>>>>> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>> b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>> index 472facfa5a71..63d639eadacd 100644\n>>>>>>> --- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>> @@ -6,6 +6,9 @@ Required properties :\n>>>>>>> - interrupts : Interrupt specifier\n>>>>>>> Optional properties :\n>>>>>>> +- mux-controls : List of mux-ctrl-specifiers containing 1 or\n>>>>>>> 2\n>>>>>>> muxes\n>>>>>>> +- mux-names : \"type-c-mode-mux\" when using 1 mux, or\n>>>>>>> + \"type-c-mode-mux\", \"usb-role-mux\" when\n>>>>>>> using\n>>>>>>> 2 muxes\n>>>>>>\n>>>>>>\n>>>>>>\n>>>>>> I'm not sure this is the right place for this. The mux is outside the\n>>>>>> FUSB302. In a type-C connector node or USB phy node would make more\n>>>>>> sense to me.\n>>>>>\n>>>>>\n>>>>>\n>>>>> The mux certainly does not belong in the USB phy node, it sits between\n>>>>> the\n>>>>> USB PHY\n>>>>> and the Type-C connector and can for example also mux the Type-C pins to\n>>>>> a\n>>>>> Display\n>>>>> Port PHY.\n>>>>\n>>>>\n>>>> Thinking about this some more, the mux(es) should be its own node(s).\n>>>> Then the question is where to put the nodes.\n>>>\n>>>\n>>> Right, the mux will be its own node per\n>>> Documentation/devicetree/bindings/mux/mux-controller.txt\n>>> the bindings bit this patch is adding is only adding phandles pointing\n>>> to that mux-node as the fusb302 \"consumes\" the mux functionality.\n>>>\n>>> So as such (the fusb302 is the component which should control the mux)\n>>> I do believe that the bindings this patch adds are correct.\n>>\n>> Humm, that's not how the mux binding works. The mux controller is what\n>> drives the mux select lines and is the provider. The consumer is the\n>> mux device itself. What decides the mux state is determined by what\n>> you are muxing, not which node has mux-controls property.\n>>\n>> By putting mux-controls in fusb302 node, you are saying fusb302 is a\n>> mux (or contains a mux).\n>>\n>>\n>>>>> As for putting it in a type-C connector node, currently we do not have\n>>>>> such\n>>>>> a node,\n>>>>\n>>>>\n>>>> Well, you should. Type-C connectors are certainly complicated enough\n>>>> that we'll need one. Plus we already require connector nodes for\n>>>> display outputs, so what do we do once you add display muxing?\n>>>\n>>>\n>>> An interesting question, I'm working on this for x86 + ACPI boards actually,\n>>> not a board using DT I've been adding DT bindings docs for device-properties\n>>> I use because that seems like the right thing to do where the binding is\n>>> obvious\n>>> (which I believe it is in this case as the fusb302 is the mux consumer) and\n>>> because the device-property code should work the same on x86 + ACPI\n>>> (where some platform-specific drivers attach the device properties) and\n>>> on e.g. ARM + DT.\n>>>\n>>> The rest should probably be left to be figured out when an actual DT\n>>> using device using the fusb302 or another Type-C controller shows up.\n>>\n>> Well this is a new one (maybe, I suppose others have sneaked by). If\n>> ACPI folks want to use DT bindings, then what do I care. But I have no\n>> interest in reviewing ACPI properties. The whole notion of sharing\n>> bindings between DT and ACPI beyond anything trivial is flawed IMO.\n>> The ptifalls have been discussed multiple times before, so I'm not\n>> going to repeat them here.\n> \n> Ok, so shall I just drop the Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n> part of this patch then ?\n\nI totally agree with the concern that Rob expressed about handling USB C\nmuxes in a non-adhoc way. And this makes this series scary. I don't know\nenough details about USB C muxes and PD (I just have a very superficial\nmental model) to tell if this series is going down the right path. Or\nhow terrible it will be to fix things up if not?\n\nThe series extends the mux subsystem with muxes that pins semantics\nto specific states. That is new, and shows up exactly here when Rob is\nnot happy about the binding. And if Rob does not want this in the\nDT bindings then I'm not so sure it is wise to do it at all? This\nproblem doesn't go away just because you remove the binding. I think\nI would feel much better if there was a path forward on how to\nrepresent USB C muxes in DT and how that would fit with the driver\nstructure.\n\nIf you compare to the i2c-muxes, there is a relatively new i2c-mux-gpmux\ndriver that uses some general purpose mux from the mux subsystem to\nimplement an i2c-mux. If USB C muxes where to be done similarly, I'd\nimagine there should be a general abstraction of what USB C muxes provide\nsomewhere outside of the mux subsystem, and a bunch of implementations\nof that abstraction. One of those implementations could be to use \"raw\"\nmuxes from the mux subsystem. Of course, this is not what this series is\ndoing.\n\nAlso, muxes that are not general purpose such as the ones added to the\nmux subsystem by this series could perhaps be repurposed for some other\napplication, but since the interface implemented does not really obey\nthe rules (the provided mux controller interacts with different sets of\nsignals depending on the state) this will not be possible.\n\nThese issues are what has caused me to do a lot of thinking and to sit\nsilent, sorry about that, but I would like input from someone more\nexperienced. If possible. But I'm not sure where to turn?\n\nAs a crazy example, why is it not possible to hook up one signal pair\nfrom the USB C mux, not to DP, but instead to some I2C controller? Then,\nif done right, i2c-mux-gpmux could be hooked up with the relevant mux\ncontroller and use the signal pair for I2C, with the mux controller\nacting as a gate. So, maybe a bit crazy, but something like that is how\nI think it should work from the mux subsystem point of view. And while\nmaybe crazy and while it might not be technically possible to do I2C\nover a USB C connector for some reason, I do think that whatever\nabstraction you come up with for USB C muxes, it has to deal with and\nbroker requests from both the USB subsystem and whatever other\nsubsystems deals with the alt pairs. Be it graphics for DP signals, or\nwhatever. IIUC, the alt signals need not be graphics, and it would be\nsad to implement the USB C mux it in a way that makes it hard to use\nthe alt pairs for something else.\n\n[maybe my understanding of USB C is just wrong]\n\nCheers,\nPeter\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=axentiatech.onmicrosoft.com\n\theader.i=@axentiatech.onmicrosoft.com header.b=\"fuYhLm+9\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=peda@axentia.se; "],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y10pd63qYz9tXF\n\tfor ;\n\tMon, 25 Sep 2017 20:35:21 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S934467AbdIYKfJ (ORCPT );\n\tMon, 25 Sep 2017 06:35:09 -0400","from mail-db5eur01on0136.outbound.protection.outlook.com\n\t([104.47.2.136]:27520\n\t\"EHLO EUR01-DB5-obe.outbound.protection.outlook.com\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1753875AbdIYKfF (ORCPT );\n\tMon, 25 Sep 2017 06:35:05 -0400","from [192.168.13.3] (81.224.168.30) by\n\tHE1PR0202MB2554.eurprd02.prod.outlook.com (2603:10a6:3:90::7) with\n\tMicrosoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.77.7;\n\tMon, 25 Sep 2017 10:35:01 +0000"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=axentiatech.onmicrosoft.com; s=selector1-axentia-se;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=M1FQoux5oKmXgMB9C2EUl/phMvAWUj0Q32IZpYOJdSY=;\n\tb=fuYhLm+9Veb8FRsEmhXzB1IH0wnrVBuFLQERlfVI+vKvnwKXwlOEytrqYuZKAYoXMBrKAxisPyYX+29rYosW/+jOoAGLm14azgc4LdPBkPXj2RsU20wB2OwEpakNRJTzkECYr059ry+vNArfE6VHlhbgPGzH2GV77hvEdrPfrMY=","Subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","To":"Hans de Goede , Rob Herring ","Cc":"MyungJoo Ham ,\n\tChanwoo Choi ,\n\tGuenter Roeck , \n\tHeikki Krogerus ,\n\tDarren Hart ,\n\tAndy Shevchenko , \n\tMathias Nyman ,\n\t\"linux-kernel@vger.kernel.org\" ,\n\tplatform-driver-x86@vger.kernel.org, devel@driverdev.osuosl.org,\n\tKuppuswamy Sathyanarayanan , \n\tSathyanarayanan Kuppuswamy Natarajan ,\n\tGreg Kroah-Hartman ,\n\tLinux USB List ,\n\tMark Rutland ,\n\t\"devicetree@vger.kernel.org\" ","References":"<20170905164221.11266-1-hdegoede@redhat.com>\n\t<20170905164221.11266-11-hdegoede@redhat.com>\n\t<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>\n\t\n\t\n\t\n\t\n\t","From":"Peter Rosin ","Organization":"Axentia Technologies AB","Message-ID":"<3d207dc4-1dba-ce34-21af-bab001ec68c5@axentia.se>","Date":"Mon, 25 Sep 2017 12:34:58 +0200","User-Agent":"Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[81.224.168.30]","X-ClientProxiedBy":"HE1PR05CA0272.eurprd05.prod.outlook.com\n\t(2603:10a6:3:fc::24) To HE1PR0202MB2554.eurprd02.prod.outlook.com\n\t(2603:10a6:3:90::7)","X-MS-PublicTrafficType":"Email","X-MS-Office365-Filtering-Correlation-Id":"7e4d12b4-1888-45bc-ad33-08d504011434","X-Microsoft-Antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(22001)(2017030254152)(2017082002075)(2017052603199)(201703131423075)(201702281549075);\n\tSRVR:HE1PR0202MB2554; ","X-Microsoft-Exchange-Diagnostics":["1; HE1PR0202MB2554;\n\t3:QsNIGl02cYo7Vh4DE2ZBZiIlV4weI8aJQWw+AaPhYP0wzQGioMowUCGtxd6xb45z8jWzN2JOiiRY//aQCfZa4HpxvfKE+RYfOU1myinJm8PO95W1gxR5ibERcxS4eynln0Lmr5TXsmSEM/Y7msmlKmBpyyXRVUaRKEFCcCLMXJv/7SjGYX34AZGS7Tz6t74tWDCN5KqXpQPS67nGusZ55vSssFcIant2hAALE18M+xTAi06R5Q2NMNp/WIlht37w;\n\t25:CykzkxnOUIdcyAlLStugk0rBwbsX8qqYEMF9B4Qd02VRgRHLPDwXNWd+1OS0b/ABYtDzsRSsnRUpt/Q3Roz0tSzjNNuSRr396RHCVFnVVFxibyNXcNbhpfI1M5JjvRIJ4eihuF0LR65jNus98Gdut5pTcBZ+gF1AxGPZVYIrQD64w/1aGSWxL1suswvByEpytlNJo/M6R2Uyq7qyByzefMopKZf0ZGoLZcr08vzewtDOAuDwd42AYttfSd8w9SS+Ui3J+2JFvyQyGtpzTWxs0cI5PlvmsTJ0JfJTYFccW33fZElEnGCLJ+NFAVOdoKsleLZzl0l/HHiQWMleaDNu1g==;\n\t31:mZAk3cVUxZqNquzaTw/C/+b0XJ9dHwOMEVheq+EkjYS0C2wYdy+TW6KTRQCMsBafBar0QLqvTdGWS68cpqeUjUU8hCdTRh4rejIcr8xUMeu8SdFIrx9drjA8bHwPu2ms8KTMWobiS9UnJuIfPB/euUKcZYjniqow1wqx7Hfm4ErkbyTKnhbBascUad5rIC8IVq+nI/EBOxHW7y1QS7ThOrKWUVz9Rx+4qWN2IP23l0A=","1; HE1PR0202MB2554;\n\t4:4tbG+cWrbYepuRsllOL1kGu76g1OfRw+8gF4y1aeKiHBxWHqJ1u8j2rmLUFE9cVOjNuDclGynkwZabWWAUgd8xYRYIKmOkPv4TJtRzRp+5Ku9AM2dpzmhIqXYjfOo074fGtofZvpN0YKw+R69LoH4o76zNobmbKCFuhfeerbdRrlfug+Zioz53HjlhpAp1v9j7lnjgzsxhu2N8HIrxgFBfpuDjE7zS0wUs344VhmbjBf6n3vIVnLcgPoW/FzD3utvU1kA3WOCKC2LudjegArm/cVRyyuofcva1+9v/gJqAzhNs8qJu9igqFLWQZKLfzbbHmWqW34fZS7zK6UL/8lkA==","=?utf-8?q?1=3BHE1PR0202MB2554=3B23=3A8m?=\n\t=?utf-8?q?Hz1ZaQbGpgPKeEAPTnOi0h5L1+WH+S9Duin3YLhAZiQ9ux/oLRWk5/a7?=\n\t=?utf-8?q?0feUXH/94cWaG9QRYxjFN+b3lcF5ScOF0XSmgKDIwxosMVpR8woCRAKE?=\n\t=?utf-8?q?qz74N19B1CNH1qfJaU+FO90IUujn0oYz/ltMESXJ+STINeRqZOT6fFRd?=\n\t=?utf-8?q?Nucn7K4XCEfMzbJx7dw1/ZAu37WZAJbipTpU8l3RV7YkytE2eN3Xx+0E?=\n\t=?utf-8?q?eWrHaZKi5WkbnAqRlvBkMfkR7RywSUjypuaH6lZgGIbjn9rBq+fcykPm?=\n\t=?utf-8?q?8atqrLYwK8by4a8ElsYh/bFUWPtqSeCNpC7LFPK1DMs2KydB4s7edyCE?=\n\t=?utf-8?q?tOXoD4JrqiHG0VVFkHWEgoIq0tBaOnQA7/8NlptSxtgbOHTLLZxquXgU?=\n\t=?utf-8?q?3/LGXJfBE9sWVcFkaqWoOm5nXv1w6z+bFRVANEzMkP983C+wziURbfS8?=\n\t=?utf-8?q?lOEJQDkvnLVWJt1LvvXYppTtsb5nCp18k/BW7f3xHSXPT0Wz46ECmt75?=\n\t=?utf-8?q?NvKlcDKsuiHzc9veJQ8WQ2H66sdUqWR0dj4/L7NexgzgHGrfD5c2VccG?=\n\t=?utf-8?q?clr768RRUaiidd6PC6Vs8+1VBiHi/qn+wX516FRyiIyQMhQC4A2TZ9tR?=\n\t=?utf-8?q?M5w0Wg4fc6ZDFMH9xK+hf2rUP+EmzEoLZv1khAzXwMSI28jwSyCZePIy?=\n\t=?utf-8?q?X8URqpsSOqzePcgL7yE0ZWjpVYdlmWD278OAsgp+aev77I3RVKvthISF?=\n\t=?utf-8?q?4lOEAg5zRcyGlrveJNIZ1eI1eZYMC0xpx3xroqN4jH/q744P6kjMOGvs?=\n\t=?utf-8?q?yt6m/iKlRpFeU7g5lSMs0eCsmVq4kBiUVF2iktQ59raZcbM0jvAep5mk?=\n\t=?utf-8?q?XCKLTAGcSp1/fbHrDFD0/A93B3QzTZo0ReA96qU38Zsibslb8Us98sNI?=\n\t=?utf-8?q?kIEHVOxEWrVxkhq2CH4me5XW6Kb5CGkuvWCggBhOBv1TU6kP5NpKLG6K?=\n\t=?utf-8?q?dHZp0lzuZAvmUDZxsleqAyivEYzRPoCdk+n1QKnyAbDPgak0P7IEpUMi?=\n\t=?utf-8?q?NC54htmP1sxC5BCm4LffIs2dukslTGEUIZum4F3Kr7NgYTjfWtMomjIt?=\n\t=?utf-8?q?0TvfdE4ow9V2HmC/rzMyJOe7okSOQgNJF4AbUtkCfTBrWxX3U001YyRl?=\n\t=?utf-8?q?UDFjtc7488awUNQHTXhLkhLsXEr4gPJBQFEpCSfEQVLQCs1o7uh6bNpj?=\n\t=?utf-8?q?7mR7nybRuo28hdR3WbJJHXInV5CIRR6A2RzkjgkFLJgSN05ONJwoK2Hl?=\n\t=?utf-8?q?OPfD1lJVsKI0CYt78GCpol4WKb8FwUZ3QPMLInKHeC7mAONmLMVh9aub?=\n\t=?utf-8?q?2CndAI+HsrXVBWa7uBst6RrpfzB++zt5QoNvHaZjIIZdeP01iJXQf2ln?=\n\t=?utf-8?q?LCfugmz11RneyyJckRXg7iYka3FudlFybONuLRq0Jdy98ZuMKOx1u067?=\n\t=?utf-8?q?nPuYcfnd6TqBZfVbNtM+i9ZZPpWpzHlU08Ex8qA2TjdBgQ563AUPlbkT?=\n\t=?utf-8?q?Nox3PtX/N9IctFqj+Ef44K3ll+c2ahMAYErfUbeCc46gvOsQGxqxSzPS?=\n\t=?utf-8?q?25eSsnc4fn0CBuSWrnFqJLDD+XbUr/3CLxYP1Ha+aXmS4LJ2HLy2Ihfr?=\n\t=?utf-8?q?Yfaxyrjf2N3UTnUKVh7zemfeQcYC4TNUBkW/I8Y0h7iDdqMJw3eA=3D?=\n\t=?utf-8?q?=3D?=","1; HE1PR0202MB2554;\n\t6:0Il3QgK9fc6LRwrvnwKJp4geQvQtLntMpzwoOt6tf30sFmc/bOWenMbIBAK+jTP0DQ+F5oGj9E2dsRvzvxGO+UbSQ9d8iSuIxTjEQ5PI82G3PpfomX1zWuKC+2mXqPeLckaOUAKJvV47tZlGMeBvQa8GfsQHbiYJyLGnycKmIteL8KRNRnt2VYb7n8vR9tCg4j2R3MopUrlCW4qWoOC8koNn3Nj7XY6pnKrYLZ3sj44EK3DElj2CRDdGQ0oqCgONpDARZ04bdRQf4BiAfIi18qexCU/uPylCM5rK9nzfl2HUR1MSrfUGS20mOCo9L9L+bwhv2nvEJX8nsYr+Qsn9Pw==;\n\t5:Q/kGptfbwCbz4shLplpvjFm8bGau2btxiZ0pTB55ARFY7cTXIkr20gEH6roOySRl9cftFtIryiZoaYgSTMA1Y0ka2h1bptK/yvnxxLymm+K6Ii8a/oNXU3qxo16MjRPd3PQGO2EvplwxWx1xU0sfIA==;\n\t24:PWYng+y+BMimsr9+R7s0Pv+jPbxy+M4pouguRpVyTOmqU1Xk5XX4oK0p7ukpTcapXPlQQzA9BeGLF6XV5EjHNR+/3Rky1OmfONAG92Kzwz4=;\n\t7:wbiwPJTCTEm4mQcXaA2s+1qez8qx+OMiTQllgkSNJ2RaCv90PgC7rEJrP4kOc61UqPtWcLjFWMUMQI8NM6gM9qbRMBEIUdc5VGN2+/WlEwFLFHr9zMweK1Bci6P+wW/58axhDeCMtVdsh5o/bIV3j5guGXJwP0OUU8YeuqRZoO3D9J7AIygAYHgUjlrjh5FeMeQL64Keg/XdyRwRFonpz1bwVoe47Vl0MRpAlavM84s="],"X-MS-TrafficTypeDiagnostic":"HE1PR0202MB2554:","X-Exchange-Antispam-Report-Test":"UriScan:(180628864354917)(9452136761055);","X-Microsoft-Antispam-PRVS":"","X-Exchange-Antispam-Report-CFA-Test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(10201501046)(100000703101)(100105400095)(93006095)(93001095)(3002001)(6041248)(2016111802025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123555025)(20161123564025)(20161123558100)(20161123562025)(6072148)(6043046)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:HE1PR0202MB2554; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:HE1PR0202MB2554; ","X-Forefront-PRVS":"04410E544A","X-Forefront-Antispam-Report":"SFV:NSPM;\n\tSFS:(10019020)(6009001)(6049001)(39830400002)(346002)(376002)(377454003)(189002)(199003)(377424004)(24454002)(101416001)(117156002)(6486002)(316002)(189998001)(81156014)(53936002)(16576012)(31686004)(3846002)(74482002)(305945005)(23676002)(6116002)(7416002)(77096006)(7736002)(8936002)(65806001)(105586002)(54906003)(83506001)(110136005)(81166006)(58126008)(39060400002)(36756003)(478600001)(86362001)(54356999)(3260700006)(50986999)(4326008)(76176999)(33646002)(65956001)(53546010)(8676002)(2950100002)(25786009)(97736004)(5660300001)(47776003)(31696002)(5890100001)(68736007)(230700001)(106356001)(6666003)(229853002)(66066001)(93886005)(65826007)(2906002)(50466002)(6246003)(64126003)(52103002)(158003001)(42262002);\n\tDIR:OUT; SFP:1102; SCL:1; SRVR:HE1PR0202MB2554; H:[192.168.13.3]; FPR:;\n\tSPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; ","Received-SPF":"None (protection.outlook.com: axentia.se does not designate\n\tpermitted sender hosts)","SpamDiagnosticOutput":"1:99","SpamDiagnosticMetadata":"NSPM","X-OriginatorOrg":"axentia.se","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"25 Sep 2017 10:35:01.1080\n\t(UTC)","X-MS-Exchange-CrossTenant-FromEntityHeader":"Hosted","X-MS-Exchange-CrossTenant-Id":"4ee68585-03e1-4785-942a-df9c1871a234","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"HE1PR0202MB2554","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1774661,"web_url":"http://patchwork.ozlabs.org/comment/1774661/","msgid":"<1aa88cc3-b2ab-2308-1039-c1b7729c313b@redhat.com>","list_archive_url":null,"date":"2017-09-25T11:35:31","subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","submitter":{"id":1893,"url":"http://patchwork.ozlabs.org/api/people/1893/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 25-09-17 12:34, Peter Rosin wrote:\n> On 2017-09-13 17:48, Hans de Goede wrote:\n>> Hi,\n>>\n>> On 13-09-17 17:07, Rob Herring wrote:\n>>> On Wed, Sep 13, 2017 at 9:06 AM, Hans de Goede wrote:\n>>>> Hi,\n>>>>\n>>>>\n>>>> On 13-09-17 15:38, Rob Herring wrote:\n>>>>>\n>>>>> On Wed, Sep 13, 2017 at 3:56 AM, Hans de Goede \n>>>>> wrote:\n>>>>>>\n>>>>>> Hi,\n>>>>>>\n>>>>>>\n>>>>>> On 13-09-17 00:20, Rob Herring wrote:\n>>>>>>>\n>>>>>>>\n>>>>>>> On Tue, Sep 05, 2017 at 06:42:20PM +0200, Hans de Goede wrote:\n>>>>>>>>\n>>>>>>>>\n>>>>>>>> Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()\n>>>>>>>> to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.\n>>>>>>>>\n>>>>>>>> Also document the mux-names used by the generic tcpc_mux_dev code in\n>>>>>>>> our devicetree bindings.\n>>>>>>>>\n>>>>>>>> Cc: Rob Herring \n>>>>>>>> Cc: Mark Rutland \n>>>>>>>> Cc: devicetree@vger.kernel.org\n>>>>>>>> Signed-off-by: Hans de Goede \n>>>>>>>> ---\n>>>>>>>> Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++\n>>>>>>>> drivers/staging/typec/fusb302/fusb302.c | 11\n>>>>>>>> ++++++++++-\n>>>>>>>> 2 files changed, 13 insertions(+), 1 deletion(-)\n>>>>>>>>\n>>>>>>>> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>>> b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>>> index 472facfa5a71..63d639eadacd 100644\n>>>>>>>> --- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>>> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>>> @@ -6,6 +6,9 @@ Required properties :\n>>>>>>>> - interrupts : Interrupt specifier\n>>>>>>>> Optional properties :\n>>>>>>>> +- mux-controls : List of mux-ctrl-specifiers containing 1 or\n>>>>>>>> 2\n>>>>>>>> muxes\n>>>>>>>> +- mux-names : \"type-c-mode-mux\" when using 1 mux, or\n>>>>>>>> + \"type-c-mode-mux\", \"usb-role-mux\" when\n>>>>>>>> using\n>>>>>>>> 2 muxes\n>>>>>>>\n>>>>>>>\n>>>>>>>\n>>>>>>> I'm not sure this is the right place for this. The mux is outside the\n>>>>>>> FUSB302. In a type-C connector node or USB phy node would make more\n>>>>>>> sense to me.\n>>>>>>\n>>>>>>\n>>>>>>\n>>>>>> The mux certainly does not belong in the USB phy node, it sits between\n>>>>>> the\n>>>>>> USB PHY\n>>>>>> and the Type-C connector and can for example also mux the Type-C pins to\n>>>>>> a\n>>>>>> Display\n>>>>>> Port PHY.\n>>>>>\n>>>>>\n>>>>> Thinking about this some more, the mux(es) should be its own node(s).\n>>>>> Then the question is where to put the nodes.\n>>>>\n>>>>\n>>>> Right, the mux will be its own node per\n>>>> Documentation/devicetree/bindings/mux/mux-controller.txt\n>>>> the bindings bit this patch is adding is only adding phandles pointing\n>>>> to that mux-node as the fusb302 \"consumes\" the mux functionality.\n>>>>\n>>>> So as such (the fusb302 is the component which should control the mux)\n>>>> I do believe that the bindings this patch adds are correct.\n>>>\n>>> Humm, that's not how the mux binding works. The mux controller is what\n>>> drives the mux select lines and is the provider. The consumer is the\n>>> mux device itself. What decides the mux state is determined by what\n>>> you are muxing, not which node has mux-controls property.\n>>>\n>>> By putting mux-controls in fusb302 node, you are saying fusb302 is a\n>>> mux (or contains a mux).\n>>>\n>>>\n>>>>>> As for putting it in a type-C connector node, currently we do not have\n>>>>>> such\n>>>>>> a node,\n>>>>>\n>>>>>\n>>>>> Well, you should. Type-C connectors are certainly complicated enough\n>>>>> that we'll need one. Plus we already require connector nodes for\n>>>>> display outputs, so what do we do once you add display muxing?\n>>>>\n>>>>\n>>>> An interesting question, I'm working on this for x86 + ACPI boards actually,\n>>>> not a board using DT I've been adding DT bindings docs for device-properties\n>>>> I use because that seems like the right thing to do where the binding is\n>>>> obvious\n>>>> (which I believe it is in this case as the fusb302 is the mux consumer) and\n>>>> because the device-property code should work the same on x86 + ACPI\n>>>> (where some platform-specific drivers attach the device properties) and\n>>>> on e.g. ARM + DT.\n>>>>\n>>>> The rest should probably be left to be figured out when an actual DT\n>>>> using device using the fusb302 or another Type-C controller shows up.\n>>>\n>>> Well this is a new one (maybe, I suppose others have sneaked by). If\n>>> ACPI folks want to use DT bindings, then what do I care. But I have no\n>>> interest in reviewing ACPI properties. The whole notion of sharing\n>>> bindings between DT and ACPI beyond anything trivial is flawed IMO.\n>>> The ptifalls have been discussed multiple times before, so I'm not\n>>> going to repeat them here.\n>>\n>> Ok, so shall I just drop the Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>> part of this patch then ?\n> \n> I totally agree with the concern that Rob expressed about handling USB C\n> muxes in a non-adhoc way. And this makes this series scary. I don't know\n> enough details about USB C muxes and PD (I just have a very superficial\n> mental model) to tell if this series is going down the right path. Or\n> how terrible it will be to fix things up if not?\n> \n> The series extends the mux subsystem with muxes that pins semantics\n> to specific states. That is new, and shows up exactly here when Rob is\n> not happy about the binding. And if Rob does not want this in the\n> DT bindings then I'm not so sure it is wise to do it at all? This\n> problem doesn't go away just because you remove the binding. I think\n> I would feel much better if there was a path forward on how to\n> represent USB C muxes in DT and how that would fit with the driver\n> structure.\n> \n> If you compare to the i2c-muxes, there is a relatively new i2c-mux-gpmux\n> driver that uses some general purpose mux from the mux subsystem to\n> implement an i2c-mux. If USB C muxes where to be done similarly, I'd\n> imagine there should be a general abstraction of what USB C muxes provide\n> somewhere outside of the mux subsystem, and a bunch of implementations\n> of that abstraction. One of those implementations could be to use \"raw\"\n> muxes from the mux subsystem. Of course, this is not what this series is\n> doing.\n> \n> Also, muxes that are not general purpose such as the ones added to the\n> mux subsystem by this series could perhaps be repurposed for some other\n> application, but since the interface implemented does not really obey\n> the rules (the provided mux controller interacts with different sets of\n> signals depending on the state) this will not be possible.\n> \n> These issues are what has caused me to do a lot of thinking and to sit\n> silent, sorry about that, but I would like input from someone more\n> experienced. If possible. But I'm not sure where to turn?\n> \n> As a crazy example, why is it not possible to hook up one signal pair\n> from the USB C mux, not to DP, but instead to some I2C controller? Then,\n> if done right, i2c-mux-gpmux could be hooked up with the relevant mux\n> controller and use the signal pair for I2C, with the mux controller\n> acting as a gate. So, maybe a bit crazy, but something like that is how\n> I think it should work from the mux subsystem point of view. And while\n> maybe crazy and while it might not be technically possible to do I2C\n> over a USB C connector for some reason, I do think that whatever\n> abstraction you come up with for USB C muxes, it has to deal with and\n> broker requests from both the USB subsystem and whatever other\n> subsystems deals with the alt pairs. Be it graphics for DP signals, or\n> whatever. IIUC, the alt signals need not be graphics, and it would be\n> sad to implement the USB C mux it in a way that makes it hard to use\n> the alt pairs for something else.\n> \n> [maybe my understanding of USB C is just wrong]\n\nSo 2 things:\n\n1) The Type-C subsys does actually abstract the mux outside of the\nType-C port-manager (TCPM) core in the form of a tcpc_mux_dev, so for\nDT based platforms we could instantiate a tcpc_mux_dev from a node\nwhich then represents the mux as usual.\n\n2) What is very different from how the mux subsys currently is used,\nis who is in control of the mux. Currently a subsys which wants to\nroute data through the mux selects the mux in the right mode, and\nthen deselects it when done. This assumes that the mux can mux\nthe data paths to the requested destination at all times, just not\nto multiple destinations at once. With Type-C and any moment in\ntime, there really is only one correct setting as the mux is\nconnect to a Type-C device on the other side which typically\nonly has one config. So what happens with Type-C is that the TCPM\nnegotiates with the externally connected Type-C device, and then\nsets the mux to the one and only correct setting for that device\nto be fully functional.\n\nSo your i2c example, for example, will not work with the normal\nway where the i2c subsys asks the mux to mux a data-pair to the i2c\ncontroller, as that only makes sense when your theoretical Type-C\ndevice which talks i2c over a pair is connected externally, where\nas when something else is connected then trying to talk i2c might\neven damage the connected device. So with Type-C it is the TCPM\nand only the TCPM which controls the mux.\n\nRegards,\n\nHans\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=hdegoede@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y128F5ykDz9t67\n\tfor ;\n\tMon, 25 Sep 2017 21:35:41 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751852AbdIYLfj (ORCPT\n\t);\n\tMon, 25 Sep 2017 07:35:39 -0400","from mx1.redhat.com ([209.132.183.28]:52104 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751850AbdIYLfh (ORCPT );\n\tMon, 25 Sep 2017 07:35:37 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 6CE7085547;\n\tMon, 25 Sep 2017 11:35:37 +0000 (UTC)","from shalem.localdomain (ovpn-117-212.ams2.redhat.com\n\t[10.36.117.212])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 675BE60494;\n\tMon, 25 Sep 2017 11:35:33 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 6CE7085547","Subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","To":"Peter Rosin , Rob Herring ","Cc":"MyungJoo Ham ,\n\tChanwoo Choi ,\n\tGuenter Roeck , \n\tHeikki Krogerus ,\n\tDarren Hart ,\n\tAndy Shevchenko , \n\tMathias Nyman ,\n\t\"linux-kernel@vger.kernel.org\" ,\n\tplatform-driver-x86@vger.kernel.org, devel@driverdev.osuosl.org,\n\tKuppuswamy Sathyanarayanan , \n\tSathyanarayanan Kuppuswamy Natarajan ,\n\tGreg Kroah-Hartman ,\n\tLinux USB List ,\n\tMark Rutland ,\n\t\"devicetree@vger.kernel.org\" ","References":"<20170905164221.11266-1-hdegoede@redhat.com>\n\t<20170905164221.11266-11-hdegoede@redhat.com>\n\t<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>\n\t\n\t\n\t\n\t\n\t\n\t<3d207dc4-1dba-ce34-21af-bab001ec68c5@axentia.se>","From":"Hans de Goede ","Message-ID":"<1aa88cc3-b2ab-2308-1039-c1b7729c313b@redhat.com>","Date":"Mon, 25 Sep 2017 13:35:31 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<3d207dc4-1dba-ce34-21af-bab001ec68c5@axentia.se>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]); Mon, 25 Sep 2017 11:35:37 +0000 (UTC)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1774743,"web_url":"http://patchwork.ozlabs.org/comment/1774743/","msgid":"<86782207-43e4-8cf3-775a-b3bf5a1946b9@axentia.se>","list_archive_url":null,"date":"2017-09-25T13:45:58","subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","submitter":{"id":66028,"url":"http://patchwork.ozlabs.org/api/people/66028/","name":"Peter Rosin","email":"peda@axentia.se"},"content":"On 2017-09-25 13:35, Hans de Goede wrote:\n> Hi,\n> \n> On 25-09-17 12:34, Peter Rosin wrote:\n>> On 2017-09-13 17:48, Hans de Goede wrote:\n>>> Hi,\n>>>\n>>> On 13-09-17 17:07, Rob Herring wrote:\n>>>> On Wed, Sep 13, 2017 at 9:06 AM, Hans de Goede wrote:\n>>>>> Hi,\n>>>>>\n>>>>>\n>>>>> On 13-09-17 15:38, Rob Herring wrote:\n>>>>>>\n>>>>>> On Wed, Sep 13, 2017 at 3:56 AM, Hans de Goede \n>>>>>> wrote:\n>>>>>>>\n>>>>>>> Hi,\n>>>>>>>\n>>>>>>>\n>>>>>>> On 13-09-17 00:20, Rob Herring wrote:\n>>>>>>>>\n>>>>>>>>\n>>>>>>>> On Tue, Sep 05, 2017 at 06:42:20PM +0200, Hans de Goede wrote:\n>>>>>>>>>\n>>>>>>>>>\n>>>>>>>>> Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()\n>>>>>>>>> to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.\n>>>>>>>>>\n>>>>>>>>> Also document the mux-names used by the generic tcpc_mux_dev code in\n>>>>>>>>> our devicetree bindings.\n>>>>>>>>>\n>>>>>>>>> Cc: Rob Herring \n>>>>>>>>> Cc: Mark Rutland \n>>>>>>>>> Cc: devicetree@vger.kernel.org\n>>>>>>>>> Signed-off-by: Hans de Goede \n>>>>>>>>> ---\n>>>>>>>>> Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++\n>>>>>>>>> drivers/staging/typec/fusb302/fusb302.c | 11\n>>>>>>>>> ++++++++++-\n>>>>>>>>> 2 files changed, 13 insertions(+), 1 deletion(-)\n>>>>>>>>>\n>>>>>>>>> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>>>> b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>>>> index 472facfa5a71..63d639eadacd 100644\n>>>>>>>>> --- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>>>> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>>>> @@ -6,6 +6,9 @@ Required properties :\n>>>>>>>>> - interrupts : Interrupt specifier\n>>>>>>>>> Optional properties :\n>>>>>>>>> +- mux-controls : List of mux-ctrl-specifiers containing 1 or\n>>>>>>>>> 2\n>>>>>>>>> muxes\n>>>>>>>>> +- mux-names : \"type-c-mode-mux\" when using 1 mux, or\n>>>>>>>>> + \"type-c-mode-mux\", \"usb-role-mux\" when\n>>>>>>>>> using\n>>>>>>>>> 2 muxes\n>>>>>>>>\n>>>>>>>>\n>>>>>>>>\n>>>>>>>> I'm not sure this is the right place for this. The mux is outside the\n>>>>>>>> FUSB302. In a type-C connector node or USB phy node would make more\n>>>>>>>> sense to me.\n>>>>>>>\n>>>>>>>\n>>>>>>>\n>>>>>>> The mux certainly does not belong in the USB phy node, it sits between\n>>>>>>> the\n>>>>>>> USB PHY\n>>>>>>> and the Type-C connector and can for example also mux the Type-C pins to\n>>>>>>> a\n>>>>>>> Display\n>>>>>>> Port PHY.\n>>>>>>\n>>>>>>\n>>>>>> Thinking about this some more, the mux(es) should be its own node(s).\n>>>>>> Then the question is where to put the nodes.\n>>>>>\n>>>>>\n>>>>> Right, the mux will be its own node per\n>>>>> Documentation/devicetree/bindings/mux/mux-controller.txt\n>>>>> the bindings bit this patch is adding is only adding phandles pointing\n>>>>> to that mux-node as the fusb302 \"consumes\" the mux functionality.\n>>>>>\n>>>>> So as such (the fusb302 is the component which should control the mux)\n>>>>> I do believe that the bindings this patch adds are correct.\n>>>>\n>>>> Humm, that's not how the mux binding works. The mux controller is what\n>>>> drives the mux select lines and is the provider. The consumer is the\n>>>> mux device itself. What decides the mux state is determined by what\n>>>> you are muxing, not which node has mux-controls property.\n>>>>\n>>>> By putting mux-controls in fusb302 node, you are saying fusb302 is a\n>>>> mux (or contains a mux).\n>>>>\n>>>>\n>>>>>>> As for putting it in a type-C connector node, currently we do not have\n>>>>>>> such\n>>>>>>> a node,\n>>>>>>\n>>>>>>\n>>>>>> Well, you should. Type-C connectors are certainly complicated enough\n>>>>>> that we'll need one. Plus we already require connector nodes for\n>>>>>> display outputs, so what do we do once you add display muxing?\n>>>>>\n>>>>>\n>>>>> An interesting question, I'm working on this for x86 + ACPI boards actually,\n>>>>> not a board using DT I've been adding DT bindings docs for device-properties\n>>>>> I use because that seems like the right thing to do where the binding is\n>>>>> obvious\n>>>>> (which I believe it is in this case as the fusb302 is the mux consumer) and\n>>>>> because the device-property code should work the same on x86 + ACPI\n>>>>> (where some platform-specific drivers attach the device properties) and\n>>>>> on e.g. ARM + DT.\n>>>>>\n>>>>> The rest should probably be left to be figured out when an actual DT\n>>>>> using device using the fusb302 or another Type-C controller shows up.\n>>>>\n>>>> Well this is a new one (maybe, I suppose others have sneaked by). If\n>>>> ACPI folks want to use DT bindings, then what do I care. But I have no\n>>>> interest in reviewing ACPI properties. The whole notion of sharing\n>>>> bindings between DT and ACPI beyond anything trivial is flawed IMO.\n>>>> The ptifalls have been discussed multiple times before, so I'm not\n>>>> going to repeat them here.\n>>>\n>>> Ok, so shall I just drop the Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>> part of this patch then ?\n>>\n>> I totally agree with the concern that Rob expressed about handling USB C\n>> muxes in a non-adhoc way. And this makes this series scary. I don't know\n>> enough details about USB C muxes and PD (I just have a very superficial\n>> mental model) to tell if this series is going down the right path. Or\n>> how terrible it will be to fix things up if not?\n>>\n>> The series extends the mux subsystem with muxes that pins semantics\n>> to specific states. That is new, and shows up exactly here when Rob is\n>> not happy about the binding. And if Rob does not want this in the\n>> DT bindings then I'm not so sure it is wise to do it at all? This\n>> problem doesn't go away just because you remove the binding. I think\n>> I would feel much better if there was a path forward on how to\n>> represent USB C muxes in DT and how that would fit with the driver\n>> structure.\n>>\n>> If you compare to the i2c-muxes, there is a relatively new i2c-mux-gpmux\n>> driver that uses some general purpose mux from the mux subsystem to\n>> implement an i2c-mux. If USB C muxes where to be done similarly, I'd\n>> imagine there should be a general abstraction of what USB C muxes provide\n>> somewhere outside of the mux subsystem, and a bunch of implementations\n>> of that abstraction. One of those implementations could be to use \"raw\"\n>> muxes from the mux subsystem. Of course, this is not what this series is\n>> doing.\n>>\n>> Also, muxes that are not general purpose such as the ones added to the\n>> mux subsystem by this series could perhaps be repurposed for some other\n>> application, but since the interface implemented does not really obey\n>> the rules (the provided mux controller interacts with different sets of\n>> signals depending on the state) this will not be possible.\n>>\n>> These issues are what has caused me to do a lot of thinking and to sit\n>> silent, sorry about that, but I would like input from someone more\n>> experienced. If possible. But I'm not sure where to turn?\n>>\n>> As a crazy example, why is it not possible to hook up one signal pair\n>> from the USB C mux, not to DP, but instead to some I2C controller? Then,\n>> if done right, i2c-mux-gpmux could be hooked up with the relevant mux\n>> controller and use the signal pair for I2C, with the mux controller\n>> acting as a gate. So, maybe a bit crazy, but something like that is how\n>> I think it should work from the mux subsystem point of view. And while\n>> maybe crazy and while it might not be technically possible to do I2C\n>> over a USB C connector for some reason, I do think that whatever\n>> abstraction you come up with for USB C muxes, it has to deal with and\n>> broker requests from both the USB subsystem and whatever other\n>> subsystems deals with the alt pairs. Be it graphics for DP signals, or\n>> whatever. IIUC, the alt signals need not be graphics, and it would be\n>> sad to implement the USB C mux it in a way that makes it hard to use\n>> the alt pairs for something else.\n>>\n>> [maybe my understanding of USB C is just wrong]\n> \n> So 2 things:\n> \n> 1) The Type-C subsys does actually abstract the mux outside of the\n> Type-C port-manager (TCPM) core in the form of a tcpc_mux_dev, so for\n> DT based platforms we could instantiate a tcpc_mux_dev from a node\n> which then represents the mux as usual.\n> \n> 2) What is very different from how the mux subsys currently is used,\n> is who is in control of the mux. Currently a subsys which wants to\n> route data through the mux selects the mux in the right mode, and\n> then deselects it when done. This assumes that the mux can mux\n> the data paths to the requested destination at all times, just not\n> to multiple destinations at once. With Type-C and any moment in\n> time, there really is only one correct setting as the mux is\n> connect to a Type-C device on the other side which typically\n> only has one config. So what happens with Type-C is that the TCPM\n> negotiates with the externally connected Type-C device, and then\n> sets the mux to the one and only correct setting for that device\n> to be fully functional.\n> \n> So your i2c example, for example, will not work with the normal\n> way where the i2c subsys asks the mux to mux a data-pair to the i2c\n> controller, as that only makes sense when your theoretical Type-C\n> device which talks i2c over a pair is connected externally, where\n> as when something else is connected then trying to talk i2c might\n> even damage the connected device. So with Type-C it is the TCPM\n> and only the TCPM which controls the mux.\n\nIf I get this correctly, some code (the TCPM?) negotiates with the\nother side over PD, and after that the way to set up the USB C mux\nis known and will not change until the cable is unplugged (unless\nwe go wild and renegotiate, but let's assume that will not happen). \nWhat I don't get is why do you want to squeeze what the TCPM(?) is\ndoing with its specialized mux into the existing mux subsystem\ninterface?\n\nI mean, the mux interface isn't really a perfect fit with its\nlocking and support for multiple consumers etc that really just gets\nin the way of setting a register in some chip to some value. Which\nis all that is really needed when you know that the TCPM is the only\none accessing the chip.\n\nAnd from the point of view of the mux subsystem, the USB C muxes\nlike the Pericom driver will be in a class of their own. Muxes of\nthat class can really only be used by one thing -- the TCPM code.\n\nSo, I don't see the benefit of doing this through the mux subsystem.\nMy hope is that the TCPM code is generic, and that by putting the\nUSB C mux inside the mux subsystem, you can keep the TCPM code\ngeneric? That might be good for the TCPM code, but it does create a\nnew class of devices in the mux subsystem, and opens the door for\nother classes later on.\n\nWhy not just add a function pointer that the generic TCPM code calls\nif it is set, and then add some code somewhere more local to the TCPM\ncode to back that call, and completely avoid the mux subsystem?\n\nCheers,\nPeter\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=axentiatech.onmicrosoft.com\n\theader.i=@axentiatech.onmicrosoft.com header.b=\"EyQjyUiN\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=peda@axentia.se; "],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y152s6lL5z9t5c\n\tfor ;\n\tMon, 25 Sep 2017 23:46:13 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S935676AbdIYNqM (ORCPT );\n\tMon, 25 Sep 2017 09:46:12 -0400","from mail-eopbgr10113.outbound.protection.outlook.com\n\t([40.107.1.113]:39552\n\t\"EHLO EUR02-HE1-obe.outbound.protection.outlook.com\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S934319AbdIYNqK (ORCPT );\n\tMon, 25 Sep 2017 09:46:10 -0400","from [192.168.13.3] (81.224.168.30) by\n\tAM5PR0202MB2547.eurprd02.prod.outlook.com (2603:10a6:203:6d::8) with\n\tMicrosoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.56.11;\n\tMon, 25 Sep 2017 13:46:03 +0000"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=axentiatech.onmicrosoft.com; s=selector1-axentia-se;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=USqSWuQGt8bfWBZH5DpL22vq2q0kl43latPcGj7ZvQg=;\n\tb=EyQjyUiNBwSrlEQmPV57mMqq8R9J8yXmx24hyywa2DlRCF8oeIYqGZ937HZCcvGd1fmnQx910Cdno8LlAs8U9fM48x4ZN3W52S2XHCLL2YIJiqEEHlZgBYSmjETO9fUeToZ0zinvu87Ejnp6aERnlYdHD/Q5MslbVi7zDSXNVFY=","Subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","To":"Hans de Goede , Rob Herring ","Cc":"MyungJoo Ham ,\n\tChanwoo Choi ,\n\tGuenter Roeck , \n\tHeikki Krogerus ,\n\tDarren Hart ,\n\tAndy Shevchenko , \n\tMathias Nyman ,\n\t\"linux-kernel@vger.kernel.org\" ,\n\tplatform-driver-x86@vger.kernel.org, devel@driverdev.osuosl.org,\n\tKuppuswamy Sathyanarayanan , \n\tSathyanarayanan Kuppuswamy Natarajan ,\n\tGreg Kroah-Hartman ,\n\tLinux USB List ,\n\tMark Rutland ,\n\t\"devicetree@vger.kernel.org\" ","References":"<20170905164221.11266-1-hdegoede@redhat.com>\n\t<20170905164221.11266-11-hdegoede@redhat.com>\n\t<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>\n\t\n\t\n\t\n\t\n\t\n\t<3d207dc4-1dba-ce34-21af-bab001ec68c5@axentia.se>\n\t<1aa88cc3-b2ab-2308-1039-c1b7729c313b@redhat.com>","From":"Peter Rosin ","Organization":"Axentia Technologies AB","Message-ID":"<86782207-43e4-8cf3-775a-b3bf5a1946b9@axentia.se>","Date":"Mon, 25 Sep 2017 15:45:58 +0200","User-Agent":"Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<1aa88cc3-b2ab-2308-1039-c1b7729c313b@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[81.224.168.30]","X-ClientProxiedBy":"DB6PR07CA0080.eurprd07.prod.outlook.com\n\t(2603:10a6:6:2b::18) To AM5PR0202MB2547.eurprd02.prod.outlook.com\n\t(2603:10a6:203:6d::8)","X-MS-PublicTrafficType":"Email","X-MS-Office365-Filtering-Correlation-Id":"bad7bc9f-a390-4671-b3a5-08d5041bc49e","X-Microsoft-Antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(22001)(2017030254152)(2017082002075)(201703131423075)(201702281549075);\n\tSRVR:AM5PR0202MB2547; ","X-Microsoft-Exchange-Diagnostics":["1; AM5PR0202MB2547;\n\t3:csm7kND6YtusMWbj1mrpUxWOf5eI13Jgj5wgdAUd/gXbOaZQ0jdoEDKPbWzdolbJLXJXdqwWJt1BjIbdZjfKCfvQq3niutSyYhOboGLm6jvt5DrkBjvozi7Lh6E3nZ7weysuy1vdNzlzrVvG/+JL+arElt21qNpHQ6llO1z1cGsBd/tkoaSnj89hq5m8cl+zExrVVzch1JaJh5RkIldzl3wkwQN3ZyEjNSvyvJt3uoPpq0ACRtRmHB2eoJQa6bNi;\n\t25:sjMU5o6Sj4mQheTO0z+u/nqE3X+tkalGn58qqWWQXjRb5jEpbNvKhTz48AvpIcB6wKopFsQyyrVPV+9dveluqI2vn1MazFfmK4wLW5FwBMwbVhpeFf3/e+EI96lpeNalCFKr+ed16cebS2AIS/H+nGANAUNbP1nQhmGr3EQ1K0kMSU8eHI8DVyWAP84QwX5HEx3qGrDOM6DNgnUwXjCp9EW6ze+/kyoGWX9EGOjwqBBYvEYh/tGX0gNTtUOawKxI7wZxCU/42dTrsFJ4ZxJ9SKe+lJCN0b4XnMpd8n8AOJ5bhESilqvFOPR5J+insMGITuDEzjXPEqryzHUj8WiCqg==;\n\t31:uyF9Cq4Zrnc+QgMosxCMe3oAjwgFUNGyXn+dlWLW56TUkWSnP1G4m7L8RGfWYpBp+za1rShvZ7sC+HjgVtflqX1kAfkaJzdk3U7ts3MPHezUknk44DY5zHQV98UJ0ZZYsGZfvmFh/6rpkWAf686gST8oqi9M8UySJike2zEwtS/oy05PNkn6WUYcpTecQYZ7ayM1XOHxzozmzpE0PClHFTQiau+BUia844ouQTxu8lA=","1; AM5PR0202MB2547;\n\t4:4rsbGa2vycYenI6n/zP+SaLobdLOzt7rJnyMPEaXfCrIlpmpv5/jPIkNBAiv8sTZygJ+rqX3i0emhz1Aw2kb5g/W0hOLz+lpWtimfHo8fEQ0NroHPXKpDElh8mXx1LLxTspsNRe8nwAgBU+wEJSSMQR4HE35/lYJcHYXaurW/mtdArGIijPkZ+TTuLikSD+FQT/r3k4YLCrZEk4UsSQT/Jgqq62+6C/c5i1C+as7yWWxVYofOd8Fckzr4tpHuhKNAjaToX9b22oNoaB8nLsw8/oBGgSTepscKIaAeM4NBsaJef1klWjXLE2fFasM4Yym/S8H1EuEXh3i9dAor1SsiGJFrn8tb7MzIOXBVBnkP1DVjp5TiCRAA+1UV27Yf0E7nvkPMsWsHewmmiGa7i19nQ==","=?utf-8?q?1=3BAM5PR0202MB2547=3B23=3ANN?=\n\t=?utf-8?q?wHvHE9RzI2ERcHGznmLSrUA1KHJz7ReGQZEoJ75pFsRur1MvvBxtJ8I9?=\n\t=?utf-8?q?+wn8xGxpqhw3JLytHrngj4/3gmQSbBn4agnnFky3ELFxxb7u13AGq2uS?=\n\t=?utf-8?q?jIXTTtUF3HO9p08AsaLFD7UoR4hMf4g0R+BSw5NCVuGHM4TJVd4iFuKk?=\n\t=?utf-8?q?L8OHmEDyijfBm28+y7z3wLxabSvXfJ+tSphvAnI/wk3TKk/N/UNaMDpD?=\n\t=?utf-8?q?G5RCN9uNX/vrHx3zIQnE7QknGDGduRhzIY85RMoP6rBVc0rFxKxlO+KV?=\n\t=?utf-8?q?G2X2EUYCaQwKe66/AVRzTXOHuzLykjDW8tftWLPRD8S5DLbtSIY/bQQA?=\n\t=?utf-8?q?2lWS4P8rQsaT4RQ8uZTYT0vC9HZUv9H+zbHHvAPqpuAvxPV6/fmxAzbW?=\n\t=?utf-8?q?GFkLNxjY6jDW5EdyfV1B5jWszb4tsqiZmpTShzWJP/+OQ0ydCXRQKqhu?=\n\t=?utf-8?q?nvBfpUWsaDNl0Y/+d/OCE/xYDskWgb6Cf0zbU4FldgPjZbwP20icC0Ew?=\n\t=?utf-8?q?uj3P98XUFss6ug2BByAAT5E30U0iI8U70hPvwB3DUWy5XGhnfYupmK1p?=\n\t=?utf-8?q?ecwGtKBlV4KJuat7yS6JflF1HQXJU58UWFmBqmDPjtEhIS2Iq8x7G14l?=\n\t=?utf-8?q?Z1cy61b9nctQ8WQe2El5xRuUAqavW/GM9UOLfnZ0j1mP6kvmI8DKNgYC?=\n\t=?utf-8?q?eBIzHg1w8TyZBypSf3Sr7LPxFViizHUAXavbOyM9bEelgActHzqC9Ros?=\n\t=?utf-8?q?pEwwAYXCi37DG0BbyOU/160zFac3C1+nil8vMJQRW9GoEnwiAd0jkPiZ?=\n\t=?utf-8?q?oOovLq+wwTiBt5g66s9jegQ0R3RzLLVFCfRZf7hLcV5RZjgeMf35o46h?=\n\t=?utf-8?q?epm8+18v1TAnporiiuLTrw6wMPsfda42s67ajMeZ0Q6sGm65llPhkNJj?=\n\t=?utf-8?q?T7G5ZhYosY9ZQJsUtB9wy5h32byUh79oK92QEJqyR/lx/9kDQSSP7lUV?=\n\t=?utf-8?q?7Wj1NiSHb7BAcUZDFGxy2crgzeMAZNOiDcz1qq9q5tM5E13GZgk9oTq7?=\n\t=?utf-8?q?dZReJu43r/VG172RKu17OrrNSR8oUeaUVM/wzgx3eeMVdseYTlC3nCjI?=\n\t=?utf-8?q?BtBXbPfcGLSiMACZ7lcTFDr0pFefpRfQuLxGEdAFvQtYK6oi9qnHTu/z?=\n\t=?utf-8?q?BE9ukxy6G9EcWcbg/96g4l+bfV6rpvzvocE8mHZ+fH2gMSDUukZRs/KA?=\n\t=?utf-8?q?rhSzqROzDdkoolAziKFTSy+wQoR/2x9wpl8Xh2jdmLHq5LwhC4aXSEIT?=\n\t=?utf-8?q?Aq4Gu9NbR/ZXnlyq/e7JLU7oRFB+8S1o6623MHpa+D7To239SY4hiB2X?=\n\t=?utf-8?q?Z+aPLP0GQqQM07GSzsxif2SiRenYRBG0Wg036uFSpsKeLKjJdfz7ZC9p?=\n\t=?utf-8?q?7wnV3wM1YgnaH234TytUAhNRBUHjeEkAcHHma49+a624edCLqS/M0Ro6?=\n\t=?utf-8?q?HqVazQ/25iYCebTriObNjYIcY6NcTfwNeZm9QyeyexiHReXyPfMDoUCY?=\n\t=?utf-8?q?w4PKs9FsVNyyC0KMiyY3Kf+AexzKsiWduN/HZtz45E0cRtSqP1+OATgK?=\n\t=?utf-8?q?xyc5fghCDgOhTN0K7ktNpf9W2vK9189F9+TrwRJfDYq94Rk5/xHg2B84?=\n\t=?utf-8?q?VyXT2Dq1UNXyq/MArX6Sr5PGJoAHt62KsLxNsTEYbuPIV54tHC7YXzTz?=\n\t=?utf-8?q?rGXs5qax3jtaVihgoHK9xurd0FI8aKnKp6UZvG?=","1; AM5PR0202MB2547;\n\t6:MBo8Al9R0lMGWm3U6qlyTMpKDIiABmjHW61iHPjwSxRYDHEPeitvADZd3Zo2739/BZtvuF7FBGSNt/H0RAqoJHuMvZrrEL/s/TQqh0r75EiGr+i/51Ocvkkyl3EOE6IlpRLko/cN8T+ImtbXITdr93zaDCGnwXtsLdEodovLO3F9vw2zBkUqF9dbIUO8ZtdOauTNDPripINw8p0/LGI5Mh02oKmLKAbUIAJXi8ELN1r3y0GvKLRA8A4Dcd5IAZmu8XPwWLLsDczfJIQ5Yx/vTi5BvItF6tr8mCwsnLnLVkeGGNxKX1ipdexBkRCifcvX1mATbvsjHEL46Lzyl/938A==;\n\t5:g+tcfrz8GPtZktO7ZEWGH2UNbZNflXeb8wePHiQjWjVrHCTyhNtdu8dCjeUmEdGlq8ddzEEql8hzlIKCgFTITSz8XhzfumNCEJv4gn7MrSL1W8QB3KACJVmgFS+iO3ILEGQuVmynp/yec6+Y+INDPQ==;\n\t24:svGXFOIH33qFrmQNPvALW7anKqvzO9qQo+maT2BbQ2fEnKiSs22eEh5/Rs8rGKr+oLU7HN6FkfuLSkcUuwWJeUoKc6eeb94McxvdIPqyy24=;\n\t7:Yd6jpR89bk6Pxy6tOlv9azKzdTLSM4vyR2PDYr6LU13YkrwSBGZ/jP38+Ko0bMgK2OnXY7MODzEWu8wNqK4FKbWSjEi6+V8gPJ1iydYPcXqmvKOpxJ7EE5KvCU9jf2KOsxz7aW9wNM38MexcTxhdqll3YY7DoE+rfz2tVo3TqBEuVhlBgyU2rgNjw8GI7q8saX6fFnd1v+6OgrvFe2saj+Bgzi8Fq7ptCKOeB5cyI8I="],"X-MS-TrafficTypeDiagnostic":"AM5PR0202MB2547:","X-Exchange-Antispam-Report-Test":"UriScan:(180628864354917)(9452136761055)(21532816269658)(17755550239193);","X-Microsoft-Antispam-PRVS":"","X-Exchange-Antispam-Report-CFA-Test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(3002001)(100000703101)(100105400095)(93006095)(93001095)(10201501046)(6041248)(20161123560025)(20161123562025)(2016111802025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123564025)(20161123558100)(20161123555025)(6043046)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:AM5PR0202MB2547; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:AM5PR0202MB2547; ","X-Forefront-PRVS":"04410E544A","X-Forefront-Antispam-Report":"SFV:NSPM;\n\tSFS:(10019020)(4630300001)(7370300001)(6049001)(6009001)(376002)(346002)(39830400002)(377454003)(377424004)(199003)(24454002)(189002)(2906002)(110136005)(66066001)(68736007)(3260700006)(36756003)(189998001)(5890100001)(117156002)(97736004)(478600001)(65956001)(31686004)(83506001)(6116002)(53936002)(31696002)(54906003)(6666003)(65806001)(7416002)(6486002)(305945005)(93886005)(6246003)(7736002)(53546010)(81166006)(101416001)(86362001)(229853002)(77096006)(4326008)(23676002)(25786009)(50986999)(47776003)(230700001)(54356999)(76176999)(3846002)(7350300001)(65826007)(33646002)(50466002)(8676002)(74482002)(64126003)(5660300001)(16576012)(105586002)(2950100002)(316002)(106356001)(58126008)(81156014)(52103002)(158003001)(42262002);\n\tDIR:OUT; SFP:1102; SCL:1; SRVR:AM5PR0202MB2547; H:[192.168.13.3]; FPR:;\n\tSPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; ","Received-SPF":"None (protection.outlook.com: axentia.se does not designate\n\tpermitted sender hosts)","SpamDiagnosticOutput":"1:99","SpamDiagnosticMetadata":"NSPM","X-OriginatorOrg":"axentia.se","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"25 Sep 2017 13:46:03.1218\n\t(UTC)","X-MS-Exchange-CrossTenant-FromEntityHeader":"Hosted","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"AM5PR0202MB2547","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1774757,"web_url":"http://patchwork.ozlabs.org/comment/1774757/","msgid":"<17213d9a-8aa0-abe8-436c-b34d6f383025@redhat.com>","list_archive_url":null,"date":"2017-09-25T14:17:31","subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","submitter":{"id":1893,"url":"http://patchwork.ozlabs.org/api/people/1893/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 25-09-17 15:45, Peter Rosin wrote:\n> On 2017-09-25 13:35, Hans de Goede wrote:\n>> Hi,\n>>\n>> On 25-09-17 12:34, Peter Rosin wrote:\n>>> On 2017-09-13 17:48, Hans de Goede wrote:\n>>>> Hi,\n>>>>\n>>>> On 13-09-17 17:07, Rob Herring wrote:\n>>>>> On Wed, Sep 13, 2017 at 9:06 AM, Hans de Goede wrote:\n>>>>>> Hi,\n>>>>>>\n>>>>>>\n>>>>>> On 13-09-17 15:38, Rob Herring wrote:\n>>>>>>>\n>>>>>>> On Wed, Sep 13, 2017 at 3:56 AM, Hans de Goede \n>>>>>>> wrote:\n>>>>>>>>\n>>>>>>>> Hi,\n>>>>>>>>\n>>>>>>>>\n>>>>>>>> On 13-09-17 00:20, Rob Herring wrote:\n>>>>>>>>>\n>>>>>>>>>\n>>>>>>>>> On Tue, Sep 05, 2017 at 06:42:20PM +0200, Hans de Goede wrote:\n>>>>>>>>>>\n>>>>>>>>>>\n>>>>>>>>>> Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()\n>>>>>>>>>> to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.\n>>>>>>>>>>\n>>>>>>>>>> Also document the mux-names used by the generic tcpc_mux_dev code in\n>>>>>>>>>> our devicetree bindings.\n>>>>>>>>>>\n>>>>>>>>>> Cc: Rob Herring \n>>>>>>>>>> Cc: Mark Rutland \n>>>>>>>>>> Cc: devicetree@vger.kernel.org\n>>>>>>>>>> Signed-off-by: Hans de Goede \n>>>>>>>>>> ---\n>>>>>>>>>> Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 3 +++\n>>>>>>>>>> drivers/staging/typec/fusb302/fusb302.c | 11\n>>>>>>>>>> ++++++++++-\n>>>>>>>>>> 2 files changed, 13 insertions(+), 1 deletion(-)\n>>>>>>>>>>\n>>>>>>>>>> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>>>>> b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>>>>> index 472facfa5a71..63d639eadacd 100644\n>>>>>>>>>> --- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>>>>> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>>>>>>>> @@ -6,6 +6,9 @@ Required properties :\n>>>>>>>>>> - interrupts : Interrupt specifier\n>>>>>>>>>> Optional properties :\n>>>>>>>>>> +- mux-controls : List of mux-ctrl-specifiers containing 1 or\n>>>>>>>>>> 2\n>>>>>>>>>> muxes\n>>>>>>>>>> +- mux-names : \"type-c-mode-mux\" when using 1 mux, or\n>>>>>>>>>> + \"type-c-mode-mux\", \"usb-role-mux\" when\n>>>>>>>>>> using\n>>>>>>>>>> 2 muxes\n>>>>>>>>>\n>>>>>>>>>\n>>>>>>>>>\n>>>>>>>>> I'm not sure this is the right place for this. The mux is outside the\n>>>>>>>>> FUSB302. In a type-C connector node or USB phy node would make more\n>>>>>>>>> sense to me.\n>>>>>>>>\n>>>>>>>>\n>>>>>>>>\n>>>>>>>> The mux certainly does not belong in the USB phy node, it sits between\n>>>>>>>> the\n>>>>>>>> USB PHY\n>>>>>>>> and the Type-C connector and can for example also mux the Type-C pins to\n>>>>>>>> a\n>>>>>>>> Display\n>>>>>>>> Port PHY.\n>>>>>>>\n>>>>>>>\n>>>>>>> Thinking about this some more, the mux(es) should be its own node(s).\n>>>>>>> Then the question is where to put the nodes.\n>>>>>>\n>>>>>>\n>>>>>> Right, the mux will be its own node per\n>>>>>> Documentation/devicetree/bindings/mux/mux-controller.txt\n>>>>>> the bindings bit this patch is adding is only adding phandles pointing\n>>>>>> to that mux-node as the fusb302 \"consumes\" the mux functionality.\n>>>>>>\n>>>>>> So as such (the fusb302 is the component which should control the mux)\n>>>>>> I do believe that the bindings this patch adds are correct.\n>>>>>\n>>>>> Humm, that's not how the mux binding works. The mux controller is what\n>>>>> drives the mux select lines and is the provider. The consumer is the\n>>>>> mux device itself. What decides the mux state is determined by what\n>>>>> you are muxing, not which node has mux-controls property.\n>>>>>\n>>>>> By putting mux-controls in fusb302 node, you are saying fusb302 is a\n>>>>> mux (or contains a mux).\n>>>>>\n>>>>>\n>>>>>>>> As for putting it in a type-C connector node, currently we do not have\n>>>>>>>> such\n>>>>>>>> a node,\n>>>>>>>\n>>>>>>>\n>>>>>>> Well, you should. Type-C connectors are certainly complicated enough\n>>>>>>> that we'll need one. Plus we already require connector nodes for\n>>>>>>> display outputs, so what do we do once you add display muxing?\n>>>>>>\n>>>>>>\n>>>>>> An interesting question, I'm working on this for x86 + ACPI boards actually,\n>>>>>> not a board using DT I've been adding DT bindings docs for device-properties\n>>>>>> I use because that seems like the right thing to do where the binding is\n>>>>>> obvious\n>>>>>> (which I believe it is in this case as the fusb302 is the mux consumer) and\n>>>>>> because the device-property code should work the same on x86 + ACPI\n>>>>>> (where some platform-specific drivers attach the device properties) and\n>>>>>> on e.g. ARM + DT.\n>>>>>>\n>>>>>> The rest should probably be left to be figured out when an actual DT\n>>>>>> using device using the fusb302 or another Type-C controller shows up.\n>>>>>\n>>>>> Well this is a new one (maybe, I suppose others have sneaked by). If\n>>>>> ACPI folks want to use DT bindings, then what do I care. But I have no\n>>>>> interest in reviewing ACPI properties. The whole notion of sharing\n>>>>> bindings between DT and ACPI beyond anything trivial is flawed IMO.\n>>>>> The ptifalls have been discussed multiple times before, so I'm not\n>>>>> going to repeat them here.\n>>>>\n>>>> Ok, so shall I just drop the Documentation/devicetree/bindings/usb/fcs,fusb302.txt\n>>>> part of this patch then ?\n>>>\n>>> I totally agree with the concern that Rob expressed about handling USB C\n>>> muxes in a non-adhoc way. And this makes this series scary. I don't know\n>>> enough details about USB C muxes and PD (I just have a very superficial\n>>> mental model) to tell if this series is going down the right path. Or\n>>> how terrible it will be to fix things up if not?\n>>>\n>>> The series extends the mux subsystem with muxes that pins semantics\n>>> to specific states. That is new, and shows up exactly here when Rob is\n>>> not happy about the binding. And if Rob does not want this in the\n>>> DT bindings then I'm not so sure it is wise to do it at all? This\n>>> problem doesn't go away just because you remove the binding. I think\n>>> I would feel much better if there was a path forward on how to\n>>> represent USB C muxes in DT and how that would fit with the driver\n>>> structure.\n>>>\n>>> If you compare to the i2c-muxes, there is a relatively new i2c-mux-gpmux\n>>> driver that uses some general purpose mux from the mux subsystem to\n>>> implement an i2c-mux. If USB C muxes where to be done similarly, I'd\n>>> imagine there should be a general abstraction of what USB C muxes provide\n>>> somewhere outside of the mux subsystem, and a bunch of implementations\n>>> of that abstraction. One of those implementations could be to use \"raw\"\n>>> muxes from the mux subsystem. Of course, this is not what this series is\n>>> doing.\n>>>\n>>> Also, muxes that are not general purpose such as the ones added to the\n>>> mux subsystem by this series could perhaps be repurposed for some other\n>>> application, but since the interface implemented does not really obey\n>>> the rules (the provided mux controller interacts with different sets of\n>>> signals depending on the state) this will not be possible.\n>>>\n>>> These issues are what has caused me to do a lot of thinking and to sit\n>>> silent, sorry about that, but I would like input from someone more\n>>> experienced. If possible. But I'm not sure where to turn?\n>>>\n>>> As a crazy example, why is it not possible to hook up one signal pair\n>>> from the USB C mux, not to DP, but instead to some I2C controller? Then,\n>>> if done right, i2c-mux-gpmux could be hooked up with the relevant mux\n>>> controller and use the signal pair for I2C, with the mux controller\n>>> acting as a gate. So, maybe a bit crazy, but something like that is how\n>>> I think it should work from the mux subsystem point of view. And while\n>>> maybe crazy and while it might not be technically possible to do I2C\n>>> over a USB C connector for some reason, I do think that whatever\n>>> abstraction you come up with for USB C muxes, it has to deal with and\n>>> broker requests from both the USB subsystem and whatever other\n>>> subsystems deals with the alt pairs. Be it graphics for DP signals, or\n>>> whatever. IIUC, the alt signals need not be graphics, and it would be\n>>> sad to implement the USB C mux it in a way that makes it hard to use\n>>> the alt pairs for something else.\n>>>\n>>> [maybe my understanding of USB C is just wrong]\n>>\n>> So 2 things:\n>>\n>> 1) The Type-C subsys does actually abstract the mux outside of the\n>> Type-C port-manager (TCPM) core in the form of a tcpc_mux_dev, so for\n>> DT based platforms we could instantiate a tcpc_mux_dev from a node\n>> which then represents the mux as usual.\n>>\n>> 2) What is very different from how the mux subsys currently is used,\n>> is who is in control of the mux. Currently a subsys which wants to\n>> route data through the mux selects the mux in the right mode, and\n>> then deselects it when done. This assumes that the mux can mux\n>> the data paths to the requested destination at all times, just not\n>> to multiple destinations at once. With Type-C and any moment in\n>> time, there really is only one correct setting as the mux is\n>> connect to a Type-C device on the other side which typically\n>> only has one config. So what happens with Type-C is that the TCPM\n>> negotiates with the externally connected Type-C device, and then\n>> sets the mux to the one and only correct setting for that device\n>> to be fully functional.\n>>\n>> So your i2c example, for example, will not work with the normal\n>> way where the i2c subsys asks the mux to mux a data-pair to the i2c\n>> controller, as that only makes sense when your theoretical Type-C\n>> device which talks i2c over a pair is connected externally, where\n>> as when something else is connected then trying to talk i2c might\n>> even damage the connected device. So with Type-C it is the TCPM\n>> and only the TCPM which controls the mux.\n> \n> If I get this correctly, some code (the TCPM?) negotiates with the\n> other side over PD, and after that the way to set up the USB C mux\n> is known and will not change until the cable is unplugged (unless\n> we go wild and renegotiate, but let's assume that will not happen).\n\nCorrect.\n\n> What I don't get is why do you want to squeeze what the TCPM(?) is\n> doing with its specialized mux into the existing mux subsystem\n> interface?\n\nThat is a good question, this really started with the micro-usb\notg stuff, where we have more or less the same problem in a much\nsimpler way:\n\n1) We have a cable plugged in which determined if we should mux the\nusb2 data lines on a micro-usb to an usb-host or an usb-device/gadget\ncontroller.\n\n2) Some way to detect the type of cable plugged in\n\n3) A hardware block or dedicated chip which is independent from 2,\nwhich needs to be controlled by the code implementing 2.\n\nSo we need some glue layer with an abstract (device independent)\ninterface between 2 and 3 so that we can plug random combinations\nof 2 and 3 together. In the beginning I tried using the extcon\nframework for that, but that is not really a good fit.\n\nThen someone else did some patches for the otg-mux found on the\nIntel Cherry Trail block where using the mux subsys for this and\nthat seemed like a good idea to me.\n\nAnd sofar I must say that from my pov the mux subsys does work\nquite well for this, but now that I better understand the initial\ndesign of the mux subsys I can understand that you are reluctant\nabout my use case.\n\n\n> I mean, the mux interface isn't really a perfect fit with its\n> locking and support for multiple consumers etc that really just gets\n> in the way of setting a register in some chip to some value. Which\n> is all that is really needed when you know that the TCPM is the only\n> one accessing the chip.\n> \n> And from the point of view of the mux subsystem, the USB C muxes\n> like the Pericom driver will be in a class of their own. Muxes of\n> that class can really only be used by one thing -- the TCPM code.\n> \n> So, I don't see the benefit of doing this through the mux subsystem.\n> My hope is that the TCPM code is generic, and that by putting the\n> USB C mux inside the mux subsystem, you can keep the TCPM code\n> generic?\n\nCorrect, the TCPM code is generic and for example the Cherry Trail\nSoC OTG mux needs to be controller by either:\n\nA) Something which is detecting the type of cable connected to a\nmicro-usb (either a PMIC or a gpio reading the ID pin) on devices\nwith a micro-usb connecter; or\n\nB) The TCPM code on devices with a Type-C connector.\n\nSo this is another example where the mux-subsys more or less is\na pretty good fit.\n\nAs for the locking, you're right that the locking is not necessary,\nbut the Type-C connector and its muxes have a clearly defined idle\nstate, so the deselect functionality also is a decent fit there.\n\n> That might be good for the TCPM code, but it does create a\n> new class of devices in the mux subsystem, and opens the door for\n> other classes later on.\n> \n> Why not just add a function pointer that the generic TCPM code calls\n> if it is set, and then add some code somewhere more local to the TCPM\n> code to back that call, and completely avoid the mux subsystem?\n\nSee the above example about 2 completely different ways how we\nneed to control the Cherry Trail SoC OTG mux. We really need some\nsort of abstraction layer here, and then some way to hook the TCPM\nand the Type-C and OTG-mux together, either through pnode handles\nin DT on DT platforms or through something like the lookup table\ncode one of my patches add on x86.\n\nFor me using the mux subsys for this works well. But as said I\ncan understand you being reluctant. Alternatively we could add a\nnew usb_mux subsys for this I guess.\n\nRegards,\n\nHans\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=hdegoede@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y15lN4RM2z9sRV\n\tfor ;\n\tTue, 26 Sep 2017 00:17:52 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S934451AbdIYORj (ORCPT );\n\tMon, 25 Sep 2017 10:17:39 -0400","from mx1.redhat.com ([209.132.183.28]:41300 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S933355AbdIYORh (ORCPT );\n\tMon, 25 Sep 2017 10:17:37 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id E210881DE5;\n\tMon, 25 Sep 2017 14:17:36 +0000 (UTC)","from shalem.localdomain (ovpn-117-212.ams2.redhat.com\n\t[10.36.117.212])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 5AFFE6F120;\n\tMon, 25 Sep 2017 14:17:33 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com E210881DE5","Subject":"Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support\n\tusing tcpc_gen_mux support","To":"Peter Rosin , Rob Herring ","Cc":"MyungJoo Ham ,\n\tChanwoo Choi ,\n\tGuenter Roeck , \n\tHeikki Krogerus ,\n\tDarren Hart ,\n\tAndy Shevchenko , \n\tMathias Nyman ,\n\t\"linux-kernel@vger.kernel.org\" ,\n\tplatform-driver-x86@vger.kernel.org, devel@driverdev.osuosl.org,\n\tKuppuswamy Sathyanarayanan , \n\tSathyanarayanan Kuppuswamy Natarajan ,\n\tGreg Kroah-Hartman ,\n\tLinux USB List ,\n\tMark Rutland ,\n\t\"devicetree@vger.kernel.org\" ","References":"<20170905164221.11266-1-hdegoede@redhat.com>\n\t<20170905164221.11266-11-hdegoede@redhat.com>\n\t<20170912222027.hzpejmmxkwbw6seu@rob-hp-laptop>\n\t\n\t\n\t\n\t\n\t\n\t<3d207dc4-1dba-ce34-21af-bab001ec68c5@axentia.se>\n\t<1aa88cc3-b2ab-2308-1039-c1b7729c313b@redhat.com>\n\t<86782207-43e4-8cf3-775a-b3bf5a1946b9@axentia.se>","From":"Hans de Goede ","Message-ID":"<17213d9a-8aa0-abe8-436c-b34d6f383025@redhat.com>","Date":"Mon, 25 Sep 2017 16:17:31 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<86782207-43e4-8cf3-775a-b3bf5a1946b9@axentia.se>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]); Mon, 25 Sep 2017 14:17:37 +0000 (UTC)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"devicetree@vger.kernel.org"}}]