Re: [PATCH v2 2/2] interconnect: qcom: Add Milos interconnect provider driver

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 7/8/25 2:42 PM, Luca Weiss wrote:
> On Tue Jul 8, 2025 at 1:30 PM CEST, Konrad Dybcio wrote:
>> On 7/8/25 12:20 PM, Luca Weiss wrote:
>>> Add driver for the Qualcomm interconnect buses found in Milos based
>>> platforms. The topology consists of several NoCs that are controlled by
>>> a remote processor that collects the aggregated bandwidth for each
>>> master-slave pairs.
>>>
>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>> ---
>>
>> [...]
>>
>>> +static struct qcom_icc_bcm * const aggre1_noc_bcms[] = {
>>> +};
>>
>> You can remove the empty bcm arrays and .(num_)bcms assignments
>> for them
> 
> Sure!
> 
>>
>> [...]
>>
>>> +static const struct of_device_id qnoc_of_match[] = {
>>> +	{ .compatible = "qcom,milos-aggre1-noc", .data = &milos_aggre1_noc},
>>> +	{ .compatible = "qcom,milos-aggre2-noc", .data = &milos_aggre2_noc},
>>> +	{ .compatible = "qcom,milos-clk-virt", .data = &milos_clk_virt},
>>> +	{ .compatible = "qcom,milos-cnoc-cfg", .data = &milos_cnoc_cfg},
>>> +	{ .compatible = "qcom,milos-cnoc-main", .data = &milos_cnoc_main},
>>> +	{ .compatible = "qcom,milos-gem-noc", .data = &milos_gem_noc},
>>> +	{ .compatible = "qcom,milos-lpass-ag-noc", .data = &milos_lpass_ag_noc},
>>> +	{ .compatible = "qcom,milos-mc-virt", .data = &milos_mc_virt},
>>> +	{ .compatible = "qcom,milos-mmss-noc", .data = &milos_mmss_noc},
>>> +	{ .compatible = "qcom,milos-nsp-noc", .data = &milos_nsp_noc},
>>> +	{ .compatible = "qcom,milos-pcie-anoc", .data = &milos_pcie_anoc},
>>> +	{ .compatible = "qcom,milos-system-noc", .data = &milos_system_noc},
>>> +	{ }
>>
>> a space before '}' would be neat
> 
> There is a space :)

Alright, before all the ones that clash with a letter ;)

> 
>>
>> Konrad
>>
>>> +};
>>> +MODULE_DEVICE_TABLE(of, qnoc_of_match);
>>> +
>>> +static struct platform_driver qnoc_driver = {
>>> +	.probe = qcom_icc_rpmh_probe,
>>> +	.remove = qcom_icc_rpmh_remove,
>>> +	.driver = {
>>> +		.name = "qnoc-milos",
>>> +		.of_match_table = qnoc_of_match,
>>> +		.sync_state = icc_sync_state,
>>
>> Are there any issues with sync_state? (hopefully not)
> 
> Don't think so, I don't see any sync_state pending warnings in dmesg so
> I assume it's 'synced'? Anything I should look out for in particular?

Boot failures.. stemming from not describing resources for peripherals
that use them. But if you're not seeing any, that's fabulous.

> 
> Also since it looks like I'll anyways send a v3, I've already ported the
> QoS settings, and don't think I'm seeing any issues booting up with
> that. So I'll include it with v3.

Great, thank you

Konrad