Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <fed6bad9-bf9b-90b5-57fd-083401f6c990@quantatw.com>
Date:   Wed, 9 Feb 2022 11:34:23 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.0
Subject: Re: [PATCH v2 2/2] usb: typec: fusb302: add support of
 supported_pd_rev
Content-Language: en-US
To:     Guenter Roeck <linux@roeck-us.net>,
        Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        Patrick Williams <patrick@stwcx.xyz>
References: <20220208082026.4855-1-potin.lai@quantatw.com>
 <20220208112226.9108-1-potin.lai@quantatw.com>
 <20220208112226.9108-3-potin.lai@quantatw.com>
 <1acc1733-da53-6255-5cad-15f79850f44f@roeck-us.net>
From:   Potin Lai <potin.lai@quantatw.com>
In-Reply-To: <1acc1733-da53-6255-5cad-15f79850f44f@roeck-us.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZHN3L0JDa0pwZUZaY2V3a3BoWE8wYW5GOUdmM3NGRGxBZ1Q3M3ZEZmlENUtJ?=
 =?utf-8?B?OS9nT20zVmxzUXJVaG4yb2RrZG9jSkpzdlY1RGxLanZMb1dyalhDSVA5T3hx?=
 =?utf-8?B?ci9YRzRGV001QVU2WEN0VVl6TVpzcmNaaVJERjRaUktlZVFkZXlhTTI5ZU5P?=
 =?utf-8?B?aTNBbk5kQWJSSHIxVUVpai96YU9ET0VobHhNQk1sbGp5b3krRGh3ajZTa2V0?=
 =?utf-8?B?ZFRWcGwxUEJFNytPM3lqUGJEbkpTOVlSMURUTXcrRFE1c3FYUTVha3NBSHhJ?=
 =?utf-8?B?YnRvaDF6YSt2T2RhUEVIYnJDK2NKMVJHaVcybHIrTGFwbzVna2JpWE1EKzNX?=
 =?utf-8?B?SVgwa0l6LzljTEtZL05QS0ZHbWZKUGpwamZTZ2tUMUFoMERTTFJUUzZvbTFk?=
 =?utf-8?B?eHFUNjNpNG9RV0VFU2ZLalpxUzQrVXVFVDUrcW9TLzM1Ym0weitxTEg3Z2Vs?=
 =?utf-8?B?YjNlZllIQnlXcm9VTmhiMVJJWGp0OStFMFVSQWFYQnJiZ3ZHemQ5TGF6SWtH?=
 =?utf-8?B?WlE2aUxSemMwb096VDVIaE8yZjlreUpkNm5lRTFON0puYzR1SUZJSzlMUHlD?=
 =?utf-8?B?MDQzRUZMc1JuRzVUQWQwdGV2UWgxcDdwWVNEbkxzME9tb0c0a0Z6SnJxc1Nz?=
 =?utf-8?B?K0pTaFd4cjA1ejNraGhFMGR4Z0VQUy9OWUJxbzhpbDIraFIxYytlSVAxRnVo?=
 =?utf-8?B?Vm5VZFR0M0xUcGlYNHVNejFkV05sb3BodkRUcGV6VTRwY3kyVk43TEpnT0FU?=
 =?utf-8?B?RXY2RSs1a0EzTGp5aGROb3FOWExaWEgwbkwwSGZrQnlubjVYVCtzQnZPakpr?=
 =?utf-8?B?QmcwUit6QSsyRTd1bURvV2pVQWFqTGxEVmVITHRvWTV0alc1dUpkVTU5amhk?=
 =?utf-8?B?MGxWcmU4QTd0cG5PKzgyZU02dFg5eThITXFCVS9rUmd1S09sSHlwRWlNcWNH?=
 =?utf-8?B?TXRnOUhKLzM0bi9rQ0d1cmpNZXRlYkwrV3REajd4elZmZDV4TjBpNytDVldG?=
 =?utf-8?B?SUl3Vm5palBDUEJWZVpMd3BwWTJvek5BY0JtTlFEa25TbHlqUHlYY2RLVFcv?=
 =?utf-8?B?YkduYkMrMTRHRE9DRlJLaStZTVQ4TzUySHZDUTJUNWFmT3J6OXAwM05JZHI3?=
 =?utf-8?B?TXpWR3dZdXpxNTFodlQrVlg5L3NOTlVxNnN5ZVRjT1g1a3pJQk5uYXBlQmRD?=
 =?utf-8?B?SXViQ1d6SERSWXB2eWFlbDBobG1PNENiaUtJM2lCdXVEQWRLYWtUbVVzVHV6?=
 =?utf-8?B?emlwc0h3Q3ZVSDdpUzFWeGs2LzUvUGJJTHRXTjFyY1RtYmtqUElablFLN3dv?=
 =?utf-8?B?NEY0WlJHN0xzcVFsL1JIclVWRGd0REh6UjV3alF5bHkyYmlEeE1uMFcyYXpw?=
 =?utf-8?B?WjY2WXhleTBBL09xM2dnS24vRUZNM20xV0tDSkdoc3RNaXlOWmRPVlAxMjhJ?=
 =?utf-8?B?MEtDSkFIUVVXcGd5Yi9XQ29UL1E2R2Vkc0xCcmkwU3Y5dnBCdFV0YlhPZ1Fa?=
 =?utf-8?B?WW0yNS9BbVdXUFFxeTdJUFFWWFVhZmlqTFBaYVg4dnVFWEprRFNFRGZVS2Zt?=
 =?utf-8?B?MzBldUNGSmVJMFhyWm44UjNBSkdLL0swem83R2R3SDJTTFhrRGdqaGNnL015?=
 =?utf-8?B?d3BLdVQ4Y2hNeEhUNmNJN3M5UjVEaDhLVm5RVnRyRGp5SjdEZzNJRFNxY2cy?=
 =?utf-8?B?WUkxZHBjU3pyQWprZTcveTIvemVEc1JTa1A0TDR0b1VCQ05WOERvanJBOFFv?=
 =?utf-8?B?M0d4aUtvWXE0bFdtWkhPZWRvWFZXb2lYYTRiTUlIbllHN1paRVBiRSs3bEVI?=
 =?utf-8?B?MWZLNzFlRmhwQytQR1U2d0MydnRjVnVtbW50VDhMV0hrWXY3NEtBY0FYaG9G?=
 =?utf-8?B?ekpxZzhqbzlvUmlYcDVzbUtSUXh6YVJXd3Q3NjBsaUpIWGwxcFMzQ29sM1Y3?=
 =?utf-8?B?bkVkeG5HUjJ5MGRISmUzd28wbU5NRE96WG5yWXU5VEF3THBMVmN3Tktma0x4?=
 =?utf-8?B?aFlkTDFsOFJrZEhhcUdsNXJqd2hBQ25yYmU3RDZoR09xUm1ZdXBLSEViaXRj?=
 =?utf-8?B?QlN2dFpkUzJ6U3k0MnN0QXV3MkE1Q2FrYjVNSkFseGhwdElCNUdpMDU4clJP?=
 =?utf-8?B?NHJNd2t4dTBCdmN1YlRqSTR5SFZYSzBzZkNrMVVsYUl0MHVYUW1qM1p1eUVY?=
 =?utf-8?Q?Ut1nZSgE9RH5FS5wkTHEeoY=3D?=
X-OriginatorOrg: quantatw.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1d4cdf85-9be4-4bd2-7d78-08d9eb7d117b
X-MS-Exchange-CrossTenant-AuthSource: HK0PR04MB3282.apcprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2022 03:34:24.8099
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 179b0327-07fc-4973-ac73-8de7313561b2
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: XMEg2uwb96jrQbLnk51Rja8BCLoeYIgaTHcJMput0z5LX8hz0Kpoqz9LrjUn0/P1dseBaukTaIa7KKEG6Unupg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: HK0PR04MB3025
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE autolearn=no
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


Guenter Roeck 於 2022/2/8 下午 11:22 寫道:
> On 2/8/22 03:22, Potin Lai wrote:
>> Add support for passing supported PD rev to TCPM.
>> If "supported-pd-rev" property exist, then return supported_pd_rev as
>> defined value in DTS, otherwise return PD_MAX_REV
>>
>> Example of DTS:
>>
>> fusb302: typec-portc@22 {
>>      compatible = "fcs,fusb302";
>>      reg = <0x22>;
>>      ...
>>      supported-pd-rev=<1>; // PD_REV20
>>      ...
>> };
>>
>
> The new property needs to be documented. However, I am not entirely sure
> I understand why it is needed. Wouldn't the supported PD revision
> be a chip (fusb302) specific constant ? If so, why does it need a
> devicetree property ? I think that needs some additional explanation.
>
> Thanks,
> Guenter
>

My initially intend was adding flexibility for developer to decided 
which PD revision
they want to use for negotiation.

I saw your reply in another patch,  I agree with you, it will be simple 
and consistent if
move "supported-pd-rev" to tcpm fwnode as other capabilities.

Just want to double confirm, is "usb-connector.yaml" right place to put 
documentation
if adding "supported-pd-rev" in fwnode?

Thanks,
Potin

>> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
>> ---
>>   drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/usb/typec/tcpm/fusb302.c 
>> b/drivers/usb/typec/tcpm/fusb302.c
>> index 72f9001b0792..8cff92d58b96 100644
>> --- a/drivers/usb/typec/tcpm/fusb302.c
>> +++ b/drivers/usb/typec/tcpm/fusb302.c
>> @@ -109,6 +109,9 @@ struct fusb302_chip {
>>       enum typec_cc_status cc2;
>>       u32 snk_pdo[PDO_MAX_OBJECTS];
>>   +    /* supported pd rev */
>> +    u32 supported_pd_rev;
>> +
>>   #ifdef CONFIG_DEBUG_FS
>>       struct dentry *dentry;
>>       /* lock for log buffer access */
>> @@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev 
>> *dev, enum tcpm_transmit_type type,
>>       return ret;
>>   }
>>   +static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev)
>> +{
>> +    struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>> +                         tcpc_dev);
>> +    return chip->supported_pd_rev;
>> +}
>> +
>>   static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl)
>>   {
>>       if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX)
>> @@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev 
>> *fusb302_tcpc_dev)
>>       fusb302_tcpc_dev->set_roles = tcpm_set_roles;
>>       fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
>>       fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
>> +    fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev;
>>   }
>>     static const char * const cc_polarity_name[] = {
>> @@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client 
>> *client,
>>       struct fusb302_chip *chip;
>>       struct i2c_adapter *adapter = client->adapter;
>>       struct device *dev = &client->dev;
>> +    struct device_node *np = dev->of_node;
>>       const char *name;
>>       int ret = 0;
>>   @@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client 
>> *client,
>>           dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", 
>> ret);
>>           goto tcpm_unregister_port;
>>       }
>> +
>> +    if (of_property_read_u32(np, "supported-pd-rev",
>> +                &chip->supported_pd_rev) < 0) {
>> +        chip->supported_pd_rev = PD_MAX_REV;
>> +    } else if (chip->supported_pd_rev > PD_MAX_REV) {
>> +        chip->supported_pd_rev = PD_MAX_REV;
>> +    }
>
> The else part is also checked in the tcpm code and thus seems
> to be redundant.
>
>> +
>>       enable_irq_wake(chip->gpio_int_n_irq);
>>       i2c_set_clientdata(client, chip);
>