On 7/11/19 16:44, Bjorn Andersson wrote: > On Tue 25 Jun 09:47 PDT 2019, Jorge Ramirez-Ortiz wrote: > >> There is clock controller functionality in the APCS hardware block of >> qcs404 devices similar to msm8916. >> >> Co-developed-by: Niklas Cassel <niklas.cassel@xxxxxxxxxx> >> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxxxxx> >> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@xxxxxxxxxx> >> --- >> drivers/mailbox/qcom-apcs-ipc-mailbox.c | 18 ++++++++++-------- >> 1 file changed, 10 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c >> index 705e17a5479c..a05dc3aabac7 100644 >> --- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c >> +++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c >> @@ -89,16 +89,18 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev) >> return ret; >> } >> >> - if (of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global")) { >> - apcs->clk = platform_device_register_data(&pdev->dev, >> - "qcom-apcs-msm8916-clk", >> - -1, NULL, 0); >> - if (IS_ERR(apcs->clk)) >> - dev_err(&pdev->dev, "failed to register APCS clk\n"); >> - } >> - >> platform_set_drvdata(pdev, apcs); >> >> + if (!of_device_is_compatible(np, "qcom,msm8916-apcs-kpss-global") && >> + !of_device_is_compatible(np, "qcom,qcs404-apcs-apps-global")) > > If the remainder of the function was a long snippet I think this would > motivate the somewhat unusual early return. But I think it would be > cleaner to just add to the existing conditional. sure can do that. I dont agree (I wouldnt have bothered otherwise :)) but will do > > Regards, > Bjorn > >> + return 0; >> + >> + apcs->clk = platform_device_register_data(&pdev->dev, >> + "qcom-apcs-msm8916-clk", >> + -1, NULL, 0); >> + if (IS_ERR(apcs->clk)) >> + dev_err(&pdev->dev, "failed to register APCS clk\n"); >> + >> return 0; >> } >> >> -- >> 2.21.0 >> >