| Message ID | 20200706054732.99387-6-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor SDEI client driver | expand |
Hi Gavin, On 06/07/2020 06:47, Gavin Shan wrote: > There are some logics in sdei_get_conduit() can be safely dropped: > > * There are no associated device node with the platform device, > so it's pointless to check on it. This is for DT. Its looking up the conduit in the binding. See Documentation/devicetree/bindings/arm/firmware/sdei.txt. > * ACPI functionality has been verified when the platform device > is added in sdei_init(). So it's unnecessary to recheck. This is so that a kernel built without ACPI support can have all that code removed at compile time. The check appears repeatedly to ensure the compiler knows this is dead code. Thanks, James
Hi James, On 7/22/20 6:42 AM, James Morse wrote: > On 06/07/2020 06:47, Gavin Shan wrote: >> There are some logics in sdei_get_conduit() can be safely dropped: >> >> * There are no associated device node with the platform device, >> so it's pointless to check on it. > > This is for DT. Its looking up the conduit in the binding. See > Documentation/devicetree/bindings/arm/firmware/sdei.txt. > Yeah, I obviously missed the DT case. > >> * ACPI functionality has been verified when the platform device >> is added in sdei_init(). So it's unnecessary to recheck. > > This is so that a kernel built without ACPI support can have all that code removed at > compile time. The check appears repeatedly to ensure the compiler knows this is dead code. > Yes, this patch isn't meaningful and should be dropped except to remove the duplicate checker on ACPI enablement in sdei_get_conduit() if (np) { : } else if (IS_ENABLED(CONFIG_ACPI) && !acpi_disabled) { : } I will have new patch to replace this one, to remove the duplicate checker in v2. Thanks, Gavin
diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c index 7e7b26b1f91b..ca0e3f5eb907 100644 --- a/drivers/firmware/arm_sdei.c +++ b/drivers/firmware/arm_sdei.c @@ -933,49 +933,15 @@ int sdei_unregister_ghes(struct ghes *ghes) return err; } -static int sdei_get_conduit(struct platform_device *pdev) -{ - const char *method; - struct device_node *np = pdev->dev.of_node; - - sdei_firmware_call = NULL; - if (np) { - if (of_property_read_string(np, "method", &method)) { - pr_warn("missing \"method\" property\n"); - return SMCCC_CONDUIT_NONE; - } - - if (!strcmp("hvc", method)) { - sdei_firmware_call = &sdei_smccc_hvc; - return SMCCC_CONDUIT_HVC; - } else if (!strcmp("smc", method)) { - sdei_firmware_call = &sdei_smccc_smc; - return SMCCC_CONDUIT_SMC; - } - - pr_warn("invalid \"method\" property: %s\n", method); - } else if (IS_ENABLED(CONFIG_ACPI) && !acpi_disabled) { - if (acpi_psci_use_hvc()) { - sdei_firmware_call = &sdei_smccc_hvc; - return SMCCC_CONDUIT_HVC; - } else { - sdei_firmware_call = &sdei_smccc_smc; - return SMCCC_CONDUIT_SMC; - } - } - - return SMCCC_CONDUIT_NONE; -} - static int sdei_probe(struct platform_device *pdev) { int err; u64 ver = 0; int conduit; - conduit = sdei_get_conduit(pdev); - if (!sdei_firmware_call) - return 0; + conduit = acpi_psci_use_hvc() ? SMCCC_CONDUIT_HVC : SMCCC_CONDUIT_SMC; + sdei_firmware_call = acpi_psci_use_hvc() ? + &sdei_smccc_hvc : &sdei_smccc_smc; err = sdei_api_get_version(&ver); if (err == -EOPNOTSUPP)
There are some logics in sdei_get_conduit() can be safely dropped: * There are no associated device node with the platform device, so it's pointless to check on it. * ACPI functionality has been verified when the platform device is added in sdei_init(). So it's unnecessary to recheck. With above logics dropped, sdei_get_conduit() is quite simple and it's not worthy to maintain a separate function. The logic is merged to sdei_probe() as it's the only caller. Signed-off-by: Gavin Shan <gshan@redhat.com> --- drivers/firmware/arm_sdei.c | 40 +++---------------------------------- 1 file changed, 3 insertions(+), 37 deletions(-)