Re: [PATCH v3 5/6] clk: qcom: Add MSM8998 Multimedia Clock Controller (MMCC) driver

Jeffrey Hugo
2019-05-01T14:25:52Z

On 4/30/2019 9:43 PM, Bjorn Andersson wrote:
> On Tue 30 Apr 19:27 PDT 2019, Jeffrey Hugo wrote:
>> +static const struct of_device_id mmcc_msm8998_match_table[] = {
>> + { .compatible = "qcom,mmcc-msm8998" },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, mmcc_msm8998_match_table);
>> +
>> +static int mmcc_msm8998_probe(struct platform_device *pdev)
>> +{
>> + struct regmap *regmap;
>> +
>
> Don't you want to wait for "xo" here as well?

No, I don't want to. As far as I recall, Stephen would like to make a
clear divide between clock providers, and clock consumers. Since we
have the uart issue in gcc, and gcc is pretty critical to the entire
SoC, it seems like there is a reason (not sure I'd call it "good") to
wait for xo there.

Here, I'm less confident in the reasoning. mmcc is not really critical
to the SoC, and everything it services is "optional". If you have a
headless system with no display output, you won't even need it. On
system where there is a display, I expect the realistic driver ordering
to be that everything which consumes a mmcc clock to come up well after
xo is available.

In short, seems like a bit of a kludge to maybe avoid an issue which
doesn't seem like would happen.

>
>> + regmap = qcom_cc_map(pdev, &mmcc_msm8998_desc);
>> + if (IS_ERR(regmap))
>> + return PTR_ERR(regmap);
>> +
>> + return qcom_cc_really_probe(pdev, &mmcc_msm8998_desc, regmap);
>> +}
> [..]
>> +MODULE_DESCRIPTION("QCOM MMCC MSM8998 Driver");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS("platform:mmcc-msm8998");
>
> MODULE_DEVICE_TABLE() will provide the alias for module auto loading, so
> drop this.

Huh. I did not know that. Will put on the list to fixup.

>
> Regards,
> Bjorn
>

--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.