[05/14] drivers/firmware/sdei: Remove sdei_get_conduit()
diff mbox series

Message ID 20200706054732.99387-6-gshan@redhat.com
State New
Headers show
Series
  • Refactor SDEI client driver
Related show

Commit Message

Gavin Shan July 6, 2020, 5:47 a.m. UTC
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(-)

Comments

James Morse July 21, 2020, 8:42 p.m. UTC | #1
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
Gavin Shan July 22, 2020, 3:50 a.m. UTC | #2
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

Patch
diff mbox series

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)