2015-11-15 10:19:45

by Saurabh Sengar

[permalink] [raw]
Subject: [PATCH] mfd: ssbi: removing unnecessary strcmp

passing the actual enum value for controller type from device tree,
instead of passing the string and converting it to enum

Signed-off-by: Saurabh Sengar <[email protected]>
---
Documentation/devicetree/bindings/arm/msm/ssbi.txt | 6 +++---
arch/arm/boot/dts/qcom-apq8064.dtsi | 3 ++-
arch/arm/boot/dts/qcom-ipq8064.dtsi | 3 ++-
arch/arm/boot/dts/qcom-msm8660.dtsi | 3 ++-
arch/arm/boot/dts/qcom-msm8960.dtsi | 3 ++-
drivers/mfd/ssbi.c | 17 ++++++-----------
include/dt-bindings/mfd/qcom,ssbi.h | 19 +++++++++++++++++++
7 files changed, 36 insertions(+), 18 deletions(-)
create mode 100644 include/dt-bindings/mfd/qcom,ssbi.h

diff --git a/Documentation/devicetree/bindings/arm/msm/ssbi.txt b/Documentation/devicetree/bindings/arm/msm/ssbi.txt
index 54fd5ce..61a37e0 100644
--- a/Documentation/devicetree/bindings/arm/msm/ssbi.txt
+++ b/Documentation/devicetree/bindings/arm/msm/ssbi.txt
@@ -10,9 +10,9 @@ These require the following properties:

- qcom,controller-type
indicates the SSBI bus variant the controller should use to talk
- with the slave device. This should be one of "ssbi", "ssbi2", or
- "pmic-arbiter". The type chosen is determined by the attached
- slave.
+ with the slave device. This should be one of MSM_SBI_CTRL_SSBI,
+ MSM_SBI_CTRL_SSBI2 or MSM_SBI_CTRL_PMIC_ARBITER.
+ The type chosen is determined by the attached slave.

