From: Nikunj Kela <quic_nkela@quicinc.com> To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, <sudeep.holla@arm.com> Cc: <cristian.marussi@arm.com>, <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>, <agross@kernel.org>, <andersson@kernel.org>, <konrad.dybcio@linaro.org>, <linux-arm-kernel@lists.infradead.org>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org> Subject: Re: [PATCH 2/2] firmware: arm_scmi: Add qcom hvc/shmem transport Date: Tue, 18 Jul 2023 14:16:04 -0700 [thread overview] Message-ID: <e8399fcf-e0d8-cc31-d9a7-b0f4f7cc3e71@quicinc.com> (raw) In-Reply-To: <ec3d7769-8a5f-d938-7f77-351ddfe6fb45@linaro.org> On 7/18/2023 11:42 AM, Krzysztof Kozlowski wrote: > On 18/07/2023 20:25, Nikunj Kela wrote: >>>> + >>>> + scmi_info = devm_kzalloc(dev, sizeof(*scmi_info), GFP_KERNEL); >>>> + if (!scmi_info) >>>> + return -ENOMEM; >>>> + >>>> + np = of_parse_phandle(cdev->of_node, "shmem", 0); >>>> + if (!of_device_is_compatible(np, "arm,scmi-shmem")) >>> You leak here reference. >> Wouldn't the devm_* API take care of that implicitly? It is same in >> smc.c as well. > Thanks for bringing my attention to this. I sent a fix for smc.c. Fix > your patch as well, please. Thanks, I thought you were referring to kzalloc cleanup. Will include this fix. BTW, you may need to fix mailbox.c as well. > >>>> + return -ENXIO; >>>> + >>>> + ret = of_address_to_resource(np, 0, &res); >>>> + of_node_put(np); >>>> + if (ret) { >>>> + dev_err(cdev, "failed to get SCMI Tx shared memory\n"); >>>> + return ret; >>>> + } >>>> + >>>> + size = resource_size(&res); >>>> + >>>> + /* let's map 2 additional ulong since >>>> + * func-id & capability-id are kept after shmem. >>>> + * +-------+ >>>> + * | | >>>> + * | shmem | >>>> + * | | >>>> + * | | >>>> + * +-------+ <-- size >>>> + * | funcId| >>>> + * +-------+ <-- size + sizeof(ulong) >>>> + * | capId | >>>> + * +-------+ <-- size + 2*sizeof(ulong) >>>> + */ >>>> + >>>> + scmi_info->shmem = devm_ioremap(dev, res.start, >>>> + size + 2 * sizeof(unsigned long)); >>>> + if (!scmi_info->shmem) { >>>> + dev_err(dev, "failed to ioremap SCMI Tx shared memory\n"); >>>> + return -EADDRNOTAVAIL; >>>> + } >>>> + >>>> + func_id = readl((void *)(scmi_info->shmem) + size); >>>> + >>>> +#ifdef CONFIG_ARM64 >>>> + cap_id = readq((void *)(scmi_info->shmem) + size + >>>> + sizeof(unsigned long)); >>>> +#else >>>> + cap_id = readl((void *)(scmi_info->shmem) + size + >>>> + sizeof(unsigned long)); >>>> +#endif >>>> + >>>> + /* >>>> + * If there is an interrupt named "a2p", then the service and >>>> + * completion of a message is signaled by an interrupt rather than by >>>> + * the return of the hvc call. >>>> + */ >>>> + irq = of_irq_get_byname(cdev->of_node, "a2p"); >>>> + if (irq > 0) { >>>> + ret = devm_request_irq(dev, irq, qcom_hvc_msg_done_isr, >>>> + IRQF_NO_SUSPEND, >>>> + dev_name(dev), scmi_info); >>>> + if (ret) { >>>> + dev_err(dev, "failed to setup SCMI completion irq\n"); >>> return dev_err_probe, unless this is not called in probe... but then >>> using devm-interface raises questions. >> This is copied as is from existing smc.c > I understand and I hope you understand the code you copied. If there is > a bug in existing code, please do not copy it to new code (like leaking > OF node reference). > > Best regards, > Krzysztof >
WARNING: multiple messages have this Message-ID (diff)
From: Nikunj Kela <quic_nkela@quicinc.com> To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, <sudeep.holla@arm.com> Cc: <cristian.marussi@arm.com>, <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>, <agross@kernel.org>, <andersson@kernel.org>, <konrad.dybcio@linaro.org>, <linux-arm-kernel@lists.infradead.org>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org> Subject: Re: [PATCH 2/2] firmware: arm_scmi: Add qcom hvc/shmem transport Date: Tue, 18 Jul 2023 14:16:04 -0700 [thread overview] Message-ID: <e8399fcf-e0d8-cc31-d9a7-b0f4f7cc3e71@quicinc.com> (raw) In-Reply-To: <ec3d7769-8a5f-d938-7f77-351ddfe6fb45@linaro.org> On 7/18/2023 11:42 AM, Krzysztof Kozlowski wrote: > On 18/07/2023 20:25, Nikunj Kela wrote: >>>> + >>>> + scmi_info = devm_kzalloc(dev, sizeof(*scmi_info), GFP_KERNEL); >>>> + if (!scmi_info) >>>> + return -ENOMEM; >>>> + >>>> + np = of_parse_phandle(cdev->of_node, "shmem", 0); >>>> + if (!of_device_is_compatible(np, "arm,scmi-shmem")) >>> You leak here reference. >> Wouldn't the devm_* API take care of that implicitly? It is same in >> smc.c as well. > Thanks for bringing my attention to this. I sent a fix for smc.c. Fix > your patch as well, please. Thanks, I thought you were referring to kzalloc cleanup. Will include this fix. BTW, you may need to fix mailbox.c as well. > >>>> + return -ENXIO; >>>> + >>>> + ret = of_address_to_resource(np, 0, &res); >>>> + of_node_put(np); >>>> + if (ret) { >>>> + dev_err(cdev, "failed to get SCMI Tx shared memory\n"); >>>> + return ret; >>>> + } >>>> + >>>> + size = resource_size(&res); >>>> + >>>> + /* let's map 2 additional ulong since >>>> + * func-id & capability-id are kept after shmem. >>>> + * +-------+ >>>> + * | | >>>> + * | shmem | >>>> + * | | >>>> + * | | >>>> + * +-------+ <-- size >>>> + * | funcId| >>>> + * +-------+ <-- size + sizeof(ulong) >>>> + * | capId | >>>> + * +-------+ <-- size + 2*sizeof(ulong) >>>> + */ >>>> + >>>> + scmi_info->shmem = devm_ioremap(dev, res.start, >>>> + size + 2 * sizeof(unsigned long)); >>>> + if (!scmi_info->shmem) { >>>> + dev_err(dev, "failed to ioremap SCMI Tx shared memory\n"); >>>> + return -EADDRNOTAVAIL; >>>> + } >>>> + >>>> + func_id = readl((void *)(scmi_info->shmem) + size); >>>> + >>>> +#ifdef CONFIG_ARM64 >>>> + cap_id = readq((void *)(scmi_info->shmem) + size + >>>> + sizeof(unsigned long)); >>>> +#else >>>> + cap_id = readl((void *)(scmi_info->shmem) + size + >>>> + sizeof(unsigned long)); >>>> +#endif >>>> + >>>> + /* >>>> + * If there is an interrupt named "a2p", then the service and >>>> + * completion of a message is signaled by an interrupt rather than by >>>> + * the return of the hvc call. >>>> + */ >>>> + irq = of_irq_get_byname(cdev->of_node, "a2p"); >>>> + if (irq > 0) { >>>> + ret = devm_request_irq(dev, irq, qcom_hvc_msg_done_isr, >>>> + IRQF_NO_SUSPEND, >>>> + dev_name(dev), scmi_info); >>>> + if (ret) { >>>> + dev_err(dev, "failed to setup SCMI completion irq\n"); >>> return dev_err_probe, unless this is not called in probe... but then >>> using devm-interface raises questions. >> This is copied as is from existing smc.c > I understand and I hope you understand the code you copied. If there is > a bug in existing code, please do not copy it to new code (like leaking > OF node reference). > > Best regards, > Krzysztof > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-07-18 21:16 UTC|newest] Thread overview: 186+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-07-18 16:08 [PATCH 0/2] Add qcom hvc/shmem transport Nikunj Kela 2023-07-18 16:08 ` Nikunj Kela 2023-07-18 16:08 ` [PATCH 1/2] dt-bindings: arm: Add qcom specific hvc transport for SCMI Nikunj Kela 2023-07-18 16:08 ` Nikunj Kela 2023-07-18 17:21 ` Rob Herring 2023-07-18 17:21 ` Rob Herring 2023-07-18 18:12 ` Krzysztof Kozlowski 2023-07-18 18:12 ` Krzysztof Kozlowski 2023-07-18 18:18 ` Nikunj Kela 2023-07-18 18:18 ` Nikunj Kela 2023-07-19 10:39 ` Sudeep Holla 2023-07-19 10:39 ` Sudeep Holla 2023-07-19 13:58 ` Nikunj Kela 2023-07-19 13:58 ` Nikunj Kela 2023-07-18 16:08 ` [PATCH 2/2] firmware: arm_scmi: Add qcom hvc/shmem transport Nikunj Kela 2023-07-18 16:08 ` Nikunj Kela 2023-07-18 18:17 ` Krzysztof Kozlowski 2023-07-18 18:17 ` Krzysztof Kozlowski 2023-07-18 18:25 ` Nikunj Kela 2023-07-18 18:25 ` Nikunj Kela 2023-07-18 18:42 ` Krzysztof Kozlowski 2023-07-18 18:42 ` Krzysztof Kozlowski 2023-07-18 21:16 ` Nikunj Kela [this message] 2023-07-18 21:16 ` Nikunj Kela 2023-07-19 6:15 ` Krzysztof Kozlowski 2023-07-19 6:15 ` Krzysztof Kozlowski 2023-07-18 18:29 ` Bjorn Andersson 2023-07-18 18:29 ` Bjorn Andersson 2023-07-18 18:53 ` Nikunj Kela 2023-07-18 18:53 ` Nikunj Kela 2023-07-18 19:07 ` Bjorn Andersson 2023-07-18 19:07 ` Bjorn Andersson 2023-07-18 19:10 ` Nikunj Kela 2023-07-18 19:10 ` Nikunj Kela 2023-07-18 19:30 ` Bjorn Andersson 2023-07-18 19:30 ` Bjorn Andersson 2023-07-18 22:05 ` Nikunj Kela 2023-07-18 22:05 ` Nikunj Kela 2023-07-19 10:55 ` Cristian Marussi 2023-07-19 10:55 ` Cristian Marussi 2023-07-19 14:02 ` Nikunj Kela 2023-07-19 14:02 ` Nikunj Kela 2023-07-23 2:15 ` kernel test robot 2023-07-23 2:15 ` kernel test robot 2023-07-24 16:44 ` [PATCH v2 0/3] " Nikunj Kela 2023-07-24 16:44 ` [PATCH v2 1/3] dt-bindings: arm: convert nested if-else construct to allOf Nikunj Kela 2023-07-25 6:01 ` Krzysztof Kozlowski 2023-07-24 16:44 ` [PATCH v2 2/3] dt-bindings: arm: Add qcom specific hvc transport for SCMI Nikunj Kela 2023-07-25 6:06 ` Krzysztof Kozlowski 2023-07-24 16:44 ` [PATCH v2 3/3] firmware: arm_scmi: Add qcom hvc/shmem transport Nikunj Kela 2023-07-25 17:03 ` Cristian Marussi 2023-07-25 17:12 ` Nikunj Kela 2023-07-31 14:04 ` Nikunj Kela 2023-07-31 14:04 ` Nikunj Kela 2023-08-01 7:27 ` kernel test robot 2023-08-01 7:27 ` kernel test robot 2023-08-11 17:57 ` [PATCH v3 0/3] " Nikunj Kela 2023-08-11 17:57 ` Nikunj Kela 2023-08-11 17:57 ` [PATCH v3 1/3] dt-bindings: arm: convert nested if-else construct to allOf Nikunj Kela 2023-08-11 17:57 ` Nikunj Kela 2023-08-11 17:57 ` [PATCH v3 2/3] dt-bindings: arm: Add qcom specific hvc transport for SCMI Nikunj Kela 2023-08-11 17:57 ` Nikunj Kela 2023-08-11 17:57 ` [PATCH v3 3/3] firmware: arm_scmi: Add qcom hvc/shmem transport Nikunj Kela 2023-08-11 17:57 ` Nikunj Kela 2023-09-05 16:06 ` [PATCH v3 0/3] " Nikunj Kela 2023-09-05 16:06 ` Nikunj Kela 2023-09-05 16:37 ` Krzysztof Kozlowski 2023-09-05 16:37 ` Krzysztof Kozlowski 2023-09-07 10:36 ` Sudeep Holla 2023-09-07 10:36 ` Sudeep Holla 2023-09-07 14:20 ` Nikunj Kela 2023-09-07 14:20 ` Nikunj Kela 2023-09-07 16:16 ` [PATCH 0/2] " Konrad Dybcio 2023-09-07 16:16 ` Konrad Dybcio 2023-09-07 22:32 ` Nikunj Kela 2023-09-07 22:32 ` Nikunj Kela 2023-09-11 19:43 ` [PATCH v4 0/4] Add qcom hvc/shmem transport support Nikunj Kela 2023-09-11 19:43 ` Nikunj Kela 2023-09-11 19:43 ` [PATCH v4 1/4] firmware: arm_scmi: Add polling support for completion in smc Nikunj Kela 2023-09-11 19:43 ` Nikunj Kela 2023-10-02 18:18 ` Brian Masney 2023-10-02 18:18 ` Brian Masney 2023-10-02 18:36 ` Nikunj Kela 2023-10-02 18:36 ` Nikunj Kela 2023-10-03 10:33 ` Sudeep Holla 2023-10-03 10:33 ` Sudeep Holla 2023-10-03 10:50 ` Cristian Marussi 2023-10-03 10:50 ` Cristian Marussi 2023-10-03 15:53 ` Nikunj Kela 2023-10-03 15:53 ` Nikunj Kela 2023-10-04 16:11 ` Sudeep Holla 2023-10-04 16:11 ` Sudeep Holla 2023-10-05 3:25 ` Nikunj Kela 2023-10-05 3:25 ` Nikunj Kela 2023-09-11 19:43 ` [PATCH v4 2/4] dt-bindings: arm: convert nested if-else construct to allOf Nikunj Kela 2023-09-11 19:43 ` Nikunj Kela 2023-09-11 19:43 ` [PATCH v4 3/4] dt-bindings: arm: Add new compatible for smc/hvc transport for SCMI Nikunj Kela 2023-09-11 19:43 ` Nikunj Kela 2023-10-03 10:44 ` Sudeep Holla 2023-10-03 10:44 ` Sudeep Holla 2023-10-03 15:59 ` Nikunj Kela 2023-10-03 15:59 ` Nikunj Kela 2023-10-04 15:53 ` Sudeep Holla 2023-10-04 15:53 ` Sudeep Holla 2023-10-05 21:51 ` Nikunj Kela 2023-10-05 21:51 ` Nikunj Kela 2023-09-11 19:43 ` [PATCH v4 4/4] firmware: arm_scmi: Add qcom hvc/shmem transport support Nikunj Kela 2023-09-11 19:43 ` Nikunj Kela 2023-10-02 18:34 ` Brian Masney 2023-10-02 18:34 ` Brian Masney 2023-10-02 18:39 ` Brian Masney 2023-10-02 18:39 ` Brian Masney 2023-10-02 18:45 ` Nikunj Kela 2023-10-02 18:45 ` Nikunj Kela 2023-10-02 18:42 ` Nikunj Kela 2023-10-02 18:42 ` Nikunj Kela 2023-10-03 10:48 ` Sudeep Holla 2023-10-03 10:48 ` Sudeep Holla 2023-10-03 11:19 ` Sudeep Holla 2023-10-03 11:19 ` Sudeep Holla 2023-10-03 16:16 ` Nikunj Kela 2023-10-03 16:16 ` Nikunj Kela 2023-10-04 16:06 ` Sudeep Holla 2023-10-04 16:06 ` Sudeep Holla 2023-10-04 17:48 ` Nikunj Kela 2023-10-04 17:48 ` Nikunj Kela 2023-10-05 22:20 ` Bjorn Andersson 2023-10-05 22:20 ` Bjorn Andersson 2023-10-05 22:33 ` Nikunj Kela 2023-10-05 22:33 ` Nikunj Kela 2023-10-06 7:26 ` Sudeep Holla 2023-10-06 7:26 ` Sudeep Holla 2023-09-18 15:01 ` [PATCH v4 0/4] " Nikunj Kela 2023-09-18 15:01 ` Nikunj Kela 2023-09-18 15:15 ` Sudeep Holla 2023-09-18 15:15 ` Sudeep Holla 2023-09-18 15:54 ` Brian Masney 2023-09-18 15:54 ` Brian Masney 2023-09-19 8:56 ` Sudeep Holla 2023-09-19 8:56 ` Sudeep Holla 2023-10-02 17:31 ` Nikunj Kela 2023-10-02 17:31 ` Nikunj Kela 2023-10-02 17:58 ` Cristian Marussi 2023-10-02 17:58 ` Cristian Marussi 2023-10-03 10:34 ` Sudeep Holla 2023-10-03 10:34 ` Sudeep Holla 2023-09-18 20:32 ` Krzysztof Kozlowski 2023-09-18 20:32 ` Krzysztof Kozlowski 2023-10-06 16:42 ` [PATCH v5 0/2] Add qcom smc/hvc " Nikunj Kela 2023-10-06 16:42 ` Nikunj Kela 2023-10-06 16:42 ` [PATCH v5 1/2] dt-bindings: arm: Add new compatible for smc/hvc transport for SCMI Nikunj Kela 2023-10-06 16:42 ` Nikunj Kela 2023-10-06 20:08 ` Brian Masney 2023-10-06 20:08 ` Brian Masney 2023-10-09 14:41 ` Sudeep Holla 2023-10-09 14:41 ` Sudeep Holla 2023-10-09 14:52 ` Nikunj Kela 2023-10-09 14:52 ` Nikunj Kela 2023-10-09 21:03 ` Konrad Dybcio 2023-10-09 21:03 ` Konrad Dybcio 2023-10-06 16:42 ` [PATCH v5 2/2] firmware: arm_scmi: Add qcom smc/hvc transport support Nikunj Kela 2023-10-06 16:42 ` Nikunj Kela 2023-10-06 20:17 ` Brian Masney 2023-10-06 20:17 ` Brian Masney 2023-10-09 14:47 ` Sudeep Holla 2023-10-09 14:47 ` Sudeep Holla 2023-10-09 14:59 ` Nikunj Kela 2023-10-09 14:59 ` Nikunj Kela 2023-10-09 15:29 ` Sudeep Holla 2023-10-09 15:29 ` Sudeep Holla 2023-10-09 17:49 ` Nikunj Kela 2023-10-09 17:49 ` Nikunj Kela 2023-10-09 19:08 ` Sudeep Holla 2023-10-09 19:08 ` Sudeep Holla 2023-10-09 19:16 ` Nikunj Kela 2023-10-09 19:16 ` Nikunj Kela 2023-10-09 19:14 ` [PATCH v6 0/2] " Nikunj Kela 2023-10-09 19:14 ` Nikunj Kela 2023-10-09 19:14 ` [PATCH v6 1/2] dt-bindings: arm: Add new compatible for smc/hvc transport for SCMI Nikunj Kela 2023-10-09 19:14 ` Nikunj Kela 2023-10-09 19:14 ` [PATCH v6 2/2] firmware: arm_scmi: Add qcom smc/hvc transport support Nikunj Kela 2023-10-09 19:14 ` Nikunj Kela 2023-10-10 10:42 ` Sudeep Holla 2023-10-10 10:42 ` Sudeep Holla 2023-10-10 10:21 ` [PATCH v6 0/2] " Sudeep Holla 2023-10-10 10:21 ` Sudeep Holla
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=e8399fcf-e0d8-cc31-d9a7-b0f4f7cc3e71@quicinc.com \ --to=quic_nkela@quicinc.com \ --cc=agross@kernel.org \ --cc=andersson@kernel.org \ --cc=conor+dt@kernel.org \ --cc=cristian.marussi@arm.com \ --cc=devicetree@vger.kernel.org \ --cc=konrad.dybcio@linaro.org \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=krzysztof.kozlowski@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=robh+dt@kernel.org \ --cc=sudeep.holla@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.