Received-SPF: pass (google.com: domain of linux-kernel+bounces-33023-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <1426b1e4-f42b-44e4-8d21-e675132866b3@wolfvision.net>
Date: Mon, 22 Jan 2024 13:35:30 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/3] ASoC: dt-bindings: xmos,xvf3500: add bindings for
 XMOS XVF3500
Content-Language: en-US
To: Takashi Iwai <tiwai@suse.de>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
 Rob Herring <robh+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, Liam Girdwood <lgirdwood@gmail.com>,
 Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
 Takashi Iwai <tiwai@suse.com>, Rob Herring <robh@kernel.org>,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
 linux-sound@vger.kernel.org, linux-usb@vger.kernel.org
References: <20240115-feature-xvf3500_driver-v1-0-ed9cfb48bb85@wolfvision.net>
 <20240115-feature-xvf3500_driver-v1-2-ed9cfb48bb85@wolfvision.net>
 <333c2986-c7c2-4a46-90cf-b59ae206e55a@linaro.org>
 <96abddcc-fa65-4f27-84fe-2281fe0fcf1c@wolfvision.net>
 <644f7f02-405d-47fb-bc72-4d54e897255f@linaro.org>
 <5db4b898-93d5-446f-bfed-b57847f9967a@wolfvision.net>
 <435f502c-1e1b-4d40-8dcc-34487905d69c@linaro.org>
 <b7f76546-9998-43e0-abff-a4e73817dbae@wolfvision.net>
 <47bdc31c-50d2-4d33-9339-5132b6364539@linaro.org>
 <16027339-0a82-4dd1-86aa-19fda6e23f88@wolfvision.net>
 <aeeb0dfb-87e2-4024-9d4a-0b9529477315@linaro.org>
 <b888d958-4eb0-4c1f-ace6-b2b85faa5113@wolfvision.net>
 <87ede9y3ue.wl-tiwai@suse.de>
From: Javier Carrasco <javier.carrasco@wolfvision.net>
In-Reply-To: <87ede9y3ue.wl-tiwai@suse.de>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?SWdaUG0rQm0rOFY4cERkbWR5T1ZrOS9Lb3lUUlNaM3pOeGdCTktISFlrSG0z?=
 =?utf-8?B?Zmo5QVgraC8ybml6a2ZpSWZXM1M4c0ZlaFA0cXdVbTVDVFc3Q05LNzVFdTVN?=
 =?utf-8?B?T0lhMVM2emFzQW8wUmlFQlJtL1l0WTlxOU5UWjltL1dHYURKVUNNZVlxcjNE?=
 =?utf-8?B?SVdkTXo1Qnh4N1ZRSVByQnFzTUF6aFFEbHVoZGJrczhHMXNwSWgzVEFMTU9s?=
 =?utf-8?B?ZDhkdUFGQlJsbm94TWdTeGFZY3MwYklsWFhnVUZOUFR3Y1RUaGJjcldtUFR2?=
 =?utf-8?B?Y1lPVnAweHdBTTg2VHM4aWhBQnVCVEtHdGkxNDloZUNWL1dTRldGd1RKK2hX?=
 =?utf-8?B?cXZZSkhEWnR1dXFMV0V1NlRtYmVndWRFaXFxUzhGVUVMUnJJMStsVFUrWW9n?=
 =?utf-8?B?dFIyNGxoQVVadWR1VlI3QkgwWU9lRjdZcGs1N2Y4VEQ1bTVxdUo0RVdRNmF1?=
 =?utf-8?B?TzZpamh4Vnd5SU5TVTgzWGtycEJpRDMzOFBxZ0pDQVdMaXYwMm44S0ZacTg4?=
 =?utf-8?B?UTVWWkR5NDNxTFRqYlhPMzcvREtFZW10dkRNMWJ3S2gvTm9wZkxwWUYzNDkv?=
 =?utf-8?B?VFV0S0MxYXVyakM4QzNLVVllSlNFbno2MTFYWXpWY3V0czhUZTZjL3JGdVd4?=
 =?utf-8?B?bmZFSC9PdFFudlBpUWF4WW1Db3lyTFo5QkR1Wlg2alJSVko3Z0NTUlJJcEEr?=
 =?utf-8?B?SnN1MXZGQzRjb2NSdGwyUGRHeThkVURnT2NPaC9ZNC9JWWcxK2ZkQWZKM1pE?=
 =?utf-8?B?aFVmdnVXdTZUR2pKUGhxZDFybVlYUERlbDVUUmZFbDhGcXVhaTRUb0N5T0lu?=
 =?utf-8?B?di9rWG85aFZYQXN5UVkxWGRlMm5vb3dTYVU3b0lQb0duT2o0MWZHQ3lFM09M?=
 =?utf-8?B?djhSai9udlVTOEVSZjhCeDlnTmhyRSsrRlZSS2hwT0FRbSt2a1ZqNFVTazdB?=
 =?utf-8?B?YTQrT2N4ZG5RaE4vOUI4OXJPTE1SZUFJdU94T0VLendmbXV1UnNjdkNSdjgv?=
 =?utf-8?B?b1c5SmpWRmtuSlNxUCt6Z0t2YTF5YW1SZlZteTMxZ0RWWUFxekRBYnZxdDI1?=
 =?utf-8?B?dlBaMW9yajNpSlZFSTZsMGtPOTFJRVJHNGdoUlRiSDk1NVZoNWxreUNKZ2VL?=
 =?utf-8?B?WVN6RmFQdlMwbG03VUdodEhaUHNFajBuMU13QUlBWmRNSlViY3lVUlZKckc4?=
 =?utf-8?B?aTQzYklRUjNzV1dIa2d2MVFJRnMxa0Q1RmYyMkNpSlBQYmJjakZXbk14RHZm?=
 =?utf-8?B?YjRBdzk1N1ova3RqOW5wWGhnRWtDbnFlaElWcmhZTXlnVi9vL0lYQUpOKzRB?=
 =?utf-8?B?K0VpdCszbXNYVDJJd0xqeHZOeTNFTWZ6WENQVUQzbDdDR3JWNEs3NktERWZx?=
 =?utf-8?B?K1UrNzR5TFBMK2dEOUwzMjk5cXF0dmRHdWVTN3JwQUg4NjFORzdwU1pVS3Y4?=
 =?utf-8?B?V01Ta3JyRXg0YlFUK1ZxWTJKakpqS0F2Vnc1VTBJd2RxaVhzS0oyUzBHUVpG?=
 =?utf-8?B?TjNWczlPK1RtTE5WMlgzTWxhMFRZRkZoUlZ0RmJ3SlZrcXFEcnYxU1pvMzVw?=
 =?utf-8?B?VGZvNitDa3NDQnpJTzZLUGphdE5NV2JYbzd3eTdUeDlFQWdGaDZUSDdEcFZr?=
 =?utf-8?B?bVBoVDBhR1dNQWwyRVhMaXNsbDhrSzZsOHI2dkoxQ0VDNTNoSGRsN0p5WGov?=
 =?utf-8?B?OWp4U2RtZCtBeXU0dzM1akNNWVBnUk00ZGt4TEltTUNJZXpFTEowODdLbXNS?=
 =?utf-8?B?OCtqbVVJUkRLWk9BR2lsZnpmRzlEeUN2aWxuMHgzZXU5OFJoL1dDUTd1M0w5?=
 =?utf-8?B?NUprQ1IvREN5anhSNW8xdDBZYUJJWU0ySEZLOWs0R1JlS05nNndxTkF3UHFo?=
 =?utf-8?B?clFrOVRZUWxMckhsNnRhejZGaEJva3ZWTGg1djVpWnY3Q25kcjhQTlhWaHBW?=
 =?utf-8?B?clhKOXVQeW5IRmZBUFlPc3NYVXB0RGxUdlRDSWJPbWVJTEFDUlVvTWxrQ3NX?=
 =?utf-8?B?NTJCYk1jT2RQV0VyaGhBd3BLZU1TY0x0VXlzeGxYR1M1dDhGZWd1UG1hNHQr?=
 =?utf-8?B?WmVQU1U0MTVHbFNiU3FNTUNIeGtqQ2VJL29icHhVYnhNMDhEY1l3RWluV21q?=
 =?utf-8?B?RFVWRHVkUnBSa2tLdE1KTFphUGtuWFdsV28xT1ZYdG5wREJrVTV4Rm93NXh6?=
 =?utf-8?B?Y1NzYXVMY1NoUXpJYVJvTUtLY0laTlM3eHVDVGhZcDRvNU9ybWJxbU1EOVdz?=
 =?utf-8?Q?vwzn5Z7rd6O2zzy7pVvTEcyOB39oE+X/f2puFD4nqI=3D?=
X-OriginatorOrg: wolfvision.net
X-MS-Exchange-CrossTenant-Network-Message-Id: 9d9882db-d512-41d5-8b6a-08dc1b46a015
X-MS-Exchange-CrossTenant-AuthSource: VE1PR08MB4974.eurprd08.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Jan 2024 12:35:32.8708
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: e94ec9da-9183-471e-83b3-51baa8eb804f
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: YNeF04Dn2ivHqu+uNjhDP7HwViMO1lHrxAw6zVoQlerHrNoVfScoXao3C6kGYMMkEH+izbdbq7uB5PLqRtT5ptZ+L8TPgxjLhqqFFBJ0Y1o=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS4PR08MB8094

On 22.01.24 13:01, Takashi Iwai wrote:
> On Tue, 16 Jan 2024 08:29:04 +0100,
> Javier Carrasco wrote:
>>
>> On 15.01.24 21:43, Krzysztof Kozlowski wrote:
>>> On 15/01/2024 20:43, Javier Carrasco wrote:
>>>> On 15.01.24 19:11, Krzysztof Kozlowski wrote:
>>>>> On 15/01/2024 17:24, Javier Carrasco wrote:
>>>>>> Do you mean that the XVF3500 should not be represented as a platform
>>>>>> device and instead it should turn into an USB device represented as a
>>>>>> node of an USB controller? Something like this (Rockchip SoC):
>>>>>>
>>>>>> &usb_host1_xhci {
>>>>>> 	...
>>>>>>
>>>>>> 	xvf3500 {
>>>>>> 		...
>>>>>> 	};
>>>>>> };
>>>>>>
>>>>>> Did I get you right or is that not the correct representation? Thank you
>>>>>> again.
>>>>>
>>>>> I believe it should be just like onboard hub. I don't understand why
>>>>> onboard hub was limited to hub, because other USB devices also could be
>>>>> designed similarly by hardware folks :/
>>>>>
>>>>> And if we talk about Linux drivers, then your current solution does not
>>>>> support suspend/resume and device unbind.
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>>
>>>>
>>>> Actually this series is an attempt to get rid of a misuse of the
>>>> onboard_usb_hub driver by a device that is not a HUB, but requires the
>>>> platform-part of that driver for the initialization.
>>>
>>> That's just naming issue, isn't it?
>>>
>>>>
>>>> What would be the best approach to provide support upstream? Should I
>>>> turn this driver into a generic USB driver that does what the
>>>> platform-part of the onboard HUB does? Or are we willing to accept
>>>
>>> No, because you did not solve the problems I mentioned. This is neither
>>> accurate hardware description nor proper Linux driver model handling PM
>>> and unbind.
>>>
>> You mentioned the PM handling twice, but I am not sure what you mean.
>> The driver provides callbacks for SIMPLE_DEV_PM_OPS, which I tested in
>> freeze and memory power states with positive results. On the other hand,
>> I suppose that you insisted for a good reason, so I would be grateful if
>> you could show me what I am doing wrong. The macro pattern was taken
>> from other devices under sound/, which also check CONFIG_PM_SLEEP,
>> but maybe I took a bad example or missed something.
> 
> FWIW, the patterns in sound/ are somewhat outdated and need to be
> refreshed.  Nowadays one should use DEFINE_SIMPLE_DEV_PM_OPS() instead
> (that should work without ifdef).
> 
> 
> thanks,
> 
> Takashi

Thank you for your feedback. I noticed that the pattern looks different,
but given that many devices in sound/ still use that pattern, I just
followed suit. In that case I will only use DEFINE_SIMPLE_DEV_PM_OPS.

Thanks again and best regards,
Javier Carrasco