The slave device should be the single child node of the ssbi device
with a compatible field.
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index a4c1762..e391a01 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -5,6 +5,7 @@
#include <dt-bindings/reset/qcom,gcc-msm8960.h>
#include <dt-bindings/clock/qcom,mmcc-msm8960.h>
#include <dt-bindings/soc/qcom,gsbi.h>
+#include <dt-bindings/mfd/qcom,ssbi.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
/ {
model = "Qualcomm APQ8064";
@@ -339,7 +340,7 @@
qcom,ssbi@500000 {
compatible = "qcom,ssbi";
reg = <0x00500000 0x1000>;
- qcom,controller-type = "pmic-arbiter";
+ qcom,controller-type = <MSM_SBI_CTRL_PMIC_ARBITER>;

pmicintc: pmic@0 {
compatible = "qcom,pm8921";
diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index fa69863..a3ba13a 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -3,6 +3,7 @@
#include "skeleton.dtsi"
#include <dt-bindings/clock/qcom,gcc-ipq806x.h>
#include <dt-bindings/clock/qcom,lcc-ipq806x.h>
+#include <dt-bindings/mfd/qcom,ssbi.h>
#include <dt-bindings/soc/qcom,gsbi.h>

/ {
@@ -307,7 +308,7 @@
qcom,ssbi@500000 {
compatible = "qcom,ssbi";
reg = <0x00500000 0x1000>;
- qcom,controller-type = "pmic-arbiter";
+ qcom,controller-type = <MSM_SBI_CTRL_PMIC_ARBITER>;
};

gcc: clock-controller@900000 {
diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi
index e5f7f33..51b28d2 100644
--- a/arch/arm/boot/dts/qcom-msm8660.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8660.dtsi
@@ -4,6 +4,7 @@

#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/clock/qcom,gcc-msm8660.h>
+#include <dt-bindings/mfd/qcom,ssbi.h>
#include <dt-bindings/soc/qcom,gsbi.h>

/ {
@@ -112,7 +113,7 @@
qcom,ssbi@500000 {
compatible = "qcom,ssbi";
reg = <0x500000 0x1000>;
- qcom,controller-type = "pmic-arbiter";
+ qcom,controller-type = <MSM_SBI_CTRL_PMIC_ARBITER>;

pmicintc: pmic@0 {
compatible = "qcom,pm8058";
diff --git a/arch/arm/boot/dts/qcom-msm8960.dtsi b/arch/arm/boot/dts/qcom-msm8960.dtsi
index 134cd91..d01cee5 100644
--- a/arch/arm/boot/dts/qcom-msm8960.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8960.dtsi
@@ -5,6 +5,7 @@
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/clock/qcom,gcc-msm8960.h>
#include <dt-bindings/mfd/qcom-rpm.h>
+#include <dt-bindings/mfd/qcom,ssbi.h>
#include <dt-bindings/soc/qcom,gsbi.h>

/ {
@@ -171,7 +172,7 @@
qcom,ssbi@500000 {
compatible = "qcom,ssbi";
reg = <0x500000 0x1000>;
- qcom,controller-type = "pmic-arbiter";
+ qcom,controller-type = <MSM_SBI_CTRL_PMIC_ARBITER>;

pmicintc: pmic@0 {
compatible = "qcom,pm8921";
diff --git a/drivers/mfd/ssbi.c b/drivers/mfd/ssbi.c
index 27986f6..cf4d983 100644
--- a/drivers/mfd/ssbi.c
+++ b/drivers/mfd/ssbi.c
@@ -274,7 +274,7 @@ static int ssbi_probe(struct platform_device *pdev)
struct device_node *np = pdev->dev.of_node;
struct resource *mem_res;
struct ssbi *ssbi;
- const char *type;
+ u32 type;

ssbi = devm_kzalloc(&pdev->dev, sizeof(*ssbi), GFP_KERNEL);
if (!ssbi)
@@ -287,22 +287,17 @@ static int ssbi_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, ssbi);

- type = of_get_property(np, "qcom,controller-type", NULL);
- if (type == NULL) {
+ if (of_property_read_u32(np, "qcom,controller-type", &type)) {
dev_err(&pdev->dev, "Missing qcom,controller-type property\n");
return -EINVAL;
}
- dev_info(&pdev->dev, "SSBI controller type: '%s'\n", type);
- if (strcmp(type, "ssbi") == 0)
- ssbi->controller_type = MSM_SBI_CTRL_SSBI;
- else if (strcmp(type, "ssbi2") == 0)
- ssbi->controller_type = MSM_SBI_CTRL_SSBI2;
- else if (strcmp(type, "pmic-arbiter") == 0)
- ssbi->controller_type = MSM_SBI_CTRL_PMIC_ARBITER;
- else {
+
+ dev_info(&pdev->dev, "SSBI controller type: '%d'\n", type);
+ if (type > MSM_SBI_CTRL_PMIC_ARBITER) {
dev_err(&pdev->dev, "Unknown qcom,controller-type\n");
return -EINVAL;
}
+ ssbi->controller_type = type;

if (ssbi->controller_type == MSM_SBI_CTRL_PMIC_ARBITER) {
ssbi->read = ssbi_pa_read_bytes;
diff --git a/include/dt-bindings/mfd/qcom,ssbi.h b/include/dt-bindings/mfd/qcom,ssbi.h
new file mode 100644
index 0000000..b6bf5bc
--- /dev/null
+++ b/include/dt-bindings/mfd/qcom,ssbi.h
@@ -0,0 +1,19 @@
+/* Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#ifndef __DT_MFD_QCOM_SSBI_H
+#define __DT_MFD_QCOM_SSBI_H
+
+#define MSM_SBI_CTRL_SSBI 0
+#define MSM_SBI_CTRL_SSBI2 1
+#define MSM_SBI_CTRL_PMIC_ARBITER 2
+
+#endif
--
1.9.1


2015-11-16 12:51:00

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH] mfd: ssbi: removing unnecessary strcmp

On Sun, 15 Nov 2015, Saurabh Sengar wrote:

> passing the actual enum value for controller type from device tree,
> instead of passing the string and converting it to enum

Initially I would have probably preferred this version, but I can't
think of a good enough reason to break the current ABI in order to
replace it.

> Signed-off-by: Saurabh Sengar <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/msm/ssbi.txt | 6 +++---
> arch/arm/boot/dts/qcom-apq8064.dtsi | 3 ++-
> arch/arm/boot/dts/qcom-ipq8064.dtsi | 3 ++-
> arch/arm/boot/dts/qcom-msm8660.dtsi | 3 ++-
> arch/arm/boot/dts/qcom-msm8960.dtsi | 3 ++-
> drivers/mfd/ssbi.c | 17 ++++++-----------
> include/dt-bindings/mfd/qcom,ssbi.h | 19 +++++++++++++++++++
> 7 files changed, 36 insertions(+), 18 deletions(-)
> create mode 100644 include/dt-bindings/mfd/qcom,ssbi.h
>
> diff --git a/Documentation/devicetree/bindings/arm/msm/ssbi.txt b/Documentation/devicetree/bindings/arm/msm/ssbi.txt
> index 54fd5ce..61a37e0 100644
> --- a/Documentation/devicetree/bindings/arm/msm/ssbi.txt
> +++ b/Documentation/devicetree/bindings/arm/msm/ssbi.txt
> @@ -10,9 +10,9 @@ These require the following properties:
>
> - qcom,controller-type
> indicates the SSBI bus variant the controller should use to talk
> - with the slave device. This should be one of "ssbi", "ssbi2", or
> - "pmic-arbiter". The type chosen is determined by the attached
> - slave.
> + with the slave device. This should be one of MSM_SBI_CTRL_SSBI,
> + MSM_SBI_CTRL_SSBI2 or MSM_SBI_CTRL_PMIC_ARBITER.
> + The type chosen is determined by the attached slave.
>
> The slave device should be the single child node of the ssbi device
> with a compatible field.
> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> index a4c1762..e391a01 100644
> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> @@ -5,6 +5,7 @@
> #include <dt-bindings/reset/qcom,gcc-msm8960.h>
> #include <dt-bindings/clock/qcom,mmcc-msm8960.h>
> #include <dt-bindings/soc/qcom,gsbi.h>
> +#include <dt-bindings/mfd/qcom,ssbi.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> / {
> model = "Qualcomm APQ8064";
> @@ -339,7 +340,7 @@
> qcom,ssbi@500000 {
> compatible = "qcom,ssbi";
> reg = <0x00500000 0x1000>;
> - qcom,controller-type = "pmic-arbiter";
> + qcom,controller-type = <MSM_SBI_CTRL_PMIC_ARBITER>;
>
> pmicintc: pmic@0 {
> compatible = "qcom,pm8921";
> diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
> index fa69863..a3ba13a 100644
> --- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
> @@ -3,6 +3,7 @@
> #include "skeleton.dtsi"
> #include <dt-bindings/clock/qcom,gcc-ipq806x.h>
> #include <dt-bindings/clock/qcom,lcc-ipq806x.h>
> +#include <dt-bindings/mfd/qcom,ssbi.h>
> #include <dt-bindings/soc/qcom,gsbi.h>
>
> / {
> @@ -307,7 +308,7 @@
> qcom,ssbi@500000 {
> compatible = "qcom,ssbi";
> reg = <0x00500000 0x1000>;
> - qcom,controller-type = "pmic-arbiter";
> + qcom,controller-type = <MSM_SBI_CTRL_PMIC_ARBITER>;
> };
>
> gcc: clock-controller@900000 {
> diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi
> index e5f7f33..51b28d2 100644
> --- a/arch/arm/boot/dts/qcom-msm8660.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi
> @@ -4,6 +4,7 @@
>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/clock/qcom,gcc-msm8660.h>
> +#include <dt-bindings/mfd/qcom,ssbi.h>
> #include <dt-bindings/soc/qcom,gsbi.h>
>
> / {
> @@ -112,7 +113,7 @@
> qcom,ssbi@500000 {
> compatible = "qcom,ssbi";
> reg = <0x500000 0x1000>;
> - qcom,controller-type = "pmic-arbiter";
> + qcom,controller-type = <MSM_SBI_CTRL_PMIC_ARBITER>;
>
> pmicintc: pmic@0 {
> compatible = "qcom,pm8058";
> diff --git a/arch/arm/boot/dts/qcom-msm8960.dtsi b/arch/arm/boot/dts/qcom-msm8960.dtsi
> index 134cd91..d01cee5 100644
> --- a/arch/arm/boot/dts/qcom-msm8960.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8960.dtsi
> @@ -5,6 +5,7 @@
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/clock/qcom,gcc-msm8960.h>
> #include <dt-bindings/mfd/qcom-rpm.h>
> +#include <dt-bindings/mfd/qcom,ssbi.h>
> #include <dt-bindings/soc/qcom,gsbi.h>
>
> / {
> @@ -171,7 +172,7 @@
> qcom,ssbi@500000 {
> compatible = "qcom,ssbi";
> reg = <0x500000 0x1000>;
> - qcom,controller-type = "pmic-arbiter";
> + qcom,controller-type = <MSM_SBI_CTRL_PMIC_ARBITER>;
>
> pmicintc: pmic@0 {
> compatible = "qcom,pm8921";
> diff --git a/drivers/mfd/ssbi.c b/drivers/mfd/ssbi.c
> index 27986f6..cf4d983 100644
> --- a/drivers/mfd/ssbi.c
> +++ b/drivers/mfd/ssbi.c
> @@ -274,7 +274,7 @@ static int ssbi_probe(struct platform_device *pdev)
> struct device_node *np = pdev->dev.of_node;
> struct resource *mem_res;
> struct ssbi *ssbi;
> - const char *type;
> + u32 type;
>
> ssbi = devm_kzalloc(&pdev->dev, sizeof(*ssbi), GFP_KERNEL);
> if (!ssbi)
> @@ -287,22 +287,17 @@ static int ssbi_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, ssbi);
>
> - type = of_get_property(np, "qcom,controller-type", NULL);
> - if (type == NULL) {
> + if (of_property_read_u32(np, "qcom,controller-type", &type)) {
> dev_err(&pdev->dev, "Missing qcom,controller-type property\n");
> return -EINVAL;
> }
> - dev_info(&pdev->dev, "SSBI controller type: '%s'\n", type);
> - if (strcmp(type, "ssbi") == 0)
> - ssbi->controller_type = MSM_SBI_CTRL_SSBI;
> - else if (strcmp(type, "ssbi2") == 0)
> - ssbi->controller_type = MSM_SBI_CTRL_SSBI2;
> - else if (strcmp(type, "pmic-arbiter") == 0)
> - ssbi->controller_type = MSM_SBI_CTRL_PMIC_ARBITER;
> - else {
> +
> + dev_info(&pdev->dev, "SSBI controller type: '%d'\n", type);
> + if (type > MSM_SBI_CTRL_PMIC_ARBITER) {
> dev_err(&pdev->dev, "Unknown qcom,controller-type\n");
> return -EINVAL;
> }
> + ssbi->controller_type = type;
>
> if (ssbi->controller_type == MSM_SBI_CTRL_PMIC_ARBITER) {
> ssbi->read = ssbi_pa_read_bytes;
> diff --git a/include/dt-bindings/mfd/qcom,ssbi.h b/include/dt-bindings/mfd/qcom,ssbi.h
> new file mode 100644
> index 0000000..b6bf5bc
> --- /dev/null
> +++ b/include/dt-bindings/mfd/qcom,ssbi.h
> @@ -0,0 +1,19 @@
> +/* Copyright (c) 2015, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +#ifndef __DT_MFD_QCOM_SSBI_H
> +#define __DT_MFD_QCOM_SSBI_H
> +
> +#define MSM_SBI_CTRL_SSBI 0
> +#define MSM_SBI_CTRL_SSBI2 1
> +#define MSM_SBI_CTRL_PMIC_ARBITER 2
> +
> +#endif

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog